Repository: incubator-hawq
Updated Branches:
  refs/heads/master 2cc152eb0 -> 7f3658d3a


HAWQ-1048. Support OR, NOT logical operators in the HAWQ/PXF Bridge.


Project: http://git-wip-us.apache.org/repos/asf/incubator-hawq/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-hawq/commit/7f3658d3
Tree: http://git-wip-us.apache.org/repos/asf/incubator-hawq/tree/7f3658d3
Diff: http://git-wip-us.apache.org/repos/asf/incubator-hawq/diff/7f3658d3

Branch: refs/heads/master
Commit: 7f3658d3aa5933028835b753df8fb6cfc4934856
Parents: 2cc152e
Author: Oleksandr Diachenko <odiache...@pivotal.io>
Authored: Thu Oct 13 14:19:12 2016 -0700
Committer: Oleksandr Diachenko <odiache...@pivotal.io>
Committed: Thu Oct 13 14:19:29 2016 -0700

----------------------------------------------------------------------
 src/backend/access/external/pxffilters.c        | 328 ++++++++++++-------
 .../access/external/test/pxffilters_test.c      | 106 +++---
 src/include/access/pxffilters.h                 |  10 +-
 3 files changed, 260 insertions(+), 184 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/7f3658d3/src/backend/access/external/pxffilters.c
----------------------------------------------------------------------
diff --git a/src/backend/access/external/pxffilters.c 
b/src/backend/access/external/pxffilters.c
index 354c719..56aea55 100644
--- a/src/backend/access/external/pxffilters.c
+++ b/src/backend/access/external/pxffilters.c
@@ -31,14 +31,14 @@
 #include "utils/guc.h"
 #include "utils/lsyscache.h"
 
-static List* pxf_make_filter_list(List* quals);
+static List* pxf_make_expression_items_list(List *quals, Node *parent, bool 
*logicalOpsNum);
 static void pxf_free_filter(PxfFilterDesc* filter);
-static void pxf_free_filter_list(List *filters);
 static char* pxf_serialize_filter_list(List *filters);
 static bool opexpr_to_pxffilter(OpExpr *expr, PxfFilterDesc *filter);
 static bool supported_filter_type(Oid type);
 static void const_to_str(Const *constval, StringInfo buf);
 static List* append_attr_from_var(Var* var, List* attrs);
+static void enrich_trivial_expression(List *expressionItems);
 
 /*
  * All supported HAWQ operators, and their respective HFDS operator code.
@@ -135,7 +135,40 @@ dbop_pxfop_map pxf_supported_opr[] =
        {1097 /* date_gt */, PXFOP_GT},
        {1096 /* date_le */, PXFOP_LE},
        {1098 /* date_ge */, PXFOP_GE},
-       {1094 /* date_ne */, PXFOP_NE}
+       {1094 /* date_ne */, PXFOP_NE},
+
+       /* float8 */
+       {Float8EqualOperator  /* float8eq */, PXFOP_EQ},
+       {672 /* float8lt */, PXFOP_LT},
+       {674 /* float8gt */, PXFOP_GT},
+       {673 /* float8le */, PXFOP_LE},
+       {675 /* float8ge */, PXFOP_GE},
+       {671 /* float8ne */, PXFOP_NE},
+
+       /* float48 */
+       {1120 /* float48eq */, PXFOP_EQ},
+       {1122 /* float48lt */, PXFOP_LT},
+       {1123 /* float48gt */, PXFOP_GT},
+       {1124 /* float48le */, PXFOP_LE},
+       {1125 /* float48ge */, PXFOP_GE},
+       {1121 /* float48ne */, PXFOP_NE},
+
+       /* bpchar */
+       {BPCharEqualOperator  /* bpchareq */, PXFOP_EQ},
+       {1058  /* bpcharlt */, PXFOP_LT},
+       {1060 /* bpchargt */, PXFOP_GT},
+       {1059 /* bpcharle */, PXFOP_LE},
+       {1061 /* bpcharge */, PXFOP_GE},
+       {1057 /* bpcharne */, PXFOP_NE}
+
+       /* bytea */
+       // TODO: uncomment once HAWQ-1085 is done
+       //,{ByteaEqualOperator  /* byteaeq */, PXFOP_EQ},
+       //{1957  /* bytealt */, PXFOP_LT},
+       //{1959 /* byteagt */, PXFOP_GT},
+       //{1958 /* byteale */, PXFOP_LE},
+       //{1960 /* byteage */, PXFOP_GE},
+       //{1956 /* byteane */, PXFOP_NE}
 
 };
 
