[CARBONDATA-3174] Fix trailing space issue with varchar column for SDK #2988
Conversation
|
Can one of the admins verify this patch? |
|
add to whitelist |
|
retest this please |
|
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1761/ |
|
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1972/ |
|
Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10020/ |
| s"""CREATE TABLE test using carbon options('long_string_columns'='subject,messagebody') | ||
| |LOCATION '$writerPath'""" | ||
| .stripMargin) | ||
| sql("select * from test").show() |
There was a problem hiding this comment.
Why is this test case not checking the correctness of the results? Such as checkAnswer ， assert and so on
|
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1786/ |
|
Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10046/ |
|
Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1998/ |
|
retest this please |
|
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1793/ |
|
Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10053/ |
|
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2005/ |
| @@ -55,7 +55,7 @@ | |||
| * @param type datatype of field, specified in strings. | |||
| */ | |||
| public Field(String name, String type) { | |||
| this.name = name; | |||
| this.name = name.toLowerCase().trim(); | |||
There was a problem hiding this comment.
CarbonWriterBuilder.updateSchemaFields() is already converting to lowercase, just add trim in that method. No need to handle for each here.
| @@ -2490,6 +2490,54 @@ class TestNonTransactionalCarbonTable extends QueryTest with BeforeAndAfterAll { | |||
| FileUtils.deleteDirectory(new File(writerPath)) | |||
| } | |||
|
|
|||
| test("check varchar with trailing space") { | |||
There was a problem hiding this comment.
No need to duplicate test cases. In the existing varchar columns test case, add a trailing space to one of the columns.
There was a problem hiding this comment.
besides, this is for varchar columns, why not update the code there?
|
@Shubh18s : why for only varchar columns ? how it was handled other columns ? I guess this problem is there for other columns also |
|
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1801/ |
|
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2013/ |
|
Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10062/ |
|
@Shubh18s : I have checked the code, After this change, sort_columns and invertedIndexFor are affected. |
| @@ -747,7 +747,7 @@ private Schema updateSchemaFields(Schema schema, Set<String> longStringColumns) | |||
| Field[] fields = schema.getFields(); | |||
| for (int i = 0; i < fields.length; i++) { | |||
| if (fields[i] != null) { | |||
| fields[i].updateNameToLowerCase(); | |||
| //fields[i].updateName(); | |||
|
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1814/ |
|
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1815/ |
|
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2025/ |
|
Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10074/ |
|
LGTM |
1 similar comment
|
LGTM |
What was the issue? After doing SDK Write, Select * was failing for 'long_string_columns' with trailing space. What has been changed? Removed the trailing space in ColumnName. This closes #2988
What was the issue? After doing SDK Write, Select * was failing for 'long_string_columns' with trailing space. What has been changed? Removed the trailing space in ColumnName. This closes apache#2988
What was the issue?
After doing SDK Write, Select * was failing for 'long_string_columns' with trailing space.
What has been changed?
Removed the trailing space in ColumnName.
Be sure to do all of the following checklist to help us incorporate
your contribution quickly and easily:
Any interfaces changed?
Any backward compatibility impacted?
Document update required?
Testing done
Added a test case.
For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.