Visitar URL original
[GLUTEN-13201][CORE] Fall back on RANGE frame bounds that cannot be offloaded by LuciferYang · Pull Request #13202 · apache/gluten · GitHub
Skip to content

[GLUTEN-13201][CORE] Fall back on RANGE frame bounds that cannot be offloaded - #13202

Merged
jackylee-ch merged 13 commits into
apache:mainfrom
LuciferYang:gcfix3/range-frame-bound-fallback
Oct 12, 2026
Merged

jackylee-ch merged 13 commits into
apache:mainfrom
LuciferYang:gcfix3/range-frame-bound-fallback

Conversation

@LuciferYang

@LuciferYang LuciferYang commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

What changes are proposed in this pull request?

Two RANGE window frame shapes failed the query instead of falling back.

On Velox and Bolt, a date order key with an interval bound, e.g. range between interval '1' month preceding and current row, failed planning with UnresolvedException. The range frame boundary pre-computation turns a date key into an integer and adds the bound to it, which only works for an integer bound. It now pre-computes only when every literal bound is integral, and supportWindowExec rejects a literal RANGE bound that is not integral, so the window falls back. The rejection lives in WindowFunctionsBuilder and both backends call it, so the Velox tests cover the code Bolt runs. Both parts are needed: without the first the planning error stays, and without the second the window is offloaded with a constant interval offset that Velox rejects at runtime.

WindowFunctionNode converted literal frame bounds with Long.valueOf. A bound that does not parse as a long, such as the -10.00 a decimal key turns 10 PRECEDING into, threw NumberFormatException, which validation does not turn into a fallback. This matters on ClickHouse, whose supportWindowExec did not check frame bounds. It now throws GlutenNotSupportException, and ClickHouse's supportWindowExec rejects a literal bound that does not parse as a long up front, so the window falls back whether or not native validation is on. Bounds that convert today, such as a decimal(10,0) 10, are still offloaded.

Date keys with integer bounds are still pre-computed and offloaded as before.

How was this patch tested?

VeloxWindowExpressionSuite and BoltWindowExpressionSuite add "range frame with an interval bound on a date key falls back": year-month interval bounds on a date key, and an interval bound next to an integer bound in the same window operator, each compared against vanilla Spark and checked to run as WindowExec. A Spark 4.0+ test does the same for a day interval bound, which Spark 4 also accepts on a date key. WindowFunctionNodeTest checks that decimal, double and null bounds throw GlutenNotSupportException, that an integer bound still converts, that the up-front checks agree with the conversion, and that the Velox/Bolt check rejects a null integral bound. GlutenClickHouseTPCHSaltNullParquetSuite adds "window range with a non-integral bound falls back": 1.5 PRECEDING and 10 PRECEDING on a decimal key are compared against vanilla Spark and checked to run as WindowExec, with native validation both on and off; it runs only on the ClickHouse CI. No CI workflow runs the Bolt suite.

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

Yes. Generated-by: Claude Opus.

Related issue: #13201

Copilot AI balanced review requested due to automatic review settings October 5, 2026 13:37
@github-actions github-actions Bot added CORE works for Gluten Core VELOX labels Oct 5, 2026
@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

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 review overview

🟡 Changes recommended

The Bolt-specific regression lacks backend integration coverage.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
What changed in this PR

Ensures unsupported RANGE window bounds fall back instead of failing during planning or execution.

Changes:

  • Restricts RANGE-bound precomputation and backend offloading to integral bounds.
  • Converts invalid bound parsing into a fallback-compatible exception.
  • Adds Velox and Substrait regression tests.
File Description
PullOutProjectHelper.scala Guards RANGE-bound precomputation by bound type.
WindowFunctionNode.java Converts parsing failures into unsupported-operation fallback.
WindowFunctionNodeTest.java Tests integral and non-integral bound conversion.
VeloxBackend.scala Rejects non-integral RANGE bounds.
BoltBackend.scala Rejects non-integral RANGE bounds.
VeloxWindowExpressionSuite.scala Tests fallback for date keys with interval bounds.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread backends-bolt/src/main/scala/org/apache/gluten/backendsapi/bolt/BoltBackend.scala Outdated
Copilot AI balanced review requested due to automatic review settings October 5, 2026 13:48
@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

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 review overview

🔵 Needs a closer look

Bolt has no executing CI coverage, and ClickHouse is covered only through the shared unit-level conversion path.

Review effort: Balanced
Findings: 1 Low severity

Open (1)

Copilot AI balanced review requested due to automatic review settings October 5, 2026 15:07

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.

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 2 Medium severity · 1 Low severity

Open (3)
Resolved since last review (1)

Copilot AI balanced review requested due to automatic review settings October 5, 2026 15:41

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.

Comment thread backends-bolt/src/main/scala/org/apache/gluten/backendsapi/bolt/BoltBackend.scala Outdated
Copilot AI balanced review requested due to automatic review settings October 5, 2026 16:18
@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

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.

@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

Copilot AI balanced review requested due to automatic review settings October 6, 2026 00:21

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 encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

Copilot AI balanced review requested due to automatic review settings October 6, 2026 01:04
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

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 review overview

🔵 Needs a closer look

The planner changes span three backends, and the new Bolt regression suite is not exercised by CI.

Review effort: Balanced
Findings: None

Copilot AI balanced review requested due to automatic review settings October 6, 2026 01:27
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

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 review overview

🟡 Changes recommended

ClickHouse still fails instead of falling back when native validation is disabled.

Review effort: Balanced
Findings: 1 High severity

Open (1)

Copilot AI balanced review requested due to automatic review settings October 6, 2026 01:48
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

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 review overview

🟡 Changes recommended

Integral-typed null bounds can still bypass Velox/Bolt validation and fail when native validation is disabled.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (1)

Copilot AI balanced review requested due to automatic review settings October 6, 2026 02:10
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

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 review overview

🟢 Approval recommended

The fallback checks are consistent with serialization behavior and are covered across the affected backend paths.

Review effort: Balanced
Findings: None

Resolved since last review (1)

@infvg infvg 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.

lgtm!

@jackylee-ch jackylee-ch 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.

👍

@jackylee-ch
jackylee-ch merged commit 0376299 into apache:main Oct 12, 2026
59 checks passed
@LuciferYang

Copy link
Copy Markdown
Contributor Author

Thank you @jackylee-ch

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

Labels

CLICKHOUSE CORE works for Gluten Core VELOX

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants