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

contrib/postgres_fdw/expected/postgres_fdw.out

@@ -6987,7 +6987,7 @@ FDW Options: (schema_name 'import_source', table_name 't3')
                     Foreign table "import_dest1.t4"
  Column |  Type   | Collation | Nullable | Default |    FDW Options     
 --------+---------+-----------+----------+---------+--------------------
- c1     | integer |           | not null |         | (column_name 'c1')
+ c1     | integer |           |          |         | (column_name 'c1')
 Server: loopback
 FDW Options: (schema_name 'import_source', table_name 't4')
 
@@ -7051,7 +7051,7 @@ FDW Options: (schema_name 'import_source', table_name 't3')
                     Foreign table "import_dest2.t4"
  Column |  Type   | Collation | Nullable | Default |    FDW Options     
 --------+---------+-----------+----------+---------+--------------------
- c1     | integer |           | not null |         | (column_name 'c1')
+ c1     | integer |           |          |         | (column_name 'c1')
 Server: loopback
 FDW Options: (schema_name 'import_source', table_name 't4')
 

doc/src/sgml/ref/create_table.sgml

@@ -454,11 +454,6 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXI
       these constraints on individual partitions.
      </para>
 
-     <para>
-      When using range partitioning, a <literal>NOT NULL</literal> constraint
-      is added to each non-expression column in the partition key.
-     </para>
-
     </listitem>
    </varlistentry>
 

src/backend/catalog/partition.c

