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.

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;