Thread: [Bug Fix] ECPG: could not use some CREATE TABLE AS syntax
Hi, I found some "CREATE TABLE ... AS ... " syntaxes could not be used in ECPG. [PROBLEM] First, ECPG command is failed when the source code (*.pgc) has "IF NOT EXISTS". ------------------------------------------------------------------- EXEC SQL CREATE TABLE IF NOT EXISTS test_cta AS SELECT * FROM test; ------------------------------------------------------------------- Second, ECPG command is succeeded when the source code (*.pgc) has following embedded SQL. However, created c program hasno "WITH NO DATA". ------------------------------------------------------------------ EXEC SQL CREATE TABLE test_cta AS SELECT * FROM test WITH NO DATA; ------------------------------------------------------------------ [Investigation] In my investigation, parse.pl ignore type CreateAsStmt of gram.y and CreateAsStmt is defined in ecpg.trailer. ECPG use ecpg.trailer'sCreateAsStmt. However, ecpg.trailer lacks some syntaxes. I feel ignoring type CreateAsStmt of gram.y is strange. Seeing ecpg.trailer, it seems that ECPG wanted to output the message"CREATE TABLE AS cannot specify INTO" but is this needed now? In view of the maintenance, ECPG should use not ecpg.trailer'sdefinition but gram.y's one. I attached the patch for this and I will register this for next CF. Regards, Daisuke Higuchi
Attachment
Higuchi-san, > I found some "CREATE TABLE ... AS ... " syntaxes could not be used in > ECPG. > ... > [Investigation] > In my investigation, parse.pl ignore type CreateAsStmt of gram.y and > CreateAsStmt is defined in ecpg.trailer. ECPG use ecpg.trailer's > CreateAsStmt. However, ecpg.trailer lacks some syntaxes. Correct, the syntax of create as statement was changed and those changes have not been added to ecpg. > I feel ignoring type CreateAsStmt of gram.y is strange. Seeing > ecpg.trailer, it seems that ECPG wanted to output the message "CREATE > TABLE AS cannot specify INTO" but is this needed now? In view of the > maintenance, ECPG should use not ecpg.trailer's definition but > gram.y's one. I beg to disagree, or I don't understand. Why would ecpg's changes to the statement be wrong nowadays? > I attached the patch for this and I will register this for next CF. I think the patch does not work correctly. The statement CREATE TABLE a AS SELECT * INTO test FROM a; is accepted with your patch, but it is not accepted in current ecpg nor is it accepted by the backend when you execute it through ecpg. The whole change of this rule has been made to make sure this syntax is not accepted. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Hi, Meskes-san,
Thanks for your response!
> I beg to disagree, or I don't understand. Why would ecpg's changes to
> the statement be wrong nowadays?
I might confuse you, but it does not mean that it is wrong to reject CREATE TABLE AS ... INTO ... syntax in ECPG.
My goal is to accept syntax which is currently rejected by ECPG. To realize that, I am considering following two ways:
(a) new syntax of create as statement should be added to ECPG
(b) make ECPG to use not ecpg.trailer but gram.y in the syntax of create as statement
In (a), we need to keep similar codes in both ecpg.trailer and gram.y. Also, if the syntax of create as statement will
bechanged in the future, there is a possibility that it will not be reflected in ECPG like this bug. Therefore, I
thoughtthat (b) was better and created a patch. And, in order to make it the simplest code, some SQL which is rejected
incurrent ECPG is accepted in my patch's ECPG.
> The statement CREATE TABLE a AS SELECT * INTO test FROM a; is accepted
> with your patch, but it is not accepted in current ecpg nor is it accepted
> by the backend when you execute it through ecpg.
Indeed, CREATE TABLE a AS SELECT * INTO test FROM a; is accepted in my patch's ECPG, but the backend always reject, but
whichSQL should be rejected in both ECPG and the backend? Following inappropriate SQL are accepted in ECPG but rejected
bythe backend (I am wondering why only CREATE TABLE .. AS .. INTO is rejected and other inappropriate SQL are accepted
incurrent ECPG. ).
- EXEC SQL delete from test where c1 = (select into c2 from test2);
From the viewpoint of compatibility, if (b) is not good, I will consider (a) solution like following:
diff --git a/src/interfaces/ecpg/preproc/ecpg.trailer b/src/interfaces/ecpg/preproc/ecpg.trailer
@@ -34,7 +34,14 @@ CreateAsStmt: CREATE OptTemp TABLE create_as_target AS {FoundInto = 0;} SelectSt
if (FoundInto == 1)
mmerror(PARSE_ERROR, ET_ERROR, "CREATE TABLE AS cannot specify INTO");
- $$ = cat_str(6, mm_strdup("create"), $2, mm_strdup("table"), $4, mm_strdup("as"), $7);
+ $$ = cat_str(7, mm_strdup("create"), $2, mm_strdup("table"), $4, mm_strdup("as"), $7, $8);
+ }
+ | CREATE OptTemp TABLE IF_P NOT EXISTS create_as_target AS {FoundInto = 0;} SelectStmt opt_with_data
+ {
+ if (FoundInto == 1)
+ mmerror(PARSE_ERROR, ET_ERROR, "CREATE TABLE AS cannot specify INTO");
+
+ $$ = cat_str(7, mm_strdup("create"), $2, mm_strdup("table if not exists"), $7, mm_strdup("as"), $10,
$11);
}
;
I also want to hear your opinion. I will change my opinion flexibly.
Regards,
Daisuke, Higuchi
Hi Higuchi-san,
> My goal is to accept syntax which is currently rejected by ECPG. To
> realize that, I am considering following two ways:
> (a) new syntax of create as statement should be added to ECPG
Correct.
> (b) make ECPG to use not ecpg.trailer but gram.y in the syntax of
> create as statement
I don't see how this would be possible to be honest.
> In (a), we need to keep similar codes in both ecpg.trailer and
> gram.y. Also, if the syntax of create as statement will be changed in
> the future, there is a possibility that it will not be reflected in
> ECPG like this bug. Therefore, I thought that (b) was better and
> created a patch. And, in order to make it the simplest code, some SQL
> which is rejected in current ECPG is accepted in my patch's ECPG.
Yes, I fully understand that. However, in doing so you accept
statements that the backend later on rejects. The sole reason for the
big ecpg grammar is to prevent those cases whenever possible.
> Indeed, CREATE TABLE a AS SELECT * INTO test FROM a; is accepted in
> my patch's ECPG, but the backend always reject, but which SQL should
> be rejected in both ECPG and the backend? Following inappropriate SQL
> are accepted in ECPG but rejected by the backend (I am wondering why
> only CREATE TABLE .. AS .. INTO is rejected and other inappropriate
> SQL are accepted in current ECPG. ).
That does sound like a bug to me. There may cases where it is not
possible to catch an invalid syntax for one reason or another. But I
would definitely go the extra mile to make the parsers as compatible as
possible.
> From the viewpoint of compatibility, if (b) is not good, I will
> consider (a) solution like following:
>
> diff --git a/src/interfaces/ecpg/preproc/ecpg.trailer
> b/src/interfaces/ecpg/preproc/ecpg.trailer
> @@ -34,7 +34,14 @@ CreateAsStmt: CREATE OptTemp TABLE
> create_as_target AS {FoundInto = 0;} SelectSt
> if (FoundInto == 1)
> mmerror(PARSE_ERROR, ET_ERROR,
> "CREATE TABLE AS cannot specify INTO");
>
> - $$ = cat_str(6, mm_strdup("create"), $2,
> mm_strdup("table"), $4, mm_strdup("as"), $7);
> + $$ = cat_str(7, mm_strdup("create"), $2,
> mm_strdup("table"), $4, mm_strdup("as"), $7, $8);
> + }
> + | CREATE OptTemp TABLE IF_P NOT EXISTS
> create_as_target AS {FoundInto = 0;} SelectStmt opt_with_data
> + {
> + if (FoundInto == 1)
> + mmerror(PARSE_ERROR, ET_ERROR, "CREATE
> TABLE AS cannot specify INTO");
> +
> + $$ = cat_str(7, mm_strdup("create"), $2,
> mm_strdup("table if not exists"), $7, mm_strdup("as"), $10, $11);
> }
> ;
>
> I also want to hear your opinion. I will change my opinion flexibly.
I agree that this the way to go.
Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Jabber: michael at xmpp dot meskes dot org
VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Hi, Meskes-san > Yes, I fully understand that. However, in doing so you accept > statements that the backend later on rejects. The sole reason for > the big ecpg grammar is to prevent those cases whenever possible. Ok, I agree with you. > > I also want to hear your opinion. I will change my opinion flexibly. > I agree that this the way to go. I updated and attached the patch. As I show in previous post, this version is that "IF NOT EXISTS" keyword and variable for"WITH NO DATA" are added to ecpg.trailer. Regards, Daisuke, Higuchi
Attachment
Hi Higuchi-san, > I updated and attached the patch. As I show in previous post, this > version is that "IF NOT EXISTS" keyword and variable for "WITH NO > DATA" are added to ecpg.trailer. Thank you, committed. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Hi, Meskes-san > Thank you, committed. Thank you! By the way, I have found another bugs which are related to ECPG , so I will post that later. Regards, Daisuke, Higuchi