fix: cached session settings survived a rollback

src/main/java/com/google/cloud/spanner/pgadapter/session/SessionState.java

@@ -434,16 +434,25 @@ public void commit() {
         settings.put(toKey(setting.getExtension(), setting.getName()), setting);
       }
     }
+    // Dropping the local settings can uncover a different value, so the cache must go. Promoting
+    // the transaction settings cannot: internalGet(..) returns the same PGSetting instance either
+    // way. The distinction matters, as commit() runs for every statement in autocommit mode.
+    if (localSettings != null) {
+      invalidateCache();
+    }
     this.localSettings = null;
     this.transactionSettings = null;
-    invalidateCache();
   }
 
   /** Rolls back the current transaction and abandons any pending changes to the settings. */
   public void rollback() {
+    // Both maps are dropped here, so the cached values are only stale if the transaction changed
+    // something.
+    if (localSettings != null || transactionSettings != null) {
+      invalidateCache();
+    }
     this.localSettings = null;
     this.transactionSettings = null;
-    invalidateCache();
   }
 
   /** Returns the PostgreSQL version. */

src/test/java/com/google/cloud/spanner/pgadapter/session/SessionStateTest.java

@@ -31,6 +31,7 @@
 import com.google.cloud.spanner.pgadapter.error.PGException;
 import com.google.cloud.spanner.pgadapter.metadata.OptionsMetadata;
 import com.google.cloud.spanner.pgadapter.metadata.OptionsMetadata.DdlTransactionMode;
+import com.google.cloud.spanner.pgadapter.session.PGSetting.Context;
 import com.google.cloud.spanner.pgadapter.statements.PgCatalog;
 import com.google.cloud.spanner.pgadapter.utils.ClientAutoDetector.WellKnownClient;
 import com.google.common.collect.ImmutableMap;
@@ -206,6 +207,72 @@ public void testOverwriteLocalSettingWithSessionSetting() {
     assertEquals("my-app", state.get(null, "application_name").getSetting());
   }
 
+  @Test
+  public void testRollbackInvalidatesCachedValues() {
+    SessionState state = new SessionState(mock(OptionsMetadata.class));
+    ZoneId zoneIdBeforeTransaction = state.getTimezone();
+    int bufferSizeBeforeTransaction = state.getBinaryConversionBufferSize();
+
+    state.set(null, "timezone", "America/New_York");
+    state.set("spanner", "binary_conversion_buffer_size", "1024");
+    // Read the values while the transaction is active, so they are cached.
+    assertEquals(ZoneId.of("America/New_York"), state.getTimezone());
+    assertEquals(1024, state.getBinaryConversionBufferSize());
+
+    state.rollback();
+
+    assertEquals(zoneIdBeforeTransaction, state.getTimezone());
+    assertEquals(bufferSizeBeforeTransaction, state.getBinaryConversionBufferSize());
+  }
+
+  @Test
+  public void testRollbackInvalidatesCachedLocalValues() {
+    SessionState state = new SessionState(mock(OptionsMetadata.class));
+    ZoneId zoneIdBeforeTransaction = state.getTimezone();
+
+    state.setLocal(null, "timezone", "America/New_York");
+    assertEquals(ZoneId.of("America/New_York"), state.getTimezone());
+
+    state.rollback();
+
+    assertEquals(zoneIdBeforeTransaction, state.getTimezone());
+  }
+
+  @Test
+  public void testCommitInvalidatesCachedLocalValues() {
+    SessionState state = new SessionState(mock(OptionsMetadata.class));
+    ZoneId zoneIdBeforeTransaction = state.getTimezone();
+
+    // A local setting is dropped by a commit, so the cached value must be dropped as well.
+    state.setLocal(null, "timezone", "America/New_York");
+    assertEquals(ZoneId.of("America/New_York"), state.getTimezone());
+
+    state.commit();
+
+    assertEquals(zoneIdBeforeTransaction, state.getTimezone());
+  }
+
+  @Test
+  public void testCommitAndRollbackKeepCachedValuesIfNothingChanged() {
+    SessionState state = new SessionState(mock(OptionsMetadata.class));
+    ZoneId cachedZoneId = state.getTimezone();
+    assertEquals(ZoneId.of("UTC"), cachedZoneId);
+
+    // Modify the setting directly instead of through set(..). That bypasses the cache
+    // invalidation that set(..) does, which makes the cached value deliberately stale. Any read
+    // that still returns the stale value therefore proves that the cache was not invalidated.
+    state.get(null, "timezone").setSetting(Context.SUPERUSER, "America/New_York");
+    assertEquals(cachedZoneId, state.getTimezone());
+
+    // Neither of these changes any value that is visible to the session, so they must not throw
+    // away the cached values.
+    state.commit();
+    assertEquals(cachedZoneId, state.getTimezone());
+
+    state.rollback();
+    assertEquals(cachedZoneId, state.getTimezone());
+  }
+
   @Test
   public void testGetAll() {
     SessionState state = new SessionState(mock(OptionsMetadata.class));