Skip to content

[GLUTEN-13340][VL] Fix regr_r2's intermediate field order - #13343

Open
LuciferYang wants to merge 6 commits into
apache:mainfrom
LuciferYang:gcfix-regr-r2-order
Open

LuciferYang wants to merge 6 commits into
apache:mainfrom
LuciferYang:gcfix-regr-r2-order

Conversation

@LuciferYang

Copy link
Copy Markdown
Contributor

What changes are proposed in this pull request?

VeloxIntermediateData maps regr_r2's Velox intermediate row to Spark's buffer attributes with Corr's order, because RegrR2 extends PearsonCorrelation. Velox's regr_r2 has a different layout, (c2, count, m2X, meanX, meanY, m2Y) with X being the second argument, and Spark's RegrR2(y, x) passes its arguments to PearsonCorrelation(y, x), so Spark's xAvg and xMk describe y. Plans that run one regr_r2 phase in Velox and another in vanilla Spark returned wrong results. When every phase runs in Velox, the two directions of the mapping cancel out.

This adds a RegrR2 order, ck, n, yMk, yAvg, xAvg, xMk, matched before the PearsonCorrelation case. The intermediate types are the same for both functions, so the type mapping is unchanged.

The Bolt backend has the same mapping in BoltIntermediateData. It is left for a follow-up because no CI job runs the Bolt tests.

How was this patch tested?

Added two tests to VeloxAggregateFunctionsSuite. Both fail without this change, and both check that the plan has a Velox aggregation and a vanilla Spark aggregation computing regr_r2.

  • Velox partial, vanilla final: histogram_numeric(distinct z, 3) is not offloaded, so the last two of the four aggregations Spark plans for a single DISTINCT run in vanilla Spark and merge the buffer Velox produced.
  • Vanilla partial, Velox final: the partial aggregation is forced to fall back. This test only compares groups where x and y both vary. For a constant x or y, Velox's final regr_r2 result matches Spark only from SPARK-55969 on, so it differs from every Spark version Gluten builds against except 4.2, whichever mapping is used.

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

Yes. Generated-by: Claude Opus.

Related issue: #13340.

Copilot AI balanced review requested due to automatic review settings October 16, 2026 17:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@LuciferYang LuciferYang changed the title [GLUTEN-13340][VL] Fix regr_r2's intermediate field order for mixed plans [GLUTEN-13340][VL] Fix regr_r2's intermediate field order Oct 16, 2026
@github-actions github-actions Bot added the VELOX label Oct 16, 2026
@jackylee-ch

Copy link
Copy Markdown
Contributor

This PR currently has merge conflicts with main. Could you merge the latest main and resolve them?

# Conflicts:
#	backends-velox/src/test/scala/org/apache/gluten/execution/VeloxAggregateFunctionsSuite.scala
Copilot AI balanced review requested due to automatic review settings October 21, 2026 08:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@LuciferYang

Copy link
Copy Markdown
Contributor Author

Thanks, merged the latest main and resolved the conflict. It was only the aggregate import line in VeloxAggregateFunctionsSuite, which #13350 also changed.

Copilot AI balanced review requested due to automatic review settings October 26, 2026 13:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

Copilot AI balanced review requested due to automatic review settings October 30, 2026 02:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@LuciferYang

Copy link
Copy Markdown
Contributor Author

@jackylee-ch The conflict is resolved. Could you take another look?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants