[GitHub] [carbondata] ajantha-bhat opened a new pull request #3779: [TEMP] comparator changes

classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] ajantha-bhat opened a new pull request #3779: [TEMP] comparator changes

GitBox

ajantha-bhat opened a new pull request #3779:
URL: https://github.com/apache/carbondata/pull/3779


    ### Why is this PR needed?
   
   
    ### What changes were proposed in this PR?
   
       
    ### 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]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3779: [TEMP] comparator changes

GitBox

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






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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3779: [CARBONDATA-3835] Fix global sort string comparator issue

GitBox
In reply to this post by GitBox

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


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


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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3779: [CARBONDATA-3835] Fix global sort string comparator issue

GitBox
In reply to this post by GitBox

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


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


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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] QiangCai commented on a change in pull request #3779: [CARBONDATA-3835] Fix global sort string comparator issue

GitBox
In reply to this post by GitBox

QiangCai commented on a change in pull request #3779:
URL: https://github.com/apache/carbondata/pull/3779#discussion_r432825318



##########
File path: integration/spark/src/main/scala/org/apache/carbondata/spark/load/DataLoadProcessBuilderOnSpark.scala
##########
@@ -237,10 +238,18 @@ object DataLoadProcessBuilderOnSpark {
         CarbonProperties.getInstance().getGlobalSortRddStorageLevel()))
     }
     val sortColumnsLength = model.getCarbonDataLoadSchema.getCarbonTable.getSortColumns.size()
-    val sortColumnDataTypes = dataTypes.take(sortColumnsLength)
+    var sortColumnDataTypes = dataTypes.take(sortColumnsLength)
+    sortColumnDataTypes = sortColumnDataTypes.map { datatype =>
+      val updatedType = datatype match {
+        case StringType => ByteType
+        case TimestampType | DateType => LongType
+        case _ => datatype
+      }
+      updatedType
+    }

Review comment:
       how about to change it as following?
   ```scala
        sortColumnDataTypes = sortColumnDataTypes.map {
           case StringType => ByteType
           case TimestampType | DateType => LongType
           case datatype => datatype
       }
   ```




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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] ajantha-bhat commented on a change in pull request #3779: [CARBONDATA-3835] Fix global sort string comparator issue

GitBox
In reply to this post by GitBox

ajantha-bhat commented on a change in pull request #3779:
URL: https://github.com/apache/carbondata/pull/3779#discussion_r432825992



##########
File path: integration/spark/src/main/scala/org/apache/carbondata/spark/load/DataLoadProcessBuilderOnSpark.scala
##########
@@ -237,10 +238,18 @@ object DataLoadProcessBuilderOnSpark {
         CarbonProperties.getInstance().getGlobalSortRddStorageLevel()))
     }
     val sortColumnsLength = model.getCarbonDataLoadSchema.getCarbonTable.getSortColumns.size()
-    val sortColumnDataTypes = dataTypes.take(sortColumnsLength)
+    var sortColumnDataTypes = dataTypes.take(sortColumnsLength)
+    sortColumnDataTypes = sortColumnDataTypes.map { datatype =>
+      val updatedType = datatype match {
+        case StringType => ByteType
+        case TimestampType | DateType => LongType
+        case _ => datatype
+      }
+      updatedType
+    }

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]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3779: [CARBONDATA-3835] Fix global sort string comparator issue

GitBox
In reply to this post by GitBox

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


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


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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3779: [CARBONDATA-3835] Fix global sort string comparator issue

GitBox
In reply to this post by GitBox

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


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


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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3779: [CARBONDATA-3835] Fix global sort issues

GitBox
In reply to this post by GitBox

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


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


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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] ajantha-bhat commented on pull request #3779: [CARBONDATA-3835] Fix global sort issues

GitBox
In reply to this post by GitBox

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


   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]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3779: [CARBONDATA-3835] Fix global sort issues

GitBox
In reply to this post by GitBox

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


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


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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3779: [CARBONDATA-3835] Fix global sort issues

GitBox
In reply to this post by GitBox

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


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


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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3779: [CARBONDATA-3835] Fix global sort issues

GitBox
In reply to this post by GitBox

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


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/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]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3779: [CARBONDATA-3835] Fix global sort issues

GitBox
In reply to this post by GitBox

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


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


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


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] jackylk commented on pull request #3779: [CARBONDATA-3835] Fix global sort issues

GitBox
In reply to this post by GitBox

jackylk commented on pull request #3779:
URL: https://github.com/apache/carbondata/pull/3779#issuecomment-636452177


   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]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] QiangCai commented on pull request #3779: [CARBONDATA-3835] Fix global sort issues

GitBox
In reply to this post by GitBox

QiangCai commented on pull request #3779:
URL: https://github.com/apache/carbondata/pull/3779#issuecomment-636473791


   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]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] asfgit closed pull request #3779: [CARBONDATA-3835] Fix global sort issues

GitBox
In reply to this post by GitBox

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


   


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