spark icon indicating copy to clipboard operation
spark copied to clipboard

[SPARK-47094][SQL] SPJ : fix bucket reducer function

Open himadripal opened this issue 1 year ago • 5 comments

What changes were proposed in this pull request?

SPJ compatible bucket issue has an implementation of reducible function. This patch fixes the implementation and make it same as in apache iceberg one.

Why are the changes needed?

With this fix, incompatible number of buckets do not return 1 as GCD, hence the buckets do not reduce to 1 when it used in incompatible number of buckets.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

With unit tests

Was this patch authored or co-authored using generative AI tooling?

No.

himadripal avatar Jun 27 '24 14:06 himadripal

@szehon-ho please take a look.

himadripal avatar Jun 27 '24 19:06 himadripal

With this fix, incompatible number of buckets do not return 1 as GCD, hence the buckets do not reduce to 1 when it used in incompatible number of buckets.

So previously when it is reduced to 1, is it a correctness issue? Or just performance issue?

viirya avatar Oct 19 '24 00:10 viirya

With this fix, incompatible number of buckets do not return 1 as GCD, hence the buckets do not reduce to 1 when it used in incompatible number of buckets.

So previously when it is reduced to 1, is it a correctness issue? Or just performance issue?

performance issue, if it reduces to 1, there will be only task doing the work.

himadripal avatar Oct 19 '24 00:10 himadripal

@viirya it seems it is a test transform, but good to have a good example

szehon-ho avatar Oct 19 '24 00:10 szehon-ho

@viirya it seems it is a test transform, but good to have a good example

Oh okay, I didn't see it is test only code.

viirya avatar Oct 19 '24 05:10 viirya

@viirya please take another look,

himadripal avatar Oct 24 '24 23:10 himadripal

cc @huaxingao

viirya avatar Oct 29 '24 05:10 viirya

Merged to master. Thanks @himadripal @szehon-ho @viirya

huaxingao avatar Oct 30 '24 16:10 huaxingao

Thank you all.

dongjoon-hyun avatar Oct 30 '24 17:10 dongjoon-hyun