Thread: Fixes gram.y

Fixes gram.y

From
"Rod Taylor"
Date:
I truely don't know what I did to create the nasty patch (source file
certainly didn't look like what it resulted in) -- then again there
have been alot of changes since the domain patch was created.

This should fix gram.y

If anyone knows a better way of creating patches other than diff -rc ,
please speak up.
--
Rod Taylor

Your eyes are weary from staring at the CRT. You feel sleepy. Notice
how restful it is to watch the cursor blink. Close your eyes. The
opinions stated above are yours. You cannot imagine why you ever felt
otherwise.


Re: Fixes gram.y

From
Bruce Momjian
Date:
Rod Taylor wrote:
> I truely don't know what I did to create the nasty patch (source file
> certainly didn't look like what it resulted in) -- then again there
> have been alot of changes since the domain patch was created.
>
> This should fix gram.y
>
> If anyone knows a better way of creating patches other than diff -rc ,
> please speak up.

I have applied the following new patch.  It moves DROP DATABASE as you
suggested, and fixes the CREATE TABLE tag to show just CREATE and not
CREATE DOMAIN.   Actually, CREATE DOMAIN should output just DOMAIN too,
unless someone can tell my why that is not consistent.  Patch applied to
CVS.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
Index: src/backend/parser/gram.y
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/gram.y,v
retrieving revision 2.292
diff -c -r2.292 gram.y
*** src/backend/parser/gram.y    19 Mar 2002 02:18:18 -0000    2.292
--- src/backend/parser/gram.y    19 Mar 2002 12:46:30 -0000
***************
*** 3184,3189 ****
--- 3184,3197 ----
                  {
                      $$ = lconsi(3, makeListi1(-1));
                  }
+         | OWNER opt_equal name
+                 {
+                     $$ = lconsi(4, makeList1($3));
+                 }
+         | OWNER opt_equal DEFAULT
+                 {
+                     $$ = lconsi(4, makeList1(NULL));
+                 }
          ;


***************
*** 3199,3212 ****
                      DropdbStmt *n = makeNode(DropdbStmt);
                      n->dbname = $3;
                      $$ = (Node *)n;
-                 }
-         | OWNER opt_equal name
-                 {
-                     $$ = lconsi(4, makeList1($3));
-                 }
-         | OWNER opt_equal DEFAULT
-                 {
-                     $$ = lconsi(4, makeList1(NULL));
                  }
          ;

--- 3207,3212 ----
Index: src/backend/tcop/postgres.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/tcop/postgres.c,v
retrieving revision 1.255
diff -c -r1.255 postgres.c
*** src/backend/tcop/postgres.c    19 Mar 2002 02:18:20 -0000    1.255
--- src/backend/tcop/postgres.c    19 Mar 2002 12:46:33 -0000
***************
*** 2213,2220 ****
              break;

          case T_CreateDomainStmt:
          case T_CreateStmt:
!             tag = "CREATE DOMAIN";
              break;

          case T_DropStmt:
--- 2213,2223 ----
              break;

          case T_CreateDomainStmt:
+             tag = "CREATE";            /* CREATE DOMAIN */
+             break;
+
          case T_CreateStmt:
!             tag = "CREATE";
              break;

          case T_DropStmt:

Re: Fixes gram.y

From
Bruce Momjian
Date:
Rod Taylor wrote:
> I truely don't know what I did to create the nasty patch (source file
> certainly didn't look like what it resulted in) -- then again there
> have been alot of changes since the domain patch was created.

Yes, that patch has been around for a while and I am sure went through
several merges.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Re: [PATCHES] Fixes gram.y

From
Thomas Lockhart
Date:
...
> I have applied the following new patch.  It moves DROP DATABASE as you
> suggested, and fixes the CREATE TABLE tag to show just CREATE and not
> CREATE DOMAIN.   Actually, CREATE DOMAIN should output just DOMAIN too,
> unless someone can tell my why that is not consistent.

Consistant or not, I'm not sure how only "DOMAIN" emitted as a result of
"CREATE DOMAIN" could extend to the other operations such as "DROP
DOMAIN". What would you return for that one? istm that "CREATE" needs to
show up as the first word in the response, and that if necessary we
should extend the other CREATE operations to qualify their return string
also.

                    - Thomas

Re: [PATCHES] Fixes gram.y

From
Peter Eisentraut
Date:
Bruce Momjian writes:

> I have applied the following new patch.  It moves DROP DATABASE as you
> suggested, and fixes the CREATE TABLE tag to show just CREATE and not
> CREATE DOMAIN.   Actually, CREATE DOMAIN should output just DOMAIN too,
> unless someone can tell my why that is not consistent.  Patch applied to
> CVS.

There is a standard for this.  CREATE DOMAIN shows CREATE DOMAIN.

--
Peter Eisentraut   peter_e@gmx.net


Re: [PATCHES] Fixes gram.y

