Skip to content

Commit 095eb98

Browse files
ildusildus
committed
Fix pathlist hooks
1 parent 102bb31 commit 095eb98

File tree

9 files changed

+154
-67
lines changed

9 files changed

+154
-67
lines changed

‎sql/pathman_basic.sql

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@ 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-
:gdb
15-
SELECT pg_sleep(10);
1614
SELECT pathman.create_hash_partitions('test.hash_rel', 'value + 1', 3);
1715
ALTER TABLE test.hash_rel ALTER COLUMN value SET NOT NULL;
1816
SELECT pathman.create_hash_partitions('test.hash_rel', 'value', 3, partition_data:=false);

‎src/hooks.c

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -248,15 +248,15 @@ pathman_rel_pathlist_hook(PlannerInfo *root,
248248
Relation parent_rel; /* parent's relation (heap) */
249249
Oid *children; /* selected children oids */
250250
List *ranges, /* a list of IndexRanges */
251-
*wrappers, /* a list of WrapperNodes */
252-
*rel_part_clauses = NIL; /* clauses with part. column */
251+
*wrappers; /* a list of WrapperNodes */
253252
PathKey *pathkeyAsc = NULL,
254253
*pathkeyDesc = NULL;
255254
double paramsel = 1.0; /* default part selectivity */
256255
WalkerContext context;
257256
ListCell *lc;
258257
int i;
259258
Node *expr;
259+
bool modify_append_nodes;
260260

261261
/* Make copy of partitioning expression and fix Var's varno attributes */
262262
expr = copyObject(prel->expr);
@@ -306,6 +306,12 @@ pathman_rel_pathlist_hook(PlannerInfo *root,
306306
ranges = irange_list_intersection(ranges, wrap->rangeset);
307307
}
308308

309+
/*
310+
* Walker should been have filled these parameter while checking.
311+
* Runtime[Merge]Append is pointless if there are no params in clauses.
312+
*/
313+
modify_append_nodes = context.found_params;
314+
309315
/* Get number of selected partitions */
310316
irange_len = irange_list_length(ranges);
311317
if (prel->enable_parent)
@@ -382,12 +388,7 @@ pathman_rel_pathlist_hook(PlannerInfo *root,
382388
pg_pathman_enable_runtime_merge_append))
383389
return;
384390

385-
/* Check that rel's RestrictInfo contains partitioned column */
386-
rel_part_clauses = get_partitioned_attr_clauses(rel->baserestrictinfo,
387-
prel, rel->relid);
388-
389-
/* Runtime[Merge]Append is pointless if there are no params in clauses */
390-
if (!clause_contains_params((Node *) rel_part_clauses))
391+
if (!modify_append_nodes)
391392
return;
392393

393394
/* Generate Runtime[Merge]Append paths if needed */
@@ -416,7 +417,7 @@ pathman_rel_pathlist_hook(PlannerInfo *root,
416417
* Skip if neither rel->baserestrictinfo nor
417418
* ppi->ppi_clauses reference partition attribute
418419
*/
419-
if (!(rel_part_clauses || ppi_part_clauses))
420+
if (!(modify_append_nodes || ppi_part_clauses))
420421
continue;
421422

422423
if (IsA(cur_path, AppendPath) && pg_pathman_enable_runtimeappend)

‎src/include/pathman.h

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -141,10 +141,12 @@ typedef struct
141141

142142
typedef struct
143143
{
144-
Node *prel_expr; /* expression from PartRelationInfo */
145-
const PartRelationInfo *prel; /* main partitioning structure */
146-
ExprContext *econtext; /* for ExecEvalExpr() */
147-
bool for_insert; /* are we in PartitionFilter now? */
144+
Node *prel_expr; /* expression from PartRelationInfo */
145+
const PartRelationInfo *prel; /* main partitioning structure */
146+
ExprContext *econtext; /* for ExecEvalExpr() */
147+
bool for_insert; /* are we in PartitionFilter now? */
148+
bool found_params; /* mark if left or right argument
149+
of clause is Param */
148150
} WalkerContext;
149151

150152
/* Usual initialization procedure for WalkerContext */
@@ -154,6 +156,7 @@ typedef struct
154156
(context)->prel = (prel_info); \
155157
(context)->econtext = (ecxt); \
156158
(context)->for_insert = (for_ins); \
159+
(context)->found_params = (false); \
157160
} while (0)
158161

