Repository: incubator-hawq Updated Branches: refs/heads/master ae2af74b2 -> 24e36c935
Fix handling of implicit AND expressions Co-authored-by: Alex Denissov <[email protected]> Co-authored-by: Shivram Mani <[email protected]> Modify 'pxf_make_expression_items_list()' and 'add_extra_and_expression_items()' so that the latter procedure processes the expression list completely, without any interference with 'pxf_make_expression_items_list()'. This is identical to https://github.com/greenplum-db/gpdb/commit/3ac93fb4259436c87b6c1705bdb05c003ca93423; see also https://github.com/greenplum-db/gpdb/pull/5470. Project: http://git-wip-us.apache.org/repos/asf/incubator-hawq/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-hawq/commit/24e36c93 Tree: http://git-wip-us.apache.org/repos/asf/incubator-hawq/tree/24e36c93 Diff: http://git-wip-us.apache.org/repos/asf/incubator-hawq/diff/24e36c93 Branch: refs/heads/master Commit: 24e36c935c303fd1fdb62cdfa140653e390f3c50 Parents: a12ed20 Author: Ivan Leskin <[email protected]> Authored: Fri Aug 31 16:37:13 2018 +0300 Committer: Shivram Mani <[email protected]> Committed: Fri Aug 31 07:50:13 2018 -0700 ---------------------------------------------------------------------- src/backend/access/external/pxffilters.c | 57 +++++++++++---------------- 1 file changed, 24 insertions(+), 33 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/24e36c93/src/backend/access/external/pxffilters.c ---------------------------------------------------------------------- diff --git a/src/backend/access/external/pxffilters.c b/src/backend/access/external/pxffilters.c index 25d5616..cf76061 100644 --- a/src/backend/access/external/pxffilters.c +++ b/src/backend/access/external/pxffilters.c @@ -31,7 +31,7 @@ #include "utils/guc.h" #include "utils/lsyscache.h" -static List* pxf_make_expression_items_list(List *quals, Node *parent, int *logicalOpsNum); +static List* pxf_make_expression_items_list(List *quals, Node *parent); static void pxf_free_filter(PxfFilterDesc* filter); static char* pxf_serialize_filter_list(List *filters); static bool opexpr_to_pxffilter(OpExpr *expr, PxfFilterDesc *filter); @@ -43,7 +43,7 @@ static bool supported_operator_type_scalar_array_op_expr(Oid type, PxfFilterDesc static void scalar_const_to_str(Const *constval, StringInfo buf); static void list_const_to_str(Const *constval, StringInfo buf); static List* append_attr_from_var(Var* var, List* attrs); -static void add_extra_and_expression_items(List *expressionItems, int extraAndOperatorsNum, BoolExpr **extraNodePointer); +static void add_extra_and_expression_items(List *expressionItems, int extraAndOperatorsNum); static List* get_attrs_from_expr(Expr *expr, bool* expressionIsSupported); /* @@ -298,14 +298,15 @@ pxf_free_expression_items_list(List *expressionItems) * */ static List * -pxf_make_expression_items_list(List *quals, Node *parent, int *logicalOpsNum) +pxf_make_expression_items_list(List *quals, Node *parent) { ExpressionItem *expressionItem = NULL; List *result = NIL; ListCell *lc = NULL; ListCell *ilc = NULL; + int quals_size = list_length(quals); - if (list_length(quals) == 0) + if (quals_size == 0) return NIL; foreach (lc, quals) @@ -319,19 +320,22 @@ pxf_make_expression_items_list(List *quals, Node *parent, int *logicalOpsNum) switch (tag) { - case T_Var: // IN(single_value) + case T_Var: + /* IN(single_value) */ case T_OpExpr: + /* Comparison operators >, >=, =, etc */ case T_ScalarArrayOpExpr: + /* IN(multiple values) */ case T_NullTest: { result = lappend(result, expressionItem); break; } case T_BoolExpr: + /* Logical operators AND, OR, NOT */ { - (*logicalOpsNum)++; BoolExpr *expr = (BoolExpr *) node; - List *inner_result = pxf_make_expression_items_list(expr->args, node, logicalOpsNum); + List *inner_result = pxf_make_expression_items_list(expr->args, node); result = list_concat(result, inner_result); int childNodesNum = 0; @@ -376,6 +380,14 @@ pxf_make_expression_items_list(List *quals, Node *parent, int *logicalOpsNum) } } + if ( quals_size > 1 && parent == NULL ) + { + /* + If we find more than 1 qualifier, it means we have at least two expressions that are implicitly AND-ed by the query planner. Here, to make it explicit, we will need to add additional AND operators to compensate for the missing ones. + */ + add_extra_and_expression_items(result, quals_size - 1); + } + return result; } @@ -1248,30 +1260,12 @@ serializePxfFilterQuals(List *quals) if (pxf_enable_filter_pushdown) { + /* expressionItems will contain all the expressions including comparator and logical operators in postfix order */ + List *expressionItems = pxf_make_expression_items_list(quals, NULL); - BoolExpr *extraBoolExprNodePointer = NULL; - int logicalOpsNum = 0; - List *expressionItems = pxf_make_expression_items_list(quals, NULL, &logicalOpsNum); - - /* - * The 'expressionItems' are always explicitly AND'ed. If there are extra - * logical operations present, they are the items in the same list. We - * need to add AND items only for "meaningful" expression items (not - * including these logical operations) - */ - if (expressionItems) - { - int extraAndOperatorsNum = expressionItems->length - 1 - logicalOpsNum; - - add_extra_and_expression_items(expressionItems, extraAndOperatorsNum, &extraBoolExprNodePointer); - } - + /* result will contain seralized version of the above postfix ordered expressions list */ result = pxf_serialize_filter_list(expressionItems); - if (extraBoolExprNodePointer) - { - pfree(extraBoolExprNodePointer); - } pxf_free_expression_items_list(expressionItems); } @@ -1282,14 +1276,12 @@ serializePxfFilterQuals(List *quals) /* * Adds a given number of AND expression items to an existing list of expression items - * Note that this will not work if OR operators are introduced */ void -add_extra_and_expression_items(List *expressionItems, int extraAndOperatorsNum, BoolExpr **extraNodePointer) +add_extra_and_expression_items(List *expressionItems, int extraAndOperatorsNum) { - if ((!expressionItems) || (extraAndOperatorsNum < 1)) + if (!expressionItems || extraAndOperatorsNum < 1) { - *extraNodePointer = NULL; return; } @@ -1298,7 +1290,6 @@ add_extra_and_expression_items(List *expressionItems, int extraAndOperatorsNum, BoolExpr *andExpr = makeNode(BoolExpr); andExpr->boolop = AND_EXPR; - *extraNodePointer = andExpr; andExpressionItem->node = (Node *) andExpr; andExpressionItem->parent = NULL;
