Skip to content

Commit fe24d78

Browse files
committed
Improve plpgsql's error reporting for no-such-column cases.
Given a column reference foo.bar, where there is a composite plpgsql variable foo but it doesn't contain a column bar, the pre-9.0 coding would immediately throw a "record foo has no field bar" error. In 9.0 the parser hook instead falls through to let the core parser see if it can resolve the reference. If not, you get a complaint about "missing FROM-clause entry for table foo", which while in some sense correct isn't terribly helpful. Complicate things a bit so that we can throw the old error message if neither the core parser nor the hook are able to resolve the column reference, while not changing the behavior in any other case. Per bug #5757 from Andrey Galkin.
1 parent 6cc2deb commit fe24d78

File tree

1 file changed

+36
-7
lines changed

1 file changed

+36
-7
lines changed

src/pl/plpgsql/src/pl_comp.c

+36-7
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,8 @@ static void add_dummy_return(PLpgSQL_function *function);
9999
static Node *plpgsql_pre_column_ref(ParseState *pstate, ColumnRef *cref);
100100
static Node *plpgsql_post_column_ref(ParseState *pstate, ColumnRef *cref, Node *var);
101101
static Node *plpgsql_param_ref(ParseState *pstate, ParamRef *pref);
102-
static Node *resolve_column_ref(PLpgSQL_expr *expr, ColumnRef *cref);
102+
static Node *resolve_column_ref(ParseState *pstate, PLpgSQL_expr *expr,
103+
ColumnRef *cref, bool error_if_no_field);
103104
static Node *make_datum_param(PLpgSQL_expr *expr, int dno, int location);
104105
static PLpgSQL_row *build_row_from_class(Oid classOid);
105106
static PLpgSQL_row *build_row_from_vars(PLpgSQL_variable **vars, int numvars);
@@ -945,7 +946,7 @@ plpgsql_pre_column_ref(ParseState *pstate, ColumnRef *cref)
945946
PLpgSQL_expr *expr = (PLpgSQL_expr *) pstate->p_ref_hook_state;
946947

947948
if (expr->func->resolve_option == PLPGSQL_RESOLVE_VARIABLE)
948-
return resolve_column_ref(expr, cref);
949+
return resolve_column_ref(pstate, expr, cref, false);
949950
else
950951
return NULL;
951952
}
@@ -965,7 +966,17 @@ plpgsql_post_column_ref(ParseState *pstate, ColumnRef *cref, Node *var)
965966
if (expr->func->resolve_option == PLPGSQL_RESOLVE_COLUMN && var != NULL)
966967
return NULL; /* there's a table column, prefer that */
967968

968-
myvar = resolve_column_ref(expr, cref);
969+
/*
970+
* If we find a record/row variable but can't match a field name, throw
971+
* error if there was no core resolution for the ColumnRef either. In
972+
* that situation, the reference is inevitably going to fail, and
973+
* complaining about the record/row variable is likely to be more
974+
* on-point than the core parser's error message. (It's too bad we
975+
* don't have access to transformColumnRef's internal crerr state here,
976+
* as in case of a conflict with a table name this could still be less
977+
* than the most helpful error message possible.)
978+
*/
979+
myvar = resolve_column_ref(pstate, expr, cref, (var == NULL));
969980

970981
if (myvar != NULL && var != NULL)
971982
{
@@ -1010,9 +1021,13 @@ plpgsql_param_ref(ParseState *pstate, ParamRef *pref)
10101021
* resolve_column_ref attempt to resolve a ColumnRef as a plpgsql var
10111022
*
10121023
* Returns the translated node structure, or NULL if name not found
1024+
*
1025+
* error_if_no_field tells whether to throw error or quietly return NULL if
1026+
* we are able to match a record/row name but don't find a field name match.
10131027
*/
10141028
static Node *
1015-
resolve_column_ref(PLpgSQL_expr *expr, ColumnRef *cref)
1029+
resolve_column_ref(ParseState *pstate, PLpgSQL_expr *expr,
1030+
ColumnRef *cref, bool error_if_no_field)
10161031
{
10171032
PLpgSQL_execstate *estate;
10181033
PLpgSQL_nsitem *nse;
@@ -1147,9 +1162,16 @@ resolve_column_ref(PLpgSQL_expr *expr, ColumnRef *cref)
11471162
/*
11481163
* We should not get here, because a RECFIELD datum should
11491164
* have been built at parse time for every possible qualified
1150-
* reference to fields of this record. But if we do, fall out
1151-
* and return NULL.
1165+
* reference to fields of this record. But if we do, handle
1166+
* it like field-not-found: throw error or return NULL.
11521167
*/
1168+
if (error_if_no_field)
1169+
ereport(ERROR,
1170+
(errcode(ERRCODE_UNDEFINED_COLUMN),
1171+
errmsg("record \"%s\" has no field \"%s\"",
1172+
(nnames_field == 1) ? name1 : name2,
1173+
colname),
1174+
parser_errposition(pstate, cref->location)));
11531175
}
11541176
break;
11551177
case PLPGSQL_NSTYPE_ROW:
@@ -1170,7 +1192,14 @@ resolve_column_ref(PLpgSQL_expr *expr, ColumnRef *cref)
11701192
cref->location);
11711193
}
11721194
}
1173-
/* Not found, so return NULL */
1195+
/* Not found, so throw error or return NULL */
1196+
if (error_if_no_field)
1197+
ereport(ERROR,
1198+
(errcode(ERRCODE_UNDEFINED_COLUMN),
1199+
errmsg("record \"%s\" has no field \"%s\"",
1200+
(nnames_field == 1) ? name1 : name2,
1201+
colname),
1202+
parser_errposition(pstate, cref->location)));
11741203
}
11751204
break;
11761205
default:

0 commit comments

Comments
 (0)