[SPARK-58619][CORE][CONNECT] Assign SQLSTATE 08003 to the INVALID_HANDLE session sub-conditions

[j1wonpark]

What changes were proposed in this pull request?

Per the review discussion below, this PR returns to the sub-condition level SQLSTATE override so the wire-visible condition names stay unchanged, and hardens it as a documented exception:

• [CORE]: ErrorSubInfo gains an optional sqlState, and ErrorClassesJsonReader.getSqlState resolves the sub-condition's SQLSTATE first, falling back to the condition's. A new SparkThrowableSuite invariant fails any sub-condition sqlState outside an explicit allowlist (currently the three INVALID_HANDLE.SESSION_* sub-conditions), and the error README documents the exception, discourages new overrides even within the same error class, and notes that a future server-side compatibility layer may remove it.
• [CONNECT]: INVALID_HANDLE.SESSION_CHANGED/SESSION_CLOSED/SESSION_NOT_FOUND declare SQLSTATE 08003 (connection does not exist). Names, messages, and every other sub-condition are unchanged.

Why are the changes needed?

These sub-conditions mean the server-side session backing a Connect client is gone, which 08003 describes, but they inherit INVALID_HANDLE's generic HY000, so tools detecting dead connections by SQLSTATE class 08 cannot recognize them and the Connect JDBC driver hard-codes a remapping (raised in the SPARK-57933 review). Moving them to a class-08 condition is not viable: released Connect clients match the condition names for session invalidation and reattach re-execution, and nothing in a new client can repair that direction (#57831 (comment)).

Does this PR introduce any user-facing change?

Yes. The SQLSTATE of the three conditions changes from HY000 to 08003; the condition names are unchanged. Noted in sql-migration-guide.md.

How was this patch tested?

• SparkThrowableSuite: sub-first resolution incl. degenerate inputs (confirmed failing before the fix), the allowlist invariant (any sub-condition declaring its own SQLSTATE must be on the documented allowlist; verified to fail on an injected non-allowlisted override), pins for SESSION_* = 08003 and every other INVALID_HANDLE sub-condition = HY000, and the error-states validation extended to sub-condition SQLSTATEs.
• SparkConnectSessionManagerSuite asserts getSqlState == "08003" at the server throw sites and SparkSessionE2ESuite asserts the client-received exception carries 08003 end-to-end (both previously untested).
• JdbcErrorUtilsSuite re-run: the JDBC driver's existing hard-coded mapping stays consistent.

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

Generated-by: Claude Code (claude-fable-5)

[pan3793]

cc @nchammas @MaxGekk @LuciferYang, what do you think about this? and do you have concerns about changing the existing SQL state?

[uros-b]

Thank you @j1wonpark!

[nchammas]

There may be a use case for sub-conditions to override the parent condition error state, but in this specific case I believe it is papering over an incorrect grouping of conditions under INVALID_HANDLE.

We should consider restructuring the conditions so that any group of sub-conditions all correctly belong to the same error class.

If we want to support sub-conditions overriding the parent error state, I think they must share the same error class. If they don't, to me that's a sign the sub-conditions are grouped incorrectly.

In other words:

• OK
  • Parent condition error state: HY000
  • Sub-condition error states: HY000, HY001
• Not OK
  • Parent condition error state: HY000
  • Sub-condition error states: HY000, 08003

This aligns with the guidance that PostgreSQL gives:

Thus, an application that does not recognize the specific error code might still be able to infer what to do from the error class.

