You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@carbondata.apache.org by GitBox <gi...@apache.org> on 2020/08/25 13:09:20 UTC

[GitHub] [carbondata] QiangCai opened a new pull request #3898: [CARBONDATA-3960] Default column comment should be null

QiangCai opened a new pull request #3898:
URL: https://github.com/apache/carbondata/pull/3898


    ### Why is this PR needed?
   1. column comment is an empty string by default when adding a column
   2. there are too many redundancy codes
   
    ### What changes were proposed in this PR?
   1. change default column comment to null
   2. re-factory code
   
    ### 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:
users@infra.apache.org

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3898: [CARBONDATA-3960] Default comment should be null when adding columns

Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on pull request #3898:
URL: https://github.com/apache/carbondata/pull/3898#issuecomment-680080019


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


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [carbondata] akashrn5 commented on pull request #3898: [CARBONDATA-3960] Default comment should be null when adding columns

Posted by GitBox <gi...@apache.org>.
akashrn5 commented on pull request #3898:
URL: https://github.com/apache/carbondata/pull/3898#issuecomment-684935390


   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:
users@infra.apache.org

[GitHub] [carbondata] QiangCai commented on a change in pull request #3898: [CARBONDATA-3960] Default comment should be null when adding columns

Posted by GitBox <gi...@apache.org>.
QiangCai commented on a change in pull request #3898:
URL: https://github.com/apache/carbondata/pull/3898#discussion_r479598589



##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/parser/CarbonSpark2SqlParser.scala
##########
@@ -693,7 +693,7 @@ class CarbonSpark2SqlParser extends CarbonDDLSqlParser {
       var columnComment: String = ""
       var plainComment: String = ""
       if (col.getComment().isDefined) {
-        columnComment = " comment \"" + col.getComment().get + "\""
+        columnComment = " comment '" + col.getComment().get + "'"

Review comment:
       Double quotes will lead column comments to be null in some spark versions.
   
   




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3898: [CARBONDATA-3960] Default comment should be null when adding columns

Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on pull request #3898:
URL: https://github.com/apache/carbondata/pull/3898#issuecomment-680082252


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


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [carbondata] asfgit closed pull request #3898: [CARBONDATA-3960] Default comment should be null when adding columns

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #3898:
URL: https://github.com/apache/carbondata/pull/3898


   


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3898: [CARBONDATA-3960] Default comment should be null when adding columns

Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on pull request #3898:
URL: https://github.com/apache/carbondata/pull/3898#issuecomment-685014264


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


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3898: [CARBONDATA-3960] Default comment should be null when adding columns

Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on pull request #3898:
URL: https://github.com/apache/carbondata/pull/3898#issuecomment-684845760


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


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [carbondata] akashrn5 commented on a change in pull request #3898: [CARBONDATA-3960] Default comment should be null when adding columns

Posted by GitBox <gi...@apache.org>.
akashrn5 commented on a change in pull request #3898:
URL: https://github.com/apache/carbondata/pull/3898#discussion_r480909454



##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/carbonTableSchemaCommon.scala
##########
@@ -624,9 +625,7 @@ class TableNewProcessor(cm: TableModel) {
     if (isVarcharColumn(colName)) {
       columnSchema.setDataType(DataTypes.VARCHAR)
     }
-    // TODO: Need to fill RowGroupID, converted type

Review comment:
       is this `TODO` is completed/not required?




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3898: [CARBONDATA-3960] Default comment should be null when adding columns

Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on pull request #3898:
URL: https://github.com/apache/carbondata/pull/3898#issuecomment-684838466


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


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3898: [CARBONDATA-3960] Default comment should be null when adding columns

Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on pull request #3898:
URL: https://github.com/apache/carbondata/pull/3898#issuecomment-685018314


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


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [carbondata] akashrn5 commented on a change in pull request #3898: [CARBONDATA-3960] Default comment should be null when adding columns

Posted by GitBox <gi...@apache.org>.
akashrn5 commented on a change in pull request #3898:
URL: https://github.com/apache/carbondata/pull/3898#discussion_r478994240



##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/parser/CarbonSpark2SqlParser.scala
##########
@@ -693,7 +693,7 @@ class CarbonSpark2SqlParser extends CarbonDDLSqlParser {
       var columnComment: String = ""
       var plainComment: String = ""
       if (col.getComment().isDefined) {
-        columnComment = " comment \"" + col.getComment().get + "\""
+        columnComment = " comment '" + col.getComment().get + "'"

Review comment:
       why this change? I didn't get the reason




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [carbondata] akashrn5 commented on pull request #3898: [CARBONDATA-3960] Default comment should be null when adding columns

Posted by GitBox <gi...@apache.org>.
akashrn5 commented on pull request #3898:
URL: https://github.com/apache/carbondata/pull/3898#issuecomment-685326743


   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:
users@infra.apache.org

[GitHub] [carbondata] QiangCai commented on a change in pull request #3898: [CARBONDATA-3960] Default comment should be null when adding columns

Posted by GitBox <gi...@apache.org>.
QiangCai commented on a change in pull request #3898:
URL: https://github.com/apache/carbondata/pull/3898#discussion_r481766413



##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/carbonTableSchemaCommon.scala
##########
@@ -624,9 +625,7 @@ class TableNewProcessor(cm: TableModel) {
     if (isVarcharColumn(colName)) {
       columnSchema.setDataType(DataTypes.VARCHAR)
     }
-    // TODO: Need to fill RowGroupID, converted type

Review comment:
       not required now




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [carbondata] akashrn5 commented on pull request #3898: [CARBONDATA-3960] Default comment should be null when adding columns

Posted by GitBox <gi...@apache.org>.
akashrn5 commented on pull request #3898:
URL: https://github.com/apache/carbondata/pull/3898#issuecomment-684773205


   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:
users@infra.apache.org