Github user benchristel commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1369#discussion_r214204676
--- Diff: src/backend/access/external/pxffilters.c ---
@@ -1238,64 +1236,77 @@ list_const_to_str(Const *constval, StringInfo buf)
* serializePxfFilterQuals
*
* Wrapper around pxf_make_filter_list -> pxf_serialize_filter_list.
- * Currently the only function exposed to the outside, however
- * we could expose the others in the future if needed.
*
* The function accepts the scan qual list and produces a serialized
* string that represents the push down filters (See called functions
* headers for more information).
*/
-char *serializePxfFilterQuals(List *quals)
+char *
+serializePxfFilterQuals(List *quals)
{
char *result = NULL;
if (pxf_enable_filter_pushdown)
{
- int logicalOpsNum = 0;
- List *expressionItems = pxf_make_expression_items_list(quals,
NULL, &logicalOpsNum);
+ 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;
- //Trivial expression means list of OpExpr implicitly ANDed
- bool isTrivialExpression = logicalOpsNum == 0 &&
expressionItems && expressionItems->length > 1;
+ add_extra_and_expression_items(expressionItems,
extraAndOperatorsNum, &extraBoolExprNodePointer);
+ }
- if (isTrivialExpression)
+ result = pxf_serialize_filter_list(expressionItems);
+
+ if (extraBoolExprNodePointer)
{
- enrich_trivial_expression(expressionItems);
+ pfree(extraBoolExprNodePointer);
}
- result = pxf_serialize_filter_list(expressionItems);
- pxf_free_expression_items_list(expressionItems,
isTrivialExpression);
+ pxf_free_expression_items_list(expressionItems);
}
-
elog(DEBUG1, "serializePxfFilterQuals: filter result: %s", (result ==
NULL) ? "null" : result);
return result;
}
/*
- * Takes list of expression items which supposed to be just a list of
OpExpr
- * and adds needed number of AND items
- *
+ * 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 enrich_trivial_expression(List *expressionItems) {
-
+void
+add_extra_and_expression_items(List *expressionItems, int
extraAndOperatorsNum, BoolExpr **extraNodePointer)
+{
+ if ((!expressionItems) || (extraAndOperatorsNum < 1))
--- End diff --
I think the extra parens here can be removed.
---
