From 2f28cc9b4f605b981801a307e44de6cbc9f504e5 Mon Sep 17 00:00:00 2001 From: jian he Date: Thu, 12 Dec 2024 12:15:16 +0800 Subject: [PATCH v11 2/2] print out the error position for some DDL commands. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This can be particularly helpful when working with a sequence of DML commands, such as `create schema create schema_element`. It also makes it easier to quickly identify the relevant error area in a single DDL command these DDL commands will printout error position: ALTER TABLE ALTER COLUMN SET DATA TYPE; CREATE TABLE OF type; ALTER TABLE ALTER COLUMN ADD GENERATED ALWAYS AS IDENTITY; CREATE TYPE LIKE type. CREATE TABLE while specifying conflict constraint. Author: Kirill Reshke Author: Jian He Reviewed-By: Michaël Paquier Reviewed-By: Álvaro Herrera discussion: https://postgr.es/m/CALdSSPhqfvKbDwqJaY=yEePi_aq61GmMpW88i6ZH7CMG_2Z4Cg@mail.gmail.com --- src/backend/commands/tablecmds.c | 43 +++++++++++++++-------- src/backend/commands/typecmds.c | 2 +- src/backend/parser/parse_utilcmd.c | 21 +++++++---- src/test/regress/expected/alter_table.out | 6 ++++ src/test/regress/expected/constraints.out | 14 ++++++++ src/test/regress/expected/float8.out | 2 ++ src/test/regress/expected/identity.out | 2 ++ src/test/regress/expected/typed_table.out | 4 +++ 8 files changed, 72 insertions(+), 22 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 6ccae4cb4a..efa38b1470 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -593,7 +593,8 @@ static void ATPrepAlterColumnType(List **wqueue, AlterTableUtilityContext *context); static bool ATColumnChangeRequiresRewrite(Node *expr, AttrNumber varattno); static ObjectAddress ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, - AlterTableCmd *cmd, LOCKMODE lockmode); + AlterTableCmd *cmd, LOCKMODE lockmode, + AlterTableUtilityContext *context); static void RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, Relation rel, AttrNumber attnum, const char *colName); static void RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab); @@ -639,7 +640,9 @@ static ObjectAddress ATExecAddInherit(Relation child_rel, RangeVar *parent, LOCK static ObjectAddress ATExecDropInherit(Relation rel, RangeVar *parent, LOCKMODE lockmode); static void drop_parent_dependency(Oid relid, Oid refclassid, Oid refobjid, DependencyType deptype); -static ObjectAddress ATExecAddOf(Relation rel, const TypeName *ofTypename, LOCKMODE lockmode); +static ObjectAddress ATExecAddOf(Relation rel, const TypeName *ofTypename, + LOCKMODE lockmode, + AlterTableUtilityContext *context); static void ATExecDropOf(Relation rel, LOCKMODE lockmode); static void ATExecReplicaIdentity(Relation rel, ReplicaIdentityStmt *stmt, LOCKMODE lockmode); static void ATExecGenericOptions(Relation rel, List *options); @@ -5413,7 +5416,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, break; case AT_AlterColumnType: /* ALTER COLUMN TYPE */ /* parse transformation was done earlier */ - address = ATExecAlterColumnType(tab, rel, cmd, lockmode); + address = ATExecAlterColumnType(tab, rel, cmd, lockmode, context); break; case AT_AlterColumnGenericOptions: /* ALTER COLUMN OPTIONS */ address = @@ -5537,7 +5540,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, address = ATExecDropInherit(rel, (RangeVar *) cmd->def, lockmode); break; case AT_AddOf: - address = ATExecAddOf(rel, (TypeName *) cmd->def, lockmode); + address = ATExecAddOf(rel, (TypeName *) cmd->def, lockmode, context); break; case AT_DropOf: ATExecDropOf(rel, lockmode); @@ -13218,10 +13221,12 @@ ATPrepAlterColumnType(List **wqueue, AclResult aclresult; bool is_expr; + pstate->p_sourcetext = context->queryString; if (rel->rd_rel->reloftype && !recursing) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("cannot alter column type of typed table"))); + errmsg("cannot alter column type of typed table"), + parser_errposition(pstate, def->location))); /* lookup the attribute so we can check inheritance status */ tuple = SearchSysCacheAttName(RelationGetRelid(rel), colName); @@ -13237,8 +13242,8 @@ ATPrepAlterColumnType(List **wqueue, if (attnum <= 0) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot alter system column \"%s\"", - colName))); + errmsg("cannot alter system column \"%s\"",colName), + parser_errposition(pstate, def->location))); /* * Cannot specify USING when altering type of a generated column, because @@ -13271,14 +13276,14 @@ ATPrepAlterColumnType(List **wqueue, colName, RelationGetRelationName(rel)))); /* Look up the target type */ - typenameTypeIdAndMod(NULL, typeName, &targettype, &targettypmod); + typenameTypeIdAndMod(pstate, typeName, &targettype, &targettypmod); aclresult = object_aclcheck(TypeRelationId, targettype, GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) aclcheck_error_type(aclresult, targettype); /* And the collation */ - targetcollid = GetColumnDefCollation(NULL, def, targettype); + targetcollid = GetColumnDefCollation(pstate, def, targettype); /* make sure datatype is legal for a column */ CheckAttributeType(colName, targettype, targetcollid, @@ -13537,7 +13542,8 @@ ATColumnChangeRequiresRewrite(Node *expr, AttrNumber varattno) */ static ObjectAddress ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, - AlterTableCmd *cmd, LOCKMODE lockmode) + AlterTableCmd *cmd, LOCKMODE lockmode, + AlterTableUtilityContext *context) { char *colName = cmd->name; ColumnDef *def = (ColumnDef *) cmd->def; @@ -13558,6 +13564,10 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, SysScanDesc scan; HeapTuple depTup; ObjectAddress address; + ParseState *pstate; + + pstate = make_parsestate(NULL); + pstate->p_sourcetext = context->queryString; /* * Clear all the missing values if we're rewriting the table, since this @@ -13596,11 +13606,11 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, colName))); /* Look up the target type (should not fail, since prep found it) */ - typeTuple = typenameType(NULL, typeName, &targettypmod); + typeTuple = typenameType(pstate, typeName, &targettypmod); tform = (Form_pg_type) GETSTRUCT(typeTuple); targettype = tform->oid; /* And the collation */ - targetcollid = GetColumnDefCollation(NULL, def, targettype); + targetcollid = GetColumnDefCollation(pstate, def, targettype); /* * If there is a default expression for the column, get it and ensure we @@ -16976,7 +16986,8 @@ drop_parent_dependency(Oid relid, Oid refclassid, Oid refobjid, * The address of the type is returned. */ static ObjectAddress -ATExecAddOf(Relation rel, const TypeName *ofTypename, LOCKMODE lockmode) +ATExecAddOf(Relation rel, const TypeName *ofTypename, LOCKMODE lockmode, + AlterTableUtilityContext *context) { Oid relid = RelationGetRelid(rel); Type typetuple; @@ -16993,9 +17004,13 @@ ATExecAddOf(Relation rel, const TypeName *ofTypename, LOCKMODE lockmode) ObjectAddress tableobj, typeobj; HeapTuple classtuple; + ParseState *pstate; /* Validate the type. */ - typetuple = typenameType(NULL, ofTypename, NULL); + pstate = make_parsestate(NULL); + pstate->p_sourcetext = context->queryString; + + typetuple = typenameType(pstate, ofTypename, NULL); check_of_type(typetuple); typeform = (Form_pg_type) GETSTRUCT(typetuple); typeid = typeform->oid; diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index 6127313956..4f20b5be06 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -348,7 +348,7 @@ DefineType(ParseState *pstate, List *names, List *parameters) Type likeType; Form_pg_type likeForm; - likeType = typenameType(NULL, defGetTypeName(likeTypeEl), NULL); + likeType = typenameType(pstate, defGetTypeName(likeTypeEl), NULL); likeForm = (Form_pg_type) GETSTRUCT(likeType); internalLength = likeForm->typlen; byValue = likeForm->typbyval; diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index 0f324ee4e3..7223911bf1 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -756,7 +756,8 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column) if (cxt->ispartitioned && constraint->is_no_inherit) ereport(ERROR, errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("not-null constraints on partitioned tables cannot be NO INHERIT")); + errmsg("not-null constraints on partitioned tables cannot be NO INHERIT"), + parser_errposition(cxt->pstate, constraint->location)); /* Disallow conflicting [NOT] NULL markings */ if (saw_nullable && !column->is_not_null) @@ -771,7 +772,9 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column) ereport(ERROR, errcode(ERRCODE_SYNTAX_ERROR), errmsg("conflicting NO INHERIT declarations for not-null constraints on column \"%s\"", - column->colname)); + column->colname), + parser_errposition(cxt->pstate, constraint->location)); + /* * If this is the first time we see this column being marked @@ -806,7 +809,8 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column) ereport(ERROR, errcode(ERRCODE_SYNTAX_ERROR), errmsg("conflicting NO INHERIT declarations for not-null constraints on column \"%s\"", - column->colname)); + column->colname), + parser_errposition(cxt->pstate, constraint->location)); if (!notnull_constraint->conname && constraint->conname) notnull_constraint->conname = constraint->conname; @@ -1072,7 +1076,8 @@ transformTableConstraint(CreateStmtContext *cxt, Constraint *constraint) if (cxt->ispartitioned && constraint->is_no_inherit) ereport(ERROR, errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("not-null constraints on partitioned tables cannot be NO INHERIT")); + errmsg("not-null constraints on partitioned tables cannot be NO INHERIT"), + parser_errposition(cxt->pstate, constraint->location)); cxt->nnconstraints = lappend(cxt->nnconstraints, constraint); break; @@ -1615,7 +1620,7 @@ transformOfType(CreateStmtContext *cxt, TypeName *ofTypename) Assert(ofTypename); - tuple = typenameType(NULL, ofTypename, NULL); + tuple = typenameType(cxt->pstate, ofTypename, NULL); check_of_type(tuple); ofTypeId = ((Form_pg_type) GETSTRUCT(tuple))->oid; ofTypename->typeOid = ofTypeId; /* cached for later */ @@ -3627,7 +3632,8 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt, ereport(ERROR, (errcode(ERRCODE_UNDEFINED_COLUMN), errmsg("column \"%s\" of relation \"%s\" does not exist", - cmd->name, RelationGetRelationName(rel)))); + cmd->name, RelationGetRelationName(rel)), + parser_errposition(pstate, def->location))); if (attnum > 0 && TupleDescAttr(tupdesc, attnum - 1)->attidentity) @@ -3667,7 +3673,8 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt, ereport(ERROR, (errcode(ERRCODE_UNDEFINED_COLUMN), errmsg("column \"%s\" of relation \"%s\" does not exist", - cmd->name, RelationGetRelationName(rel)))); + cmd->name, RelationGetRelationName(rel)), + parser_errposition(pstate, def->location))); generateSerialExtraStmts(&cxt, newdef, get_atttype(relid, attnum), diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 12852aa612..57263f0709 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -3416,10 +3416,16 @@ ALTER TABLE comment_test ALTER COLUMN positive_col SET DATA TYPE bigint; -- Some error cases. ALTER TABLE comment_test ALTER COLUMN xmin SET DATA TYPE x; ERROR: cannot alter system column "xmin" +LINE 1: ALTER TABLE comment_test ALTER COLUMN xmin SET DATA TYPE x; + ^ ALTER TABLE comment_test ALTER COLUMN id SET DATA TYPE x; ERROR: type "x" does not exist +LINE 1: ALTER TABLE comment_test ALTER COLUMN id SET DATA TYPE x; + ^ ALTER TABLE comment_test ALTER COLUMN id SET DATA TYPE int COLLATE "C"; ERROR: collations are not supported by type integer +LINE 1: ...LE comment_test ALTER COLUMN id SET DATA TYPE int COLLATE "C... + ^ -- Check that the comments are intact. SELECT col_description('comment_test'::regclass, 1) as comment; comment diff --git a/src/test/regress/expected/constraints.out b/src/test/regress/expected/constraints.out index 71200c90ed..ff3b710468 100644 --- a/src/test/regress/expected/constraints.out +++ b/src/test/regress/expected/constraints.out @@ -925,6 +925,8 @@ create table notnull_tbl_fail (a serial constraint foo not null constraint bar n ERROR: conflicting not-null constraint names "foo" and "bar" create table notnull_tbl_fail (a serial constraint foo not null no inherit constraint foo not null); ERROR: conflicting NO INHERIT declarations for not-null constraints on column "a" +LINE 1: create table notnull_tbl_fail (a serial constraint foo not n... + ^ create table notnull_tbl_fail (a int constraint foo not null, constraint foo not null a no inherit); ERROR: conflicting NO INHERIT declaration for not-null constraint on column "a" create table notnull_tbl_fail (a serial constraint foo not null, constraint bar not null a); @@ -935,12 +937,18 @@ create table notnull_tbl_fail (a serial, constraint foo not null a no inherit); ERROR: conflicting NO INHERIT declaration for not-null constraint on column "a" create table notnull_tbl_fail (a serial not null no inherit); ERROR: conflicting NO INHERIT declarations for not-null constraints on column "a" +LINE 1: create table notnull_tbl_fail (a serial not null no inherit)... + ^ create table notnull_tbl_fail (like notnull_tbl1, constraint foo2 not null a); ERROR: conflicting not-null constraint names "foo" and "foo2" create table notnull_tbl_fail (a int primary key constraint foo not null no inherit); ERROR: conflicting NO INHERIT declarations for not-null constraints on column "a" +LINE 1: create table notnull_tbl_fail (a int primary key constraint ... + ^ create table notnull_tbl_fail (a int not null no inherit primary key); ERROR: conflicting NO INHERIT declarations for not-null constraints on column "a" +LINE 1: create table notnull_tbl_fail (a int not null no inherit pri... + ^ create table notnull_tbl_fail (a int primary key, not null a no inherit); ERROR: conflicting NO INHERIT declaration for not-null constraint on column "a" create table notnull_tbl_fail (a int, primary key(a), not null a no inherit); @@ -949,6 +957,8 @@ create table notnull_tbl_fail (a int generated by default as identity, constrain ERROR: conflicting NO INHERIT declaration for not-null constraint on column "a" create table notnull_tbl_fail (a int generated by default as identity not null no inherit); ERROR: conflicting NO INHERIT declarations for not-null constraints on column "a" +LINE 1: ..._tbl_fail (a int generated by default as identity not null n... + ^ drop table notnull_tbl1; -- NOT NULL NO INHERIT CREATE TABLE ATACC1 (a int, NOT NULL a NO INHERIT); @@ -998,8 +1008,12 @@ DROP TABLE ATACC1, ATACC2, ATACC3; -- NOT NULL NO INHERIT is not possible on partitioned tables CREATE TABLE ATACC1 (a int NOT NULL NO INHERIT) PARTITION BY LIST (a); ERROR: not-null constraints on partitioned tables cannot be NO INHERIT +LINE 1: CREATE TABLE ATACC1 (a int NOT NULL NO INHERIT) PARTITION BY... + ^ CREATE TABLE ATACC1 (a int, NOT NULL a NO INHERIT) PARTITION BY LIST (a); ERROR: not-null constraints on partitioned tables cannot be NO INHERIT +LINE 1: CREATE TABLE ATACC1 (a int, NOT NULL a NO INHERIT) PARTITION... + ^ -- it's not possible to override a no-inherit constraint with an inheritable one CREATE TABLE ATACC2 (a int, CONSTRAINT a_is_not_null NOT NULL a NO INHERIT); CREATE TABLE ATACC1 (a int); diff --git a/src/test/regress/expected/float8.out b/src/test/regress/expected/float8.out index 4965ee5554..9ef9793fe9 100644 --- a/src/test/regress/expected/float8.out +++ b/src/test/regress/expected/float8.out @@ -1026,6 +1026,8 @@ LINE 1: create function xfloat8out(xfloat8) returns cstring immutabl... ^ create type xfloat8 (input = xfloat8in, output = xfloat8out, like = no_such_type); ERROR: type "no_such_type" does not exist +LINE 1: ...8 (input = xfloat8in, output = xfloat8out, like = no_such_ty... + ^ create type xfloat8 (input = xfloat8in, output = xfloat8out, like = float8); create cast (xfloat8 as float8) without function; create cast (float8 as xfloat8) without function; diff --git a/src/test/regress/expected/identity.out b/src/test/regress/expected/identity.out index 0398a19484..0b370235ea 100644 --- a/src/test/regress/expected/identity.out +++ b/src/test/regress/expected/identity.out @@ -45,6 +45,8 @@ ERROR: column "a" of relation "itest4" must be declared NOT NULL before identit ALTER TABLE itest4 ALTER COLUMN a SET NOT NULL; ALTER TABLE itest4 ALTER COLUMN c ADD GENERATED ALWAYS AS IDENTITY; -- error, column c does not exist ERROR: column "c" of relation "itest4" does not exist +LINE 1: ALTER TABLE itest4 ALTER COLUMN c ADD GENERATED ALWAYS AS ID... + ^ ALTER TABLE itest4 ALTER COLUMN a ADD GENERATED ALWAYS AS IDENTITY; -- ok ALTER TABLE itest4 ALTER COLUMN a DROP NOT NULL; -- error, disallowed ERROR: column "a" of relation "itest4" is an identity column diff --git a/src/test/regress/expected/typed_table.out b/src/test/regress/expected/typed_table.out index b6fbda3f21..885f085e15 100644 --- a/src/test/regress/expected/typed_table.out +++ b/src/test/regress/expected/typed_table.out @@ -1,5 +1,7 @@ CREATE TABLE ttable1 OF nothing; ERROR: type "nothing" does not exist +LINE 1: CREATE TABLE ttable1 OF nothing; + ^ CREATE TYPE person_type AS (id int, name text); CREATE TABLE persons OF person_type; CREATE TABLE IF NOT EXISTS persons OF person_type; @@ -36,6 +38,8 @@ ALTER TABLE persons RENAME COLUMN id TO num; ERROR: cannot rename column of typed table ALTER TABLE persons ALTER COLUMN name TYPE varchar; ERROR: cannot alter column type of typed table +LINE 1: ALTER TABLE persons ALTER COLUMN name TYPE varchar; + ^ CREATE TABLE stuff (id int); ALTER TABLE persons INHERIT stuff; ERROR: cannot change inheritance of typed table -- 2.34.1