New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CARBONDATA-2262] Support the syntax of 'STORED AS CARBONDATA' to create table #2078
Conversation
Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3193/ |
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/4427/ |
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/3948/ |
Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3202/ |
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/4435/ |
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/3956/ |
Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3206/ |
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/4439/ |
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/3961/ |
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/4449/ |
Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3216/ |
retest this please |
Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3219/ |
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/4452/ |
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/3968/ |
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/3970/ |
@jackylk Please review it. |
@@ -91,6 +91,9 @@ class CarbonHelperSqlAstBuilder(conf: SQLConf, | |||
case Some(value) => | |||
if (value.children.get(1).getText.equalsIgnoreCase("by")) { | |||
value.storageHandler().STRING().getSymbol.getText | |||
} else if (value.children.get(1).getText.equalsIgnoreCase("as") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you call value.children.get(1).getText
once and use the result
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, done
@@ -326,7 +326,8 @@ class CarbonSqlAstBuilder(conf: SQLConf, parser: CarbonSpark2SqlParser, sparkSes | |||
val fileStorage = helper.getFileStorage(ctx.createFileFormat) | |||
|
|||
if (fileStorage.equalsIgnoreCase("'carbondata'") || | |||
fileStorage.equalsIgnoreCase("'org.apache.carbondata.format'")) { | |||
fileStorage.equalsIgnoreCase("carbondata") || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
incorrect identation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line 329 is repeated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's different: 'carbondata' and carbondata.
carbon supported syntax "stored by 'carbondata'" before, but didn't supported syntax "stored by carbondata".
we need support syntax "STORED AS carbondata" like "STORED AS parquet"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stored by carbondata
is not required
@@ -325,7 +325,8 @@ class CarbonSqlAstBuilder(conf: SQLConf, parser: CarbonSpark2SqlParser, sparkSes | |||
val fileStorage = helper.getFileStorage(ctx.createFileFormat) | |||
|
|||
if (fileStorage.equalsIgnoreCase("'carbondata'") || | |||
fileStorage.equalsIgnoreCase("'org.apache.carbondata.format'")) { | |||
fileStorage.equalsIgnoreCase("carbondata") || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
repeated logic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's different: 'carbondata' and carbondata.
|
||
test("CARBONDATA-734: Don't Support the syntax of 'STORED BY 'PARQUET''") { | ||
try { | ||
sql("CREATE TABLE src_pqt(key INT, value STRING) STORED AS 'PARQUET'") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not correct, stored as parquet or orc should be supported
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now carbon support syntax "stored as parquet", but don't support syntax "stored as 'parquet'".
Whether do we support syntax "stored as 'parquet'"?
Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3263/ |
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/4492/ |
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4004/ |
retest this please |
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/4504/ |
Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3275/ |
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/4510/ |
Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3282/ |
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4016/ |
retest this please |
retest sdv please |
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/4513/ |
Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3285/ |
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4018/ |
retest this please |
retest sdv please |
Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3299/ |
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/4527/ |
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4027/ |
…ate table fix scalastyle fix spark2.1 error optimize optimize optimize according review comment optimize
f7d203a
to
67b4615
Compare
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/4583/ |
Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3357/ |
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4079/ |
retest sdv please |
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4081/ |
LGTM |
…ate table This closes apache#2078
…ate table This closes apache#2078
Support the syntax of 'STORED AS CARBONDATA' to create table, for example:
sql("CREATE TABLE carbon_table(key INT, value STRING) STORED AS CARBONDATA")
Improving compatibility with Hive and giving better user experience
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
Please provide details on
- Whether new unit test cases have been added or why no new tests are required?
- How it is tested? Please attach test report.
- Is it a performance related change? Please attach the performance test report.
- Any additional information to help reviewers in testing this change.
For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.