Skip to content

Commit d03fd32

Browse files
funbringerfunbringer
committed
check that expression's columns are NOT NULL
1 parent d765b14 commit d03fd32

File tree

6 files changed

+75
-46
lines changed

6 files changed

+75
-46
lines changed

‎expected/pathman_basic.out‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,17 @@ CREATE TABLE test.hash_rel (
99
INSERT INTO test.hash_rel VALUES (1, 1);
1010
INSERT INTO test.hash_rel VALUES (2, 2);
1111
INSERT INTO test.hash_rel VALUES (3, 3);
12+
\set VERBOSITY default
1213
SELECT pathman.create_hash_partitions('test.hash_rel', 'value', 3);
13-
ERROR: partitioning key "value" must be marked NOT NULL
14+
ERROR: failed to analyze partitioning expression "value"
15+
DETAIL: column "value" should be marked NOT NULL
16+
CONTEXT: SQL statement "SELECT pathman.validate_expression(parent_relid, expression)"
17+
PL/pgSQL function pathman.prepare_for_partitioning(regclass,text,boolean) line 9 at PERFORM
18+
SQL statement "SELECT pathman.prepare_for_partitioning(parent_relid,
19+
expression,
20+
partition_data)"
21+
PL/pgSQL function pathman.create_hash_partitions(regclass,text,integer,boolean,text[],text[]) line 4 at PERFORM
22+
\set VERBOSITY terse
1423
ALTER TABLE test.hash_rel ALTER COLUMN value SET NOT NULL;
1524
SELECT pathman.create_hash_partitions('test.hash_rel', 'value', 3, partition_data:=false);
1625
create_hash_partitions
@@ -129,8 +138,17 @@ CREATE TABLE test.range_rel (
129138
CREATE INDEX ON test.range_rel (dt);
130139
INSERT INTO test.range_rel (dt, txt)
131140
SELECT g, md5(g::TEXT) FROM generate_series('2015-01-01', '2015-04-30', '1 day'::interval) as g;
141+
\set VERBOSITY default
132142
SELECT pathman.create_range_partitions('test.range_rel', 'dt', '2015-01-01'::DATE, '1 month'::INTERVAL);
133-
ERROR: partitioning key "dt" must be marked NOT NULL
143+
ERROR: failed to analyze partitioning expression "dt"
144+
DETAIL: column "dt" should be marked NOT NULL
145+
CONTEXT: SQL statement "SELECT pathman.validate_expression(parent_relid, expression)"
146+
PL/pgSQL function pathman.prepare_for_partitioning(regclass,text,boolean) line 9 at PERFORM
147+
SQL statement "SELECT pathman.prepare_for_partitioning(parent_relid,
148+
expression,
149+
partition_data)"
150+
PL/pgSQL function pathman.create_range_partitions(regclass,text,anyelement,interval,integer,boolean) line 12 at PERFORM
151+
\set VERBOSITY terse
134152
ALTER TABLE test.range_rel ALTER COLUMN dt SET NOT NULL;
135153
SELECT pathman.create_range_partitions('test.range_rel', 'dt', '2015-01-01'::DATE, '1 month'::INTERVAL, 2);
136154
ERROR: not enough partitions to fit all values of "dt"

‎expected/pathman_calamity.out‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -280,11 +280,11 @@ ERROR: 'parttype' should not be NULL
280280
SELECT validate_interval_value('pg_class', 'oid', 1, 'HASH', NULL); /* not ok */
281281
ERROR: interval should be NULL for HASH partitioned table
282282
SELECT validate_interval_value('pg_class', 'expr', 2, '1 mon', NULL); /* not ok */
283-
ERROR: cannot find type name for attribute "expr" of relation "pg_class"
283+
ERROR: failed to analyze partitioning expression "expr"
284284
SELECT validate_interval_value('pg_class', 'expr', 2, NULL, 'cooked_expr'); /* not ok */
285285
ERROR: unrecognized token: "cooked_expr"
286286
SELECT validate_interval_value('pg_class', 'EXPR', 1, 'HASH', NULL); /* not ok */
287-
ERROR: cannot find type name for attribute "expr" of relation "pg_class"
287+
ERROR: failed to analyze partitioning expression "EXPR"
288288
/* check function validate_relname() */
289289
SELECT validate_relname('calamity.part_test');
290290
validate_relname
@@ -304,7 +304,7 @@ ERROR: 'relid' should not be NULL
304304
SELECT validate_expression('calamity.part_test', NULL); /* not ok */
305305
ERROR: 'expression' should not be NULL
306306
SELECT validate_expression('calamity.part_test', 'valval'); /* not ok */
307-
ERROR: cannot find type name for attribute "valval" of relation "part_test"
307+
ERROR: failed to analyze partitioning expression "valval"
308308
SELECT validate_expression('calamity.part_test', 'random()'); /* not ok */
309309
ERROR: failed to analyze partitioning expression "random()"
310310
SELECT validate_expression('calamity.part_test', 'val'); /* OK */
@@ -580,7 +580,7 @@ ERROR: relation "0" does not exist
580580
SELECT add_to_pathman_config('calamity.part_test', NULL); /* no expr */
581581
ERROR: 'expression' should not be NULL
582582
SELECT add_to_pathman_config('calamity.part_test', 'V_A_L'); /* wrong expr */
583-
ERROR: cannot find type name for attribute "v_a_l" of relation "part_test"
583+
ERROR: failed to analyze partitioning expression "V_A_L"
584584
SELECT add_to_pathman_config('calamity.part_test', 'val'); /* OK */
585585
add_to_pathman_config
586586
-----------------------

