Thread: patch: INSERT INTO t VALUES (a, b, ..., DEFAULT, ...)
hello hackers here is the patch for the INSERT INTO t VALUES (..., DEFAULT, ...) from the current TODO list. It's my first patch so please forgive me if it's not the best solution - I'm now learning about the codebase and this implementation really hunted me through some deep and dark places... Your comments on how to do it right or what is additionally to do are welcome, of course Well, here how it works: 1. only 3 files are related, all from the src/backend/parser 2. my comments are all beginning with "pavlo (pb@pbit.org)" 3. gram.y - here I added a rule for the DEFAULT-element in the target_list used for the INSERT-statement. It now replaces DEFAULT by an anti-thing like "@default" because I couldn't find out were it fails if I leave DEFAULT unchainged. If smb. knows a way to do it I'll drop this @default 4. parse_coerce.c - coerce_type - here I faked the standard handling failing when it tries to convert the @default-string 5. parse_target.c - updateTargetListEntry - here the @default-string is handled. I just look up if there is a default value for the column and use it instead of the @default place holder 6. I tested it with psql 7. I tested it under Sun Solaris8 SPARC and FreeBSD 4.0 Intel 8. I used "cvs diff > patch" to diff changes Please let me know if it's ok or if some changes must be done. rgds Pavlo Baron
"Pavlo Baron" <pb@pbit.org> writes:
> 3. gram.y - here I added a rule for the DEFAULT-element in the target_list
> used for the INSERT-statement. It now replaces DEFAULT by an anti-thing like
> "@default" because I couldn't find out were it fails if I leave DEFAULT
> unchainged. If smb. knows a way to do it I'll drop this @default
This would breakINSERT INTO foo(textcolumn) VALUES ('@default')
which I find hardly acceptable.
The only way to do it without breaking valid data entries is to
introduce a new parse node type to represent a DEFAULT placeholder.
I also wonder what's going to happen if I write DEFAULT in a SELECT's
targetlist, which is possible given where you made the grammar change.
regards, tom lane
> "Pavlo Baron" <pb@pbit.org> writes:
> > 3. gram.y - here I added a rule for the DEFAULT-element in the
target_list
> > used for the INSERT-statement. It now replaces DEFAULT by an anti-thing
like
> > "@default" because I couldn't find out were it fails if I leave DEFAULT
> > unchainged. If smb. knows a way to do it I'll drop this @default
>
Tom Lane writes:
> This would break
> INSERT INTO foo(textcolumn) VALUES ('@default')
> which I find hardly acceptable.
>
> The only way to do it without breaking valid data entries is to
> introduce a new parse node type to represent a DEFAULT placeholder.
I know what you mean and I hope to know where to do it. I thought, there
could be some similar cases handled a similar way. I'll try to implement
providing a new parse node type to represent a DEFAULT placeholder.
Tom Lane writes:
>
> I also wonder what's going to happen if I write DEFAULT in a SELECT's
> targetlist, which is possible given where you made the grammar change.
I also thought about it, but maybe I tested a wrong statement. Could you
give me an example on what you mean would possibly appear funny? a
select-statement...
Maybe I don't understand what the targetlist means in the case of the select
statement. I tried smth. like "select f1, f2 from tab1;". I think, "f1, f2"
is a targetlist and if I try to use DEFAULT in the list, a parser error is
generated as it was before I put my changes.
I could specify a new targetlist branch used only in the case of the INSERT
statement, and that's what I had before I minimized my code to that what you
see now. I think, I see a way to declare a rule only used with the INSERT
statement, but I couldn't find any problem caused by using default in the
targetlist of the SELECT stmt. What should we do?
rgds
Pavlo Baron
here is a new patch containing all changes you (Tom) suggested to make. I
still use my "pavlo (pbpbit.org)" for me to locate my code; feel free to
illiminate them before integrating :-)
Tom Lane:
> This would break
> INSERT INTO foo(textcolumn) VALUES ('@default')
> which I find hardly acceptable.
>
> The only way to do it without breaking valid data entries is to
> introduce a new parse node type to represent a DEFAULT placeholder.
Now there is a newly declared parse node type "Default" - the corresponding
structure has no data. The "@default" hack is now illiminated - I'm the
happiest about it
Tom Lane:
>
> I also wonder what's going to happen if I write DEFAULT in a SELECT's
> targetlist, which is possible given where you made the grammar change.
The grammer now contains two new rules: "insert_target_list" and
"insert_target_el", the SELECT and INSERT don't use the same targetlist
anymore, but the "insert_target_el" completely inherits "target_el" to avoid
multiple declarations and it just provides the new DEFAULT-rule.
I hope, this patch is ok - to me, it looks correct now
rgds
Pavlo Baron
Can we get a context diff, diff -c for this?
---------------------------------------------------------------------------
Pavlo Baron wrote:
> here is a new patch containing all changes you (Tom) suggested to make. I
> still use my "pavlo (pbpbit.org)" for me to locate my code; feel free to
> illiminate them before integrating :-)
>
> Tom Lane:
> > This would break
> > INSERT INTO foo(textcolumn) VALUES ('@default')
> > which I find hardly acceptable.
> >
> > The only way to do it without breaking valid data entries is to
> > introduce a new parse node type to represent a DEFAULT placeholder.
>
> Now there is a newly declared parse node type "Default" - the corresponding
> structure has no data. The "@default" hack is now illiminated - I'm the
> happiest about it
>
> Tom Lane:
> >
> > I also wonder what's going to happen if I write DEFAULT in a SELECT's
> > targetlist, which is possible given where you made the grammar change.
>
> The grammer now contains two new rules: "insert_target_list" and
> "insert_target_el", the SELECT and INSERT don't use the same targetlist
> anymore, but the "insert_target_el" completely inherits "target_el" to avoid
> multiple declarations and it just provides the new DEFAULT-rule.
>
> I hope, this patch is ok - to me, it looks correct now
>
> rgds
> Pavlo Baron
>
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/users-lounge/docs/faq.html
-- 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,
Pennsylvania19026
Is this ready for application? Patch is at: http://candle.pha.pa.us/cgi-bin/pgpatches2 --------------------------------------------------------------------------- Pavlo Baron wrote: > here is a new patch containing all changes you (Tom) suggested to make. I > still use my "pavlo (pbpbit.org)" for me to locate my code; feel free to > illiminate them before integrating :-) > > Tom Lane: > > This would break > > INSERT INTO foo(textcolumn) VALUES ('@default') > > which I find hardly acceptable. > > > > The only way to do it without breaking valid data entries is to > > introduce a new parse node type to represent a DEFAULT placeholder. > > Now there is a newly declared parse node type "Default" - the corresponding > structure has no data. The "@default" hack is now illiminated - I'm the > happiest about it > > Tom Lane: > > > > I also wonder what's going to happen if I write DEFAULT in a SELECT's > > targetlist, which is possible given where you made the grammar change. > > The grammer now contains two new rules: "insert_target_list" and > "insert_target_el", the SELECT and INSERT don't use the same targetlist > anymore, but the "insert_target_el" completely inherits "target_el" to avoid > multiple declarations and it just provides the new DEFAULT-rule. > > I hope, this patch is ok - to me, it looks correct now > > rgds > Pavlo Baron > [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 5: Have you checked our extensive FAQ? > > http://www.postgresql.org/users-lounge/docs/faq.html -- 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, Pennsylvania19026
Oops, this is a non-context diff. Would you please resubmit as a
context diff, diff -c?
---------------------------------------------------------------------------
Pavlo Baron wrote:
> here is a new patch containing all changes you (Tom) suggested to make. I
> still use my "pavlo (pbpbit.org)" for me to locate my code; feel free to
> illiminate them before integrating :-)
>
> Tom Lane:
> > This would break
> > INSERT INTO foo(textcolumn) VALUES ('@default')
> > which I find hardly acceptable.
> >
> > The only way to do it without breaking valid data entries is to
> > introduce a new parse node type to represent a DEFAULT placeholder.
>
> Now there is a newly declared parse node type "Default" - the corresponding
> structure has no data. The "@default" hack is now illiminated - I'm the
> happiest about it
>
> Tom Lane:
> >
> > I also wonder what's going to happen if I write DEFAULT in a SELECT's
> > targetlist, which is possible given where you made the grammar change.
>
> The grammer now contains two new rules: "insert_target_list" and
> "insert_target_el", the SELECT and INSERT don't use the same targetlist
> anymore, but the "insert_target_el" completely inherits "target_el" to avoid
> multiple declarations and it just provides the new DEFAULT-rule.
>
> I hope, this patch is ok - to me, it looks correct now
>
> rgds
> Pavlo Baron
>
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/users-lounge/docs/faq.html
-- 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,
Pennsylvania19026