-
Notifications
You must be signed in to change notification settings - Fork 107
Allow writing materialized views from queries with less than four columns #2639
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
base: master
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2639 +/- ##
==========================================
+ Coverage 91.52% 91.55% +0.02%
==========================================
Files 479 479
Lines 41177 41184 +7
Branches 5474 5476 +2
==========================================
+ Hits 37688 37704 +16
+ Misses 1910 1902 -8
+ Partials 1579 1578 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
joka921
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only some comments, but this is a nice small PR:)
src/engine/MaterializedViews.cpp
Outdated
| << " empty column(s) will be appended." << std::endl; | ||
|
|
||
| for (uint8_t i = 0; i < 4 - numCols; ++i) { | ||
| result.push_back({Variable{absl::StrCat("?_empty_", i)}, std::nullopt}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe make the pattern a little longer and stranger to decrease the probability of accidental mismatches..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we currently manually select this empty garbage column?
If yes,
Can you please change the design s.t. when READING, we never can obtain those columns (because they have no name on that side of the API s.t. this only becomes an artifact when writing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes currently they can be read. But this is a very nice idea. If we fully hide them, I believe we don't even need any name for them.
Overview
Conformance check passed ✅No test result changes. |
|
joka921
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks much cleaner now:) Thank you very much!



If the query for writing a materialized view has less than four columns, empty columns are now appended automatically.
This is a prerequisite for detecting certain join patterns cleanly (the user may now write a query with a single join on two index scans), which may be used for query rewriting.