Skip to content

Commit 3ec76ff

Browse files
robertmhaasrobertmhaas
committed
Don't explicitly mark range partitioning columns NOT NULL.
This seemed like a good idea originally because there's no way to mark a range partition as accepting NULL, but that now seems more like a current limitation than something we want to lock down for all time. For example, there's a proposal to add the notion of a default partition which accepts all rows not otherwise routed, which directly conflicts with the idea that a range-partitioned table should never allow nulls anywhere. So let's change this while we still can, by putting the NOT NULL test into the partition constraint instead of changing the column properties. Amit Langote and Robert Haas, reviewed by Amit Kapila Discussion: http://postgr.es/m/8e2dd63d-c6fb-bb74-3c2b-ed6d63629c9d@lab.ntt.co.jp
1 parent 2df537e commit 3ec76ff

File tree

9 files changed

+64
-138
lines changed

9 files changed

+64
-138
lines changed

‎contrib/postgres_fdw/expected/postgres_fdw.out

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6987,7 +6987,7 @@ FDW Options: (schema_name 'import_source', table_name 't3')
69876987
Foreign table "import_dest1.t4"
69886988
Column | Type | Collation | Nullable | Default | FDW Options
69896989
--------+---------+-----------+----------+---------+--------------------
6990-
c1 | integer | | not null | | (column_name 'c1')
6990+
c1 | integer | | | | (column_name 'c1')
69916991
Server: loopback
69926992
FDW Options: (schema_name 'import_source', table_name 't4')
69936993

@@ -7051,7 +7051,7 @@ FDW Options: (schema_name 'import_source', table_name 't3')
70517051
Foreign table "import_dest2.t4"
70527052
Column | Type | Collation | Nullable | Default | FDW Options
70537053
--------+---------+-----------+----------+---------+--------------------
7054-
c1 | integer | | not null | | (column_name 'c1')
7054+
c1 | integer | | | | (column_name 'c1')
70557055
Server: loopback
70567056
FDW Options: (schema_name 'import_source', table_name 't4')
70577057

‎doc/src/sgml/ref/create_table.sgml

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -454,11 +454,6 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXI
454454
these constraints on individual partitions.
455455
</para>
456456

457-
<para>
458-
When using range partitioning, a <literal>NOT NULL</literal> constraint
459-
is added to each non-expression column in the partition key.
460-
</para>
461-
462457
</listitem>
463458
</varlistentry>
464459

‎src/backend/catalog/partition.c

