akashrn5 opened a new pull request #3962: URL: https://github.com/apache/carbondata/pull/3962 ### 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] |
CarbonDataQA1 commented on pull request #3962: URL: https://github.com/apache/carbondata/pull/3962#issuecomment-700164625 Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/4259/ ---------------------------------------------------------------- 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] |
In reply to this post by GitBox
CarbonDataQA1 commented on pull request #3962: URL: https://github.com/apache/carbondata/pull/3962#issuecomment-700169378 Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2514/ ---------------------------------------------------------------- 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] |
In reply to this post by GitBox
CarbonDataQA1 commented on pull request #3962: URL: https://github.com/apache/carbondata/pull/3962#issuecomment-701344984 Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/4283/ ---------------------------------------------------------------- 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] |
In reply to this post by GitBox
CarbonDataQA1 commented on pull request #3962: URL: https://github.com/apache/carbondata/pull/3962#issuecomment-701346144 Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2538/ ---------------------------------------------------------------- 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] |
In reply to this post by GitBox
Indhumathi27 commented on a change in pull request #3962: URL: https://github.com/apache/carbondata/pull/3962#discussion_r498158964 ########## File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestSIWithSecondryIndex.scala ########## @@ -423,6 +423,17 @@ class TestSIWithSecondryIndex extends QueryTest with BeforeAndAfterAll { sql("drop table table2") } + test("test SI creation with special char column") { + sql("drop table if exists special_char") Review comment: can add drop table to afterAll also ########## File path: core/src/main/java/org/apache/carbondata/core/util/CarbonUtil.java ########## @@ -1881,7 +1881,9 @@ public static TableInfo convertGsonToTableInfo(Map<String, String> properties) { gsonBuilder.registerTypeAdapter(DataType.class, new DataTypeAdapter()); Gson gson = gsonBuilder.create(); - TableInfo tableInfo = gson.fromJson(builder.toString(), TableInfo.class); + // if the column name contains backslash in the column name, then fromJson will remove that, + // so replace like below to keep the "\" in column name and write the proper name in the schema + TableInfo tableInfo = gson.fromJson(builder.toString().replace("\\", "\\\\"), TableInfo.class); Review comment: can you add a test case with carbon session ########## File path: integration/spark/src/main/scala/org/apache/spark/sql/secondaryindex/command/SICreationCommand.scala ########## @@ -340,7 +336,7 @@ private[sql] case class CarbonCreateSecondaryIndexCommand( try { sparkSession.sql( s"""CREATE TABLE $databaseName.$indexTableName - |(${ fields.mkString(",") }) + |(${ rawSchema }) Review comment: ```suggestion ($rawSchema) ``` ---------------------------------------------------------------- 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] |
In reply to this post by GitBox
akashrn5 commented on a change in pull request #3962: URL: https://github.com/apache/carbondata/pull/3962#discussion_r498175600 ########## File path: core/src/main/java/org/apache/carbondata/core/util/CarbonUtil.java ########## @@ -1881,7 +1881,9 @@ public static TableInfo convertGsonToTableInfo(Map<String, String> properties) { gsonBuilder.registerTypeAdapter(DataType.class, new DataTypeAdapter()); Gson gson = gsonBuilder.create(); - TableInfo tableInfo = gson.fromJson(builder.toString(), TableInfo.class); + // if the column name contains backslash in the column name, then fromJson will remove that, + // so replace like below to keep the "\" in column name and write the proper name in the schema + TableInfo tableInfo = gson.fromJson(builder.toString().replace("\\", "\\\\"), TableInfo.class); Review comment: actually `QueryTest ` has spark session without `carbonSession`, and we cannot create another `sparkSession `when one is running. In example we cant add as test case, so i think since i have added the proper comment, it should be fine i guess. ########## File path: index/secondary-index/src/test/scala/org/apache/carbondata/spark/testsuite/secondaryindex/TestSIWithSecondryIndex.scala ########## @@ -423,6 +423,17 @@ class TestSIWithSecondryIndex extends QueryTest with BeforeAndAfterAll { sql("drop table table2") } + test("test SI creation with special char column") { + sql("drop table if exists special_char") 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] |
In reply to this post by GitBox
Indhumathi27 commented on a change in pull request #3962: URL: https://github.com/apache/carbondata/pull/3962#discussion_r498176312 ########## File path: core/src/main/java/org/apache/carbondata/core/util/CarbonUtil.java ########## @@ -1881,7 +1881,9 @@ public static TableInfo convertGsonToTableInfo(Map<String, String> properties) { gsonBuilder.registerTypeAdapter(DataType.class, new DataTypeAdapter()); Gson gson = gsonBuilder.create(); - TableInfo tableInfo = gson.fromJson(builder.toString(), TableInfo.class); + // if the column name contains backslash in the column name, then fromJson will remove that, + // so replace like below to keep the "\" in column name and write the proper name in the schema + TableInfo tableInfo = gson.fromJson(builder.toString().replace("\\", "\\\\"), TableInfo.class); Review comment: you can add in CarbonSessionExample ---------------------------------------------------------------- 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] |
In reply to this post by GitBox
Indhumathi27 commented on a change in pull request #3962: URL: https://github.com/apache/carbondata/pull/3962#discussion_r498178139 ########## File path: core/src/main/java/org/apache/carbondata/core/util/CarbonUtil.java ########## @@ -1881,7 +1881,9 @@ public static TableInfo convertGsonToTableInfo(Map<String, String> properties) { gsonBuilder.registerTypeAdapter(DataType.class, new DataTypeAdapter()); Gson gson = gsonBuilder.create(); - TableInfo tableInfo = gson.fromJson(builder.toString(), TableInfo.class); + // if the column name contains backslash in the column name, then fromJson will remove that, + // so replace like below to keep the "\" in column name and write the proper name in the schema + TableInfo tableInfo = gson.fromJson(builder.toString().replace("\\", "\\\\"), TableInfo.class); Review comment: ok ---------------------------------------------------------------- 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] |
In reply to this post by GitBox
CarbonDataQA1 commented on pull request #3962: URL: https://github.com/apache/carbondata/pull/3962#issuecomment-702138034 Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/4291/ ---------------------------------------------------------------- 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] |
In reply to this post by GitBox
CarbonDataQA1 commented on pull request #3962: URL: https://github.com/apache/carbondata/pull/3962#issuecomment-702138964 Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2544/ ---------------------------------------------------------------- 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] |
In reply to this post by GitBox
Indhumathi27 commented on pull request #3962: URL: https://github.com/apache/carbondata/pull/3962#issuecomment-704028143 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] |
In reply to this post by GitBox
CarbonDataQA1 commented on pull request #3962: URL: https://github.com/apache/carbondata/pull/3962#issuecomment-704068740 Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/4307/ ---------------------------------------------------------------- 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] |
In reply to this post by GitBox
CarbonDataQA1 commented on pull request #3962: URL: https://github.com/apache/carbondata/pull/3962#issuecomment-704070143 Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2558/ ---------------------------------------------------------------- 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] |
In reply to this post by GitBox
Indhumathi27 commented on pull request #3962: URL: https://github.com/apache/carbondata/pull/3962#issuecomment-704178705 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] |
In reply to this post by GitBox
asfgit closed pull request #3962: URL: https://github.com/apache/carbondata/pull/3962 ---------------------------------------------------------------- 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] |
Free forum by Nabble | Edit this page |