159162
/* Check that WalkerContext contains ExprContext (plan execution stage) */

‎src/include/relation_info.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ typedef struct
139139

140140
const char *attname; /* original expression */
141141
Node *expr; /* planned expression */
142+
List *expr_vars; /* vars from expression, lazy */
142143
Oid atttype; /* expression type */
143144
int32 atttypmod; /* expression type modifier */
144145
bool attbyval; /* is partitioned column stored by value? */

‎src/include/utils.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
bool clause_contains_params(Node *clause);
2929
bool is_date_type_internal(Oid typid);
3030
bool check_security_policy_internal(Oid relid, Oid role);
31+
bool match_expr_to_operand(Node *operand, Node *expr);
3132

3233
/*
3334
* Misc.

‎src/nodes_common.c

Lines changed: 112 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
* ------------------------------------------------------------------------
99
*/
1010

11+
#include "init.h"
1112
#include "nodes_common.h"
1213
#include "runtimeappend.h"
1314
#include "utils.h"
@@ -128,41 +129,83 @@ replace_tlist_varnos(List *tlist, Index old_varno, Index new_varno)
128129
return temp_tlist;
129130
}
130131

132+
/* Check that one of arguments of OpExpr is expression */
133+
static bool
134+
extract_vars(Node *node, PartRelationInfo *prel)
135+
{
136+
if (node == NULL)
137+
return false;
138+
139+
if (IsA(node, Var))
140+
{
141+
prel->expr_vars = lappend(prel->expr_vars, node);
142+
return false;
143+
}
144+
145+
return expression_tree_walker(node, extract_vars, (void *) prel);
146+
}
147+
148+
149+
/*
150+
* This function fills 'expr_vars' attribute in PartRelationInfo.
151+
* For now it's static because there are no other places where we need it.
152+
*/
153+
static inline List *
154+
extract_vars_from_expression(PartRelationInfo *prel)
155+
{
156+
if (prel->expr_vars == NIL)
157+
{
158+
MemoryContext ctx;
159+
160+
prel->expr_vars = NIL;
161+
ctx = MemoryContextSwitchTo(PathmanRelationCacheContext);
162+
extract_vars(prel->expr, prel);
163+
MemoryContextSwitchTo(ctx);
164+
}
165+
166+
return prel->expr_vars;
167+
}
168+
131169
/* Append partition attribute in case it's not present in target list */
132170
static List *
133171
append_part_attr_to_tlist(List *tlist, Index relno, const PartRelationInfo *prel)
134172
{
135-
ListCell *lc;
136-
bool part_attr_found = false;
173+
ListCell *lc,
174+
*lc_var;
175+
List *vars = extract_vars_from_expression((PartRelationInfo *) prel);
176+
List *vars_not_found = NIL;
137177

138-
foreach (lc, tlist)
178+
foreach (lc_var, vars)
139179
{
140-
TargetEntry *te = (TargetEntry *) lfirst(lc);
141-
Var *var = (Var *) te->expr;
180+
bool part_attr_found = false;
181+
Var *expr_var = (Var *) lfirst(lc_var);
182+
183+
foreach (lc, tlist)
184+
{
185+
TargetEntry *te = (TargetEntry *) lfirst(lc);
186+
Var *var = (Var *) te->expr;
187+
188+
if (IsA(var, Var) && var->varoattno == expr_var->varattno)
189+
{
190+
part_attr_found = true;
191+
break;
192+
}
193+
}
142194

143-
/* FIX this
144-
if (IsA(var, Var) && var->varoattno == prel->attnum)
145-
part_attr_found = true;
146-
*/
195+
if (!part_attr_found)
196+
vars_not_found = lappend(vars_not_found, expr_var);
147197
}
148198

149-
/* FIX this
150-
if (!part_attr_found)
199+
foreach(lc, vars_not_found)
151200
{
152-
Var *newvar = makeVar(relno,
153-
prel->attnum,
154-
prel->atttype,
155-
prel->atttypmod,
156-
prel->attcollid,
157-
0);
158-
201+
Var *var = (Var *) lfirst(lc);
159202
Index last_item = list_length(tlist) + 1;
160-
161-
tlist = lappend(tlist, makeTargetEntry((Expr *) newvar,
203+
tlist = lappend(tlist, makeTargetEntry((Expr *) var,
162204
last_item,
163205
NULL, false));
164-
} */
206+
}
165207

