Fix implicit AND expression addition

Fix implicit addition of extra 'BoolExpr' to a list of expression items.

Before, there was a check that the expression items list did not contain logical operators
(and if it did, no extra implicit AND operators were added).

This behaviour is incorrect. Consider the following query:

SELECT * FROM table_ex WHERE bool1=false AND id1=60003;

Such query will be translated as a list of three items: 'BoolExpr', 'Var' and 'OpExpr'.
Due to the presence of a 'BoolExpr', extra implicit 'BoolExpr' will not be added, and
we get an error "stack is not empty ...".

This commit changes the signatures of some internal functions in pxffilters.c to fix this error.
We pass a number of required extra 'BoolExpr's to 'add_extra_and_expression_items'.

As 'BoolExpr's of different origin may be present in the list of expression items,
the mechanism of freeing the BoolExpr node changes.

The current mechanism of implicit AND expressions addition is suitable only before
OR operators are introduced (in that case, we will have to add ANDs to different parts
of a list, not just the end, as done now).

src/backend/access/external/pxffilters.c

@@ -6,9 +6,9 @@
  * to you under the Apache License, Version 2.0 (the
  * "License"); you may not use this file except in compliance
  * with the License.  You may obtain a copy of the License at
- * 
+ *
  *   http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing,
  * software distributed under the License is distributed on an
  * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
@@ -21,7 +21,7 @@
  * pxffilters.c
  *
  * Functions for handling push down of supported scan level filters to PXF.
- * 
+ *
  */
 #include "access/pxffilters.h"
 #include "catalog/pg_operator.h"
@@ -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 enrich_trivial_expression(List *expressionItems);
+static void add_extra_and_expression_items(List *expressionItems, int extraAndOperatorsNum, BoolExpr **extraNodePointer);
 static List* get_attrs_from_expr(Expr *expr, bool* expressionIsSupported);
 
 /*
@@ -264,22 +264,21 @@ Oid pxf_supported_types[] =
 };
 
 static void
-pxf_free_expression_items_list(List *expressionItems, bool freeBoolExprNodes)
+pxf_free_expression_items_list(List *expressionItems)
 {
-	ExpressionItem 	*expressionItem = NULL;
-	int previousLength;
+	ExpressionItem *expressionItem = NULL;
 
 	while (list_length(expressionItems) > 0)
 	{
-		expressionItem = (ExpressionItem *) lfirst(list_head(expressionItems));
-		if (freeBoolExprNodes && nodeTag(expressionItem->node) == T_BoolExpr)
-		{
-			pfree((BoolExpr *)expressionItem->node);
-		}
+		expressionItem = (ExpressionItem *) linitial(expressionItems);
 		pfree(expressionItem);
 
-		/* to avoid freeing already freed items - delete all occurrences of current expression*/
-		previousLength = expressionItems->length + 1;
+		/*
+		 * to avoid freeing already freed items - delete all occurrences of
+		 * current expression
+		 */
+		int			previousLength = expressionItems->length + 1;
+
 		while (expressionItems != NULL && previousLength > expressionItems->length)
 		{
 			previousLength = expressionItems->length;
@@ -297,7 +296,6 @@ pxf_free_expression_items_list(List *expressionItems, bool freeBoolExprNodes)
  *
  * Basically this function just transforms expression tree to Reversed Polish Notation list.
  *
- *
  */
 static List *
 pxf_make_expression_items_list(List *quals, Node *parent, int *logicalOpsNum)
@@ -306,7 +304,7 @@ pxf_make_expression_items_list(List *quals, Node *parent, int *logicalOpsNum)
 	List			*result = NIL;
 	ListCell		*lc = NULL;
 	ListCell		*ilc = NULL;
-	
+
 	if (list_length(quals) == 0)
 		return NIL;
 
@@ -377,7 +375,7 @@ pxf_make_expression_items_list(List *quals, Node *parent, int *logicalOpsNum)
 				break;
 		}
 	}
-	
+
 	return result;
 }
 
@@ -872,7 +870,7 @@ append_attr_from_func_args(FuncExpr *expr, List* attrs, bool* expressionIsSuppor
 		} else if (IsA(node, Var)) {
 			attrs = append_attr_from_var((Var *) node, attrs);
 		} else if (IsA(node, OpExpr)) {
-			attrs = get_attrs_from_expr((OpExpr *) node, expressionIsSupported);
+			attrs = get_attrs_from_expr((Expr *) node, expressionIsSupported);
 		} else {
 			*expressionIsSupported = false;
 			return NIL;
@@ -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))
+	{
+		*extraNodePointer = NULL;
+		return;
+	}
 
-	int logicalOpsNumNeeded = expressionItems->length - 1;
+	ExpressionItem *andExpressionItem = (ExpressionItem *) palloc0(sizeof(ExpressionItem));
 
-	if (logicalOpsNumNeeded > 0)
-	{
-		ExpressionItem *andExpressionItem = (ExpressionItem *) palloc0(sizeof(ExpressionItem));
-		BoolExpr *andExpr = makeNode(BoolExpr);
+	BoolExpr   *andExpr = makeNode(BoolExpr);
 
-		andExpr->boolop = AND_EXPR;
+	andExpr->boolop = AND_EXPR;
+	*extraNodePointer = andExpr;
 
-		andExpressionItem->node = (Node *) andExpr;
-		andExpressionItem->parent = NULL;
-		andExpressionItem->processed = false;
+	andExpressionItem->node = (Node *) andExpr;
+	andExpressionItem->parent = NULL;
+	andExpressionItem->processed = false;
 
-		for (int i = 0; i < logicalOpsNumNeeded; i++) {
-			expressionItems = lappend(expressionItems, andExpressionItem);
-		}
+	for (int i = 0; i < extraAndOperatorsNum; i++)
+	{
+		expressionItems = lappend(expressionItems, andExpressionItem);
 	}
 }