From
Bruce Momjian
Date:
Peter Eisentraut wrote:
> Bruce Momjian writes:
>
> > I have applied the following new patch.  It moves DROP DATABASE as you
> > suggested, and fixes the CREATE TABLE tag to show just CREATE and not
> > CREATE DOMAIN.   Actually, CREATE DOMAIN should output just DOMAIN too,
> > unless someone can tell my why that is not consistent.  Patch applied to
> > CVS.
>
> There is a standard for this.  CREATE DOMAIN shows CREATE DOMAIN.

OK, CVS changed to emit CREATE DOMAIN.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Re: [PATCHES] Fixes gram.y

From
Bruce Momjian
Date:
Thomas Lockhart wrote:
> ...
> > I have applied the following new patch.  It moves DROP DATABASE as you
> > suggested, and fixes the CREATE TABLE tag to show just CREATE and not
> > CREATE DOMAIN.   Actually, CREATE DOMAIN should output just DOMAIN too,

                                                                ^^^^^^

Should have been CREATE here.  Sorry.


> > unless someone can tell my why that is not consistent.
>
> Consistant or not, I'm not sure how only "DOMAIN" emitted as a result of
> "CREATE DOMAIN" could extend to the other operations such as "DROP
> DOMAIN". What would you return for that one? istm that "CREATE" needs to
> show up as the first word in the response, and that if necessary we
> should extend the other CREATE operations to qualify their return string
> also.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Re: [PATCHES] Fixes gram.y

From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Peter Eisentraut wrote:
>> There is a standard for this.  CREATE DOMAIN shows CREATE DOMAIN.

> OK, CVS changed to emit CREATE DOMAIN.

What's standard about it?  I count 9 existing statements that use
"CREATE", vs 4 that use "CREATE xxx".  (And of those four, CREATE
VERSION is dead code...)  The closest existing statement, CREATE
TYPE, emits "CREATE".

Plain "CREATE" seems like the conforming choice, unless we'd like
to do a wholesale revision of existing command tags.  Which is
not necessarily an unreasonable thing to do.  But just making CREATE
DOMAIN emit "CREATE DOMAIN" isn't improving consistency at all.

            regards, tom lane

Re: [PATCHES] Fixes gram.y

From
Bruce Momjian
Date:
Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Peter Eisentraut wrote:
> >> There is a standard for this.  CREATE DOMAIN shows CREATE DOMAIN.
>
> > OK, CVS changed to emit CREATE DOMAIN.
>
> What's standard about it?  I count 9 existing statements that use
> "CREATE", vs 4 that use "CREATE xxx".  (And of those four, CREATE
> VERSION is dead code...)  The closest existing statement, CREATE
> TYPE, emits "CREATE".
>
> Plain "CREATE" seems like the conforming choice, unless we'd like
> to do a wholesale revision of existing command tags.  Which is
> not necessarily an unreasonable thing to do.  But just making CREATE
> DOMAIN emit "CREATE DOMAIN" isn't improving consistency at all.

I assumed Peter meant some kind of ANSI SQL standard, but I am kind of
lost how they define that level of detail in the standard.  I agree a
wholesale cleanup there would be a good idea.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Re: [PATCHES] Fixes gram.y

From
Peter Eisentraut
Date:
Tom Lane writes:

> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Peter Eisentraut wrote:
> >> There is a standard for this.  CREATE DOMAIN shows CREATE DOMAIN.
>
> > OK, CVS changed to emit CREATE DOMAIN.
>
> What's standard about it?

ISO/IEC 9075-2:1999 clause 19.1 general rule 1 c) to be exact. ;-)

--
Peter Eisentraut   peter_e@gmx.net


Re: [PATCHES] Fixes gram.y

From
Tom Lane
Date:
Peter Eisentraut <peter_e@gmx.net> writes:
>> What's standard about it?

> ISO/IEC 9075-2:1999 clause 19.1 general rule 1 c) to be exact. ;-)

Hmm.  Looks like we need a wholesale revision of command tags, indeed.
At least if we want to consider command tags to be the data that
satisfies this spec requirement.

            regards, tom lane

Re: [PATCHES] Fixes gram.y

From
Peter Eisentraut
Date:
Tom Lane writes:

> Peter Eisentraut <peter_e@gmx.net> writes:
> >> What's standard about it?
>
> > ISO/IEC 9075-2:1999 clause 19.1 general rule 1 c) to be exact. ;-)
>
> Hmm.  Looks like we need a wholesale revision of command tags, indeed.
> At least if we want to consider command tags to be the data that
> satisfies this spec requirement.

We would need to do:

ALTER  -> ALTER <type of object>
DROP   -> DROP <type of object>
CREATE -> CREATE <type of object>

Those look reasonable, and we already do that in some cases.

CLOSE   -> CLOSE CURSOR
DECLARE -> DECLARE CURSOR

No opinion here.

COMMIT   -> COMMIT WORK
ROLLBACK -> ROLLBACK WORK

Doesn't matter to me.

DELETE -> DELETE WHERE
UPDATE -> UPDATE WHERE

I'd prefer not to do those.

SET CONSTRAINTS -> SET CONSTRAINT [sic]
SET VARIABLE    -> SET TIME ZONE
SET VARIABLE    -> SET TRANSACTION
SET VARIABLE    -> SET SESSION AUTHORIZATION

