You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "msyavuz (via GitHub)" <gi...@apache.org> on 2026/09/10 11:47:33 UTC
[PR] fix(mongodb): treat the Mongo database as the SQL Lab schema so Data Preview returns rows [superset]
msyavuz opened a new pull request, #44141:
URL: https://github.com/apache/superset/pull/44141
### SUMMARY
SQL Lab's Data Preview returns "The query returned no data" for every MongoDB collection. PyMongoSQL takes the whole `FROM` reference as the collection name and always queries the database in the connection URI, so the preview query `SELECT * FROM testdb.orders` looks for a collection literally named `testdb.orders` and matches nothing. The schema picked in SQL Lab was also ignored for the same reason: `adjust_engine_params` was the base no-op, so `SELECT * FROM items` with `otherdb` selected still ran against the URI's database.
The fix treats a Mongo database as the SQLAlchemy schema, the same way the MySQL spec does:
- `supports_dynamic_schema = True` and `adjust_engine_params` swaps the URI database to the selected schema.
- `get_schema_from_engine_params` / `get_default_schema` read the database from the URI instead of returning `None`.
- `quote_table` emits the bare collection name, so `select_star` generates `FROM orders` and relies on the connection for the database.
Charts on a Mongo dataset with `schema` set hit the same `schema.table` rendering through PyMongoSQL's compiler; that is not covered here.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
### TESTING INSTRUCTIONS
1. `docker run -d --rm -p 27017:27017 mongo:7`, seed a few documents into `testdb.orders` and `otherdb.items`.
2. Add a database with `mongodb://localhost:27017/testdb?mode=superset`.
3. In SQL Lab, pin `orders` and `items` to the result panel and open their Data preview tabs: both return rows (before: no data for either).
4. Select the `otherdb` schema and run `SELECT * FROM items`: returns rows (before: no data).
Unit tests in `tests/unit_tests/db_engine_specs/test_mongodb.py` cover the URI swap, default schema, and unqualified `select_star` SQL.
### ADDITIONAL INFORMATION
- [ ] Has associated issue: No
- [ ] Required feature flags: None
- [ ] Changes UI: No, backend only
- [ ] Includes DB Migration: No
- [ ] Introduces new feature or API: No
- [ ] Removes existing feature or API: No
--
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(mongodb): treat the Mongo database as the SQL Lab schema so Data Preview returns rows [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #44141:
URL: https://github.com/apache/superset/pull/44141#issuecomment-5764014835
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 5b453e16024d9e8f4832a9d5caadcfc390e82b00 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6ab15c5b5ca50100080ed750 |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-44141--superset-docs-preview.netlify.app](https://deploy-preview-44141--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#DcrLDYAgDADQXXrWWA9V4waOUaCKiQjBLxp3l9s7vBdC9C7s0AMUoCKv2ma7LfF5PNU435Xz6-SNKkOUc5ar3LQVx3kbCYtPg8m_YVWTJkWaCWtE7FBMSwjfDw) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-44141--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(mongodb): treat the Mongo database as the SQL Lab schema so Data Preview returns rows [superset]
Posted by "msyavuz (via GitHub)" <gi...@apache.org>.
msyavuz commented on code in PR #44141:
URL: https://github.com/apache/superset/pull/44141#discussion_r4064249509
##########
superset/db_engine_specs/mongodb.py:
##########
@@ -102,6 +115,48 @@ class MongoDBEngineSpec(BaseEngineSpec):
),
}
+ @classmethod
+ def adjust_engine_params(
+ cls,
+ uri: URL,
+ connect_args: dict[str, Any],
+ catalog: Optional[str] = None,
+ schema: Optional[str] = None,
+ ) -> tuple[URL, dict[str, Any]]:
+ uri, new_connect_args = super().adjust_engine_params(
+ uri,
+ connect_args,
+ catalog,
+ schema,
+ )
+
+ if schema:
+ uri = uri.set(database=schema)
Review Comment:
Good catch. The schema now goes through PyMongoSQL's `database` connect arg instead of rewriting the URI, so the implicit `authSource` stays on the original database. Covered by a real-driver test with and without an explicit `authSource`.
##########
superset/db_engine_specs/mongodb.py:
##########
@@ -102,6 +115,48 @@ class MongoDBEngineSpec(BaseEngineSpec):
),
}
+ @classmethod
+ def adjust_engine_params(
+ cls,
+ uri: URL,
+ connect_args: dict[str, Any],
+ catalog: Optional[str] = None,
+ schema: Optional[str] = None,
+ ) -> tuple[URL, dict[str, Any]]:
+ uri, new_connect_args = super().adjust_engine_params(
+ uri,
+ connect_args,
+ catalog,
+ schema,
+ )
+
+ if schema:
+ uri = uri.set(database=schema)
+
+ return uri, new_connect_args
Review Comment:
Fixed in the same change: the selected schema overrides `connect_args["database"]` and keeps `authSource`, and the schema helpers now read that arg before the URI. The `{"database": "dbone", "authSource": "dbone"}` case is in the tests.
--
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(mongodb): treat the Mongo database as the SQL Lab schema so Data Preview returns rows [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on code in PR #44141:
URL: https://github.com/apache/superset/pull/44141#discussion_r4079954780
##########
superset/db_engine_specs/mongodb.py:
##########
@@ -38,6 +45,13 @@ class MongoDBEngineSpec(BaseEngineSpec):
engine_name = "MongoDB"
force_column_alias_quotes = False
+ # A MongoDB database is exposed as a SQLAlchemy schema. PyMongoSQL treats the
+ # whole FROM reference as the collection name, so the schema is selected through
+ # the driver's ``database`` connect argument rather than by qualifying the
+ # collection. The URI database is left untouched because MongoDB also uses it as
+ # the default ``authSource``.
+ supports_dynamic_schema = True
Review Comment:
Flipping this also changes how `raise_for_access` resolves unqualified tables: `get_default_schema_for_query` returns `query.schema` now instead of probing the connection, so the permission check and the execution agree on the same database. That is the right call, but nothing in the tests pins it.
Could you add it to `test_engine_metadata`, or drop in a small `get_default_schema_for_query` case? A future edit shouldn't be able to flip it back quietly.
##########
superset/db_engine_specs/mongodb.py:
##########
@@ -102,6 +116,49 @@ class MongoDBEngineSpec(BaseEngineSpec):
),
}
+ @classmethod
+ def adjust_engine_params(
+ cls,
+ uri: URL,
+ connect_args: dict[str, Any],
+ catalog: Optional[str] = None,
+ schema: Optional[str] = None,
+ ) -> tuple[URL, dict[str, Any]]:
+ uri, new_connect_args = super().adjust_engine_params(
+ uri,
+ connect_args,
+ catalog,
+ schema,
+ )
+
+ if schema:
+ new_connect_args = {**new_connect_args, "database": schema}
+
+ return uri, new_connect_args
+
+ @classmethod
+ def get_schema_from_engine_params(
+ cls,
+ sqlalchemy_uri: URL,
+ connect_args: dict[str, Any],
+ ) -> Optional[str]:
+ # PyMongoSQL gives the ``database`` connect argument precedence over the URI.
+ return connect_args.get("database") or sqlalchemy_uri.database or None
+
+ @classmethod
+ def get_default_schema(
+ cls,
+ database: Database,
+ catalog: Optional[str],
+ ) -> Optional[str]:
+ return cls.get_schema_from_engine_params(
+ make_url_safe(database.sqlalchemy_uri), database.connect_args
+ )
+
+ @classmethod
+ def quote_table(cls, table: Table, dialect: Dialect) -> str:
Review Comment:
The base contract here is "fully quote a table name, including the schema and catalog", so dropping the schema is a deliberate break and worth a docstring saying why.
Without one, the next person reading `FROM orders` in the preview SQL has no way to know it's only correct because the connection is bound to that schema.
##########
superset/db_engine_specs/mongodb.py:
##########
@@ -102,6 +116,49 @@ class MongoDBEngineSpec(BaseEngineSpec):
),
}
+ @classmethod
+ def adjust_engine_params(
+ cls,
+ uri: URL,
+ connect_args: dict[str, Any],
+ catalog: Optional[str] = None,
+ schema: Optional[str] = None,
+ ) -> tuple[URL, dict[str, Any]]:
+ uri, new_connect_args = super().adjust_engine_params(
+ uri,
+ connect_args,
+ catalog,
+ schema,
+ )
+
+ if schema:
+ new_connect_args = {**new_connect_args, "database": schema}
+
+ return uri, new_connect_args
+
+ @classmethod
+ def get_schema_from_engine_params(
+ cls,
+ sqlalchemy_uri: URL,
+ connect_args: dict[str, Any],
+ ) -> Optional[str]:
+ # PyMongoSQL gives the ``database`` connect argument precedence over the URI.
+ return connect_args.get("database") or sqlalchemy_uri.database or None
+
+ @classmethod
+ def get_default_schema(
+ cls,
+ database: Database,
+ catalog: Optional[str],
+ ) -> Optional[str]:
+ return cls.get_schema_from_engine_params(
+ make_url_safe(database.sqlalchemy_uri), database.connect_args
+ )
+
+ @classmethod
+ def quote_table(cls, table: Table, dialect: Dialect) -> str:
+ return dialect.identifier_preparer.quote(table.table)
Review Comment:
Datasets don't come through here: `SqlaTable.get_sqla_table` builds `table(self.table_name, schema=self.schema)`, so a chart on a Mongo dataset still emits `"testdb"."orders"` and comes back empty.
The description says as much, but with the schema selector actually working people will land there fast. Mind opening a tracking issue and linking it?
--
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(mongodb): treat the Mongo database as the SQL Lab schema so Data Preview returns rows [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44141:
URL: https://github.com/apache/superset/pull/44141#issuecomment-5795805266
**Bito Automatic Review Skipped – PR Already Merged**<br/><br/>Bito scheduled an automatic review for this pull request, but the review was skipped because this PR was merged before the review could be run.<br/>No action is needed if you didn't intend to review it. To get a review, you can type `/review` in a comment and save it
--
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(mongodb): treat the Mongo database as the SQL Lab schema so Data Preview returns rows [superset]
Posted by "msyavuz (via GitHub)" <gi...@apache.org>.
msyavuz merged PR #44141:
URL: https://github.com/apache/superset/pull/44141
--
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(mongodb): treat the Mongo database as the SQL Lab schema so Data Preview returns rows [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #44141:
URL: https://github.com/apache/superset/pull/44141#issuecomment-5618528201
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/44141?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 `66.66667%` with `7 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 66.21%. Comparing base ([`e5c2a3e`](https://app.codecov.io/gh/apache/superset/commit/e5c2a3e4f68291a9bac490cee0829bf5cea76ed3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`8019399`](https://app.codecov.io/gh/apache/superset/commit/801939919e4a8a1824256a17c6b5b7999a12a7f8?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/44141?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/mongodb.py](https://app.codecov.io/gh/apache/superset/pull/44141?src=pr&el=tree&filepath=superset%2Fdb_engine_specs%2Fmongodb.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL21vbmdvZGIucHk=) | 66.66% | [7 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/44141?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 (e5c2a3e) and HEAD (8019399). Click for more details.
>
> <details><summary>HEAD has 2 uploads less than BASE</summary>
>
>| Flag | BASE (e5c2a3e) | HEAD (8019399) |
>|------|------|------|
>|python|5|3|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #44141 +/- ##
===========================================
- Coverage 80.12% 66.21% -13.92%
===========================================
Files 2924 2924
Lines 172370 172390 +20
Branches 40018 40019 +1
===========================================
- Hits 138120 114147 -23973
- Misses 31659 55862 +24203
+ Partials 2591 2381 -210
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/44141/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/44141/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `37.35% <66.66%> (+<0.01%)` | :arrow_up: |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/44141/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/44141/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.24% <66.66%> (+<0.01%)` | :arrow_up: |
| [python](https://app.codecov.io/gh/apache/superset/pull/44141/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.76% <66.66%> (-27.72%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/44141/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.63% <66.66%> (+<0.01%)` | :arrow_up: |
| [unit](https://app.codecov.io/gh/apache/superset/pull/44141/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/44141?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(mongodb): treat the Mongo database as the SQL Lab schema so Data Preview returns rows [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44141:
URL: https://github.com/apache/superset/pull/44141#issuecomment-5618178926
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(mongodb): treat the Mongo database as the SQL Lab schema so Data Preview returns rows [superset]
Posted by "msyavuz (via GitHub)" <gi...@apache.org>.
msyavuz commented on code in PR #44141:
URL: https://github.com/apache/superset/pull/44141#discussion_r4082759531
##########
superset/db_engine_specs/mongodb.py:
##########
@@ -38,6 +45,13 @@ class MongoDBEngineSpec(BaseEngineSpec):
engine_name = "MongoDB"
force_column_alias_quotes = False
+ # A MongoDB database is exposed as a SQLAlchemy schema. PyMongoSQL treats the
+ # whole FROM reference as the collection name, so the schema is selected through
+ # the driver's ``database`` connect argument rather than by qualifying the
+ # collection. The URI database is left untouched because MongoDB also uses it as
+ # the default ``authSource``.
+ supports_dynamic_schema = True
Review Comment:
Pinned in cbfb5f7c1d: test_get_default_schema_for_query asserts it returns the query schema, alongside a supports_dynamic_schema check.
##########
superset/db_engine_specs/mongodb.py:
##########
@@ -102,6 +116,49 @@ class MongoDBEngineSpec(BaseEngineSpec):
),
}
+ @classmethod
+ def adjust_engine_params(
+ cls,
+ uri: URL,
+ connect_args: dict[str, Any],
+ catalog: Optional[str] = None,
+ schema: Optional[str] = None,
+ ) -> tuple[URL, dict[str, Any]]:
+ uri, new_connect_args = super().adjust_engine_params(
+ uri,
+ connect_args,
+ catalog,
+ schema,
+ )
+
+ if schema:
+ new_connect_args = {**new_connect_args, "database": schema}
+
+ return uri, new_connect_args
+
+ @classmethod
+ def get_schema_from_engine_params(
+ cls,
+ sqlalchemy_uri: URL,
+ connect_args: dict[str, Any],
+ ) -> Optional[str]:
+ # PyMongoSQL gives the ``database`` connect argument precedence over the URI.
+ return connect_args.get("database") or sqlalchemy_uri.database or None
+
+ @classmethod
+ def get_default_schema(
+ cls,
+ database: Database,
+ catalog: Optional[str],
+ ) -> Optional[str]:
+ return cls.get_schema_from_engine_params(
+ make_url_safe(database.sqlalchemy_uri), database.connect_args
+ )
+
+ @classmethod
+ def quote_table(cls, table: Table, dialect: Dialect) -> str:
Review Comment:
Added a docstring in cbfb5f7c1d.
##########
superset/db_engine_specs/mongodb.py:
##########
@@ -102,6 +116,49 @@ class MongoDBEngineSpec(BaseEngineSpec):
),
}
+ @classmethod
+ def adjust_engine_params(
+ cls,
+ uri: URL,
+ connect_args: dict[str, Any],
+ catalog: Optional[str] = None,
+ schema: Optional[str] = None,
+ ) -> tuple[URL, dict[str, Any]]:
+ uri, new_connect_args = super().adjust_engine_params(
+ uri,
+ connect_args,
+ catalog,
+ schema,
+ )
+
+ if schema:
+ new_connect_args = {**new_connect_args, "database": schema}
+
+ return uri, new_connect_args
+
+ @classmethod
+ def get_schema_from_engine_params(
+ cls,
+ sqlalchemy_uri: URL,
+ connect_args: dict[str, Any],
+ ) -> Optional[str]:
+ # PyMongoSQL gives the ``database`` connect argument precedence over the URI.
+ return connect_args.get("database") or sqlalchemy_uri.database or None
+
+ @classmethod
+ def get_default_schema(
+ cls,
+ database: Database,
+ catalog: Optional[str],
+ ) -> Optional[str]:
+ return cls.get_schema_from_engine_params(
+ make_url_safe(database.sqlalchemy_uri), database.connect_args
+ )
+
+ @classmethod
+ def quote_table(cls, table: Table, dialect: Dialect) -> str:
+ return dialect.identifier_preparer.quote(table.table)
Review Comment:
Opened #44576 to track it.
--
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(mongodb): treat the Mongo database as the SQL Lab schema so Data Preview returns rows [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on code in PR #44141:
URL: https://github.com/apache/superset/pull/44141#discussion_r4066152935
##########
superset/db_engine_specs/mongodb.py:
##########
@@ -102,6 +116,49 @@ class MongoDBEngineSpec(BaseEngineSpec):
),
}
+ @classmethod
+ def adjust_engine_params(
+ cls,
+ uri: URL,
+ connect_args: dict[str, Any],
+ catalog: Optional[str] = None,
+ schema: Optional[str] = None,
+ ) -> tuple[URL, dict[str, Any]]:
+ uri, new_connect_args = super().adjust_engine_params(
+ uri,
+ connect_args,
+ catalog,
+ schema,
+ )
+
+ if schema:
+ new_connect_args = {**new_connect_args, "database": schema}
Review Comment:
Could we also update `Database._get_sqla_engine()` to put these returned arguments back into `engine_kwargs["connect_args"]` before the connection mutator and cache lookup? [That caller](https://github.com/apache/superset/blob/5b453e16024d9e8f4832a9d5caadcfc390e82b00/superset/models/core.py#L650-L655) captures the new dictionary in a local `connect_args` variable, but `create_engine()` still receives the original dictionary.
I reproduced this through `Database.get_sqla_engine(schema="dbtwo")`, using a URI ending in `/dbone?mode=superset` and only `{"connect": false}` in the configured connection arguments. SQLAlchemy receives just `{"connect": False}`, and the driver's `database_name` remains `dbone`. With the unqualified preview SQL, selecting `dbtwo.orders` therefore reads `dbone.orders`, or returns no data if that collection doesn't exist there.
The new driver test passes the returned arguments directly to `create_engine()`, so it misses this path. A regression test through `Database.get_sqla_engine()` would catch it and verify that the selected schema reaches the connection.
--
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(mongodb): treat the Mongo database as the SQL Lab schema so Data Preview returns rows [superset]
Posted by "msyavuz (via GitHub)" <gi...@apache.org>.
msyavuz commented on code in PR #44141:
URL: https://github.com/apache/superset/pull/44141#discussion_r4070714651
##########
superset/db_engine_specs/mongodb.py:
##########
@@ -102,6 +116,49 @@ class MongoDBEngineSpec(BaseEngineSpec):
),
}
+ @classmethod
+ def adjust_engine_params(
+ cls,
+ uri: URL,
+ connect_args: dict[str, Any],
+ catalog: Optional[str] = None,
+ schema: Optional[str] = None,
+ ) -> tuple[URL, dict[str, Any]]:
+ uri, new_connect_args = super().adjust_engine_params(
+ uri,
+ connect_args,
+ catalog,
+ schema,
+ )
+
+ if schema:
+ new_connect_args = {**new_connect_args, "database": schema}
Review Comment:
Good catch, this was a real bug. `_get_sqla_engine()` bound the returned dict to a local and passed the original to `create_engine()`. Specs like postgres change it in place so it never showed up, but Mongo returns a copy. Fixed in a7656cdc2f: `engine_kwargs["connect_args"]` is reassigned right after `adjust_engine_params`, before impersonation, the mutator and the cache key. Added `test_get_sqla_engine_applies_selected_schema`, which goes through `get_sqla_engine(schema="dbtwo")` on the real driver and asserts the database is `dbtwo` while `authSource` stays `dbone`.
--
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(mongodb): treat the Mongo database as the SQL Lab schema so Data Preview returns rows [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on code in PR #44141:
URL: https://github.com/apache/superset/pull/44141#discussion_r4063666298
##########
superset/db_engine_specs/mongodb.py:
##########
@@ -102,6 +115,48 @@ class MongoDBEngineSpec(BaseEngineSpec):
),
}
+ @classmethod
+ def adjust_engine_params(
+ cls,
+ uri: URL,
+ connect_args: dict[str, Any],
+ catalog: Optional[str] = None,
+ schema: Optional[str] = None,
+ ) -> tuple[URL, dict[str, Any]]:
+ uri, new_connect_args = super().adjust_engine_params(
+ uri,
+ connect_args,
+ catalog,
+ schema,
+ )
+
+ if schema:
+ uri = uri.set(database=schema)
+
+ return uri, new_connect_args
Review Comment:
There's also a case where the selected schema still gets ignored: PyMongoSQL gives `connect_args["database"]` precedence over the database in the URI. With `{"database": "dbone", "authSource": "dbone"}` in the connection arguments and `dbtwo` selected in SQL Lab, this method returns a `/dbtwo` URI but the driver still uses `dbone`.
Because the preview SQL now uses the bare collection name, a preview of `dbtwo.orders` would read `dbone.orders` if both exist. Can we apply the selected schema to the effective `database` argument too, and have the schema helpers account for that argument? This is reproducible with the real driver using `connect=False`, so it can be covered without a running MongoDB server.
##########
superset/db_engine_specs/mongodb.py:
##########
@@ -102,6 +115,48 @@ class MongoDBEngineSpec(BaseEngineSpec):
),
}
+ @classmethod
+ def adjust_engine_params(
+ cls,
+ uri: URL,
+ connect_args: dict[str, Any],
+ catalog: Optional[str] = None,
+ schema: Optional[str] = None,
+ ) -> tuple[URL, dict[str, Any]]:
+ uri, new_connect_args = super().adjust_engine_params(
+ uri,
+ connect_args,
+ catalog,
+ schema,
+ )
+
+ if schema:
+ uri = uri.set(database=schema)
Review Comment:
Could we preserve the authentication database when switching schemas? With `mongodb://user:pass@host:27017/dbone?mode=superset`, selecting `dbtwo` changes both the query database and the implicit `authSource` to `dbtwo`. A Mongo user created in `dbone` with access to `dbtwo` would then fail authentication, including when listing that schema's collections, which previously used the original connection.
I checked the resulting credentials with PyMongoSQL 0.7.3 and `connect=False`: the authentication source changes from `dbone` to `dbtwo`. The driver accepts a separate `database` connection argument, so selecting the query database that way could preserve the original authentication settings. A regression case without an explicit `authSource`, plus a control with one, would cover this. [MongoDB documents the default here](https://www.mongodb.com/docs/manual/reference/connection-string-options/#authentication-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(mongodb): treat the Mongo database as the SQL Lab schema so Data Preview returns rows [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44141:
URL: https://github.com/apache/superset/pull/44141#issuecomment-5766353426
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(mongodb): treat the Mongo database as the SQL Lab schema so Data Preview returns rows [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44141:
URL: https://github.com/apache/superset/pull/44141#issuecomment-5777667217
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