Copilot commented on code in PR #6828:
URL: https://github.com/apache/hive/pull/6828#discussion_r4132615480
##########
service/src/java/org/apache/hive/service/cli/session/PersistableSessionUtils.java:
##########
@@ -210,6 +210,8 @@ public static HiveSessionSnapshot
captureSnapshot(SessionHandle sessionHandle, S
.currentDatabase(sessionState != null ?
sessionState.getCurrentDatabase() : null)
.overriddenConfigurations(sessionState != null
? new HashMap<>(sessionState.getOverriddenConfigurations()) : null)
+ .hiveVariables(sessionState != null
+ ? new HashMap<>(sessionState.getHiveVariables()) : null)
Review Comment:
Capturing the map here does not persist every Hive-variable mutation.
`CLIService#setApplicationName` reaches `HiveSessionImpl.setApplicationName`,
which writes `wmapp` into this map directly but does not call
`notifySessionStateChanged`; snapshots are otherwise refreshed only on session
open or matching statements. If failover occurs after an application-name
change and before another `SET`, the new `wmapp` value is lost. Please trigger
a snapshot update when this API mutates the map (or cover all such mutation
paths).
##########
service/src/java/org/apache/hive/service/cli/session/PersistableSessionUtils.java:
##########
@@ -210,6 +210,8 @@ public static HiveSessionSnapshot
captureSnapshot(SessionHandle sessionHandle, S
.currentDatabase(sessionState != null ?
sessionState.getCurrentDatabase() : null)
.overriddenConfigurations(sessionState != null
? new HashMap<>(sessionState.getOverriddenConfigurations()) : null)
+ .hiveVariables(sessionState != null
+ ? new HashMap<>(sessionState.getHiveVariables()) : null)
Review Comment:
This adds a new persisted session state component, but the current tests do
not exercise capturing and hydrating `hiveVariables` (nor deserializing a
snapshot containing them). A regression test should set a hive variable,
round-trip the snapshot through the store/JSON path, hydrate a fresh session,
and assert the value is available for `${hivevar.name}` substitution; otherwise
this user-facing behavior can regress without detection.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