The first one looks like a mistake.  The other ones we could work on.

It also seems to me that CREATE TABLE AS should not print "SELECT".  I
thought Fernando Nasser had fixed that.  Maybe I'm not completely up to
date in my sources.

--
Peter Eisentraut   peter_e@gmx.net


Re: [PATCHES] Fixes gram.y

From
Tom Lane
Date:
Peter Eisentraut <peter_e@gmx.net> writes:
> Tom Lane writes:
>> Hmm.  Looks like we need a wholesale revision of command tags, indeed.

> We would need to do:

> ALTER  -> ALTER <type of object>
> DROP   -> DROP <type of object>
> CREATE -> CREATE <type of object>
> Those look reasonable, and we already do that in some cases.

These seem okay to me.

> CLOSE   -> CLOSE CURSOR
> DECLARE -> DECLARE CURSOR
> No opinion here.

No strong feeling here either.

> COMMIT   -> COMMIT WORK
> ROLLBACK -> ROLLBACK WORK
> Doesn't matter to me.

I'd vote against changing these.

> DELETE -> DELETE WHERE
> UPDATE -> UPDATE WHERE
> I'd prefer not to do those.

If we change these we will break existing client code that expects a
particular format for these tags (so it can pull out the row count).
Definitely a "no change" vote here.

> SET CONSTRAINTS -> SET CONSTRAINT [sic]
> SET VARIABLE    -> SET TIME ZONE
> SET VARIABLE    -> SET TRANSACTION
> SET VARIABLE    -> SET SESSION AUTHORIZATION
> The first one looks like a mistake.  The other ones we could work on.

I'd say leave them all as "SET VARIABLE".  There's no real information
gain here, and I'm a tad worried about overflowing limited command-tag
buffers in clients.

> It also seems to me that CREATE TABLE AS should not print "SELECT".  I
> thought Fernando Nasser had fixed that.

No, I think it's still on his to-do list (we didn't like his first
proposed patch for it).

            regards, tom lane

Re: [PATCHES] Fixes gram.y

From
Fernando Nasser
Date:
Tom Lane wrote:
>
> > It also seems to me that CREATE TABLE AS should not print "SELECT".  I
> > thought Fernando Nasser had fixed that.
>
> No, I think it's still on his to-do list (we didn't like his first
> proposed patch for it).
>

Yes, I am supposed to see if I can fix this and get rid of the "into"
field
in SelectStmt at the same time.  Right Tom?

--
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Re: [PATCHES] Fixes gram.y

From
Tom Lane
Date:
Fernando Nasser <fnasser@redhat.com> writes:
> Yes, I am supposed to see if I can fix this and get rid of the "into"
> field in SelectStmt at the same time.  Right Tom?

Yeah, we had talked about that ... but I'm not sure it's worth the
trouble.  I don't see any clean way for the SELECT grammar rule to
return info about an INTO clause, other than by including it in
SelectStmt.

Probably the easiest answer is for CreateCommandTag to just deal with
drilling down into the parsetree to see if INTO appears.

            regards, tom lane

Re: [PATCHES] Fixes gram.y

From
Bruce Momjian
Date:
Tom Lane wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
> > Tom Lane writes:
> >> Hmm.  Looks like we need a wholesale revision of command tags, indeed.
>
> > We would need to do:
>
> > ALTER  -> ALTER <type of object>
> > DROP   -> DROP <type of object>
> > CREATE -> CREATE <type of object>
> > Those look reasonable, and we already do that in some cases.
>
> These seem okay to me.

Yep, makes sense.

> > CLOSE   -> CLOSE CURSOR
> > DECLARE -> DECLARE CURSOR
> > No opinion here.
>
> No strong feeling here either.

Seems like extra noise.  Not sure either.

>
> > COMMIT   -> COMMIT WORK
> > ROLLBACK -> ROLLBACK WORK
> > Doesn't matter to me.
>
> I'd vote against changing these.

OK.

> > DELETE -> DELETE WHERE
> > UPDATE -> UPDATE WHERE
> > I'd prefer not to do those.
>
> If we change these we will break existing client code that expects a
> particular format for these tags (so it can pull out the row count).
> Definitely a "no change" vote here.
>

Hard to imagine what logic you would use to add the word WHERE.  What if
they do a DELETE without a WHERE?


> > SET CONSTRAINTS -> SET CONSTRAINT [sic]
> > SET VARIABLE    -> SET TIME ZONE
> > SET VARIABLE    -> SET TRANSACTION
> > SET VARIABLE    -> SET SESSION AUTHORIZATION
> > The first one looks like a mistake.  The other ones we could work on.
>
> I'd say leave them all as "SET VARIABLE".  There's no real information
> gain here, and I'm a tad worried about overflowing limited command-tag
> buffers in clients.

Yes, the problem here is that we have so many SET variables that aren't
standard, do we print the standard tags for the standard ones and just
SET VARIABLE for the others?  Doesn't seem worth it.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026