remove duplicate the logic b/w DataFrame API and SQL planning

[jiangzhx]

Which issue does this PR close?

now the count wildcard rules already move to Analyzer #5671
so remove duplicate the logic in SQL planning.

Closes #.

Rationale for this change

related issues: #5473 (comment)
related PR: #5671

What changes are included in this PR?

Are there any user-facing changes?

[alamb]

I am sorry for the delay in review. I will try and find more time to review this carefully tomorrow but initially I am surprised that says it removes duplicated logic adds more code than it removes 🤔

[alamb]

Can you please explain the rationale for this change?

[alamb]

Why was this removed?

[jiangzhx]

I am sorry for the delay in review. I will try and find more time to review this carefully tomorrow but initially I am surprised that says it removes duplicated logic adds more code than it removes 🤔

Because when I started to remove the duplicate logic between the DataFrame API and SQL planning, I found that count_wildcard_rule did not cover all scenarios, such as union, window, etc.
Currently, these test cases are all based on SQL, so they are not discovered when running cargo test.
After I covered more scenarios for count_wildcard_rule, it resulted in more code added than removed.

for example, before this pr.

#worked
ctx.sql("select COUNT(*) OVER(ORDER BY timestamp_col DESC RANGE BETWEEN 6 PRECEDING AND 2 FOLLOWING)  from alltypes_tiny_pages")

#failed
let df_results = ctx
    .table("alltypes_tiny_pages")
    .await?
    .select(vec![Expr::WindowFunction(expr::WindowFunction::new(
        WindowFunction::AggregateFunction(AggregateFunction::Count),
        vec![Expr::Wildcard],
        vec![],
        vec![Expr::Sort(Sort::new(
            Box::new(col("timestamp_col")),
            false,
            true,
        ))],
        WindowFrame {
            units: WindowFrameUnits::Range,
            start_bound: WindowFrameBound::Preceding(ScalarValue::IntervalDayTime(
                Some(6),
            )),
            end_bound: WindowFrameBound::Following(ScalarValue::IntervalDayTime(
                Some(2),
            )),
        },
    ))])?
    .explain(false, false)?
    .collect()
    .await?;

[alamb]

Because when I started to remove the duplicate logic between the DataFrame API and SQL planning, I found that count_wildcard_rule did not cover all scenarios, such as union, window, etc.

This makes sense -- thank you for the explanation @jiangzhx

Can you please add DataFrame tests the relevant behavior (mostly so that we don't break it in the future by accident)

[alamb]

Marking as draft to signify this PR has feedback and is not waiting for another review at the moment.

[jiangzhx]

Because when I started to remove the duplicate logic between the DataFrame API and SQL planning, I found that count_wildcard_rule did not cover all scenarios, such as union, window, etc.

This makes sense -- thank you for the explanation @jiangzhx

Can you please add DataFrame tests the relevant behavior (mostly so that we don't break it in the future by accident)

i added some testcase in tests/dataframe.rs

• test_count_wildcard_on_sort
• test_count_wildcard_on_where_exist
• test_count_wildcard_on_where_in
• test_count_wildcard_on_window
• test_count_wildcard_on_aggregate
• test_count_wildcard_on_where_scalar_subquery

[jiangzhx]

split this pr in two part.

1. update count_wildcard_rule for more scenario #6010
2. remove duplicate the logic b/w DataFrame API and SQL planning

[alamb]

Since this has been open for more than a year, closing it down. Feel free to reopen if/when you keep working on it.