Repository: incubator-carbondata Updated Branches: refs/heads/master 64f26c21e -> 30444b837
[Issue Number] CARBONDATA-242 [Description] When user provides Null member inside NOT IN filter condition the resultset is not compatible with hive result Project: http://git-wip-us.apache.org/repos/asf/incubator-carbondata/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-carbondata/commit/34e36dde Tree: http://git-wip-us.apache.org/repos/asf/incubator-carbondata/tree/34e36dde Diff: http://git-wip-us.apache.org/repos/asf/incubator-carbondata/diff/34e36dde Branch: refs/heads/master Commit: 34e36dde75e4d964c06f067586ed4121464f0ea3 Parents: 64f26c2 Author: sujith71955 <sujithchacko.2...@gmail.com> Authored: Thu Sep 15 14:53:04 2016 +0530 Committer: Venkata Ramana G <ramana.gollam...@huawei.com> Committed: Fri Sep 16 21:43:01 2016 +0530 ---------------------------------------------------------------------- .../expression/logical/FalseExpression.java | 51 ++++++++++++++++++++ .../scan/filter/FilterExpressionProcessor.java | 6 ++- .../scan/filter/intf/ExpressionType.java | 3 +- .../apache/carbondata/spark/CarbonFilters.scala | 35 +++++++++++--- .../GrtLtFilterProcessorTestCase.scala | 6 +++ 5 files changed, 93 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-carbondata/blob/34e36dde/core/src/main/java/org/apache/carbondata/scan/expression/logical/FalseExpression.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/carbondata/scan/expression/logical/FalseExpression.java b/core/src/main/java/org/apache/carbondata/scan/expression/logical/FalseExpression.java new file mode 100644 index 0000000..2b0aead --- /dev/null +++ b/core/src/main/java/org/apache/carbondata/scan/expression/logical/FalseExpression.java @@ -0,0 +1,51 @@ +package org.apache.carbondata.scan.expression.logical; + +import org.apache.carbondata.core.carbon.metadata.datatype.DataType; +import org.apache.carbondata.scan.expression.Expression; +import org.apache.carbondata.scan.expression.ExpressionResult; +import org.apache.carbondata.scan.expression.LiteralExpression; +import org.apache.carbondata.scan.expression.conditional.BinaryConditionalExpression; +import org.apache.carbondata.scan.expression.exception.FilterIllegalMemberException; +import org.apache.carbondata.scan.expression.exception.FilterUnsupportedException; +import org.apache.carbondata.scan.filter.intf.ExpressionType; +import org.apache.carbondata.scan.filter.intf.RowIntf; + + + +/** + * This class will form an expression whose evaluation will be always false. + */ +public class FalseExpression extends BinaryConditionalExpression { + + + private static final long serialVersionUID = -8390184061336799370L; + + public FalseExpression(Expression child1) { + super(child1, new LiteralExpression(null,null)); + } + + /** + * This method will always return false, mainly used in the filter expressions + * which are illogical. + * eg: columnName NOT IN('Java',NULL) + * @param value + * @return + * @throws FilterUnsupportedException + * @throws FilterIllegalMemberException + */ + @Override public ExpressionResult evaluate(RowIntf value) + throws FilterUnsupportedException, FilterIllegalMemberException { + return new ExpressionResult(DataType.BOOLEAN,false); + } + + /** + * This method will return the expression types + * @return + */ + @Override public ExpressionType getFilterExpressionType() { + return ExpressionType.FALSE; + } + @Override public String getString() { + return null; + } +} http://git-wip-us.apache.org/repos/asf/incubator-carbondata/blob/34e36dde/core/src/main/java/org/apache/carbondata/scan/filter/FilterExpressionProcessor.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/carbondata/scan/filter/FilterExpressionProcessor.java b/core/src/main/java/org/apache/carbondata/scan/filter/FilterExpressionProcessor.java index 9d996a0..b50e6e6 100644 --- a/core/src/main/java/org/apache/carbondata/scan/filter/FilterExpressionProcessor.java +++ b/core/src/main/java/org/apache/carbondata/scan/filter/FilterExpressionProcessor.java @@ -245,7 +245,9 @@ public class FilterExpressionProcessor implements FilterProcessor { case NOT_IN: return getFilterResolverBasedOnExpressionType(ExpressionType.NOT_EQUALS, false, expressionTree, tableIdentifier, expressionTree); - + case FALSE: + return getFilterResolverBasedOnExpressionType(ExpressionType.FALSE, false, + expressionTree, tableIdentifier, expressionTree); default: return getFilterResolverBasedOnExpressionType(ExpressionType.UNKNOWN, false, expressionTree, tableIdentifier, expressionTree); @@ -262,6 +264,8 @@ public class FilterExpressionProcessor implements FilterProcessor { BinaryConditionalExpression currentCondExpression = null; ConditionalExpression condExpression = null; switch (filterExpressionType) { + case FALSE: + return new RowLevelFilterResolverImpl(expression, false, false, tableIdentifier); case EQUALS: currentCondExpression = (BinaryConditionalExpression) expression; if (currentCondExpression.isSingleDimension() http://git-wip-us.apache.org/repos/asf/incubator-carbondata/blob/34e36dde/core/src/main/java/org/apache/carbondata/scan/filter/intf/ExpressionType.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/carbondata/scan/filter/intf/ExpressionType.java b/core/src/main/java/org/apache/carbondata/scan/filter/intf/ExpressionType.java index 4d658fc..0dca5a4 100644 --- a/core/src/main/java/org/apache/carbondata/scan/filter/intf/ExpressionType.java +++ b/core/src/main/java/org/apache/carbondata/scan/filter/intf/ExpressionType.java @@ -39,6 +39,7 @@ public enum ExpressionType { NOT_IN, UNKNOWN, LITERAL, - RANGE + RANGE, + FALSE } http://git-wip-us.apache.org/repos/asf/incubator-carbondata/blob/34e36dde/integration/spark/src/main/scala/org/apache/carbondata/spark/CarbonFilters.scala ---------------------------------------------------------------------- diff --git a/integration/spark/src/main/scala/org/apache/carbondata/spark/CarbonFilters.scala b/integration/spark/src/main/scala/org/apache/carbondata/spark/CarbonFilters.scala index d16d583..5fb0051 100644 --- a/integration/spark/src/main/scala/org/apache/carbondata/spark/CarbonFilters.scala +++ b/integration/spark/src/main/scala/org/apache/carbondata/spark/CarbonFilters.scala @@ -29,7 +29,7 @@ import org.apache.carbondata.core.carbon.metadata.schema.table.CarbonTable import org.apache.carbondata.core.carbon.metadata.schema.table.column.CarbonColumn import org.apache.carbondata.scan.expression.{ColumnExpression => CarbonColumnExpression, Expression => CarbonExpression, LiteralExpression => CarbonLiteralExpression} import org.apache.carbondata.scan.expression.conditional._ -import org.apache.carbondata.scan.expression.logical.{AndExpression, OrExpression} +import org.apache.carbondata.scan.expression.logical.{AndExpression, FalseExpression, OrExpression} import org.apache.carbondata.spark.util.CarbonScalaUtil /** @@ -258,16 +258,30 @@ object CarbonFilters { case Not(EqualTo(l@Literal(v, t), Cast(a: Attribute, _))) => new Some(new NotEqualsExpression(transformExpression(a).get, transformExpression(l).get)) - case Not(In(a: Attribute, list)) if !list.exists(!_.isInstanceOf[Literal]) => + case Not(In(a: Attribute, list)) + if !list.exists(!_.isInstanceOf[Literal]) => + if (list.exists(x => (isNullLiteral(x.asInstanceOf[Literal])))) { + Some(new FalseExpression(transformExpression(a).get)) + } + else { Some(new NotInExpression(transformExpression(a).get, - new ListExpression(convertToJavaList(list.map(transformExpression(_).get))))) + new ListExpression(convertToJavaList(list.map(transformExpression(_).get))))) + } case In(a: Attribute, list) if !list.exists(!_.isInstanceOf[Literal]) => Some(new InExpression(transformExpression(a).get, new ListExpression(convertToJavaList(list.map(transformExpression(_).get))))) case Not(In(Cast(a: Attribute, _), list)) if !list.exists(!_.isInstanceOf[Literal]) => - Some(new NotInExpression(transformExpression(a).get, - new ListExpression(convertToJavaList(list.map(transformExpression(_).get))))) + /* if any illogical expression comes in NOT IN Filter like + NOT IN('scala',NULL) this will be treated as false expression and will + always return no result. */ + if (list.exists(x => (isNullLiteral(x.asInstanceOf[Literal])))) { + Some(new FalseExpression(transformExpression(a).get)) + } + else { + Some(new NotInExpression(transformExpression(a).get, new ListExpression( + convertToJavaList(list.map(transformExpression(_).get))))) + } case In(Cast(a: Attribute, _), list) if !list.exists(!_.isInstanceOf[Literal]) => Some(new InExpression(transformExpression(a).get, new ListExpression(convertToJavaList(list.map(transformExpression(_).get))))) @@ -335,7 +349,16 @@ object CarbonFilters { } exprs.flatMap(transformExpression(_, false)).reduceOption(new AndExpression(_, _)) } - + private def isNullLiteral(exp: Expression): Boolean = { + if (null != exp + && exp.isInstanceOf[Literal] + && (exp.asInstanceOf[Literal].dataType == org.apache.spark.sql.types.DataTypes.NullType) + || (exp.asInstanceOf[Literal].value == null)) { + true + } else { + false + } + } private def getActualCarbonDataType(column: String, carbonTable: CarbonTable) = { var carbonColumn: CarbonColumn = carbonTable.getDimensionByName(carbonTable.getFactTableName, column) http://git-wip-us.apache.org/repos/asf/incubator-carbondata/blob/34e36dde/integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/filterexpr/GrtLtFilterProcessorTestCase.scala ---------------------------------------------------------------------- diff --git a/integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/filterexpr/GrtLtFilterProcessorTestCase.scala b/integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/filterexpr/GrtLtFilterProcessorTestCase.scala index c2a4b80..5278344 100644 --- a/integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/filterexpr/GrtLtFilterProcessorTestCase.scala +++ b/integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/filterexpr/GrtLtFilterProcessorTestCase.scala @@ -136,6 +136,12 @@ class GrtLtFilterProcessorTestCase extends QueryTest with BeforeAndAfterAll { Seq(Row(0)) ) } + test("In condition With improper format query regarding Null filter") { + checkAnswer( + sql("select empid from a12_allnull " + "where empid not in ('china',NULL)"), + Seq() + ) + } //no null test cases
