Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit21c9e49

Browse files
committed
Revise attribute handling code on partition creation
The original code to propagate NOT NULL and default expressionsspecified when creating a partition was mostly copy-pasted fromtyped-tables creation, but not being a great match it contained someduplicity, inefficiency and bugs.This commit fixes the bug that NOT NULL constraints declared in theparent table would not be honored in the partition. One reported issuethat is not fixed is that a DEFAULT declared in the child is not usedwhen inserting through the parent. That would amount to a behavioralchange that's better not back-patched.This rewrite makes the code simpler:1. instead of checking for duplicate column names in its own block,reuse the original one that already did that;2. instead of concatenating the list of columns from parent and the onedeclared in the partition and scanning the result to (incorrectly)propagate defaults and not-null constraints, just scan the lattersearching the former for a match, and merging sensibly. This worksbecause we know the list in the parent is already correct and there canonly be one parent.This rewrite makes ColumnDef->is_from_parent unused, so it's removedon branch master; on released branches, it's kept as an unused field inorder not to cause ABI incompatibilities.This commit also adds a test case for creating partitions withcollations mismatching that on the parent table, something that isclosely related to the code being patched. No code change is introducedthough, since that'd be a behavior change that could break some (broken)working applications.Amit Langote wrote a less invasive fix for the originalNOT NULL/defaults bug, but while I kept the tests he added, I ended upnot using his original code. Ashutosh Bapat reviewed Amit's fix. Amitreviewed mine.Author: Álvaro Herrera, Amit LangoteReviewed-by: Ashutosh Bapat, Amit LangoteReported-by: Jürgen Strobel (bug #15212)Discussion:https://postgr.es/m/152746742177.1291.9847032632907407358@wrigleys.postgresql.org
1 parent018e5fe commit21c9e49

File tree

9 files changed

+92
-63
lines changed

9 files changed

+92
-63
lines changed

‎src/backend/commands/sequence.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,6 @@ DefineSequence(ParseState *pstate, CreateSeqStmt *seq)
172172
coldef->is_local= true;
173173
coldef->is_not_null= true;
174174
coldef->is_from_type= false;
175-
coldef->is_from_parent= false;
176175
coldef->storage=0;
177176
coldef->raw_default=NULL;
178177
coldef->cooked_default=NULL;

‎src/backend/commands/tablecmds.c

Lines changed: 46 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1649,17 +1649,6 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
16491649
errmsg("tables can have at most %d columns",
16501650
MaxHeapAttributeNumber)));
16511651

1652-
/*
1653-
* In case of a partition, there are no new column definitions, only dummy
1654-
* ColumnDefs created for column constraints. We merge them with the
1655-
* constraints inherited from the parent.
1656-
*/
1657-
if (is_partition)
1658-
{
1659-
saved_schema=schema;
1660-
schema=NIL;
1661-
}
1662-
16631652
/*
16641653
* Check for duplicate names in the explicit list of attributes.
16651654
*
@@ -1673,17 +1662,19 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
16731662
ListCell*rest=lnext(entry);
16741663
ListCell*prev=entry;
16751664

1676-
if (coldef->typeName==NULL)
1677-
1665+
if (!is_partition&&coldef->typeName==NULL)
1666+
{
16781667
/*
16791668
* Typed table column option that does not belong to a column from
16801669
* the type. This works because the columns from the type come
1681-
* first in the list.
1670+
* first in the list. (We omit this check for partition column
1671+
* lists; those are processed separately below.)
16821672
*/
16831673
ereport(ERROR,
16841674
(errcode(ERRCODE_UNDEFINED_COLUMN),
16851675
errmsg("column \"%s\" does not exist",
16861676
coldef->colname)));
1677+
}
16871678

16881679
while (rest!=NULL)
16891680
{
@@ -1716,6 +1707,17 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
17161707
}
17171708
}
17181709

