Skip to content

CI Red: Fix union in view table test #15300

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 19, 2025
Merged

Conversation

jayzhan211
Copy link
Contributor

@jayzhan211 jayzhan211 commented Mar 19, 2025

Which issue does this PR close?

This CI failure is caused by the conflict with #15135, so is not detected

  • Closes #.

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@github-actions github-actions bot added the sqllogictest SQL Logic Tests (.slt) label Mar 19, 2025
07)----SortExec: expr=[y@0 ASC NULLS LAST], preserve_partitioning=[false]
08)------DataSourceExec: partitions=1, partition_sizes=[1]

# optimize_subquery_sort in create_relation removes Sort so the result is not sorted.
Copy link
Contributor Author

@jayzhan211 jayzhan211 Mar 19, 2025

Choose a reason for hiding this comment

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

I have no idea why we have optimize plan rule optimize_subquery_sort in create_relation, I think we should move such rule to optimizer 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before #15201 - This plan doesn't meet the optimize rule condition because we didn't inline view table at this point, so the Sort is not removed.

Now - The view table inlined so Sort is removed by this function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think removing Sort makes sense, but we need to move this rule to optimizer.

Copy link
Member

Choose a reason for hiding this comment

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

I agree using ORDER BY in a view is meaningless.

@jayzhan211 jayzhan211 changed the title Fix union in view table test CI Red: Fix union in view table test Mar 19, 2025
@jayzhan211 jayzhan211 marked this pull request as ready for review March 19, 2025 01:17
@jayzhan211 jayzhan211 requested a review from jonahgao March 19, 2025 02:22
Copy link
Member

@jonahgao jonahgao left a comment

Choose a reason for hiding this comment

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

LGTM, Thank you @jayzhan211

07)----SortExec: expr=[y@0 ASC NULLS LAST], preserve_partitioning=[false]
08)------DataSourceExec: partitions=1, partition_sizes=[1]

# optimize_subquery_sort in create_relation removes Sort so the result is not sorted.
Copy link
Member

Choose a reason for hiding this comment

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

I agree using ORDER BY in a view is meaningless.

@jonahgao jonahgao merged commit 6c5c5c1 into apache:main Mar 19, 2025
29 checks passed
@jayzhan211 jayzhan211 deleted the view-sort branch March 19, 2025 02:49
07)----SortExec: expr=[y@0 ASC NULLS LAST], preserve_partitioning=[false]
08)------DataSourceExec: partitions=1, partition_sizes=[1]

# optimize_subquery_sort in create_relation removes Sort so the result is not sorted.
query I
SELECT * FROM v1;
Copy link
Contributor

Choose a reason for hiding this comment

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

In this Select query, Postgre does not drop the Sort operation, FYI @jayzhan211 @jonahgao

Copy link
Member

Choose a reason for hiding this comment

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

I think either is fine. A view is a type of relation, and a relation is unordered.
In the SQL Server doc, it states

The ORDER BY clause does not guarantee ordered results when the view is queried, unless ORDER BY is also specified in the query itself.

Postgres also does not drop the Sort operation in subqueries but optimize_subquery_sort does.

psql=> explain select * from (select from u1 order by x);
                                 QUERY PLAN
----------------------------------------------------------------------------
 Subquery Scan on unnamed_subquery  (cost=158.51..186.76 rows=2260 width=0)
   ->  Sort  (cost=158.51..164.16 rows=2260 width=4)
         Sort Key: u1.x
         ->  Seq Scan on u1  (cost=0.00..32.60 rows=2260 width=4)
(4 rows)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sqllogictest SQL Logic Tests (.slt)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants