codeant-ai-for-open-source[bot] commented on code in PR #42603:
URL: https://github.com/apache/superset/pull/42603#discussion_r3680227987
##########
superset/databases/schemas.py:
##########
@@ -1455,6 +1455,19 @@ def convert_column_data_types(
) from ex
return data
+ @post_load
+ def normalize_schema(self, data: dict[str, Any], **kwargs: Any) ->
dict[str, Any]:
+ # Broken clients stringify an unset schema into the multipart body as
+ # "undefined"/"null"; neither is a plausible real schema name, so both
+ # are treated as unset alongside empty/whitespace values (see #36305).
+ if (schema := data.get("schema")) is not None:
+ schema = schema.strip()
+ if not schema or schema.lower() in ("undefined", "null"):
+ data.pop("schema", None)
+ else:
Review Comment:
**Suggestion:** This changes the meaning of valid explicit schema names:
databases such as PostgreSQL allow quoted identifiers named `NULL` or
`undefined`, and identifiers containing leading or trailing whitespace. Those
schemas previously reached the upload command unchanged, but are now silently
converted to the default schema, potentially writing the file to a different
schema or failing authorization. Only normalize values known to have originated
from an unset multipart value, or preserve explicitly supplied schema
identifiers. [data type]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Explicit uploads can target the wrong database schema.
- ⚠️ Valid schema selections may fail authorization unexpectedly.
- ❌ Dataset metadata can record the resolved default schema.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8f0b205771564865871b9ab64174726f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=8f0b205771564865871b9ab64174726f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/databases/schemas.py
**Line:** 1463:1467
**Comment:**
*Data Type: This changes the meaning of valid explicit schema names:
databases such as PostgreSQL allow quoted identifiers named `NULL` or
`undefined`, and identifiers containing leading or trailing whitespace. Those
schemas previously reached the upload command unchanged, but are now silently
converted to the default schema, potentially writing the file to a different
schema or failing authorization. Only normalize values known to have originated
from an unset multipart value, or preserve explicitly supplied schema
identifiers.
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%2F42603&comment_hash=eb39acabdabba27517964cdbfe75be0406698e13d7936fbb1d4ba8f67da4a50e&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42603&comment_hash=eb39acabdabba27517964cdbfe75be0406698e13d7936fbb1d4ba8f67da4a50e&reaction=dislike'>👎</a>
##########
superset/commands/database/uploaders/base.py:
##########
@@ -261,10 +261,28 @@ def validate_file_size(cls, file: Any) -> None:
if size is not None and size > max_file_size:
raise DatabaseUploadFileTooLarge()
+ @staticmethod
+ def _resolve_default_schema(database: Database) -> Optional[str]:
+ """Resolve the database's default schema so uploaded datasets carry an
+ explicit schema instead of NULL, which would otherwise duplicate an
+ existing dataset over the same table (see #36305)."""
+ try:
+ return database.get_default_schema(database.get_default_catalog())
+ except Exception: # pylint: disable=broad-except
+ # Resolution opens an inspector connection; a failure here must
+ # degrade to the no-schema behavior rather than fail the upload.
+ logger.warning(
+ "Unable to resolve default schema for upload; proceeding
without one",
+ exc_info=True,
+ )
+ return None
+
def validate(self) -> None:
self._model = DatabaseDAO.find_by_id(self._model_id)
if not self._model:
raise DatabaseNotFoundError()
+ if not self._schema:
+ self._schema = self._resolve_default_schema(self._model)
if not schema_allows_file_upload(self._model, self._schema):
raise DatabaseSchemaUploadNotAllowed()
Review Comment:
**Suggestion:** The resolved default schema is checked with exact string
membership, while the schema picker/API treats configured upload schemas
case-insensitively. If the database reports its default schema as `PUBLIC` and
the allow-list contains `public`, the UI presents the schema as allowed but
this upload is rejected with `DatabaseSchemaUploadNotAllowed`. Normalize both
values consistently before the allow-list check. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Default-schema uploads can be rejected unexpectedly.
- ⚠️ UI allow-list results differ from backend authorization.
- ❌ Uploads fail before dataframe persistence and dataset creation.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=6ac1e6b66a824139a7891c0b746292e0&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=6ac1e6b66a824139a7891c0b746292e0&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/commands/database/uploaders/base.py
**Line:** 286:287
**Comment:**
*Api Mismatch: The resolved default schema is checked with exact string
membership, while the schema picker/API treats configured upload schemas
case-insensitively. If the database reports its default schema as `PUBLIC` and
the allow-list contains `public`, the UI presents the schema as allowed but
this upload is rejected with `DatabaseSchemaUploadNotAllowed`. Normalize both
values consistently before the allow-list check.
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%2F42603&comment_hash=b395f80a7139479c35852e297f144fc25f782c7150e52f5c02dc1f8a5f4b1306&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42603&comment_hash=b395f80a7139479c35852e297f144fc25f782c7150e52f5c02dc1f8a5f4b1306&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: [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]
