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

Commit705d433

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 parent12d5f39 commit705d433

File tree

11 files changed

+91
-65
lines changed

11 files changed

+91
-65
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
@@ -1892,17 +1892,6 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
18921892
errmsg("tables can have at most %d columns",
18931893
MaxHeapAttributeNumber)));
18941894

1895-
/*
1896-
* In case of a partition, there are no new column definitions, only dummy
1897-
* ColumnDefs created for column constraints. We merge them with the
1898-
* constraints inherited from the parent.
1899-
*/
1900-
if (is_partition)
1901-
{
1902-
saved_schema=schema;
1903-
schema=NIL;
1904-
}
1905-
19061895
/*
19071896
* Check for duplicate names in the explicit list of attributes.
19081897
*
@@ -1916,17 +1905,19 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
19161905
ListCell*rest=lnext(entry);
19171906
ListCell*prev=entry;
19181907

1919-
if (coldef->typeName==NULL)
1920-
1908+
if (!is_partition&&coldef->typeName==NULL)
1909+
{
19211910
/*
19221911
* Typed table column option that does not belong to a column from
19231912
* the type. This works because the columns from the type come
1924-
* first in the list.
1913+
* first in the list. (We omit this check for partition column
1914+
* lists; those are processed separately below.)
19251915
*/
19261916
ereport(ERROR,
19271917
(errcode(ERRCODE_UNDEFINED_COLUMN),
19281918
errmsg("column \"%s\" does not exist",
19291919
coldef->colname)));
1920+
}
19301921

19311922
while (rest!=NULL)
19321923
{
@@ -1959,6 +1950,17 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
19591950
}
19601951
}
19611952

1953+
/*
1954+
* In case of a partition, there are no new column definitions, only dummy
1955+
* ColumnDefs created for column constraints. Set them aside for now and
1956+
* process them at the end.
1957+
*/
1958+
if (is_partition)
1959+
{
1960+
saved_schema=schema;
1961+
schema=NIL;
1962+
}
1963+
19621964
/*
19631965
* Scan the parents left-to-right, and merge their attributes to form a
19641966
* list of inherited attributes (inhSchema). Also check to see if we need
@@ -2175,7 +2177,6 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
21752177
def->is_local= false;
21762178
def->is_not_null=attribute->attnotnull;
21772179
def->is_from_type= false;
2178-
def->is_from_parent= true;
21792180
def->storage=attribute->attstorage;
21802181
def->raw_default=NULL;
21812182
def->cooked_default=NULL;
@@ -2428,59 +2429,51 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
24282429
/*
24292430
* Now that we have the column definition list for a partition, we can
24302431
* check whether the columns referenced in the column constraint specs
2431-
* actually exist. Also, we mergethe constraints into the corresponding
2432-
* columndefinitions.
2432+
* actually exist. Also, we mergeNOT NULL and defaults into each
2433+
*correspondingcolumndefinition.
24332434
*/
2434-
if (is_partition&&list_length(saved_schema)>0)
2435+
if (is_partition)
24352436
{
2436-
schema=list_concat(schema,saved_schema);
2437-
2438-
foreach(entry,schema)
2437+
foreach(entry,saved_schema)
24392438
{
2440-
ColumnDef*coldef=lfirst(entry);
2441-
ListCell*rest=lnext(entry);
2442-
ListCell*prev=entry;
2439+
ColumnDef*restdef=lfirst(entry);
2440+
boolfound= false;
2441+
ListCell*l;
24432442

2444-
/*
2445-
* Partition column option that does not belong to a column from
2446-
* the parent. This works because the columns from the parent
2447-
* come first in the list (see above).
2448-
*/
2449-
if (coldef->typeName==NULL)
2450-
ereport(ERROR,
2451-
(errcode(ERRCODE_UNDEFINED_COLUMN),
2452-
errmsg("column \"%s\" does not exist",
2453-
coldef->colname)));
2454-
while (rest!=NULL)
2443+
foreach(l,schema)
24552444
{
2456-
ColumnDef*restdef=lfirst(rest);
2457-
ListCell*next=lnext(rest);/* need to save it in case we
2458-
* delete it */
2445+
ColumnDef*coldef=lfirst(l);
24592446

24602447
if (strcmp(coldef->colname,restdef->colname)==0)
24612448
{
2449+
found= true;
2450+
coldef->is_not_null |=restdef->is_not_null;
2451+
24622452
/*
2463-
* merge the column options into the column from the
2464-
* parent
2453+
* Override the parent's default value for this column
2454+
* (coldef->cooked_default) with the partition's local
2455+
* definition (restdef->raw_default), if there's one. It
2456+
* should be physically impossible to get a cooked default
2457+
* in the local definition or a raw default in the
2458+
* inherited definition, but make sure they're nulls, for
2459+
* future-proofing.
24652460
*/
2466-
if (coldef->is_from_parent)
2461+
Assert(restdef->cooked_default==NULL);
2462+
Assert(coldef->raw_default==NULL);
2463+
if (restdef->raw_default)
24672464
{
2468-
coldef->is_not_null=restdef->is_not_null;
24692465
coldef->raw_default=restdef->raw_default;
2470-
coldef->cooked_default=restdef->cooked_default;
2471-
coldef->constraints=restdef->constraints;
2472-
coldef->is_from_parent= false;
2473-
list_delete_cell(schema,rest,prev);
2466+
coldef->cooked_default=NULL;
24742467
}
2475-
else
2476-
ereport(ERROR,
2477-
(errcode(ERRCODE_DUPLICATE_COLUMN),
2478-
errmsg("column \"%s\" specified more than once",
2479-
coldef->colname)));
24802468
}
2481-
prev=rest;
2482-
rest=next;
24832469
}
2470+
2471+
/* complain for constraints on columns not in parent */
2472+
if (!found)
2473+
ereport(ERROR,
2474+
(errcode(ERRCODE_UNDEFINED_COLUMN),
2475+
errmsg("column \"%s\" does not exist",
2476+
restdef->colname)));
24842477
}
24852478
}
24862479

