Re: [WIP] RE: [HACKERS] DECLARE STATEMENT setting up a connection in ECPG - Mailing list 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.