Fixes gram.y

Started by Rod Tayloralmost 24 years ago16 messages
#1Rod Taylor
rbt@zort.ca
1 attachment(s)

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.

Attachments:

gram.diffapplication/octet-stream; name=gram.diffDownload
*** gram.y.orig	Tue Mar 19 07:27:48 2002
--- gram.y	Tue Mar 19 07:30:24 2002
***************
*** 3184,3205 ****
  				{
  					$$ = lconsi(3, makeListi1(-1));
  				}
- 		;
- 
- 
- /*****************************************************************************
-  *
-  *		DROP DATABASE
-  *
-  *
-  *****************************************************************************/
- 
- DropdbStmt:	DROP DATABASE database_name
- 				{
- 					DropdbStmt *n = makeNode(DropdbStmt);
- 					n->dbname = $3;
- 					$$ = (Node *)n;
- 				}
  		| OWNER opt_equal name 
  				{
  					$$ = lconsi(4, makeList1($3));
--- 3184,3189 ----
***************
*** 3217,3222 ****
--- 3201,3221 ----
  opt_equal: '='								{ $$ = TRUE; }
  		| /*EMPTY*/							{ $$ = FALSE; }
  		;
+ 
+ 
+ /*****************************************************************************
+  *
+  *		DROP DATABASE
+  *
+  *
+  *****************************************************************************/
+ 
+ DropdbStmt:	DROP DATABASE database_name
+ 				{
+ 					DropdbStmt *n = makeNode(DropdbStmt);
+ 					n->dbname = $3;
+ 					$$ = (Node *)n;
+ 				};
  
  
  /*****************************************************************************
#2Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Rod Taylor (#1)
1 attachment(s)
Re: [HACKERS] Fixes gram.y

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

Attachments:

/bjm/difftext/plainDownload
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:
#3Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Rod Taylor (#1)
Re: [HACKERS] Fixes gram.y

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
#4Thomas Lockhart
lockhart@fourpalms.org
In reply to: Bruce Momjian (#2)
Re: [HACKERS] Fixes gram.y

...

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

#5Peter Eisentraut
peter_e@gmx.net
In reply to: Bruce Momjian (#2)
Re: [HACKERS] Fixes gram.y

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

#6Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Peter Eisentraut (#5)
Re: [HACKERS] Fixes gram.y

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
#7Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Thomas Lockhart (#4)
Re: [HACKERS] Fixes gram.y

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
#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#6)
Re: [HACKERS] Fixes gram.y

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

#9Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Tom Lane (#8)
Re: [HACKERS] Fixes gram.y

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
#10Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#8)
Re: [HACKERS] Fixes gram.y

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

#11Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#10)
Re: [HACKERS] Fixes gram.y

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

#12Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#11)
Re: [HACKERS] Fixes gram.y

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

#13Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#12)
Re: [HACKERS] Fixes gram.y

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

#14Fernando Nasser
fnasser@redhat.com
In reply to: Peter Eisentraut (#12)
Re: [HACKERS] Fixes gram.y

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

#15Tom Lane
tgl@sss.pgh.pa.us
In reply to: Fernando Nasser (#14)
Re: [HACKERS] Fixes gram.y

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

#16Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Tom Lane (#13)
Re: [HACKERS] Fixes gram.y

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