Re: ECPG doesn't compile CREATE AS EXECUTE properly.
On Tue, Jul 06, 2021 at 05:47:34PM +0900, Kyotaro Horiguchi wrote: > More accurately, I didn't come up with the way to split out some of > the rule-components in a rule out as another rule using the existing > infrastructure. > > [...] > > Then add the following component to the rule "stmt". I see. That sounds interesting as solution, and consistent with the existing cursor queries. The ECPG parser is not that advanced, and we may not really need to make it more complicated with sub-clause handling like INEs. So I'd be rather in favor of what you are describing here. -- Michael signature.asc Description: PGP signature
Re: ECPG doesn't compile CREATE AS EXECUTE properly.
Thanks for the comment.
At Tue, 6 Jul 2021 11:17:47 +0900, Michael Paquier wrote
in
> On Thu, Jul 01, 2021 at 06:45:25PM +0900, Kyotaro Horiguchi wrote:
> > Separating "CREATE TABLE AS EXECUTE" from ExecuteStmt would be cleaner
> > but I avoided to change the syntax tree. Instead the attched make
> > distinction of $$.type of ExecuteStmt between NULL and "" to use to
> > notify the returned name is name of a prepared statement or a full
> > statement.
>
> I am not so sure, and using an empty string makes the code a bit
> harder to follow. How would that look with the grammar split you have
I agree to that.
> in mind? Maybe that makes the code more consistent with the PREPARE
> block a couple of lines above?
More accurately, I didn't come up with the way to split out some of
the rule-components in a rule out as another rule using the existing
infrastructure.
preproc.y:
ExecuteStmt:
EXECUTE prepared_name execute_param_clause execute_rest
{}
| CREATE OptTemp TABLE create_as_target AS EXECUTE prepared_name
execute_param_clause opt_with_data execute_rest
{}
| CREATE OptTemp TABLE IF_P NOT EXISTS create_as_target AS EXECUTE
prepared_name execute_param_clause opt_with_data execute_rest
{}
;
I can directly edit this as the following:
ExecuteStmt:
EXECUTE prepared_name execute_param_clause execute_rest
{}
;
CreateExecuteStmt:
| CREATE OptTemp TABLE create_as_target AS EXECUTE prepared_name
execute_param_clause opt_with_data execute_rest
{}
| CREATE OptTemp TABLE IF_P NOT EXISTS create_as_target AS EXECUTE
prepared_name execute_param_clause opt_with_data execute_rest
{}
;
Then add the following component to the rule "stmt".
| CreateExecuteStmt:
{ output_statement(..., ECPGst_normal); }
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Re: ECPG doesn't compile CREATE AS EXECUTE properly.
On Thu, Jul 01, 2021 at 06:45:25PM +0900, Kyotaro Horiguchi wrote: > Separating "CREATE TABLE AS EXECUTE" from ExecuteStmt would be cleaner > but I avoided to change the syntax tree. Instead the attched make > distinction of $$.type of ExecuteStmt between NULL and "" to use to > notify the returned name is name of a prepared statement or a full > statement. I am not so sure, and using an empty string makes the code a bit harder to follow. How would that look with the grammar split you have in mind? Maybe that makes the code more consistent with the PREPARE block a couple of lines above? -- Michael signature.asc Description: PGP signature
Re: ECPG doesn't compile CREATE AS EXECUTE properly.
At Thu, 01 Jul 2021 18:45:25 +0900 (JST), Kyotaro Horiguchi
wrote in
> I'll post the test part later.
A version incluedes the test part.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
>From 6ef902608483f395983da5063db3e0af8d61ed4e Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi
Date: Thu, 1 Jul 2021 14:56:42 +0900
Subject: [PATCH v2 1/2] Fix ECPG's CREATE TABLE AS EXECUTE
The syntax was precompiled into uncompilable .c statement.
Avoiding impact on parse tree structure, use ExecuteStmt.type to
notify whether the returned string is a statment name or a full
statement.
---
src/interfaces/ecpg/preproc/ecpg.addons | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/interfaces/ecpg/preproc/ecpg.addons b/src/interfaces/ecpg/preproc/ecpg.addons
index b6e3412cef..820608605b 100644
--- a/src/interfaces/ecpg/preproc/ecpg.addons
+++ b/src/interfaces/ecpg/preproc/ecpg.addons
@@ -34,7 +34,9 @@ ECPG: stmtUpdateStmt block
ECPG: stmtExecuteStmt block
{
check_declared_list($1.name);
- if ($1.type == NULL || strlen($1.type) == 0)
+ if ($1.type == NULL)
+ output_statement($1.name, 1, ECPGst_normal);
+ else if (strlen($1.type) == 0)
output_statement($1.name, 1, ECPGst_execute);
else
{
@@ -362,14 +364,18 @@ ECPG: ExecuteStmtEXECUTEprepared_nameexecute_param_clauseexecute_rest block
{
$$.name = $2;
$$.type = $3;
+ if ($$.type == NULL)
+ $$.type = "";
}
ECPG: ExecuteStmtCREATEOptTempTABLEcreate_as_targetASEXECUTEprepared_nameexecute_param_clauseopt_with_dataexecute_rest block
{
$$.name = cat_str(8,mm_strdup("create"),$2,mm_strdup("table"),$4,mm_strdup("as execute"),$7,$8,$9);
+ $$.type = NULL;
}
ECPG: ExecuteStmtCREATEOptTempTABLEIF_PNOTEXISTScreate_as_targetASEXECUTEprepared_nameexecute_param_clauseopt_with_dataexecute_rest block
{
$$.name = cat_str(8,mm_strdup("create"),$2,mm_strdup("table if not exists"),$7,mm_strdup("as execute"),$10,$11,$12);
+ $$.type = NULL;
}
ECPG: DeclareCursorStmtDECLAREcursor_namecursor_optionsCURSORopt_holdFORSelectStmt block
{
--
2.27.0
>From a1bc18dacddae1af616d1ce167cae1c246483a2c Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi
Date: Thu, 1 Jul 2021 19:13:09 +0900
Subject: [PATCH v2 2/2] test part
---
.../ecpg/test/expected/sql-execute.c | 29 ++-
.../ecpg/test/expected/sql-execute.stderr | 22 ++
src/interfaces/ecpg/test/sql/execute.pgc | 3 ++
3 files changed, 41 insertions(+), 13 deletions(-)
diff --git a/src/interfaces/ecpg/test/expected/sql-execute.c b/src/interfaces/ecpg/test/expected/sql-execute.c
index 10e9ad56b5..f5e9cfd664 100644
--- a/src/interfaces/ecpg/test/expected/sql-execute.c
+++ b/src/interfaces/ecpg/test/expected/sql-execute.c
@@ -302,29 +302,42 @@ if (sqlca.sqlcode < 0) sqlprint();}
printf("name[%d]=%8.8s\tamount[%d]=%d\tletter[%d]=%c\n", i, n, i, a, i, l);
}
+ { ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_normal, "create table newt as execute \"f\" ( 2 )", ECPGt_EOIT, ECPGt_EORT);
+#line 107 "execute.pgc"
+
+if (sqlca.sqlcode < 0) sqlprint();}
+#line 107 "execute.pgc"
+
+
{ ECPGdeallocate(__LINE__, 0, NULL, "f");
-#line 107 "execute.pgc"
+#line 109 "execute.pgc"
if (sqlca.sqlcode < 0) sqlprint();}
-#line 107 "execute.pgc"
+#line 109 "execute.pgc"
+
+ { ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_normal, "drop table newt", ECPGt_EOIT, ECPGt_EORT);
+#line 110 "execute.pgc"
+
+if (sqlca.sqlcode < 0) sqlprint();}
+#line 110 "execute.pgc"
{ ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_normal, "drop table test", ECPGt_EOIT, ECPGt_EORT);
-#line 108 "execute.pgc"
+#line 111 "execute.pgc"
if (sqlca.sqlcode < 0) sqlprint();}
-#line 108 "execute.pgc"
+#line 111 "execute.pgc"
{ ECPGtrans(__LINE__, NULL, "commit");
-#line 109 "execute.pgc"
+#line 112 "execute.pgc"
if (sqlca.sqlcode < 0) sqlprint();}
-#line 109 "execute.pgc"
+#line 112 "execute.pgc"
{ ECPGdisconnect(__LINE__, "CURRENT");
-#line 110 "execute.pgc"
+#line 113 "execute.pgc"
if (sqlca.sqlcode < 0) sqlprint();}
-#line 110 "execute.pgc"
+#line 113 "execute.pgc"
return 0;
diff --git a/src/interfaces/ecpg/test/expected/sql-execute.stderr b/src/interfaces/ecpg/test/expected/sql-execute.stderr
index d8bc3c6524..26b72cf27f 100644
--- a/src/interfaces/ecpg/test/expected/sql-execute.stderr
+++ b/src/interfaces/ecpg/test/expected/sql-execute.stderr
@@ -156,15 +156,27 @@
[NO_PID]: sqlca: code: 0, state: 0
[NO_PID]: ecpg_get_data on line 94: RESULT: t offset: -1; array: no
[NO_PID]: sqlca: code: 0, state: 0
-[NO_PID]: deallocate_one on line 107: name f
+[NO_PID]: ecpg_execute on line 107: query: create table newt as execute "f" ( 2 ); with 0 parameter(s) on connection main
[NO_PID]: sqlca: code: 0, state: 0
-[NO_PID]: ecpg_execute on line 108: query: drop table test; with 0 parameter(s) on connection main
+[NO_PID]: ecpg_execute on line 107: using PQexec
[NO_PID]: sqlca: code: 0, state: 0
-[NO_PID]: ecpg_execute on line 108: using
ECPG doesn't compile CREATE AS EXECUTE properly.
Hello.
While I looked a patch, I found that the following ECPG statement
generates uncompilable .c source.
EXEC SQL CREATE TABLE t AS stmt;
ecpgtest.c:
#line 42 "ecpgtest.pgc"
printf("1:dbname=%s\n", dbname);
{ ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_execute, create table
t as execute "stmt", ECPGt_EOIT, ECPGt_EORT);
This is apparently broken. The cause is that the rule ExecutStmt is
assumed to return a statement name when type is empty (or null), while
it actually returns a full statement for the CREATE TABLE AS EXECUTE
syntax.
Separating "CREATE TABLE AS EXECUTE" from ExecuteStmt would be cleaner
but I avoided to change the syntax tree. Instead the attched make
distinction of $$.type of ExecuteStmt between NULL and "" to use to
notify the returned name is name of a prepared statement or a full
statement.
I'll post the test part later.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
>From 75a99097a988c81802d659cf8a58d74060d36409 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi
Date: Thu, 1 Jul 2021 14:56:42 +0900
Subject: [PATCH v1] Fix ECPG's CREATE TABLE AS EXECUTE
The syntax was precompiled into uncompilable .c statement.
Avoiding impact on parse tree structure, use ExecuteStmt.type to
notify whether the returned string is a statment name or a full
statement.
---
src/interfaces/ecpg/preproc/ecpg.addons | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/interfaces/ecpg/preproc/ecpg.addons b/src/interfaces/ecpg/preproc/ecpg.addons
index b6e3412cef..820608605b 100644
--- a/src/interfaces/ecpg/preproc/ecpg.addons
+++ b/src/interfaces/ecpg/preproc/ecpg.addons
@@ -34,7 +34,9 @@ ECPG: stmtUpdateStmt block
ECPG: stmtExecuteStmt block
{
check_declared_list($1.name);
- if ($1.type == NULL || strlen($1.type) == 0)
+ if ($1.type == NULL)
+ output_statement($1.name, 1, ECPGst_normal);
+ else if (strlen($1.type) == 0)
output_statement($1.name, 1, ECPGst_execute);
else
{
@@ -362,14 +364,18 @@ ECPG: ExecuteStmtEXECUTEprepared_nameexecute_param_clauseexecute_rest block
{
$$.name = $2;
$$.type = $3;
+ if ($$.type == NULL)
+ $$.type = "";
}
ECPG: ExecuteStmtCREATEOptTempTABLEcreate_as_targetASEXECUTEprepared_nameexecute_param_clauseopt_with_dataexecute_rest block
{
$$.name = cat_str(8,mm_strdup("create"),$2,mm_strdup("table"),$4,mm_strdup("as execute"),$7,$8,$9);
+ $$.type = NULL;
}
ECPG: ExecuteStmtCREATEOptTempTABLEIF_PNOTEXISTScreate_as_targetASEXECUTEprepared_nameexecute_param_clauseopt_with_dataexecute_rest block
{
$$.name = cat_str(8,mm_strdup("create"),$2,mm_strdup("table if not exists"),$7,mm_strdup("as execute"),$10,$11,$12);
+ $$.type = NULL;
}
ECPG: DeclareCursorStmtDECLAREcursor_namecursor_optionsCURSORopt_holdFORSelectStmt block
{
--
2.27.0
