[GitHub] [carbondata] marchpure opened a new pull request #3553: [CARBONDATA-3648] Support Alter Table Compaction Level Threshold

[GitBox]

marchpure opened a new pull request #3553: [CARBONDATA-3648] Support Alter Table Compaction Level Threshold
URL: https://github.com/apache/carbondata/pull/3553
 
 
    ### Why is this PR needed?
   (1) The Alter Table sould support Compaction Level Threshold.
   (2) The upper limit of Compaction Level Threshold is 100, which is too small to meet the scenario with massive small files
   (3) There is a bug to limit alter table in windows env.
   
    ### What changes were proposed in this PR?
   (1) AlterTableUtil support alter Compaction_Level_Threshold
   (2) CarbonProperties increases the upper limit of Compaction_Level_Threshold to 10000 from 100.
   (3) Fix the bug which limit alter table in windows env
       
    ### Does this PR introduce any user interface change?
    - No
   
    ### Is any new testcase added?
    - Yes
   
       
   

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3553: [CARBONDATA-3648] Support Alter Table Compaction Level Threshold
URL: https://github.com/apache/carbondata/pull/3553#issuecomment-569937203
 
 
   Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/1380/
   

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3553: [CARBONDATA-3648] Support Alter Table Compaction Level Threshold
URL: https://github.com/apache/carbondata/pull/3553#issuecomment-569944040
 
 
   Build Failed with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/1390/
   

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3553: [CARBONDATA-3648] Support Alter Table Compaction Level Threshold
URL: https://github.com/apache/carbondata/pull/3553#issuecomment-569957033
 
 
   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1402/
   

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