@@ -1449,17 +1449,18 @@ get_range_key_properties(PartitionKey key, int keynum,
  * as the lower bound tuple and (au, bu, cu) as the upper bound tuple, we
  * generate an expression tree of the following form:
  *
+ *  (a IS NOT NULL) and (b IS NOT NULL) and (c IS NOT NULL)
+ *		AND
  *	(a > al OR (a = al AND b > bl) OR (a = al AND b = bl AND c >= cl))
  *		AND
  *	(a < au OR (a = au AND b < bu) OR (a = au AND b = bu AND c < cu))
  *
- * If, say, b were an expression key instead of a simple column, we also
- * append (b IS NOT NULL) to the AND's argument list.
- *
  * It is often the case that a prefix of lower and upper bound tuples contains
  * the same values, for example, (al = au), in which case, we will emit an
  * expression tree of the following form:
  *
+ *  (a IS NOT NULL) and (b IS NOT NULL) and (c IS NOT NULL)
+ *		AND
  *	(a = al)
  *		AND
  *	(b > bl OR (b = bl AND c >= cl))
@@ -1472,11 +1473,11 @@ get_range_key_properties(PartitionKey key, int keynum,
  *	(b < bu) OR (b = bu), which is simplified to (b <= bu)
  *
  * In most common cases with only one partition column, say a, the following
- * expression tree will be generated: a >= al AND a < au
+ * expression tree will be generated: a IS NOT NULL AND a >= al AND a < au
  *
  * If all values of both lower and upper bounds are UNBOUNDED, the partition
  * does not really have a constraint, except the IS NOT NULL constraint for
- * any expression keys.
+ * partition keys.
  *
  * If we end up with an empty result list, we append return a single-member
  * list containing a constant-true expression in that case, because callers
@@ -1512,24 +1513,30 @@ get_qual_for_range(PartitionKey key, PartitionBoundSpec *spec)
 	num_or_arms = key->partnatts;
 
 	/*
-	 * A range-partitioned table does not allow partition keys to be null. For
-	 * simple columns, their NOT NULL constraint suffices for the enforcement
-	 * of non-nullability.  But for the expression keys, which are still
-	 * nullable, we must emit a IS NOT NULL expression.  Collect them in
-	 * result first.
+	 * A range-partitioned table does not currently allow partition keys to
+	 * be null, so emit an IS NOT NULL expression for each key column.
 	 */
 	partexprs_item = list_head(key->partexprs);
 	for (i = 0; i < key->partnatts; i++)
-	{
-		if (key->partattrs[i] == 0)
 	{
 		Expr	   *keyCol;
 
+		if (key->partattrs[i] != 0)
+		{
+			keyCol = (Expr *) makeVar(1,
+									  key->partattrs[i],
+									  key->parttypid[i],
+									  key->parttypmod[i],
+									  key->parttypcoll[i],
+									  0);
+		}
+		else
+		{
 			if (partexprs_item == NULL)
 				elog(ERROR, "wrong number of partition key expressions");
-			keyCol = lfirst(partexprs_item);
+			keyCol = copyObject(lfirst(partexprs_item));
 			partexprs_item = lnext(partexprs_item);
-			Assert(!IsA(keyCol, Var));
+		}
 
 		nulltest = makeNode(NullTest);
 		nulltest->arg = keyCol;
@@ -1538,7 +1545,6 @@ get_qual_for_range(PartitionKey key, PartitionBoundSpec *spec)
 		nulltest->location = -1;
 		result = lappend(result, nulltest);
 	}
-	}
 
 	/*
 	 * Iterate over the key columns and check if the corresponding lower and
@@ -1948,7 +1954,8 @@ get_partition_for_tuple(PartitionDispatch *pd,
 		{
 			*failed_at = parent;
 			*failed_slot = slot;
-			return -1;
+			result = -1;
+			goto error_exit;
 		}
 
 		/*
@@ -1964,12 +1971,21 @@ get_partition_for_tuple(PartitionDispatch *pd,
 
 		if (key->strategy == PARTITION_STRATEGY_RANGE)
 		{
-			/* Disallow nulls in the range partition key of the tuple */
+			/*
+			 * Since we cannot route tuples with NULL partition keys through
+			 * a range-partitioned table, simply return that no partition
+			 * exists
+			 */
 			for (i = 0; i < key->partnatts; i++)
+			{
 				if (isnull[i])
-					ereport(ERROR,
-							(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
-						errmsg("range partition key of row contains null")));
+				{
+					*failed_at = parent;
+					*failed_slot = slot;
+					result = -1;
+					goto error_exit;
+				}
+			}
 		}
 
 		/*
@@ -2032,6 +2048,7 @@ get_partition_for_tuple(PartitionDispatch *pd,
 			parent = pd[-parent->indexes[cur_index]];
 	}
 
+error_exit:
 	ecxt->ecxt_scantuple = ecxt_scantuple_old;
 	return result;
 }

src/backend/commands/tablecmds.c

@@ -805,13 +805,11 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 	if (stmt->partspec)
 	{
 		char		strategy;
-		int			partnatts,
-					i;
+		int			partnatts;
 		AttrNumber	partattrs[PARTITION_MAX_KEYS];
 		Oid			partopclass[PARTITION_MAX_KEYS];
 		Oid			partcollation[PARTITION_MAX_KEYS];
 		List	   *partexprs = NIL;
-		List	   *cmds = NIL;
 
 		/*
 		 * We need to transform the raw parsetrees corresponding to partition
@@ -828,33 +826,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 		partnatts = list_length(stmt->partspec->partParams);
 		StorePartitionKey(rel, strategy, partnatts, partattrs, partexprs,
 						  partopclass, partcollation);
-
-		/* Force key columns to be NOT NULL when using range partitioning */
-		if (strategy == PARTITION_STRATEGY_RANGE)
-		{
-			for (i = 0; i < partnatts; i++)
-			{
-				AttrNumber	partattno = partattrs[i];
-				Form_pg_attribute attform = descriptor->attrs[partattno - 1];
-
-				if (partattno != 0 && !attform->attnotnull)
-				{
-					/* Add a subcommand to make this one NOT NULL */
-					AlterTableCmd *cmd = makeNode(AlterTableCmd);
-
-					cmd->subtype = AT_SetNotNull;
-					cmd->name = pstrdup(NameStr(attform->attname));
-					cmds = lappend(cmds, cmd);
-				}
-			}
-
-			/*
-			 * Although, there cannot be any partitions yet, we still need to
-			 * pass true for recurse; ATPrepSetNotNull() complains if we don't
-			 */
-			if (cmds != NIL)
-				AlterTableInternal(RelationGetRelid(rel), cmds, true);
-		}
 	}
 
 	/*
@@ -5702,32 +5673,6 @@ ATExecDropNotNull(Relation rel, const char *colName, LOCKMODE lockmode)
 		heap_close(parent, AccessShareLock);
 	}
 
-	/*
-	 * If the table is a range partitioned table, check that the column is not
-	 * in the partition key.
-	 */
-	if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
-	{
-		PartitionKey key = RelationGetPartitionKey(rel);
-
-		if (get_partition_strategy(key) == PARTITION_STRATEGY_RANGE)
-		{
-			int			partnatts = get_partition_natts(key),
-						i;
-
-			for (i = 0; i < partnatts; i++)
-			{
-				AttrNumber	partattnum = get_partition_col_attnum(key, i);
-
-				if (partattnum == attnum)
-					ereport(ERROR,
-							(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
-							 errmsg("column \"%s\" is in range partition key",
-									colName)));
-			}
-		}
-	}
-
 	/*
 	 * Okay, actually perform the catalog change ... if needed
 	 */

src/test/regress/expected/alter_table.out

