feat(optimizer): add `columns_monotonicity` field for PlanNode #17600

stdrc · 2024-07-08T06:18:14Z

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

Following #17554, this PR adds a new field columns_monotonicity to plan_base::StreamExtra, and derives the property in each stream node. As a result, we can use the property as a hint to optimize algorithm, or require input nodes to have a specific variant of property.

Fixes #13983.

Checklist

I have written necessary rustdoc comments
I have added necessary unit tests and integration tests
I have added test labels as necessary. See details.
I have added fuzzing tests or opened an issue to track them. (Optional, recommended for new SQL features Sqlsmith: Sql feature generation #7934).
My PR contains breaking changes. (If it deprecates some features, please create a tracking issue to remove them in the future).
All checks passed in ./risedev check (or alias, ./risedev c)
My PR changes performance-critical code. (Please run macro/micro-benchmarks and show the results.)

My PR contains critical fixes that are necessary to be merged into the latest release. (Please check out the details)

Documentation

My PR needs documentation updates. (Please use the Release note section below to summarize the impact on users)

Release note

If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.

stdrc · 2024-07-08T06:18:28Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @stdrc and the rest of your teammates on Graphite

st1page

generally LGTM

src/frontend/src/optimizer/plan_node/stream_group_topn.rs

src/frontend/src/optimizer/plan_node/stream_project_set.rs

src/frontend/src/optimizer/plan_node/stream_union.rs

src/frontend/src/optimizer/plan_node/stream_fs_fetch.rs

xxchan

Any background about what makes you want to resurrect this work?

xxchan · 2024-07-16T21:24:22Z

src/frontend/src/optimizer/property/monotonicity.rs

@@ -271,3 +292,53 @@ impl MonotonicityAnalyzer {
        Inherent(Unknown)
    }
 }
+
+#[derive(Debug, Default, Clone, PartialEq, Eq, Hash)]
+pub struct MonotonicityMap(BTreeMap<usize, Monotonicity>);


Can you add comments? The meaning of the BTreeMap is not clear to me. Is it sth like [Monotonicity; num_cols]?

xxchan · 2024-07-16T21:28:34Z

src/frontend/src/optimizer/property/monotonicity.rs

+impl IndexMut<usize> for MonotonicityMap {
+    fn index_mut(&mut self, idx: usize) -> &mut Self::Output {
+        self.0.entry(idx).or_insert(Monotonicity::Unknown)
+    }
+}


If you want to maintain the invariant of not containing Unknown, I think we should not implement IndexMut, and stick to insert, because you will be able to m[i]=Unknown.

src/frontend/src/optimizer/property/monotonicity.rs

src/frontend/src/optimizer/plan_node/stream_eowc_over_window.rs

xxchan · 2024-07-16T22:31:27Z

src/frontend/src/optimizer/plan_node/stream_topn.rs

+            false,
+            false,
+            watermark_columns,
+            MonotonicityMap::new(),


Are TopN's order columns monotonic?

I'm afraid not. Currently I tend to define monotonicity as a property ignoring Ops. While you did remind me that TopN can produce watermarks.

Signed-off-by: Richard Chien <[email protected]>

stdrc · 2024-07-17T07:13:00Z

Any background about what makes you want to resurrect this work?

Three reasons:

