From b9a3ef55b253d885081c2d0e9dc45802cab71c7b Mon Sep 17 00:00:00 2001 From: Stephen Frost Date: Thu, 27 Apr 2017 20:14:39 -0400 Subject: [PATCH] Remove unnecessairly duplicated gram.y productions Declarative partitioning duplicated the TypedTableElement productions, evidently to remove the need to specify WITH OPTIONS when creating partitions. Instead, simply make WITH OPTIONS optional in the TypedTableElement production and remove all of the duplicate PartitionElement-related productions. This change simplifies the syntax and makes WITH OPTIONS optional when adding defaults, constraints or storage parameters to columns when creating either typed tables or partitions. Also update pg_dump to no longer include WITH OPTIONS, since it's not necessary, and update the documentation to reflect that WITH OPTIONS is now optional. --- doc/src/sgml/ref/create_foreign_table.sgml | 2 +- doc/src/sgml/ref/create_table.sgml | 4 +- src/backend/parser/gram.y | 68 ++++++++-------------- src/bin/pg_dump/pg_dump.c | 17 +++--- src/test/regress/expected/create_table.out | 2 +- src/test/regress/expected/sanity_check.out | 3 + src/test/regress/expected/typed_table.out | 37 ++++++++++-- src/test/regress/sql/create_table.sql | 2 +- src/test/regress/sql/typed_table.sql | 21 ++++++- 9 files changed, 94 insertions(+), 62 deletions(-) diff --git a/doc/src/sgml/ref/create_foreign_table.sgml b/doc/src/sgml/ref/create_foreign_table.sgml index 5d0dcf567b..065c982082 100644 --- a/doc/src/sgml/ref/create_foreign_table.sgml +++ b/doc/src/sgml/ref/create_foreign_table.sgml @@ -29,7 +29,7 @@ CREATE FOREIGN TABLE [ IF NOT EXISTS ] table_name CREATE FOREIGN TABLE [ IF NOT EXISTS ] table_name PARTITION OF parent_table [ ( - { column_name WITH OPTIONS [ column_constraint [ ... ] ] + { column_name [ WITH OPTIONS ] [ column_constraint [ ... ] ] | table_constraint } [, ... ] ) ] partition_bound_spec diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml index a3dc744efa..484f81898b 100644 --- a/doc/src/sgml/ref/create_table.sgml +++ b/doc/src/sgml/ref/create_table.sgml @@ -35,7 +35,7 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXI CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXISTS ] table_name OF type_name [ ( - { column_name WITH OPTIONS [ column_constraint [ ... ] ] + { column_name [ WITH OPTIONS ] [ column_constraint [ ... ] ] | table_constraint } [, ... ] ) ] @@ -46,7 +46,7 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXI CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXISTS ] table_name PARTITION OF parent_table [ ( - { column_name [ column_constraint [ ... ] ] + { column_name [ WITH OPTIONS ] [ column_constraint [ ... ] ] | table_constraint } [, ... ] ) ] FOR VALUES partition_bound_spec diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 89d2836c49..21cdc7c7da 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -576,8 +576,6 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); %type part_strategy %type part_elem %type part_params -%type OptPartitionElementList PartitionElementList -%type PartitionElement %type ForValues %type partbound_datum %type partbound_datum_list @@ -3131,7 +3129,7 @@ CreateStmt: CREATE OptTemp TABLE qualified_name '(' OptTableElementList ')' $$ = (Node *)n; } | CREATE OptTemp TABLE qualified_name PARTITION OF qualified_name - OptPartitionElementList ForValues OptPartitionSpec OptWith + OptTypedTableElementList ForValues OptPartitionSpec OptWith OnCommitOption OptTableSpace { CreateStmt *n = makeNode(CreateStmt); @@ -3150,7 +3148,7 @@ CreateStmt: CREATE OptTemp TABLE qualified_name '(' OptTableElementList ')' $$ = (Node *)n; } | CREATE OptTemp TABLE IF_P NOT EXISTS qualified_name PARTITION OF - qualified_name OptPartitionElementList ForValues OptPartitionSpec + qualified_name OptTypedTableElementList ForValues OptPartitionSpec OptWith OnCommitOption OptTableSpace { CreateStmt *n = makeNode(CreateStmt); @@ -3213,11 +3211,6 @@ OptTypedTableElementList: | /*EMPTY*/ { $$ = NIL; } ; -OptPartitionElementList: - '(' PartitionElementList ')' { $$ = $2; } - | /*EMPTY*/ { $$ = NIL; } - ; - TableElementList: TableElement { @@ -3240,17 +3233,6 @@ TypedTableElementList: } ; -PartitionElementList: - PartitionElement - { - $$ = list_make1($1); - } - | PartitionElementList ',' PartitionElement - { - $$ = lappend($1, $3); - } - ; - TableElement: columnDef { $$ = $1; } | TableLikeClause { $$ = $1; } @@ -3262,28 +3244,6 @@ TypedTableElement: | TableConstraint { $$ = $1; } ; -PartitionElement: - TableConstraint { $$ = $1; } - | ColId ColQualList - { - ColumnDef *n = makeNode(ColumnDef); - n->colname = $1; - n->typeName = NULL; - n->inhcount = 0; - n->is_local = true; - n->is_not_null = false; - n->is_from_type = false; - n->storage = 0; - n->raw_default = NULL; - n->cooked_default = NULL; - n->collOid = InvalidOid; - SplitColQualList($2, &n->constraints, &n->collClause, - yyscanner); - n->location = @1; - $$ = (Node *) n; - } - ; - columnDef: ColId Typename create_generic_options ColQualList { ColumnDef *n = makeNode(ColumnDef); @@ -3305,7 +3265,25 @@ columnDef: ColId Typename create_generic_options ColQualList } ; -columnOptions: ColId WITH OPTIONS ColQualList +columnOptions: ColId ColQualList + { + ColumnDef *n = makeNode(ColumnDef); + n->colname = $1; + n->typeName = NULL; + n->inhcount = 0; + n->is_local = true; + n->is_not_null = false; + n->is_from_type = false; + n->storage = 0; + n->raw_default = NULL; + n->cooked_default = NULL; + n->collOid = InvalidOid; + SplitColQualList($2, &n->constraints, &n->collClause, + yyscanner); + n->location = @1; + $$ = (Node *)n; + } + | ColId WITH OPTIONS ColQualList { ColumnDef *n = makeNode(ColumnDef); n->colname = $1; @@ -4872,7 +4850,7 @@ CreateForeignTableStmt: $$ = (Node *) n; } | CREATE FOREIGN TABLE qualified_name - PARTITION OF qualified_name OptPartitionElementList ForValues + PARTITION OF qualified_name OptTypedTableElementList ForValues SERVER name create_generic_options { CreateForeignTableStmt *n = makeNode(CreateForeignTableStmt); @@ -4893,7 +4871,7 @@ CreateForeignTableStmt: $$ = (Node *) n; } | CREATE FOREIGN TABLE IF_P NOT EXISTS qualified_name - PARTITION OF qualified_name OptPartitionElementList ForValues + PARTITION OF qualified_name OptTypedTableElementList ForValues SERVER name create_generic_options { CreateForeignTableStmt *n = makeNode(CreateForeignTableStmt); diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index e9b5c8a448..2fda350faa 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -15267,13 +15267,16 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) continue; } - /* Attribute type */ - if ((tbinfo->reloftype || tbinfo->partitionOf) && - !dopt->binary_upgrade) - { - appendPQExpBufferStr(q, " WITH OPTIONS"); - } - else + /* + * Attribute type + * + * In binary-upgrade mode, we always include the type. + * If we aren't in binary-upgrade mode, then we skip the + * type when creating a typed table ('OF type_name') or a + * partition ('PARTITION OF'), since the type comes from + * the parent/partitioned table. + */ + if (dopt->binary_upgrade || (!tbinfo->reloftype && !tbinfo->partitionOf)) { appendPQExpBuffer(q, " %s", tbinfo->atttypnames[j]); diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out index b6c75d2e81..3f94250df2 100644 --- a/src/test/regress/expected/create_table.out +++ b/src/test/regress/expected/create_table.out @@ -624,7 +624,7 @@ SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::reg -- specify PARTITION BY for a partition CREATE TABLE fail_part_col_not_found PARTITION OF parted FOR VALUES IN ('c') PARTITION BY RANGE (c); ERROR: column "c" named in partition key does not exist -CREATE TABLE part_c PARTITION OF parted FOR VALUES IN ('c') PARTITION BY RANGE ((b)); +CREATE TABLE part_c PARTITION OF parted (b WITH OPTIONS NOT NULL DEFAULT 0) FOR VALUES IN ('c') PARTITION BY RANGE ((b)); -- create a level-2 partition CREATE TABLE part_c_1_10 PARTITION OF part_c FOR VALUES FROM (1) TO (10); -- Partition bound in describe output diff --git a/src/test/regress/expected/sanity_check.out b/src/test/regress/expected/sanity_check.out index 753ad81e43..6750152e0f 100644 --- a/src/test/regress/expected/sanity_check.out +++ b/src/test/regress/expected/sanity_check.out @@ -93,6 +93,9 @@ onek|t onek2|t path_tbl|f person|f +persons|f +persons2|t +persons3|t pg_aggregate|t pg_am|t pg_amop|t diff --git a/src/test/regress/expected/typed_table.out b/src/test/regress/expected/typed_table.out index 141d3bcf87..5cdd019244 100644 --- a/src/test/regress/expected/typed_table.out +++ b/src/test/regress/expected/typed_table.out @@ -1,3 +1,7 @@ +-- Clean up in case a prior regression run failed +SET client_min_messages TO 'warning'; +DROP TYPE IF EXISTS person_type CASCADE; +RESET client_min_messages; CREATE TABLE ttable1 OF nothing; ERROR: type "nothing" does not exist CREATE TYPE person_type AS (id int, name text); @@ -102,7 +106,32 @@ SELECT id, namelen(persons) FROM persons; 1 | 4 (1 row) -DROP TYPE person_type CASCADE; -NOTICE: drop cascades to 2 other objects -DETAIL: drop cascades to table persons -drop cascades to function namelen(person_type) +CREATE TABLE persons2 OF person_type ( + id WITH OPTIONS PRIMARY KEY, + UNIQUE (name) +); +\d persons2 + Table "public.persons2" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + id | integer | | not null | + name | text | | | +Indexes: + "persons2_pkey" PRIMARY KEY, btree (id) + "persons2_name_key" UNIQUE CONSTRAINT, btree (name) +Typed table of type: person_type + +CREATE TABLE persons3 OF person_type ( + PRIMARY KEY (id), + name NOT NULL DEFAULT '' +); +\d persons3 + Table "public.persons3" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+---------- + id | integer | | not null | + name | text | | not null | ''::text +Indexes: + "persons3_pkey" PRIMARY KEY, btree (id) +Typed table of type: person_type + diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql index b00d9e87b8..f08942f7d2 100644 --- a/src/test/regress/sql/create_table.sql +++ b/src/test/regress/sql/create_table.sql @@ -578,7 +578,7 @@ SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::reg -- specify PARTITION BY for a partition CREATE TABLE fail_part_col_not_found PARTITION OF parted FOR VALUES IN ('c') PARTITION BY RANGE (c); -CREATE TABLE part_c PARTITION OF parted FOR VALUES IN ('c') PARTITION BY RANGE ((b)); +CREATE TABLE part_c PARTITION OF parted (b WITH OPTIONS NOT NULL DEFAULT 0) FOR VALUES IN ('c') PARTITION BY RANGE ((b)); -- create a level-2 partition CREATE TABLE part_c_1_10 PARTITION OF part_c FOR VALUES FROM (1) TO (10); diff --git a/src/test/regress/sql/typed_table.sql b/src/test/regress/sql/typed_table.sql index 25aaccb8bc..1cdceef363 100644 --- a/src/test/regress/sql/typed_table.sql +++ b/src/test/regress/sql/typed_table.sql @@ -1,3 +1,10 @@ +-- Clean up in case a prior regression run failed +SET client_min_messages TO 'warning'; + +DROP TYPE IF EXISTS person_type CASCADE; + +RESET client_min_messages; + CREATE TABLE ttable1 OF nothing; CREATE TYPE person_type AS (id int, name text); @@ -60,4 +67,16 @@ INSERT INTO persons VALUES (1, 'test'); CREATE FUNCTION namelen(person_type) RETURNS int LANGUAGE SQL AS $$ SELECT length($1.name) $$; SELECT id, namelen(persons) FROM persons; -DROP TYPE person_type CASCADE; +CREATE TABLE persons2 OF person_type ( + id WITH OPTIONS PRIMARY KEY, + UNIQUE (name) +); + +\d persons2 + +CREATE TABLE persons3 OF person_type ( + PRIMARY KEY (id), + name NOT NULL DEFAULT '' +); + +\d persons3 -- 2.40.0