@@ -3079,13 +3079,6 @@ ALTER TABLE partitioned DROP COLUMN b;
 ERROR:  cannot drop column referenced in partition key expression
 ALTER TABLE partitioned ALTER COLUMN b TYPE char(5);
 ERROR:  cannot alter type of column referenced in partition key expression
--- cannot drop NOT NULL on columns in the range partition key
-ALTER TABLE partitioned ALTER COLUMN a DROP NOT NULL;
-ERROR:  column "a" is in range partition key
--- it's fine however to drop one on the list partition key column
-CREATE TABLE list_partitioned (a int not null) partition by list (a);
-ALTER TABLE list_partitioned ALTER a DROP NOT NULL;
-DROP TABLE list_partitioned;
 -- partitioned table cannot participate in regular inheritance
 CREATE TABLE nonpartitioned (
 	a int,

src/test/regress/expected/create_table.out

@@ -407,18 +407,6 @@ SELECT relkind FROM pg_class WHERE relname = 'partitioned';
  p
 (1 row)
 
--- check that range partition key columns are marked NOT NULL
-SELECT attname, attnotnull FROM pg_attribute
-  WHERE attrelid = 'partitioned'::regclass AND attnum > 0
-  ORDER BY attnum;
- attname | attnotnull 
----------+------------
- a       | t
- b       | f
- c       | t
- d       | t
-(4 rows)
-
 -- prevent a function referenced in partition key from being dropped
 DROP FUNCTION plusone(int);
 ERROR:  cannot drop function plusone(integer) because other objects depend on it
@@ -435,10 +423,10 @@ ERROR:  cannot inherit from partitioned table "partitioned2"
             Table "public.partitioned"
  Column |  Type   | Collation | Nullable | Default 
 --------+---------+-----------+----------+---------
- a      | integer |           | not null | 
+ a      | integer |           |          | 
  b      | integer |           |          | 
- c      | text    |           | not null | 
- d      | text    |           | not null | 
+ c      | text    |           |          | 
+ d      | text    |           |          | 
 Partition key: RANGE (a oid_ops, plusone(b), c, d COLLATE "C")
 
 \d partitioned2
@@ -544,9 +532,9 @@ CREATE TABLE part_forced_oids PARTITION OF oids_parted FOR VALUES FROM (1) TO (1
                              Table "public.part_forced_oids"
  Column |  Type   | Collation | Nullable | Default | Storage | Stats target | Description 
 --------+---------+-----------+----------+---------+---------+--------------+-------------
- a      | integer |           | not null |         | plain   |              | 
+ a      | integer |           |          |         | plain   |              | 
 Partition of: oids_parted FOR VALUES FROM (1) TO (10)
-Partition constraint: ((a >= 1) AND (a < 10))
+Partition constraint: ((a IS NOT NULL) AND (a >= 1) AND (a < 10))
 Has OIDs: yes
 
 DROP TABLE oids_parted, part_forced_oids;
@@ -678,7 +666,7 @@ Partitions: part_c_1_10 FOR VALUES FROM (1) TO (10)
  a      | text    |           |          |         | extended |              | 
  b      | integer |           | not null | 0       | plain    |              | 
 Partition of: part_c FOR VALUES FROM (1) TO (10)
-Partition constraint: ((a IS NOT NULL) AND (a = ANY (ARRAY['c'::text])) AND (b >= 1) AND (b < 10))
+Partition constraint: ((a IS NOT NULL) AND (a = ANY (ARRAY['c'::text])) AND (b IS NOT NULL) AND (b >= 1) AND (b < 10))
 Check constraints:
     "check_a" CHECK (length(a) > 0)
 
@@ -706,9 +694,9 @@ CREATE TABLE unbounded_range_part PARTITION OF range_parted4 FOR VALUES FROM (UN
 --------+---------+-----------+----------+---------+---------+--------------+-------------
  a      | integer |           |          |         | plain   |              | 
  b      | integer |           |          |         | plain   |              | 
- c      | integer |           | not null |         | plain   |              | 
+ c      | integer |           |          |         | plain   |              | 
 Partition of: range_parted4 FOR VALUES FROM (UNBOUNDED, UNBOUNDED, UNBOUNDED) TO (UNBOUNDED, UNBOUNDED, UNBOUNDED)
-Partition constraint: ((abs(a) IS NOT NULL) AND (abs(b) IS NOT NULL))
+Partition constraint: ((abs(a) IS NOT NULL) AND (abs(b) IS NOT NULL) AND (c IS NOT NULL))
 
 DROP TABLE unbounded_range_part;
 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
 --------+---------+-----------+----------+---------+---------+--------------+-------------
  a      | integer |           |          |         | plain   |              | 
  b      | integer |           |          |         | plain   |              | 
- c      | integer |           | not null |         | plain   |              | 
+ c      | integer |           |          |         | plain   |              | 
 Partition of: range_parted4 FOR VALUES FROM (UNBOUNDED, UNBOUNDED, UNBOUNDED) TO (1, UNBOUNDED, UNBOUNDED)
-Partition constraint: ((abs(a) IS NOT NULL) AND (abs(b) IS NOT NULL) AND (abs(a) <= 1))
+Partition constraint: ((abs(a) IS NOT NULL) AND (abs(b) IS NOT NULL) AND (c IS NOT NULL) AND (abs(a) <= 1))
 
 CREATE TABLE range_parted4_2 PARTITION OF range_parted4 FOR VALUES FROM (3, 4, 5) TO (6, 7, UNBOUNDED);
 \d+ range_parted4_2
@@ -729,9 +717,9 @@ CREATE TABLE range_parted4_2 PARTITION OF range_parted4 FOR VALUES FROM (3, 4, 5
 --------+---------+-----------+----------+---------+---------+--------------+-------------
  a      | integer |           |          |         | plain   |              | 
  b      | integer |           |          |         | plain   |              | 
- c      | integer |           | not null |         | plain   |              | 
+ c      | integer |           |          |         | plain   |              | 
 Partition of: range_parted4 FOR VALUES FROM (3, 4, 5) TO (6, 7, UNBOUNDED)
-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))))
+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))))
 
 CREATE TABLE range_parted4_3 PARTITION OF range_parted4 FOR VALUES FROM (6, 8, UNBOUNDED) TO (9, UNBOUNDED, UNBOUNDED);
 \d+ range_parted4_3
