Skip to content

Commit ef33cf7

Browse files
funbringerfunbringer
committed
clean regression tests up a little bit, check that expression references the table
1 parent e8afda6 commit ef33cf7

File tree

4 files changed

+78
-29
lines changed

4 files changed

+78
-29
lines changed

‎expected/pathman_calamity.out

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,7 @@ ERROR: 'expression' should not be NULL
306306
SELECT validate_expression('calamity.part_test', 'valval'); /* not ok */
307307
ERROR: cannot find type name for attribute "valval" of relation "part_test"
308308
SELECT validate_expression('calamity.part_test', 'random()'); /* not ok */
309-
ERROR: functions in partitioning expression must be marked IMMUTABLE
309+
ERROR: failed to analyze partitioning expression (random())
310310
SELECT validate_expression('calamity.part_test', 'val'); /* OK */
311311
validate_expression
312312
---------------------

‎expected/pathman_expressions.out

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,20 @@ SELECT COUNT(*) FROM test_exprs.hash_rel;
2727
5
2828
(1 row)
2929

30+
/* Try using constant expression */
31+
SELECT create_hash_partitions('test_exprs.hash_rel', '1 + 1', 4);
32+
ERROR: partitioning expression should reference table "hash_rel"
33+
\set VERBOSITY default
34+
/* Try using mutable expression */
3035
SELECT create_hash_partitions('test_exprs.hash_rel', 'random()', 4);
31-
ERROR: functions in partitioning expression must be marked IMMUTABLE
36+
ERROR: failed to analyze partitioning expression (random())
37+
DETAIL: functions in partitioning expression must be marked IMMUTABLE
38+
CONTEXT: SQL statement "SELECT public.validate_expression(parent_relid, expression)"
39+
PL/pgSQL function prepare_for_partitioning(regclass,text,boolean) line 9 at PERFORM
40+
SQL statement "SELECT public.prepare_for_partitioning(parent_relid,
41+
expression,
42+
partition_data)"
43+
PL/pgSQL function create_hash_partitions(regclass,text,integer,boolean,text[],text[]) line 4 at PERFORM
3244
/* Check that 'pathman_hooks_enabled' is true (1 partition in plan) */
3345
EXPLAIN (COSTS OFF) INSERT INTO test_exprs.canary_copy
3446
SELECT * FROM test_exprs.canary WHERE val = 1;
@@ -40,7 +52,7 @@ SELECT * FROM test_exprs.canary WHERE val = 1;
4052
Filter: (val = 1)
4153
(4 rows)
4254

43-
\set VERBOSITY default
55+
/* Try using missing columns */
4456
SELECT create_hash_partitions('test_exprs.hash_rel', 'value * value2))', 4);
4557
ERROR: failed to parse partitioning expression (value * value2)))
4658
DETAIL: syntax error at or near ")"
@@ -132,10 +144,36 @@ EXPLAIN (COSTS OFF) SELECT * FROM test_exprs.hash_rel WHERE (value * value2) = 5
132144
CREATE TABLE test_exprs.range_rel (id SERIAL PRIMARY KEY, dt TIMESTAMP, txt TEXT);
133145
INSERT INTO test_exprs.range_rel (dt, txt)
134146
SELECT g, md5(g::TEXT) FROM generate_series('2015-01-01', '2020-04-30', '1 month'::interval) as g;
135-
SELECT create_range_partitions('test_exprs.range_rel', 'RANDOM()', '15 years'::INTERVAL, '1 year'::INTERVAL, 10);
136-
ERROR: functions in partitioning expression must be marked IMMUTABLE
147+
/* Try using constant expression */
148+
SELECT create_range_partitions('test_exprs.range_rel', '''16 years''::interval',
149+
'15 years'::INTERVAL, '1 year'::INTERVAL, 10);
150+
ERROR: partitioning expression should reference table "range_rel"
151+
\set VERBOSITY default
152+
/* Try using mutable expression */
153+
SELECT create_range_partitions('test_exprs.range_rel', 'RANDOM()',
154+
'15 years'::INTERVAL, '1 year'::INTERVAL, 10);
155+
ERROR: failed to analyze partitioning expression (RANDOM())
156+
DETAIL: functions in partitioning expression must be marked IMMUTABLE
157+
CONTEXT: SQL statement "SELECT public.validate_expression(parent_relid, expression)"
158+
PL/pgSQL function prepare_for_partitioning(regclass,text,boolean) line 9 at PERFORM
159+
SQL statement "SELECT public.prepare_for_partitioning(parent_relid,
160+
expression,
161+
partition_data)"
162+
PL/pgSQL function create_range_partitions(regclass,text,anyelement,interval,integer,boolean) line 12 at PERFORM
163+
/* Check that 'pathman_hooks_enabled' is true (1 partition in plan) */
164+
EXPLAIN (COSTS OFF) INSERT INTO test_exprs.canary_copy
165+
SELECT * FROM test_exprs.canary WHERE val = 1;
166+
QUERY PLAN
167+
----------------------------------
168+
Insert on canary_copy
169+
-> Append
170+
-> Seq Scan on canary_0
171+
Filter: (val = 1)
172+
(4 rows)
173+
174+
\set VERBOSITY terse
137175
SELECT create_range_partitions('test_exprs.range_rel', 'AGE(dt, ''2000-01-01''::DATE)',
138-
'15 years'::INTERVAL, '1 year'::INTERVAL, 10);
176+
'15 years'::INTERVAL, '1 year'::INTERVAL, 10);
139177
create_range_partitions
140178
-------------------------
141179
10

‎sql/pathman_expressions.sql

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,22 @@ INSERT INTO test_exprs.hash_rel (value, value2)
2525
SELECT val, val * 2 FROM generate_series(1, 5) val;
2626

2727
SELECT COUNT(*) FROM test_exprs.hash_rel;
28+
29+
30+
/* Try using constant expression */
31+
SELECT create_hash_partitions('test_exprs.hash_rel', '1 + 1', 4);
32+
33+
34+
\set VERBOSITY default
35+
36+
/* Try using mutable expression */
2837
SELECT create_hash_partitions('test_exprs.hash_rel', 'random()', 4);
2938

3039
/* Check that 'pathman_hooks_enabled' is true (1 partition in plan) */
3140
EXPLAIN (COSTS OFF) INSERT INTO test_exprs.canary_copy
3241
SELECT * FROM test_exprs.canary WHERE val = 1;
3342

34-
35-
\set VERBOSITY default
36-
43+
/* Try using missing columns */
3744
SELECT create_hash_partitions('test_exprs.hash_rel', 'value * value2))', 4);
3845
SELECT create_hash_partitions('test_exprs.hash_rel', 'value * value3', 4);
3946

@@ -66,9 +73,28 @@ CREATE TABLE test_exprs.range_rel (id SERIAL PRIMARY KEY, dt TIMESTAMP, txt TEXT
6673

6774
INSERT INTO test_exprs.range_rel (dt, txt)
6875
SELECT g, md5(g::TEXT) FROM generate_series('2015-01-01', '2020-04-30', '1 month'::interval) as g;
69-
SELECT create_range_partitions('test_exprs.range_rel', 'RANDOM()', '15 years'::INTERVAL, '1 year'::INTERVAL, 10);
76+
77+
78+
/* Try using constant expression */
79+
SELECT create_range_partitions('test_exprs.range_rel', '''16 years''::interval',
80+
'15 years'::INTERVAL, '1 year'::INTERVAL, 10);
81+
82+
83+
\set VERBOSITY default
84+
85+
/* Try using mutable expression */
86+
SELECT create_range_partitions('test_exprs.range_rel', 'RANDOM()',
87+
'15 years'::INTERVAL, '1 year'::INTERVAL, 10);
88+
89+
/* Check that 'pathman_hooks_enabled' is true (1 partition in plan) */
90+
EXPLAIN (COSTS OFF) INSERT INTO test_exprs.canary_copy
91+
SELECT * FROM test_exprs.canary WHERE val = 1;
92+
93+
\set VERBOSITY terse
94+
95+
7096
SELECT create_range_partitions('test_exprs.range_rel', 'AGE(dt, ''2000-01-01''::DATE)',
71-
'15 years'::INTERVAL, '1 year'::INTERVAL, 10);
97+
'15 years'::INTERVAL, '1 year'::INTERVAL, 10);
7298
INSERT INTO test_exprs.range_rel_1 (dt, txt) VALUES ('2020-01-01'::DATE, md5('asdf'));
7399
SELECT COUNT(*) FROM test_exprs.range_rel_6;
74100
INSERT INTO test_exprs.range_rel_6 (dt, txt) VALUES ('2020-01-01'::DATE, md5('asdf'));

‎src/relation_info.c

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -183,8 +183,9 @@ refresh_pathman_relation_info(Oid relid,
183183
fix_opfuncids(prel->expr);
184184

185185
expr_varnos = pull_varnos(prel->expr);
186-
if (bms_singleton_member(expr_varnos) != PART_EXPR_VARNO)
187-
elog(ERROR, "partitioning expression may reference only one table");
186+
if (bms_num_members(expr_varnos) != 1)
187+
elog(ERROR, "partitioning expression should reference table \"%s\"",
188+
get_rel_name(relid));
188189

189190
/* Extract Vars and varattnos of partitioning expression */
190191
prel->expr_vars = NIL;
@@ -631,10 +632,6 @@ cook_partitioning_expression(const Oid relid,
631632
*raw_expr;
632633
List *query_tree_list;
633634

634-
volatile bool ok_rewrite = false; /* must be volatile, else
635-
cpu register might change
636-
in case of longjmp() */
637-
638635
char *query_string,
639636
*expr_serialized = ""; /* keep compiler happy */
640637

@@ -645,9 +642,6 @@ cook_partitioning_expression(const Oid relid,
645642

646643
AssertTemporaryContext();
647644

648-
/* Make clang analyzer happy (v 3.5) */
649-
(void) ok_rewrite;
650-
651645
/*
652646
* We use separate memory context here, just to make sure we won't
653647
* leave anything behind after parsing, rewriting and planning.
@@ -705,7 +699,6 @@ cook_partitioning_expression(const Oid relid,
705699

706700
/* This will fail with ERROR in case of wrong expression */
707701
query_tree_list = pg_analyze_and_rewrite(parse_tree, query_string, NULL, 0);
708-
ok_rewrite = true; /* tell PG_CATCH that we're fine */
709702

710703
if (list_length(query_tree_list) != 1)
711704
elog(ERROR, "partitioning expression produced more than 1 query");
@@ -740,14 +733,6 @@ cook_partitioning_expression(const Oid relid,
740733
/* Don't forget to enable pg_pathman's hooks */
741734
pathman_hooks_enabled = true;
742735

743-
/*
744-
* Simply rethrow if rewrite of AST was successful.
745-
* NOTE: We aim to modify only ERRORs that
746-
* are relevant to analyze and rewrite steps.
747-
*/
748-
if (ok_rewrite)
749-
PG_RE_THROW();
750-
751736
/* Switch to the original context & copy edata */
752737
MemoryContextSwitchTo(old_mcxt);
753738
error = CopyErrorData();

0 commit comments

Comments
 (0)