zzcclp commented on a change in pull request #3553: [CARBONDATA-3648] Support Alter Table Compaction Level Threshold
URL: https://github.com/apache/carbondata/pull/3553#discussion_r362247975
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/util/path/CarbonTablePath.java
 ##########
 @@ -81,7 +81,7 @@ public static String getFolderContainingFile(String carbonFilePath) {
     int lastIndex = carbonFilePath.lastIndexOf('/');
     // below code for handling windows environment
     if (-1 == lastIndex) {
-      lastIndex = carbonFilePath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR);
+      lastIndex = carbonFilePath.lastIndexOf(CarbonCommonConstants.WINDOWS_FILE_SEPARATOR);
 
 Review comment:
   why it need to use windows file separator?

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

marchpure commented on a change in pull request #3553: [CARBONDATA-3648] Support Alter Table Compaction Level Threshold
URL: https://github.com/apache/carbondata/pull/3553#discussion_r362250557
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/util/path/CarbonTablePath.java
 ##########
 @@ -81,7 +81,7 @@ public static String getFolderContainingFile(String carbonFilePath) {
     int lastIndex = carbonFilePath.lastIndexOf('/');
     // below code for handling windows environment
     if (-1 == lastIndex) {
-      lastIndex = carbonFilePath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR);
+      lastIndex = carbonFilePath.lastIndexOf(CarbonCommonConstants.WINDOWS_FILE_SEPARATOR);
 
 Review comment:
   carbonFilePath is in type of '/root/schema', while it is in type of 'D:\schema'. The separator is different, dividing into FILE_SEPARATOR ('/') and WINDOWS_FILE_SEPARATOR ('\').
   
   In the Windows Env.  The variant 'lastIndex' is always -1, carbonFilePath.substring(0, lastIndex) will be throw SubStringIndexOutOfBound Exception.

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

marchpure commented on a change in pull request #3553: [CARBONDATA-3648] Support Alter Table Compaction Level Threshold
URL: https://github.com/apache/carbondata/pull/3553#discussion_r362250557
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/util/path/CarbonTablePath.java
 ##########
 @@ -81,7 +81,7 @@ public static String getFolderContainingFile(String carbonFilePath) {
     int lastIndex = carbonFilePath.lastIndexOf('/');
     // below code for handling windows environment
     if (-1 == lastIndex) {
-      lastIndex = carbonFilePath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR);
+      lastIndex = carbonFilePath.lastIndexOf(CarbonCommonConstants.WINDOWS_FILE_SEPARATOR);
 
 Review comment:
   carbonFilePath is in type of '/root/schema', while it is in type of 'D:\schema'. The separator is different, dividing into FILE_SEPARATOR ('/') and WINDOWS_FILE_SEPARATOR ('\').
   
   In the Windows Env.  The variant 'lastIndex' is always -1, carbonFilePath.substring(0, lastIndex) will throw SubStringIndexOutOfBound Exception.

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

zzcclp commented on a change in pull request #3553: [CARBONDATA-3648] Support Alter Table Compaction Level Threshold
URL: https://github.com/apache/carbondata/pull/3553#discussion_r362294232
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/util/path/CarbonTablePath.java
 ##########
 @@ -81,7 +81,7 @@ public static String getFolderContainingFile(String carbonFilePath) {
     int lastIndex = carbonFilePath.lastIndexOf('/');
     // below code for handling windows environment
     if (-1 == lastIndex) {
-      lastIndex = carbonFilePath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR);
+      lastIndex = carbonFilePath.lastIndexOf(CarbonCommonConstants.WINDOWS_FILE_SEPARATOR);
 
 Review comment:
   in the linux env, is lastIndex right if use 'WINDOWS_FILE_SEPARATOR'?
   it better set the right separator according to system env, for example: use java.io.File.separator

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3553: [CARBONDATA-3648] Support Alter Table Compaction Level Threshold
URL: https://github.com/apache/carbondata/pull/3553#issuecomment-570054610
 
 
   Build Failed  with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/1385/
   

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3553: [CARBONDATA-3648] Support Alter Table Compaction Level Threshold
URL: https://github.com/apache/carbondata/pull/3553#issuecomment-570054689
 
 
   Build Failed  with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1407/
   

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3553: [CARBONDATA-3648] Support Alter Table Compaction Level Threshold
URL: https://github.com/apache/carbondata/pull/3553#issuecomment-570054753
 
 
   Build Failed with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/1395/
   

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

marchpure commented on issue #3553: [CARBONDATA-3648] Support Alter Table Compaction Level Threshold
URL: https://github.com/apache/carbondata/pull/3553#issuecomment-570103346
 
 
   retest this please

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3553: [CARBONDATA-3648] Support Alter Table Compaction Level Threshold
URL: https://github.com/apache/carbondata/pull/3553#issuecomment-570111636
 
 
   Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/1386/
   

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3553: [CARBONDATA-3648] Support Alter Table Compaction Level Threshold
URL: https://github.com/apache/carbondata/pull/3553#issuecomment-570115879
 
 
   Build Failed with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/1396/
   

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3553: [CARBONDATA-3648] Support Alter Table Compaction Level Threshold
URL: https://github.com/apache/carbondata/pull/3553#issuecomment-570116842
 
 
   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1408/
   

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

marchpure commented on a change in pull request #3553: [CARBONDATA-3648] Support Alter Table Compaction Level Threshold
URL: https://github.com/apache/carbondata/pull/3553#discussion_r362379914
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/util/path/CarbonTablePath.java
 ##########
 @@ -81,7 +81,7 @@ public static String getFolderContainingFile(String carbonFilePath) {
     int lastIndex = carbonFilePath.lastIndexOf('/');
     // below code for handling windows environment
     if (-1 == lastIndex) {
-      lastIndex = carbonFilePath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR);
+      lastIndex = carbonFilePath.lastIndexOf(CarbonCommonConstants.WINDOWS_FILE_SEPARATOR);
 
 Review comment:
   Yes, java.io.File.separator is more elegant. I have modified code to use java.io.File.separator as you suggested.

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

ydvpankaj97 commented on issue #3553: [CARBONDATA-3648] Support Alter Table Compaction Level Threshold
URL: https://github.com/apache/carbondata/pull/3553#issuecomment-570473543
 
 
   retest this please

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

ydvpankaj97 removed a comment on issue #3553: [CARBONDATA-3648] Support Alter Table Compaction Level Threshold
URL: https://github.com/apache/carbondata/pull/3553#issuecomment-570473543
 
 
   retest this please

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

jackylk commented on a change in pull request #3553: [CARBONDATA-3648] Support Alter Table Compaction Level Threshold
URL: https://github.com/apache/carbondata/pull/3553#discussion_r363097882
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java
 ##########
 @@ -802,6 +802,17 @@ private CarbonCommonConstants() {
    */
   public static final String DEFAULT_SEGMENT_LEVEL_THRESHOLD = "4,3";
 
+
+  /**
+   * Min value of Number of segments to be merged in levels
 
 Review comment:
   What does `in levels` mean? Can you rephrase it?

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

ajantha-bhat commented on a change in pull request #3553: [CARBONDATA-3648] Support Alter Table Compaction Level Threshold
URL: https://github.com/apache/carbondata/pull/3553#discussion_r363569674
 
 

 ##########
 File path: integration/spark2/src/main/scala/org/apache/spark/util/AlterTableUtil.scala
 ##########
 @@ -533,6 +533,7 @@ object AlterTableUtil {
       "COMMENT",
       "COLUMN_META_CACHE",
       "CACHE_LEVEL",
+      "COMPACTION_LEVEL_THRESHOLD",
 
 Review comment:
   Here it is added as a table properties right ? Need to add validation also ?
   I guess now it will accept any value.

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services