repeat_row: Document refine_single_time_operator_selection's assumptions#36184
Draft
ggevay wants to merge 7 commits intoMaterializeInc:mainfrom
Draft
repeat_row: Document refine_single_time_operator_selection's assumptions#36184ggevay wants to merge 7 commits intoMaterializeInc:mainfrom
repeat_row: Document refine_single_time_operator_selection's assumptions#36184ggevay wants to merge 7 commits intoMaterializeInc:mainfrom
Conversation
Also disallow it in constructs that are implemented by WITH ORDINALITY under the hood: - ROWS FROM - implicit ROWS FROM, i.e., multiple table functions in a SELECT clause
…`preserves_monotonicity`
ggevay
added
A-optimization
Contributor
Author
|
Bringing this back to draft, because I realized there is an issue: With Edit: Being discussed on slack. |
ggevay
force-pushed
the
repeat_row-3-refine_single_time_operator_selection
branch
from
2f40b20 to
10b4ddb
Compare
…ck inside the functions Also, clarify the assumptions of `refine_single_time_operator_selection`.
ggevay
force-pushed
the
repeat_row-3-refine_single_time_operator_selection
branch
from
10b4ddb to
8952f54
Compare
Contributor
Author
|
Adjusted the PR, see the new PR description. Ready for review. |
ggevay
changed the title
repeat_row: Fix refine_single_time_operator_selection to bail out for repeat_rowrepeat_row: Document refine_single_time_operator_selection's assumptions
Contributor
Author
|
Bringing this back to draft, because we'd like to think through the exact invariants around monotonic operators more thoroughly, as discussed on zoom. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
This is the 3. PR in the
repeat_rowproductionization work stream. This is on top of #36183. Resolves SQL-163.Only the last 2 commits are new:
refine_single_time_operator_selectionandrefine_single_time_consolidation.This is now a pure refactoring and comment clarification PR, because I dropped its original last commit.
The original last commit wanted to bail out from
refine_single_time_operator_selectionwhen it seesrepeat_row. However, I realized that that would not really solve anything, because the dataflow's inputs might also have negative accumulations (consolidated to a single time), coming fromrepeat_rowin other dataflows. So, in this longish slack thread, we decided to not worry about monotonic operators getting negatively accumulating input, but instead just add the errors from monotonic operators to the list of errors that we now have to promote from "definitely internal error" to "maybe you usedrepeat_rowwrong, or internal error" (which I will do in a follow-up PR). Note that we are not totally sure whether all monotonic operators will reliably error out when presented with non-monotonic data. However, in the slack thread we also decided to not aim for always erroring when there is negatively accumulating data, because this seems to be ~impossible to achieve without adding further resource usage to various operators (not just monotonic operators).(Another possibility would be to bail out from
refine_single_time_operator_selectionwhen there isrepeat_rowin either the current dataflow or any of its inputs, recursively, but that would lose too much performance.)