Skip to content

Commit 02ff474

Browse files
funbringerfunbringer
committed
improved function cook_partitioning_expression()
1 parent ba12800 commit 02ff474

File tree

7 files changed

+108
-76
lines changed

7 files changed

+108
-76
lines changed

‎expected/pathman_basic.out‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1511,9 +1511,9 @@ SELECT pathman.create_partitions_from_range('test."RangeRel"', 'dt', '2015-01-01
15111511
DROP TABLE test."RangeRel" CASCADE;
15121512
NOTICE: drop cascades to 6 other objects
15131513
SELECT * FROM pathman.pathman_config;
1514-
partrel | expr | parttype | range_interval | cooked_expr
1515-
--------------------+------+----------+----------------+------------------------------------------------------------------------------------------------------------------------
1516-
test.num_range_rel | id | 2 | 1000 | {VAR :varno 1 :varattno 1 :vartype 23 :vartypmod -1 :varcollid 0 :varlevelsup 0 :varnoold 1 :varoattno 1 :location -1}
1514+
partrel | expr | parttype | range_interval | cooked_expr
1515+
--------------------+------+----------+----------------+-----------------------------------------------------------------------------------------------------------------------
1516+
test.num_range_rel | id | 2 | 1000 | {VAR :varno 1 :varattno 1 :vartype 23 :vartypmod -1 :varcollid 0 :varlevelsup 0 :varnoold 1 :varoattno 1 :location 8}
15171517
(1 row)
15181518

15191519
CREATE TABLE test."RangeRel" (

‎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: failed to analyze partitioning expression (random())
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: 36 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -27,43 +27,52 @@ SELECT COUNT(*) FROM test_exprs.hash_rel;
2727
5
2828
(1 row)
2929

30+
\set VERBOSITY default
3031
/* Try using constant expression */
3132
SELECT create_hash_partitions('test_exprs.hash_rel', '1 + 1', 4);
32-
ERROR: partitioning expression should reference table "hash_rel"
33-
\set VERBOSITY default
33+
ERROR: failed to analyze partitioning expression "1 + 1"
34+
DETAIL: partitioning expression should reference table "hash_rel"
35+
CONTEXT: SQL statement "SELECT public.validate_expression(parent_relid, expression)"
36+
PL/pgSQL function prepare_for_partitioning(regclass,text,boolean) line 9 at PERFORM
37+
SQL statement "SELECT public.prepare_for_partitioning(parent_relid,
38+
expression,
39+
partition_data)"
40+
PL/pgSQL function create_hash_partitions(regclass,text,integer,boolean,text[],text[]) line 4 at PERFORM
41+
/* Try using multiple queries */
42+
SELECT create_hash_partitions('test_exprs.hash_rel',
43+
'value, (select oid from pg_class limit 1)',
44+
4);
45+
ERROR: failed to analyze partitioning expression "value, (select oid from pg_class limit 1)"
46+
DETAIL: subqueries are not allowed in partitioning expression
47+
CONTEXT: SQL statement "SELECT public.validate_expression(parent_relid, expression)"
48+
PL/pgSQL function prepare_for_partitioning(regclass,text,boolean) line 9 at PERFORM
49+
SQL statement "SELECT public.prepare_for_partitioning(parent_relid,
50+
expression,
51+
partition_data)"
52+
PL/pgSQL function create_hash_partitions(regclass,text,integer,boolean,text[],text[]) line 4 at PERFORM
3453
/* Try using mutable expression */
3554
SELECT create_hash_partitions('test_exprs.hash_rel', 'random()', 4);
36-
ERROR: failed to analyze partitioning expression (random())
55+
ERROR: failed to analyze partitioning expression "random()"
3756
DETAIL: functions in partitioning expression must be marked IMMUTABLE
3857
CONTEXT: SQL statement "SELECT public.validate_expression(parent_relid, expression)"
3958
PL/pgSQL function prepare_for_partitioning(regclass,text,boolean) line 9 at PERFORM
4059
SQL statement "SELECT public.prepare_for_partitioning(parent_relid,
4160
expression,
4261
partition_data)"
4362
PL/pgSQL function create_hash_partitions(regclass,text,integer,boolean,text[],text[]) line 4 at PERFORM
44-
/* Check that 'pathman_hooks_enabled' is true (1 partition in plan) */
45-
EXPLAIN (COSTS OFF) INSERT INTO test_exprs.canary_copy
46-
SELECT * FROM test_exprs.canary WHERE val = 1;
47-
QUERY PLAN
48-
----------------------------------
49-
Insert on canary_copy
50-
-> Append
51-
-> Seq Scan on canary_0
52-
Filter: (val = 1)
53-
(4 rows)
54-
55-
/* Try using missing columns */
63+
/* Try using broken parentheses */
5664
SELECT create_hash_partitions('test_exprs.hash_rel', 'value * value2))', 4);
57-
ERROR: failed to parse partitioning expression (value * value2)))
65+
ERROR: failed to parse partitioning expression "value * value2))"
5866
DETAIL: syntax error at or near ")"
5967
QUERY: SELECT public.validate_expression(parent_relid, expression)
6068
CONTEXT: PL/pgSQL function prepare_for_partitioning(regclass,text,boolean) line 9 at PERFORM
6169
SQL statement "SELECT public.prepare_for_partitioning(parent_relid,
6270
expression,
6371
partition_data)"
6472
PL/pgSQL function create_hash_partitions(regclass,text,integer,boolean,text[],text[]) line 4 at PERFORM
73+
/* Try using missing columns */
6574
SELECT create_hash_partitions('test_exprs.hash_rel', 'value * value3', 4);
66-
ERROR: failed to analyze partitioning expression (value * value3)
75+
ERROR: failed to analyze partitioning expression "value * value3"
6776
DETAIL: column "value3" does not exist
6877
HINT: Perhaps you meant to reference the column "hash_rel.value" or the column "hash_rel.value2".
6978
QUERY: SELECT public.validate_expression(parent_relid, expression)
@@ -144,15 +153,22 @@ EXPLAIN (COSTS OFF) SELECT * FROM test_exprs.hash_rel WHERE (value * value2) = 5
144153
CREATE TABLE test_exprs.range_rel (id SERIAL PRIMARY KEY, dt TIMESTAMP, txt TEXT);
145154
INSERT INTO test_exprs.range_rel (dt, txt)
146155
SELECT g, md5(g::TEXT) FROM generate_series('2015-01-01', '2020-04-30', '1 month'::interval) as g;
156+
\set VERBOSITY default
147157
/* Try using constant expression */
148158
SELECT create_range_partitions('test_exprs.range_rel', '''16 years''::interval',
149159
'15 years'::INTERVAL, '1 year'::INTERVAL, 10);
150-
ERROR: partitioning expression should reference table "range_rel"
151-
\set VERBOSITY default
160+
ERROR: failed to analyze partitioning expression "'16 years'::interval"
161+
DETAIL: partitioning expression should reference table "range_rel"
162+
CONTEXT: SQL statement "SELECT public.validate_expression(parent_relid, expression)"
163+
PL/pgSQL function prepare_for_partitioning(regclass,text,boolean) line 9 at PERFORM
164+
SQL statement "SELECT public.prepare_for_partitioning(parent_relid,
165+
expression,
166+
partition_data)"
167+
PL/pgSQL function create_range_partitions(regclass,text,anyelement,interval,integer,boolean) line 12 at PERFORM
152168
/* Try using mutable expression */
153169
SELECT create_range_partitions('test_exprs.range_rel', 'RANDOM()',
154170
'15 years'::INTERVAL, '1 year'::INTERVAL, 10);
155-
ERROR: failed to analyze partitioning expression (RANDOM())
171+
ERROR: failed to analyze partitioning expression "RANDOM()"
156172
DETAIL: functions in partitioning expression must be marked IMMUTABLE
157173
CONTEXT: SQL statement "SELECT public.validate_expression(parent_relid, expression)"
158174
PL/pgSQL function prepare_for_partitioning(regclass,text,boolean) line 9 at PERFORM

‎sql/pathman_expressions.sql‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,21 +27,24 @@ INSERT INTO test_exprs.hash_rel (value, value2)
2727
SELECT COUNT(*) FROM test_exprs.hash_rel;
2828

2929

30+
31+
\set VERBOSITY default
32+
3033
/* Try using constant expression */
3134
SELECT create_hash_partitions('test_exprs.hash_rel', '1 + 1', 4);
3235

33-
34-
\set VERBOSITY default
36+
/* Try using multiple queries */
37+
SELECT create_hash_partitions('test_exprs.hash_rel',
38+
'value, (select oid from pg_class limit 1)',
39+
4);
3540

3641
/* Try using mutable expression */
3742
SELECT create_hash_partitions('test_exprs.hash_rel', 'random()', 4);
3843

39-
/* Check that 'pathman_hooks_enabled' is true (1 partition in plan) */
40-
EXPLAIN (COSTS OFF) INSERT INTO test_exprs.canary_copy
41-
SELECT * FROM test_exprs.canary WHERE val = 1;
44+
/* Try using broken parentheses */
45+
SELECT create_hash_partitions('test_exprs.hash_rel', 'value * value2))', 4);
4246

4347
/* Try using missing columns */
44-
SELECT create_hash_partitions('test_exprs.hash_rel', 'value * value2))', 4);
4548
SELECT create_hash_partitions('test_exprs.hash_rel', 'value * value3', 4);
4649

4750
/* Check that 'pathman_hooks_enabled' is true (1 partition in plan) */
@@ -75,13 +78,13 @@ INSERT INTO test_exprs.range_rel (dt, txt)
7578
SELECT g, md5(g::TEXT) FROM generate_series('2015-01-01', '2020-04-30', '1 month'::interval) as g;
7679

7780

81+
82+
\set VERBOSITY default
83+
7884
/* Try using constant expression */
7985
SELECT create_range_partitions('test_exprs.range_rel', '''16 years''::interval',
8086
'15 years'::INTERVAL, '1 year'::INTERVAL, 10);
8187

82-
83-
\set VERBOSITY default
84-
8588
/* Try using mutable expression */
8689
SELECT create_range_partitions('test_exprs.range_rel', 'RANDOM()',
8790
'15 years'::INTERVAL, '1 year'::INTERVAL, 10);

‎src/hooks.c‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,6 @@ pathman_join_pathlist_hook(PlannerInfo *root,
9191
set_join_pathlist_next(root, joinrel, outerrel,
9292
innerrel, jointype, extra);
9393

94-
/* Hooks can be disabled */
95-
if (!pathman_hooks_enabled)
96-
return;
97-
9894
/* Check that both pg_pathman & RuntimeAppend nodes are enabled */
9995
if (!IsPathmanReady() || !pg_pathman_enable_runtimeappend)
10096
return;
@@ -289,10 +285,6 @@ pathman_rel_pathlist_hook(PlannerInfo *root,
289285
if (set_rel_pathlist_hook_next != NULL)
290286
set_rel_pathlist_hook_next(root, rel, rti, rte);
291287

292-
/* Hooks can be disabled */
293-
if (!pathman_hooks_enabled)
294-
return;
295-
296288
/* Make sure that pg_pathman is ready */
297289
if (!IsPathmanReady())
298290
return;
@@ -554,7 +546,7 @@ pathman_planner_hook(Query *parse, int cursorOptions, ParamListInfo boundParams)
554546
uint32 query_id = parse->queryId;
555547

556548
/* Save the result in case it changes */
557-
bool pathman_ready = pathman_hooks_enabled && IsPathmanReady();
549+
bool pathman_ready = IsPathmanReady();
558550

559551
PG_TRY();
560552
{

‎src/nodes_common.c‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -348,9 +348,6 @@ canonicalize_custom_exprs_mutator(Node *node, void *cxt)
348348
/* Restore original 'varattno' */
349349
var->varattno = var->varoattno;
350350

351-
/* Forget 'location' */
352-
var->location = -1;
353-
354351
return (Node *) var;
355352
}
356353

‎src/relation_info.c‎

Lines changed: 55 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@
4747

4848

4949
/* Error messages for partitioning expression */
50-
#define PARSE_PART_EXPR_ERROR "failed to parse partitioning expression (%s)"
51-
#define COOK_PART_EXPR_ERROR "failed to analyze partitioning expression (%s)"
50+
#define PARSE_PART_EXPR_ERROR "failed to parse partitioning expression \"%s\""
51+
#define COOK_PART_EXPR_ERROR "failed to analyze partitioning expression \"%s\""
5252

5353

5454
/* Comparison function info */
@@ -105,6 +105,8 @@ static void fill_pbin_with_bounds(PartBoundInfo *pbin,
105105

106106
static int cmp_range_entries(const void *p1, const void *p2, void *arg);
107107

108+
static bool query_contains_subqueries(Node *node, void *context);
109+
108110

109111
void
110112
init_relation_info_static_data(void)
@@ -139,7 +141,6 @@ refresh_pathman_relation_info(Oid relid,
139141
Datum param_values[Natts_pathman_config_params];
140142
bool param_isnull[Natts_pathman_config_params];
141143
char *expr;
142-
Relids expr_varnos;
143144
HeapTuple htup;
144145
MemoryContext old_mcxt;
145146

@@ -182,11 +183,6 @@ refresh_pathman_relation_info(Oid relid,
182183
prel->expr = (Node *) stringToNode(expr);
183184
fix_opfuncids(prel->expr);
184185

185-
expr_varnos = pull_varnos(prel->expr);
186-
if (bms_num_members(expr_varnos) != 1)
187-
elog(ERROR, "partitioning expression should reference table \"%s\"",
188-
get_rel_name(relid));
189-
190186
/* Extract Vars and varattnos of partitioning expression */
191187
prel->expr_vars = NIL;
192188
prel->expr_atts = NULL;
@@ -559,6 +555,17 @@ fill_prel_with_partitions(PartRelationInfo *prel,
559555
#endif
560556
}
561557

558+
/* qsort comparison function for RangeEntries */
559+
static int
560+
cmp_range_entries(const void *p1, const void *p2, void *arg)
561+
{
562+
const RangeEntry *v1 = (const RangeEntry *) p1;
563+
const RangeEntry *v2 = (const RangeEntry *) p2;
564+
cmp_func_info *info = (cmp_func_info *) arg;
565+
566+
return cmp_bounds(&info->flinfo, info->collid, &v1->min, &v2->min);
567+
}
568+
562569

563570
/*
564571
* Partitioning expression routines.
@@ -691,34 +698,47 @@ cook_partitioning_expression(const Oid relid,
691698

692699
PG_TRY();
693700
{
694-
TargetEntry *target_entry;
695-
696-
Query *expr_query;
697-
PlannedStmt *expr_plan;
698-
Node *expr;
701+
Query *query;
702+
Node *expr;
703+
Relids expr_varnos;
699704

700705
/* This will fail with ERROR in case of wrong expression */
701706
query_tree_list = pg_analyze_and_rewrite(parse_tree, query_string, NULL, 0);
702707

708+
/* Sanity check #1 */
703709
if (list_length(query_tree_list) != 1)
704710
elog(ERROR, "partitioning expression produced more than 1 query");
705711

706-
expr_query = (Query *) linitial(query_tree_list);
712+
query = (Query *) linitial(query_tree_list);
707713

708-
/* Plan this query. We reuse 'expr_node' here */
709-
expr_plan = pg_plan_query(expr_query, 0, NULL);
714+
/* Sanity check #2 */
715+
if (list_length(query->targetList) != 1)
716+
elog(ERROR, "there should be exactly 1 partitioning expression");
710717

711-
target_entry = IsA(expr_plan->planTree, IndexOnlyScan) ?
712-
linitial(((IndexOnlyScan *) expr_plan->planTree)->indextlist) :
713-
linitial(expr_plan->planTree->targetlist);
718+
/* Sanity check #3 */
719+
if (query_tree_walker(query, query_contains_subqueries, NULL, 0))
720+
elog(ERROR, "subqueries are not allowed in partitioning expression");
714721

715-
expr = eval_const_expressions(NULL, (Node *) target_entry->expr);
722+
expr = (Node *) ((TargetEntry *) linitial(query->targetList))->expr;
723+
expr = eval_const_expressions(NULL, expr);
724+
725+
/* Sanity check #4 */
716726
if (contain_mutable_functions(expr))
717727
ereport(ERROR,
718728
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
719729
errmsg("functions in partitioning expression"
720730
" must be marked IMMUTABLE")));
721731

732+
/* Sanity check #5 */
733+
expr_varnos = pull_varnos(expr);
734+
if (bms_num_members(expr_varnos) != 1 ||
735+
((RangeTblEntry *) linitial(query->rtable))->relid != relid)
736+
{
737+
elog(ERROR, "partitioning expression should reference table \"%s\"",
738+
get_rel_name(relid));
739+
}
740+
bms_free(expr_varnos);
741+
722742
Assert(expr);
723743
expr_serialized = nodeToString(expr);
724744

@@ -755,6 +775,7 @@ cook_partitioning_expression(const Oid relid,
755775
/* Switch to previous mcxt */
756776
MemoryContextSwitchTo(old_mcxt);
757777

778+
/* Get Datum of serialized expression (right mcxt) */
758779
expr_datum = CStringGetTextDatum(expr_serialized);
759780

760781
/* Free memory */
@@ -763,6 +784,20 @@ cook_partitioning_expression(const Oid relid,
763784
return expr_datum;
764785
}
765786

787+
/* Check if query has subqueries */
788+
static bool
789+
query_contains_subqueries(Node *node, void *context)
790+
{
791+
if (node == NULL)
792+
return false;
793+
794+
/* We've met a subquery */
795+
if (IsA(node, Query))
796+
return true;
797+
798+
return expression_tree_walker(node, query_contains_subqueries, NULL);
799+
}
800+
766801

767802
/*
768803
* Functions for delayed invalidation.
@@ -1308,17 +1343,6 @@ fill_pbin_with_bounds(PartBoundInfo *pbin,
13081343
}
13091344
}
13101345

1311-
/* qsort comparison function for RangeEntries */
1312-
static int
1313-
cmp_range_entries(const void *p1, const void *p2, void *arg)
1314-
{
1315-
const RangeEntry *v1 = (const RangeEntry *) p1;
1316-
const RangeEntry *v2 = (const RangeEntry *) p2;
1317-
cmp_func_info *info = (cmp_func_info *) arg;
1318-
1319-
return cmp_bounds(&info->flinfo, info->collid, &v1->min, &v2->min);
1320-
}
1321-
13221346

13231347
/*
13241348
* Common PartRelationInfo checks. Emit ERROR if anything is wrong.

0 commit comments

Comments
 (0)