Apply workaround for #5444 to DataFrame::describe

[jiangzhx]

Which issue does this PR close?

• Apply a patch while Expr.alias function not work with count aggregation #5444 is not fixed
• use the alltypes_tiny_pages.parquet file as a data test set.

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

[alamb]

Thank you @jiangzhx -- looks like an improvement to me. Maybe @Jefffrey also has some time to review this PR as they were commenting on #5444

[alamb]

I don't fully understand this change, but it seems ok.

[jiangzhx]

This doesn't fix the issue 5444, but it did lead me to discover a better way to speed up the operation of describe.

[Jefffrey]

I don't fully understand these changes either, will need to take a closer look as I'm not yet familiar with describe(...), but it seems this PR is a workaround to the actual issue of #5444 and wouldn't resolve it

[jiangzhx]

I don't fully understand these changes either, will need to take a closer look as I'm not yet familiar with describe(...), but it seems this PR is a workaround to the actual issue of #5444 and wouldn't resolve it

yes,#5444 need find other way to fix. it's should not to close .
but #5444 lead me to discover a better way to speed up the operation of describe.

[Jefffrey]

Is the method of speeding up referring to being able to leverage aggregate_statistics physical optimizer if possible to be able to get the stats faster? Then you hit the issue in #5444 due to these lines in describe(...):

https://github.com/apache/arrow-datafusion/blob/e9852074bacd8c891d84eba38b3417aa16a2d18c/datafusion/core/src/dataframe.rs#L441-L454

Where can't find the column by name since the name of the count(...) column is wrong hence goes to the None case. So this is a workaround to make it work?

Is my understanding correct @jiangzhx ?

[jiangzhx]

Is the method of speeding up referring to being able to leverage aggregate_statistics physical optimizer if possible to be able to get the stats faster? Then you hit the issue in #5444 due to these lines in describe(...):

https://github.com/apache/arrow-datafusion/blob/e9852074bacd8c891d84eba38b3417aa16a2d18c/datafusion/core/src/dataframe.rs#L441-L454

Where can't find the column by name since the name of the count(...) column is wrong hence goes to the None case. So this is a workaround to make it work?

Is my understanding correct @jiangzhx ?

yes, you are right.

the problem was caused by this function
called Result::unwrap()on anErr value: ArrowError(InvalidArgumentError("It is not possible to concatenate arrays of different data types."))

https://github.com/apache/arrow-datafusion/blob/e9852074bacd8c891d84eba38b3417aa16a2d18c/datafusion/core/src/dataframe.rs#L457-L463

[Jefffrey]

Ok I understand now, thanks.

My opinion is to not have this workaround unless its urgently needed (since this would fix a bug in describe(...) which otherwise can't be avoided at the moment I think), as it adds a bit of complexity to the describe(...) method which would become redundant when the core issue #5444 is resolved (in which case this workaround wouldn't be needed anymore), if things work out right.

But if can be diligent to remove this workaround when #5444 is fixed (and confirmed to also fix this without workaround) then it seems fine.

[jiangzhx]

Ok I understand now, thanks.

My opinion is to not have this workaround unless its urgently needed (since this would fix a bug in describe(...) which otherwise can't be avoided at the moment I think), as it adds a bit of complexity to the describe(...) method which would become redundant when the core issue #5444 is resolved (in which case this workaround wouldn't be needed anymore), if things work out right.

But if can be diligent to remove this workaround when #5444 is fixed (and confirmed to also fix this without workaround) then it seems fine.

Thanks for your suggestion @Jefffrey . How about this solution? It might be easier to understand.

//count aggregation
let cnt = self.clone().aggregate(
    vec![],
    original_schema_fields
        .clone()
        .map(|f| count(col(f.name())))
        .collect::<Vec<_>>(),
)?;
let cnt = cnt
    .clone()
    .select(
        cnt.schema()
            .fields()
            .iter()
            .zip(original_schema_fields.clone())
            .map(|(count_field, orgin_field)| {
                col(count_field.name()).alias(orgin_field.name())
            })
            .collect::<Vec<_>>(),
    )
    .unwrap();
let count_record_batch = cnt.collect().await.unwrap()；

[Jefffrey]

@jiangzhx That certainly is a more straightforward workaround, if you decide to go with it then having a comment explaining why the workaround is there would be helpful too

[jiangzhx]

@jiangzhx That certainly is a more straightforward workaround, if you decide to go with it then having a comment explaining why the workaround is there would be helpful too

The issue has been resolved and the fix implemented was based on your suggestion.
thank you @Jefffrey .

[alamb]

👍 -- thank you this comment makes it much clearer what is going on

[alamb]

LGTM -- thank you @jiangzhx

[Jefffrey]

Looks good, just some minor nitpicks

[alamb]

Thanks @jiangzhx and thanks @Jefffrey for the review 🙏

[ursabot]

Benchmark runs are scheduled for baseline = d0bd28e and contender = 99ef989. 99ef989 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ec2-t3-xlarge-us-east-2] ec2-t3-xlarge-us-east-2
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on test-mac-arm] test-mac-arm
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ursa-i9-9960x] ursa-i9-9960x
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ursa-thinkcentre-m75q] ursa-thinkcentre-m75q
Buildkite builds:
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java