The effort towards a unified state cleaning approach for </<= and >/>= is blocked (refactor(dyn-filter): properly clean state and propagate watermark in DynamicFilter #17694), so, to get rid of the hack to determine condition_always_relax, we need feat(dyn-filter): derive condition_always_relax from column monotonicity #17704, which needs monotonicity property.
Even we can avoid condition_always_relax after unifying dynamic filter state cleaning by refactor(dyn-filter): properly clean state and propagate watermark in DynamicFilter #17694, we still need monotonicity property to infer the out_append_only property.
We may be able to utilize this property to avoid extra computation for reordering, e.g. to avoid StreamEowcSort before StreamEowcOverWindow if the input is already NonDecreasing.

github-actions bot added the Invalid PR Title label Jul 8, 2024

stdrc mentioned this pull request Jul 8, 2024

refactor(optimizer): unify watermark derivation and monotonicity derivation #17554

Merged

9 tasks

stdrc changed the title ~~add columns_monotonicity field for PlanNode~~ feat(optimizer): [WIP] add columns_monotonicity field for PlanNode Jul 8, 2024

github-actions bot removed the Invalid PR Title label Jul 8, 2024

stdrc marked this pull request as ready for review July 8, 2024 06:18

github-actions bot added the type/feature label Jul 8, 2024

stdrc marked this pull request as draft July 8, 2024 06:19

stdrc changed the title ~~feat(optimizer): [WIP] add columns_monotonicity field for PlanNode~~ feat(optimizer): add columns_monotonicity field for PlanNode Jul 9, 2024

stdrc force-pushed the rc/monotonicity-as-a-property branch from fa356fc to 2d4e7b2 Compare July 9, 2024 08:29

stdrc force-pushed the rc/column-monotonicity branch from 9f264c4 to 6b9feea Compare July 9, 2024 08:32

stdrc force-pushed the rc/monotonicity-as-a-property branch from 2d4e7b2 to d40c9c9 Compare July 9, 2024 08:32

stdrc marked this pull request as ready for review July 9, 2024 08:32

stdrc requested review from BugenZhao, yuhao-su, st1page and chenzl25 July 9, 2024 08:33

Base automatically changed from rc/column-monotonicity to main July 10, 2024 05:25

stdrc force-pushed the rc/monotonicity-as-a-property branch from d40c9c9 to 5f46d7a Compare July 10, 2024 06:41

st1page approved these changes Jul 11, 2024

View reviewed changes

stdrc force-pushed the rc/monotonicity-as-a-property branch 2 times, most recently from 986df96 to 48d85fd Compare July 16, 2024 08:58

stdrc changed the base branch from main to rc/stream-node-new-clean-up July 16, 2024 08:58

stdrc mentioned this pull request Jul 16, 2024

refactor(optimizer): some clean up for stream nodes' constructors #17696

Merged

9 tasks

stdrc force-pushed the rc/stream-node-new-clean-up branch from 043bb8a to 3320e87 Compare July 16, 2024 09:47

stdrc force-pushed the rc/monotonicity-as-a-property branch from 48d85fd to bbbdd0e Compare July 16, 2024 09:47

Base automatically changed from rc/stream-node-new-clean-up to main July 16, 2024 10:42

stdrc force-pushed the rc/monotonicity-as-a-property branch from bbbdd0e to ac30c1f Compare July 16, 2024 13:07

stdrc mentioned this pull request Jul 16, 2024

feat(dyn-filter): derive condition_always_relax from column monotonicity #17704

Merged

9 tasks

xxchan reviewed Jul 16, 2024

View reviewed changes

src/frontend/src/optimizer/property/monotonicity.rs Outdated Show resolved Hide resolved

xxchan reviewed Jul 16, 2024

View reviewed changes

src/frontend/src/optimizer/plan_node/stream_eowc_over_window.rs Outdated Show resolved Hide resolved

xxchan reviewed Jul 16, 2024

View reviewed changes

stdrc added 4 commits July 17, 2024 11:33

add columns_monotonicity field for PlanNode

ba3594e

Signed-off-by: Richard Chien <[email protected]>

derive monotonicity in each stream node

01fda00

Signed-off-by: Richard Chien <[email protected]>

update

3332463

Signed-off-by: Richard Chien <[email protected]>

update

15a38e7

Signed-off-by: Richard Chien <[email protected]>

stdrc force-pushed the rc/monotonicity-as-a-property branch from ac30c1f to 15a38e7 Compare July 17, 2024 03:38

stdrc added 2 commits July 17, 2024 14:46

add comment for monotonicity

27444b5

Signed-off-by: Richard Chien <[email protected]>

fix monotonicity for StreamEowcOverWindow

0141a2b

Signed-off-by: Richard Chien <[email protected]>

stdrc requested a review from xxchan July 17, 2024 07:14

stdrc added this pull request to the merge queue Jul 17, 2024

Merged via the queue into main with commit 6e2c82f Jul 17, 2024
30 of 31 checks passed

stdrc deleted the rc/monotonicity-as-a-property branch July 17, 2024 10:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(optimizer): add `columns_monotonicity` field for PlanNode #17600

feat(optimizer): add `columns_monotonicity` field for PlanNode #17600

stdrc commented Jul 8, 2024 •

edited

Loading

stdrc commented Jul 8, 2024 •

edited

Loading

st1page left a comment

xxchan left a comment

xxchan Jul 16, 2024

xxchan Jul 16, 2024

xxchan Jul 16, 2024

stdrc Jul 17, 2024

stdrc commented Jul 17, 2024 •

edited

Loading

feat(optimizer): add columns_monotonicity field for PlanNode #17600

feat(optimizer): add columns_monotonicity field for PlanNode #17600

Conversation

stdrc commented Jul 8, 2024 • edited Loading

What's changed and what's your intention?

Checklist

Documentation

Release note

stdrc commented Jul 8, 2024 • edited Loading

st1page left a comment

Choose a reason for hiding this comment

xxchan left a comment

Choose a reason for hiding this comment

xxchan Jul 16, 2024

Choose a reason for hiding this comment

xxchan Jul 16, 2024

Choose a reason for hiding this comment

xxchan Jul 16, 2024

Choose a reason for hiding this comment

stdrc Jul 17, 2024

Choose a reason for hiding this comment

stdrc commented Jul 17, 2024 • edited Loading

feat(optimizer): add `columns_monotonicity` field for PlanNode #17600

feat(optimizer): add `columns_monotonicity` field for PlanNode #17600

stdrc commented Jul 8, 2024 •

edited

Loading

stdrc commented Jul 8, 2024 •

edited

Loading

stdrc commented Jul 17, 2024 •

edited

Loading