‎expected/pathman_expressions.out‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,8 @@ SELECT create_hash_partitions('test_exprs.canary', 'val', 5);
109109
*/
110110
CREATE TABLE test_exprs.hash_rel (
111111
id SERIAL PRIMARY KEY,
112-
value INTEGER,
113-
value2 INTEGER
112+
value INTEGER NOT NULL,
113+
value2 INTEGER NOT NULL
114114
);
115115
INSERT INTO test_exprs.hash_rel (value, value2)
116116
SELECT val, val * 2 FROM generate_series(1, 5) val;
@@ -243,7 +243,7 @@ EXPLAIN (COSTS OFF) SELECT * FROM test_exprs.hash_rel WHERE (value * value2) = 5
243243
/*
244244
* Test RANGE
245245
*/
246-
CREATE TABLE test_exprs.range_rel (id SERIAL PRIMARY KEY, dt TIMESTAMP, txt TEXT);
246+
CREATE TABLE test_exprs.range_rel (id SERIAL PRIMARY KEY, dt TIMESTAMP NOT NULL, txt TEXT);
247247
INSERT INTO test_exprs.range_rel (dt, txt)
248248
SELECT g, md5(g::TEXT) FROM generate_series('2015-01-01', '2020-04-30', '1 month'::interval) as g;
249249
\set VERBOSITY default

