Re: INSERT and parentheses

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Marko Tiikkaja <marko(dot)tiikkaja(at)cs(dot)helsinki(dot)fi>
Cc: igor polishchuk <ora4dba(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: INSERT and parentheses
Date: 2010-09-18 18:41:33
Message-ID: 16242.1284835293@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Marko Tiikkaja <marko(dot)tiikkaja(at)cs(dot)helsinki(dot)fi> writes:
> [ patch for a misleading error message ]

I've committed the attached revised version of this patch. The main
change is to only emit the hint if the insertion source can be shown
to be a RowExpr with exactly the number of columns expected by the
INSERT. This should avoid emitting the hint in cases where it's not
relevant, and allows a specific wording for the hint, as was recommended
by Stephen Frost.

regards, tom lane

Index: analyze.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/analyze.c,v
retrieving revision 1.403
diff -c -r1.403 analyze.c
*** analyze.c 27 Aug 2010 20:30:08 -0000 1.403
--- analyze.c 18 Sep 2010 18:27:33 -0000
***************
*** 47,52 ****
--- 47,53 ----
static Query *transformInsertStmt(ParseState *pstate, InsertStmt *stmt);
static List *transformInsertRow(ParseState *pstate, List *exprlist,
List *stmtcols, List *icolumns, List *attrnos);
+ static int count_rowexpr_columns(ParseState *pstate, Node *expr);
static Query *transformSelectStmt(ParseState *pstate, SelectStmt *stmt);
static Query *transformValuesClause(ParseState *pstate, SelectStmt *stmt);
static Query *transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt);
***************
*** 726,737 ****
--- 727,753 ----
list_length(icolumns))))));
if (stmtcols != NIL &&
list_length(exprlist) < list_length(icolumns))
+ {
+ /*
+ * We can get here for cases like INSERT ... SELECT (a,b,c) FROM ...
+ * where the user accidentally created a RowExpr instead of separate
+ * columns. Add a suitable hint if that seems to be the problem,
+ * because the main error message is quite misleading for this case.
+ * (If there's no stmtcols, you'll get something about data type
+ * mismatch, which is less misleading so we don't worry about giving
+ * a hint in that case.)
+ */
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("INSERT has more target columns than expressions"),
+ ((list_length(exprlist) == 1 &&
+ count_rowexpr_columns(pstate, linitial(exprlist)) ==
+ list_length(icolumns)) ?
+ errhint("The insertion source is a row expression containing the same number of columns expected by the INSERT. Did you accidentally use extra parentheses?") : 0),
parser_errposition(pstate,
exprLocation(list_nth(icolumns,
list_length(exprlist))))));
+ }

/*
* Prepare columns for assignment to target table.
***************
*** 762,767 ****
--- 778,826 ----
return result;
}

+ /*
+ * count_rowexpr_columns -
+ * get number of columns contained in a ROW() expression;
+ * return -1 if expression isn't a RowExpr or a Var referencing one.
+ *
+ * This is currently used only for hint purposes, so we aren't terribly
+ * tense about recognizing all possible cases. The Var case is interesting
+ * because that's what we'll get in the INSERT ... SELECT (...) case.
+ */
+ static int
+ count_rowexpr_columns(ParseState *pstate, Node *expr)
+ {
+ if (expr == NULL)
+ return -1;
+ if (IsA(expr, RowExpr))
+ return list_length(((RowExpr *) expr)->args);
+ if (IsA(expr, Var))
+ {
+ Var *var = (Var *) expr;
+ AttrNumber attnum = var->varattno;
+
+ if (attnum > 0 && var->vartype == RECORDOID)
+ {
+ RangeTblEntry *rte;
+
+ rte = GetRTEByRangeTablePosn(pstate, var->varno, var->varlevelsup);
+ if (rte->rtekind == RTE_SUBQUERY)
+ {
+ /* Subselect-in-FROM: examine sub-select's output expr */
+ TargetEntry *ste = get_tle_by_resno(rte->subquery->targetList,
+ attnum);
+
+ if (ste == NULL || ste->resjunk)
+ return -1;
+ expr = (Node *) ste->expr;
+ if (IsA(expr, RowExpr))
+ return list_length(((RowExpr *) expr)->args);
+ }
+ }
+ }
+ return -1;
+ }
+

/*
* transformSelectStmt -

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Kevin Grittner 2010-09-18 18:44:48 Re: Review: Row-level Locks & SERIALIZABLE transactions, postgres vs. Oracle
Previous Message Andrew Dunstan 2010-09-18 18:30:03 Re: compile/install of git