Re: [WIP] RE: DECLARE STATEMENT setting up a connection in ECPG

From: "Ideriha, Takeshi" <ideriha(dot)takeshi(at)jp(dot)fujitsu(dot)com>
To: Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>
Cc: "Okano, Naoki" <okano(dot)naoki(at)jp(dot)fujitsu(dot)com>, Michael Meskes <meskes(at)postgresql(dot)org>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [WIP] RE: DECLARE STATEMENT setting up a connection in ECPG
Date: 2017-03-22 08:57:15
Message-ID: 4E72940DA2BF16479384A86D54D0988A565AC2D3@G01JPEXMBKW04
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi, thank you very much for reviewing.
Attached is v6 patch.

>There was a minor conflict in applying 004_declareXX patch.

I rebased 004_declareStmt_test_v6.patch.

>Some comments in 001_declareStmt_preproc_v5.patch:

>+ if (INFORMIX_MODE)
>+ {
>+ if (pg_strcasecmp(stmt+strlen("close "), "database") == 0)
>+ {
>+ if (connection)
>+ mmerror(PARSE_ERROR, ET_ERROR, "AT option not allowed in CLOSE DATABASE statement");
+
>+ fprintf(base_yyout, "{ ECPGdisconnect(__LINE__, \"CURRENT\");");
>+ whenever_action(2);
>+ free(stmt);
>+ break;
>+ }
>+ }

>The same code block is present in the stmtClosePortalStmt condition to verify the close
>statement. Because of the above code addition, the code present in stmtClosePortalStmt
>is a dead code. So remove the code present in stmtClosePortalStmt or try to reuse it.

I remove almost all the stmtClosePortalStmt code
and just leave the interface which actually does nothing not to affect other codes.

But I'm not sure this implementation is good or not.
Maybe I should completely remove stmtClosePortalStmt code
and rename ClosePortalStmtCLOSEcursor_name to stmtClosePortalStmt to make code easier to understand.

>+static void
>+output_cursor_name(char *str)

>This function needs some comments to explain the code flow for better understanding.

I add some explanation that output_cursor_name() filters escape sequences.

>+/*
>+ * Translate the EXEC SQL DECLARE STATEMENT into ECPGdeclare function
>+ */

>How about using Transform instead of Translate? (similar references also)

I changed two comments with the word Translate.

regards,
Ideriha, Takeshi

Attachment Content-Type Size
001_declareStmt_preproc_v6.patch application/octet-stream 18.1 KB
002_declareStmt_ecpglib_v6.patch application/octet-stream 23.3 KB
003_declareStmt_doc_v6.patch application/octet-stream 4.2 KB
004_declareStmt_test_v6.patch application/octet-stream 71.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message vinayak 2017-03-22 09:11:58 Re: ANALYZE command progress checker
Previous Message Craig Ringer 2017-03-22 08:53:14 Re: Logical decoding on standby