[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

2020-09-23 Thread GitBox 


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

2020-09-24 Thread GitBox 


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

2020-09-24 Thread GitBox 


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

2020-09-25 Thread GitBox 


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

2020-09-28 Thread GitBox 


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

2020-09-28 Thread GitBox 


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

2020-10-11 Thread GitBox 


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