You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "aminghadersohi (via GitHub)" <gi...@apache.org> on 2026/09/26 12:17:23 UTC
[PR] fix(db2): set current_schema to the catalog name of the selected schema [superset]
aminghadersohi opened a new pull request, #44706:
URL: https://github.com/apache/superset/pull/44706
### SUMMARY
SQL Lab passes the selected schema to `Db2EngineSpec.get_prequeries` as listed by the SQLAlchemy inspector. The DB2 dialect normalizes unquoted (upper-case) DB2 names to lower case, so a schema stored as `DB2INST1` is listed as `db2inst1`. `get_prequeries` quoted that value as-is:
```sql
set current_schema "db2inst1"
```
A quoted identifier is case-sensitive in DB2, so this selects a different, usually non-existent, schema, and every unqualified query in SQL Lab then fails:
```
SQL0204N "db2inst1.ORDERS" is an undefined name. SQLSTATE=42704
```
This change converts the listed name back to the name stored in the catalog with the dialect's `denormalize_name` before quoting it (`db2inst1` → `DB2INST1`, mixed-case names unchanged), keeping the existing quote escaping.
### BEFORE/AFTER
Against a local DB2 LUW 11.5.9.0 server, running `sql_lab.execute_sql_statements` for `SELECT SUM(AMOUNT) AS TOTAL FROM ORDERS` with the schema picked from `Database.get_all_schema_names()`:
- Before: query fails with `SQL0204N "sqllab….ORDERS" is an undefined name`.
- After: query succeeds and returns the expected row.
### TESTING INSTRUCTIONS
`pytest tests/unit_tests/db_engine_specs/test_db2.py` — `test_get_prequeries` now covers a lower-case listed name, a mixed-case name and quote escaping (fails before this change, 15 passed after).
### ADDITIONAL INFORMATION
- [x] Has associated issue: no
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration
- [ ] 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(db2): set current_schema to the catalog name of the selected schema [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44706:
URL: https://github.com/apache/superset/pull/44706#issuecomment-5846202059
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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(db2): set current_schema to the catalog name of the selected schema [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on code in PR #44706:
URL: https://github.com/apache/superset/pull/44706#discussion_r4140675161
##########
superset/db_engine_specs/db2.py:
##########
@@ -153,5 +153,10 @@ def get_prequeries(
"""
if not schema:
return []
- escaped = schema.replace('"', '""')
+ # Schema names come from the inspector, where unquoted (upper-case) DB2
+ # names are normalized to lower case. Quoting that name as-is would
+ # select a different, usually non-existent, schema, so convert it back
+ # to the name stored in the catalog first.
+ name = database.get_dialect().denormalize_name(schema)
Review Comment:
Applied in d782f1e: `get_prequeries` now calls `cls.denormalize_name(database.get_dialect(), schema)`, so the `requires_name_normalize` gate lives in one place. Added `test_get_prequeries_without_name_normalization` to cover the ungated path.
--
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(db2): set current_schema to the catalog name of the selected schema [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on code in PR #44706:
URL: https://github.com/apache/superset/pull/44706#discussion_r4137485895
##########
superset/db_engine_specs/db2.py:
##########
@@ -153,5 +153,10 @@ def get_prequeries(
"""
if not schema:
return []
- escaped = schema.replace('"', '""')
+ # Schema names come from the inspector, where unquoted (upper-case) DB2
+ # names are normalized to lower case. Quoting that name as-is would
+ # select a different, usually non-existent, schema, so convert it back
+ # to the name stored in the catalog first.
+ name = database.get_dialect().denormalize_name(schema)
Review Comment:
Nit, take it or leave it. `BaseEngineSpec.denormalize_name` already wraps this and only calls the dialect when `requires_name_normalize` is set, which is what `connectors/sqla/utils.py` and `models/helpers.py` go through.
Same result for ibm_db_sa, just keeps the gate in one place:
```suggestion
name = cls.denormalize_name(database.get_dialect(), schema)
```
--
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(db2): set current_schema to the catalog name of the selected schema [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi merged PR #44706:
URL: https://github.com/apache/superset/pull/44706
--
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(db2): set current_schema to the catalog name of the selected schema [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #44706:
URL: https://github.com/apache/superset/pull/44706#issuecomment-5846231854
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/44706?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 `0%` with `2 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 57.14%. Comparing base ([`0501796`](https://app.codecov.io/gh/apache/superset/commit/05017968378f81e44071f44aa16796ec5d2e0b9a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`ec26e85`](https://app.codecov.io/gh/apache/superset/commit/ec26e85bb50842c1dbf639b73fc25b455fdde412?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/44706?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/db\_engine\_specs/db2.py](https://app.codecov.io/gh/apache/superset/pull/44706?src=pr&el=tree&filepath=superset%2Fdb_engine_specs%2Fdb2.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2RiMi5weQ==) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/44706?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (0501796) and HEAD (ec26e85). Click for more details.
>
> <details><summary>HEAD has 28 uploads less than BASE</summary>
>
>| Flag | BASE (0501796) | HEAD (ec26e85) |
>|------|------|------|
>|hive|4|1|
>|python|24|2|
>|presto|4|1|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #44706 +/- ##
===========================================
- Coverage 81.12% 57.14% -23.99%
===========================================
Files 2956 2956
Lines 178436 178437 +1
Branches 41341 41341
===========================================
- Hits 144762 101961 -42801
- Misses 30969 75493 +44524
+ Partials 2705 983 -1722
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/44706/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/44706/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `36.78% <0.00%> (-0.01%)` | :arrow_down: |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/44706/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/44706/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/44706/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `38.68% <0.00%> (-0.01%)` | :arrow_down: |
| [python](https://app.codecov.io/gh/apache/superset/pull/44706/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `38.73% <0.00%> (-46.68%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/44706/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/44706/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
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/44706?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(db2): set current_schema to the catalog name of the selected schema [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #44706:
URL: https://github.com/apache/superset/pull/44706#issuecomment-5905660413
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | ce7c64bad6ce9447f864c2f31a8e0ea46d0dac0a |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6abcaecbcd98de00084b1db8 |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-44706--superset-docs-preview.netlify.app](https://deploy-preview-44706--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><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#DcpLCoAgFADAu7x1gkWEdYOO8X5hkGlWkER3z90s5oWUY0gXTAANUMadffWyPkaoMynrcWsu5mSvAQ3jqfWJpi2WWeockBiViWV0otZa11Mr5OD7AQ) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-44706--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(db2): set current_schema to the catalog name of the selected schema [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44706:
URL: https://github.com/apache/superset/pull/44706#issuecomment-5906044237
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers an incremental AI Review.
- `/review full` - Manually triggers a full AI Review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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(db2): set current_schema to the catalog name of the selected schema [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on PR #44706:
URL: https://github.com/apache/superset/pull/44706#issuecomment-5903763518
Re: the bito suggestion (untyped lambda mock stub in `tests/unit_tests/db_engine_specs/test_db2.py`): replaced with a typed nested `denormalize_name(name: str) -> str` helper in d782f1e.
--
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(db2): set current_schema to the catalog name of the selected schema [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on PR #44706:
URL: https://github.com/apache/superset/pull/44706#issuecomment-5906135378
@fitzee thanks for the live Db2 check. Both points are addressed in ce7c64b:
- **Trade-off:** the `get_prequeries` docstring now states that a quoted lower-case schema (`CREATE SCHEMA "lowonly"`) resolves to `LOWONLY`, matching `get_table_names`, so it isn't reachable through unqualified names in SQL Lab.
- **Test nit:** `test_get_prequeries` no longer re-implements `denormalize_name`. It builds the real dialect with `make_url("db2+ibm_db://u:p@h/d").get_dialect()()`, and `pytest.importorskip("ibm_db_sa")` skips it when the driver isn't installed.
- CI doesn't install `ibm-db-sa` (no Linux arm64 wheel; see `requirements/development.in`), and the testcontainers db2 job only runs `-m testcontainers`. So the same assertions also run against SQLAlchemy's own `DefaultDialect` normalization with `requires_name_normalize = True`. It returns the same result as ibm_db_sa 0.4.4 for every input in the test.
- With ibm_db_sa installed locally, both variants pass, and both fail with the denormalization removed.
Master is merged in and CI is green.
--
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