[Enhancement] (nereids)implement showCreateTableCommand in nereids #43137
Conversation
This file contains 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. Since 2024-03-18, the Document has been moved to doris-website. |
|
run buildall |
TPC-H: Total hot run time: 41366 ms |
TPC-DS: Total hot run time: 196696 ms |
ClickBench: Total hot run time: 32.35 s |
starocean999
reviewed
Nov 4, 2024
| } | ||
|
|
||
| @Override | ||
| public void run(ConnectContext ctx, StmtExecutor executor) throws Exception { |
There was a problem hiding this comment.
ShowCreateTableStmt's analyze method needs to be re-implemented in nereids. For example, add a private void validate(ConnectContext ctx) throws UserException method to do the job, and call it at the beginning of run method.
There was a problem hiding this comment.
Done - please review
msridhar78
force-pushed
the
show_create_table
branch
4 times, most recently
from
84e8c15 to
4f8cbc3
Compare
| public ShowResultSet doRun(ConnectContext ctx, StmtExecutor executor) throws Exception { | ||
| validate(ctx); | ||
| // If dbName is not provided, use the default database from the context | ||
| String effectiveDbName = (tblNameInfo.getDb() == null || tblNameInfo.getDb().isEmpty()) |
There was a problem hiding this comment.
please refer to handleShowCreateTable() in ShowExecutor.java. The old code just needs some minor changes, it's easier to reivew. thanks.
There was a problem hiding this comment.
done - please review
msridhar78
force-pushed
the
show_create_table
branch
from
4f8cbc3 to
6ff2719
Compare
|
|
||
| // Fetch the catalog, database, and table metadata | ||
| Env env = Env.getCurrentEnv(); | ||
| DatabaseIf db = env.getCatalogMgr().getCatalogOrAnalysisException(effectiveCatalog) |
There was a problem hiding this comment.
Suggested change
| DatabaseIf db = env.getCatalogMgr().getCatalogOrAnalysisException(effectiveCatalog) | |
| DatabaseIf db = env.getCatalogMgr().getCatalogOrAnalysisException(tblNameInfo.getCtl()) | |
| .getDbOrMetaException(tblNameInfo.getDb()); |
There was a problem hiding this comment.
done - pls review
| public ShowResultSet doRun(ConnectContext ctx, StmtExecutor executor) throws Exception { | ||
| validate(ctx); | ||
| // If dbName is not provided, use the default database from the context | ||
| String effectiveDbName = (tblNameInfo.getDb() == null || tblNameInfo.getDb().isEmpty()) |
There was a problem hiding this comment.
Suggested change
| String effectiveDbName = (tblNameInfo.getDb() == null || tblNameInfo.getDb().isEmpty()) |
There was a problem hiding this comment.
done - pls review
| ? ctx.getDatabase() : tblNameInfo.getDb(); | ||
|
|
||
| // If catalog is not provided, use the default catalog (e.g., internal) | ||
| String effectiveCatalog = (tblNameInfo.getCtl() == null || tblNameInfo.getCtl().isEmpty()) |
There was a problem hiding this comment.
Suggested change
| String effectiveCatalog = (tblNameInfo.getCtl() == null || tblNameInfo.getCtl().isEmpty()) |
msridhar78
force-pushed
the
show_create_table
branch
from
6ff2719 to
06c8ceb
Compare
|
run buildall |
|
pls resolve conflicts and regression cases fail |
msridhar78
force-pushed
the
show_create_table
branch
from
06c8ceb to
df61de2
Compare
|
run buildall |
msridhar78
force-pushed
the
show_create_table
branch
from
df61de2 to
9213e5e
Compare
|
run buildall |
1 similar comment
|
run buildall |
TPC-H: Total hot run time: 45237 ms |
|
run performance |
|
run p0 |
TPC-H: Total hot run time: 44763 ms |
|
run buildall |
TPC-H: Total hot run time: 45402 ms |
ClickBench: Total hot run time: 32.94 s |
starocean999
approved these changes
Nov 20, 2024
github-actions
bot
added
the
approved
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
LiBinfeng-01
approved these changes
Nov 20, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Issue Number: close #42736
implement showCreateTableCommand in nereids
Changed Session Variables:
2024-11-18 11:17:06.052 INFO [suite-thread-1] (Suite.groovy:387) - Execute sql: DROP TABLE IF EXISTS
tb_show_create_table2024-11-18 11:17:06.070 INFO [suite-thread-1] (ScriptContext.groovy:120) - Run test_show_create_table_nereids in /root/doris/doris/regression-test/suites/nereids_p0/show/test_show_create_table_nereids.groovy succeed
2024-11-18 11:17:25.342 INFO [main] (RegressionTest.groovy:281) - Start to run single scripts
2024-11-18 11:18:43.673 INFO [main] (RegressionTest.groovy:410) - Success suites:
/root/doris/doris/regression-test/suites/nereids_p0/show/test_show_create_table_nereids.groovy: group=query,arrow_flight_sql,p0, name=test_show_create_table_nereids
2024-11-18 11:18:43.691 INFO [main] (RegressionTest.groovy:492) - All suites success.