You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hawq.apache.org by sansanichfb <gi...@git.apache.org> on 2016/10/27 23:51:24 UTC
[GitHub] incubator-hawq pull request #981: HAWQ-1114. Implement filter-push down for ...
GitHub user sansanichfb opened a pull request:
https://github.com/apache/incubator-hawq/pull/981
HAWQ-1114. Implement filter-push down for IN on HAWQ bridge side.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sansanichfb/incubator-hawq HAWQ-1114
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-hawq/pull/981.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #981
----
commit b318fa0e7d038084ded2bc99c6e631aa26700b21
Author: Oleksandr Diachenko <od...@pivotal.io>
Date: 2016-10-13T23:00:03Z
HAWQ-1103. Draft implementation without taking care of different charsets.
commit d9f77da1db31e260092fbce603686f92540848f3
Author: Oleksandr Diachenko <od...@pivotal.io>
Date: 2016-10-13T23:45:33Z
HAWQ-1103. Updated unit-tests.
commit 2497b6f327944e1fbdc5434dddacab0fcacfa7f9
Author: Oleksandr Diachenko <od...@pivotal.io>
Date: 2016-10-14T18:45:02Z
HAWQ-1103. Added test cases with string containing a quote.
commit 354422287af2cb1af0e9465821d8ec9f6062fa10
Author: Oleksandr Diachenko <od...@pivotal.io>
Date: 2016-10-18T21:44:49Z
HAWQ-1103. Fixed warnings.
commit 0ad9e5195ae88a3f70505878c5fa658cea418701
Author: Oleksandr Diachenko <od...@pivotal.io>
Date: 2016-10-20T21:31:57Z
HAWQ-1114. Initial version.
commit 580f3c942d23f4e761080de9fd1120d3545c2bfb
Author: Oleksandr Diachenko <od...@pivotal.io>
Date: 2016-10-21T00:20:06Z
HAWQ-1114. Implemented IN operator on HAWQ bridge side.
commit 95355c55cb1a2c73b3778303b548934c7d442424
Author: Oleksandr Diachenko <od...@pivotal.io>
Date: 2016-10-21T21:29:05Z
HAWQ-1114. Fixed unit-tests.
commit 5d1e7b18a0cd676c8ddd0099b682fe3d458396dd
Author: Oleksandr Diachenko <od...@pivotal.io>
Date: 2016-10-24T19:06:49Z
HAWQ-1114. Fixed unit-tests.
commit 87355225f4e5ee89a98e9c3c22ec19e357b6c7b2
Author: Oleksandr Diachenko <od...@pivotal.io>
Date: 2016-10-24T20:21:38Z
HAWQ-1114. Fixed unit-tests.
commit 4d1f2a66e271845e3162e2ade10c59a14a823478
Author: Oleksandr Diachenko <od...@pivotal.io>
Date: 2016-10-24T20:28:47Z
Merge master into HAWQ-1114.
commit c9317c4870e8bb446c0617a8e3a18ee338811d6d
Author: Oleksandr Diachenko <od...@pivotal.io>
Date: 2016-10-24T23:01:20Z
Revert "Merge master into HAWQ-1114."
This reverts commit 4d1f2a66e271845e3162e2ade10c59a14a823478.
commit c3c91c265b5a3596034b9c8387ee25885619e5cc
Author: Oleksandr Diachenko <od...@pivotal.io>
Date: 2016-10-25T18:53:27Z
HAWQ-1114. Fixed unit-tests.
commit bc64055bb1ad4e483b0e8132da0f4b9b2e284982
Author: Oleksandr Diachenko <od...@pivotal.io>
Date: 2016-10-27T02:27:10Z
HAWQ-1114. Updated logic to use arrays instead of parsing comma-delimited string.
commit 63df933c0ff8b36010f9ce8625857af84fbc1c42
Author: Oleksandr Diachenko <od...@pivotal.io>
Date: 2016-10-27T23:50:25Z
HAWQ-1114. Updated logic to use arrays instead of parsing comma-delimited string.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request #981: HAWQ-1114. Implement filter-push down for ...
Posted by sansanichfb <gi...@git.apache.org>.
Github user sansanichfb commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/981#discussion_r88119564
--- Diff: src/backend/access/external/pxffilters.c ---
@@ -187,13 +200,18 @@ Oid pxf_supported_types[] =
BYTEAOID,
BOOLOID,
DATEOID,
- TIMESTAMPOID
+ TIMESTAMPOID,
+ /* complex datatypes*/
+ INT2ARRAYOID,
+ INT4ARRAYOID,
+ INT8ARRAYOID,
+ BOOLARRAYOID,
+ TEXTARRAYOID
--- End diff --
ANYARRAYOID is a pseudo-type which cannot be used to define table columns, but are valid as function argument and result types.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request #981: HAWQ-1114. Implement filter-push down for ...
Posted by shivzone <gi...@git.apache.org>.
Github user shivzone commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/981#discussion_r85980886
--- Diff: src/backend/access/external/pxffilters.c ---
@@ -187,13 +200,18 @@ Oid pxf_supported_types[] =
BYTEAOID,
BOOLOID,
DATEOID,
- TIMESTAMPOID
+ TIMESTAMPOID,
+ /* complex datatypes*/
+ INT2ARRAYOID,
+ INT4ARRAYOID,
+ INT8ARRAYOID,
+ BOOLARRAYOID,
+ TEXTARRAYOID
--- End diff --
Can you check what ANYARRAYOID means ? If we can support this datatype as well along with the other array datatypes
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request #981: HAWQ-1114. Implement filter-push down for ...
Posted by sansanichfb <gi...@git.apache.org>.
Github user sansanichfb closed the pull request at:
https://github.com/apache/incubator-hawq/pull/981
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request #981: HAWQ-1114. Implement filter-push down for ...
Posted by sansanichfb <gi...@git.apache.org>.
Github user sansanichfb commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/981#discussion_r88117946
--- Diff: src/backend/access/external/pxffilters.c ---
@@ -357,6 +393,46 @@ pxf_serialize_filter_list(List *expressionItems)
switch (tag)
{
+ case T_Var:
+ {
+ elog(DEBUG1, "pxf_serialize_filter_list: node tag %d (T_Var)", tag);
+ PxfFilterDesc *filter = (PxfFilterDesc *) palloc0(sizeof(PxfFilterDesc));
+ Var *var = (Var *) node;
+ if (var_to_pxffilter(var, filter))
+ {
+ PxfOperand l = filter->l;
+ PxfOperand r = filter->r;
+ PxfOperatorCode o = filter->op;
+ if (pxfoperand_is_attr(l) && pxfoperand_is_scalar_const(r))
--- End diff --
Left operand constant and right operand is attribute translates in SQL:
WHERE (1,2,3) IN c1 which has incorrect syntax and should be never the case.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request #981: HAWQ-1114. Implement filter-push down for ...
Posted by sansanichfb <gi...@git.apache.org>.
Github user sansanichfb commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/981#discussion_r88154859
--- Diff: src/backend/access/external/pxffilters.c ---
@@ -496,23 +629,116 @@ opexpr_to_pxffilter(OpExpr *expr, PxfFilterDesc *filter)
return false;
}
- /* is operator supported? if so, set the corresponding PXFOP */
- for (i = 0; i < nargs; i++)
+ return true;
+}
+
+static bool
+scalar_array_op_expr_to_pxffilter(ScalarArrayOpExpr *expr, PxfFilterDesc *filter)
+{
+
+ Node *leftop = NULL;
+ Node *rightop = NULL;
+
+ leftop = (Node *) linitial(expr->args);
+ rightop = (Node *) lsecond(expr->args);
+ Oid leftop_type = exprType(leftop);
+ Oid rightop_type = exprType(rightop);
+
+ /*
+ * check if supported type -
+ */
+ if (!supported_filter_type(rightop_type) || !supported_filter_type(leftop_type))
+ return false;
+
+ /*
+ * check if supported operator -
+ */
+ if(!supported_operator_type(expr->opno, filter))
+ return false;
+
+ if (IsA(leftop, Var) && IsA(rightop, Const))
{
- /* NOTE: switch to hash table lookup if */
- /* array grows. for now it's cheap enough */
- if(expr->opno == pxf_supported_opr[i].dbop)
- {
- filter->op = pxf_supported_opr[i].pxfop;
- return true; /* filter qualifies! */
- }
+ filter->l.opcode = PXF_ATTR_CODE;
+ filter->l.attnum = ((Var *) leftop)->varattno;
+ filter->l.consttype = InvalidOid;
+ if (filter->l.attnum <= InvalidAttrNumber)
+ return false; /* system attr not supported */
+
+ filter->r.opcode = PXF_LIST_CONST_CODE;
+ filter->r.attnum = InvalidAttrNumber;
+ filter->r.conststr = makeStringInfo();
+ list_const_to_str((Const *)rightop, filter->r.conststr);
+ filter->r.consttype = ((Const *)rightop)->consttype;
+ }
+ else if (IsA(leftop, Const) && IsA(rightop, Var))
+ {
+ filter->l.opcode = PXF_LIST_CONST_CODE;
+ filter->l.attnum = InvalidAttrNumber;
+ filter->l.conststr = makeStringInfo();
+ list_const_to_str((Const *)leftop, filter->l.conststr);
+ filter->l.consttype = ((Const *)leftop)->consttype;
+
+ filter->r.opcode = PXF_ATTR_CODE;
+ filter->r.attnum = ((Var *) rightop)->varattno;
+ filter->r.consttype = InvalidOid;
+ if (filter->r.attnum <= InvalidAttrNumber)
+ return false; /* system attr not supported */
+ }
+ else
+ {
+ elog(DEBUG1, "pxf_serialize_filter_list: expression is not a Var+Const");
+ return false;
}
- 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;
+
+ return true;
+}
+
+static bool
+var_to_pxffilter(Var *var, PxfFilterDesc *filter)
+{
+ Oid var_type = InvalidOid;
+
+ if ((!var) || (!filter))
+ return false;
+
+ var_type = exprType(var);
+
+ /*
+ * check if supported type -
+ */
+ if (!supported_filter_type(var_type))
+ return false;
+
+ /*
+ * check if supported operator -
+ */
+ if (!supported_operator_type(BooleanEqualOperator, filter))
+ return false;
+
+ /* arguments must be VAR and CONST */
+ if (IsA(var, Var))
+ {
+ filter->l.opcode = PXF_ATTR_CODE;
+ filter->l.attnum = var->varattno;
+ filter->l.consttype = InvalidOid;
--- End diff --
InvalidOid here indicates here that we use PxfOperand to store PXF_ATTR_CODE value(might be also PXF_SCALAR_CONST_CODE, PXF_LIST_CONST_CODE), and to not misuse filter->l.consttype value by mistake.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request #981: HAWQ-1114. Implement filter-push down for ...
Posted by shivzone <gi...@git.apache.org>.
Github user shivzone commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/981#discussion_r89230552
--- Diff: src/backend/access/external/pxffilters.c ---
@@ -719,6 +1029,143 @@ const_to_str(Const *constval, StringInfo buf)
/*
+ * list_const_to_str
+ *
+ */
+static void
+list_const_to_str(Const *constval, StringInfo buf)
+{
+ StringInfo interm_buf;
+ Datum *dats;
+ ArrayType *arr;
+ int len;
+
+ if (constval->constisnull)
+ {
+ elog(DEBUG1, "Null constant is not expected in this context.");
+ return;
+ }
+
+ if (constval->constbyval) {
--- End diff --
This check seems a bit unnecessary as we would never get this expression down to our code
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request #981: HAWQ-1114. Implement filter-push down for ...
Posted by kavinderd <gi...@git.apache.org>.
Github user kavinderd commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/981#discussion_r88100166
--- Diff: src/backend/access/external/pxffilters.c ---
@@ -496,23 +629,116 @@ opexpr_to_pxffilter(OpExpr *expr, PxfFilterDesc *filter)
return false;
}
- /* is operator supported? if so, set the corresponding PXFOP */
- for (i = 0; i < nargs; i++)
+ return true;
+}
+
+static bool
+scalar_array_op_expr_to_pxffilter(ScalarArrayOpExpr *expr, PxfFilterDesc *filter)
+{
+
+ Node *leftop = NULL;
+ Node *rightop = NULL;
+
+ leftop = (Node *) linitial(expr->args);
+ rightop = (Node *) lsecond(expr->args);
+ Oid leftop_type = exprType(leftop);
+ Oid rightop_type = exprType(rightop);
+
+ /*
+ * check if supported type -
+ */
+ if (!supported_filter_type(rightop_type) || !supported_filter_type(leftop_type))
+ return false;
+
+ /*
+ * check if supported operator -
+ */
+ if(!supported_operator_type(expr->opno, filter))
+ return false;
+
+ if (IsA(leftop, Var) && IsA(rightop, Const))
{
- /* NOTE: switch to hash table lookup if */
- /* array grows. for now it's cheap enough */
- if(expr->opno == pxf_supported_opr[i].dbop)
- {
- filter->op = pxf_supported_opr[i].pxfop;
- return true; /* filter qualifies! */
- }
+ filter->l.opcode = PXF_ATTR_CODE;
+ filter->l.attnum = ((Var *) leftop)->varattno;
+ filter->l.consttype = InvalidOid;
+ if (filter->l.attnum <= InvalidAttrNumber)
+ return false; /* system attr not supported */
+
+ filter->r.opcode = PXF_LIST_CONST_CODE;
+ filter->r.attnum = InvalidAttrNumber;
+ filter->r.conststr = makeStringInfo();
+ list_const_to_str((Const *)rightop, filter->r.conststr);
+ filter->r.consttype = ((Const *)rightop)->consttype;
+ }
+ else if (IsA(leftop, Const) && IsA(rightop, Var))
+ {
+ filter->l.opcode = PXF_LIST_CONST_CODE;
+ filter->l.attnum = InvalidAttrNumber;
+ filter->l.conststr = makeStringInfo();
+ list_const_to_str((Const *)leftop, filter->l.conststr);
+ filter->l.consttype = ((Const *)leftop)->consttype;
+
+ filter->r.opcode = PXF_ATTR_CODE;
+ filter->r.attnum = ((Var *) rightop)->varattno;
+ filter->r.consttype = InvalidOid;
+ if (filter->r.attnum <= InvalidAttrNumber)
+ return false; /* system attr not supported */
+ }
+ else
+ {
+ elog(DEBUG1, "pxf_serialize_filter_list: expression is not a Var+Const");
+ return false;
}
- 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;
+
+ return true;
+}
+
+static bool
+var_to_pxffilter(Var *var, PxfFilterDesc *filter)
+{
+ Oid var_type = InvalidOid;
+
+ if ((!var) || (!filter))
+ return false;
+
+ var_type = exprType(var);
+
+ /*
+ * check if supported type -
+ */
+ if (!supported_filter_type(var_type))
+ return false;
+
+ /*
+ * check if supported operator -
+ */
+ if (!supported_operator_type(BooleanEqualOperator, filter))
+ return false;
+
+ /* arguments must be VAR and CONST */
+ if (IsA(var, Var))
+ {
+ filter->l.opcode = PXF_ATTR_CODE;
+ filter->l.attnum = var->varattno;
+ filter->l.consttype = InvalidOid;
--- End diff --
Why is this `InvalidOid` is that just a default value?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request #981: HAWQ-1114. Implement filter-push down for ...
Posted by sansanichfb <gi...@git.apache.org>.
Github user sansanichfb commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/981#discussion_r89412039
--- Diff: src/backend/access/external/pxffilters.c ---
@@ -719,6 +1029,143 @@ const_to_str(Const *constval, StringInfo buf)
/*
+ * list_const_to_str
+ *
+ */
+static void
+list_const_to_str(Const *constval, StringInfo buf)
+{
+ StringInfo interm_buf;
+ Datum *dats;
+ ArrayType *arr;
+ int len;
+
+ if (constval->constisnull)
+ {
+ elog(DEBUG1, "Null constant is not expected in this context.");
+ return;
+ }
+
+ if (constval->constbyval) {
--- End diff --
This check is needed because bridge part relies on data passed by planner.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request #981: HAWQ-1114. Implement filter-push down for ...
Posted by shivzone <gi...@git.apache.org>.
Github user shivzone commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/981#discussion_r85998628
--- Diff: src/backend/access/external/pxffilters.c ---
@@ -357,6 +393,46 @@ pxf_serialize_filter_list(List *expressionItems)
switch (tag)
{
+ case T_Var:
+ {
+ elog(DEBUG1, "pxf_serialize_filter_list: node tag %d (T_Var)", tag);
+ PxfFilterDesc *filter = (PxfFilterDesc *) palloc0(sizeof(PxfFilterDesc));
+ Var *var = (Var *) node;
+ if (var_to_pxffilter(var, filter))
+ {
+ PxfOperand l = filter->l;
+ PxfOperand r = filter->r;
+ PxfOperatorCode o = filter->op;
+ if (pxfoperand_is_attr(l) && pxfoperand_is_scalar_const(r))
--- End diff --
Can we have a scenario which is left operand is scalar const and right operand is attirbute ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq issue #981: HAWQ-1114. Implement filter-push down for IN on H...
Posted by sansanichfb <gi...@git.apache.org>.
Github user sansanichfb commented on the issue:
https://github.com/apache/incubator-hawq/pull/981
@shivzone good point about PXFOP_IN, added it. we should distinguis three cases:
1) WHERE scalar_column IN (1,2,3)
2) WHERE array_column = '{1,2,3}'
2) WHERE array_column IN (1,2,3)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq issue #981: HAWQ-1114. Implement filter-push down for IN on H...
Posted by shivzone <gi...@git.apache.org>.
Github user shivzone commented on the issue:
https://github.com/apache/incubator-hawq/pull/981
Why aren't we adding PXFOP_IN to PxfOperatorCode enum ?
For the other operators, PXF service expects to see the opcode value to correspond to the PxfOperatorCode enum
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request #981: HAWQ-1114. Implement filter-push down for ...
Posted by wenxiaozxw <gi...@git.apache.org>.
Github user wenxiaozxw commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/981#discussion_r104351270
--- Diff: src/backend/access/external/pxffilters.c ---
@@ -376,6 +467,46 @@ pxf_serialize_filter_list(List *expressionItems)
switch (tag)
{
+ case T_Var:
--- End diff --
Which scenario is the expr type is T_Var?
In you comment is IN(single_value),but IN(single_value) is also T_OpExpr(Var + Const)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request #981: HAWQ-1114. Implement filter-push down for ...
Posted by shivzone <gi...@git.apache.org>.
Github user shivzone commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/981#discussion_r89229211
--- Diff: src/backend/access/external/pxffilters.c ---
@@ -719,6 +1029,143 @@ const_to_str(Const *constval, StringInfo buf)
/*
+ * list_const_to_str
--- End diff --
Please provided extended description of the purpose of this static function. It is very useful function in the context of our expression handling, so good to provide description here
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---