From 9231a0ca46fdc32d543d1f3e3c86a9f11ecfe2be Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Wed, 22 Aug 2018 10:50:49 +0200 Subject: [PATCH 3/3] Add location information to Value nodes This allows better location information in several commands. This change also adds better location reporting to the COPY command, in particular pointers into its various column lists. Support for other commands using Value nodes can be added. --- src/backend/commands/copy.c | 22 ++++++++++++---------- src/backend/commands/trigger.c | 18 +++++++++--------- src/backend/nodes/copyfuncs.c | 1 + src/backend/nodes/equalfuncs.c | 2 ++ src/backend/nodes/value.c | 4 ++++ src/backend/parser/gram.y | 4 +++- src/backend/parser/parse_utilcmd.c | 10 ++++++---- src/include/nodes/value.h | 1 + src/test/regress/expected/alter_table.out | 8 ++++++++ src/test/regress/expected/copy2.out | 2 ++ 10 files changed, 48 insertions(+), 24 deletions(-) diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index 9bc67ce60f..1e0290236f 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -328,7 +328,7 @@ static Datum CopyReadBinaryAttribute(CopyState cstate, static void CopyAttributeOutText(CopyState cstate, char *string); static void CopyAttributeOutCSV(CopyState cstate, char *string, bool use_quote, bool single_attr); -static List *CopyGetAttnums(TupleDesc tupDesc, Relation rel, +static List *CopyGetAttnums(ParseState *pstate, TupleDesc tupDesc, Relation rel, List *attnamelist); static char *limit_printout_length(const char *str); @@ -850,7 +850,7 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt, rte->requiredPerms = (is_from ? ACL_INSERT : ACL_SELECT); tupDesc = RelationGetDescr(rel); - attnums = CopyGetAttnums(tupDesc, rel, stmt->attlist); + attnums = CopyGetAttnums(pstate, tupDesc, rel, stmt->attlist); foreach(cur, attnums) { int attno = lfirst_int(cur) - @@ -1583,7 +1583,7 @@ BeginCopy(ParseState *pstate, } /* Generate or convert list of attributes to process */ - cstate->attnumlist = CopyGetAttnums(tupDesc, cstate->rel, attnamelist); + cstate->attnumlist = CopyGetAttnums(pstate, tupDesc, cstate->rel, attnamelist); num_phys_attrs = tupDesc->natts; @@ -1601,7 +1601,7 @@ BeginCopy(ParseState *pstate, List *attnums; ListCell *cur; - attnums = CopyGetAttnums(tupDesc, cstate->rel, cstate->force_quote); + attnums = CopyGetAttnums(pstate, tupDesc, cstate->rel, cstate->force_quote); foreach(cur, attnums) { @@ -1624,7 +1624,7 @@ BeginCopy(ParseState *pstate, List *attnums; ListCell *cur; - attnums = CopyGetAttnums(tupDesc, cstate->rel, cstate->force_notnull); + attnums = CopyGetAttnums(pstate, tupDesc, cstate->rel, cstate->force_notnull); foreach(cur, attnums) { @@ -1647,7 +1647,7 @@ BeginCopy(ParseState *pstate, List *attnums; ListCell *cur; - attnums = CopyGetAttnums(tupDesc, cstate->rel, cstate->force_null); + attnums = CopyGetAttnums(pstate, tupDesc, cstate->rel, cstate->force_null); foreach(cur, attnums) { @@ -1671,7 +1671,7 @@ BeginCopy(ParseState *pstate, cstate->convert_select_flags = (bool *) palloc0(num_phys_attrs * sizeof(bool)); - attnums = CopyGetAttnums(tupDesc, cstate->rel, cstate->convert_select); + attnums = CopyGetAttnums(pstate, tupDesc, cstate->rel, cstate->convert_select); foreach(cur, attnums) { @@ -4907,7 +4907,7 @@ CopyAttributeOutCSV(CopyState cstate, char *string, * rel can be NULL ... it's only used for error reports. */ static List * -CopyGetAttnums(TupleDesc tupDesc, Relation rel, List *attnamelist) +CopyGetAttnums(ParseState *pstate, TupleDesc tupDesc, Relation rel, List *attnamelist) { List *attnums = NIL; @@ -4931,7 +4931,8 @@ CopyGetAttnums(TupleDesc tupDesc, Relation rel, List *attnamelist) foreach(l, attnamelist) { - char *name = strVal(lfirst(l)); + Value *val = lfirst(l); + char *name = strVal(val); int attnum; int i; @@ -4955,7 +4956,8 @@ CopyGetAttnums(TupleDesc tupDesc, Relation rel, List *attnamelist) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_COLUMN), errmsg("column \"%s\" of relation \"%s\" does not exist", - name, RelationGetRelationName(rel)))); + name, RelationGetRelationName(rel)), + parser_errposition(pstate, val->location))); else ereport(ERROR, (errcode(ERRCODE_UNDEFINED_COLUMN), diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 2436692eb8..1c5da78fdf 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -161,6 +161,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, Oid funcoid, Oid parentTriggerOid, Node *whenClause, bool isInternal, bool in_partition) { + ParseState *pstate; int16 tgtype; int ncolumns; int16 *columns; @@ -188,6 +189,10 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, char *newtablename = NULL; bool partition_recurse; + /* Set up a pstate to parse with */ + pstate = make_parsestate(NULL); + pstate->p_sourcetext = queryString; + if (OidIsValid(relOid)) rel = heap_open(relOid, ShareRowExclusiveLock); else @@ -562,15 +567,10 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, */ if (!whenClause && stmt->whenClause) { - ParseState *pstate; RangeTblEntry *rte; List *varList; ListCell *lc; - /* Set up a pstate to parse with */ - pstate = make_parsestate(NULL); - pstate->p_sourcetext = queryString; - /* * Set up RTEs for OLD and NEW references. * @@ -648,8 +648,6 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, whenRtable = pstate->p_rtable; qual = nodeToString(whenClause); - - free_parsestate(pstate); } else if (!whenClause) { @@ -892,7 +890,8 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, columns = (int16 *) palloc(ncolumns * sizeof(int16)); foreach(cell, stmt->columns) { - char *name = strVal(lfirst(cell)); + Value *val = lfirst(cell); + char *name = strVal(val); int16 attnum; int j; @@ -902,7 +901,8 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, ereport(ERROR, (errcode(ERRCODE_UNDEFINED_COLUMN), errmsg("column \"%s\" of relation \"%s\" does not exist", - name, RelationGetRelationName(rel)))); + name, RelationGetRelationName(rel)), + parser_errposition(pstate, val->location))); /* Check for duplicates */ for (j = i - 1; j >= 0; j--) diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 7c8220cf65..b9d8855b8b 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -4738,6 +4738,7 @@ _copyValue(const Value *from) (int) from->type); break; } + COPY_LOCATION_FIELD(location); return newnode; } diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 378f2facb8..abf7701a28 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -2975,6 +2975,8 @@ _equalValue(const Value *a, const Value *b) break; } + COMPARE_LOCATION_FIELD(location); + return true; } diff --git a/src/backend/nodes/value.c b/src/backend/nodes/value.c index 2a30307baf..f41a5c0be7 100644 --- a/src/backend/nodes/value.c +++ b/src/backend/nodes/value.c @@ -26,6 +26,7 @@ makeInteger(int i) v->type = T_Integer; v->val.ival = i; + v->location = -1; return v; } @@ -41,6 +42,7 @@ makeFloat(char *numericStr) v->type = T_Float; v->val.str = numericStr; + v->location = -1; return v; } @@ -56,6 +58,7 @@ makeString(char *str) v->type = T_String; v->val.str = str; + v->location = -1; return v; } @@ -71,5 +74,6 @@ makeBitString(char *str) v->type = T_BitString; v->val.str = str; + v->location = -1; return v; } diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 4bd2223f26..e9a3a8efab 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -3798,7 +3798,9 @@ columnList: columnElem: ColId { - $$ = (Node *) makeString($1); + Value *v = makeString($1); + v->location = @1; + $$ = (Node *) v; } ; diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index 656b1b5f1b..91531dd88d 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -2148,7 +2148,8 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt) { foreach(lc, constraint->keys) { - char *key = strVal(lfirst(lc)); + Value *val = lfirst(lc); + char *key = strVal(val); bool found = false; ColumnDef *column = NULL; ListCell *columns; @@ -2236,7 +2237,7 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_COLUMN), errmsg("column \"%s\" named in key does not exist", key), - parser_errposition(cxt->pstate, constraint->location))); + parser_errposition(cxt->pstate, val->location))); /* Check for PRIMARY KEY(foo, foo) */ foreach(columns, index->indexParams) @@ -2275,7 +2276,8 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt) /* Add included columns to index definition */ foreach(lc, constraint->including) { - char *key = strVal(lfirst(lc)); + Value *val = lfirst(lc); + char *key = strVal(val); bool found = false; ColumnDef *column = NULL; ListCell *columns; @@ -2360,7 +2362,7 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_COLUMN), errmsg("column \"%s\" named in key does not exist", key), - parser_errposition(cxt->pstate, constraint->location))); + parser_errposition(cxt->pstate, val->location))); /* OK, add it to the index definition */ iparam = makeNode(IndexElem); diff --git a/src/include/nodes/value.h b/src/include/nodes/value.h index 1665714515..8583a9f874 100644 --- a/src/include/nodes/value.h +++ b/src/include/nodes/value.h @@ -47,6 +47,7 @@ typedef struct Value int ival; /* machine integer */ char *str; /* string */ } val; + int location; } Value; #define intVal(v) (((Value *)(v))->val.ival) diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 0218c2c362..a9d6b3aa03 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -1485,8 +1485,12 @@ copy attest to stdout; 2 3 copy attest(a) to stdout; ERROR: column "a" of relation "attest" does not exist +LINE 1: copy attest(a) to stdout; + ^ copy attest("........pg.dropped.1........") to stdout; ERROR: column "........pg.dropped.1........" of relation "attest" does not exist +LINE 1: copy attest("........pg.dropped.1........") to stdout; + ^ copy attest from stdin; ERROR: extra data after last expected column CONTEXT: COPY attest, line 1: "10 11 12" @@ -1506,8 +1510,12 @@ select * from attest; copy attest(a) from stdin; ERROR: column "a" of relation "attest" does not exist +LINE 1: copy attest(a) from stdin; + ^ copy attest("........pg.dropped.1........") from stdin; ERROR: column "........pg.dropped.1........" of relation "attest" does not exist +LINE 1: copy attest("........pg.dropped.1........") from stdin; + ^ copy attest(b,c) from stdin; select * from attest; b | c diff --git a/src/test/regress/expected/copy2.out b/src/test/regress/expected/copy2.out index eb9e4b9774..171afed28c 100644 --- a/src/test/regress/expected/copy2.out +++ b/src/test/regress/expected/copy2.out @@ -28,6 +28,8 @@ COPY x (a, b, c, d, e) from stdin; -- non-existent column in column list: should fail COPY x (xyz) from stdin; ERROR: column "xyz" of relation "x" does not exist +LINE 1: COPY x (xyz) from stdin; + ^ -- too many columns in column list: should fail COPY x (a, b, c, d, e, d, c) from stdin; ERROR: column "d" specified more than once -- 2.18.0