Skip to content

Fix merge window optimizers#18967

Merged
feilong-liu merged 1 commit intoprestodb:masterfrom
feilong-liu:fix_merge_window_optimizer
Feb 7, 2023
Merged

Fix merge window optimizers#18967
feilong-liu merged 1 commit intoprestodb:masterfrom
feilong-liu:fix_merge_window_optimizer

Conversation

@feilong-liu
Copy link
Contributor

What's the change?

When the optimizer GatherAndMergeWindows merges or reorders two window nodes, it will check if one window node takes the other window's output as input or not. If yes, the two window nodes cannot be merged or reordered.

However, the current implementation to check this dependency misses the input used in Frame. For example,

SELECT
    a, b, c, rnk, SUM(c) OVER (PARTITION BY a ORDER BY b rows BETWEEN rnk PRECEDING AND rnk FOLLOWING)
FROM (
    SELECT
        a, b, c, RANK() OVER (PARTITION BY a ORDER BY b) AS rnk
    FROM (
        VALUES (1, 1, 1), (1, 2, 1), (1, 3, 1), (2, 1, 1), (2, 2, 1), (2, 3, 1)
    ) AS t(a, b, c)
)

This query will throw error (GENERIC_INTERNAL_ERROR) Invalid node. Frame bounds ([rank, rank]) not in source plan output .

This PR fixes it by adding the Frame into account.

Test plan - (Please fill in how you tested your changes)

Add unit tests. Also run locally to make sure that the query above runs fine.

== RELEASE NOTES ==

General Changes
* Fix bug in `GatherAndMergeWindows` optimization rule.

@feilong-liu feilong-liu requested a review from a team as a code owner January 24, 2023 18:42
@feilong-liu feilong-liu force-pushed the fix_merge_window_optimizer branch 3 times, most recently from 2f66c91 to 793523d Compare January 26, 2023 01:28
@rschlussel rschlussel requested a review from pranjalssh January 26, 2023 16:17
@feilong-liu feilong-liu force-pushed the fix_merge_window_optimizer branch 3 times, most recently from 8879349 to 55d8819 Compare February 2, 2023 21:54
When the optimizer GatherAndMergeWindows merges or reorders two window
nodes, it will check if one window node takes the other window's output
as input or not. If yes, the two window nodes cannot be merged or reordered.
However, the current implementation to check this dependency misses the
input used in Frame. This commit fixes it by adding the Frame into account.
@feilong-liu feilong-liu force-pushed the fix_merge_window_optimizer branch from 55d8819 to 36f6107 Compare February 7, 2023 17:22
@feilong-liu feilong-liu merged commit cd1cf8d into prestodb:master Feb 7, 2023
@wanglinsong wanglinsong mentioned this pull request Feb 25, 2023
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants