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

From: "wangsh(dot)fnst(at)fujitsu(dot)com" <wangsh(dot)fnst(at)fujitsu(dot)com>
To: "kuroda(dot)hayato(at)fujitsu(dot)com" <kuroda(dot)hayato(at)fujitsu(dot)com>
Cc: "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Michael Paquier <michael(at)paquier(dot)xyz>, 'Kyotaro Horiguchi' <horikyota(dot)ntt(at)gmail(dot)com>, Michael Meskes <meskes(at)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Subject: RE: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
Date: 2021-07-20 08:02:44
Message-ID: OSBPR01MB42141999ED8EFDD4D8FDA42CF2E29@OSBPR01MB4214.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi kuroda-san:

I find another problem about declare statement. The test source looks like:
> EXEC SQL AT con1 DECLARE stmt STATEMENT;
> EXEC SQL PREPARE stmt AS SELECT version();
> EXEC SQL DECLARE cur CURSOR FOR stmt;
> EXEC SQL WHENEVER SQLERROR STOP;

The outout about ecpg:
>test.pgc:14: ERROR: AT option not allowed in WHENEVER statement

After a simple research, I found that after calling function check_declared_list,
the variable connection will be updated, but in some case(e.g. ECPGCursorStmt)
reset connection is missing.

I'm not sure, but how about modify the ecpg.trailer:
> tatement: ecpgstart at toplevel_stmt ';' { connection = NULL; }
> | ecpgstart toplevel_stmt ';'

I think there are lots of 'connection = NULL;' in source, and we should find a
good location to reset the connection.

Best regards.
Shenhao Wang

-----Original Message-----
From: kuroda(dot)hayato(at)fujitsu(dot)com <kuroda(dot)hayato(at)fujitsu(dot)com>
Sent: Monday, July 12, 2021 12:05 PM
To: 'Kyotaro Horiguchi' <horikyota(dot)ntt(at)gmail(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: RE: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE

Dear Horiguchi-san,

Thank you for reviewing! I attached new version.
Sorry for delaying reply.

> 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 tried to implement. Is it correct?

> 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.

Oh, I didn't notice that. Fixed.
I'm wondering why a output function is not implemented, like output_describe_statement(),
but anyway I put a connection reset in ecpg.addons.

> 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.

Actually this comes from Oracle's specification. Pro*C precompiler
overwrite their connection in the situation, hence I followed that.
But I agree this might be confused and I added the warning report.
How do you think? Is it still strange?

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2021-07-20 08:29:28 Re: row filtering for logical replication
Previous Message Michael Paquier 2021-07-20 07:54:47 Re: OpenSSL 3.0.0 compatibility