1710+
/*
1711+
* In case of a partition, there are no new column definitions, only dummy
1712+
* ColumnDefs created for column constraints. Set them aside for now and
1713+
* process them at the end.
1714+
*/
1715+
if (is_partition)
1716+
{
1717+
saved_schema=schema;
1718+
schema=NIL;
1719+
}
1720+
17191721
/*
17201722
* Scan the parents left-to-right, and merge their attributes to form a
17211723
* list of inherited attributes (inhSchema). Also check to see if we need
@@ -1931,7 +1933,6 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
19311933
def->is_local= false;
19321934
def->is_not_null=attribute->attnotnull;
19331935
def->is_from_type= false;
1934-
def->is_from_parent= true;
19351936
def->storage=attribute->attstorage;
19361937
def->raw_default=NULL;
19371938
def->cooked_default=NULL;
@@ -2184,59 +2185,51 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
21842185
/*
21852186
* Now that we have the column definition list for a partition, we can
21862187
* check whether the columns referenced in the column constraint specs
2187-
* actually exist. Also, we mergethe constraints into the corresponding
2188-
* columndefinitions.
2188+
* actually exist. Also, we mergeNOT NULL and defaults into each
2189+
*correspondingcolumndefinition.
21892190
*/
2190-
if (is_partition&&list_length(saved_schema)>0)
2191+
if (is_partition)
21912192
{
2192-
schema=list_concat(schema,saved_schema);
2193-
2194-
foreach(entry,schema)
2193+
foreach(entry,saved_schema)
21952194
{
2196-
ColumnDef*coldef=lfirst(entry);
2197-
ListCell*rest=lnext(entry);
2198-
ListCell*prev=entry;
2195+
ColumnDef*restdef=lfirst(entry);
2196+
boolfound= false;
2197+
ListCell*l;
21992198

2200-
/*
2201-
* Partition column option that does not belong to a column from
2202-
* the parent. This works because the columns from the parent
2203-
* come first in the list (see above).
2204-
*/
2205-
if (coldef->typeName==NULL)
2206-
ereport(ERROR,
2207-
(errcode(ERRCODE_UNDEFINED_COLUMN),
2208-
errmsg("column \"%s\" does not exist",
2209-
coldef->colname)));
2210-
while (rest!=NULL)
2199+
foreach(l,schema)
22112200
{
2212-
ColumnDef*restdef=lfirst(rest);
2213-
ListCell*next=lnext(rest);/* need to save it in case we
2214-
* delete it */
2201+
ColumnDef*coldef=lfirst(l);
22152202

22162203
if (strcmp(coldef->colname,restdef->colname)==0)
22172204
{
2205+
found= true;
2206+
coldef->is_not_null |=restdef->is_not_null;
2207+
22182208
/*
2219-
* merge the column options into the column from the
2220-
* parent
2209+
* Override the parent's default value for this column
2210+
* (coldef->cooked_default) with the partition's local
2211+
* definition (restdef->raw_default), if there's one. It
2212+
* should be physically impossible to get a cooked default
2213+
* in the local definition or a raw default in the
2214+
* inherited definition, but make sure they're nulls, for
2215+
* future-proofing.
22212216
*/
2222-
if (coldef->is_from_parent)
2217+
Assert(restdef->cooked_default==NULL);
2218+
Assert(coldef->raw_default==NULL);
2219+
if (restdef->raw_default)
22232220
{
2224-
coldef->is_not_null=restdef->is_not_null;
22252221
coldef->raw_default=restdef->raw_default;
2226-
coldef->cooked_default=restdef->cooked_default;
2227-
coldef->constraints=restdef->constraints;
2228-
coldef->is_from_parent= false;
2229-
list_delete_cell(schema,rest,prev);
2222+
coldef->cooked_default=NULL;
22302223
}
2231-
else
2232-
ereport(ERROR,
2233-
(errcode(ERRCODE_DUPLICATE_COLUMN),
2234-
errmsg("column \"%s\" specified more than once",
2235-
coldef->colname)));
22362224
}
2237-
prev=rest;
2238-
rest=next;
22392225
}
2226+
2227+
/* complain for constraints on columns not in parent */
2228+
if (!found)
2229+
ereport(ERROR,
2230+
(errcode(ERRCODE_UNDEFINED_COLUMN),
2231+
errmsg("column \"%s\" does not exist",
2232+
restdef->colname)));
22402233
}
22412234
}
22422235

