Thread: Fixes gram.y
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.
Attachment
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:
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
...
> 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
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
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
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
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
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
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
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
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
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
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
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
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