ozankabak commented on code in PR #16217:
URL: https://github.com/apache/datafusion/pull/16217#discussion_r2126725189
##########
datafusion/physical-expr-common/src/sort_expr.rs:
##########
@@ -334,177 +313,142 @@ fn to_str(options: &SortOptions) -> &str {
}
}
-///`LexOrdering` contains a `Vec<PhysicalSortExpr>`, which represents
-/// a lexicographical ordering.
-///
-/// For example, `vec![a ASC, b DESC]` represents a lexicographical ordering
-/// that first sorts by column `a` in ascending order, then by column `b` in
-/// descending order.
-#[derive(Debug, Default, Clone, PartialEq, Eq, Hash)]
-pub struct LexOrdering {
- inner: Vec<PhysicalSortExpr>,
-}
-
-impl AsRef<LexOrdering> for LexOrdering {
- fn as_ref(&self) -> &LexOrdering {
- self
+// Cross-conversion utilities between `PhysicalSortExpr` and
`PhysicalSortRequirement`
+impl From<PhysicalSortExpr> for PhysicalSortRequirement {
+ fn from(value: PhysicalSortExpr) -> Self {
+ Self::new(value.expr, Some(value.options))
}
}
-impl LexOrdering {
- /// Creates a new [`LexOrdering`] from a vector
- pub fn new(inner: Vec<PhysicalSortExpr>) -> Self {
- Self { inner }
- }
-
- /// Return an empty LexOrdering (no expressions)
- pub fn empty() -> &'static LexOrdering {
- static EMPTY_ORDER: LazyLock<LexOrdering> =
LazyLock::new(LexOrdering::default);
- &EMPTY_ORDER
- }
-
- /// Returns the number of elements that can be stored in the LexOrdering
- /// without reallocating.
- pub fn capacity(&self) -> usize {
- self.inner.capacity()
- }
-
- /// Clears the LexOrdering, removing all elements.
- pub fn clear(&mut self) {
- self.inner.clear()
- }
-
- /// Takes ownership of the actual vector of `PhysicalSortExpr`s in the
LexOrdering.
- pub fn take_exprs(self) -> Vec<PhysicalSortExpr> {
- self.inner
- }
-
- /// Returns `true` if the LexOrdering contains `expr`
- pub fn contains(&self, expr: &PhysicalSortExpr) -> bool {
- self.inner.contains(expr)
- }
-
- /// Add all elements from `iter` to the LexOrdering.
- pub fn extend<I: IntoIterator<Item = PhysicalSortExpr>>(&mut self, iter:
I) {
- self.inner.extend(iter)
- }
-
- /// Remove all elements from the LexOrdering where `f` evaluates to
`false`.
- pub fn retain<F>(&mut self, f: F)
- where
- F: FnMut(&PhysicalSortExpr) -> bool,
- {
- self.inner.retain(f)
- }
-
- /// Returns `true` if the LexOrdering contains no elements.
- pub fn is_empty(&self) -> bool {
- self.inner.is_empty()
- }
-
- /// Returns an iterator over each `&PhysicalSortExpr` in the LexOrdering.
- pub fn iter(&self) -> core::slice::Iter<PhysicalSortExpr> {
- self.inner.iter()
+impl From<PhysicalSortRequirement> for PhysicalSortExpr {
+ /// The default sort options `ASC, NULLS LAST` when the requirement does
+ /// not specify sort options. This default is consistent with PostgreSQL.
+ ///
+ /// Reference: <https://www.postgresql.org/docs/current/queries-order.html>
+ fn from(value: PhysicalSortRequirement) -> Self {
+ let options = value
+ .options
+ .unwrap_or_else(|| SortOptions::new(false, false));
+ Self::new(value.expr, options)
}
+}
- /// Returns the number of elements in the LexOrdering.
- pub fn len(&self) -> usize {
- self.inner.len()
- }
+/// This object represents a lexicographical ordering and contains a vector
+/// of `PhysicalSortExpr` objects.
+///
+/// For example, a `vec![a ASC, b DESC]` represents a lexicographical ordering
+/// that first sorts by column `a` in ascending order, then by column `b` in
+/// descending order. The ordering is non-degenerate, meaning it contains at
+/// least one element, and it is duplicate-free, meaning it does not contain
+/// multiple entries for the same column.
Review Comment:
Looks good, will incorporate.
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
