Skip to content

Continuing Work of [PR9127]: improved using existing orderings - #9273

Merged
mustafasrepo merged 15 commits into
apache:mainfrom
synnada-ai:feature/substitute
Feb 21, 2024
Merged

mustafasrepo merged 15 commits into
apache:mainfrom
synnada-ai:feature/substitute

Conversation

@mustafasrepo

@mustafasrepo mustafasrepo commented Feb 19, 2024 •

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Closes #8838.

Rationale for this change

What changes are included in this PR?

@Lordworms PR did a great job in solving the issue [#8838]. However, there were some minor issues in that PR

  • The table and and csv file it refers were not consistent
  • Substitution couldn't generate more than 1 valid ordering from an existing ordering. See comment

Are these changes tested?

Yes,

Are there any user-facing changes?

@github-actions github-actions Bot added physical-expr Changes to the physical-expr crates sqllogictest SQL Logic Tests (.slt) labels Feb 19, 2024
@mustafasrepo mustafasrepo changed the title Continuing Work of [PR9127](https://github.com/apache/arrow-datafusion/pull/9127) Continuing Work of [PR9127] Feb 19, 2024
# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
@Lordworms

Copy link
Copy Markdown
Contributor

Thanks! @mustafasrepo

@alamb alamb changed the title Continuing Work of [PR9127] Continuing Work of [PR9127]: improved using existing orderings Feb 20, 2024

@alamb alamb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @mustafasrepo -- this looks good to me

Comment thread datafusion/physical-expr/src/equivalence/properties.rs
Comment thread datafusion/physical-expr/src/expressions/cast.rs
Comment thread datafusion/sqllogictest/test_files/monotonic_projection_test.slt
Comment thread datafusion/sqllogictest/test_files/monotonic_projection_test.slt Outdated
Comment thread datafusion/sqllogictest/test_files/monotonic_projection_test.slt
Comment thread datafusion/physical-expr/src/equivalence/properties.rs
Comment thread datafusion/sqllogictest/test_files/monotonic_projection_test.slt
# Conflicts:
#	datafusion/physical-expr/src/equivalence/properties.rs
#	datafusion/physical-plan/src/projection.rs
#	datafusion/sqllogictest/test_files/monotonic_projection_test.slt
@mustafasrepo

Copy link
Copy Markdown
Contributor Author

Thanks @alamb as always for your detailed review.

@mustafasrepo
mustafasrepo merged commit 95b735d into apache:main Feb 21, 2024
@mustafasrepo
mustafasrepo deleted the feature/substitute branch March 27, 2024 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

physical-expr Changes to the physical-expr crates sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Eliminating multi-column sort when major column is a one-to-one and monotonic expression

3 participants