refactor: do ambiguous_distinct_check in select

[lichuang]

Which issue does this PR close?

• Closes Unify SQL planning for ORDER BY, HAVING, DISTINCT, etc #10326.

refactor: do ambiguous_distinct_check in select

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

[lichuang]

@jonahgao in #10234 (comment) comment:

I think that we should handle ORDER BY similarly to HAVING, use the merged schema, add the missing columns directly in the select list, instead of traversing the plan looking for projection node.

If not traversing the plan looking for projection node, how to do ambiguous_distinct_check check? This check only be called when handling projection node in distinct, case test_distinct_on_sort_by_unprojected test this case.

[jonahgao]

@jonahgao in #10234 (comment) comment:

I think that we should handle ORDER BY similarly to HAVING, use the merged schema, add the missing columns directly in the select list, instead of traversing the plan looking for projection node.

If not traversing the plan looking for projection node, how to do ambiguous_distinct_check check? This check only be called when handling projection node in distinct, case test_distinct_on_sort_by_unprojected test this case.

My thought is to handle it in select_to_plan, where the original select list and the distinct flag are both available.

[lichuang]

@jonahgao in #10234 (comment) comment:

I think that we should handle ORDER BY similarly to HAVING, use the merged schema, add the missing columns directly in the select list, instead of traversing the plan looking for projection node.

If not traversing the plan looking for projection node, how to do ambiguous_distinct_check check? This check only be called when handling projection node in distinct, case test_distinct_on_sort_by_unprojected test this case.

My thought is to handle it in select_to_plan, where the original select list and the distinct flag are both available.

@jonahgao select_to_plan only works with SQL API, but sometimes people use DataFrame API directly, where test_distinct_sort_by_unprojected is this case, so only check in select_to_plan not works for DataFrame API.

[jonahgao]

@jonahgao select_to_plan only works with SQL API, but sometimes people use DataFrame API directly, where test_distinct_sort_by_unprojected is this case, so only check in select_to_plan not works for DataFrame API.

My plan is for the DataFrame to keep using add_missing_columns, and for the SQL API to reuse the processing logic of HAVING. This can make the planning of SQL ORDER BY faster and more accurate.

SQL has well-known specs for handling ORDER BY. For example, ORDER BY can only reference columns from the SELECT list and the table expression, and there are limitations imposed by the GROUP BY clause. I haven’t found similar specs for sorting DataFrames, and I think they are different concepts.

[lichuang]

@jonahgao please help me review this pr

[alamb]

This PR seems to be failing CI tests -- marking it as draft while those are resolved to make it clear this isn't waiitng on review (I am trying to reduce the review queue)

[github-actions]

Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days.