[GitHub] [carbondata] VenuReddy2103 commented on a change in pull request #3953: [CARBONDATA-4008]Fixed IN filter on date column is returning 0 results when 'carbon.push.rowfilters.for.vector' is true
VenuReddy2103 commented on a change in pull request #3953: URL: https://github.com/apache/carbondata/pull/3953#discussion_r494050860 ## File path: integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/filterexpr/TestInFilter.scala ## @@ -165,8 +168,27 @@ class TestInFilter extends QueryTest with BeforeAndAfterAll{ Seq(Row(4, 1.00, 2.00, 3.00))) } - override def afterAll(): Unit = { + test("test infilter with date, timestamp columns") { +sql("create table test_table(i int, dt date, ts timestamp) stored as carbondata") +sql("insert into test_table select 1, '2020-03-30', '2020-03-30 10:00:00'") Review comment: Have already added drop table in afterEach() This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] VenuReddy2103 commented on a change in pull request #3953: [CARBONDATA-4008]Fixed IN filter on date column is returning 0 results when 'carbon.push.rowfilters.for.vector' is true
VenuReddy2103 commented on a change in pull request #3953: URL: https://github.com/apache/carbondata/pull/3953#discussion_r494091568 ## File path: core/src/main/java/org/apache/carbondata/core/scan/filter/executer/RowLevelFilterExecutorImpl.java ## @@ -138,6 +145,8 @@ public RowLevelFilterExecutorImpl(List dimColEvalua this.exp = exp; this.tableIdentifier = tableIdentifier; this.complexDimensionInfoMap = complexDimensionInfoMap; +this.dateDictionaryGenerator = + DirectDictionaryKeyGeneratorFactory.getDirectDictionaryGenerator(DataTypes.DATE); Review comment: Yeah not happening for timestamp. But have added testcase for timestamp as well. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] VenuReddy2103 commented on a change in pull request #3953: [CARBONDATA-4008]Fixed IN filter on date column is returning 0 results when 'carbon.push.rowfilters.for.vector' is true
VenuReddy2103 commented on a change in pull request #3953: URL: https://github.com/apache/carbondata/pull/3953#discussion_r494091568 ## File path: core/src/main/java/org/apache/carbondata/core/scan/filter/executer/RowLevelFilterExecutorImpl.java ## @@ -138,6 +145,8 @@ public RowLevelFilterExecutorImpl(List dimColEvalua this.exp = exp; this.tableIdentifier = tableIdentifier; this.complexDimensionInfoMap = complexDimensionInfoMap; +this.dateDictionaryGenerator = + DirectDictionaryKeyGeneratorFactory.getDirectDictionaryGenerator(DataTypes.DATE); Review comment: Yeah not happening for timestamp. But have added testcase for infilter with timestamp as well. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] VenuReddy2103 commented on a change in pull request #3953: [CARBONDATA-4008]Fixed IN filter on date column is returning 0 results when 'carbon.push.rowfilters.for.vector' is true
VenuReddy2103 commented on a change in pull request #3953: URL: https://github.com/apache/carbondata/pull/3953#discussion_r494050860 ## File path: integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/filterexpr/TestInFilter.scala ## @@ -165,8 +168,27 @@ class TestInFilter extends QueryTest with BeforeAndAfterAll{ Seq(Row(4, 1.00, 2.00, 3.00))) } - override def afterAll(): Unit = { + test("test infilter with date, timestamp columns") { +sql("create table test_table(i int, dt date, ts timestamp) stored as carbondata") +sql("insert into test_table select 1, '2020-03-30', '2020-03-30 10:00:00'") Review comment: Have already added drop table in afterEach() ## File path: core/src/main/java/org/apache/carbondata/core/scan/filter/executer/RowLevelFilterExecutorImpl.java ## @@ -138,6 +145,8 @@ public RowLevelFilterExecutorImpl(List dimColEvalua this.exp = exp; this.tableIdentifier = tableIdentifier; this.complexDimensionInfoMap = complexDimensionInfoMap; +this.dateDictionaryGenerator = + DirectDictionaryKeyGeneratorFactory.getDirectDictionaryGenerator(DataTypes.DATE); Review comment: Yeah not happening for timestamp. But have added testcase for timestamp as well. ## File path: core/src/main/java/org/apache/carbondata/core/scan/filter/executer/RowLevelFilterExecutorImpl.java ## @@ -138,6 +145,8 @@ public RowLevelFilterExecutorImpl(List dimColEvalua this.exp = exp; this.tableIdentifier = tableIdentifier; this.complexDimensionInfoMap = complexDimensionInfoMap; +this.dateDictionaryGenerator = + DirectDictionaryKeyGeneratorFactory.getDirectDictionaryGenerator(DataTypes.DATE); Review comment: Yeah not happening for timestamp. But have added testcase for infilter with timestamp as well. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] VenuReddy2103 commented on a change in pull request #3953: [CARBONDATA-4008]Fixed IN filter on date column is returning 0 results when 'carbon.push.rowfilters.for.vector' is true
VenuReddy2103 commented on a change in pull request #3953: URL: https://github.com/apache/carbondata/pull/3953#discussion_r496212659 ## File path: core/src/main/java/org/apache/carbondata/core/scan/filter/executer/RowLevelFilterExecutorImpl.java ## @@ -106,6 +108,11 @@ */ boolean isNaturalSorted; + /** + * date direct dictionary generator + */ + private DirectDictionaryGenerator dateDictionaryGenerator; + public RowLevelFilterExecutorImpl(List dimColEvaluatorInfoList, Review comment: Have checked this. We are using IncludeFilterExecutorImpl as long as their is no cast expression involved. Cast operation is involved only when the data type of column to compare is not same as the literal values it is compared with. Each literal value in the InFilter can be of different type. Spark chooses one datatype for the entire list of literal values. For example, i IN (3, 2, 1.0) -> all values treated as decimal i IN (3, 2.0, '1.0') -> all values are treated as string. Note: 2.0 is not same as 2 when casted as string. When the cast operation is present, `SparkUnknownExpression` with spark cast(`SparkExpression`) is used. `SparkUnknownExpression.evaluate()` takes row's column value. Using spark cast expression, validates it and converts it to datatype of literal value it chose. And then compare the converted column value against literal values. So validation and type conversions are handled with spark cast expression. IMO, IncludeFilterExecutorImpl is already used when possible. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] VenuReddy2103 commented on a change in pull request #3953: [CARBONDATA-4008]Fixed IN filter on date column is returning 0 results when 'carbon.push.rowfilters.for.vector' is true
VenuReddy2103 commented on a change in pull request #3953: URL: https://github.com/apache/carbondata/pull/3953#discussion_r496212659 ## File path: core/src/main/java/org/apache/carbondata/core/scan/filter/executer/RowLevelFilterExecutorImpl.java ## @@ -106,6 +108,11 @@ */ boolean isNaturalSorted; + /** + * date direct dictionary generator + */ + private DirectDictionaryGenerator dateDictionaryGenerator; + public RowLevelFilterExecutorImpl(List dimColEvaluatorInfoList, Review comment: Have checked this. We are using IncludeFilterExecutorImpl as long as their is no cast expression involved. Cast is involved only when the data type of column to compare is not same as the literal values it is compared with. Each literal value in the InFilter can be of different type. Spark chooses one datatype for the entire list of literal values. For example, with i as int column i IN (3, 2, 1.0) -> all values treated as decimal i IN (3, 2.0, '1.0') -> all values are treated as string. Note: 2.0 is not same as 2 when casted as string. When the cast operation is present, `SparkUnknownExpression` with spark cast(`SparkExpression`) is used. `SparkUnknownExpression.evaluate()` takes row's column value. Using spark cast expression, validates it and converts it to datatype of literal value it chose. And then compare the converted column value against literal values. So validation and type conversions are handled with spark cast expression. IMO, IncludeFilterExecutorImpl is already used when possible. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] VenuReddy2103 commented on a change in pull request #3953: [CARBONDATA-4008]Fixed IN filter on date column is returning 0 results when 'carbon.push.rowfilters.for.vector' is true
VenuReddy2103 commented on a change in pull request #3953: URL: https://github.com/apache/carbondata/pull/3953#discussion_r503075660 ## File path: integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/filterexpr/TestInFilter.scala ## @@ -165,8 +168,27 @@ class TestInFilter extends QueryTest with BeforeAndAfterAll{ Seq(Row(4, 1.00, 2.00, 3.00))) } - override def afterAll(): Unit = { + test("test infilter with date, timestamp columns") { +sql("create table test_table(i int, dt date, ts timestamp) stored as carbondata") +sql("insert into test_table select 1, '2020-03-30', '2020-03-30 10:00:00'") +sql("insert into test_table select 2, '2020-07-04', '2020-07-04 14:12:15'") +sql("insert into test_table select 3, '2020-09-23', '2020-09-23 12:30:45'") + +checkAnswer(sql("select * from test_table where dt IN ('2020-03-30', '2020-09-23')"), Review comment: Testcase is already present in base This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org