You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hawq.apache.org by kavinderd <gi...@git.apache.org> on 2016/09/16 22:34:43 UTC
[GitHub] incubator-hawq pull request #906: HAWQ-964. Support for OR and NOT Logical O...
GitHub user kavinderd opened a pull request:
https://github.com/apache/incubator-hawq/pull/906
HAWQ-964. Support for OR and NOT Logical Operators
Signed-off-by: Leslie Chang <hi...@gmail.com>
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kavinderd/incubator-hawq HAWQ-964
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-hawq/pull/906.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 #906
----
commit fec6e3055f6b42b5932cbbe34672282d93790e6c
Author: Kavinder Dhaliwal <ka...@gmail.com>
Date: 2016-09-15T17:56:20Z
HAWQ-964. Support for OR and NOT Logical Operators
Signed-off-by: Leslie Chang <hi...@gmail.com>
----
---
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 #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r81829765
--- Diff: pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/FilterParser.java ---
@@ -81,6 +88,8 @@
* @throws Exception if building the filter failed
*/
public Object build(Operation operation, Object left, Object right) throws Exception;
+ public Object build(LogicalOperation operation, Object left, Object right) throws Exception;
+ public Object build(LogicalOperation operation, Object filter) throws Exception;
--- End diff --
This is confusing -- a LogicalOperation can be assumed a type of Operation, but it is not -- a different Enum and no inheritance. Why do we need such a distinction ?
If we really need to keep track of whether an operation is logical or not, have a boolean flag on the enum value.
If we consolidate, we can have just one Enum, with numbers for mapping and a flag for logical (if needed). This will reduce number of functions / enums and code overall.
---
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 #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r82032792
--- Diff: pxf/pxf-hbase/src/main/java/org/apache/hawq/pxf/plugins/hbase/HBaseFilterBuilder.java ---
@@ -71,14 +74,19 @@ public HBaseFilterBuilder(HBaseTupleDescription tupleDescription) {
* @throws Exception if parsing failed
*/
public Filter getFilterObject(String filterString) throws Exception {
- FilterParser parser = new FilterParser(this);
- Object result = parser.parse(filterString);
+ // First check for NOT, HBase does not support this
+ if (filterString.contains(NOT_OP)) {
--- End diff --
Hmm, that's possible. I'll test and fix if needed
---
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 #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r79487252
--- Diff: pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/FilterParser.java ---
@@ -396,4 +387,12 @@ private Operation reverseOp(Operation operation) {
operatorTranslationMap.put(8, Operation.HDOP_LIKE);
return operatorTranslationMap;
}
+
+ static private Map<Integer, LogicalOperation> initLogicalOperatorTransMap() {
+ Map<Integer, LogicalOperation> integerLogicalOperationMap = new HashMap<>();
--- End diff --
rename this object to logicalOperatorTransloationMap to stay in sync with convnetion used in other function
---
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 #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r79488894
--- Diff: pxf/pxf-hive/src/test/java/org/apache/hawq/pxf/plugins/hive/HiveORCSearchArgumentExample.java ---
@@ -0,0 +1,84 @@
+package org.apache.hawq.pxf.plugins.hive;
+
+import org.apache.hadoop.hive.ql.io.sarg.SearchArgument;
+import org.apache.hadoop.hive.ql.io.sarg.SearchArgumentFactory;
+import org.apache.hawq.pxf.api.BasicFilter;
+import org.apache.hawq.pxf.api.LogicalFilter;
+import org.apache.hawq.pxf.api.utilities.ColumnDescriptor;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.Arrays;
+import java.util.List;
+
+public class HiveORCSearchArgumentExample {
--- End diff --
Rename this class as HiveORCSearchArgumentTest. All of the test classes end with name ***Test
---
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 #906: HAWQ-964. Support for OR and NOT Logical O...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-hawq/pull/906
---
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 #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r83091487
--- Diff: pxf/pxf-hbase/src/main/java/org/apache/hawq/pxf/plugins/hbase/HBaseFilterBuilder.java ---
@@ -71,14 +74,19 @@ public HBaseFilterBuilder(HBaseTupleDescription tupleDescription) {
* @throws Exception if parsing failed
*/
public Filter getFilterObject(String filterString) throws Exception {
- FilterParser parser = new FilterParser(this);
- Object result = parser.parse(filterString);
+ // First check for NOT, HBase does not support this
+ if (filterString.contains(NOT_OP)) {
+ return null;
+ } else {
+ FilterParser parser = new FilterParser(this);
+ Object result = parser.parse(filterString);
+
+ if (!(result instanceof Filter)) {
+ throw new Exception("String " + filterString + " resolved to no filter");
--- End diff --
Rewrite error message to something like "String couldn't be resolved to any supported filter"?
---
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 #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r79486684
--- Diff: pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/FilterParser.java ---
@@ -220,6 +193,24 @@ public Object parse(String filter) throws Exception {
// Store result on stack
operandsStack.push(result);
break;
+ case 'l':
+ LogicalOperation logicalOperation = logicalOperationTranslationMap.get(safeToInt(parseNumber()));
+
+ if (logicalOperation == null) {
+ throw new FilterStringSyntaxException("unknown op ending at " + index);
+ }
+
+ if (logicalOperation == LogicalOperation.HDOP_NOT) {
+ Object exp = operandsStack.pop();
+ result = filterBuilder.build(logicalOperation, exp);
+ } else {
--- End diff --
might be good to explicitly check for enum values here just in case we add support for more logical operations. If not atleast add a comment here about the logical operations being handled in this 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 #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r83103736
--- Diff: pxf/pxf-hive/src/main/java/org/apache/hawq/pxf/plugins/hive/HiveDataFragmenter.java ---
@@ -404,24 +411,41 @@ private String buildFilterStringForHive() throws Exception {
HiveFilterBuilder eval = new HiveFilterBuilder(inputData);
Object filter = eval.getFilterObject(filterInput);
- String prefix = "";
-
- if (filter instanceof List) {
-
- for (Object f : (List<?>) filter) {
- if (buildSingleFilter(f, filtersString, prefix)) {
- // Set 'and' operator between each matched partition filter.
- prefix = " and ";
- }
- }
-
+ if (filter instanceof LogicalFilter) {
+ buildCompoundFilter((LogicalFilter) filter, filtersString);
} else {
- buildSingleFilter(filter, filtersString, prefix);
+ buildSingleFilter(filter, filtersString, "");
}
return filtersString.toString();
}
+ private void buildCompoundFilter(LogicalFilter filter, StringBuilder filterString) throws Exception{
+ String prefix;
+ switch(filter.getOperator()) {
--- End diff --
Since LogicalOperation is generic, do you think we should put a very Hive specific string in that 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 #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r79487950
--- Diff: pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/FilterParser.java ---
@@ -220,6 +193,24 @@ public Object parse(String filter) throws Exception {
// Store result on stack
operandsStack.push(result);
break;
+ case 'l':
+ LogicalOperation logicalOperation = logicalOperationTranslationMap.get(safeToInt(parseNumber()));
--- End diff --
Add comment above this mentioning this is to handle logical operators
---
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 #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r82033272
--- Diff: pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/FilterParser.java ---
@@ -396,4 +387,12 @@ private Operation reverseOp(Operation operation) {
operatorTranslationMap.put(8, Operation.HDOP_LIKE);
return operatorTranslationMap;
}
+
+ static private Map<Integer, LogicalOperation> initLogicalOperatorTransMap() {
+ Map<Integer, LogicalOperation> integerLogicalOperationMap = new HashMap<>();
--- End diff --
That's true. I like that idea. @shivzone any objections to @denalex suggestion?
---
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 #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r79445438
--- Diff: pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/FilterParser.java ---
@@ -65,6 +66,12 @@
HDOP_LIKE
}
+ public enum LogicalOperation {
--- End diff --
Please add a comment above this enum explaining this enum and maybe update the comment above Operation enum to explicitly state that it is for Comparison Operators
---
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 #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r83097391
--- Diff: pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/FilterParser.java ---
@@ -396,4 +387,12 @@ private Operation reverseOp(Operation operation) {
operatorTranslationMap.put(8, Operation.HDOP_LIKE);
return operatorTranslationMap;
}
+
+ static private Map<Integer, LogicalOperation> initLogicalOperatorTransMap() {
+ Map<Integer, LogicalOperation> integerLogicalOperationMap = new HashMap<>();
--- End diff --
I think we'll need to go with an array. I haven't found a way to do a reverse lookup of a Enum based on `int` values. Enum.valueOf() takes a 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 #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r81827921
--- Diff: pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/FilterParser.java ---
@@ -396,4 +387,12 @@ private Operation reverseOp(Operation operation) {
operatorTranslationMap.put(8, Operation.HDOP_LIKE);
return operatorTranslationMap;
}
+
+ static private Map<Integer, LogicalOperation> initLogicalOperatorTransMap() {
+ Map<Integer, LogicalOperation> integerLogicalOperationMap = new HashMap<>();
--- End diff --
array will be a better structure for faster access of lookup of integer from 0..N to an object.
---
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 #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r83097937
--- Diff: pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/FilterParser.java ---
@@ -396,4 +387,12 @@ private Operation reverseOp(Operation operation) {
operatorTranslationMap.put(8, Operation.HDOP_LIKE);
return operatorTranslationMap;
}
+
+ static private Map<Integer, LogicalOperation> initLogicalOperatorTransMap() {
+ Map<Integer, LogicalOperation> integerLogicalOperationMap = new HashMap<>();
--- End diff --
Nvm. Enum.values() returns an array of values
---
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 #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r83101774
--- Diff: pxf/pxf-hbase/src/main/java/org/apache/hawq/pxf/plugins/hbase/HBaseFilterBuilder.java ---
@@ -122,25 +130,23 @@ public Filter getFilterObject(String filterString) throws Exception {
public Object build(FilterParser.Operation opId,
Object leftOperand,
Object rightOperand) throws Exception {
- if (leftOperand instanceof Filter) {
- if (opId != FilterParser.Operation.HDOP_AND ||
- !(rightOperand instanceof Filter)) {
- throw new Exception("Only AND is allowed between compound expressions");
- }
-
- return handleCompoundOperations((Filter) leftOperand, (Filter) rightOperand);
- }
-
- if (!(rightOperand instanceof FilterParser.Constant)) {
- throw new Exception("expressions of column-op-column are not supported");
- }
// Assume column is on the left
return handleSimpleOperations(opId,
(FilterParser.ColumnIndex) leftOperand,
(FilterParser.Constant) rightOperand);
}
+ @Override
+ public Object build(FilterParser.LogicalOperation opId, Object leftOperand, Object rightOperand) {
+ return handleCompoundOperations(opId, (Filter) leftOperand, (Filter) rightOperand);
+ }
+
+ @Override
+ public Object build(FilterParser.LogicalOperation opId, Object leftOperand) {
+ return null;
--- End diff --
Yeah, HBase doesn't support NOT operations. I don't think this method will ever be invoked in the Hbase case because I already return null when filtering a Hbase filter with 'l2' as an op 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 #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r83092607
--- Diff: pxf/pxf-hive/src/main/java/org/apache/hawq/pxf/plugins/hive/HiveAccessor.java ---
@@ -205,55 +217,138 @@ private boolean isFiltered(List<HivePartition> partitionFields,
return testOneFilter(partitionFields, filter, inputData);
}
- /*
- * We are testing one filter against all the partition fields. The filter
- * has the form "fieldA = valueA". The partitions have the form
- * partitionOne=valueOne/partitionTwo=ValueTwo/partitionThree=valueThree 1.
- * For a filter to match one of the partitions, lets say partitionA for
- * example, we need: fieldA = partittionOne and valueA = valueOne. If this
- * condition occurs, we return true. 2. If fieldA does not match any one of
- * the partition fields we also return true, it means we ignore this filter
- * because it is not on a partition field. 3. If fieldA = partittionOne and
- * valueA != valueOne, then we return false.
- */
- private boolean testOneFilter(List<HivePartition> partitionFields,
- Object filter, InputData input) {
- // Let's look first at the filter
- FilterParser.BasicFilter bFilter = (FilterParser.BasicFilter) filter;
+ private boolean testForUnsupportedOperators(List<Object> filterList) {
--- End diff --
Method name is a bit confusing. Could we rename it to something like hasUnsupportedOperator?
---
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 #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r82032692
--- Diff: pxf/pxf-hbase/src/main/java/org/apache/hawq/pxf/plugins/hbase/HBaseFilterBuilder.java ---
@@ -71,14 +74,19 @@ public HBaseFilterBuilder(HBaseTupleDescription tupleDescription) {
* @throws Exception if parsing failed
*/
public Filter getFilterObject(String filterString) throws Exception {
- FilterParser parser = new FilterParser(this);
- Object result = parser.parse(filterString);
+ // First check for NOT, HBase does not support this
+ if (filterString.contains(NOT_OP)) {
+ return null;
+ } else {
--- End diff --
Ok, sounds good
---
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 #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r83092740
--- Diff: pxf/pxf-hive/src/main/java/org/apache/hawq/pxf/plugins/hive/HiveAccessor.java ---
@@ -205,55 +217,138 @@ private boolean isFiltered(List<HivePartition> partitionFields,
return testOneFilter(partitionFields, filter, inputData);
}
- /*
- * We are testing one filter against all the partition fields. The filter
- * has the form "fieldA = valueA". The partitions have the form
- * partitionOne=valueOne/partitionTwo=ValueTwo/partitionThree=valueThree 1.
- * For a filter to match one of the partitions, lets say partitionA for
- * example, we need: fieldA = partittionOne and valueA = valueOne. If this
- * condition occurs, we return true. 2. If fieldA does not match any one of
- * the partition fields we also return true, it means we ignore this filter
- * because it is not on a partition field. 3. If fieldA = partittionOne and
- * valueA != valueOne, then we return false.
- */
- private boolean testOneFilter(List<HivePartition> partitionFields,
- Object filter, InputData input) {
- // Let's look first at the filter
- FilterParser.BasicFilter bFilter = (FilterParser.BasicFilter) filter;
+ private boolean testForUnsupportedOperators(List<Object> filterList) {
+ boolean nonAndOp = true;
+ for (Object filter : filterList) {
+ if (filter instanceof LogicalFilter) {
+ if (((LogicalFilter) filter).getOperator() != FilterParser.LogicalOperation.HDOP_AND)
+ return false;
+ if (((LogicalFilter) filter).getFilterList() != null)
+ nonAndOp = testForUnsupportedOperators(((LogicalFilter) filter).getFilterList());
+ }
+ }
+ return nonAndOp;
+ }
- boolean isFilterOperationEqual = (bFilter.getOperation() == FilterParser.Operation.HDOP_EQ);
- if (!isFilterOperationEqual) /*
+ private boolean testForPartitionEquality(List<HivePartition> partitionFields, List<Object> filterList, InputData input) {
+ boolean partitionAllowed = true;
+ for (Object filter : filterList) {
+ if (filter instanceof BasicFilter) {
+ BasicFilter bFilter = (BasicFilter) filter;
+ boolean isFilterOperationEqual = (bFilter.getOperation() == FilterParser.Operation.HDOP_EQ);
+ if (!isFilterOperationEqual) /*
* in case this is not an "equality filter"
* we ignore it here - in partition
* filtering
*/{
- return true;
- }
+ return true;
+ }
+
+ int filterColumnIndex = bFilter.getColumn().index();
+ String filterValue = bFilter.getConstant().constant().toString();
+ ColumnDescriptor filterColumn = input.getColumn(filterColumnIndex);
+ String filterColumnName = filterColumn.columnName();
- int filterColumnIndex = bFilter.getColumn().index();
- String filterValue = bFilter.getConstant().constant().toString();
- ColumnDescriptor filterColumn = input.getColumn(filterColumnIndex);
- String filterColumnName = filterColumn.columnName();
+ for (HivePartition partition : partitionFields) {
+ if (filterColumnName.equals(partition.name)) {
- for (HivePartition partition : partitionFields) {
- if (filterColumnName.equals(partition.name)) {
/*
* the filter field matches a partition field, but the values do
* not match
*/
- return filterValue.equals(partition.val);
- }
- }
+ boolean keepPartition = filterValue.equals(partition.val);
+
+ /*
+ * If the string comparison fails then we should check the comparison of
+ * the two operands as typed values
+ */
+ if (!keepPartition && !partition.val.equals("__HIVE_DEFAULT_PARTITION__")){
--- End diff --
Make "__HIVE_DEFAULT_PARTITION__" a constant?
---
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 #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r83090314
--- Diff: pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/FilterParser.java ---
@@ -396,4 +387,12 @@ private Operation reverseOp(Operation operation) {
operatorTranslationMap.put(8, Operation.HDOP_LIKE);
return operatorTranslationMap;
}
+
+ static private Map<Integer, LogicalOperation> initLogicalOperatorTransMap() {
+ Map<Integer, LogicalOperation> integerLogicalOperationMap = new HashMap<>();
--- End diff --
+1 on using enum built-in structure instead of addition integerLogicalOperationMap.
---
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 #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r83097860
--- Diff: pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/FilterParser.java ---
@@ -396,4 +387,12 @@ private Operation reverseOp(Operation operation) {
operatorTranslationMap.put(8, Operation.HDOP_LIKE);
return operatorTranslationMap;
}
+
+ static private Map<Integer, LogicalOperation> initLogicalOperatorTransMap() {
+ Map<Integer, LogicalOperation> integerLogicalOperationMap = new HashMap<>();
--- End diff --
check here: http://stackoverflow.com/questions/11047756/getting-enum-associated-with-int-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 #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r83090586
--- Diff: pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/BasicFilter.java ---
@@ -0,0 +1,37 @@
+package org.apache.hawq.pxf.api;
--- End diff --
Missing Apache header?
---
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 #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r83098043
--- Diff: pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/FilterParser.java ---
@@ -396,4 +387,12 @@ private Operation reverseOp(Operation operation) {
operatorTranslationMap.put(8, Operation.HDOP_LIKE);
return operatorTranslationMap;
}
+
+ static private Map<Integer, LogicalOperation> initLogicalOperatorTransMap() {
+ Map<Integer, LogicalOperation> integerLogicalOperationMap = new HashMap<>();
--- End diff --
@denalex Thanks. I had stumbled on the same thing
---
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 #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r83092030
--- Diff: pxf/pxf-hbase/src/main/java/org/apache/hawq/pxf/plugins/hbase/HBaseFilterBuilder.java ---
@@ -122,25 +130,23 @@ public Filter getFilterObject(String filterString) throws Exception {
public Object build(FilterParser.Operation opId,
Object leftOperand,
Object rightOperand) throws Exception {
- if (leftOperand instanceof Filter) {
- if (opId != FilterParser.Operation.HDOP_AND ||
- !(rightOperand instanceof Filter)) {
- throw new Exception("Only AND is allowed between compound expressions");
- }
-
- return handleCompoundOperations((Filter) leftOperand, (Filter) rightOperand);
- }
-
- if (!(rightOperand instanceof FilterParser.Constant)) {
- throw new Exception("expressions of column-op-column are not supported");
- }
// Assume column is on the left
return handleSimpleOperations(opId,
(FilterParser.ColumnIndex) leftOperand,
(FilterParser.Constant) rightOperand);
}
+ @Override
+ public Object build(FilterParser.LogicalOperation opId, Object leftOperand, Object rightOperand) {
+ return handleCompoundOperations(opId, (Filter) leftOperand, (Filter) rightOperand);
+ }
+
+ @Override
+ public Object build(FilterParser.LogicalOperation opId, Object leftOperand) {
+ return null;
--- End diff --
Does it mean that we just ignore unary logical operator for HBase?
---
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 #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r83099497
--- Diff: pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/FilterParser.java ---
@@ -396,4 +387,12 @@ private Operation reverseOp(Operation operation) {
operatorTranslationMap.put(8, Operation.HDOP_LIKE);
return operatorTranslationMap;
}
+
+ static private Map<Integer, LogicalOperation> initLogicalOperatorTransMap() {
+ Map<Integer, LogicalOperation> integerLogicalOperationMap = new HashMap<>();
--- End diff --
please review http://stackoverflow.com/questions/5292790/convert-integer-value-to-matching-java-enum for discussion on dangers of relying on ordinal values. I think for us, we might want to maintain explicit number as the private member of enum, then build and cache the array once for all future access.
---
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 #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r79488252
--- Diff: pxf/pxf-hbase/src/main/java/org/apache/hawq/pxf/plugins/hbase/HBaseFilterBuilder.java ---
@@ -52,12 +52,15 @@
*/
public class HBaseFilterBuilder implements FilterParser.FilterBuilder {
private Map<FilterParser.Operation, CompareFilter.CompareOp> operatorsMap;
+ private Map<FilterParser.LogicalOperation, FilterList.Operator> logicalOperatorsMap;
private byte[] startKey;
private byte[] endKey;
private HBaseTupleDescription tupleDescription;
+ private static final String NOT_OP = "l2";
--- End diff --
Why is this explcitly defined here unlike other operators ?
---
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 #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r81858427
--- Diff: pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/FilterParser.java ---
@@ -81,6 +88,8 @@
* @throws Exception if building the filter failed
*/
public Object build(Operation operation, Object left, Object right) throws Exception;
+ public Object build(LogicalOperation operation, Object left, Object right) throws Exception;
+ public Object build(LogicalOperation operation, Object filter) throws Exception;
--- End diff --
The reason for having a separate LogicalOperator enum is that fundamentally a Logical Operator and a Comparison Operator are different. The grammar for a Logical Operators is the following
Logical Operation -> Expr1 (Logical Operator) Expr2
OR
Logical Operation -> (Logical Operator) Expr1 [ This is the NOT case]
Here "Expr" will stand for either a Logical Operation or a Conditional One
A Conditional Operator will have the following form: (Note: they can be in reverse form)
Conditional Operation -> Column (Conditional Operator) Constant
So when it comes to parsing a Filter, a Logical Operator will always have 1/2 children that are themselves an Expression. Whereas a Conditional Operation will always have 2 (primitive or constant) children and those children will always be leaf nodes.
Thus the reason for having separate op codes is to have a clear way of distinguishing Logical Operations from Conditional Ones and internally the changes of this Pull Request introduce two Java Objects `Basic Filter` and `Logical Filter`. A `BasicFilter` can only have a `column` and `constant` as fields but a `LogicalFilter` has a list of subexpressions (`filterList`) as a field.
All of the above doesn't explain why we couldn't have a unified enum, but I hope it better explains why we felt that the two cases are different enough to warrant a separate code. Additionally, when we build the filter string in HAWQ itself HAWQ also has two separate Expr types `OpExpr` and `BoolExpr` so our approach follows this convention.
---
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 #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r83094033
--- Diff: pxf/pxf-hive/src/main/java/org/apache/hawq/pxf/plugins/hive/HiveDataFragmenter.java ---
@@ -404,24 +411,41 @@ private String buildFilterStringForHive() throws Exception {
HiveFilterBuilder eval = new HiveFilterBuilder(inputData);
Object filter = eval.getFilterObject(filterInput);
- String prefix = "";
-
- if (filter instanceof List) {
-
- for (Object f : (List<?>) filter) {
- if (buildSingleFilter(f, filtersString, prefix)) {
- // Set 'and' operator between each matched partition filter.
- prefix = " and ";
- }
- }
-
+ if (filter instanceof LogicalFilter) {
+ buildCompoundFilter((LogicalFilter) filter, filtersString);
} else {
- buildSingleFilter(filter, filtersString, prefix);
+ buildSingleFilter(filter, filtersString, "");
}
return filtersString.toString();
}
+ private void buildCompoundFilter(LogicalFilter filter, StringBuilder filterString) throws Exception{
+ String prefix;
+ switch(filter.getOperator()) {
--- End diff --
I would add prefix as a member value to LogicalOperation enum and just invoke getter 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.
---
[GitHub] incubator-hawq pull request #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r82033359
--- Diff: pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/FilterParser.java ---
@@ -220,6 +193,24 @@ public Object parse(String filter) throws Exception {
// Store result on stack
operandsStack.push(result);
break;
+ case 'l':
+ LogicalOperation logicalOperation = logicalOperationTranslationMap.get(safeToInt(parseNumber()));
+
+ if (logicalOperation == null) {
+ throw new FilterStringSyntaxException("unknown op ending at " + index);
+ }
+
+ if (logicalOperation == LogicalOperation.HDOP_NOT) {
+ Object exp = operandsStack.pop();
+ result = filterBuilder.build(logicalOperation, exp);
+ } else {
--- End diff --
Ok, I'll add the check
---
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 #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r79487522
--- Diff: pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/FilterParser.java ---
@@ -81,6 +88,8 @@
* @throws Exception if building the filter failed
*/
public Object build(Operation operation, Object left, Object right) throws Exception;
+ public Object build(LogicalOperation operation, Object left, Object right) throws Exception;
--- End diff --
Since these are functions in an interface, each function needs to have a well defnined javadoc comment
---
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 #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r83044785
--- Diff: pxf/pxf-hive/src/main/java/org/apache/hawq/pxf/plugins/hive/HiveDataFragmenter.java ---
@@ -405,24 +411,41 @@ private String buildFilterStringForHive() throws Exception {
HiveFilterBuilder eval = new HiveFilterBuilder(inputData);
Object filter = eval.getFilterObject(filterInput);
- String prefix = "";
-
- if (filter instanceof List) {
-
- for (Object f : (List<?>) filter) {
- if (buildSingleFilter(f, filtersString, prefix)) {
- // Set 'and' operator between each matched partition filter.
- prefix = " and ";
- }
- }
-
+ if (filter instanceof LogicalFilter) {
+ buildCompoundFilter((LogicalFilter) filter, filtersString);
} else {
- buildSingleFilter(filter, filtersString, prefix);
+ buildSingleFilter(filter, filtersString, "");
}
return filtersString.toString();
}
+ private void buildCompoundFilter(LogicalFilter filter, StringBuilder filterString) throws Exception{
+ String prefix;
+ switch(filter.getOperator()) {
+ case HDOP_AND:
+ prefix = " and ";
+ break;
+ case HDOP_OR:
+ prefix = " or ";
+ break;
+ case HDOP_NOT:
+ prefix = " not ";
+ break;
+ default:
+ prefix = "";
+ }
+
+ for (int i = 0; i < filter.getFilterList().size(); i++) {
--- End diff --
for (Object f : filter.getFilterList() )
is more compact notation that collapses 2 lines into one
---
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 #906: HAWQ-964. Support for OR and NOT Logical O...
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/906#discussion_r83100399
--- Diff: pxf/pxf-hbase/src/main/java/org/apache/hawq/pxf/plugins/hbase/HBaseFilterBuilder.java ---
@@ -52,12 +52,15 @@
*/
public class HBaseFilterBuilder implements FilterParser.FilterBuilder {
private Map<FilterParser.Operation, CompareFilter.CompareOp> operatorsMap;
+ private Map<FilterParser.LogicalOperation, FilterList.Operator> logicalOperatorsMap;
private byte[] startKey;
private byte[] endKey;
private HBaseTupleDescription tupleDescription;
+ private static final String NOT_OP = "l2";
--- End diff --
Because HBase doesn't support NOT pushdown so we ignore any filters with NOT only in HBase
---
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.
---