‎sql/pathman_basic.sql‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,13 @@ CREATE TABLE test.hash_rel (
1111
INSERT INTO test.hash_rel VALUES (1, 1);
1212
INSERT INTO test.hash_rel VALUES (2, 2);
1313
INSERT INTO test.hash_rel VALUES (3, 3);
14+
15+
\set VERBOSITY default
1416
SELECT pathman.create_hash_partitions('test.hash_rel', 'value', 3);
17+
\set VERBOSITY terse
18+
1519
ALTER TABLE test.hash_rel ALTER COLUMN value SET NOT NULL;
20+
1621
SELECT pathman.create_hash_partitions('test.hash_rel', 'value', 3, partition_data:=false);
1722
EXPLAIN (COSTS OFF) SELECT * FROM test.hash_rel;
1823
SELECT * FROM test.hash_rel;
@@ -39,8 +44,13 @@ CREATE TABLE test.range_rel (
3944
CREATE INDEX ON test.range_rel (dt);
4045
INSERT INTO test.range_rel (dt, txt)
4146
SELECT g, md5(g::TEXT) FROM generate_series('2015-01-01', '2015-04-30', '1 day'::interval) as g;
47+
48+
\set VERBOSITY default
4249
SELECT pathman.create_range_partitions('test.range_rel', 'dt', '2015-01-01'::DATE, '1 month'::INTERVAL);
50+
\set VERBOSITY terse
51+
4352
ALTER TABLE test.range_rel ALTER COLUMN dt SET NOT NULL;
53+
4454
SELECT pathman.create_range_partitions('test.range_rel', 'dt', '2015-01-01'::DATE, '1 month'::INTERVAL, 2);
4555
SELECT pathman.create_range_partitions('test.range_rel', 'DT', '2015-01-01'::DATE, '1 month'::INTERVAL);
4656
SELECT COUNT(*) FROM test.range_rel;

‎sql/pathman_expressions.sql‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ SELECT create_hash_partitions('test_exprs.canary', 'val', 5);
4747

4848
CREATE TABLE test_exprs.hash_rel (
4949
id SERIAL PRIMARY KEY,
50-
value INTEGER,
51-
value2 INTEGER
50+
value INTEGER NOT NULL,
51+
value2 INTEGER NOT NULL
5252
);
5353
INSERT INTO test_exprs.hash_rel (value, value2)
5454
SELECT val, val * 2 FROM generate_series(1, 5) val;
@@ -101,7 +101,7 @@ EXPLAIN (COSTS OFF) SELECT * FROM test_exprs.hash_rel WHERE (value * value2) = 5
101101
* Test RANGE
102102
*/
103103

104-
CREATE TABLE test_exprs.range_rel (id SERIAL PRIMARY KEY, dt TIMESTAMP, txt TEXT);
104+
CREATE TABLE test_exprs.range_rel (id SERIAL PRIMARY KEY, dt TIMESTAMP NOT NULL, txt TEXT);
105105

106106
INSERT INTO test_exprs.range_rel (dt, txt)
107107
SELECT g, md5(g::TEXT) FROM generate_series('2015-01-01', '2020-04-30', '1 month'::interval) as g;

‎src/relation_info.c‎

Lines changed: 35 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -632,8 +632,7 @@ cook_partitioning_expression(const Oid relid,
632632
const char *expr_cstr,
633633
Oid *expr_type_out) /* ret value #1 */
634634
{
635-
Node *parse_tree,
636-
*raw_expr;
635+
Node *parse_tree;
637636
List *query_tree_list;
638637

639638
char *query_string,
@@ -658,37 +657,8 @@ cook_partitioning_expression(const Oid relid,
658657
old_mcxt = MemoryContextSwitchTo(parse_mcxt);
659658

660659
/* First we have to build a raw AST */
661-
raw_expr = parse_partitioning_expression(relid, expr_cstr,
662-
&query_string, &parse_tree);
663-
664-
/* Check if raw_expr is NULLable */
665-
if (IsA(raw_expr, ColumnRef))
666-
{
667-
ColumnRef *column = (ColumnRef *) raw_expr;
668-
669-
if (list_length(column->fields) == 1)
670-
{
671-
HeapTuple htup;
672-
bool attnotnull;
673-
char *attname = strVal(linitial(column->fields));
674-
675-
/* check if attribute is nullable */
676-
htup = SearchSysCacheAttName(relid, attname);
677-
if (HeapTupleIsValid(htup))
678-
{
679-
Form_pg_attribute att_tup = (Form_pg_attribute) GETSTRUCT(htup);
680-
attnotnull = att_tup->attnotnull;
681-
ReleaseSysCache(htup);
682-
}
683-
else elog(ERROR, "cannot find type name for attribute \"%s\""
684-
" of relation \"%s\"",
685-
attname, get_rel_name_or_relid(relid));
686-
687-
if (!attnotnull)
688-
elog(ERROR, "partitioning key \"%s\" must be marked NOT NULL",
689-
attname);
690-
}
691-
}
660+
(void) parse_partitioning_expression(relid, expr_cstr,
661+
&query_string, &parse_tree);
692662

693663
/* We don't need pg_pathman's magic here */
694664
pathman_hooks_enabled = false;
@@ -697,7 +667,9 @@ cook_partitioning_expression(const Oid relid,
697667
{
698668
Query *query;
699669
Node *expr;
670+
int expr_attr;
700671
Relids expr_varnos;
672+
Bitmapset *expr_varattnos = NULL;
701673

702674
/* This will fail with ERROR in case of wrong expression */
703675
query_tree_list = pg_analyze_and_rewrite(parse_tree, query_string, NULL, 0);
@@ -729,12 +701,41 @@ cook_partitioning_expression(const Oid relid,
729701
/* Sanity check #5 */
730702
expr_varnos = pull_varnos(expr);
731703
if (bms_num_members(expr_varnos) != 1 ||
732-
((RangeTblEntry *) linitial(query->rtable))->relid != relid)
704+
relid != ((RangeTblEntry *) linitial(query->rtable))->relid)
733705
{
734706
elog(ERROR, "partitioning expression should reference table \"%s\"",
735707
get_rel_name(relid));
736708
}
709+
710+
/* Sanity check #6 */
711+
pull_varattnos(expr, bms_singleton_member(expr_varnos), &expr_varattnos);
712+
expr_attr = -1;
713+
while ((expr_attr = bms_next_member(expr_varattnos, expr_attr)) >= 0)
714+
{
715+
AttrNumber attnum = expr_attr + FirstLowInvalidHeapAttributeNumber;
716+
HeapTuple htup;
717+
718+
htup = SearchSysCache2(ATTNUM,
719+
ObjectIdGetDatum(relid),
720+
Int16GetDatum(attnum));
721+
if (HeapTupleIsValid(htup))
722+
{
723+
bool nullable;
724+
725+
/* Fetch 'nullable' and free syscache tuple */
726+
nullable = !((Form_pg_attribute) GETSTRUCT(htup))->attnotnull;
727+
ReleaseSysCache(htup);
728+
729+
if (nullable)
730+
ereport(ERROR, (errcode(ERRCODE_NOT_NULL_VIOLATION),
731+
errmsg("column \"%s\" should be marked NOT NULL",
732+
get_attname(relid, attnum))));
733+
}
734+
}
735+
736+
/* Free sets */
737737
bms_free(expr_varnos);
738+
bms_free(expr_varattnos);
738739

739740
Assert(expr);
740741
expr_serialized = nodeToString(expr);

0 commit comments

Comments
 (0)