codeant-ai-for-open-source[bot] commented on code in PR #44816:
URL: https://github.com/apache/superset/pull/44816#discussion_r4152921665
##########
superset/datasets/schemas.py:
##########
@@ -101,6 +192,22 @@ class DatasetColumnsPutSchema(Schema):
datetime_format = fields.String(
allow_none=True, validate=[Length(1, 100), validate_python_date_format]
)
+ partition_value_transform = fields.String(
+ allow_none=True,
+ metadata={
+ "description": (
+ "SQL expression containing a :value placeholder. Filters on "
+ "this column are mirrored onto the dataset's partition column "
+ "with the value passed through this transform."
+ )
+ },
+ )
Review Comment:
Yes. The typed field should use the same `MAX_TRANSFORM_LENGTH` bound as
`extra` validation and `parse_mapping`; otherwise an overlong transform is
accepted by the API, persisted, then treated as inactive on read.
Update the schema field to:
```python
partition_value_transform = fields.String(
allow_none=True,
validate=Length(1, MAX_TRANSFORM_LENGTH),
metadata={
"description": (
"SQL expression containing a :value placeholder. Filters on "
"this column are mirrored onto the dataset's partition column "
"with the value passed through this transform."
)
},
)
```
Also add a schema test asserting that a value longer than
`MAX_TRANSFORM_LENGTH` is rejected. This keeps the typed API, `extra`
validation, and storage reader consistent, while preventing oversized input
from reaching parsing or persistence.
##########
superset-frontend/src/components/Datasource/DatasourceModal/index.tsx:
##########
@@ -183,6 +183,8 @@ const DatasourceModal:
FunctionComponent<DatasourceModalProps> = ({
currency_code_column: datasource.currency_code_column ?? null,
normalize_columns: datasource.normalize_columns,
always_filter_main_dttm: datasource.always_filter_main_dttm,
+ partition_column: datasource.partition_column ?? null,
+ partition_mapped_column: datasource.partition_mapped_column ?? null,
Review Comment:
Yes, this is a valid issue. `DatasourceModal` currently always includes:
```ts
partition_column: datasource.partition_column ?? null,
partition_mapped_column: datasource.partition_mapped_column ?? null,
```
When `extra` was edited directly, these values may be stale (or `null`).
`DatasetDAO` treats present typed fields as a patch, so they overwrite the
mapping parsed from `extra`.
The editor should treat `extra` as authoritative when it is the source of
the edit. Track the initial `extra` value and omit the typed mapping fields—and
the per-column transform fields—when `extra` has changed independently. This
lets the DAO persist the mapping from `extra` without applying a stale typed
patch.
Alternatively, the backend can enforce the same rule: when `extra` is
present in the request, do not apply typed mapping fields unless they were
explicitly changed. The latter is safer for non-frontend clients, while the
frontend change prevents the problematic payload from being generated. This
should be fixed before merging; otherwise editing the Extra box can silently
discard the newly entered mapping.
--
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]
