akashrn5 commented on a change in pull request #3748:
URL: https://github.com/apache/carbondata/pull/3748#discussion_r422918566



##########
File path: 
index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestSIWithPartition.scala
##########
@@ -356,8 +356,32 @@ class TestSIWithPartition extends QueryTest with 
BeforeAndAfterAll {
     }
   }
 
+  test("test secondary index with partition table having mutiple partition 
columns") {
+    sql("drop table if exists partition_table")
+    sql(s"""
+         | CREATE TABLE partition_table (stringField string, intField int, 
shortField short, stringField1 string)
+         | STORED AS carbondata
+         | PARTITIONED BY (hour_ string, date_ string, sec_ string)
+         | TBLPROPERTIES ('SORT_COLUMNS'='hour_,date_,stringField', 
'SORT_SCOPE'='GLOBAL_SORT')
+      """.stripMargin
+    ).collect()
+    sql(s"drop index if exists si_1 on partition_table")
+    sql(s"create index si_1 on partition_table(stringField1) as 'carbondata'")

Review comment:
       please give some meaningful name to SI based on the test scenario.

##########
File path: 
index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestSIWithPartition.scala
##########
@@ -356,8 +356,32 @@ class TestSIWithPartition extends QueryTest with 
BeforeAndAfterAll {
     }
   }
 
+  test("test secondary index with partition table having mutiple partition 
columns") {
+    sql("drop table if exists partition_table")
+    sql(s"""
+         | CREATE TABLE partition_table (stringField string, intField int, 
shortField short, stringField1 string)
+         | STORED AS carbondata
+         | PARTITIONED BY (hour_ string, date_ string, sec_ string)
+         | TBLPROPERTIES ('SORT_COLUMNS'='hour_,date_,stringField', 
'SORT_SCOPE'='GLOBAL_SORT')
+      """.stripMargin
+    ).collect()

Review comment:
       remove the collect

##########
File path: 
core/src/main/java/org/apache/carbondata/core/indexstore/blockletindex/BlockIndex.java
##########
@@ -793,9 +794,12 @@ private boolean addBlockBasedOnMinMaxValue(FilterExecuter 
filterExecuter, byte[]
     BitSet bitSet = null;
     if (filterExecuter instanceof ImplicitColumnFilterExecutor) {
       String uniqueBlockPath;
-      if (segmentPropertiesWrapper.getCarbonTable().isHivePartitionTable()) {
-        uniqueBlockPath = filePath
-            
.substring(segmentPropertiesWrapper.getCarbonTable().getTablePath().length() + 
1);
+      CarbonTable carbonTable = segmentPropertiesWrapper.getCarbonTable();
+      if (carbonTable.isHivePartitionTable()) {
+        boolean isStandardTable = 
CarbonUtil.isStandardCarbonTable(carbonTable);

Review comment:
       please add a comment explaining the scenario here with example.




----------------------------------------------------------------
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


Reply via email to