You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hawq.apache.org by shivzone <gi...@git.apache.org> on 2016/10/19 22:51:55 UTC
[GitHub] incubator-hawq pull request #968: HAWQ 1116
GitHub user shivzone opened a pull request:
https://github.com/apache/incubator-hawq/pull/968
HAWQ 1116
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/shivzone/incubator-hawq HAWQ-1116
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-hawq/pull/968.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 #968
----
commit a462318c286dd603a99458bd20c73dcbf9dd2a2a
Author: Shivram Mani <sh...@gmail.com>
Date: 2016-10-19T21:15:15Z
HAWQ-1116. Support isNull and isNotNull in HAWQ/PXF bridge
commit bb1ba763af8d5ffeb1328556492278e508284c26
Author: Shivram Mani <sh...@gmail.com>
Date: 2016-10-19T22:50:57Z
HAWQ-1116. Support isNull and isNotNull in HAWQ/PXF bridge PPD
----
---
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 #968: HAWQ 1116
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/968#discussion_r84389764
--- Diff: src/backend/access/external/test/pxffilters_test.c ---
@@ -323,10 +323,32 @@ 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) {
+NullTest* build_null_expr(void* arg, NullTestType nullType)
--- End diff --
Should we have Expr *arg instead of void* arg?
---
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 #968: HAWQ 1116
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/968#discussion_r84521841
--- Diff: src/backend/access/external/pxffilters.c ---
@@ -406,6 +406,29 @@ pxf_serialize_filter_list(List *expressionItems)
appendStringInfo(resbuf, "%c%d", PXF_LOGICAL_OPERATOR_CODE, boolType);
break;
}
+ case T_NullTest:
+ {
+ elog(DEBUG1, "pxf_serialize_filter_list: node tag %d (T_NullTest)", tag);
+ NullTest *expr = (NullTest *) node;
+
+ /* filter expression for T_NullTest will not have any constant value */
+ if (expr->nulltesttype == IS_NULL)
+ {
+ appendStringInfo(resbuf, "%c%d%c%d", PXF_ATTR_CODE, ((Var *) expr->arg)->varattno - 1, PXF_OPERATOR_CODE, PXFOP_IS_NULL);
+ }
+ else if (expr->nulltesttype == IS_NOT_NULL)
+ {
+ appendStringInfo(resbuf, "%c%d%c%d", PXF_ATTR_CODE, ((Var *) expr->arg)->varattno - 1, PXF_OPERATOR_CODE, PXFOP_IS_NOTNULL);
+ }
+ else
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_INTERNAL_ERROR),
+ errmsg("internal error in pxffilters.c:pxf_serialize_"
+ "filter_list. Found a NullTest filter with incorrect opcode")));
--- End diff --
Incorrect NullTestType instead of opcode?
---
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 #968: HAWQ 1116
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/968#discussion_r84387054
--- Diff: src/backend/access/external/pxffilters.c ---
@@ -406,6 +406,22 @@ pxf_serialize_filter_list(List *expressionItems)
appendStringInfo(resbuf, "%c%d", PXF_LOGICAL_OPERATOR_CODE, boolType);
break;
}
+ case T_NullTest:
+ {
+ elog(DEBUG1, "pxf_serialize_filter_list: node tag %d (T_NullTest)", tag);
+ NullTest *expr = (NullTest *) node;
+
+ /* filter expression for T_NullTest will not have any constant value */
+ if (expr->nulltesttype == IS_NULL)
--- End diff --
good point
---
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 #968: HAWQ 1116
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/968#discussion_r84334739
--- Diff: src/backend/access/external/pxffilters.c ---
@@ -406,6 +406,22 @@ pxf_serialize_filter_list(List *expressionItems)
appendStringInfo(resbuf, "%c%d", PXF_LOGICAL_OPERATOR_CODE, boolType);
break;
}
+ case T_NullTest:
+ {
+ elog(DEBUG1, "pxf_serialize_filter_list: node tag %d (T_NullTest)", tag);
+ NullTest *expr = (NullTest *) node;
+
+ /* filter expression for T_NullTest will not have any constant value */
+ if (expr->nulltesttype == IS_NULL)
--- End diff --
Maybe put this into a conditional statement instead of an if/else block. But it's fine as is too
```
op = expr->nulltestype == IS_NULL ? PXFOP_IS_NULL : PXFOP_IS_NOTNULL;
appendStringInfo(resbuf, "%c%d%c%d", PXF_ATTR_CODE, ((Var *) expr->arg)->varattno - 1, PXF_OPERATOR_CODE, op);
```
---
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 #968: HAWQ 1116
Posted by shivzone <gi...@git.apache.org>.
Github user shivzone closed the pull request at:
https://github.com/apache/incubator-hawq/pull/968
---
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 #968: HAWQ 1116
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/968#discussion_r84333314
--- Diff: src/backend/access/external/test/pxffilters_test.c ---
@@ -323,6 +323,30 @@ OpExpr* build_op_expr(void* left, void* right, int op)
return expr;
}
+NullTest* build_null_expr(void* arg, NullTestType nullType)
+{
+ NullTest *expr = (NullTest*) palloc0(sizeof(NullTest));
+ expr->nulltesttype = nullType;
+ expr->arg = arg;
+
+ expr->xpr.type = T_NullTest;
+ return expr;
+}
+
+ExpressionItem* build_null_expression_item(int attnum, Oid attrtype, NullTestType nullType) {
+
--- End diff --
style: the '{' should be on a new line
---
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 #968: HAWQ 1116
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/968#discussion_r84333580
--- Diff: src/backend/access/external/test/pxffilters_test.c ---
@@ -507,6 +542,23 @@ void test__pxf_serialize_filter_list__oneFilter(void **state) {
}
+void test__pxf_serialize_fillter_list__nullFilter(void **state) {
+
--- End diff --
brace on newline
---
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 #968: HAWQ 1116
Posted by denalex <gi...@git.apache.org>.
Github user denalex commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/968#discussion_r84383699
--- Diff: src/backend/access/external/pxffilters.c ---
@@ -406,6 +406,22 @@ pxf_serialize_filter_list(List *expressionItems)
appendStringInfo(resbuf, "%c%d", PXF_LOGICAL_OPERATOR_CODE, boolType);
break;
}
+ case T_NullTest:
+ {
+ elog(DEBUG1, "pxf_serialize_filter_list: node tag %d (T_NullTest)", tag);
+ NullTest *expr = (NullTest *) node;
+
+ /* filter expression for T_NullTest will not have any constant value */
+ if (expr->nulltesttype == IS_NULL)
--- End diff --
+1 -- 2 lines are better than 8 and there is less repetition
---
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 #968: HAWQ 1116
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/968#discussion_r84389149
--- Diff: src/backend/access/external/pxffilters.c ---
@@ -406,6 +406,16 @@ pxf_serialize_filter_list(List *expressionItems)
appendStringInfo(resbuf, "%c%d", PXF_LOGICAL_OPERATOR_CODE, boolType);
break;
}
+ case T_NullTest:
+ {
+ elog(DEBUG1, "pxf_serialize_filter_list: node tag %d (T_NullTest)", tag);
+ NullTest *expr = (NullTest *) node;
+
+ /* filter expression for T_NullTest will not have any constant value */
+ PxfOperatorCode o = (expr->nulltesttype == IS_NULL) ? PXFOP_IS_NULL : PXFOP_IS_NOTNULL;
--- End diff --
It would be safer to explicitly check whether expr->nulltesttype == IS_NULL or expr->nulltesttype == IS_NOTNULL instead of relying to default case. As an example in exising codebase: https://github.com/apache/incubator-hawq/blob/master/src/backend/executor/execQual.c#L3828
---
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 #968: HAWQ 1116
Posted by shivzone <gi...@git.apache.org>.
Github user shivzone closed the pull request at:
https://github.com/apache/incubator-hawq/pull/968
---
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 #968: HAWQ 1116
Posted by shivzone <gi...@git.apache.org>.
GitHub user shivzone reopened a pull request:
https://github.com/apache/incubator-hawq/pull/968
HAWQ 1116
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/shivzone/incubator-hawq HAWQ-1116
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-hawq/pull/968.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 #968
----
commit bc0ac446c1d970635fcd11378f7fee37aa9ff670
Author: Shivram Mani <sh...@gmail.com>
Date: 2016-10-24T23:18:08Z
HAWQ-1116. Support isNull and isNotNull in HAWQ/PXF bridge PPD
commit fd350101a612a9c38c83573fff9cc5d7843a0cf6
Author: Alexander Denissov <ad...@pivotal.io>
Date: 2016-10-25T18:53:44Z
HAWQ-963. PXF support for IS_NULL and IS_NOT_NULL filters
(close #974)
commit 01866e79882ec478a0e4441bf499000e83df13f5
Author: Alexander Denissov <ad...@pivotal.io>
Date: 2016-10-25T21:51:45Z
HAWQ-963. Updated unit test
commit 107155184dc4190f844cbff962b937ee5dfa4ae1
Author: Shivram Mani <sh...@gmail.com>
Date: 2016-10-24T23:18:08Z
HAWQ-1116. Support isNull and isNotNull in HAWQ/PXF bridge PPD
commit 14f74a1db7a30b02d2de35456acfc8448e8d9416
Author: Shivram Mani <sh...@gmail.com>
Date: 2016-10-26T16:20:46Z
HAWQ-1116. Handle null constant
commit 56f5ab73c2606dda49063f5214fc2f1ef45e71cf
Author: Shivram Mani <sh...@gmail.com>
Date: 2016-10-26T16:20:53Z
Merge branch 'HAWQ-1116' of https://github.com/shivzone/incubator-hawq into HAWQ-1116
commit 4665a9a2d9c5146eae15215b5e96b836a6f5cf2c
Author: Shivram Mani <sh...@gmail.com>
Date: 2016-10-26T21:17:01Z
HAWQ-1116. Fix hawq pxf manfilters unittest
----
---
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.
---