Marko Tiikkaja <marko.tiikk...@cs.helsinki.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 -

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

Reply via email to