TODO question
what is meant by the TODO-item "Disallow missing columns in INSERT ...
VALUES, per ANSI" ?
rgds
Pavlo Baron
On Thu, 27 Dec 2001, Pavlo Baron wrote:
what is meant by the TODO-item "Disallow missing columns in INSERT ...
VALUES, per ANSI" ?
INSERT INTO foo(a,b) values(1,2,3);
Vince.
--
==========================================================================
Vince Vielhaber -- KA8CSH email: vev@michvhf.com http://www.pop4.net
56K Nationwide Dialup from $16.00/mo at Pop4 Networking
Online Campground Directory http://www.camping-usa.com
Online Giftshop Superstore http://www.cloudninegifts.com
==========================================================================
Vince Vielhaber:
INSERT INTO foo(a,b) values(1,2,3);
sorry, but I'm a little bit confused: what should or should not happen with
this:
if I execute this INSERT on a table having 2 cols, an error is generated:
ERROR: INSERT has more expressions than target columns
has it already been fixed? or what should be exactly disallowed? every time
I execute such statement with any cols/vals combination having different
cols/vals number it fails. Should it be allowed now?
rgds
Pavlo Baron
Vince Vielhaber <vev@michvhf.com> writes:
On Thu, 27 Dec 2001, Pavlo Baron wrote:
what is meant by the TODO-item "Disallow missing columns in INSERT ...
VALUES, per ANSI" ?
INSERT INTO foo(a,b) values(1,2,3);
I think you mean
regression=# INSERT INTO foo(a,b,c) values(1,2);
INSERT 172219 1
as the other behaves as expected:
regression=# INSERT INTO foo(a,b) values(1,2,3);
ERROR: INSERT has more expressions than target columns
I am not sure why this is on the TODO list, as opposed to having been
fixed out-of-hand; it sure doesn't look complex to the naked eye.
Perhaps there were some compatibility concerns, or some other issue.
Pavlo would be well advised to go search the archives for awhile to
understand the background of the TODO item.
regards, tom lane
On Thu, 27 Dec 2001, Pavlo Baron wrote:
Vince Vielhaber:
INSERT INTO foo(a,b) values(1,2,3);
sorry, but I'm a little bit confused: what should or should not happen with
this:
if I execute this INSERT on a table having 2 cols, an error is generated:
ERROR: INSERT has more expressions than target columnshas it already been fixed? or what should be exactly disallowed? every time
I execute such statement with any cols/vals combination having different
cols/vals number it fails. Should it be allowed now?
Sorry, I had it backwards.
insert into foo(a,b,c) values(1,2);
Vince.
--
==========================================================================
Vince Vielhaber -- KA8CSH email: vev@michvhf.com http://www.pop4.net
56K Nationwide Dialup from $16.00/mo at Pop4 Networking
Online Campground Directory http://www.camping-usa.com
Online Giftshop Superstore http://www.cloudninegifts.com
==========================================================================
I am not sure why this is on the TODO list, as opposed to having been
fixed out-of-hand; it sure doesn't look complex to the naked eye.
Perhaps there were some compatibility concerns, or some other issue.
Pavlo would be well advised to go search the archives for awhile to
understand the background of the TODO item.
It has the potential to break existing queries so I don't think we were
worked up about fixing it quickly. Whoever does will have to weather
the complaints, but I agree it should be done. :-)
There are actually lots of nice easy items on the TODO list. Most
people that they are easy for are focusing on other things. I found
some time to pick off some easy ones for 7.2 and I hope more for 7.3.
--
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 19026I am not sure why this is on the TODO list, as opposed to having been
fixed out-of-hand; it sure doesn't look complex to the naked eye.
Perhaps there were some compatibility concerns, or some other issue.
Pavlo would be well advised to go search the archives for awhile to
understand the background of the TODO item.
Actually, the bigest problem with these small items is not the coding
but making a patch everyone likes (at least for me). :-)
--
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 19026Bruce Momjian:
Actually, the bigest problem with these small items is not the coding
but making a patch everyone likes (at least for me). :-)
ok, but what's the exact process of integrating patches made by new
contributers (or smb. who tries to become a contributer, smb. like me ,-) )?
I made (and remade, thank you Tom) a patch for "Allow INSERT INTO my_table
VALUES (a, b, c, DEFAULT, x, y, z, ...)" from the TODO list being a small
nice item. How do I know if it's been accepted? I explicitely wanted to
begin with such small items, because: what looks small'n'easy for you guys
would take some time for me to implement, and if I've got it after I jogged
through the whole parser, maybe it would look small'n'easy for me, too.
rgds
Pavlo Baron
ok, but what's the exact process of integrating patches made by new
contributers (or smb. who tries to become a contributer, smb. like me ,-) )?
I made (and remade, thank you Tom) a patch for "Allow INSERT INTO my_table
VALUES (a, b, c, DEFAULT, x, y, z, ...)" from the TODO list being a small
nice item. How do I know if it's been accepted? I explicitely wanted to
begin with such small items, because: what looks small'n'easy for you guys
would take some time for me to implement, and if I've got it after I jogged
through the whole parser, maybe it would look small'n'easy for me, too.
You have seen the process for acceptance, with the exception of the "OK,
applied!" exchange that happens at the end. That is because we have not
quite released 7.2, and the source tree is essentially frozen for the
next month or so.
btw, it *may* be premature to assume that your patches are completely
accepted yet, since none of us are very focused on new features at this
moment.
If you had submitted patches three months ago, they would be in the 7.2
release ;)
- Thomas
Thomas Lockhart <lockhart@fourpalms.org> writes:
btw, it *may* be premature to assume that your patches are completely
accepted yet, since none of us are very focused on new features at this
moment.
In fact the patch seemed quite incomplete to me; adding a new parsenode
type requires much more than just a struct declaration. But this isn't
the right time of the cycle to be reviewing new-feature patches.
BTW, patches should usually be sent to pgsql-patches not pgsql-hackers.
regards, tom lane
Tom Lane:
In fact the patch seemed quite incomplete to me; adding a new parsenode
type requires much more than just a struct declaration.
btw, it's not correct, that just a new structure has been declared. I added
the T_Default to the Type-Enum and it seems to me, my new parsenode type has
been full-automatically integrated in the parser-workflow. In the gram.y,
there is a new set of rules describing the DEFAULT value in the INSERT
stmt - this is the place, where it's being identified and node-ed (using
it's type), the transformation has got the new T_Default-case leaving this
node "as is", and it's being transformed (replaced by the default value
taken from the relation specified by the corresponding parsestate-field)
later.
But this isn't
the right time of the cycle to be reviewing new-feature patches.
ok, but I hope you've got a 3%-free--ear-capacity at least to answer some of
my questions (having a very bad timing ,-) ). I don't ask offen and about
every step, but sometimes it breaks through...
BTW, patches should usually be sent to pgsql-patches not pgsql-hackers.
...where they will get dusty before the new release has been finished... ,)
no problem, I'll wait a little with my patches but not with my questions ,)
sorry if I increased your current stress level :-)
rgds
Pavlo Baron
Thomas Lockhart:
You have seen the process for acceptance, with the exception of the "OK,
applied!" exchange that happens at the end. That is because we have not
quite released 7.2, and the source tree is essentially frozen for the
next month or so.
no problem, if I don't hear a *NO!*, it could be a *maybe yes...*
btw, it *may* be premature to assume that your patches are completely
accepted yet, since none of us are very focused on new features at this
moment.
I really don't assume the immediate acceptance of my patches, I'm just
impregnating the codebase, so I ask some questions, and there are, IMHO, not
very lot of them, and I hope it's not a big problem if I submit my patches
sometimes with a complete description on what I did so nobody really needs
(I know very very very precise about a Release completion phase %-( )
immediately to take a look at the code I wrote but possibly at the concept I
used doing that.
If you had submitted patches three months ago, they would be in the 7.2
release ;)
I think, my timing is perfect! Before you guys are got rid of the 7.2, I'll
be ready to take care of some new features and learn it by doing and
providing portions which have a chance to survive! I think, I've got a lot
of ideas and usefull experiences, so may be some of them would be usefull
for PostgreSQL, too.
Excuse me my enthusiasm, but I don't think it's a problem
rgds
Pavlo Baron
Tom Lane:
In fact the patch seemed quite incomplete to me; adding a new parsenode
type requires much more than just a struct declaration.btw, it's not correct, that just a new structure has been declared. I added
the T_Default to the Type-Enum and it seems to me, my new parsenode type has
been full-automatically integrated in the parser-workflow. In the gram.y,
there is a new set of rules describing the DEFAULT value in the INSERT
stmt - this is the place, where it's being identified and node-ed (using
it's type), the transformation has got the new T_Default-case leaving this
node "as is", and it's being transformed (replaced by the default value
taken from the relation specified by the corresponding parsestate-field)
later.But this isn't
the right time of the cycle to be reviewing new-feature patches.ok, but I hope you've got a 3%-free--ear-capacity at least to answer some of
my questions (having a very bad timing ,-) ). I don't ask offen and about
every step, but sometimes it breaks through...
Sure, ask away. We will do our best. In fact, adding a new node is
pretty tricky. There is a developer's FAQ item about it, number 7. I
assume you read that already.
I may be able to take you patch and add the needed node support stuff,
and send the patch back to you so you can continue on 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 19026Bruce Momjian writes:
Sure, ask away. We will do our best. In fact, adding a new node is
pretty tricky. There is a developer's FAQ item about it, number 7. I
assume you read that already.
hm...do you mean the current version of FAQ on the web? there are 2 seventh
items in the table of contents, both jumping to
----------------------------------------snip--------------------------------
-
7) I just added a field to a structure. What else should I do?
The structures passing around from the parser, rewrite, optimizer, and
executor require quite a bit of support. Most structures have support
routines in src/backend/nodes used to create, copy, read, and output those
structures. Make sure you add support for your new field to these files.
Find any other places the structure may need code for your new field. mkid
is helpful with this (see above).
----------------------------------------snap--------------------------------
-
Is that what you mean? It confuses me a bit, surely because I'm new here...
I didn't change any existing struct, and coudn't find any struct I which
could grow bacause of my new parsenode type. The type-enum contains a
corresponding range of values (I think, it was smth. starting with 700 upto
...) - I just added my new type here.
Or do I use a wrong copy of FAQ?
I may be able to take you patch and add the needed node support stuff,
and send the patch back to you so you can continue on it.
thanx. please, feel free - I attached my last patch to this email.
I see, that my patch provides the needed operation, but maybe it's not
enough and could cause some side effects. I'm looking forward to your
modifications (I hope it doesn't keep you from your current work, at least
not crucial). What I'm just interested in is, if my changes would be a
subset of your code or if what I did is an absolute b*-sh* ,)
rgds
Pavlo Baron
Index: src/backend/parser/parse_target.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/parser/parse_target.c,v
retrieving revision 1.76
diff -c -r1.76 parse_target.c
*** src/backend/parser/parse_target.c 2001/11/05 17:46:26 1.76
--- src/backend/parser/parse_target.c 2001/12/28 23:13:43
***************
*** 60,65 ****
--- 60,77 ----
if (IsA(expr, Ident) &&((Ident *) expr)->isRel)
elog(ERROR, "You can't use relation names alone in the target list, try
relation.*.");+ /* pavlo (pb@pbit.org: 2001-12-27: handle the DEFAULT in INSERT
INTO foo VALUES (..., DEFAULT, ...))*/
+ if (IsA(expr, Default))
+ {
+ if (pstate->p_target_relation->rd_att->attrs[(AttrNumber)
pstate->p_last_resno - 1]->atthasdef)
+ {
+ Const *con = (Const *)
stringToNode(pstate->p_target_relation->rd_att->constr->defval[(AttrNumber)
pstate->p_last_resno - 1].adbin);
+ expr = con;
+ }
+ else
+ elog(ERROR, "no default value for column \"%s\"
found\nDEFAULT cannot be inserted", colname);
+ }
+
type_id = exprType(expr);
type_mod = exprTypmod(expr);***************
*** 260,266 ****
{
tle->expr = CoerceTargetExpr(pstate, tle->expr, type_id,
attrtype, attrtypmod);
! if (tle->expr == NULL)
elog(ERROR, "column \"%s\" is of type '%s'"
" but expression is of type '%s'"
"\n\tYou will need to rewrite or cast the expression",
--- 272,278 ----
{
tle->expr = CoerceTargetExpr(pstate, tle->expr, type_id,
attrtype, attrtypmod);
! if (tle->expr == NULL)
elog(ERROR, "column \"%s\" is of type '%s'"
" but expression is of type '%s'"
"\n\tYou will need to rewrite or cast the expression",
***************
*** 299,305 ****
int32 attrtypmod)
{
if (can_coerce_type(1, &type_id, &attrtype))
! expr = coerce_type(pstate, expr, type_id, attrtype, attrtypmod);#ifndef DISABLE_STRING_HACKS
--- 311,317 ----
int32 attrtypmod)
{
if (can_coerce_type(1, &type_id, &attrtype))
! expr = coerce_type(pstate, expr, type_id, attrtype,
attrtypmod);#ifndef DISABLE_STRING_HACKS
***************
*** 525,528 ****
}
return strength;
! }
--- 537,540 ----
} return strength;
! }
\ No newline at end of file
Index: src/backend/parser/parse_expr.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/parser/parse_expr.c,v
retrieving revision 1.105
diff -c -r1.105 parse_expr.c
*** src/backend/parser/parse_expr.c 2001/11/12 20:05:24 1.105
--- src/backend/parser/parse_expr.c 2001/12/28 23:15:08
***************
*** 121,126 ****
--- 121,131 ----
result = (Node *) make_const(val);
break;
}
+ case T_Default: /* pavlo (pb@pbit.org): 2001-12-27:
transormation for the DEFAULT value for INSERT INTO foo VALUES (...,
DEFAULT, ...)*/
+ {
+ result = (Default *) expr;
+ break;
+ }
case T_ParamNo:
{
ParamNo *pno = (ParamNo *) expr;
***************
*** 1066,1069 ****
}
else
return typename->name;
! }
--- 1071,1074 ----
}
else
return typename->name;
! }
\ No newline at end of file
Index: src/backend/parser/gram.y
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/parser/gram.y,v
retrieving revision 2.276
diff -c -r2.276 gram.y
*** src/backend/parser/gram.y 2001/12/09 04:39:39 2.276
--- src/backend/parser/gram.y 2001/12/28 23:15:39
***************
*** 195,201 ****
opt_column_list, columnList, opt_name_list,
sort_clause, sortby_list, index_params, index_list, name_list,
from_clause, from_list, opt_array_bounds,
! expr_list, attrs, target_list, update_target_list,
def_list, opt_indirection, group_clause, TriggerFuncArgs,
select_limit, opt_select_limit--- 195,201 ----
opt_column_list, columnList, opt_name_list,
sort_clause, sortby_list, index_params, index_list, name_list,
from_clause, from_list, opt_array_bounds,
! expr_list, attrs, target_list, insert_target_list, update_target_list,
def_list, opt_indirection, group_clause, TriggerFuncArgs,
select_limit, opt_select_limit***************
*** 253,259 ****
%type <node> table_ref
%type <jexpr> joined_table
%type <range> relation_expr
! %type <target> target_el, update_target_el
%type <paramno> ParamNo
%type <typnam> Typename, SimpleTypename, ConstTypename
--- 253,259 ----
%type <node> table_ref
%type <jexpr> joined_table
%type <range> relation_expr
! %type <target> target_el, update_target_el, insert_target_el
%type <paramno> ParamNo %type <typnam> Typename, SimpleTypename, ConstTypename
***************
*** 3302,3308 ****
}
;
! insert_rest: VALUES '(' target_list ')'
{
$$ = makeNode(InsertStmt);
$$->cols = NIL;
--- 3302,3308 ----
}
;! insert_rest: VALUES '(' insert_target_list ')'
{
$$ = makeNode(InsertStmt);
$$->cols = NIL;
***************
*** 5482,5488 ****
*
****************************************************************************
*/
! /* Target lists as found in SELECT ... and INSERT VALUES ( ... ) */
target_list: target_list ',' target_el
{ $$ = lappend($1, $3); }
--- 5482,5488 ----
*****************************************************************************
*/
! /* Target lists as found in SELECT ... */
target_list: target_list ',' target_el
{ $$ = lappend($1, $3); }
***************
*** 5490,5496 ****
--- 5490,5522 ----
{ $$ = makeList1($1); }
;+ /* Target lists as found in INSERT VALUES ( ... ) */
+
+ /* pavlo (pb@pbit.org): 2001-12-27: parse node based handling for the
DEFAULT value added;
+ now it's possible to INSERT INTO ... VALUES (..., FEFAULT, ...)!
+ */
+ insert_target_list: insert_target_list ',' insert_target_el
+ { $$ = lappend($1, $3); }
+ | insert_target_el
+ { $$ = makeList1($1); }
+ | insert_target_list ',' target_el
+ { $$ = lappend($1, $3); }
+ | target_el
+ { $$ = makeList1($1); }
+ ;
+
+ insert_target_el: DEFAULT
+ {
+ Default *n = makeNode(Default);
+ $$ = makeNode(ResTarget);
+ $$->name = NULL;
+ $$->indirection = NULL;
+ $$->val = (Node *)n;
+ }
+ ;
+
/* AS is not optional because shift/red conflict with unary ops */
+
target_el: a_expr AS ColLabel
{
$$ = makeNode(ResTarget);
***************
*** 6380,6383 ****
strcpy(newval+1, oldval);
v->val.str = newval;
}
! }
--- 6406,6409 ----
strcpy(newval+1, oldval);
v->val.str = newval;
}
! }
\ No newline at end of file
Index: src/include/nodes/parsenodes.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/nodes/parsenodes.h,v
retrieving revision 1.151
diff -c -r1.151 parsenodes.h
*** src/include/nodes/parsenodes.h 2001/11/05 17:46:34 1.151
--- src/include/nodes/parsenodes.h 2001/12/28 23:16:30
***************
*** 1005,1010 ****
--- 1005,1019 ----
} A_Const; /*
+ * pavlo (pb@pbit.org): 2001.12.27:
+ * Default - the DEFAULT constant expression used in the target list of
INSERT INTO ... VALUES (..., DEFAULT, ...)
+ */
+ typedef struct Default
+ {
+ NodeTag type;
+ } Default;
+
+ /*
* TypeCast - a CAST expression
*
* NOTE: for mostly historical reasons, A_Const and ParamNo parsenodes
contain
***************
*** 1355,1358 ****
*/
typedef SortClause GroupClause;! #endif /* PARSENODES_H */
--- 1364,1367 ----
*/
typedef SortClause GroupClause;! #endif /* PARSENODES_H */
\ No newline at end of file
Attachments:
patch.diffapplication/octet-stream; name=patch.diffDownload+72-20
BTW, inserting the actual default expression in the parser is no good.
We just finished getting rid of that behavior last month:
2001-11-02 15:23 tgl
* src/backend/: catalog/heap.c, optimizer/prep/preptlist.c,
parser/analyze.c: Add default expressions to INSERTs during
planning, not during parse analysis. This keeps stored rules from
prematurely absorbing default information, which is necessary for
ALTER TABLE SET DEFAULT to work unsurprisingly with rules. See
pgsql-bugs discussion 24-Oct-01.
The way I would tackle this is to do the work in
transformInsertStmt: if you find a Default node in the input targetlist,
you can simply omit the corresponding entry of the column list. In
other words, transform
INSERT INTO foo (col1, col2, col3) VALUES (11, DEFAULT, 13);
into
INSERT INTO foo (col1, col3) VALUES (11, 13);
Then you can rely on the planner to insert the correct defaults at planning
time.
My inclination would be to twiddle the order of operations so that the
Default node is spotted and intercepted before being fed to
transformExpr. This would probably mean doing some surgery on
transformTargetList. The advantage of doing it that way is that
transformExpr and subsequent processing need never see Default nodes
and can just error out if they do. The way you have it, Default needs
to pass through transformExpr, which raises a bunch of questions in my
mind about what parts of the system will need to accept it. There's
an established distinction between "pre-parse-analysis" node types
(eg, Attr) and "post-parse-analysis" node types (eg, Var), and we
understand which places need to support which node types as long as
that's the distinction. If you allow Default to pass through
transformExpr then you're to some extent allowing it to be a
post-parse-analysis node type, which doesn't strike me as a good idea.
regards, tom lane
Bruce Momjian writes:
Sure, ask away. We will do our best. In fact, adding a new node is
pretty tricky. There is a developer's FAQ item about it, number 7. I
assume you read that already.hm...do you mean the current version of FAQ on the web? there are 2 seventh
items in the table of contents, both jumping to
----------------------------------------snip--------------------------------
-
7) I just added a field to a structure. What else should I do?
The structures passing around from the parser, rewrite, optimizer, and
executor require quite a bit of support. Most structures have support
routines in src/backend/nodes used to create, copy, read, and output those
structures. Make sure you add support for your new field to these files.
Find any other places the structure may need code for your new field. mkid
is helpful with this (see above).----------------------------------------snap--------------------------------
-Is that what you mean? It confuses me a bit, surely because I'm new here...
I didn't change any existing struct, and coudn't find any struct I which
could grow bacause of my new parsenode type. The type-enum contains a
corresponding range of values (I think, it was smth. starting with 700 upto
...) - I just added my new type here.
Or do I use a wrong copy of FAQ?
Yes, that is the one. The steps for adding an entry to an existing
structure is similar to add an entirely new one.
I may be able to take you patch and add the needed node support stuff,
and send the patch back to you so you can continue on it.thanx. please, feel free - I attached my last patch to this email.
I see, that my patch provides the needed operation, but maybe it's not
enough and could cause some side effects. I'm looking forward to your
modifications (I hope it doesn't keep you from your current work, at least
not crucial). What I'm just interested in is, if my changes would be a
subset of your code or if what I did is an absolute b*-sh* ,)
I think it would be a subset. I haven't looked at it closely yet,
though.
--
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 19026Bruce Momjian writes:
Sure, ask away. We will do our best. In fact, adding a new node is
pretty tricky. There is a developer's FAQ item about it, number 7. I
assume you read that already.hm...do you mean the current version of FAQ on the web? there are 2 seventh
items in the table of contents, both jumping to
I have updated the developer's FAQ to remove the duplicate entries. I
have wanted to split the FAQ into two sections anyway, and this was a
good time to do 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 19026Bruce Momjian writes:
Sure, ask away. We will do our best. In fact, adding a new node is
pretty tricky. There is a developer's FAQ item about it, number 7. I
assume you read that already.hm...do you mean the current version of FAQ on the web? there are 2
seventh
items in the table of contents, both jumping to
----------------------------------------snip------------------------------
--
-
7) I just added a field to a structure. What else should I do?
The structures passing around from the parser, rewrite, optimizer, and
executor require quite a bit of support. Most structures have support
routines in src/backend/nodes used to create, copy, read, and output
those
structures. Make sure you add support for your new field to these files.
Find any other places the structure may need code for your new field.
mkid
is helpful with this (see above).
----------------------------------------snap------------------------------
--
-
Is that what you mean? It confuses me a bit, surely because I'm new
here...
I didn't change any existing struct, and coudn't find any struct I which
could grow bacause of my new parsenode type. The type-enum contains a
corresponding range of values (I think, it was smth. starting with 700
upto
...) - I just added my new type here.
Or do I use a wrong copy of FAQ?Yes, that is the one. The steps for adding an entry to an existing
structure is similar to add an entirely new one.
What structure could you suggest me to pick out and to follow in the code
which would be handled similarily to my "Default"? This one is a one-way
use'n'drop struct and wouldn't appear anywhere else.
I may be able to take you patch and add the needed node support stuff,
and send the patch back to you so you can continue on it.thanx. please, feel free - I attached my last patch to this email.
I see, that my patch provides the needed operation, but maybe it's not
enough and could cause some side effects. I'm looking forward to your
modifications (I hope it doesn't keep you from your current work, at
least
not crucial). What I'm just interested in is, if my changes would be a
subset of your code or if what I did is an absolute b*-sh* ,)I think it would be a subset. I haven't looked at it closely yet,
though.
oops, a new patch attached: the last one wouldn't compile because of missing
T_Default value I just forgot to diff-in (or out) - sorry
rgds
Pavlo Baron
Index: src/backend/parser/parse_target.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/parser/parse_target.c,v
retrieving revision 1.76
diff -c -r1.76 parse_target.c
*** src/backend/parser/parse_target.c 2001/11/05 17:46:26 1.76
--- src/backend/parser/parse_target.c 2001/12/28 23:13:43
***************
*** 60,65 ****
--- 60,77 ----
if (IsA(expr, Ident) &&((Ident *) expr)->isRel)
elog(ERROR, "You can't use relation names alone in the target list, try
relation.*.");+ /* pavlo (pb@pbit.org: 2001-12-27: handle the DEFAULT in INSERT
INTO foo VALUES (..., DEFAULT, ...))*/
+ if (IsA(expr, Default))
+ {
+ if (pstate->p_target_relation->rd_att->attrs[(AttrNumber)
pstate->p_last_resno - 1]->atthasdef)
+ {
+ Const *con = (Const *)
stringToNode(pstate->p_target_relation->rd_att->constr->defval[(AttrNumber)
pstate->p_last_resno - 1].adbin);
+ expr = con;
+ }
+ else
+ elog(ERROR, "no default value for column \"%s\"
found\nDEFAULT cannot be inserted", colname);
+ }
+
type_id = exprType(expr);
type_mod = exprTypmod(expr);***************
*** 260,266 ****
{
tle->expr = CoerceTargetExpr(pstate, tle->expr, type_id,
attrtype, attrtypmod);
! if (tle->expr == NULL)
elog(ERROR, "column \"%s\" is of type '%s'"
" but expression is of type '%s'"
"\n\tYou will need to rewrite or cast the expression",
--- 272,278 ----
{
tle->expr = CoerceTargetExpr(pstate, tle->expr, type_id,
attrtype, attrtypmod);
! if (tle->expr == NULL)
elog(ERROR, "column \"%s\" is of type '%s'"
" but expression is of type '%s'"
"\n\tYou will need to rewrite or cast the expression",
***************
*** 299,305 ****
int32 attrtypmod)
{
if (can_coerce_type(1, &type_id, &attrtype))
! expr = coerce_type(pstate, expr, type_id, attrtype, attrtypmod);#ifndef DISABLE_STRING_HACKS
--- 311,317 ----
int32 attrtypmod)
{
if (can_coerce_type(1, &type_id, &attrtype))
! expr = coerce_type(pstate, expr, type_id, attrtype,
attrtypmod);#ifndef DISABLE_STRING_HACKS
***************
*** 525,528 ****
}
return strength;
! }
--- 537,540 ----
} return strength;
! }
\ No newline at end of file
Index: src/backend/parser/parse_expr.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/parser/parse_expr.c,v
retrieving revision 1.105
diff -c -r1.105 parse_expr.c
*** src/backend/parser/parse_expr.c 2001/11/12 20:05:24 1.105
--- src/backend/parser/parse_expr.c 2001/12/28 23:15:08
***************
*** 121,126 ****
--- 121,131 ----
result = (Node *) make_const(val);
break;
}
+ case T_Default: /* pavlo (pb@pbit.org): 2001-12-27:
transormation for the DEFAULT value for INSERT INTO foo VALUES (...,
DEFAULT, ...)*/
+ {
+ result = (Default *) expr;
+ break;
+ }
case T_ParamNo:
{
ParamNo *pno = (ParamNo *) expr;
***************
*** 1066,1069 ****
}
else
return typename->name;
! }
--- 1071,1074 ----
}
else
return typename->name;
! }
\ No newline at end of file
Index: src/backend/parser/gram.y
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/parser/gram.y,v
retrieving revision 2.276
diff -c -r2.276 gram.y
*** src/backend/parser/gram.y 2001/12/09 04:39:39 2.276
--- src/backend/parser/gram.y 2001/12/28 23:15:39
***************
*** 195,201 ****
opt_column_list, columnList, opt_name_list,
sort_clause, sortby_list, index_params, index_list, name_list,
from_clause, from_list, opt_array_bounds,
! expr_list, attrs, target_list, update_target_list,
def_list, opt_indirection, group_clause, TriggerFuncArgs,
select_limit, opt_select_limit--- 195,201 ----
opt_column_list, columnList, opt_name_list,
sort_clause, sortby_list, index_params, index_list, name_list,
from_clause, from_list, opt_array_bounds,
! expr_list, attrs, target_list, insert_target_list, update_target_list,
def_list, opt_indirection, group_clause, TriggerFuncArgs,
select_limit, opt_select_limit***************
*** 253,259 ****
%type <node> table_ref
%type <jexpr> joined_table
%type <range> relation_expr
! %type <target> target_el, update_target_el
%type <paramno> ParamNo
%type <typnam> Typename, SimpleTypename, ConstTypename
--- 253,259 ----
%type <node> table_ref
%type <jexpr> joined_table
%type <range> relation_expr
! %type <target> target_el, update_target_el, insert_target_el
%type <paramno> ParamNo %type <typnam> Typename, SimpleTypename, ConstTypename
***************
*** 3302,3308 ****
}
;
! insert_rest: VALUES '(' target_list ')'
{
$$ = makeNode(InsertStmt);
$$->cols = NIL;
--- 3302,3308 ----
}
;! insert_rest: VALUES '(' insert_target_list ')'
{
$$ = makeNode(InsertStmt);
$$->cols = NIL;
***************
*** 5482,5488 ****
*
****************************************************************************
*/
! /* Target lists as found in SELECT ... and INSERT VALUES ( ... ) */
target_list: target_list ',' target_el
{ $$ = lappend($1, $3); }
--- 5482,5488 ----
*****************************************************************************
*/
! /* Target lists as found in SELECT ... */
target_list: target_list ',' target_el
{ $$ = lappend($1, $3); }
***************
*** 5490,5496 ****
--- 5490,5522 ----
{ $$ = makeList1($1); }
;+ /* Target lists as found in INSERT VALUES ( ... ) */
+
+ /* pavlo (pb@pbit.org): 2001-12-27: parse node based handling for the
DEFAULT value added;
+ now it's possible to INSERT INTO ... VALUES (..., FEFAULT, ...)!
+ */
+ insert_target_list: insert_target_list ',' insert_target_el
+ { $$ = lappend($1, $3); }
+ | insert_target_el
+ { $$ = makeList1($1); }
+ | insert_target_list ',' target_el
+ { $$ = lappend($1, $3); }
+ | target_el
+ { $$ = makeList1($1); }
+ ;
+
+ insert_target_el: DEFAULT
+ {
+ Default *n = makeNode(Default);
+ $$ = makeNode(ResTarget);
+ $$->name = NULL;
+ $$->indirection = NULL;
+ $$->val = (Node *)n;
+ }
+ ;
+
/* AS is not optional because shift/red conflict with unary ops */
+
target_el: a_expr AS ColLabel
{
$$ = makeNode(ResTarget);
***************
*** 6380,6383 ****
strcpy(newval+1, oldval);
v->val.str = newval;
}
! }
--- 6406,6409 ----
strcpy(newval+1, oldval);
v->val.str = newval;
}
! }
\ No newline at end of file
Index: src/include/nodes/parsenodes.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/nodes/parsenodes.h,v
retrieving revision 1.151
diff -c -r1.151 parsenodes.h
*** src/include/nodes/parsenodes.h 2001/11/05 17:46:34 1.151
--- src/include/nodes/parsenodes.h 2001/12/28 23:16:30
***************
*** 1005,1010 ****
--- 1005,1019 ----
} A_Const; /*
+ * pavlo (pb@pbit.org): 2001.12.27:
+ * Default - the DEFAULT constant expression used in the target list of
INSERT INTO ... VALUES (..., DEFAULT, ...)
+ */
+ typedef struct Default
+ {
+ NodeTag type;
+ } Default;
+
+ /*
* TypeCast - a CAST expression
*
* NOTE: for mostly historical reasons, A_Const and ParamNo parsenodes
contain
***************
*** 1355,1358 ****
*/
typedef SortClause GroupClause;! #endif /* PARSENODES_H */
--- 1364,1367 ----
*/
typedef SortClause GroupClause;! #endif /* PARSENODES_H */
\ No newline at end of file
Index: src/include/nodes/nodes.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/nodes/nodes.h,v
retrieving revision 1.96
diff -c -r1.96 nodes.h
*** src/include/nodes/nodes.h 2001/11/05 17:46:34 1.96
--- src/include/nodes/nodes.h 2001/12/29 09:23:37
***************
*** 222,227 ****
--- 222,228 ----
T_CaseWhen,
T_FkConstraint,
T_PrivGrantee,
+ T_Default, /* pavlo (pb@pbit.org) : 2001.12.27: DEFAULT element of
the INSERT INTO target list*/ /*
* TAGS FOR FUNCTION-CALL CONTEXT AND RESULTINFO NODES (see fmgr.h)
***************
*** 365,368 ****
(jointype) == JOIN_FULL || \
(jointype) == JOIN_RIGHT)
! #endif /* NODES_H */
--- 366,369 ----
(jointype) == JOIN_FULL || \
(jointype) == JOIN_RIGHT)! #endif /* NODES_H */
\ No newline at end of file
Attachments:
patch.diffapplication/octet-stream; name=patch.diffDownload+75-22
Tom Lane writes:
BTW, inserting the actual default expression in the parser is no good.
We just finished getting rid of that behavior last month:2001-11-02 15:23 tgl
* src/backend/: catalog/heap.c, optimizer/prep/preptlist.c,
parser/analyze.c: Add default expressions to INSERTs during
planning, not during parse analysis. This keeps stored rules from
prematurely absorbing default information, which is necessary for
ALTER TABLE SET DEFAULT to work unsurprisingly with rules. See
pgsql-bugs discussion 24-Oct-01.The way I would tackle this is to do the work in
transformInsertStmt: if you find a Default node in the input targetlist,
you can simply omit the corresponding entry of the column list. In
other words, transformINSERT INTO foo (col1, col2, col3) VALUES (11, DEFAULT, 13);
into
INSERT INTO foo (col1, col3) VALUES (11, 13);
Then you can rely on the planner to insert the correct defaults at
planning
time.
this is an information of gold! I was really unhappy with my hacks looked
like an alien element to me (and surely being those, too). When I look on
the pointer chain I used to get the default value... And I'm not sure if my
solution would handle every kind of default expr. correctly.
My inclination would be to twiddle the order of operations so that the
Default node is spotted and intercepted before being fed to
transformExpr. This would probably mean doing some surgery on
transformTargetList. The advantage of doing it that way is that
transformExpr and subsequent processing need never see Default nodes
and can just error out if they do. The way you have it, Default needs
to pass through transformExpr, which raises a bunch of questions in my
mind about what parts of the system will need to accept it. There's
an established distinction between "pre-parse-analysis" node types
(eg, Attr) and "post-parse-analysis" node types (eg, Var), and we
understand which places need to support which node types as long as
that's the distinction. If you allow Default to pass through
transformExpr then you're to some extent allowing it to be a
post-parse-analysis node type, which doesn't strike me as a good idea.
I see. Can I say in simple words, that everything that can be parsed without
any knowledge about the database condition has to be pre-parse-analized and
the rest has to be post-parse-analized, then? Or, things needed to parse
correctly are prepared by the pre-parser-analysis? Could you point me to a
summary (if any) where I would find a rough desc. on such distinction?
I'll re-code to match the basic concepts you've explained here.
BTW, what about constructs like:
INSERT INTO foo VALUES (1,,2);
wouldn't it make the whole thing a bit simpler? or:
INSERT INTO foo(c1, c2, c3) VALUES (,1,);
I find, this short form is a bit more finger-freiendly and I think, it could
be implemented without dropping the standard one? This would better match
your idea about "to omit" the DEFAULT-ed column, since one doesn't care of
what value has to be put in if you write smth. like "...VALUES
(,,,,,4,,,1,)". Or, there could be an expression defined if not even DEFAULT
has been specified, an other expression than the DEFAULT expr., eg, (I just
spin around :-) ) it could be allowed to define smth. like OMITED appearing
as a place holder for a post-parse replacement where eg column references
could be used, eg:
db=# CREATE TABLE foo (c1 int2, c2 int2, c3 int2 OMITED c1*20, c4 int2);
db=# INSERT INTO foo values (2, 3,, 4);
db=# SELECT c3 FROM foo;
c3
----
40
(1 rows)
rgds
Pavlo Baron
"Pavlo Baron" <pb@pbit.org> writes:
I see. Can I say in simple words, that everything that can be parsed without
any knowledge about the database condition has to be pre-parse-analized and
the rest has to be post-parse-analized, then?
Exactly. The grammar phase has to operate without examining the
database, because it needs to be able to run even in transaction-aborted
state (so we can recognize COMMIT and ROLLBACK commands). Parse
analysis does the lookups and so forth to determine exactly what the
raw syntax tree really means.
summary (if any) where I would find a rough desc. on such distinction?
There's no substitute for reading the code ... the point above is
mentioned in the comments at the head of gram.y, for example.
BTW, what about constructs like:
INSERT INTO foo VALUES (1,,2);
wouldn't it make the whole thing a bit simpler?
Maybe, but it's not SQL92. Looks kind of error-prone to me anyway.
regards, tom lane