[feature](external catalog)Add partition grammar for external catalog to create table #31585
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for your contribution to Apache Doris. |
morrySnow
reviewed
Feb 29, 2024
| : functions+=partitionFunction (COMMA functions+=partitionFunction)* | ||
| ; | ||
|
|
||
| partitionFunction |
Contributor
There was a problem hiding this comment.
it is better use functionCallExpression directly, and in LogicalPlanBuilder check return expression is UnboundFunction
| @Override | ||
| public Expression visitPartitionFunction(DorisParser.PartitionFunctionContext ctx) { | ||
| String name = ctx.identity.getText(); | ||
| List<Expression> params = visit(ctx.expression(), Expression.class); |
Contributor
There was a problem hiding this comment.
do u test this syntax? i think u want get is ctx.arguments()
Comment on lines
725
to
732
| partitionFunctionCallExpression | ||
| : LEFT_PAREN partitionFunctionList RIGHT_PAREN | ||
| ; | ||
|
|
||
| partitionFunctionList | ||
| : functions+=partitionFunction (COMMA functions+=partitionFunction)* | ||
| ; | ||
|
|
Contributor
There was a problem hiding this comment.
why not merge this two statement into one?
LEFT_PAREN functions+=partitionFunction (COMMA functions+=partitionFunction)* RIGHT_PAREN
| partitionType, | ||
| ctx.partitionKeys != null ? visitIdentifierList(ctx.partitionKeys) : null, | ||
| ctx.partitions != null ? visitPartitionsDef(ctx.partitions) : null, | ||
| partitionInfo.isAutoPartition(), |
Contributor
There was a problem hiding this comment.
if u aready merge all partition info into one class, u'd better update CreateTableInfo's ctor to accept PartitionTableInfo as parameter
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 32082 ms |
TPC-DS: Total hot run time: 189294 ms |
ClickBench: Total hot run time: 26.62 s |
Contributor
Author
|
run buildall |
3 tasks
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 37617 ms |
TPC-DS: Total hot run time: 178480 ms |
ClickBench: Total hot run time: 30.21 s |
|
Load test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G' |
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 38091 ms |
TPC-DS: Total hot run time: 177078 ms |
ClickBench: Total hot run time: 31.51 s |
|
Load test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G' |
Contributor
Author
|
run buildall |
morningman
previously approved these changes
Mar 6, 2024
| } | ||
| } else if (!engineName.equals("olap")) { | ||
| if (partitionDesc != null || distributionDesc != null) { | ||
| if (!engineName.equals("hms") && distributionDesc != null) { |
Contributor
There was a problem hiding this comment.
Suggested change
| if (!engineName.equals("hms") && distributionDesc != null) { | |
| if (!engineName.equals("hive") && distributionDesc != null) { |
| + " table should not contain partition or distribution desc"); | ||
| + " table should not contain distribution desc"); | ||
| } | ||
| if (!engineName.equals("hms") && !engineName.equals("iceberg") && partitionDesc != null) { |
Contributor
There was a problem hiding this comment.
Suggested change
| if (!engineName.equals("hms") && !engineName.equals("iceberg") && partitionDesc != null) { | |
| if (!engineName.equals("hive") && !engineName.equals("iceberg") && partitionDesc != null) { |
github-actions
bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
Contributor
Author
|
run buildall |
1 similar comment
Contributor
|
run buildall |
Contributor
|
run buildall |
1 similar comment
Contributor
Author
|
run buildall |
Contributor
Author
|
run p0 |
1 similar comment
Contributor
Author
|
run p0 |
github-actions
bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
yiguolei
pushed a commit
that referenced
this pull request
Mar 9, 2024
… to create table (#31585) The `PARTITION BY` syntax used by external catalogs has been added. You can specify a column directly, or a partition function as a partition condition. Like: `PARTITION BY LIST(col1, col2, func(param), func(param1, param2), func(param1, param2, param3))` NOTICE: This PR change the grammar of `AUTO PARTITION` From ``` AUTO PARTITION BY RANGE date_trunc(`TIME_STAMP`, 'month') ``` To ``` AUTO PARTITION BY RANGE (date_trunc(`TIME_STAMP`, 'month')) ```
zhangstar333
pushed a commit
that referenced
this pull request
Apr 1, 2024
fix legacy planner grammer fix nereids planner parsing fix cases forbid auto range partition with null column fix CreateTableStmt with auto partition and some partition items. 1 and 2 are about #31585 doc pr: apache/doris-website#488
zclllyybb
added a commit
to zclllyybb/doris
that referenced
this pull request
Apr 1, 2024
…e#32737) fix legacy planner grammer fix nereids planner parsing fix cases forbid auto range partition with null column fix CreateTableStmt with auto partition and some partition items. 1 and 2 are about apache#31585 doc pr: apache/doris-website#488
zclllyybb
added a commit
to zclllyybb/doris
that referenced
this pull request
Apr 9, 2024
…e#32737) fix legacy planner grammer fix nereids planner parsing fix cases forbid auto range partition with null column fix CreateTableStmt with auto partition and some partition items. 1 and 2 are about apache#31585 doc pr: apache/doris-website#488
yiguolei
pushed a commit
that referenced
this pull request
Apr 9, 2024
… (#33412) fix legacy planner grammer fix nereids planner parsing fix cases forbid auto range partition with null column fix CreateTableStmt with auto partition and some partition items. 1 and 2 are about #31585 doc pr: apache/doris-website#488
zclllyybb
added a commit
to zclllyybb/doris
that referenced
this pull request
Apr 11, 2024
…e#32737) fix legacy planner grammer fix nereids planner parsing fix cases forbid auto range partition with null column fix CreateTableStmt with auto partition and some partition items. 1 and 2 are about apache#31585 doc pr: apache/doris-website#488
w41ter
added a commit
to w41ter/incubator-doris
that referenced
this pull request
Apr 24, 2024
AUTO PARTITION grammar has changed since apache#31585, but the output of SHOW CREATE TABLE was left out to change, so the result is not able to be recognized by the FE parser.
dataroaring
pushed a commit
that referenced
this pull request
Apr 25, 2024
AUTO PARTITION grammar has changed since #31585, but the output of SHOW CREATE TABLE was left out to change, so the result is not able to be recognized by the FE parser.
yiguolei
pushed a commit
that referenced
this pull request
Apr 25, 2024
AUTO PARTITION grammar has changed since #31585, but the output of SHOW CREATE TABLE was left out to change, so the result is not able to be recognized by the FE parser.
dataroaring
pushed a commit
that referenced
this pull request
Apr 27, 2024
AUTO PARTITION grammar has changed since #31585, but the output of SHOW CREATE TABLE was left out to change, so the result is not able to be recognized by the FE parser.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes
The
PARTITION BYsyntax used by external catalogs has been added.You can specify a column directly, or a partition function as a partition condition.
Like:
PARTITION BY LIST(col1, col2, func(param), func(param1, param2), func(param1, param2, param3))NOTICE:
This PR change the grammar of
AUTO PARTITIONFrom
To
Further comments
If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...