Skip to content

[SPARK-57207][SQL][FOLLOWUP] Fix StackOverflowError when setting timestampNanosTypes.enabled via SparkConf#56431

Closed
stevomitric wants to merge 7 commits into
apache:masterfrom
stevomitric:stevomitric/fix-nanos-conf-checkvalue-recursion
Closed

[SPARK-57207][SQL][FOLLOWUP] Fix StackOverflowError when setting timestampNanosTypes.enabled via SparkConf#56431
stevomitric wants to merge 7 commits into
apache:masterfrom
stevomitric:stevomitric/fix-nanos-conf-checkvalue-recursion

Conversation

@stevomitric

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

This PR deletes that validator and moves the requirement into the getter: SQLConf.timestampNanosTypesEnabled now returns true only when spark.sql.types.framework.enabled is also true.

Why are the changes needed?

Enabling the flag the normal way crashes Spark:

val spark = SparkSession.builder()
  .master("local[1]")
  .config("spark.sql.timestampNanosTypes.enabled", "true")
  .getOrCreate()       // fine — options are applied lazily
spark.sql("SELECT 1")  // java.lang.StackOverflowError

The validator runs while the session conf is being built (during mergeSparkConf), and SQLConf.get asks for that same conf which starts building it again, forever.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Tests in this PR.

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

Generated-by: Claude Code (Fable 5)

…enabled via SparkConf

The checkValue of TIMESTAMP_NANOS_TYPES_ENABLED called SQLConf.get,
which re-enters the session's sqlConf lazy val while mergeSparkConf is
applying this very conf during session construction, so the first use
of the session died with StackOverflowError. Enforce the Types
Framework requirement in the effective getter instead.

Co-authored-by: Isaac
@stevomitric

Copy link
Copy Markdown
Contributor Author

cc @MaxGekk PTAL.

@MaxGekk MaxGekk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

0 blocking, 0 non-blocking, 0 nits.
Clean fix — correct root cause analysis, right structural approach, and a regression test that would have caught the original bug.

@MaxGekk

MaxGekk commented Jun 10, 2026

Copy link
Copy Markdown
Member

@stevomitric Could you replace [MINOR] by the JIRA ticket which introduced the issue and add the tag [FOLLOWUP].

@stevomitric stevomitric changed the title [MINOR][SQL] Fix StackOverflowError when setting timestampNanosTypes.enabled via SparkConf [SPARK-57207][SQL][FOLLOWUP] Fix StackOverflowError when setting timestampNanosTypes.enabled via SparkConf Jun 10, 2026
@stevomitric

stevomitric commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

@stevomitric Could you replace [MINOR] by the JIRA ticket which introduced the issue and add the tag [FOLLOWUP].

Done. cc @MaxGekk

@MaxGekk

MaxGekk commented Jun 10, 2026

Copy link
Copy Markdown
Member

+1, LGTM. Merging to master/4.x.
Thank you, @stevomitric.

@MaxGekk MaxGekk closed this in 0e8a75f Jun 10, 2026
MaxGekk pushed a commit that referenced this pull request Jun 10, 2026
…stampNanosTypes.enabled via SparkConf

### What changes were proposed in this pull request?
This PR deletes that validator and moves the requirement into the getter: `SQLConf.timestampNanosTypesEnabled` now returns true only when `spark.sql.types.framework.enabled` is also true.

### Why are the changes needed?
Enabling the flag the normal way crashes Spark:

```scala
val spark = SparkSession.builder()
  .master("local[1]")
  .config("spark.sql.timestampNanosTypes.enabled", "true")
  .getOrCreate()       // fine — options are applied lazily
spark.sql("SELECT 1")  // java.lang.StackOverflowError
```

The validator runs while the session conf is being built (during `mergeSparkConf`), and `SQLConf.get` asks for that same conf which starts building it again, forever.

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
Tests in this PR.

### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Fable 5)

Closes #56431 from stevomitric/stevomitric/fix-nanos-conf-checkvalue-recursion.

Authored-by: Stevo Mitric <stevomitric2000@gmail.com>
Signed-off-by: Max Gekk <max.gekk@gmail.com>
(cherry picked from commit 0e8a75f)
Signed-off-by: Max Gekk <max.gekk@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants