Re: [Bug Fix] ECPG: could not use some CREATE TABLE AS syntax

From: Michael Meskes <meskes(at)postgresql(dot)org>
To: "Higuchi, Daisuke" <higuchi(dot)daisuke(at)jp(dot)fujitsu(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [Bug Fix] ECPG: could not use some CREATE TABLE AS syntax
Date: 2019-02-18 08:23:25
Message-ID: 986e2ec88853817ce9edc9398cfd79772592eb37.camel@postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Arthur Zakirov 2019-02-18 08:23:46 Re: [PATCH] xlogreader: do not read a file block twice
Previous Message Masahiko Sawada 2019-02-18 07:57:07 Re: Copy function for logical replication slots