‎src/backend/nodes/equalfuncs.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2539,7 +2539,6 @@ _equalColumnDef(const ColumnDef *a, const ColumnDef *b)
25392539
COMPARE_SCALAR_FIELD(is_local);
25402540
COMPARE_SCALAR_FIELD(is_not_null);
25412541
COMPARE_SCALAR_FIELD(is_from_type);
2542-
COMPARE_SCALAR_FIELD(is_from_parent);
25432542
COMPARE_SCALAR_FIELD(storage);
25442543
COMPARE_NODE_FIELD(raw_default);
25452544
COMPARE_NODE_FIELD(cooked_default);

‎src/backend/nodes/makefuncs.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,6 @@ makeColumnDef(const char *colname, Oid typeOid, int32 typmod, Oid collOid)
494494
n->is_local= true;
495495
n->is_not_null= false;
496496
n->is_from_type= false;
497-
n->is_from_parent= false;
498497
n->storage=0;
499498
n->raw_default=NULL;
500499
n->cooked_default=NULL;

‎src/backend/parser/gram.y

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3240,7 +3240,6 @@ columnDef:ColId Typename create_generic_options ColQualList
32403240
n->is_local =true;
32413241
n->is_not_null =false;
32423242
n->is_from_type =false;
3243-
n->is_from_parent =false;
32443243
n->storage =0;
32453244
n->raw_default =NULL;
32463245
n->cooked_default =NULL;
@@ -3262,7 +3261,6 @@ columnOptions:ColId ColQualList
32623261
n->is_local =true;
32633262
n->is_not_null =false;
32643263
n->is_from_type =false;
3265-
n->is_from_parent =false;
32663264
n->storage =0;
32673265
n->raw_default =NULL;
32683266
n->cooked_default =NULL;
@@ -3281,7 +3279,6 @@ columnOptions:ColId ColQualList
32813279
n->is_local =true;
32823280
n->is_not_null =false;
32833281
n->is_from_type =false;
3284-
n->is_from_parent =false;
32853282
n->storage =0;
32863283
n->raw_default =NULL;
32873284
n->cooked_default =NULL;
@@ -11884,7 +11881,6 @@ TableFuncElement:ColId Typename opt_collate_clause
1188411881
n->is_local =true;
1188511882
n->is_not_null =false;
1188611883
n->is_from_type =false;
11887-
n->is_from_parent =false;
1188811884
n->storage =0;
1188911885
n->raw_default =NULL;
1189011886
n->cooked_default =NULL;

‎src/backend/parser/parse_utilcmd.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1025,7 +1025,6 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
10251025
def->is_local= true;
10261026
def->is_not_null=attribute->attnotnull;
10271027
def->is_from_type= false;
1028-
def->is_from_parent= false;
10291028
def->storage=0;
10301029
def->raw_default=NULL;
10311030
def->cooked_default=NULL;
@@ -1293,7 +1292,6 @@ transformOfType(CreateStmtContext *cxt, TypeName *ofTypename)
12931292
n->is_local= true;
12941293
n->is_not_null= false;
12951294
n->is_from_type= true;
1296-
n->is_from_parent= false;
12971295
n->storage=0;
12981296
n->raw_default=NULL;
12991297
n->cooked_default=NULL;

