mikebridge commented on code in PR #45544:
URL: https://github.com/apache/superset/pull/45544#discussion_r4344384982
##########
superset/semantic_layers/execution.py:
##########
@@ -94,14 +94,60 @@ def semantic_namespace() -> str:
return namespace
+def metadata_namespace(connection: Connection) -> str:
+ """Read the active metadata store's identity, not the app's startup bind.
+
+ Use the caller's connection without ORM autoflush or another pool checkout.
+ Only a missing identity needs an independent, durable bootstrap
transaction.
+ Callers memoize this value only within their metadata session.
+ """
+ value: bytes | None = connection.scalar(_namespace_statement())
+ if value is not None:
+ return _decode_namespace(value)
+ if connection.dialect.name == "sqlite" and getattr(
+ connection.connection.driver_connection, "in_transaction", False
+ ):
+ # A separate writer could wait on this caller's SQLite transaction.
+ # Do not commit or roll back caller work to make bootstrap possible.
+ raise ValueError(
+ "Initialize the semantic execution namespace "
+ "before starting a SQLite transaction"
+ )
+ engine: Engine = connection.engine
+ # A separate pool preserves the configured connection creator/on-connect
+ # hooks without waiting for capacity held by this caller in the original
pool.
+ bootstrap: Engine = Engine(engine.pool.recreate(), engine.dialect,
engine.url)
Review Comment:
Good catch — the bootstrap was dropping the engine execution options. Fixed
in 6095a2e9cf; it keeps `schema_translate_map`, and the regression checks that
only the configured schema gets an identity and that a later session reads the
same one.
##########
superset/semantic_layers/execution.py:
##########
@@ -94,14 +94,60 @@ def semantic_namespace() -> str:
return namespace
+def metadata_namespace(connection: Connection) -> str:
+ """Read the active metadata store's identity, not the app's startup bind.
+
+ Use the caller's connection without ORM autoflush or another pool checkout.
+ Only a missing identity needs an independent, durable bootstrap
transaction.
+ Callers memoize this value only within their metadata session.
+ """
+ value: bytes | None = connection.scalar(_namespace_statement())
+ if value is not None:
+ return _decode_namespace(value)
+ if connection.dialect.name == "sqlite" and getattr(
+ connection.connection.driver_connection, "in_transaction", False
+ ):
+ # A separate writer could wait on this caller's SQLite transaction.
+ # Do not commit or roll back caller work to make bootstrap possible.
+ raise ValueError(
+ "Initialize the semantic execution namespace "
+ "before starting a SQLite transaction"
+ )
+ engine: Engine = connection.engine
+ # A separate pool preserves the configured connection creator/on-connect
+ # hooks without waiting for capacity held by this caller in the original
pool.
+ bootstrap: Engine = Engine(engine.pool.recreate(), engine.dialect,
engine.url)
Review Comment:
Confirmed on both default SQLite in-memory pooling and StaticPool. Fixed in
6095a2e9cf by retaining the single-connection pool; the tests pin first-use
bootstrap, identity survival after caller rollback, and refusal while caller
writes are pending.
--
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]
