Skip to content

Improve database handling in multi-stage engine #14040

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.

Sign up for GitHub

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

Jump to bottom
Merged
Jackie-Jiang merged 1 commit into from
Sep 23, 2024
Merged

Improve database handling in multi-stage engine #14040

Jackie-Jiang merged 1 commit into from
Sep 23, 2024

Conversation

@Jackie-Jiang
Copy link
Contributor

@Jackie-Jiang Jackie-Jiang commented Sep 19, 2024

Add a method to process the already split table name to avoid the overhead of splitting the table name again.

@Jackie-Jiang Jackie-Jiang added enhancement multi-stage Related to the multi-stage query engine labels Sep 19, 2024
@codecov-commenter
Copy link

codecov-commenter commented Sep 20, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 82.60870% with 4 lines in your changes missing coverage. Please review.

Project coverage is 65.06%. Comparing base (59551e4) to head (f3b78ac).
Report is 1069 commits behind head on master.

Files with missing lines Patch % Lines
...lcite/rel/rules/PinotRelDistributionTraitRule.java 66.66% 1 Missing and 2 partials ⚠️
.../query/planner/logical/RelToPlanNodeConverter.java 88.88% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##             master   #14040      +/-   ##
============================================
+ Coverage     61.75%   65.06%   +3.31%     
- Complexity      207     1533    +1326     
============================================
  Files          2436     2564     +128     
  Lines        133233   140757    +7524     
  Branches      20636    21608     +972     
============================================
+ Hits          82274    91582    +9308     
+ Misses        44911    42450    -2461     
- Partials       6048     6725     +677
Flag Coverage Δ
custom-integration1 100.00% <ø> (+99.99%) ⬆️
integration 100.00% <ø> (+99.99%) ⬆️
integration1 100.00% <ø> (+99.99%) ⬆️
integration2 0.00% <ø> (ø)
java-11 65.02% <82.60%> (+3.31%) ⬆️
java-21 64.92% <82.60%> (+3.29%) ⬆️
skip-bytebuffers-false 65.05% <82.60%> (+3.31%) ⬆️
skip-bytebuffers-true 64.87% <82.60%> (+37.15%) ⬆️
temurin 65.06% <82.60%> (+3.31%) ⬆️
unittests 65.05% <82.60%> (+3.31%) ⬆️
unittests1 56.57% <82.60%> (+9.68%) ⬆️
unittests2 35.02% <30.43%> (+7.28%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Jackie-Jiang
Copy link
Contributor Author

Jackie-Jiang commented Sep 20, 2024

The failing test has query: SELECT * FROM "default.tbl-escape-naming". IIUC this should explicitly look for table with name default.tbl-escape-naming which doesn't exist. @gortiz Do you think we should support this?

@gortiz
Copy link
Contributor

gortiz commented Sep 20, 2024

I think @shounakmk219 is more informed to answer that question.

@Jackie-Jiang
Copy link
Contributor Author

Jackie-Jiang commented Sep 20, 2024

Seems the test is added in #12591, and I believe the correct syntax should be SELECT * FROM default."tbl-escape-naming". Modified the test accordingly

@Jackie-Jiang Jackie-Jiang merged commit f618cf3 into apache:master Sep 23, 2024
@Jackie-Jiang Jackie-Jiang deleted the database_handling branch September 23, 2024 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gortiz gortiz gortiz approved these changes

@xiangfu0 xiangfu0 Awaiting requested review from xiangfu0

Assignees

No one assigned

Labels

enhancement multi-stage Related to the multi-stage query engine

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Jackie-Jiang @codecov-commenter @gortiz