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/13 23:47:39 UTC
[GitHub] incubator-hawq pull request #962: HAWQ-1103. Send constant datatype and leng...
GitHub user sansanichfb opened a pull request:
https://github.com/apache/incubator-hawq/pull/962
HAWQ-1103. Send constant datatype and length in filter to PXF.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sansanichfb/incubator-hawq HAWQ-1103
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-hawq/pull/962.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 #962
----
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.
----
---
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 #962: HAWQ-1103. Send constant datatype and leng...
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/962#discussion_r83952535
--- Diff: src/backend/access/external/pxffilters.c ---
@@ -369,14 +369,18 @@ pxf_serialize_filter_list(List *expressionItems)
PxfOperatorCode o = filter->op;
if (pxfoperand_is_attr(l) && pxfoperand_is_const(r))
{
- appendStringInfo(resbuf, "%c%d%c%s",
+ appendStringInfo(resbuf, "%c%d%c%d%c%d%c%s",
--- End diff --
Update the Function comments in line 333 to reflect the new pattern.
---
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 #962: HAWQ-1103. Send constant datatype and leng...
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/962#discussion_r83953158
--- Diff: src/backend/access/external/pxffilters.c ---
@@ -369,14 +369,18 @@ pxf_serialize_filter_list(List *expressionItems)
PxfOperatorCode o = filter->op;
if (pxfoperand_is_attr(l) && pxfoperand_is_const(r))
{
- appendStringInfo(resbuf, "%c%d%c%s",
+ appendStringInfo(resbuf, "%c%d%c%d%c%d%c%s",
PXF_ATTR_CODE, l.attnum - 1, /* Java attrs are 0-based */
- PXF_CONST_CODE, (r.conststr)->data);
+ PXF_CONST_CODE, r.consttype,
+ PXF_SIZE_BYTES, strlen(r.conststr->data),
--- End diff --
strlen would return the size without the null termination character. Is it safe to use strlen to compute the # of bytes ?
---
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 #962: HAWQ-1103. Send constant datatype and leng...
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/962#discussion_r84553034
--- Diff: src/include/access/pxffilters.h ---
@@ -53,9 +53,11 @@ typedef enum PxfOperatorCode
* by a code that will describe the operator type in the final serialized
* string that gets pushed down.
*/
-#define PXF_ATTR_CODE 'a'
-#define PXF_CONST_CODE 'c'
-#define PXF_OPERATOR_CODE 'o'
+#define PXF_ATTR_CODE 'a'
+#define PXF_CONST_CODE 'c'
+#define PXF_SIZE_BYTES 's'
+#define PXF_CONST_DATA 'd'
+#define PXF_OPERATOR_CODE 'o'
#define PXF_LOGICAL_OPERATOR_CODE 'l'
--- End diff --
It looks aligned in IDE.
---
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 #962: HAWQ-1103. Send constant datatype and leng...
Posted by sansanichfb <gi...@git.apache.org>.
Github user sansanichfb closed the pull request at:
https://github.com/apache/incubator-hawq/pull/962
---
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 #962: HAWQ-1103. Send constant datatype and leng...
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/962#discussion_r84744589
--- Diff: src/backend/access/external/test/pxffilters_test.c ---
@@ -514,23 +514,28 @@ test__pxf_serialize_filter_list__manyFilters(void **state)
List* expressionItems = NIL;
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);
+ 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);
+ ExpressionItem* expressionItem5 = build_expression_item(5, TEXTOID, "\"Ugly\" string with quotes", TEXTOID, TextEqualOperator);
+ ExpressionItem* expressionItem6 = build_expression_item(6, TEXTOID, "", TEXTOID, TextEqualOperator);
--- End diff --
Add a test for empty byte array as well
---
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 #962: HAWQ-1103. Send constant datatype and leng...
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/962#discussion_r84547112
--- Diff: src/backend/access/external/test/pxffilters_test.c ---
@@ -514,23 +514,26 @@ test__pxf_serialize_filter_list__manyFilters(void **state)
List* expressionItems = NIL;
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);
+ 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);
+ ExpressionItem* expressionItem5 = build_expression_item(5, TEXTOID, "\"Ugly\" string with quotes", TEXTOID, TextEqualOperator);
expressionItems = lappend(expressionItems, expressionItem1);
expressionItems = lappend(expressionItems, expressionItem2);
expressionItems = lappend(expressionItems, expressionItem3);
expressionItems = lappend(expressionItems, expressionItem4);
+ expressionItems = lappend(expressionItems, expressionItem5);
result = pxf_serialize_filter_list(expressionItems);
- assert_string_equal(result, "a0c\\\"1984\\\"o5a1c\\\"\"George Orwell\"\\\"o5a2c\\\"\"Winston\"\\\"o5a3c\\\"\"Eric-%\"\\\"o7");
+ assert_string_equal(result, "a0c25s4d1984o5a1c25s13dGeorge Orwello5a2c25s7dWinstono5a3c25s6dEric-%o7a4c25s25d\"Ugly\" string with quoteso5");
--- End diff --
Sure, will add it
---
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 #962: HAWQ-1103. Send constant datatype and leng...
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/962#discussion_r84523453
--- Diff: src/backend/access/external/test/pxffilters_test.c ---
@@ -514,23 +514,26 @@ test__pxf_serialize_filter_list__manyFilters(void **state)
List* expressionItems = NIL;
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);
+ 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);
+ ExpressionItem* expressionItem5 = build_expression_item(5, TEXTOID, "\"Ugly\" string with quotes", TEXTOID, TextEqualOperator);
expressionItems = lappend(expressionItems, expressionItem1);
expressionItems = lappend(expressionItems, expressionItem2);
expressionItems = lappend(expressionItems, expressionItem3);
expressionItems = lappend(expressionItems, expressionItem4);
+ expressionItems = lappend(expressionItems, expressionItem5);
result = pxf_serialize_filter_list(expressionItems);
- assert_string_equal(result, "a0c\\\"1984\\\"o5a1c\\\"\"George Orwell\"\\\"o5a2c\\\"\"Winston\"\\\"o5a3c\\\"\"Eric-%\"\\\"o7");
+ assert_string_equal(result, "a0c25s4d1984o5a1c25s13dGeorge Orwello5a2c25s7dWinstono5a3c25s6dEric-%o7a4c25s25d\"Ugly\" string with quoteso5");
--- End diff --
Can you add a test for column = "". Just to check how we handle constant length
---
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 #962: HAWQ-1103. Send constant datatype and leng...
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/962#discussion_r84551904
--- Diff: src/backend/access/external/pxffilters.c ---
@@ -369,14 +369,18 @@ pxf_serialize_filter_list(List *expressionItems)
PxfOperatorCode o = filter->op;
if (pxfoperand_is_attr(l) && pxfoperand_is_const(r))
{
- appendStringInfo(resbuf, "%c%d%c%s",
+ appendStringInfo(resbuf, "%c%d%c%d%c%d%c%s",
PXF_ATTR_CODE, l.attnum - 1, /* Java attrs are 0-based */
- PXF_CONST_CODE, (r.conststr)->data);
+ PXF_CONST_CODE, r.consttype,
+ PXF_SIZE_BYTES, strlen(r.conststr->data),
--- End diff --
It should be fine to determine number of bytes by counting them up to null terminator character. The only caveat I could foresee if char size is different from one byte on some platforms.
---
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 #962: HAWQ-1103. Send constant datatype and leng...
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/962#discussion_r83953610
--- Diff: src/include/access/pxffilters.h ---
@@ -53,9 +53,11 @@ typedef enum PxfOperatorCode
* by a code that will describe the operator type in the final serialized
* string that gets pushed down.
*/
-#define PXF_ATTR_CODE 'a'
-#define PXF_CONST_CODE 'c'
-#define PXF_OPERATOR_CODE 'o'
+#define PXF_ATTR_CODE 'a'
+#define PXF_CONST_CODE 'c'
+#define PXF_SIZE_BYTES 's'
+#define PXF_CONST_DATA 'd'
+#define PXF_OPERATOR_CODE 'o'
#define PXF_LOGICAL_OPERATOR_CODE 'l'
--- End diff --
Define extra tab here to be alligned
---
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.
---