Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE

At Fri, 2 Jul 2021 12:53:02 +0000, "kuroda(dot)hayato(at)fujitsu(dot)com" <kuroda(dot)hayato(at)fujitsu(dot)com> wrote in
> Dear Hackers,
>
> I revised my patch.

Thanks for the new version.

> > However, I perfectly agree that it's very difficult for users to find a problem from the message.
> > I will try to add information to it in the next patch.
>
> I added such a message and some tests, but I began to think this is strange.
> Now I'm wondering why the connection is checked in some DESCRIPTOR-related
> statements? In my understanding connection name is not used in ECPGallocate_desc(),
> ECPGdeallocate_desc(), ECPGget_desc() and so on.
> Hence I think lookup_descriptor() and drop_descriptor() can be removed.
> This idea can solve your first argument.

Maybe (pre)compile-time check is needed for the descriptor names.
Otherwise we don't notice some of the names are spelled wrongly until
runtime. If it were a string we can live without the check but it is
seemingly an identifier so it is strange that it is not detected at
compile-time. I guess that it is the motivation for the check.

What makes the story complex is that connection matters in the
relation between DESCRIBE and GET DESCRIPTOR. (However, connection
doesn't matter in ALLOCATE DESCRIPTOR..) Maybe the check involves
connection for this reason.

Since we don't allow descriptors with the same name even if they are
for the different connections, I think we can set the current
connection if any (which is set either by AT option or statement-bound
one) to i->connection silently if i->connection is NULL in
lookup_descriptor(). What do you think about this?

=====
I noticed the following behavior.

> EXEC SQL AT conn1 DECLARE stmt STATEMENT;
> EXEC SQL DESCRIBE stmt INTO SQL DESCRIPTOR mydesc;
> EXEC SQL SET CONNECTION conn2;
ERROR: AT option not allowed in SET CONNECTION STATEMENT

connection is "conn1" at the error time. The parser relies on
output_statement and friends for connection name reset. So the rules
that don't call the functions need to reset it by themselves.

Similary, the following sequence doesn't yield an error, which is
expected.

> EXEC SQL AT conn1 DECLARE stmt STATEMENT;
> EXEC SQL AT conn2 EXECUTE stmt INTO ..;

In this case "conn2" set by the AT option is silently overwritten with
"conn1" by check_declared_list(). I think we should reject AT option
(with a different connection) in that case.

> > You're right. This is very stupid program. I'll combine them into one.
>
> Check_declared_list() was moved to stmt:ECPGDescribe rule.
> Some similar rules still remain in ecpg.trailer, but INPUT/OUTPUT statements have
> different rules and actions and I cannot combine well.

I'm not sure what you exactly mean by the "INPUT/OUTPUT statements"
but the change of DESCRIBE INPUT/OUTPUT looks fine to me.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center