Обсуждение: [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

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

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