(PostgreSQL's "error code" is our "error state / SQLSTATE".)

[nchammas]

I don't think this is the correct use of the terminology. Refer to the error hierarchy and illustrative example at the start of this README.

A SQLSTATE is assigned at the error state level. Multiple error conditions can share the same SQLSTATE.

[nchammas]

I think it's weird -- and maybe even incorrect -- to have a sub-condition be in a completely different error class than the parent condition. HY is for "CLI-specific condition" and 08 is for "Connection Exception".

I know it's not something that this PR introduced, but I think the sub-conditions under this condition should be broken up into at least two separate conditions:

1. 08 class: SESSION_*
2. HY class: FORMAT, OPERATION_*

[j1wonpark]

Thanks @nchammas — you're right. I'll drop the override and do the restructuring instead.

The only use of ErrorSubInfo.sqlState in this PR is the cross-class case you object to, and there is no same-class case anywhere in the error JSONs that would justify the field by itself, so I'll drop it and re-scope SPARK-58619. I'll fix the README wording regardless.

For the split I'd suggest INVALID_HANDLE keeps FORMAT and the OPERATION_* sub-conditions with HY000, and the three SESSION_* ones move to a new condition with 08003. Two reasons beyond the error class:

• INVALID_HANDLE.FORMAT is thrown for a malformed session id as well as an operation id (SparkConnectSessionManager.validateSessionCreate and SparkConnectExecutionManager.ExecuteKey), so whichever condition holds it cannot be named after operations. INVALID_HANDLE's existing message stays accurate for exactly that group.
• getMessageTemplate concatenates the condition's message with the sub-condition's, so SESSION_CHANGED currently reads "The handle abc is invalid. The existing Spark server driver instance has restarted." The handle is valid in that case, and keeping SESSION_* under INVALID_HANDLE would preserve that.

Concretely I'd propose SESSION_UNAVAILABLE, with sub-conditions CHANGED / CLOSED / NOT_FOUND and the message "The session <handle> is no longer available." — every SESSION_* throw site already passes a session id as handle, so it prepends cleanly:

[SESSION_UNAVAILABLE.CHANGED] The session abc is no longer available.
The existing Spark server driver instance has restarted. Please reconnect. SQLSTATE: 08003

It avoids "handle", which is accurate only for NOT_FOUND; it follows the unprefixed naming of the neighbouring Connect conditions (INVALID_HANDLE, INVALID_CURSOR, CURSOR_NOT_FOUND, OPERATION_CANCELED) rather than the CONNECT_* form; and _UNAVAILABLE is already used in the file. Note it would be the first condition in class 08.

The rename is visible on the wire. These sites match the condition names as strings, and the first four also exist in already-released clients:

site	matches	effect if it stops matching
sql/connect/common/.../client/ResponseValidator.scala:70	[INVALID_HANDLE.SESSION_CHANGED] in the message text	silent: the session is never marked stale, so SparkSession.isUsable keeps returning true and the builder hands back a dead session
python/pyspark/sql/connect/client/core.py:2418	errorClass metadata equal to the same name	the same, via _closed
sql/connect/common/.../client/ExecutePlanResponseReattachableIterator.scala:253-254	OPERATION_NOT_FOUND and SESSION_NOT_FOUND	reattach loses one arm — under either direction, since both names are in one expression
python/pyspark/sql/connect/client/reattach.py:281-282	the same two	the same
sql/connect/client/jdbc/.../util/JdbcErrorUtils.scala:69	prefix INVALID_HANDLE.SESSION_	unreleased (SPARK-57933), so just an in-tree edit

The two reattach matchers are already a || or a list, so accepting the new name alongside the old is one line each. I'd do that much and no more — we have renamed released conditions without a migration-guide entry before, SPARK-48176 being a recent example.

Happy to implement this here if you agree with the shape.

Separately, the PySpark commit in this PR is unrelated to the above — python/pyspark/errors/error-conditions.json does not contain INVALID_HANDLE at all. It makes get_sqlstate fall back to the main condition's SQLSTATE, fixing cases such as NEAREST_BY_JOIN.UNSUPPORTED_MODE reporting none. I'll move it to its own PR either way.

[dongjoon-hyun]

LGTM with one minor suggestion.

The new Python fallback slightly widens behavior beyond the described case: for an unknown sub-class name (e.g. NEAREST_BY_JOIN.TYPO), the old code raised KeyError and returned None, while the new code returns the main class's sqlState. This matches the JVM side's lenient behavior, and the JVM pins it with a test (TEST_MAIN_STATE.NON_EXISTENT_SUB), but test_sqlstate has no corresponding case. Could you add one line to keep the two sides symmetric? e.g.

# An unknown sub-class name also falls back to the main class's sqlState.
error = PySparkRuntimeError(errorClass="NEAREST_BY_JOIN.NON_EXISTENT_SUB", messageParameters={})
self.assertEqual(error.getSqlState(), "42604")

[nchammas]

we have renamed released conditions without a migration-guide entry before, SPARK-48176 being a recent example

That might not be the best example since it was released with Spark 4.0 and reviewers may not have been that concerned about technically breaking changes in a major release.

[pan3793]

I think the community does not currently seem to treat all error condition names as part of the public API contract, nor does it subject them to sufficiently rigorous review, but I agree we should be careful about those error conditions already used by older versions of the client for control flow.

Specific to the error conditions in this PR, @nchammas, do you have a concrete suggestion for how we should handle them? Since SPARK-57933 is included in 4.3, I don't want to defer this one to the next version and accumulate the tech debt.

[nchammas]

I think the community does not currently seem to treat all error condition names as part of the public API contract, nor does it subject them to sufficiently rigorous review, but I agree we should be careful about those error conditions already used by older versions of the client for control flow.

Yes, over time it will become more important to stabilize these error conditions as more clients come to depend on them.

Specific to the error conditions in this PR, @nchammas, do you have a concrete suggestion for how we should handle them? Since SPARK-57933 is included in 4.3, I don't want to defer this one to the next version and accumulate the tech debt.

My guidance would be:

1. @j1wonpark's proposed split between INVALID_HANDLE (class HY) and SESSION_UNAVAILABLE (class 08) sounds good to me.
2. We should include a migration guide note about any error conditions we change, because they are technically part of the public interface.
3. I think we can make do with just splitting the conditions into different groups and avoid allowing sub-conditions to override the parent condition error state. That would be simplest. I prefer this solution.
4. If down the line we really want sub-conditions to be able to declare a different error state than their parents, then we must have code that enforces that the error class cannot be different. (It would probably be a test like this one.) But I think we don't need this capability for the current use case.

[pan3793]

I would like to land this in 4.3 or revert SPARK-57933 from 4.3 to avoid introducing tech debt.

words sound too verbose, can you make it shorter? also applies to the PR description.

[j1wonpark]

Landing in 4.3 sounds good to me. Updated the version references accordingly (the migration note moved to the "4.2 to 4.3" section, and the "before Spark 4.4" comments now say 4.3), and shortened both the migration note (now two sentences) and the PR description.

[pan3793]

Thanks @j1wonpark for the rework, and @nchammas for the principle, which I agree with in general: a SQLSTATE belongs to the condition, sub-conditions should stay in their condition's error class, and the HY000/08 grouping was wrong from the start.

I traced how existing clients consume these names before concluding this case deserves an exception. The condition names are a de facto wire contract: pre-4.3 clients match INVALID_HANDLE.SESSION_CHANGED and INVALID_HANDLE.SESSION_NOT_FOUND for session invalidation and reattach re-execution, and Connect promises independent server upgrades. The rename is not just a string change: Scala ResponseValidator stops marking the session invalid, so after a restart SparkSession.getOrCreate keeps returning the stale session instead of recreating it; Python core.py stops setting _closed=True, so the client keeps re-hitting the dead session instead of failing fast with NO_ACTIVE_SESSION. Nothing in a new client can repair that direction.

The per-sub-condition sqlState override is the only way to emit 08003 without changing the wire names. So: keep the principle as the default for new conditions, allow the override here as a documented wire-compatibility exception, and state in the schema/README that it does not set a precedent for regrouping sub-conditions.

cc @HyukjinKwon for thoughts on the PySpark side.

[nchammas]

The condition names are a de facto wire contract: pre-4.3 clients match INVALID_HANDLE.SESSION_CHANGED and INVALID_HANDLE.SESSION_NOT_FOUND for session invalidation and reattach re-execution, and Connect promises independent server upgrades.

Maybe a stupid question, but: Can we make the server rewrite error conditions based on the client version? Basically, have a compatibility layer so we are not forced to freeze existing error conditions for all time.

That way older clients will still work but we can continue to refine and cleanup the layout of the conditions going forward.

[pan3793]

@nchammas The compat layer might be a good idea and would generalize beyond this rename. Two things make it feasible without a protocol change: Scala and Python clients already send spark/<version> in client_type (since 4.0), and all server error serialization funnels through the two paths in ErrorUtils (buildStatusFromThrowable and the FetchErrorDetails conversion). The layer would rewrite both the errorClass metadata and the [CONDITION] message prefix, keyed on client version, with a mapping table that starts with this rename.

Two caveats: client_type is documented as logging-only, so interpreting it means amending that contract, and a custom user agent without the version token would silently skip the rewrite; and the layer must ship in the same release as the rename, otherwise the released server can never be fixed for old clients.

That is a bigger piece than this PR. My preference: land the sqlState override as the one-off exception now to unblock 08003, and design and discuss the compat layer independently in the future.

[nchammas]

If we need to get something unblocked for the upcoming release, it is what it is, I guess.

Let's just make it clear in the code and/or docs that:

1. This case is an exception to how things should be done.
2. We may redesign this with a compatibility layer down the line.
3. We will programmatically disallow other sub-conditions from having a different error class from their parent. (This can be implemented as a new test.)
4. We generally discourage contributors from changing sub-conditions to override the parent error state at all (even with the same error class).

[pan3793]

@nchammas, that makes sense.

@j1wonpark, sorry for making you back and forth, but given the situation, we need to go back to the previous approach to avoid breaking wire compatibility.