‎src/backend/nodes/copyfuncs.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2873,7 +2873,6 @@ _copyColumnDef(const ColumnDef *from)
28732873
COPY_SCALAR_FIELD(is_local);
28742874
COPY_SCALAR_FIELD(is_not_null);
28752875
COPY_SCALAR_FIELD(is_from_type);
2876-
COPY_SCALAR_FIELD(is_from_parent);
28772876
COPY_SCALAR_FIELD(storage);
28782877
COPY_NODE_FIELD(raw_default);
28792878
COPY_NODE_FIELD(cooked_default);

‎src/backend/nodes/equalfuncs.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2553,7 +2553,6 @@ _equalColumnDef(const ColumnDef *a, const ColumnDef *b)
25532553
COMPARE_SCALAR_FIELD(is_local);
25542554
COMPARE_SCALAR_FIELD(is_not_null);
25552555
COMPARE_SCALAR_FIELD(is_from_type);
2556-
COMPARE_SCALAR_FIELD(is_from_parent);
25572556
COMPARE_SCALAR_FIELD(storage);
25582557
COMPARE_NODE_FIELD(raw_default);
25592558
COMPARE_NODE_FIELD(cooked_default);

‎src/backend/nodes/makefuncs.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -496,7 +496,6 @@ makeColumnDef(const char *colname, Oid typeOid, int32 typmod, Oid collOid)
496496
n->is_local= true;
497497
n->is_not_null= false;
498498
n->is_from_type= false;
499-
n->is_from_parent= false;
500499
n->storage=0;
501500
n->raw_default=NULL;
502501
n->cooked_default=NULL;

‎src/backend/nodes/outfuncs.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2874,7 +2874,6 @@ _outColumnDef(StringInfo str, const ColumnDef *node)
28742874
WRITE_BOOL_FIELD(is_local);
28752875
WRITE_BOOL_FIELD(is_not_null);
28762876
WRITE_BOOL_FIELD(is_from_type);
2877-
WRITE_BOOL_FIELD(is_from_parent);
28782877
WRITE_CHAR_FIELD(storage);
28792878
WRITE_NODE_FIELD(raw_default);
28802879
WRITE_NODE_FIELD(cooked_default);

‎src/backend/parser/gram.y

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3388,7 +3388,6 @@ columnDef:ColId Typename create_generic_options ColQualList
33883388
n->is_local =true;
33893389
n->is_not_null =false;
33903390
n->is_from_type =false;
3391-
n->is_from_parent =false;
33923391
n->storage =0;
33933392
n->raw_default =NULL;
33943393
n->cooked_default =NULL;
@@ -3410,7 +3409,6 @@ columnOptions:ColId ColQualList
34103409
n->is_local =true;
34113410
n->is_not_null =false;
34123411
n->is_from_type =false;
3413-
n->is_from_parent =false;
34143412
n->storage =0;
34153413
n->raw_default =NULL;
34163414
n->cooked_default =NULL;
@@ -3429,7 +3427,6 @@ columnOptions:ColId ColQualList
34293427
n->is_local =true;
34303428
n->is_not_null =false;
34313429
n->is_from_type =false;
3432-
n->is_from_parent =false;
34333430
n->storage =0;
34343431
n->raw_default =NULL;
34353432
n->cooked_default =NULL;
@@ -12267,7 +12264,6 @@ TableFuncElement:ColId Typename opt_collate_clause
1226712264
n->is_local =true;
1226812265
n->is_not_null =false;
1226912266
n->is_from_type =false;
12270-
n->is_from_parent =false;
1227112267
n->storage =0;
1227212268
n->raw_default =NULL;
1227312269
n->cooked_default =NULL;