208+
list_free(vars_not_found);
166209
return tlist;
167210
}
168211

@@ -242,6 +285,37 @@ unpack_runtimeappend_private(RuntimeAppendState *scan_state, CustomScan *cscan)
242285
scan_state->enable_parent = (bool) linitial_int(lthird(runtimeappend_private));
243286
}
244287

288+
struct check_clause_context
289+
{
290+
Node *prel_expr;
291+
int count;
292+
};
293+
294+
/* Check that one of arguments of OpExpr is expression */
295+
static bool
296+
check_clause_for_expression(Node *node, struct check_clause_context *ctx)
297+
{
298+
if (node == NULL)
299+
return false;
300+
301+
if (IsA(node, OpExpr))
302+
{
303+
OpExpr *expr = (OpExpr *) node;
304+
Node *left = linitial(expr->args),
305+
*right = lsecond(expr->args);
306+
307+
if (match_expr_to_operand(left, ctx->prel_expr))
308+
ctx->count += 1;
309+
310+
if (match_expr_to_operand(right, ctx->prel_expr))
311+
ctx->count += 1;
312+
313+
return false;
314+
}
315+
316+
return expression_tree_walker(node, check_clause_for_expression, (void *) ctx);
317+
}
318+
245319
/*
246320
* Filter all available clauses and extract relevant ones.
247321
*/
@@ -250,27 +324,22 @@ get_partitioned_attr_clauses(List *restrictinfo_list,
250324
const PartRelationInfo *prel,
251325
Index partitioned_rel)
252326
{
253-
#define AdjustAttno(attno) \
254-
( (AttrNumber) (attno + FirstLowInvalidHeapAttributeNumber) )
255-
256327
List *result = NIL;
257328
ListCell *l;
258329

259330
foreach(l, restrictinfo_list)
260331
{
261-
RestrictInfo *rinfo = (RestrictInfo *) lfirst(l);
262-
Bitmapset *varattnos = NULL;
263-
int part_attno;
332+
RestrictInfo *rinfo = (RestrictInfo *) lfirst(l);
333+
struct check_clause_context ctx;
264334

265335
Assert(IsA(rinfo, RestrictInfo));
266-
pull_varattnos((Node *) rinfo->clause, partitioned_rel, &varattnos);
267336

268-
/* FIX this
269-
if (bms_get_singleton_member(varattnos, &part_attno) &&
270-
AdjustAttno(part_attno) == prel->attnum)
271-
{
337+
ctx.count = 0;
338+
ctx.prel_expr = prel->expr;
339+
check_clause_for_expression((Node *) rinfo->clause, &ctx);
340+
341+
if (ctx.count == 1)
272342
result = lappend(result, rinfo->clause);
273-
} */
274343
}
275344
return result;
276345
}
@@ -554,14 +623,23 @@ rescan_append_common(CustomScanState *node)
554623
WalkerContext wcxt;
555624
Oid *parts;
556625
int nparts;
626+
Node *prel_expr;
557627

