INSERT and parentheses

Started by Marko Tiikkajaover 15 years ago5 messages

marko.tiikkaja@cs.helsinki.fi

over 15 years ago

1 attachment(s)

Hi,

This came up on IRC today and I recall several instances of this during
the last two months or so, so I decided to send a patch. The problem in
question occurs when you have extra parentheses in an INSERT list:

INSERT INTO foo(a,b,c) SELECT (a,b,c) FROM ..; or
INSERT INTO foo(a,b,c) VALUES((0,1,2));

Both of these give you the same error:
ERROR: INSERT has more target columns than expressions

The first version is a lot more common and as it turns out, is sometimes
very hard to spot. This patch attaches a HINT message to these two
cases. The message itself could probably be a lot better, but I can't
think of anything.

Thoughts?

Regards,
Marko Tiikkaja

Attachments:

rowexpr.patchtext/plain; charset=iso-8859-1; name=rowexpr.patchDownload

*** a/src/backend/parser/analyze.c
--- b/src/backend/parser/analyze.c
***************
*** 730,742 **** transformInsertRow(ParseState *pstate, List *exprlist,
  												  list_length(icolumns))))));
  	if (stmtcols != NIL &&
  		list_length(exprlist) < list_length(icolumns))
! 		ereport(ERROR,
  				(errcode(ERRCODE_SYNTAX_ERROR),
  				 errmsg("INSERT has more target columns than expressions"),
  				 parser_errposition(pstate,
  									exprLocation(list_nth(icolumns,
  												  list_length(exprlist))))));
  
  	/*
  	 * Prepare columns for assignment to target table.
  	 */
--- 730,761 ----
  												  list_length(icolumns))))));
  	if (stmtcols != NIL &&
  		list_length(exprlist) < list_length(icolumns))
! 	{
! 		/*
! 		 * If the expression only has a single column of type record, it's
! 		 * possible that that wasn't intended.
! 		 */
! 		if (list_length(exprlist) == 1 &&
! 			(IsA(linitial(exprlist), Var) &&
! 			((Var *) linitial(exprlist))->vartype == RECORDOID) ||
! 			IsA(linitial(exprlist), RowExpr))
! 			ereport(ERROR,
! 				(errcode(ERRCODE_SYNTAX_ERROR),
! 				 errmsg("INSERT has more target columns than expressions"),
! 				 errhint("Did you accidentally use extra parentheses?"),
! 				 parser_errposition(pstate,
! 									exprLocation(list_nth(icolumns,
! 												  list_length(exprlist))))));
! 		else
! 			ereport(ERROR,
  				(errcode(ERRCODE_SYNTAX_ERROR),
  				 errmsg("INSERT has more target columns than expressions"),
  				 parser_errposition(pstate,
  									exprLocation(list_nth(icolumns,
  												  list_length(exprlist))))));
  
+ 	}
+ 
  	/*
  	 * Prepare columns for assignment to target table.
  	 */

Robert Haas

robertmhaas@gmail.com

over 15 years ago

In reply to: Marko Tiikkaja (#1)

Re: INSERT and parentheses

On Mon, Apr 26, 2010 at 8:57 AM, Marko Tiikkaja
<marko.tiikkaja@cs.helsinki.fi> wrote:

Hi,

This came up on IRC today and I recall several instances of this during
the last two months or so, so I decided to send a patch. The problem in
question occurs when you have extra parentheses in an INSERT list:

INSERT INTO foo(a,b,c) SELECT (a,b,c) FROM ..; or
INSERT INTO foo(a,b,c) VALUES((0,1,2));

Both of these give you the same error:
ERROR: INSERT has more target columns than expressions

The first version is a lot more common and as it turns out, is sometimes
very hard to spot. This patch attaches a HINT message to these two
cases. The message itself could probably be a lot better, but I can't
think of anything.

Thoughts?

I suggest adding it to the next CommitFest. Since I've never been
bitten by this, I can't get excited about the change, but I'm also not
arrogant enough to believe that everyone else's experiences are the
same as my own.

...Robert

Stephen Frost

sfrost@snowman.net

over 15 years ago

In reply to: Robert Haas (#2)

Re: INSERT and parentheses

* Robert Haas (robertmhaas@gmail.com) wrote:

The first version is a lot more common and as it turns out, is sometimes
very hard to spot. This patch attaches a HINT message to these two
cases. The message itself could probably be a lot better, but I can't
think of anything.

Thoughts?

I suggest adding it to the next CommitFest. Since I've never been
bitten by this, I can't get excited about the change, but I'm also not
arrogant enough to believe that everyone else's experiences are the
same as my own.

Not to be a pain, but the hint really is kind of terrible.. It'd
probably be better if you included somewhere that the insert appears to
be a single column with a record-type rather than multiple columns of
non-composite type..

Thanks,

Stephen

Tom Lane

tgl@sss.pgh.pa.us

over 15 years ago

In reply to: Stephen Frost (#3)

Re: INSERT and parentheses

Stephen Frost <sfrost@snowman.net> writes:

Not to be a pain, but the hint really is kind of terrible.. It'd
probably be better if you included somewhere that the insert appears to
be a single column with a record-type rather than multiple columns of
non-composite type..

I don't much care for the test, either. AFAICS, a hint like this would
only be appropriate for a RowExpr item, *not* a Var. It might also be
worth checking the number of items in the RowExpr before deciding that
the hint is appropriate.

regards, tom lane

Bruce Momjian

bruce@momjian.us

over 15 years ago

In reply to: Marko Tiikkaja (#1)

Re: INSERT and parentheses

I have added this to the next commit-fest:

https://commitfest.postgresql.org/action/commitfest_view?id=6

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

Marko Tiikkaja wrote:

Hi,

This came up on IRC today and I recall several instances of this during
the last two months or so, so I decided to send a patch. The problem in
question occurs when you have extra parentheses in an INSERT list:

INSERT INTO foo(a,b,c) SELECT (a,b,c) FROM ..; or
INSERT INTO foo(a,b,c) VALUES((0,1,2));

Both of these give you the same error:
ERROR: INSERT has more target columns than expressions

The first version is a lot more common and as it turns out, is sometimes
very hard to spot. This patch attaches a HINT message to these two
cases. The message itself could probably be a lot better, but I can't
think of anything.

Thoughts?

Regards,
Marko Tiikkaja

[ Attachment, skipping... ]

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ None of us is going to be here forever. +