From 8d18a51cc734745f173bb067d3d974fba25fd8e1 Mon Sep 17 00:00:00 2001 From: Jeff Davis Date: Sat, 31 Jul 2021 10:20:45 -0700 Subject: [PATCH] Columnar: enable chunk group filtering to work for Params. Evaluate extern params into Consts in the qual when the scan begins. Fixes #4488. --- src/backend/columnar/columnar_customscan.c | 120 ++++++++++++++++++ .../input/columnar_chunk_filtering.source | 16 +++ .../output/columnar_chunk_filtering.source | 55 ++++++++ 3 files changed, 191 insertions(+) diff --git a/src/backend/columnar/columnar_customscan.c b/src/backend/columnar/columnar_customscan.c index 88760c654..ceadcb8ff 100644 --- a/src/backend/columnar/columnar_customscan.c +++ b/src/backend/columnar/columnar_customscan.c @@ -16,12 +16,16 @@ #include "access/skey.h" #include "nodes/extensible.h" +#include "nodes/makefuncs.h" +#include "nodes/nodeFuncs.h" #include "nodes/pg_list.h" #include "nodes/plannodes.h" #include "optimizer/optimizer.h" #include "optimizer/pathnode.h" #include "optimizer/paths.h" #include "optimizer/restrictinfo.h" +#include "utils/datum.h" +#include "utils/lsyscache.h" #include "utils/relcache.h" #include "columnar/columnar_customscan.h" @@ -78,6 +82,8 @@ static void ColumnarScan_ReScanCustomScan(CustomScanState *node); static void ColumnarScan_ExplainCustomScan(CustomScanState *node, List *ancestors, ExplainState *es); +static List * ExprListBindParams(List *node, ParamListInfo paramLI); + /* saved hook value in case of unload */ static set_rel_pathlist_hook_type PreviousSetRelPathlistHook = NULL; @@ -362,6 +368,11 @@ ColumnarScan_CreateCustomScanState(CustomScan *cscan) static void ColumnarScan_BeginCustomScan(CustomScanState *cscanstate, EState *estate, int eflags) { + ColumnarScanState *columnarScanState = (ColumnarScanState *) cscanstate; + + columnarScanState->qual = ExprListBindParams(columnarScanState->qual, + estate->es_param_list_info); + /* scan slot is already initialized */ } @@ -525,3 +536,112 @@ ColumnarScan_ExplainCustomScan(CustomScanState *node, List *ancestors, chunkGroupsFiltered, es); } } + + +static Node * +ExprListBindParams_mutator(Node *node, ParamListInfo paramLI) +{ + if (node == NULL) + { + return NULL; + } + + /* copied from eval_const_expressions() */ + if (IsA(node, Param)) + { + Param *param = (Param *) node; + + /* Look to see if we've been given a value for this Param */ + if (param->paramkind == PARAM_EXTERN && + paramLI != NULL && + param->paramid > 0 && + param->paramid <= paramLI->numParams) + { + ParamExternData *prm; + ParamExternData prmdata; + + /* + * Give hook a chance in case parameter is dynamic. Tell + * it that this fetch is speculative, so it should avoid + * erroring out if parameter is unavailable. + */ + if (paramLI->paramFetch != NULL) + { + prm = paramLI->paramFetch(paramLI, param->paramid, + true, &prmdata); + } + else + { + prm = ¶mLI->params[param->paramid - 1]; + } + + /* + * We don't just check OidIsValid, but insist that the + * fetched type match the Param, just in case the hook did + * something unexpected. No need to throw an error here + * though; leave that for runtime. + */ + if (OidIsValid(prm->ptype) && + prm->ptype == param->paramtype) + { + /* OK to substitute parameter value? */ + if (prm->pflags & PARAM_FLAG_CONST) + { + /* + * Return a Const representing the param value. + * Must copy pass-by-ref datatypes, since the + * Param might be in a memory context + * shorter-lived than our output plan should be. + */ + int16 typLen; + bool typByVal; + Datum pval; + + get_typlenbyval(param->paramtype, + &typLen, &typByVal); + if (prm->isnull || typByVal) + { + pval = prm->value; + } + else + { + pval = datumCopy(prm->value, typByVal, typLen); + } + pval = datumCopy(prm->value, typByVal, typLen); + Const *con = makeConst(param->paramtype, + param->paramtypmod, + param->paramcollid, + (int) typLen, + pval, + prm->isnull, + typByVal); + con->location = param->location; + return (Node *) con; + } + } + } + + /* + * Not replaceable, so just copy the Param (no need to + * recurse) + */ + return (Node *) copyObject(param); + } + + return expression_tree_mutator(node, ExprListBindParams_mutator, (void *) paramLI); +} + + +/* + * ExprListBindParams - take a List of Exprs, and replace Params with Consts + * using paramLI, if able. + * + * The given node can be any type acceptable to expression_tree_mutator + * (e.g. List); not just an Expr. + */ +static List * +ExprListBindParams(List *exprList, ParamListInfo paramLI) +{ + return (List *) expression_tree_mutator( + (Node *) exprList, ExprListBindParams_mutator, (void *) paramLI); +} diff --git a/src/test/regress/input/columnar_chunk_filtering.source b/src/test/regress/input/columnar_chunk_filtering.source index 0fa8bc18d..332df54b8 100644 --- a/src/test/regress/input/columnar_chunk_filtering.source +++ b/src/test/regress/input/columnar_chunk_filtering.source @@ -111,3 +111,19 @@ create table part_2_columnar partition of part_table for values from (0) to (150 insert into part_table select generate_series(1,159999); select filtered_row_count('select count(*) from part_table where id > 75000'); drop table part_table; + +-- +-- https://github.com/citusdata/citus/issues/4488 +-- +create table columnar_prepared_stmt (x int, y int) using columnar; +insert into columnar_prepared_stmt select s, s from generate_series(1,5000000) s; +prepare foo (int) as select x from columnar_prepared_stmt where x = $1; +execute foo(3); +execute foo(3); +execute foo(3); +execute foo(3); +select filtered_row_count('execute foo(3)'); +select filtered_row_count('execute foo(3)'); +select filtered_row_count('execute foo(3)'); +select filtered_row_count('execute foo(3)'); +drop table columnar_prepared_stmt; diff --git a/src/test/regress/output/columnar_chunk_filtering.source b/src/test/regress/output/columnar_chunk_filtering.source index cc05fff7b..3f72e1c67 100644 --- a/src/test/regress/output/columnar_chunk_filtering.source +++ b/src/test/regress/output/columnar_chunk_filtering.source @@ -196,3 +196,58 @@ select filtered_row_count('select count(*) from part_table where id > 75000'); (1 row) drop table part_table; +-- +-- https://github.com/citusdata/citus/issues/4488 +-- +create table columnar_prepared_stmt (x int, y int) using columnar; +insert into columnar_prepared_stmt select s, s from generate_series(1,5000000) s; +prepare foo (int) as select x from columnar_prepared_stmt where x = $1; +execute foo(3); + x +--------------------------------------------------------------------- + 3 +(1 row) + +execute foo(3); + x +--------------------------------------------------------------------- + 3 +(1 row) + +execute foo(3); + x +--------------------------------------------------------------------- + 3 +(1 row) + +execute foo(3); + x +--------------------------------------------------------------------- + 3 +(1 row) + +select filtered_row_count('execute foo(3)'); + filtered_row_count +--------------------------------------------------------------------- + 9999 +(1 row) + +select filtered_row_count('execute foo(3)'); + filtered_row_count +--------------------------------------------------------------------- + 9999 +(1 row) + +select filtered_row_count('execute foo(3)'); + filtered_row_count +--------------------------------------------------------------------- + 9999 +(1 row) + +select filtered_row_count('execute foo(3)'); + filtered_row_count +--------------------------------------------------------------------- + 9999 +(1 row) + +drop table columnar_prepared_stmt;