[GitHub] [carbondata] kunal642 opened a new pull request #3822: [CARBONDATA-3887] Fixed insert failure for global sort null data

[GitBox]

kunal642 opened a new pull request #3822:
URL: https://github.com/apache/carbondata/pull/3822


    ### Why is this PR needed?
    Load data is failing with "Unsupported dataType: String" exception when null data is loaded into a string column in global_sort table as there is No handling for null data for string column
   
    ### What changes were proposed in this PR?
    Added a check for null data and handle for the same in global sort flow.
   
    ### Does this PR introduce any user interface change?
    - No
    - Yes. (please explain the change and update document)
   
    ### Is any new testcase added?
    - No
    - 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]

[GitBox]

CarbonDataQA1 commented on pull request #3822:
URL: https://github.com/apache/carbondata/pull/3822#issuecomment-653478117


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/1561/
   


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

[GitBox]

CarbonDataQA1 commented on pull request #3822:
URL: https://github.com/apache/carbondata/pull/3822#issuecomment-653479249


   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/3298/
   


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

[GitBox]

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



##########
File path: integration/spark/src/main/scala/org/apache/carbondata/spark/util/CommonUtil.scala
##########
@@ -921,52 +921,55 @@ object CommonUtil {
     var i = 0
     val fieldTypesLen = fields.length
     while (i < fieldTypesLen) {
-      if (!row.isNullAt(i)) {
-        fields(i).dataType match {
-          case StringType =>
-            data(i) = DataTypeUtil.getBytesDataDataTypeForNoDictionaryColumn(row.getString(i),
+      fields(i).dataType match {

Review comment:
       I think initially, the if check of null at row was there for all datatypes, now handled for string, other data types will be handled?




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

[GitBox]

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



##########
File path: integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/dataload/TestGlobalSortDataLoad.scala
##########
@@ -468,6 +470,16 @@ class TestGlobalSortDataLoad extends QueryTest with BeforeAndAfterEach with Befo
       sql("SELECT * FROM carbon_localsort_difftypes ORDER BY shortField"))
   }
 
+  test("test global sort with null values") {

Review comment:
       maybe can add a test case with other datatypes null value if not present in the current code.




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

[GitBox]

CarbonDataQA1 commented on pull request #3822:
URL: https://github.com/apache/carbondata/pull/3822#issuecomment-656371645


   Build Failed  with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/3341/
   


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

[GitBox]

CarbonDataQA1 commented on pull request #3822:
URL: https://github.com/apache/carbondata/pull/3822#issuecomment-656372214


   Build Failed  with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/1601/
   


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

[GitBox]

brijoobopanna commented on pull request #3822:
URL: https://github.com/apache/carbondata/pull/3822#issuecomment-656583724


   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]

[GitBox]

CarbonDataQA1 commented on pull request #3822:
URL: https://github.com/apache/carbondata/pull/3822#issuecomment-656636487


   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/3349/
   


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

[GitBox]

CarbonDataQA1 commented on pull request #3822:
URL: https://github.com/apache/carbondata/pull/3822#issuecomment-656637531


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/1609/
   


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

[GitBox]

kunal642 commented on a change in pull request #3822:
URL: https://github.com/apache/carbondata/pull/3822#discussion_r453299220



##########
File path: integration/spark/src/main/scala/org/apache/carbondata/spark/util/CommonUtil.scala
##########
@@ -921,52 +921,55 @@ object CommonUtil {
     var i = 0
     val fieldTypesLen = fields.length
     while (i < fieldTypesLen) {
-      if (!row.isNullAt(i)) {
-        fields(i).dataType match {
-          case StringType =>
-            data(i) = DataTypeUtil.getBytesDataDataTypeForNoDictionaryColumn(row.getString(i),
+      fields(i).dataType match {

Review comment:
       It is already handled for other types..Test cases is added

##########
File path: integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/dataload/TestGlobalSortDataLoad.scala
##########
@@ -468,6 +470,16 @@ class TestGlobalSortDataLoad extends QueryTest with BeforeAndAfterEach with Befo
       sql("SELECT * FROM carbon_localsort_difftypes ORDER BY shortField"))
   }
 
+  test("test global sort with null values") {

Review comment:
       done




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

[GitBox]

akashrn5 commented on pull request #3822:
URL: https://github.com/apache/carbondata/pull/3822#issuecomment-657383823


   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]

[GitBox]

CarbonDataQA1 commented on pull request #3822:
URL: https://github.com/apache/carbondata/pull/3822#issuecomment-657413267


   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/3366/
   


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

[GitBox]

CarbonDataQA1 commented on pull request #3822:
URL: https://github.com/apache/carbondata/pull/3822#issuecomment-657415115


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/1625/
   


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

[GitBox]

ajantha-bhat commented on pull request #3822:
URL: https://github.com/apache/carbondata/pull/3822#issuecomment-657475454


   LGTM


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

[GitBox]

akashrn5 commented on pull request #3822:
URL: https://github.com/apache/carbondata/pull/3822#issuecomment-657475860


   LGTM


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

[GitBox]

asfgit closed pull request #3822:
URL: https://github.com/apache/carbondata/pull/3822


   


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