Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

TODO question

0 views
Skip to first unread message

Bruce Momjian

unread,
Mar 7, 2002, 8:48:22 PM3/7/02
to

Double-oops. There is a later one under a different email subject
called TODO questino that is a context diff. I just need to know if
this is the newest version and if anyone doesn't like it.

This adds DEFAULT functionality to the INSERT values list.

> 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 (p...@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 (p...@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 (p...@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 (p...@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 (p...@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
>

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majo...@postgresql.org so that your
> message can get through to the mailing list cleanly

--
Bruce Momjian | http://candle.pha.pa.us
pg...@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

---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majo...@postgresql.org so that your
message can get through to the mailing list cleanly

Bruce Momjian

unread,
Mar 7, 2002, 9:45:19 PM3/7/02
to

Good analysis. Thanks. Pavlov, I know this code is complex. I think
these tips will help.

---------------------------------------------------------------------------

Tom Lane wrote:


> Bruce Momjian <pg...@candle.pha.pa.us> writes:
> > Double-oops. There is a later one under a different email subject
> > called TODO questino that is a context diff. I just need to know if
> > this is the newest version and if anyone doesn't like it.
>

> I don't like it. As given, it inserts default values into the query
> at parse time, whereas this must not be done until planning time.
> (Otherwise the defaults sneak into stored rules, and if you change
> defaults with ALTER TABLE you will get unexpected results.) The
> correct (and actually easier) way is to simply drop the defaulted column
> out of the analyzed query altogether.
>
> This is not Pavlo's fault exactly, since he copied the way we used
> to do it in 7.1 ... but the patch must be updated to follow 7.2
> practice.
>
> Another problem: no copy/equal/outfuncs support for the added node type.
>
> Stylistic issue: we should discourage people from putting their initials
> on every bit of code they touch. The code will soon be unreadable if
> such becomes common practice.
>
> regards, tom lane

Pavlo Baron

unread,
Mar 9, 2002, 3:41:05 AM3/9/02
to
Tom Lane wrote:

> I don't like it. As given, it inserts default values into the query
> at parse time, whereas this must not be done until planning time.
> (Otherwise the defaults sneak into stored rules, and if you change
> defaults with ALTER TABLE you will get unexpected results.) The

:(
But of course you are right - my solution was just a hack to learn about the
internals and I really didn't expect my first patch to be accepted.

> correct (and actually easier) way is to simply drop the defaulted column
> out of the analyzed query altogether.

I know what you mean.

>
> This is not Pavlo's fault exactly, since he copied the way we used
> to do it in 7.1 ... but the patch must be updated to follow 7.2
> practice.

I coudn't know about such details. But I see, my solution wasn't completely
wrong.

>
> Another problem: no copy/equal/outfuncs support for the added node type.

Is it also interesting in the case of the DEFAULT-pair being droped?

>
> Stylistic issue: we should discourage people from putting their initials
> on every bit of code they touch. The code will soon be unreadable if
> such becomes common practice.

Understandable. I don't want to immortalize my initials in a foreign work.
My comments where a kind of orientaition marks for me to quickly find my own
code. As you know, I just wanted to know if I'm on the right way
experimenting with this TODO-issue.

Sorry, but I don't think I should provide this patch. I really haven't
enough energy and time to become an internals-GURU. This software is too
mighty and it's internals contain too many special thoughts of it's core
developers, so I would never get a required motivation to participate the
kernal development.
What I'll look for in the next time is what I wanted to do at the beginning
of my postgresql-dev-adventure. I'm developing very complex applications
based on Oracle. IMHO, it's an expensive and overheaded colossus, so Pg is
my absolute favorite when I develop a low cost web-based-solution for smb. w
ho don't want (or can't) pay a sack of money for the database.
Ross J. Reedstrom pointed me to the developer group working on the
Oracle-related issues. So I'll contact those guys to discuss some ideas I've
got after working with both Pg and Oracle.

BTW, here's smth. really funny about that TODO issue: smb. sent me an
SQL-script containig inserts using that "default" placeholder, and my
modified Pg-version did it! But now, I use the stable without my
modifications.

rgds
Pavlo Baron


---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster

Tom Lane

unread,
Mar 9, 2002, 10:27:44 AM3/9/02
to
"Pavlo Baron" <p...@pbit.org> writes:
>> Another problem: no copy/equal/outfuncs support for the added node type.

> Is it also interesting in the case of the DEFAULT-pair being droped?

Yes, mainly on general principles: for example, debug dumps of raw parse
trees will fail if there's not outfuncs support for your node type.

We've been lax on this in the past but I think it's important to try to
keep those support routines complete.

> Sorry, but I don't think I should provide this patch.

Sorry to hear that. It needs to be done, and you are not that far away
from having it done.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to majo...@postgresql.org)

0 new messages