‎src/include/nodes/parsenodes.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -642,7 +642,7 @@ typedef struct ColumnDef
642642
boolis_local;/* column has local (non-inherited) def'n */
643643
boolis_not_null;/* NOT NULL constraint specified? */
644644
boolis_from_type;/* column definition came from table type */
645-
boolis_from_parent;/*column def came from partition parent */
645+
boolis_from_parent;/*XXX unused */
646646
charstorage;/* attstorage setting, or 0 for default */
647647
Node*raw_default;/* default value (untransformed parse tree) */
648648
Node*cooked_default;/* default value (transformed expr tree) */

‎src/test/regress/expected/create_table.out

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -657,6 +657,32 @@ ERROR: column "c" named in partition key does not exist
657657
CREATE TABLE part_c PARTITION OF parted (b WITH OPTIONS NOT NULL DEFAULT 0) FOR VALUES IN ('c') PARTITION BY RANGE ((b));
658658
-- create a level-2 partition
659659
CREATE TABLE part_c_1_10 PARTITION OF part_c FOR VALUES FROM (1) TO (10);
660+
-- check that NOT NULL and default value are inherited correctly
661+
create table parted_notnull_inh_test (a int default 1, b int not null default 0) partition by list (a);
662+
create table parted_notnull_inh_test1 partition of parted_notnull_inh_test (a not null, b default 1) for values in (1);
663+
insert into parted_notnull_inh_test (b) values (null);
664+
ERROR: null value in column "b" violates not-null constraint
665+
DETAIL: Failing row contains (1, null).
666+
-- note that while b's default is overriden, a's default is preserved
667+
\d parted_notnull_inh_test1
668+
Table "public.parted_notnull_inh_test1"
669+
Column | Type | Collation | Nullable | Default
670+
--------+---------+-----------+----------+---------
671+
a | integer | | not null | 1
672+
b | integer | | not null | 1
673+
Partition of: parted_notnull_inh_test FOR VALUES IN (1)
674+
675+
drop table parted_notnull_inh_test;
676+
-- check for a conflicting COLLATE clause
677+
create table parted_collate_must_match (a text collate "C", b text collate "C")
678+
partition by range (a);
679+
-- on the partition key
680+
create table parted_collate_must_match1 partition of parted_collate_must_match
681+
(a collate "POSIX") for values from ('a') to ('m');
682+
-- on another column
683+
create table parted_collate_must_match2 partition of parted_collate_must_match
684+
(b collate "POSIX") for values from ('m') to ('z');
685+
drop table parted_collate_must_match;
660686
-- Partition bound in describe output
661687
\d+ part_b
662688
Table "public.part_b"

‎src/test/regress/sql/create_table.sql

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -604,6 +604,25 @@ CREATE TABLE part_c PARTITION OF parted (b WITH OPTIONS NOT NULL DEFAULT 0) FOR
604604
-- create a level-2 partition
605605
CREATETABLEpart_c_1_10 PARTITION OF part_c FORVALUESFROM (1) TO (10);
606606

607+
-- check that NOT NULL and default value are inherited correctly
608+
createtableparted_notnull_inh_test (aint default1, bintnot null default0) partition by list (a);
609+
createtableparted_notnull_inh_test1 partition of parted_notnull_inh_test (anot null, b default1) forvaluesin (1);
610+
insert into parted_notnull_inh_test (b)values (null);
611+
-- note that while b's default is overriden, a's default is preserved
612+
\d parted_notnull_inh_test1
613+
droptable parted_notnull_inh_test;
614+
615+
-- check for a conflicting COLLATE clause
616+
createtableparted_collate_must_match (atext collate"C", btext collate"C")
617+
partition by range (a);
618+
-- on the partition key
619+
createtableparted_collate_must_match1 partition of parted_collate_must_match
620+
(a collate"POSIX") forvaluesfrom ('a') to ('m');
621+
-- on another column
622+
createtableparted_collate_must_match2 partition of parted_collate_must_match
623+
(b collate"POSIX") forvaluesfrom ('m') to ('z');
624+
droptable parted_collate_must_match;
625+
607626
-- Partition bound in describe output
608627
\d+ part_b
609628

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp