codeant-ai-for-open-source[bot] commented on code in PR #45058:
URL: https://github.com/apache/superset/pull/45058#discussion_r4210166998
##########
superset/connectors/sqla/partition_mapping.py:
##########
@@ -2297,6 +2458,44 @@ def _predicates_from_probe(
}
+#: Day/second keys a temporal column accepts as text, beyond ISO 8601.
+#: ``YYYYMMDD`` is the commonest bucketing key this feature exists for, and
+#: Postgres, Trino and BigQuery all read it as a date.
+_TEMPORAL_KEY_FORMATS = ("%Y%m%d", "%Y%m%d%H%M%S", "%Y-%m-%d %H:%M:%S")
Review Comment:
**Suggestion:** This accepts compact timestamps without checking the
database dialect; Trino rejects the 14-digit form as a temporal value, so the
emitted mirror can make chart queries fail.
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes` ยท ๐ท๏ธ `Type error`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=497fdc99f03042988b55f2b4924fdf13&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=497fdc99f03042988b55f2b4924fdf13&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/connectors/sqla/partition_mapping.py
**Line:** 2464:2464
**Comment:**
*Type Error: This accepts compact timestamps without checking the
database dialect; Trino rejects the 14-digit form as a temporal value, so the
emitted mirror can make chart queries fail.
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%2F45058&comment_hash=d552a52ddffdd81bfeadedf1e921a67f99ca074196f99294fd4de62183a9a832&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45058&comment_hash=d552a52ddffdd81bfeadedf1e921a67f99ca074196f99294fd4de62183a9a832&reaction=dislike'>๐</a>
##########
superset/commands/dataset/refresh.py:
##########
@@ -54,6 +54,21 @@ def __init__(self, model_id: int):
def run(self) -> Model:
self.validate()
assert self._model
+ # Before the metadata lands as well as after, the way
`DatasetDAO.update`
+ # does it -- and for the reason spelled out on
+ # `clear_unmapped_partition_transforms`: it reads the mapping as it
+ # stands, so one call can only enforce the invariant against one of the
+ # two resolutions a mapping change has.
+ #
+ # `fetch_metadata` can *move* the effective mapped column, by dropping
+ # the column `partition_mapped_column` names (the mapping then falls
+ # back to `main_dttm_col`) or by setting `main_dttm_col` itself. Run
only
+ # afterwards, the cleanup resolved the *new* mapping, found the newly
+ # mapped column effective and skipped it -- so a transform parked
there,
+ # which nobody asked to activate, went live and started adding its own
+ # predicate to every filter, while the previously mapped column's real
+ # transform was the one erased.
+ DatasetDAO.clear_unmapped_partition_transforms(self._model)
Review Comment:
**Suggestion:** Every refresh clears transforms on non-mapped columns, even
when metadata is unchanged or parsing fails, so a harmless refresh loses stored
configuration.
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes` ยท ๐ท๏ธ `Logic error`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=55e7c5f6f0714eecb5f122af7ec7c13c&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=55e7c5f6f0714eecb5f122af7ec7c13c&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/commands/dataset/refresh.py
**Line:** 71:71
**Comment:**
*Logic Error: Every refresh clears transforms on non-mapped columns,
even when metadata is unchanged or parsing fails, so a harmless refresh loses
stored configuration.
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%2F45058&comment_hash=c0e9f487aa27e98d6bce5fb37aecb09f9fe154818f4729d33e143434fbdb5889&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45058&comment_hash=c0e9f487aa27e98d6bce5fb37aecb09f9fe154818f4729d33e143434fbdb5889&reaction=dislike'>๐</a>
##########
superset/connectors/sqla/partition_mapping.py:
##########
@@ -1330,6 +1425,40 @@ def _to_python_scalar(value: Any) -> Any:
return value
+def _execution_identity(database: Database) -> str | None:
+ """
+ Who the warehouse will run the probe as, when that can vary by caller.
+
+ `_probe_cache_key` keys on the `Database` record, which is the *connection*
+ -- not the identity it is opened under. Two hooks make that identity a
+ function of the Superset user instead:
+
+ * ``impersonate_user`` on the database, which hands the effective username
+ to `db_engine_spec.impersonate_user`;
+ * ``DB_CONNECTION_MUTATOR``, which receives the effective username and may
+ return an entirely different account's URL.
+
+ Under either one, a cached probe result computed under user A's grants was
+ served to user B, and the preview echoed it back in ``emitted_predicate``.
+ A transform wrapping a read-capable function the denylist does not name --
+ Oracle's ``DBMS_XMLGEN.GETXML`` is the worked example -- then leaks across
+ users who were never allowed the same rows.
+
+ Returns ``None`` when neither hook is configured, which is the common case
+ and deliberately leaves the key as it was: the cache exists to keep a
+ synchronous warehouse round trip off the chart-query path, and keying every
+ deployment on the username would cost every deployment the hit rate to fix
+ the two that need it.
+
+ The *Superset* username rather than the resolved warehouse account, because
+ it is the input both hooks branch on, and reading it needs no URL parse and
+ puts no credential in the key.
+ """
+ if not (database.impersonate_user or app.config["DB_CONNECTION_MUTATOR"]):
+ return None
Review Comment:
**Suggestion:** OAuth2 connections use tokens per Superset user, but this
returns `None` for them, so cache keys collide and one user can receive another
user's probe results.
**Assessment:** ๐ด `Critical` ยท ๐ `Occurrence: Sometimes` ยท ๐ท๏ธ `Security`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=19cd3e22db6945918bff26fa87a611d9&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=19cd3e22db6945918bff26fa87a611d9&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/connectors/sqla/partition_mapping.py
**Line:** 1457:1458
**Comment:**
*Security: OAuth2 connections use tokens per Superset user, but this
returns `None` for them, so cache keys collide and one user can receive another
user's probe results.
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%2F45058&comment_hash=e67e6b2eb4c6e16c1e515a834261abcc55eef5eb0671020c72ef176f17c343f3&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45058&comment_hash=e67e6b2eb4c6e16c1e515a834261abcc55eef5eb0671020c72ef176f17c343f3&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]
