jayzhan211 commented on code in PR #27238:
URL: https://github.com/apache/datafusion/pull/27238#discussion_r4526586655
##########
datafusion/physical-expr/src/equivalence/mod.rs:
##########
@@ -2,6 +2,15 @@
// SPDX-FileCopyrightText: See the NOTICE file distributed with this work for
additional information regarding copyright ownership
// SPDX-FileContributor: Licensed to the Apache Software Foundation (ASF)
under one or more contributor license agreements
+//! Equivalence properties: the orderings, equality, constants and
+//! groupings known to hold for the output of a plan node, which the planner
+//! uses to avoid sorts and repartitions.
+//!
+//! # See Also
+//!
+//! * [`EquivalenceProperties`], which explains the concepts and how the
+//! pieces below fit together.
Review Comment:
This unindented continuation line trips `clippy::doc_lazy_continuation`,
which is why the `clippy` job fails in the fork run. Indenting it fixes it:
```suggestion
//! * [`EquivalenceProperties`], which explains the concepts and how the
//! pieces below fit together.
```
##########
datafusion/physical-expr/src/equivalence/ordering.rs:
##########
@@ -216,18 +226,11 @@ impl OrderingEquivalenceClass {
/// `ASC NULLS LAST` and `DESC NULLS FIRST`. Rows sorted both ways all have
/// the same value (or are all null).
///
- /// This is how a value that is constant only within groups of rows is
- /// recorded, since a `ConstExpr` is constant across all rows. For example,
- /// `SUM(x) OVER (PARTITION BY k)` has one value per `k`, so the window
adds
- /// `[k, w]` once for each of the four directions of `w`.
- ///
- /// ### Future Improvements
+ /// This is how a whole-partition window aggregate records that its value
+ /// is constant within each window partition. See
+ /// [Per-partition constants from window functions].
///
- /// This function may become unnecessary if any of the following
improvements
- /// are implemented:
- /// 1. `SortOptions` supports encoding constantness information.
- /// 2. `EquivalenceProperties` gains `FunctionalDependency` awareness,
eliminating
- /// the need for `Constant` and `Constraints`.
+ /// [Per-partition constants from window functions]:
crate::equivalence::EquivalenceProperties#per-partition-constants-from-window-functions
Review Comment:
No heading produces `#per-partition-constants-from-window-functions`,
including in #27250. The section there is "Example: Window Functions with
`PARTITION BY`", whose anchor is `#example-window-functions-with-partition-by`
(I checked it in the built HTML). This matters a bit more than a broken link
because `walk.rs:255` sends readers here for the four directions of `w`. Also,
the other three anchors in this PR only resolve once #27250 is merged, so could
this one land after it?
```suggestion
/// [Per-partition constants from window functions]:
crate::equivalence::EquivalenceProperties#example-window-functions-with-partition-by
```
##########
datafusion/physical-expr/src/equivalence/mod.rs:
##########
@@ -2,6 +2,15 @@
// SPDX-FileCopyrightText: See the NOTICE file distributed with this work for
additional information regarding copyright ownership
// SPDX-FileContributor: Licensed to the Apache Software Foundation (ASF)
under one or more contributor license agreements
+//! Equivalence properties: the orderings, equality, constants and
+//! groupings known to hold for the output of a plan node, which the planner
+//! uses to avoid sorts and repartitions.
Review Comment:
Optional cleanup: "the planner" can be read as the physical planner. Sorts
and repartitions are mostly avoided by physical optimizer rules
(`EnforceSorting`, `EnforceDistribution`):
```suggestion
//! groupings known to hold for the output of a plan node, which the physical
//! optimizer uses to avoid sorts and repartitions.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