558628
prel = get_pathman_relation_info(scan_state->relid);
559629
Assert(prel);
560630

631+
/* Prepare expression */
632+
prel_expr = prel->expr;
633+
if (INDEX_VAR != 1)
634+
{
635+
prel_expr = copyObject(prel_expr);
636+
ChangeVarNodes(prel_expr, 1, INDEX_VAR, 0);
637+
}
638+
561639
/* First we select all available partitions... */
562640
ranges = list_make1_irange(make_irange(0, PrelLastChild(prel), IR_COMPLETE));
563641

564-
InitWalkerContext(&wcxt, INDEX_VAR, prel, econtext, false);
642+
InitWalkerContext(&wcxt, prel_expr, prel, econtext, false);
565643
foreach (lc, scan_state->custom_exprs)
566644
{
567645
WrapperNode *wn;

‎src/pg_pathman.c

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,6 @@ static Path *get_cheapest_parameterized_child_path(PlannerInfo *root,
9999
RelOptInfo *rel,
100100
Relids required_outer);
101101

102-
static bool match_expr_to_operand(Node *operand, Node *expr);
103102
/* We can transform Param into Const provided that 'econtext' is available */
104103
#define IsConstValue(wcxt, node) \
105104
( IsA((node), Const) || (WcxtHasExprContext(wcxt) ? IsA((node), Param) : false) )
@@ -1052,6 +1051,9 @@ handle_opexpr(const OpExpr *expr, WalkerContext *context)
10521051
}
10531052
else if (IsA(param, Param) || IsA(param, Var))
10541053
{
1054+
if (IsA(param, Param))
1055+
context->found_params = true;
1056+
10551057
handle_binary_opexpr_param(prel, result, var);
10561058
return result;
10571059
}
@@ -1153,23 +1155,6 @@ handle_binary_opexpr_param(const PartRelationInfo *prel,
11531155
}
11541156

11551157

1156-
/*
1157-
* Compare clause operand with our expression
1158-
*/
1159-
static bool
1160-
match_expr_to_operand(Node *operand, Node *expr)
1161-
{
1162-
/* strip relabeling for both operand and expr */
1163-
if (operand && IsA(operand, RelabelType))
1164-
operand = (Node *) ((RelabelType *) operand)->arg;
1165-
1166-
if (expr && IsA(expr, RelabelType))
1167-
expr = (Node *) ((RelabelType *) expr)->arg;
1168-
1169-
/* compare expressions and return result right away */
1170-
return equal(expr, operand);
1171-
}
1172-
11731158
/*
11741159
* Checks if expression is a KEY OP PARAM or PARAM OP KEY, where KEY is
11751160
* partition expression and PARAM is whatever.

‎src/relation_info.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,7 @@ refresh_pathman_relation_info(Oid relid,
187187

188188
prel->attname = TextDatumGetCString(values[Anum_pathman_config_expression - 1]);
189189
prel->expr = (Node *) stringToNode(expr);
190+
prel->expr_vars = NIL;
190191
fix_opfuncids(prel->expr);
191192

192193
MemoryContextSwitchTo(oldcontext);

‎src/utils.c

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,3 +467,22 @@ extract_binary_interval_from_text(Datum interval_text, /* interval as TEXT */
467467

468468
return interval_binary;
469469
}
470+
471+
472+
/*
473+
* Compare clause operand with expression
474+
*/
475+
bool
476+
match_expr_to_operand(Node *operand, Node *expr)
477+
{
478+
/* strip relabeling for both operand and expr */
479+
if (operand && IsA(operand, RelabelType))
480+
operand = (Node *) ((RelabelType *) operand)->arg;
481+
482+
if (expr && IsA(expr, RelabelType))
483+
expr = (Node *) ((RelabelType *) expr)->arg;
484+
485+
/* compare expressions and return result right away */
486+
return equal(expr, operand);
487+
}
488+

0 commit comments

Comments
 (0)