‎src/backend/parser/parse_utilcmd.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -988,7 +988,6 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
988988
def->is_local= true;
989989
def->is_not_null=attribute->attnotnull;
990990
def->is_from_type= false;
991-
def->is_from_parent= false;
992991
def->storage=0;
993992
def->raw_default=NULL;
994993
def->cooked_default=NULL;
@@ -1265,7 +1264,6 @@ transformOfType(CreateStmtContext *cxt, TypeName *ofTypename)
12651264
n->is_local= true;
12661265
n->is_not_null= false;
12671266
n->is_from_type= true;
1268-
n->is_from_parent= false;
12691267
n->storage=0;
12701268
n->raw_default=NULL;
12711269
n->cooked_default=NULL;

‎src/include/nodes/parsenodes.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -649,7 +649,6 @@ typedef struct ColumnDef
649649
boolis_local;/* column has local (non-inherited) def'n */
650650
boolis_not_null;/* NOT NULL constraint specified? */
651651
boolis_from_type;/* column definition came from table type */
652-
boolis_from_parent;/* column def came from partition parent */
653652
charstorage;/* attstorage setting, or 0 for default */
654653
Node*raw_default;/* default value (untransformed parse tree) */
655654
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
@@ -728,6 +728,32 @@ LINE 1: ...TITION OF parted FOR VALUES IN ('c') PARTITION BY RANGE (c);
728728
CREATE TABLE part_c PARTITION OF parted (b WITH OPTIONS NOT NULL DEFAULT 0) FOR VALUES IN ('c') PARTITION BY RANGE ((b));
729729
-- create a level-2 partition
730730
CREATE TABLE part_c_1_10 PARTITION OF part_c FOR VALUES FROM (1) TO (10);
731+
-- check that NOT NULL and default value are inherited correctly
732+
create table parted_notnull_inh_test (a int default 1, b int not null default 0) partition by list (a);
733+
create table parted_notnull_inh_test1 partition of parted_notnull_inh_test (a not null, b default 1) for values in (1);
734+
insert into parted_notnull_inh_test (b) values (null);
735+
ERROR: null value in column "b" violates not-null constraint
736+
DETAIL: Failing row contains (1, null).
737+
-- note that while b's default is overriden, a's default is preserved
738+
\d parted_notnull_inh_test1
739+
Table "public.parted_notnull_inh_test1"
740+
Column | Type | Collation | Nullable | Default
741+
--------+---------+-----------+----------+---------
742+
a | integer | | not null | 1
743+
b | integer | | not null | 1
744+
Partition of: parted_notnull_inh_test FOR VALUES IN (1)
745+
746+
drop table parted_notnull_inh_test;
747+
-- check for a conflicting COLLATE clause
748+
create table parted_collate_must_match (a text collate "C", b text collate "C")
749+
partition by range (a);
750+
-- on the partition key
751+
create table parted_collate_must_match1 partition of parted_collate_must_match
752+
(a collate "POSIX") for values from ('a') to ('m');
753+
-- on another column
754+
create table parted_collate_must_match2 partition of parted_collate_must_match
755+
(b collate "POSIX") for values from ('m') to ('z');
756+
drop table parted_collate_must_match;
731757
-- Partition bound in describe output
732758
\d+ part_b
733759
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
@@ -654,6 +654,25 @@ CREATE TABLE part_c PARTITION OF parted (b WITH OPTIONS NOT NULL DEFAULT 0) FOR
654654
-- create a level-2 partition
655655
CREATETABLEpart_c_1_10 PARTITION OF part_c FORVALUESFROM (1) TO (10);
656656

657+
-- check that NOT NULL and default value are inherited correctly
658+
createtableparted_notnull_inh_test (aint default1, bintnot null default0) partition by list (a);
659+
createtableparted_notnull_inh_test1 partition of parted_notnull_inh_test (anot null, b default1) forvaluesin (1);
660+
insert into parted_notnull_inh_test (b)values (null);
661+
-- note that while b's default is overriden, a's default is preserved
662+
\d parted_notnull_inh_test1
663+
droptable parted_notnull_inh_test;
664+
665+
-- check for a conflicting COLLATE clause
666+
createtableparted_collate_must_match (atext collate"C", btext collate"C")
667+
partition by range (a);
668+
-- on the partition key
669+
createtableparted_collate_must_match1 partition of parted_collate_must_match
670+
(a collate"POSIX") forvaluesfrom ('a') to ('m');
671+
-- on another column
672+
createtableparted_collate_must_match2 partition of parted_collate_must_match
673+
(b collate"POSIX") forvaluesfrom ('m') to ('z');
674+
droptable parted_collate_must_match;
675+
657676
-- Partition bound in describe output
658677
\d+ part_b
659678

0 commit comments

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