Skip to content
This repository has been archived by the owner on Sep 18, 2023. It is now read-only.

[NSE-1075] Support dynamic merge file partition #1081

Merged
merged 6 commits into from
Sep 1, 2022

Conversation

jackylee-ch
Copy link
Contributor

What changes were proposed in this pull request?

Support merge file partition dynamically.

How was this patch tested?

unit tests.

@github-actions
Copy link

Thanks for opening a pull request!

Could you open an issue for this pull request on Github Issues?

https://github.com/oap-project/native-sql-engine/issues

Then could you also rename commit message and pull request title in the following format?

[NSE-${ISSUES_ID}] ${detailed message}

See also:

@jackylee-ch jackylee-ch changed the title [NSC-1075] Support dynamic merge file partition [NSE-1075] Support dynamic merge file partition Aug 24, 2022
@github-actions
Copy link

#1075

@PHILO-HE
Copy link
Collaborator

Thanks for your PR!
A small suggestion, I note two new configs are introduced in this PR. Could you put these two configs into GazellePluginConfig and leave some comments for users? It will make the configs more clear and we can also uniformly set a default value for them at GazellePluginConfig.
When your patch is merged into upstream and gazelle's supported spark version is upgraded, we can make a cleanup then.

@jackylee-ch
Copy link
Contributor Author

Thanks for your PR! A small suggestion, I note two new configs are introduced in this PR. Could you put these two configs into GazellePluginConfig and leave some comments for users? It will make the configs more clear and we can also uniformly set a default value for them at GazellePluginConfig. When your patch is merged into upstream and gazelle's supported spark version is upgraded, we can make a cleanup then.

Thanks for your advice. The GazellePluginConfig cannot be imported here. I have add the config to ArrowSQLConf.

@PHILO-HE
Copy link
Collaborator

OK. Let's keep using ArrowSQLConf in this patch. GazellePluginConfig can be available to be used here by moving it into a separate module where common classes are kept, which can be refactored in other PR.
Will go through internal tests.

@PHILO-HE
Copy link
Collaborator

All internal tests passed. @zhouyuan, please have a check further.

@zhouyuan zhouyuan merged commit 45a1762 into oap-project:main Sep 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants