patch: INSERT INTO t VALUES (a, b, ..., DEFAULT, ...)

Started by Pavlo Baronabout 24 years ago7 messages
#1Pavlo Baron
pb@pbit.org
1 attachment(s)

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

Attachments:

patchapplication/octet-stream; name=patchDownload
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Pavlo Baron (#1)
Re: patch: INSERT INTO t VALUES (a, b, ..., DEFAULT, ...)

"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 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 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

#3Pavlo Baron
pb@pbit.org
In reply to: Pavlo Baron (#1)
Re: patch: INSERT INTO t VALUES (a, b, ..., DEFAULT, ...)

"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

#4Pavlo Baron
pb@pbit.org
In reply to: Pavlo Baron (#1)
1 attachment(s)
Re: patch: INSERT INTO t VALUES (a, b, ..., DEFAULT, ...)

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

Attachments:

patch.diffapplication/octet-stream; name=patch.diffDownload
? config.log
? dbs
? config.cache
? config.status
? update.pb.sh
? make.pb.sh
? GNUmakefile
? pb.sql
? restart.pb.sh
? src/GNUmakefile
? src/Makefile.global
? src/backend/postgres
? src/backend/catalog/postgres.bki
? src/backend/catalog/postgres.description
? src/backend/parser/pb
? src/backend/port/Makefile
? src/bin/initdb/initdb
? src/bin/initlocation/initlocation
? src/bin/ipcclean/ipcclean
? src/bin/pg_config/pg_config
? src/bin/pg_ctl/pg_ctl
? src/bin/pg_dump/pg_dump
? src/bin/pg_dump/pg_restore
? src/bin/pg_dump/pg_dumpall
? src/bin/pg_id/pg_id
? src/bin/pg_passwd/pg_passwd
? src/bin/psql/psql
? src/bin/scripts/createlang
? src/include/pg_config.h
? src/include/stamp-h
? src/interfaces/ecpg/lib/libecpg.so.3.3.0
? src/interfaces/ecpg/preproc/ecpg
? src/interfaces/libpgeasy/libpgeasy.so.2.2
? src/interfaces/libpq/libpq.so.2.2
? src/pl/plpgsql/src/libplpgsql.so.1.0
Index: src/backend/parser/gram.y
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/parser/gram.y,v
retrieving revision 2.276
diff -r2.276 gram.y
198c198
< 		expr_list, attrs, target_list, update_target_list,
---
> 		expr_list, attrs, target_list, insert_target_list, update_target_list,
256c256
< %type <target>	target_el, update_target_el
---
> %type <target>	target_el, update_target_el, insert_target_el
3305c3305
< insert_rest:  VALUES '(' target_list ')'
---
> insert_rest: VALUES '(' insert_target_list ')'
5485c5485
< /* Target lists as found in SELECT ... and INSERT VALUES ( ... ) */
---
> /* Target lists as found in SELECT ... */
5492a5493,5517
> /* 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;
> 				}
>                 ;
> 
5493a5519
> 
6383c6409
< }
---
> }
\ No newline at end of file
Index: src/backend/parser/parse_coerce.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/parser/parse_coerce.c,v
retrieving revision 2.64
diff -r2.64 parse_coerce.c
39c39
< 	Node	   *result;
---
> 	Node	   *result;                     
69d68
< 
75d73
< 
79,80c77,78
< 			char	   *val = DatumGetCString(DirectFunctionCall1(textout,
< 													   con->constvalue));
---
> 			char	   *val = DatumGetCString(DirectFunctionCall1(textout, con->constvalue));
>                         newcon->constvalue = stringTypeDatum(targetType, val, atttypmod);
82d79
< 			newcon->constvalue = stringTypeDatum(targetType, val, atttypmod);
607c604
< }	/* PreferredType() */
---
> }	/* PreferredType() */
\ 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 -r1.105 parse_expr.c
123a124,128
>                 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;
> 			}
1069c1074
< }
---
> }
\ No newline at end of file
Index: src/backend/parser/parse_target.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/parser/parse_target.c,v
retrieving revision 1.76
diff -r1.76 parse_target.c
62a63,74
>         /* 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);
>         }
> 
263c275
< 				if (tle->expr == NULL)
---
>                                 if (tle->expr == NULL)
302c314
< 		expr = coerce_type(pstate, expr, type_id, attrtype, attrtypmod);
---
>             expr = coerce_type(pstate, expr, type_id, attrtype, attrtypmod);
528c540
< }
---
> }
\ No newline at end of file
Index: src/backend/parser/scan.l
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/parser/scan.l,v
retrieving revision 1.90
diff -r1.90 scan.l
640c640
< #endif /* FLEX_SCANNER */
---
> #endif /* FLEX_SCANNER */
\ 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 -r1.96 nodes.h
224a225
>         T_Default, /* pavlo (pb@pbit.org) : 2001.12.27: DEFAULT element of the INSERT INTO target list*/
368c369
< #endif   /* NODES_H */
---
> #endif   /* NODES_H */
\ 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 -r1.151 parsenodes.h
1007a1008,1016
>  * 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;
> 
> /*
1358c1367
< #endif   /* PARSENODES_H */
---
> #endif   /* PARSENODES_H */
\ No newline at end of file
#5Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Pavlo Baron (#4)
Re: patch: INSERT INTO t VALUES (a, b, ..., DEFAULT, ...)

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, Pennsylvania 19026
#6Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Pavlo Baron (#4)
Re: patch: INSERT INTO t VALUES (a, b, ..., DEFAULT, ...)

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, Pennsylvania 19026
#7Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Pavlo Baron (#4)
Re: patch: INSERT INTO t VALUES (a, b, ..., DEFAULT, ...)

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, Pennsylvania 19026