You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "mikebridge (via GitHub)" <gi...@apache.org> on 2026/10/20 22:56:13 UTC

[PR] fix(semantic-layer): use durable metadata cache identity [superset]

mikebridge opened a new pull request, #45544:
URL: https://github.com/apache/superset/pull/45544

   ### SUMMARY
   
   TL;DR: Web and worker processes use the same semantic-layer cache keys when they reach one metadata database through different addresses. **Expect a one-time cache miss for semantic-layer results after deploy when using the default namespace.** Existing explicit namespaces keep their key scheme.
   
   Previously, host aliases, proxy ports or database aliases could make a worker write a result that the web process could not find, causing another async submission. The fallback uses the installation identity already stored in the active metadata database, rather than the connection address. Explicit string/callable namespace overrides still win. The lookup uses the caller's existing connection, is memoized within the metadata session and does not commit its pending work. Only a missing identity requires an independent bootstrap transaction.
   
   Update web, worker and MCP roles together. Full database clones copy the installation identity too: configure distinct explicit namespaces for isolated workspaces. A metadata session must not span workspaces. Backup restoration still requires cache clearing or namespace rotation.
   
   Unlike the old address fallback, a full database clone no longer gets a distinct default cache generation merely from its different database name; the explicit namespace separation above is a behavior change to account for when cloning.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   Not applicable; backend cache identity only. No live deployment was exercised.
   
   ### TESTING INSTRUCTIONS
   
   Run `tests/unit_tests/semantic_layers/metadata_namespace_test.py` alongside `simple_cache_clear_test.py`, `execution_context_test.py`, and `models_test.py`. The regression uses independent metadata engines/sessions, real durable identities and DATA-cache writes/reads; the API path returns a cached response without another task submission across address variants. Additional cases cover a one-connection pool, NullPool connection counts and missing-row bootstrap without committing caller work.
   
   Local result: 268 tests passed including the native MCP inventory case. Changed-file hooks and POT drift pass. Independent PostgreSQL probes covered missing-row bootstrap at READ COMMITTED, REPEATABLE READ and SERIALIZABLE, with and without pending caller writes. MySQL, live proxy deployments and actual Celery delivery were not exercised. With MySQL SERIALIZABLE, a missing-row lookup can take a gap lock and delay independent bootstrap; initialize the durable identity before entering such a transaction. SQLite fails promptly if bootstrap would wait on the caller's active transaction.
   
   ### ADDITIONAL INFORMATION
   
   - [ ] Has associated issue:
   - [x] Required feature flags: `SEMANTIC_LAYERS`
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in SIP-59)
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(semantic-layer): use durable metadata cache identity [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #45544:
URL: https://github.com/apache/superset/pull/45544#discussion_r4344388525


##########
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:
   โœ… **CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved** as of `6095a2e`.
   
   For SQLite engines using `SingletonThreadPool` or `StaticPool`, bootstrap now installs the namespace through the existing engine instead of recreating the pool, preserving access to the in-memory database.
   
   <sub>If that's not right, unresolve this thread and CodeAnt will leave it open.</sub>
   
   <!-- codeant-auto-resolve-reply -->



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(semantic-layer): use durable metadata cache identity [superset]

Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #45544:
URL: https://github.com/apache/superset/pull/45544#issuecomment-6303658926

   ### <span aria-hidden="true">โœ…</span> Deploy Preview for *superset-docs-preview* ready!
   
   
   |  Name | Link |
   |:-:|------------------------|
   |<span aria-hidden="true">๐Ÿ”จ</span> Latest commit | 26a9db16b7afc60bcfdcfea403eac539d2ceae67 |
   |<span aria-hidden="true">๐Ÿ”</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6ad7f18dd0b7e02823c93c1b |
   |<span aria-hidden="true">๐Ÿ˜Ž</span> Deploy Preview | [https://deploy-preview-45544--superset-docs-preview.netlify.app](https://deploy-preview-45544--superset-docs-preview.netlify.app) |
   |<span aria-hidden="true">๐Ÿ“ฑ</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br />![QR Code](https://app.netlify.com/qr-code/eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1cmwiOiJodHRwczovL2RlcGxveS1wcmV2aWV3LTQ1NTQ0LS1zdXBlcnNldC1kb2NzLXByZXZpZXcubmV0bGlmeS5hcHAifQ.mEp3xmnv0KKb6Dex7mjWNiipTnLhVMo7mbAQYvKtBK4)<br /><br />_Use your smartphone camera to open QR code link._</details> |
   |<span aria-hidden="true">๐Ÿค–</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#Dcq7DYAgEADQXa6WhI8KuoFjwB0GEgWCNMS4u3SveC-Umu_SYAeYwFWbMAw_yIRc9KwZWgyeRfKpxdbHIV-u3A8aa7WkT2GIuNOeSyMVbgqFg-8H) |
   ---
   <!-- [superset-docs-preview Preview](https://deploy-preview-45544--superset-docs-preview.netlify.app) -->
   _To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(semantic-layer): use durable metadata cache identity [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #45544:
URL: https://github.com/apache/superset/pull/45544#issuecomment-6320524501

   ## CodeAnt PR Risk: Medium Risk
   
   - The PR needs attention before merging because default cache lookup can fail with in-memory SQLite metadata databases.
   - Recreating the pool opens a separate in-memory database, so namespace bootstrap cannot read the existing key-value table.
   
   **Assessed commit:** `719dd7473675`


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(semantic-layer): use durable metadata cache identity [superset]

Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(semantic-layer): use durable metadata cache identity [superset]

Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi merged PR #45544:
URL: https://github.com/apache/superset/pull/45544


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(semantic-layer): use durable metadata cache identity [superset]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #45544:
URL: https://github.com/apache/superset/pull/45544#issuecomment-6304021247

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/45544?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :x: Patch coverage is `20.68966%` with `23 lines` in your changes missing coverage. Please review.
   :white_check_mark: Project coverage is 64.90%. Comparing base ([`293f159`](https://app.codecov.io/gh/apache/superset/commit/293f159deaf63c20d08ada41af77c2925b89a0a0?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`26a9db1`](https://app.codecov.io/gh/apache/superset/commit/26a9db16b7afc60bcfdcfea403eac539d2ceae67?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 19 commits behind head on master.
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/45544?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [superset/semantic\_layers/execution.py](https://app.codecov.io/gh/apache/superset/pull/45544?src=pr&el=tree&filepath=superset%2Fsemantic_layers%2Fexecution.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvc2VtYW50aWNfbGF5ZXJzL2V4ZWN1dGlvbi5weQ==) | 19.04% | [17 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/45544?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [superset/semantic\_layers/models.py](https://app.codecov.io/gh/apache/superset/pull/45544?src=pr&el=tree&filepath=superset%2Fsemantic_layers%2Fmodels.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvc2VtYW50aWNfbGF5ZXJzL21vZGVscy5weQ==) | 25.00% | [6 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/45544?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #45544      +/-   ##
   ==========================================
   + Coverage   56.19%   64.90%   +8.71%     
   ==========================================
     Files        3053     3053              
     Lines      200807   200845      +38     
     Branches    46867    46874       +7     
   ==========================================
   + Hits       112842   130363   +17521     
   + Misses      86880    67532   -19348     
   - Partials     1085     2950    +1865     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/superset/pull/45544/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage ฮ” | |
   |---|---|---|
   | [hive](https://app.codecov.io/gh/apache/superset/pull/45544/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `34.89% <20.68%> (-0.01%)` | :arrow_down: |
   | [presto](https://app.codecov.io/gh/apache/superset/pull/45544/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `36.64% <20.68%> (-0.01%)` | :arrow_down: |
   | [python](https://app.codecov.io/gh/apache/superset/pull/45544/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `52.72% <20.68%> (+16.03%)` | :arrow_up: |
   | [sqlite](https://app.codecov.io/gh/apache/superset/pull/45544/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `52.61% <20.68%> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/45544?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(semantic-layer): use durable metadata cache identity [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #45544:
URL: https://github.com/apache/superset/pull/45544#discussion_r4339038412


##########
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:
   **Suggestion:** For `sqlite://` in-memory databases, the recreated pool opens a separate database; bootstrap cannot see `key_value`, so the first default cache lookup fails.
   
   **Assessment:** ๐ŸŸ  `Major` ยท ๐Ÿ” `Occurrence: Rarely` ยท ๐Ÿท๏ธ `Logic error`
   
   [![Use CodeAnt Skill](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/use-codeant-skill-flat-v2.svg)](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=52c611e2781a4da9bf421fc62e5f59d0&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=52c611e2781a4da9bf421fc62e5f59d0&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   <details>
   <summary><b>Prompt for AI Agent ๐Ÿค– </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset/semantic_layers/execution.py
   **Line:** 119:119
   **Comment:**
   	*Logic Error: For `sqlite://` in-memory databases, the recreated pool opens a separate database; bootstrap cannot see `key_value`, so the first default cache lookup fails.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45544&comment_hash=1c8169f3904d4e0635fd97410690a4b0972fd30f51a0e399b9dfe847c92a6dc2&reaction=like'>๐Ÿ‘</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45544&comment_hash=1c8169f3904d4e0635fd97410690a4b0972fd30f51a0e399b9dfe847c92a6dc2&reaction=dislike'>๐Ÿ‘Ž</a>



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(semantic-layer): use durable metadata cache identity [superset]

Posted by "mikebridge (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org