aminghadersohi commented on code in PR #45544:
URL: https://github.com/apache/superset/pull/45544#discussion_r4340365941
##########
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 for a missing identity row: `sqlite://` raises `no such table:
key_value` after recreating the pool, while the same metadata-generation probe
passes on the PR base. An already-seeded in-memory store works, so this is
confined to first-use bootstrap.
##########
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:
The bootstrap drops `schema_translate_map` from the metadata engine. If the
identity is missing in a configured non-default schema, it reads/writes the
default schema instead, so later sessions still cannot find that identity.
```suggestion
# 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,
execution_options=engine.get_execution_options(),
)
```
--
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]