Lines changed: 44 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1449,17 +1449,18 @@ get_range_key_properties(PartitionKey key, int keynum,
14491449
* as the lower bound tuple and (au, bu, cu) as the upper bound tuple, we
14501450
* generate an expression tree of the following form:
14511451
*
1452+
* (a IS NOT NULL) and (b IS NOT NULL) and (c IS NOT NULL)
1453+
* AND
14521454
* (a > al OR (a = al AND b > bl) OR (a = al AND b = bl AND c >= cl))
14531455
* AND
14541456
* (a < au OR (a = au AND b < bu) OR (a = au AND b = bu AND c < cu))
14551457
*
1456-
* If, say, b were an expression key instead of a simple column, we also
1457-
* append (b IS NOT NULL) to the AND's argument list.
1458-
*
14591458
* It is often the case that a prefix of lower and upper bound tuples contains
14601459
* the same values, for example, (al = au), in which case, we will emit an
14611460
* expression tree of the following form:
14621461
*
1462+
* (a IS NOT NULL) and (b IS NOT NULL) and (c IS NOT NULL)
1463+
* AND
14631464
* (a = al)
14641465
* AND
14651466
* (b > bl OR (b = bl AND c >= cl))
@@ -1472,11 +1473,11 @@ get_range_key_properties(PartitionKey key, int keynum,
14721473
* (b < bu) OR (b = bu), which is simplified to (b <= bu)
14731474
*
14741475
* In most common cases with only one partition column, say a, the following
1475-
* expression tree will be generated: a >= al AND a < au
1476+
* expression tree will be generated: a IS NOT NULL AND a >= al AND a < au
14761477
*
14771478
* If all values of both lower and upper bounds are UNBOUNDED, the partition
14781479
* does not really have a constraint, except the IS NOT NULL constraint for
1479-
* any expression keys.
1480+
* partition keys.
14801481
*
14811482
* If we end up with an empty result list, we append return a single-member
14821483
* list containing a constant-true expression in that case, because callers
@@ -1512,32 +1513,37 @@ get_qual_for_range(PartitionKey key, PartitionBoundSpec *spec)
15121513
num_or_arms = key->partnatts;
15131514

15141515
/*
1515-
* A range-partitioned table does not allow partition keys to be null. For
1516-
* simple columns, their NOT NULL constraint suffices for the enforcement
1517-
* of non-nullability. But for the expression keys, which are still
1518-
* nullable, we must emit a IS NOT NULL expression. Collect them in
1519-
* result first.
1516+
* A range-partitioned table does not currently allow partition keys to
1517+
* be null, so emit an IS NOT NULL expression for each key column.
15201518
*/
15211519
partexprs_item = list_head(key->partexprs);
15221520
for (i = 0; i < key->partnatts; i++)
15231521
{
1524-
if (key->partattrs[i] == 0)
1525-
{
1526-
Expr *keyCol;
1522+
Expr *keyCol;
15271523

1524+
if (key->partattrs[i] != 0)
1525+
{
1526+
keyCol = (Expr *) makeVar(1,
1527+
key->partattrs[i],
1528+
key->parttypid[i],
1529+
key->parttypmod[i],
1530+
key->parttypcoll[i],
1531+
0);
1532+
}
1533+
else
1534+
{
15281535
if (partexprs_item == NULL)
15291536
elog(ERROR, "wrong number of partition key expressions");
1530-
keyCol = lfirst(partexprs_item);
1537+
keyCol = copyObject(lfirst(partexprs_item));
15311538
partexprs_item = lnext(partexprs_item);
1532-
Assert(!IsA(keyCol, Var));
1533-
1534-
nulltest = makeNode(NullTest);
1535-
nulltest->arg = keyCol;
1536-
nulltest->nulltesttype = IS_NOT_NULL;
1537-
nulltest->argisrow = false;
1538-
nulltest->location = -1;
1539-
result = lappend(result, nulltest);
15401539
}
1540+
1541+
nulltest = makeNode(NullTest);
1542+
nulltest->arg = keyCol;
1543+
nulltest->nulltesttype = IS_NOT_NULL;
1544+
nulltest->argisrow = false;
1545+
nulltest->location = -1;
1546+
result = lappend(result, nulltest);
15411547
}
15421548

15431549
/*
@@ -1948,7 +1954,8 @@ get_partition_for_tuple(PartitionDispatch *pd,
19481954
{
19491955
*failed_at = parent;
19501956
*failed_slot = slot;
1951-
return -1;
1957+
result = -1;
1958+
goto error_exit;
19521959
}
19531960

19541961
/*
@@ -1964,12 +1971,21 @@ get_partition_for_tuple(PartitionDispatch *pd,
19641971

19651972
if (key->strategy == PARTITION_STRATEGY_RANGE)
19661973
{
1967-
/* Disallow nulls in the range partition key of the tuple */
1974+
/*
1975+
* Since we cannot route tuples with NULL partition keys through
1976+
* a range-partitioned table, simply return that no partition
1977+
* exists
1978+
*/
19681979
for (i = 0; i < key->partnatts; i++)
1980+
{
19691981
if (isnull[i])
1970-
ereport(ERROR,
1971-
(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
1972-
errmsg("range partition key of row contains null")));
1982+
{
1983+
*failed_at = parent;
1984+
*failed_slot = slot;
1985+
result = -1;
1986+
goto error_exit;
1987+
}
1988+
}
19731989
}
19741990

19751991
/*
@@ -2032,6 +2048,7 @@ get_partition_for_tuple(PartitionDispatch *pd,
20322048
parent = pd[-parent->indexes[cur_index]];
20332049
}
20342050

2051+
error_exit:
20352052
ecxt->ecxt_scantuple = ecxt_scantuple_old;
20362053
return result;
20372054
}

‎src/backend/commands/tablecmds.c

Lines changed: 1 addition & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -805,13 +805,11 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
805805
if (stmt->partspec)
806806
{
807807
char strategy;
808-
int partnatts,
809-
i;
808+
int partnatts;
810809
AttrNumber partattrs[PARTITION_MAX_KEYS];
811810
Oid partopclass[PARTITION_MAX_KEYS];
812811
Oid partcollation[PARTITION_MAX_KEYS];
813812
List *partexprs = NIL;
814-
List *cmds = NIL;
815813

816814
/*
817815
* We need to transform the raw parsetrees corresponding to partition
@@ -828,33 +826,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
828826
partnatts = list_length(stmt->partspec->partParams);
829827
StorePartitionKey(rel, strategy, partnatts, partattrs, partexprs,
830828
partopclass, partcollation);
831-
832-
/* Force key columns to be NOT NULL when using range partitioning */
833-
if (strategy == PARTITION_STRATEGY_RANGE)
834-
{
835-
for (i = 0; i < partnatts; i++)
836-
{
837-
AttrNumber partattno = partattrs[i];
838-
Form_pg_attribute attform = descriptor->attrs[partattno - 1];
839-
840-
if (partattno != 0 && !attform->attnotnull)
841-
{
842-
/* Add a subcommand to make this one NOT NULL */
843-
AlterTableCmd *cmd = makeNode(AlterTableCmd);
844-
845-
cmd->subtype = AT_SetNotNull;
846-
cmd->name = pstrdup(NameStr(attform->attname));
847-
cmds = lappend(cmds, cmd);
848-
}
849-
}
850-
851-
/*
852-
* Although, there cannot be any partitions yet, we still need to
853-
* pass true for recurse; ATPrepSetNotNull() complains if we don't
854-
*/
855-
if (cmds != NIL)
856-
AlterTableInternal(RelationGetRelid(rel), cmds, true);
857-
}
858829
}
859830

860831
/*
@@ -5702,32 +5673,6 @@ ATExecDropNotNull(Relation rel, const char *colName, LOCKMODE lockmode)
57025673
heap_close(parent, AccessShareLock);
57035674
}
57045675

5705-
/*
5706-
* If the table is a range partitioned table, check that the column is not
5707-
* in the partition key.
5708-
*/
5709-
if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
5710-
{
5711-
PartitionKey key = RelationGetPartitionKey(rel);
5712-
5713-
if (get_partition_strategy(key) == PARTITION_STRATEGY_RANGE)
5714-
{
5715-
int partnatts = get_partition_natts(key),
5716-
i;
5717-
5718-
for (i = 0; i < partnatts; i++)
5719-
{
5720-
AttrNumber partattnum = get_partition_col_attnum(key, i);
5721-
5722-
if (partattnum == attnum)
5723-
ereport(ERROR,
5724-
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
5725-
errmsg("column \"%s\" is in range partition key",
5726-
colName)));
5727-
}
5728-
}
5729-
}
5730-
57315676
/*
57325677
* Okay, actually perform the catalog change ... if needed
57335678
*/

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

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3079,13 +3079,6 @@ ALTER TABLE partitioned DROP COLUMN b;
30793079
ERROR: cannot drop column referenced in partition key expression
30803080
ALTER TABLE partitioned ALTER COLUMN b TYPE char(5);
30813081
ERROR: cannot alter type of column referenced in partition key expression
3082-
-- cannot drop NOT NULL on columns in the range partition key
3083-
ALTER TABLE partitioned ALTER COLUMN a DROP NOT NULL;
3084-
ERROR: column "a" is in range partition key
3085-
-- it's fine however to drop one on the list partition key column
3086-
CREATE TABLE list_partitioned (a int not null) partition by list (a);
3087-
ALTER TABLE list_partitioned ALTER a DROP NOT NULL;
3088-
DROP TABLE list_partitioned;
30893082
-- partitioned table cannot participate in regular inheritance
30903083
CREATE TABLE nonpartitioned (
30913084
a int,

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

Lines changed: 14 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -407,18 +407,6 @@ SELECT relkind FROM pg_class WHERE relname = 'partitioned';
407407
p
408408
(1 row)
409409

410-
-- check that range partition key columns are marked NOT NULL
411-
SELECT attname, attnotnull FROM pg_attribute
412-
WHERE attrelid = 'partitioned'::regclass AND attnum > 0
413-
ORDER BY attnum;
414-
attname | attnotnull
415-
---------+------------
416-
a | t
417-
b | f
418-
c | t
419-
d | t
420-
(4 rows)
421-
422410
-- prevent a function referenced in partition key from being dropped
423411
DROP FUNCTION plusone(int);
424412
ERROR: cannot drop function plusone(integer) because other objects depend on it
@@ -435,10 +423,10 @@ ERROR: cannot inherit from partitioned table "partitioned2"
435423
Table "public.partitioned"
436424
Column | Type | Collation | Nullable | Default
437425
--------+---------+-----------+----------+---------
438-
a | integer | | not null |
426+
a | integer | | |
439427
b | integer | | |
440-
c | text | | not null |
441-
d | text | | not null |
428+
c | text | | |
429+
d | text | | |
442430
Partition key: RANGE (a oid_ops, plusone(b), c, d COLLATE "C")
443431

444432
\d partitioned2
@@ -544,9 +532,9 @@ CREATE TABLE part_forced_oids PARTITION OF oids_parted FOR VALUES FROM (1) TO (1
544532
Table "public.part_forced_oids"
545533
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
546534
--------+---------+-----------+----------+---------+---------+--------------+-------------
547-
a | integer | | not null | | plain | |
535+
a | integer | | | | plain | |
548536
Partition of: oids_parted FOR VALUES FROM (1) TO (10)
549-
Partition constraint: ((a >= 1) AND (a < 10))
537+
Partition constraint: ((a IS NOT NULL) AND (a >= 1) AND (a < 10))
550538
Has OIDs: yes
551539

552540
DROP TABLE oids_parted, part_forced_oids;
@@ -678,7 +666,7 @@ Partitions: part_c_1_10 FOR VALUES FROM (1) TO (10)
678666
a | text | | | | extended | |
679667
b | integer | | not null | 0 | plain | |
680668
Partition of: part_c FOR VALUES FROM (1) TO (10)
681-
Partition constraint: ((a IS NOT NULL) AND (a = ANY (ARRAY['c'::text])) AND (b >= 1) AND (b < 10))
669+
Partition constraint: ((a IS NOT NULL) AND (a = ANY (ARRAY['c'::text])) AND (b IS NOT NULL) AND (b >= 1) AND (b < 10))
682670
Check constraints:
683671
"check_a" CHECK (length(a) > 0)
684672

@@ -706,9 +694,9 @@ CREATE TABLE unbounded_range_part PARTITION OF range_parted4 FOR VALUES FROM (UN
706694
--------+---------+-----------+----------+---------+---------+--------------+-------------
707695
a | integer | | | | plain | |
708696
b | integer | | | | plain | |
709-
c | integer | | not null | | plain | |
697+
c | integer | | | | plain | |
710698
Partition of: range_parted4 FOR VALUES FROM (UNBOUNDED, UNBOUNDED, UNBOUNDED) TO (UNBOUNDED, UNBOUNDED, UNBOUNDED)
711-
Partition constraint: ((abs(a) IS NOT NULL) AND (abs(b) IS NOT NULL))
699+
Partition constraint: ((abs(a) IS NOT NULL) AND (abs(b) IS NOT NULL) AND (c IS NOT NULL))
712700

713701
DROP TABLE unbounded_range_part;
714702
CREATE TABLE range_parted4_1 PARTITION OF range_parted4 FOR VALUES FROM (UNBOUNDED, UNBOUNDED, UNBOUNDED) TO (1, UNBOUNDED, UNBOUNDED);
@@ -718,9 +706,9 @@ CREATE TABLE range_parted4_1 PARTITION OF range_parted4 FOR VALUES FROM (UNBOUND
718706
--------+---------+-----------+----------+---------+---------+--------------+-------------
719707
a | integer | | | | plain | |
720708
b | integer | | | | plain | |
721-
c | integer | | not null | | plain | |
709+
c | integer | | | | plain | |
722710
Partition of: range_parted4 FOR VALUES FROM (UNBOUNDED, UNBOUNDED, UNBOUNDED) TO (1, UNBOUNDED, UNBOUNDED)
723-
Partition constraint: ((abs(a) IS NOT NULL) AND (abs(b) IS NOT NULL) AND (abs(a) <= 1))
711+
Partition constraint: ((abs(a) IS NOT NULL) AND (abs(b) IS NOT NULL) AND (c IS NOT NULL) AND (abs(a) <= 1))
724712

725713
CREATE TABLE range_parted4_2 PARTITION OF range_parted4 FOR VALUES FROM (3, 4, 5) TO (6, 7, UNBOUNDED);
726714
\d+ range_parted4_2
@@ -729,9 +717,9 @@ CREATE TABLE range_parted4_2 PARTITION OF range_parted4 FOR VALUES FROM (3, 4, 5
729717
--------+---------+-----------+----------+---------+---------+--------------+-------------
730718
a | integer | | | | plain | |
731719
b | integer | | | | plain | |
732-
c | integer | | not null | | plain | |
720+
c | integer | | | | plain | |
733721
Partition of: range_parted4 FOR VALUES FROM (3, 4, 5) TO (6, 7, UNBOUNDED)
734-
Partition constraint: ((abs(a) IS NOT NULL) AND (abs(b) IS NOT NULL) AND ((abs(a) > 3) OR ((abs(a) = 3) AND (abs(b) > 4)) OR ((abs(a) = 3) AND (abs(b) = 4) AND (c >= 5))) AND ((abs(a) < 6) OR ((abs(a) = 6) AND (abs(b) <= 7))))
722+
Partition constraint: ((abs(a) IS NOT NULL) AND (abs(b) IS NOT NULL) AND (c IS NOT NULL) AND ((abs(a) > 3) OR ((abs(a) = 3) AND (abs(b) > 4)) OR ((abs(a) = 3) AND (abs(b) = 4) AND (c >= 5))) AND ((abs(a) < 6) OR ((abs(a) = 6) AND (abs(b) <= 7))))
735723

736724
CREATE TABLE range_parted4_3 PARTITION OF range_parted4 FOR VALUES FROM (6, 8, UNBOUNDED) TO (9, UNBOUNDED, UNBOUNDED);
737725
\d+ range_parted4_3
@@ -740,9 +728,9 @@ CREATE TABLE range_parted4_3 PARTITION OF range_parted4 FOR VALUES FROM (6, 8, U
740728
--------+---------+-----------+----------+---------+---------+--------------+-------------
741729
a | integer | | | | plain | |
742730
b | integer | | | | plain | |
743-
c | integer | | not null | | plain | |
731+
c | integer | | | | plain | |
744732
Partition of: range_parted4 FOR VALUES FROM (6, 8, UNBOUNDED) TO (9, UNBOUNDED, UNBOUNDED)
745-
Partition constraint: ((abs(a) IS NOT NULL) AND (abs(b) IS NOT NULL) AND ((abs(a) > 6) OR ((abs(a) = 6) AND (abs(b) >= 8))) AND (abs(a) <= 9))
733+
Partition constraint: ((abs(a) IS NOT NULL) AND (abs(b) IS NOT NULL) AND (c IS NOT NULL) AND ((abs(a) > 6) OR ((abs(a) = 6) AND (abs(b) >= 8))) AND (abs(a) <= 9))
746734

747735
DROP TABLE range_parted4;
748736
-- cleanup

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ DETAIL: Failing row contains (a, 10).
189189
insert into part4 values ('b', 10);
190190
-- fail (partition key a has a NOT NULL constraint)
191191
insert into part1 values (null);
192-
ERROR: null value in column "a" violates not-null constraint
192+
ERROR: new row for relation "part1" violates partition constraint
193193
DETAIL: Failing row contains (null, null).
194194
-- fail (expression key (b+0) cannot be null either)
195195
insert into part1 values (1);
@@ -247,7 +247,8 @@ insert into range_parted values ('b', 1);
247247
insert into range_parted values ('b', 10);
248248
-- fail (partition key (b+0) is null)
249249
insert into range_parted values ('a');
250-
ERROR: range partition key of row contains null
250+
ERROR: no partition of relation "range_parted" found for row
251+
DETAIL: Partition key of the failing row contains (a, (b + 0)) = (a, null).
251252
select tableoid::regclass, * from range_parted;
252253
tableoid | a | b
253254
----------+---+----

‎src/test/regress/sql/alter_table.sql

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1947,14 +1947,6 @@ ALTER TABLE partitioned ALTER COLUMN a TYPE char(5);
19471947
ALTER TABLE partitioned DROP COLUMN b;
19481948
ALTER TABLE partitioned ALTER COLUMN b TYPE char(5);
19491949

1950-
-- cannot drop NOT NULL on columns in the range partition key
1951-
ALTER TABLE partitioned ALTER COLUMN a DROP NOT NULL;
1952-
1953-
-- it's fine however to drop one on the list partition key column
1954-
CREATE TABLE list_partitioned (a int not null) partition by list (a);
1955-
ALTER TABLE list_partitioned ALTER a DROP NOT NULL;
1956-
DROP TABLE list_partitioned;
1957-
19581950
-- partitioned table cannot participate in regular inheritance
19591951
CREATE TABLE nonpartitioned (
19601952
a int,

0 commit comments

Comments
 (0)