Обсуждение: ecpg: issue related to preprocessor directives
Hi All,
When the following ecpg program having preprocessor directives is compiled, the output produced is not correct.
/* test program */
exec sql define itype 1;
int main(void)
{
exec sql begin declare section;
exec sql ifdef itype;
int var1;
exec sql elif ntype;
numeric var1;
exec sql else;
float var1;
exec sql endif;
exec sql end declare section;
}
Here is the output produced by th ecpg pre-compiler when above program is compiled:
int main(void)
{
/* exec sql begin declare section */
#line 8 "2.pgc"
int var1 ;
#line 12 "2.pgc"
float var1 ;
/* exec sql end declare section */
#line 14 "2.pgc"
}
As seen from above output, both exec sql ifdef and exec sql else block got compiled which is wrong. If the above output is further compiled using gcc compiler, the compilation would fail.
When the following ecpg program having preprocessor directives is compiled, the output produced is not correct.
/* test program */
exec sql define itype 1;
int main(void)
{
exec sql begin declare section;
exec sql ifdef itype;
int var1;
exec sql elif ntype;
numeric var1;
exec sql else;
float var1;
exec sql endif;
exec sql end declare section;
}
Here is the output produced by th ecpg pre-compiler when above program is compiled:
int main(void)
{
/* exec sql begin declare section */
#line 8 "2.pgc"
int var1 ;
#line 12 "2.pgc"
float var1 ;
/* exec sql end declare section */
#line 14 "2.pgc"
}
As seen from above output, both exec sql ifdef and exec sql else block got compiled which is wrong. If the above output is further compiled using gcc compiler, the compilation would fail.
Ashutosh Sharma <ashu.coek88@gmail.com> writes:
> When the following ecpg program having preprocessor directives is compiled,
> the output produced is not correct.
> ...
> As seen from above output, both exec sql ifdef and exec sql else block got
> compiled which is wrong. If the above output is further compiled using gcc
> compiler, the compilation would fail.
Looking at pgc.l, it seems that 'elif' is treated as though it were
'endif' followed by 'ifdef', which of course completely loses the
expected property that a previous successful branch would keep the
elif branch from being expanded.
While this doesn't look terribly hard to fix, I'm a little disturbed
by the fact that the existing semantics seem to date back to 1999
(b57b0e044). We're probably risking breaking existing app code if
we change it. I think we *should* change it, of course, but I'm kind
of inclined not to back-patch.
regards, tom lane
I wrote:
> Looking at pgc.l, it seems that 'elif' is treated as though it were
> 'endif' followed by 'ifdef', which of course completely loses the
> expected property that a previous successful branch would keep the
> elif branch from being expanded.
> While this doesn't look terribly hard to fix, I'm a little disturbed
> by the fact that the existing semantics seem to date back to 1999
> (b57b0e044). We're probably risking breaking existing app code if
> we change it. I think we *should* change it, of course, but I'm kind
> of inclined not to back-patch.
Here's a proposed patch, which also clarifies the documentation,
which seemed a bit confused/misleading to me.
As stated, I'm not sure it's wise to back-patch this aggressively
... but maybe it'd be okay to squeeze it into v13?
regards, tom lane
diff --git a/doc/src/sgml/ecpg.sgml b/doc/src/sgml/ecpg.sgml
index 106ae0984e..ecdc417d0b 100644
--- a/doc/src/sgml/ecpg.sgml
+++ b/doc/src/sgml/ecpg.sgml
@@ -5695,7 +5695,7 @@ EXEC SQL UPDATE Tbl SET col = MYNUMBER;
</sect2>
<sect2 id="ecpg-ifdef">
- <title>ifdef, ifndef, else, elif, and endif Directives</title>
+ <title>ifdef, ifndef, elif, else, and endif Directives</title>
<para>
You can use the following directives to compile code sections conditionally:
@@ -5705,7 +5705,7 @@ EXEC SQL UPDATE Tbl SET col = MYNUMBER;
<listitem>
<para>
Checks a <replaceable>name</replaceable> and processes subsequent lines if
- <replaceable>name</replaceable> has been created with <literal>EXEC SQL define
+ <replaceable>name</replaceable> has been defined via <literal>EXEC SQL define
<replaceable>name</replaceable></literal>.
</para>
</listitem>
@@ -5716,30 +5716,40 @@ EXEC SQL UPDATE Tbl SET col = MYNUMBER;
<listitem>
<para>
Checks a <replaceable>name</replaceable> and processes subsequent lines if
- <replaceable>name</replaceable> has <emphasis>not</emphasis> been created with
+ <replaceable>name</replaceable> has <emphasis>not</emphasis> been defined via
<literal>EXEC SQL define <replaceable>name</replaceable></literal>.
</para>
</listitem>
</varlistentry>
<varlistentry>
- <term><literal>EXEC SQL else;</literal></term>
+ <term><literal>EXEC SQL elif <replaceable>name</replaceable>;</literal></term>
<listitem>
<para>
- Starts processing an alternative section to a section introduced by
- either <literal>EXEC SQL ifdef <replaceable>name</replaceable></literal> or
- <literal>EXEC SQL ifndef <replaceable>name</replaceable></literal>.
+ Begins an alternative section after an
+ <literal>EXEC SQL ifdef <replaceable>name</replaceable></literal> or
+ <literal>EXEC SQL ifndef <replaceable>name</replaceable></literal>
+ directive. Any number of <literal>elif</literal> sections can appear.
+ Lines following an <literal>elif</literal> will be processed
+ if <replaceable>name</replaceable> has been
+ defined <emphasis>and</emphasis> no previous section of the same
+ <literal>ifdef</literal>/<literal>ifndef</literal>...<literal>endif</literal>
+ construct has been processed.
</para>
</listitem>
</varlistentry>
<varlistentry>
- <term><literal>EXEC SQL elif <replaceable>name</replaceable>;</literal></term>
+ <term><literal>EXEC SQL else;</literal></term>
<listitem>
<para>
- Checks <replaceable>name</replaceable> and starts an alternative section if
- <replaceable>name</replaceable> has been created with <literal>EXEC SQL define
- <replaceable>name</replaceable></literal>.
+ Begins an optional, final alternative section after an
+ <literal>EXEC SQL ifdef <replaceable>name</replaceable></literal> or
+ <literal>EXEC SQL ifndef <replaceable>name</replaceable></literal>
+ directive. Subsequent lines will be processed if no previous section
+ of the same
+ <literal>ifdef</literal>/<literal>ifndef</literal>...<literal>endif</literal>
+ construct has been processed.
</para>
</listitem>
</varlistentry>
@@ -5748,7 +5758,9 @@ EXEC SQL UPDATE Tbl SET col = MYNUMBER;
<term><literal>EXEC SQL endif;</literal></term>
<listitem>
<para>
- Ends an alternative section.
+ Ends an
+ <literal>ifdef</literal>/<literal>ifndef</literal>...<literal>endif</literal>
+ construct. Subsequent lines are processed normally.
</para>
</listitem>
</varlistentry>
@@ -5756,14 +5768,20 @@ EXEC SQL UPDATE Tbl SET col = MYNUMBER;
</para>
<para>
- Example:
+ <literal>ifdef</literal>/<literal>ifndef</literal>...<literal>endif</literal>
+ constructs can be nested, up to 127 levels deep.
+ </para>
+
+ <para>
+ This example will compile exactly one of the three <literal>SET
+ TIMEZONE</literal> commands:
<programlisting>
-EXEC SQL ifndef TZVAR;
-EXEC SQL SET TIMEZONE TO 'GMT';
+EXEC SQL ifdef TZVAR;
+EXEC SQL SET TIMEZONE TO TZVAR;
EXEC SQL elif TZNAME;
EXEC SQL SET TIMEZONE TO TZNAME;
EXEC SQL else;
-EXEC SQL SET TIMEZONE TO TZVAR;
+EXEC SQL SET TIMEZONE TO 'GMT';
EXEC SQL endif;
</programlisting>
</para>
diff --git a/src/interfaces/ecpg/preproc/pgc.l b/src/interfaces/ecpg/preproc/pgc.l
index f6052798fd..514ea27a80 100644
--- a/src/interfaces/ecpg/preproc/pgc.l
+++ b/src/interfaces/ecpg/preproc/pgc.l
@@ -79,13 +79,27 @@ struct _yy_buffer
static char *old;
+/*
+ * Vars for handling ifdef/elif/endif constructs. preproc_tos is the current
+ * nesting depth of such constructs, and stacked_if_value[preproc_tos] is the
+ * state for the innermost level. (For convenience, stacked_if_value[0] is
+ * initialized as though we are in the active branch of some outermost if.)
+ * The active field is true if the current branch is active (being expanded).
+ * The saw_active field is true if we have found any successful branch,
+ * so that all subsequent branches of this level should be skipped.
+ * The else_branch field is true if we've found an 'else' (so that another
+ * 'else' or 'elif' at this level is an error.)
+ * ifcond is valid only while evaluating an if-condition; it's true if we
+ * are doing ifdef, false if ifndef.
+ */
#define MAX_NESTED_IF 128
static short preproc_tos;
-static short ifcond;
+static bool ifcond;
static struct _if_value
{
- short condition;
- short else_branch;
+ bool active;
+ bool saw_active;
+ bool else_branch;
} stacked_if_value[MAX_NESTED_IF];
%}
@@ -1165,11 +1179,26 @@ cppline {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
return S_ANYTHING;
}
}
-<C,xskip>{exec_sql}{ifdef}{space}* { ifcond = true; BEGIN(xcond); }
+<C,xskip>{exec_sql}{ifdef}{space}* {
+ if (preproc_tos >= MAX_NESTED_IF-1)
+ mmfatal(PARSE_ERROR, "too many nested EXEC SQL IFDEF conditions");
+ preproc_tos++;
+ stacked_if_value[preproc_tos].active = false;
+ stacked_if_value[preproc_tos].saw_active = false;
+ stacked_if_value[preproc_tos].else_branch = false;
+ ifcond = true;
+ BEGIN(xcond);
+ }
<C,xskip>{informix_special}{ifdef}{space}* {
/* are we simulating Informix? */
if (INFORMIX_MODE)
{
+ if (preproc_tos >= MAX_NESTED_IF-1)
+ mmfatal(PARSE_ERROR, "too many nested EXEC SQL IFDEF conditions");
+ preproc_tos++;
+ stacked_if_value[preproc_tos].active = false;
+ stacked_if_value[preproc_tos].saw_active = false;
+ stacked_if_value[preproc_tos].else_branch = false;
ifcond = true;
BEGIN(xcond);
}
@@ -1179,11 +1208,26 @@ cppline {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
return S_ANYTHING;
}
}
-<C,xskip>{exec_sql}{ifndef}{space}* { ifcond = false; BEGIN(xcond); }
+<C,xskip>{exec_sql}{ifndef}{space}* {
+ if (preproc_tos >= MAX_NESTED_IF-1)
+ mmfatal(PARSE_ERROR, "too many nested EXEC SQL IFDEF conditions");
+ preproc_tos++;
+ stacked_if_value[preproc_tos].active = false;
+ stacked_if_value[preproc_tos].saw_active = false;
+ stacked_if_value[preproc_tos].else_branch = false;
+ ifcond = false;
+ BEGIN(xcond);
+ }
<C,xskip>{informix_special}{ifndef}{space}* {
/* are we simulating Informix? */
if (INFORMIX_MODE)
{
+ if (preproc_tos >= MAX_NESTED_IF-1)
+ mmfatal(PARSE_ERROR, "too many nested EXEC SQL IFDEF conditions");
+ preproc_tos++;
+ stacked_if_value[preproc_tos].active = false;
+ stacked_if_value[preproc_tos].saw_active = false;
+ stacked_if_value[preproc_tos].else_branch = false;
ifcond = false;
BEGIN(xcond);
}
@@ -1193,16 +1237,13 @@ cppline {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
return S_ANYTHING;
}
}
-<C,xskip>{exec_sql}{elif}{space}* { /* pop stack */
- if ( preproc_tos == 0 ) {
+<C,xskip>{exec_sql}{elif}{space}* {
+ if (preproc_tos == 0)
mmfatal(PARSE_ERROR, "missing matching \"EXEC SQL IFDEF\" / \"EXEC SQL IFNDEF\"");
- }
- else if ( stacked_if_value[preproc_tos].else_branch )
+ if (stacked_if_value[preproc_tos].else_branch)
mmfatal(PARSE_ERROR, "missing \"EXEC SQL ENDIF;\"");
- else
- preproc_tos--;
-
- ifcond = true; BEGIN(xcond);
+ ifcond = true;
+ BEGIN(xcond);
}
<C,xskip>{informix_special}{elif}{space}* {
/* are we simulating Informix? */
@@ -1210,11 +1251,8 @@ cppline {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
{
if (preproc_tos == 0)
mmfatal(PARSE_ERROR, "missing matching \"EXEC SQL IFDEF\" / \"EXEC SQL IFNDEF\"");
- else if (stacked_if_value[preproc_tos].else_branch)
+ if (stacked_if_value[preproc_tos].else_branch)
mmfatal(PARSE_ERROR, "missing \"EXEC SQL ENDIF;\"");
- else
- preproc_tos--;
-
ifcond = true;
BEGIN(xcond);
}
@@ -1226,16 +1264,19 @@ cppline {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
}
<C,xskip>{exec_sql}{else}{space}*";" { /* only exec sql endif pops the stack, so take care of duplicated 'else' */
- if (stacked_if_value[preproc_tos].else_branch)
+ if ( preproc_tos == 0 )
+ mmfatal(PARSE_ERROR, "missing matching \"EXEC SQL IFDEF\" / \"EXEC SQL IFNDEF\"");
+ else if (stacked_if_value[preproc_tos].else_branch)
mmfatal(PARSE_ERROR, "more than one EXEC SQL ELSE");
else
{
stacked_if_value[preproc_tos].else_branch = true;
- stacked_if_value[preproc_tos].condition =
- (stacked_if_value[preproc_tos-1].condition &&
- !stacked_if_value[preproc_tos].condition);
+ stacked_if_value[preproc_tos].active =
+ (stacked_if_value[preproc_tos-1].active &&
+ !stacked_if_value[preproc_tos].saw_active);
+ stacked_if_value[preproc_tos].saw_active = true;
- if (stacked_if_value[preproc_tos].condition)
+ if (stacked_if_value[preproc_tos].active)
BEGIN(C);
else
BEGIN(xskip);
@@ -1245,16 +1286,19 @@ cppline {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
/* are we simulating Informix? */
if (INFORMIX_MODE)
{
- if (stacked_if_value[preproc_tos].else_branch)
+ if ( preproc_tos == 0 )
+ mmfatal(PARSE_ERROR, "missing matching \"EXEC SQL IFDEF\" / \"EXEC SQL IFNDEF\"");
+ else if (stacked_if_value[preproc_tos].else_branch)
mmfatal(PARSE_ERROR, "more than one EXEC SQL ELSE");
else
{
stacked_if_value[preproc_tos].else_branch = true;
- stacked_if_value[preproc_tos].condition =
- (stacked_if_value[preproc_tos-1].condition &&
- !stacked_if_value[preproc_tos].condition);
+ stacked_if_value[preproc_tos].active =
+ (stacked_if_value[preproc_tos-1].active &&
+ !stacked_if_value[preproc_tos].saw_active);
+ stacked_if_value[preproc_tos].saw_active = true;
- if (stacked_if_value[preproc_tos].condition)
+ if (stacked_if_value[preproc_tos].active)
BEGIN(C);
else
BEGIN(xskip);
@@ -1272,7 +1316,7 @@ cppline {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
else
preproc_tos--;
- if (stacked_if_value[preproc_tos].condition)
+ if (stacked_if_value[preproc_tos].active)
BEGIN(C);
else
BEGIN(xskip);
@@ -1286,7 +1330,7 @@ cppline {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
else
preproc_tos--;
- if (stacked_if_value[preproc_tos].condition)
+ if (stacked_if_value[preproc_tos].active)
BEGIN(C);
else
BEGIN(xskip);
@@ -1301,12 +1345,10 @@ cppline {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
<xskip>{other} { /* ignore */ }
<xcond>{identifier}{space}*";" {
- if (preproc_tos >= MAX_NESTED_IF-1)
- mmfatal(PARSE_ERROR, "too many nested EXEC SQL IFDEF conditions");
- else
{
struct _defines *defptr;
unsigned int i;
+ bool this_active;
/*
* Skip the ";" and trailing whitespace. Note that yytext
@@ -1324,13 +1366,15 @@ cppline {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
defptr = defptr->next)
/* skip */ ;
- preproc_tos++;
- stacked_if_value[preproc_tos].else_branch = false;
- stacked_if_value[preproc_tos].condition =
- (defptr ? ifcond : !ifcond) && stacked_if_value[preproc_tos-1].condition;
+ this_active = (defptr ? ifcond : !ifcond);
+ stacked_if_value[preproc_tos].active =
+ (stacked_if_value[preproc_tos-1].active &&
+ !stacked_if_value[preproc_tos].saw_active &&
+ this_active);
+ stacked_if_value[preproc_tos].saw_active |= this_active;
}
- if (stacked_if_value[preproc_tos].condition)
+ if (stacked_if_value[preproc_tos].active)
BEGIN(C);
else
BEGIN(xskip);
@@ -1442,10 +1486,12 @@ lex_init(void)
parenths_open = 0;
current_function = NULL;
- preproc_tos = 0;
yylineno = 1;
- ifcond = true;
- stacked_if_value[preproc_tos].condition = ifcond;
+
+ /* initialize state for if/else/endif */
+ preproc_tos = 0;
+ stacked_if_value[preproc_tos].active = true;
+ stacked_if_value[preproc_tos].saw_active = true;
stacked_if_value[preproc_tos].else_branch = false;
/* initialize literal buffer to a reasonable but expansible size */
diff --git a/src/interfaces/ecpg/test/expected/preproc-define.c b/src/interfaces/ecpg/test/expected/preproc-define.c
index bde15b74a0..0256609b1f 100644
--- a/src/interfaces/ecpg/test/expected/preproc-define.c
+++ b/src/interfaces/ecpg/test/expected/preproc-define.c
@@ -40,73 +40,90 @@ main(void)
{
/* exec sql begin declare section */
+
typedef char string [ 8 ];
-#line 21 "define.pgc"
+#line 22 "define.pgc"
-
-#line 22 "define.pgc"
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+#line 23 "define.pgc"
intarray amount ;
-#line 23 "define.pgc"
+#line 24 "define.pgc"
char name [ 6 ] [ 8 ] ;
-#line 24 "define.pgc"
+#line 37 "define.pgc"
char letter [ 6 ] [ 1 ] ;
#if 0
-#line 26 "define.pgc"
+#line 39 "define.pgc"
int not_used ;
#endif
/* exec sql end declare section */
-#line 29 "define.pgc"
+#line 46 "define.pgc"
int i,j;
ECPGdebug(1, stderr);
{ ECPGconnect(__LINE__, 0, "ecpg1_regression" , NULL, NULL , NULL, 0);
-#line 34 "define.pgc"
+#line 51 "define.pgc"
if (sqlca.sqlcode < 0) sqlprint();}
-#line 34 "define.pgc"
+#line 51 "define.pgc"
{ ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_normal, "create table test ( name char ( 8 ) , amount int , letter char (
1) )", ECPGt_EOIT, ECPGt_EORT);
-#line 36 "define.pgc"
+#line 53 "define.pgc"
if (sqlca.sqlcode < 0) sqlprint();}
-#line 36 "define.pgc"
+#line 53 "define.pgc"
{ ECPGtrans(__LINE__, NULL, "commit");
-#line 37 "define.pgc"
+#line 54 "define.pgc"
if (sqlca.sqlcode < 0) sqlprint();}
-#line 37 "define.pgc"
+#line 54 "define.pgc"
{ ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_normal, "insert into Test ( name , amount , letter ) values ( 'false' , 1
,'f' )", ECPGt_EOIT, ECPGt_EORT);
-#line 39 "define.pgc"
+#line 56 "define.pgc"
if (sqlca.sqlcode < 0) sqlprint();}
-#line 39 "define.pgc"
+#line 56 "define.pgc"
{ ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_normal, "insert into test ( name , amount , letter ) values ( 'true' , 2
,'t' )", ECPGt_EOIT, ECPGt_EORT);
-#line 40 "define.pgc"
+#line 57 "define.pgc"
if (sqlca.sqlcode < 0) sqlprint();}
-#line 40 "define.pgc"
+#line 57 "define.pgc"
{ ECPGtrans(__LINE__, NULL, "commit");
-#line 41 "define.pgc"
+#line 58 "define.pgc"
if (sqlca.sqlcode < 0) sqlprint();}
-#line 41 "define.pgc"
+#line 58 "define.pgc"
{ ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_normal, "select * from test", ECPGt_EOIT,
@@ -116,10 +133,10 @@ if (sqlca.sqlcode < 0) sqlprint();}
ECPGt_NO_INDICATOR, NULL , 0L, 0L, 0L,
ECPGt_char,(letter),(long)1,(long)6,(1)*sizeof(char),
ECPGt_NO_INDICATOR, NULL , 0L, 0L, 0L, ECPGt_EORT);
-#line 43 "define.pgc"
+#line 60 "define.pgc"
if (sqlca.sqlcode < 0) sqlprint();}
-#line 43 "define.pgc"
+#line 60 "define.pgc"
for (i=0, j=sqlca.sqlerrd[2]; i<j; i++)
@@ -129,16 +146,16 @@ if (sqlca.sqlcode < 0) sqlprint();}
-#line 48 "define.pgc"
+#line 65 "define.pgc"
string n ;
-#line 49 "define.pgc"
+#line 66 "define.pgc"
char l = letter [ i ] [ 0 ] ;
-#line 50 "define.pgc"
+#line 67 "define.pgc"
int a = amount [ i ] ;
/* exec sql end declare section */
-#line 51 "define.pgc"
+#line 68 "define.pgc"
strncpy(n, name[i], 8);
@@ -146,22 +163,22 @@ if (sqlca.sqlcode < 0) sqlprint();}
}
{ ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_normal, "drop table test", ECPGt_EOIT, ECPGt_EORT);
-#line 57 "define.pgc"
+#line 74 "define.pgc"
if (sqlca.sqlcode < 0) sqlprint();}
-#line 57 "define.pgc"
+#line 74 "define.pgc"
{ ECPGtrans(__LINE__, NULL, "commit");
-#line 58 "define.pgc"
+#line 75 "define.pgc"
if (sqlca.sqlcode < 0) sqlprint();}
-#line 58 "define.pgc"
+#line 75 "define.pgc"
{ ECPGdisconnect(__LINE__, "CURRENT");
-#line 59 "define.pgc"
+#line 76 "define.pgc"
if (sqlca.sqlcode < 0) sqlprint();}
-#line 59 "define.pgc"
+#line 76 "define.pgc"
return 0;
diff --git a/src/interfaces/ecpg/test/expected/preproc-define.stderr
b/src/interfaces/ecpg/test/expected/preproc-define.stderr
index 722cdc7572..869dc46b0d 100644
--- a/src/interfaces/ecpg/test/expected/preproc-define.stderr
+++ b/src/interfaces/ecpg/test/expected/preproc-define.stderr
@@ -2,53 +2,53 @@
[NO_PID]: sqlca: code: 0, state: 00000
[NO_PID]: ECPGconnect: opening database ecpg1_regression on <DEFAULT> port <DEFAULT>
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_execute on line 36: query: create table test ( name char ( 8 ) , amount int , letter char ( 1 ) ); with
0parameter(s) on connection ecpg1_regression
+[NO_PID]: ecpg_execute on line 53: query: create table test ( name char ( 8 ) , amount int , letter char ( 1 ) ); with
0parameter(s) on connection ecpg1_regression
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_execute on line 36: using PQexec
+[NO_PID]: ecpg_execute on line 53: using PQexec
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_process_output on line 36: OK: CREATE TABLE
+[NO_PID]: ecpg_process_output on line 53: OK: CREATE TABLE
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ECPGtrans on line 37: action "commit"; connection "ecpg1_regression"
+[NO_PID]: ECPGtrans on line 54: action "commit"; connection "ecpg1_regression"
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_execute on line 39: query: insert into Test ( name , amount , letter ) values ( 'false' , 1 , 'f' );
with0 parameter(s) on connection ecpg1_regression
+[NO_PID]: ecpg_execute on line 56: query: insert into Test ( name , amount , letter ) values ( 'false' , 1 , 'f' );
with0 parameter(s) on connection ecpg1_regression
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_execute on line 39: using PQexec
+[NO_PID]: ecpg_execute on line 56: using PQexec
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_process_output on line 39: OK: INSERT 0 1
+[NO_PID]: ecpg_process_output on line 56: OK: INSERT 0 1
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_execute on line 40: query: insert into test ( name , amount , letter ) values ( 'true' , 2 , 't' );
with0 parameter(s) on connection ecpg1_regression
+[NO_PID]: ecpg_execute on line 57: query: insert into test ( name , amount , letter ) values ( 'true' , 2 , 't' );
with0 parameter(s) on connection ecpg1_regression
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_execute on line 40: using PQexec
+[NO_PID]: ecpg_execute on line 57: using PQexec
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_process_output on line 40: OK: INSERT 0 1
+[NO_PID]: ecpg_process_output on line 57: OK: INSERT 0 1
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ECPGtrans on line 41: action "commit"; connection "ecpg1_regression"
+[NO_PID]: ECPGtrans on line 58: action "commit"; connection "ecpg1_regression"
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_execute on line 43: query: select * from test; with 0 parameter(s) on connection ecpg1_regression
+[NO_PID]: ecpg_execute on line 60: query: select * from test; with 0 parameter(s) on connection ecpg1_regression
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_execute on line 43: using PQexec
+[NO_PID]: ecpg_execute on line 60: using PQexec
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_process_output on line 43: correctly got 2 tuples with 3 fields
+[NO_PID]: ecpg_process_output on line 60: correctly got 2 tuples with 3 fields
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_get_data on line 43: RESULT: false offset: -1; array: no
+[NO_PID]: ecpg_get_data on line 60: RESULT: false offset: -1; array: no
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_get_data on line 43: RESULT: true offset: -1; array: no
+[NO_PID]: ecpg_get_data on line 60: RESULT: true offset: -1; array: no
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_get_data on line 43: RESULT: 1 offset: -1; array: no
+[NO_PID]: ecpg_get_data on line 60: RESULT: 1 offset: -1; array: no
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_get_data on line 43: RESULT: 2 offset: -1; array: no
+[NO_PID]: ecpg_get_data on line 60: RESULT: 2 offset: -1; array: no
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_get_data on line 43: RESULT: f offset: -1; array: no
+[NO_PID]: ecpg_get_data on line 60: RESULT: f offset: -1; array: no
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_get_data on line 43: RESULT: t offset: -1; array: no
+[NO_PID]: ecpg_get_data on line 60: RESULT: t offset: -1; array: no
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_execute on line 57: query: drop table test; with 0 parameter(s) on connection ecpg1_regression
+[NO_PID]: ecpg_execute on line 74: query: drop table test; with 0 parameter(s) on connection ecpg1_regression
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_execute on line 57: using PQexec
+[NO_PID]: ecpg_execute on line 74: using PQexec
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_process_output on line 57: OK: DROP TABLE
+[NO_PID]: ecpg_process_output on line 74: OK: DROP TABLE
[NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ECPGtrans on line 58: action "commit"; connection "ecpg1_regression"
+[NO_PID]: ECPGtrans on line 75: action "commit"; connection "ecpg1_regression"
[NO_PID]: sqlca: code: 0, state: 00000
[NO_PID]: ecpg_finish: connection ecpg1_regression closed
[NO_PID]: sqlca: code: 0, state: 00000
diff --git a/src/interfaces/ecpg/test/preproc/define.pgc b/src/interfaces/ecpg/test/preproc/define.pgc
index 0d07ebfe63..d360da7ece 100644
--- a/src/interfaces/ecpg/test/preproc/define.pgc
+++ b/src/interfaces/ecpg/test/preproc/define.pgc
@@ -17,15 +17,32 @@ int
main(void)
{
exec sql begin declare section;
+
exec sql ifdef NAMELEN;
typedef char string[NAMELEN];
intarray amount;
char name[AMOUNT][NAMELEN];
+exec sql elif AMOUNT;
+ should not get here;
+exec sql else;
+ should not get here either;
+exec sql endif;
+
+exec sql ifndef NAMELEN;
+ should not get here;
+exec sql elif AMOUNT;
+ exec sql ifdef NOSUCHNAME;
+ should not get here;
+ exec sql else;
char letter[AMOUNT][1];
#if 0
int not_used;
#endif
+ exec sql endif;
+exec sql elif AMOUNT;
+ should not get here;
exec sql endif;
+
exec sql end declare section;
int i,j;
On Sat, Aug 1, 2020 at 5:36 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Ashutosh Sharma <ashu.coek88@gmail.com> writes: > > When the following ecpg program having preprocessor directives is compiled, > > the output produced is not correct. > > ... > > As seen from above output, both exec sql ifdef and exec sql else block got > > compiled which is wrong. If the above output is further compiled using gcc > > compiler, the compilation would fail. > > Looking at pgc.l, it seems that 'elif' is treated as though it were > 'endif' followed by 'ifdef', which of course completely loses the > expected property that a previous successful branch would keep the > elif branch from being expanded. Yeah, that's right. The point is, while processing the elif branch, we remove an entry for the previous branch (ifdef, ifndef) from the stack and push a new entry for the current elif branch. So, if the elif branch is evaluated to false, the else branch gets automatically evaluated to true. And as a result of that both ifdef and else branch gets evaluated to true thereby compiling both (ifdef/ifndef, else) blocks. -- With Regards, Ashutosh Sharma EnterpriseDB:http://www.enterprisedb.com
Hi, Thanks for the patch. I've spent quite some time reviewing it and the changes look good to me. It looks very neat and is also crystal-clear. -- With Regards, Ashutosh Sharma EnterpriseDB:http://www.enterprisedb.com On Sat, Aug 1, 2020 at 8:05 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > I wrote: > > Looking at pgc.l, it seems that 'elif' is treated as though it were > > 'endif' followed by 'ifdef', which of course completely loses the > > expected property that a previous successful branch would keep the > > elif branch from being expanded. > > While this doesn't look terribly hard to fix, I'm a little disturbed > > by the fact that the existing semantics seem to date back to 1999 > > (b57b0e044). We're probably risking breaking existing app code if > > we change it. I think we *should* change it, of course, but I'm kind > > of inclined not to back-patch. > > Here's a proposed patch, which also clarifies the documentation, > which seemed a bit confused/misleading to me. > > As stated, I'm not sure it's wise to back-patch this aggressively > ... but maybe it'd be okay to squeeze it into v13? > > regards, tom lane >
> Here's a proposed patch, which also clarifies the documentation, > which seemed a bit confused/misleading to me. Thanks Tom. > As stated, I'm not sure it's wise to back-patch this aggressively > ... but maybe it'd be okay to squeeze it into v13? Fully agreed, back-patching sounds like a pretty risky approach. I guess almost everyone who ran into this, fixed their code to work correctly by now. I'd also prefer squeezing it into 13, let's not keep this around for longer. Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org
Michael Meskes <meskes@postgresql.org> writes:
>> As stated, I'm not sure it's wise to back-patch this aggressively
>> ... but maybe it'd be okay to squeeze it into v13?
> Fully agreed, back-patching sounds like a pretty risky approach. I
> guess almost everyone who ran into this, fixed their code to work
> correctly by now. I'd also prefer squeezing it into 13, let's not keep
> this around for longer.
Sounds good, done that way.
regards, tom lane