Skip to content
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-2908]the option of sort_scope don't effects while creating table by dataframe #2684

Closed
wants to merge 7 commits into from

Conversation

qiuchenjian
Copy link
Contributor

…g table by data frame

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.

@CarbonDataQA
Copy link

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/8232/

@CarbonDataQA
Copy link

Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/161/

@@ -81,6 +81,7 @@ class CarbonDataFrameWriter(sqlContext: SQLContext, val dataFrame: DataFrame) {
private def makeCreateTableString(schema: StructType, options: CarbonOption): String = {
val property = Map(
"SORT_COLUMNS" -> options.sortColumns,
"SORT_SCOPE" -> options.sortScope,
Copy link
Contributor

@jackylk jackylk Sep 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From this PR, it seems that carbon will not output error message even if there is some table properties it can not recognized during create table by DataFrame API. I think it is better to add a check for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't list all of the properties of table, any class record this ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can refer to CREATE TABLE section in http://carbondata.apache.org/data-management-on-carbondata.html

@qiuchenjian
Copy link
Contributor Author

@jackylk properties of table can be add to TableProperty in schema.thrift

@CarbonDataQA
Copy link

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/8306/

@CarbonDataQA
Copy link

Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/236/

@CarbonDataQA
Copy link

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/8/

@qiuchenjian
Copy link
Contributor Author

@jackylk revert the change of thrift, only solve sortScope

@qiuchenjian
Copy link
Contributor Author

retest this please

@CarbonDataQA
Copy link

Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/266/

@CarbonDataQA
Copy link

Build Failed with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.3/8335/

@CarbonDataQA
Copy link

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/97/

@qiuchenjian
Copy link
Contributor Author

retest this please

@CarbonDataQA
Copy link

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/101/

@CarbonDataQA
Copy link

Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/269/

@CarbonDataQA
Copy link

Build Failed with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.3/8339/

@chenliang613
Copy link
Contributor

@qiuchenjian please update the pr's title, doesn't display completely.

@qiuchenjian qiuchenjian changed the title [CARBONDATA-2908]the option of sort_scope don't effects while creatin… [CARBONDATA-2908]the option of sort_scope don't effects while creating table by data frame Sep 7, 2018
@qiuchenjian qiuchenjian changed the title [CARBONDATA-2908]the option of sort_scope don't effects while creating table by data frame [CARBONDATA-2908]the option of sort_scope don't effects while creating table by dataframe Sep 7, 2018
@jackylk
Copy link
Contributor

jackylk commented Sep 8, 2018

LGTM

@jackylk
Copy link
Contributor

jackylk commented Sep 9, 2018

merged, thanks for working on this

@asfgit asfgit closed this in 0483b46 Sep 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants