[AURON #2360] Honor config alt keys when reading from SQLConf by robreeves · Pull Request #2361 · apache/auron

robreeves · 2026-06-25T17:01:20Z

Which issue does this PR close?

Rationale for this change

Config aliases declared via ConfigOption.addAltKey(...) were silently ignored: only the primary key was ever read from Spark's SQLConf, so every alternative key in SparkAuronConfiguration had no effect. For example, setting spark.auron.enable (the alt key of spark.auron.enabled) did not disable Auron.

The cause is in SparkAuronConfiguration.getFromSpark(...): alt keys were only resolved via ConfigEntry.findEntry(...) against Spark's global registry. Auron's options are not registered there, so the lookup always fell back to synthesizing a ConfigEntryWithDefaultFunction keyed on the primary key with an empty alternatives list, and ConfigEntry#readString therefore only read the primary key.

What changes are included in this PR?

Pass the (spark-prefixed) alt keys as the synthesized ConfigEntryWithDefaultFunction's alternatives so ConfigEntry#readString reads primaryKey +: alternatives, with the primary key taking precedence.
Add a unit test (AuronQuerySuite."config alt keys are honored") asserting both primary and alt keys take effect for AURON_ENABLED, and that the primary key wins when both are set.
Update the acosh null propagation test. With alt keys now honored, the test harness's spark.auron.enable=false baseline actually falls back to vanilla Spark, which exposed that vanilla Spark and the native engine can produce the same NaN with different bit patterns (checkSparkAnswerAndOperator/QueryTest compares doubles via Double.doubleToRawLongBits). The test now keeps the exact numeric/null comparison for in-domain inputs, and compares the out-of-domain (NaN-producing) inputs via the natively-supported isnan so no raw NaN bits are compared. The shared checkSparkAnswerAndOperator checker is left unchanged.
Disabled ansi mode for a failing test and created Native arithmetic does not honor ANSI overflow semantics (e.g. negate of Int.MinValue) #2368

Are there any user-facing changes?

Yes. Configuration alternative keys now work as documented. In particular, spark.auron.enable (alias of spark.auron.enabled) and spark.auron.ui.enable (alias of spark.auron.ui.enabled) now take effect. The primary key takes precedence when both are set.

How was this patch tested?

Added AuronQuerySuite."config alt keys are honored".
Ran the full spark-extension-shims-spark test suite (Spark 3.5, Scala 2.12): 138 succeeded, 0 failed.

Auron's ConfigOption alt keys (declared via addAltKey) were silently ignored: getFromSpark only consulted alt keys via ConfigEntry.findEntry (always null for Auron's unregistered options) and then synthesized a ConfigEntryWithDefaultFunction with an empty alternatives list, so only the primary key was ever read from SQLConf. As a result, e.g. setting spark.auron.enable (alt of spark.auron.enabled) had no effect. Pass the spark-prefixed alt keys as the synthesized entry's alternatives so ConfigEntry#readString reads primary +: alternatives, with the primary key taking precedence. Also add a test asserting alt keys are honored. Fixing this makes the test harness's spark.auron.enable=false baseline actually fall back to vanilla Spark, which exposed that acosh(0.0) yields NaN with a different (implementation-defined) bit pattern in each engine; QueryTest compares doubles via Double.doubleToRawLongBits, so update the acosh test to assert NaN-ness for the out-of-domain input rather than exact equality. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

QueryTest compares doubles via Double.doubleToRawLongBits, which is bit-exact. Vanilla Spark and the native engine can produce semantically equal NaNs with different (implementation-defined) bit patterns, so the comparison would spuriously fail. Canonicalize NaN on both sides before comparing. This lets the acosh null propagation test keep its original single-query form covering the out-of-domain (NaN) input. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Fixes Auron’s Spark SQLConf integration so ConfigOption.addAltKey(...) aliases are actually consulted when reading configuration values, and updates the test harness to reflect the now-correct behavior.

Changes:

Populate synthesized ConfigEntryWithDefaultFunction alternatives with spark-prefixed alt keys so SQLConf reads primary + alternatives.
Add a unit test validating alt-key behavior and primary-key precedence for AURON_ENABLED.
Canonicalize NaN values in query-result comparisons to avoid false mismatches from differing NaN bit patterns.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
spark-extension/src/main/java/org/apache/auron/spark/configuration/SparkAuronConfiguration.java	Synthesizes Spark `ConfigEntry` with alt keys so SQLConf reads both primary and aliases.
spark-extension-shims-spark/src/test/scala/org/apache/spark/sql/AuronQueryTest.scala	Canonicalizes NaN before comparing Spark-vs-Auron results to avoid raw-bit NaN mismatches.
spark-extension-shims-spark/src/test/scala/org/apache/auron/AuronQuerySuite.scala	Adds test coverage ensuring config alt keys are honored and primary key wins on conflicts.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…cker Revert checkSparkAnswerAndOperator to plain checkAnswer and instead handle the NaN encoding difference locally in the acosh test. acosh of an out-of-domain input yields NaN, which vanilla Spark and the native engine may encode with different bits; checkAnswer/QueryTest compares doubles by raw bits. Split the test so in-domain/null values are compared numerically, and out-of-domain inputs are compared via the natively-supported isnan (a boolean) so no raw NaN bits are compared. This keeps the shared checker unchanged and avoids relaxing NaN comparison for all callers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

With the config alt-key fix, checkSparkAnswerAndOperator's baseline now actually runs vanilla Spark. On Spark 4.x ANSI mode is enabled by default, so negating Int.MinValue throws ARITHMETIC_OVERFLOW in vanilla Spark while the native engine wraps, causing the comparison to diverge (and the test to fail only on the spark-4.0/4.1 CI jobs). Run the test with spark.sql.ansi.enabled=false so both engines wrap consistently while still exercising the Int.MinValue boundary. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

robreeves · 2026-06-29T23:22:23Z

@SteNicholas can you take a look? This is a pre-req for fixing the test in #1938

ShreyeshArangath

LGTM, just one non-blocking comment

ShreyeshArangath · 2026-06-30T00:19:18Z

 class AuronQuerySuite extends AuronQueryTest with BaseAuronSQLSuite with AuronSQLTestHelper {
  import testImplicits._

+  test("config alt keys are honored") {


nit: since there are only two alt keys, and the PR description mentions spark.auron.ui.enable should we also handle that in the test?

slfan1989 · 2026-06-30T01:17:07Z

@robreeves Thanks for the contribution! @ShreyeshArangath Thanks for the review!

robreeves and others added 4 commits June 25, 2026 00:47

Remove explanatory comment on synthesized ConfigEntry alternatives

b606195

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Reword NaN canonicalization comment in plainer language

19b8426

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings June 25, 2026 17:01

github-actions Bot added the spark label Jun 25, 2026

Copilot started reviewing on behalf of robreeves June 25, 2026 17:01 View session

Copilot AI reviewed Jun 25, 2026

View reviewed changes

Comment thread spark-extension-shims-spark/src/test/scala/org/apache/spark/sql/AuronQueryTest.scala Outdated

Comment thread spark-extension-shims-spark/src/test/scala/org/apache/spark/sql/AuronQueryTest.scala Outdated

robreeves mentioned this pull request Jun 25, 2026

[AURON #1891] Implement randn() function #1938

Open

robreeves requested a review from Copilot June 25, 2026 17:47

Copilot started reviewing on behalf of robreeves June 25, 2026 17:49 View session

Copilot AI reviewed Jun 25, 2026

View reviewed changes

Comment thread spark-extension-shims-spark/src/test/scala/org/apache/auron/AuronFunctionSuite.scala

robreeves mentioned this pull request Jun 26, 2026

Native arithmetic does not honor ANSI overflow semantics (e.g. negate of Int.MinValue) #2368

Open

ShreyeshArangath approved these changes Jun 30, 2026

View reviewed changes

slfan1989 approved these changes Jun 30, 2026

View reviewed changes

slfan1989 merged commit 4a50428 into apache:master Jun 30, 2026
124 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[AURON #2360] Honor config alt keys when reading from SQLConf#2361

[AURON #2360] Honor config alt keys when reading from SQLConf#2361
slfan1989 merged 6 commits into
apache:masterfrom
robreeves:altkeys

robreeves commented Jun 25, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

robreeves commented Jun 29, 2026

Uh oh!

ShreyeshArangath left a comment

Uh oh!

ShreyeshArangath Jun 30, 2026

Uh oh!

Uh oh!

slfan1989 commented Jun 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

robreeves commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

How was this patch tested?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

robreeves commented Jun 29, 2026

Uh oh!

ShreyeshArangath left a comment

Choose a reason for hiding this comment

Uh oh!

ShreyeshArangath Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

slfan1989 commented Jun 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

robreeves commented Jun 25, 2026 •

edited

Loading