@@ -740,9 +728,9 @@ CREATE TABLE range_parted4_3 PARTITION OF range_parted4 FOR VALUES FROM (6, 8, U
 --------+---------+-----------+----------+---------+---------+--------------+-------------
  a      | integer |           |          |         | plain   |              | 
  b      | integer |           |          |         | plain   |              | 
- c      | integer |           | not null |         | plain   |              | 
+ c      | integer |           |          |         | plain   |              | 
 Partition of: range_parted4 FOR VALUES FROM (6, 8, UNBOUNDED) TO (9, UNBOUNDED, UNBOUNDED)
-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))
+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))
 
 DROP TABLE range_parted4;
 -- cleanup

src/test/regress/expected/insert.out

@@ -189,7 +189,7 @@ DETAIL:  Failing row contains (a, 10).
 insert into part4 values ('b', 10);
 -- fail (partition key a has a NOT NULL constraint)
 insert into part1 values (null);
-ERROR:  null value in column "a" violates not-null constraint
+ERROR:  new row for relation "part1" violates partition constraint
 DETAIL:  Failing row contains (null, null).
 -- fail (expression key (b+0) cannot be null either)
 insert into part1 values (1);
@@ -247,7 +247,8 @@ insert into range_parted values ('b', 1);
 insert into range_parted values ('b', 10);
 -- fail (partition key (b+0) is null)
 insert into range_parted values ('a');
-ERROR:  range partition key of row contains null
+ERROR:  no partition of relation "range_parted" found for row
+DETAIL:  Partition key of the failing row contains (a, (b + 0)) = (a, null).
 select tableoid::regclass, * from range_parted;
  tableoid | a | b  
 ----------+---+----

src/test/regress/sql/alter_table.sql

@@ -1947,14 +1947,6 @@ ALTER TABLE partitioned ALTER COLUMN a TYPE char(5);
 ALTER TABLE partitioned DROP COLUMN b;
 ALTER TABLE partitioned ALTER COLUMN b TYPE char(5);
 
--- cannot drop NOT NULL on columns in the range partition key
-ALTER TABLE partitioned ALTER COLUMN a DROP NOT NULL;
-
--- it's fine however to drop one on the list partition key column
-CREATE TABLE list_partitioned (a int not null) partition by list (a);
-ALTER TABLE list_partitioned ALTER a DROP NOT NULL;
-DROP TABLE list_partitioned;
-
 -- partitioned table cannot participate in regular inheritance
 CREATE TABLE nonpartitioned (
 	a int,

src/test/regress/sql/create_table.sql

@@ -410,11 +410,6 @@ CREATE TABLE partitioned (
 -- check relkind
 SELECT relkind FROM pg_class WHERE relname = 'partitioned';
 
--- check that range partition key columns are marked NOT NULL
-SELECT attname, attnotnull FROM pg_attribute
-  WHERE attrelid = 'partitioned'::regclass AND attnum > 0
-  ORDER BY attnum;
-
 -- prevent a function referenced in partition key from being dropped
 DROP FUNCTION plusone(int);