@@ -153,72 +186,102 @@ Oid pxf_supported_types[] =
        CHAROID,
        BYTEAOID,
        BOOLOID,
-       DATEOID
+       DATEOID,
+       TIMESTAMPOID
 };
 
+static void
+pxf_free_expression_items_list(List *expressionItems, bool freeBoolExprNodes)
+{
+       ListCell                *lc     = NULL;
+       ExpressionItem  *expressionItem = NULL;
+       int previousLength;
+
+       while (list_length(expressionItems) > 0)
+       {
+               expressionItem = (ExpressionItem *) 
lfirst(list_head(expressionItems));
+               if (freeBoolExprNodes && nodeTag(expressionItem->node) == 
T_BoolExpr)
+               {
+                       pfree((BoolExpr *)expressionItem->node);
+               }
+               pfree(expressionItem);
+
+               /* to avoid freeing already freed items - delete all 
occurrences of current expression*/
+               previousLength = expressionItems->length + 1;
+               while (expressionItems != NULL && previousLength > 
expressionItems->length)
+               {
+                       previousLength = expressionItems->length;
+                       expressionItems = list_delete_ptr(expressionItems, 
expressionItem);
+               }
+       }
+}
+
 /*
- * pxf_make_filter_list
+ * pxf_make_expression_items_list
  *
  * Given a scan node qual list, find the filters that are eligible to be used
- * by PXF, construct a PxfFilterDesc list that describes the filter 
information,
+ * by PXF, construct an expressions list, which consists of OpExpr or BoolExpr 
nodes
  * and return it to the caller.
  *
- * Caller is responsible for pfreeing the returned PxfFilterDesc List.
+ * Basically this function just transforms expression tree to Reversed Polish 
Notation list.
+ *
+ *
  */
 static List *
-pxf_make_filter_list(List *quals)
+pxf_make_expression_items_list(List *quals, Node *parent, bool *logicalOpsNum)
 {
+       ExpressionItem *expressionItem = NULL;
        List                    *result = NIL;
        ListCell                *lc = NULL;
+       ListCell                *ilc = NULL;
        
        if (list_length(quals) == 0)
                return NIL;
 
-       /*
-        * Iterate over all implicitly ANDed qualifiers and add the ones
-        * that are supported for push-down into the result filter list.
-        */
        foreach (lc, quals)
        {
                Node *node = (Node *) lfirst(lc);
                NodeTag tag = nodeTag(node);
+               expressionItem = (ExpressionItem *) 
palloc0(sizeof(ExpressionItem));
+               expressionItem->node = node;
+               expressionItem->parent = parent;
+               expressionItem->processed = false;
 
                switch (tag)
                {
                        case T_OpExpr:
                        {
-                               OpExpr                  *expr   = (OpExpr *) 
node;
-                               PxfFilterDesc   *filter;
-
-                               filter = (PxfFilterDesc *) 
palloc0(sizeof(PxfFilterDesc));
-                               elog(DEBUG5, "pxf_make_filter_list: node tag %d 
(T_OpExpr)", tag);
-
-                               if (opexpr_to_pxffilter(expr, filter))
-                                       result = lappend(result, filter);
-                               else
-                                       pfree(filter);
-
+                               result = lappend(result, expressionItem);
                                break;
                        }
                        case T_BoolExpr:
                        {
+                               (*logicalOpsNum)++;
                                BoolExpr        *expr = (BoolExpr *) node;
-                               BoolExprType boolType = expr->boolop;
-                               elog(DEBUG5, "pxf_make_filter_list: node tag %d 
(T_BoolExpr), bool node type %d %s",
-                                               tag, boolType, 
boolType==AND_EXPR ? "(AND_EXPR)" : "");
+                               List *inner_result = 
pxf_make_expression_items_list(expr->args, node, logicalOpsNum);
+                               result = list_concat(result, inner_result);
+
+                               int childNodesNum = 0;
 
-                               /* only AND_EXPR is supported */
-                               if (expr->boolop == AND_EXPR)
+                               /* Find number of child nodes on first level*/
+                               foreach (ilc, inner_result)
                                {
-                                       List *inner_result = 
pxf_make_filter_list(expr->args);
-                                       elog(DEBUG5, "pxf_make_filter_list: 
inner result size %d", list_length(inner_result));
-                                       result = list_concat(result, 
inner_result);
+                                       ExpressionItem *ei = (ExpressionItem *) 
lfirst(ilc);
+                                       if (!ei->processed && ei->parent == 
node)
+                                       {
+                                               ei->processed = true;
+                                               childNodesNum++;
+                                       }
+                               }
+
+                               for (int i = 0; i < childNodesNum - 1; i++)
+                               {
+                                       result = lappend(result, 
expressionItem);
                                }
                                break;
                        }
                        default:
-                               /* expression not supported. ignore */
-                               elog(DEBUG5, "pxf_make_filter_list: unsupported 
node tag %d", tag);
+                               elog(DEBUG1, "pxf_make_expression_items_list: 
unsupported node tag %d", tag);
                                break;
                }
        }
@@ -249,33 +312,9 @@ pxf_free_filter(PxfFilterDesc* filter)
 }
 
 /*
- * pxf_free_filter_list
- *
- * free all memory associated with the filters once no longer needed.
- * alternatively we could have allocated them in a shorter lifespan
- * memory context, however explicitly freeing them is easier and makes
- * more sense.
- */
-static void
-pxf_free_filter_list(List *filters)
-{
-       ListCell                *lc     = NULL;
-       PxfFilterDesc   *filter = NULL;
-
-       if (list_length(filters) == 0)
-               return;
-
-       foreach (lc, filters)
-       {
-               filter  = (PxfFilterDesc *) lfirst(lc);
-               pxf_free_filter(filter);
-       }
-}
-
-/*
  * pxf_serialize_filter_list
  *
- * Given a list of implicitly ANDed PxfFilterDesc objects, produce a
+ * Takes expression items list in RPN notation, produce a
  * serialized string representation in order to communicate this list
  * over the wire.
  *
@@ -287,9 +326,7 @@ pxf_free_filter_list(List *filters)
  *
  * Example filter list:
  *
- * Column(0) > 1
- * Column(0) < 5
- * Column(2) == "third"
+ * Column(0) > 1 AND Column(0) < 5 AND Column(2) == "third"
  *
  * Yields the following serialized string:
  *
@@ -297,72 +334,86 @@ pxf_free_filter_list(List *filters)
  *
  */
 static char *
-pxf_serialize_filter_list(List *filters)
+pxf_serialize_filter_list(List *expressionItems)
 {
+
        StringInfo       resbuf;
-       StringInfo       curbuf;
        ListCell        *lc = NULL;
 
-       if (list_length(filters) == 0)
+       if (list_length(expressionItems) == 0)
                return NULL;
 
        resbuf = makeStringInfo();
        initStringInfo(resbuf);
-       curbuf = makeStringInfo();
-       initStringInfo(curbuf);
 
        /*
-        * Iterate through the filters in the list and serialize them one after
-        * the other. We use buffer copying because it's clear. Considering the
-        * typical small number of memcpy's this generates overall, there's no
-        * point in optimizing, better keep it clear.
+        * Iterate through the expression items in the list and serialize them 
one after the other.
         */
-       foreach (lc, filters)
+       foreach (lc, expressionItems)
        {
-               PxfFilterDesc           *filter = (PxfFilterDesc *) lfirst(lc);
-               PxfOperand                       l              = filter->l;
-               PxfOperand                       r              = filter->r;
-               PxfOperatorCode  o              = filter->op;
-
-               /* last result is stored in 'oldbuf'. start 'curbuf' clean */
-               resetStringInfo(curbuf);
-
-               /* format the operands */
-               if (pxfoperand_is_attr(l) && pxfoperand_is_const(r))
-               {
-                       appendStringInfo(curbuf, "%c%d%c%s",
-                                                                        
PXF_ATTR_CODE, l.attnum - 1, /* Java attrs are 0-based */
-                                                                        
PXF_CONST_CODE, (r.conststr)->data);
-               }
-               else if (pxfoperand_is_const(l) && pxfoperand_is_attr(r))
-               {
-                       appendStringInfo(curbuf, "%c%s%c%d",
-                                                                        
PXF_CONST_CODE, (l.conststr)->data,
-                                                                        
PXF_ATTR_CODE, r.attnum - 1); /* Java attrs are 0-based */
-               }
-               else
-               {
-                       /* pxf_make_filter_list() should have never let this 
happen */
-                       ereport(ERROR,
-                                       (errcode(ERRCODE_INTERNAL_ERROR),
-                                        errmsg("internal error in 
pxffilters.c:pxf_serialize_"
-                                                        "filter_list. Found a 
non const+attr filter")));
-               }
-
-               /* format the operator */
-               appendStringInfo(curbuf, "%c%d", PXF_OPERATOR_CODE, o);
-
-               /* append this result to the previous result */
-               appendBinaryStringInfo(resbuf, curbuf->data, curbuf->len);
+               ExpressionItem *expressionItem = (ExpressionItem *) lfirst(lc);
+               Node *node = expressionItem->node;
+               NodeTag tag = nodeTag(node);
 
-               /* if there was a previous result, append a trailing AND 
operator */
-               if(resbuf->len > curbuf->len)
+               switch (tag)
                {
-                       appendStringInfo(resbuf, "%c%d", PXF_OPERATOR_CODE, 
PXFOP_AND);
+                       case T_OpExpr:
+                       {
+                               elog(DEBUG1, "pxf_serialize_filter_list: node 
tag %d (T_OpExpr)", tag);
+                               PxfFilterDesc *filter = (PxfFilterDesc *) 
palloc0(sizeof(PxfFilterDesc));
+                               OpExpr *expr = (OpExpr *) node;
+                               if (opexpr_to_pxffilter(expr, filter))
+                               {
+                                       PxfOperand l = filter->l;
+                                       PxfOperand r = filter->r;
+                                       PxfOperatorCode o = filter->op;
+                                       if (pxfoperand_is_attr(l) && 
pxfoperand_is_const(r))
+                                       {
+                                               appendStringInfo(resbuf, 
"%c%d%c%s",
+                                                                               
                 PXF_ATTR_CODE, l.attnum - 1, /* Java attrs are 0-based */
+                                                                               
                 PXF_CONST_CODE, (r.conststr)->data);
+                                       }
+                                       else if (pxfoperand_is_const(l) && 
pxfoperand_is_attr(r))
+                                       {
+                                               appendStringInfo(resbuf, 
"%c%s%c%d",
+                                                                               
                 PXF_CONST_CODE, (l.conststr)->data,
+                                                                               
                 PXF_ATTR_CODE, r.attnum - 1); /* Java attrs are 0-based */
+                                       }
+                                       else
+                                       {
+                                               /* opexpr_to_pxffilter() should 
have never let this happen */
+                                               ereport(ERROR,
+                                                               
(errcode(ERRCODE_INTERNAL_ERROR),
+                                                                
errmsg("internal error in pxffilters.c:pxf_serialize_"
+                                                                               
 "filter_list. Found a non const+attr filter")));
+                                       }
+                                       appendStringInfo(resbuf, "%c%d", 
PXF_OPERATOR_CODE, o);
+                                       pxf_free_filter(filter);
+                               } else {
+                                       /* if at least one expression item is 
not supported, whole filter doesn't make sense*/
+                                       elog(DEBUG1, "Query will not be 
optimized to use filter push-down.");
+                                       pfree(filter);
+                                       pfree(resbuf->data);
+                                       return NULL;
+                               }
+                               break;
+                       }
+                       case T_BoolExpr:
+                       {
+                               BoolExpr *expr = (BoolExpr *) node;
+                               BoolExprType boolType = expr->boolop;
+                               elog(DEBUG1, "pxf_serialize_filter_list: node 
tag %d (T_BoolExpr), bool node type %d", tag, boolType);
+                               appendStringInfo(resbuf, "%c%d", 
PXF_LOGICAL_OPERATOR_CODE, boolType);
+                               break;
+                       }
                }
        }
 
-       pfree(curbuf->data);
+       if (resbuf->len == 0)
+       {
+               pfree(resbuf->data);
+               return NULL;
+       }
 
        return resbuf->data;
 }
@@ -395,12 +446,12 @@ opexpr_to_pxffilter(OpExpr *expr, PxfFilterDesc *filter)
        /* only binary oprs supported currently */
        if (!rightop)
        {
-               elog(DEBUG5, "opexpr_to_pxffilter: unary op! leftop_type: %d, 
op: %d",
+               elog(DEBUG1, "opexpr_to_pxffilter: unary op! leftop_type: %d, 
op: %d",
                         leftop_type, expr->opno);
                return false;
        }
 
-       elog(DEBUG5, "opexpr_to_gphdfilter: leftop (expr type: %d, arg type: 
%d), "
+       elog(DEBUG1, "opexpr_to_gphdfilter: leftop (expr type: %d, arg type: 
%d), "
                        "rightop_type (expr type: %d, arg type %d), op: %d",
                        leftop_type, nodeTag(leftop),
                        rightop_type, nodeTag(rightop),
@@ -441,6 +492,7 @@ opexpr_to_pxffilter(OpExpr *expr, PxfFilterDesc *filter)
        }
        else
        {
+               elog(DEBUG1, "opexpr_to_pxffilter: expression is not a 
Var+Const");
                return false;
        }
 
@@ -456,9 +508,10 @@ opexpr_to_pxffilter(OpExpr *expr, PxfFilterDesc *filter)
                }
        }
 
+       elog(DEBUG1, "opexpr_to_pxffilter: operator is not supported, operator 
code: %d", expr->opno);
+
        /* NOTE: if more validation needed, add it before the operators test
         * or alternatively change it to use a false flag and return true below 
*/
-
        return false;
 }
 
@@ -540,6 +593,9 @@ supported_filter_type(Oid type)
                if (type == pxf_supported_types[i])
                        return true;
        }
+
+       elog(DEBUG1, "supported_filter_type: filter pushdown is not supported 
for datatype oid: %d", type);
+
        return false;
 }
 
@@ -585,6 +641,7 @@ const_to_str(Const *constval, StringInfo buf)
                case CHAROID:
                case BYTEAOID:
                case DATEOID:
+               case TIMESTAMPOID:
                        appendStringInfo(buf, "\\\"%s\\\"", extval);
                        break;
 
@@ -626,16 +683,53 @@ char *serializePxfFilterQuals(List *quals)
 
        if (pxf_enable_filter_pushdown)
        {
-               List *filters = pxf_make_filter_list(quals);
 
-               result  = pxf_serialize_filter_list(filters);
-               pxf_free_filter_list(filters);
+               int logicalOpsNum = 0;
+               List *expressionItems = pxf_make_expression_items_list(quals, 
NULL, &logicalOpsNum);
+
+               //Trivial expression means list of OpExpr implicitly ANDed
+               bool isTrivialExpression = logicalOpsNum == 0 && 
expressionItems && expressionItems->length > 1;
+
+               if (isTrivialExpression)
+               {
+                       enrich_trivial_expression(expressionItems);
+               }
+               result  = pxf_serialize_filter_list(expressionItems);
+               pxf_free_expression_items_list(expressionItems, 
isTrivialExpression);
        }
-       elog(DEBUG2, "serializePxfFilterQuals: filter result: %s", (result == 
NULL) ? "null" : result);
+
+
+       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
+ *
+ */
+void enrich_trivial_expression(List *expressionItems) {
+
+
+       int logicalOpsNumNeeded = expressionItems->length - 1;
+
+       if (logicalOpsNumNeeded > 0)
+       {
+               ExpressionItem *andExpressionItem = (ExpressionItem *) 
palloc0(sizeof(ExpressionItem));
+               BoolExpr *andExpr = makeNode(BoolExpr);
+
+               andExpr->boolop = AND_EXPR;
+
+               andExpressionItem->node = andExpr;
+               andExpressionItem->parent = NULL;
+               andExpressionItem->processed = false;
+
+               for (int i = 0; i < logicalOpsNumNeeded; i++) {
+                       expressionItems = lappend(expressionItems, 
andExpressionItem);
+               }
+       }
+}
 
 /*
  * Returns a list of attributes, extracted from quals.

http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/7f3658d3/src/backend/access/external/test/pxffilters_test.c
----------------------------------------------------------------------
diff --git a/src/backend/access/external/test/pxffilters_test.c 
b/src/backend/access/external/test/pxffilters_test.c
index 7dc3aec..76be330 100644
--- a/src/backend/access/external/test/pxffilters_test.c
+++ b/src/backend/access/external/test/pxffilters_test.c
@@ -65,7 +65,7 @@ test__supported_filter_type(void **state)
 
        /* go over pxf_supported_types array */
        int nargs = sizeof(pxf_supported_types) / sizeof(Oid);
-       assert_int_equal(nargs, 13);
+       assert_int_equal(nargs, 14);
        for (i = 0; i < nargs; ++i)
        {
                assert_true(supported_filter_type(pxf_supported_types[i]));
@@ -323,6 +323,21 @@ OpExpr* build_op_expr(void* left, void* right, int op)
        return expr;
 }
 
+ExpressionItem* build_expression_item(int lattnum, Oid lattrtype, char* 
rconststr, Oid rattrtype, int op) {
+
+       ExpressionItem *expressionItem = (ExpressionItem*) 
palloc0(sizeof(ExpressionItem));
+
+       Var *leftop = build_var(lattrtype, lattnum);
+       Const *rightop = build_const(rattrtype, strdup(rconststr));
+       OpExpr *operationExpression = build_op_expr(leftop, rightop, op);
+
+       expressionItem->node = operationExpression;
+       expressionItem->processed = false;
+       expressionItem->parent = NULL;
+
+       return expressionItem;
+}
+
 void run__opexpr_to_pxffilter__positive(Oid dbop, PxfOperatorCode 
expectedPxfOp)
 {
        PxfFilterDesc *filter = (PxfFilterDesc*) palloc0(sizeof(PxfFilterDesc));
@@ -475,91 +490,50 @@ test__opexpr_to_pxffilter__unsupportedOpNot(void **state)
        pfree(expr);
 }
 
-void
-test__pxf_serialize_filter_list__oneFilter(void **state)
-{
-       List* filter_list = NIL;
+void test__pxf_serialize_filter_list__oneFilter(void **state) {
 
-       PxfFilterDesc* filter = build_filter(
-                       PXF_ATTR_CODE, 1, NULL,
-                       PXF_CONST_CODE, 0, "1984",
-                       PXFOP_GT);
-       filter_list = lappend(filter_list, filter);
+       List* expressionItems = NIL;
 
-       char* result = pxf_serialize_filter_list(filter_list);
-       assert_string_equal(result, "a0c1984o2");
+       ExpressionItem* filterExpressionItem = build_expression_item(1, 
TEXTOID, "1984", TEXTOID, TextEqualOperator);
 
-       pxf_free_filter_list(filter_list);
-       filter_list = NIL;
-       pfree(result);
-
-       filter = build_filter(
-                       PXF_ATTR_CODE, 8, NULL,
-                       PXF_CONST_CODE, 0, "\"George Orwell\"",
-                       PXFOP_EQ);
-       filter_list = lappend(filter_list, filter);
+       expressionItems = lappend(expressionItems, filterExpressionItem);
 
-       result = pxf_serialize_filter_list(filter_list);
-       assert_string_equal(result, "a7c\"George Orwell\"o5");
+       char* result = pxf_serialize_filter_list(expressionItems);
+       assert_string_equal(result, "a0c\\\"1984\\\"o5");
 
-       pxf_free_filter_list(filter_list);
+       pxf_free_expression_items_list(expressionItems, true);
+       expressionItems = NIL;
        pfree(result);
+
 }
 
 void
 test__pxf_serialize_filter_list__manyFilters(void **state)
 {
        char* result = NULL;
-       List* filter_list = NIL;
+       List* expressionItems = NIL;
 
-       PxfFilterDesc* filter1 = build_filter(
-                       PXF_ATTR_CODE, 2, NULL,
-                       PXF_CONST_CODE, 0, "1983",
-                       PXFOP_GT);
-       PxfFilterDesc* filter2 = build_filter(
-                       PXF_ATTR_CODE, 3, NULL,
-                       PXF_CONST_CODE, 0, "1985",
-                       PXFOP_LT);
-       PxfFilterDesc* filter3 = build_filter(
-                       PXF_ATTR_CODE, 4, NULL,
-                       PXF_CONST_CODE, 0, "\"George Orwell\"",
-                       PXFOP_EQ);
-       PxfFilterDesc* filter4 = build_filter(
-                       PXF_ATTR_CODE, 5, NULL,
-                       PXF_CONST_CODE, 0, "\"Winston\"",
-                       PXFOP_GE);
-       PxfFilterDesc* filter5 = build_filter(
-                       PXF_ATTR_CODE, 6, NULL,
-                       PXF_CONST_CODE, 0, "\"Eric-%\"",
-                       PXFOP_LIKE);
-
-       filter_list = lappend(filter_list, filter1);
-       filter_list = lappend(filter_list, filter2);
-
-       result = pxf_serialize_filter_list(filter_list);
-       assert_string_equal(result, "a1c1983o2a2c1985o1o7");
-       pfree(result);
-
-       filter_list = lappend(filter_list, filter3);
+       ExpressionItem* expressionItem1 = build_expression_item(1, TEXTOID, 
"1984", TEXTOID, TextEqualOperator);
+       ExpressionItem* expressionItem2 = build_expression_item(2, TEXTOID, 
"\"George Orwell\"", TEXTOID, TextEqualOperator);
+       ExpressionItem* expressionItem3 = build_expression_item(3, TEXTOID, 
"\"Winston\"", TEXTOID, TextEqualOperator);
+       ExpressionItem* expressionItem4 = build_expression_item(4, TEXTOID, 
"\"Eric-%\"", TEXTOID, 1209);
 
-       result = pxf_serialize_filter_list(filter_list);
-       assert_string_equal(result, "a1c1983o2a2c1985o1o7a3c\"George 
Orwell\"o5o7");
-       pfree(result);
 
-       filter_list = lappend(filter_list, filter4);
+       expressionItems = lappend(expressionItems, expressionItem1);
+       expressionItems = lappend(expressionItems, expressionItem2);
+       expressionItems = lappend(expressionItems, expressionItem3);
+       expressionItems = lappend(expressionItems, expressionItem4);
 
-       result = pxf_serialize_filter_list(filter_list);
-       assert_string_equal(result, "a1c1983o2a2c1985o1o7a3c\"George 
Orwell\"o5o7a4c\"Winston\"o4o7");
+       result = pxf_serialize_filter_list(expressionItems);
+       assert_string_equal(result, "a0c\\\"1984\\\"o5a1c\\\"\"George 
Orwell\"\\\"o5a2c\\\"\"Winston\"\\\"o5a3c\\\"\"Eric-%\"\\\"o7");
        pfree(result);
 
-       filter_list = lappend(filter_list, filter5);
+       enrich_trivial_expression(expressionItems);
 
-       result = pxf_serialize_filter_list(filter_list);
-       assert_string_equal(result, "a1c1983o2a2c1985o1o7a3c\"George 
Orwell\"o5o7a4c\"Winston\"o4o7a5c\"Eric-%\"o8o7");
-       pfree(result);
+       assert_int_equal(expressionItems->length, 7);
 
-       pxf_free_filter_list(filter_list);
-       filter_list = NIL;
+       pxf_free_expression_items_list(expressionItems, true);
+       expressionItems = NIL;
 }
 
 int 

http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/7f3658d3/src/include/access/pxffilters.h
----------------------------------------------------------------------
diff --git a/src/include/access/pxffilters.h b/src/include/access/pxffilters.h
index f54c47c..894337b 100644
--- a/src/include/access/pxffilters.h
+++ b/src/include/access/pxffilters.h
@@ -44,7 +44,6 @@ typedef enum PxfOperatorCode
        PXFOP_GE,
        PXFOP_EQ,
        PXFOP_NE,
-       PXFOP_AND,
        PXFOP_LIKE
 
 } PxfOperatorCode;
@@ -57,6 +56,7 @@ typedef enum PxfOperatorCode
 #define PXF_ATTR_CODE          'a'
 #define PXF_CONST_CODE         'c'
 #define PXF_OPERATOR_CODE      'o'
+#define PXF_LOGICAL_OPERATOR_CODE      'l'
 
 /*
  * An Operand has any of the above codes, and the information specific to
@@ -92,6 +92,14 @@ typedef struct dbop_pxfop_map
 
 } dbop_pxfop_map;
 
+
+typedef struct ExpressionItem
+{
+       Node    *node;
+       Node    *parent;
+       bool    processed;
+} ExpressionItem;
+
 static inline bool pxfoperand_is_attr(PxfOperand x)
 {
        return (x.opcode == PXF_ATTR_CODE);

Reply via email to