hughhhh commented on PR #44816: URL: https://github.com/apache/superset/pull/44816#issuecomment-5943835224
Closing in favour of **#43870**, which now carries the whole feature — model, migration, query rewrite, preview, editor UI and the Explore indicator — in a single PR against `master`. This PR's contribution was the storage question: it replaced the four new columns with a `PartitionMappingStore` port over `tables.extra` so the feature could ship with no Alembic migration. #43870 keeps the migration, so that trade is not being taken. The argument for real columns is in its summary; the argument against the blob is the one this PR's own description makes about `extra` being a user-editable free-text box that `buildExtraJsonObject` rebuilds on every save. Two things from here were worth keeping and are in #43870 independently of the storage choice: - `_transform_is_usable` omitted the non-deterministic denylist, so a transform calling `now()` that reached storage without passing `UpdateDatasetCommand` was mirrored at query time and its result cached. Fixed in `7951536198` by deleting the function in favour of `is_transform_active`, the same shape this PR proposed. - `columns.partition_value_transform` was missing from the dataset GET payload while `DatasourceModal` PUT it back for every column, so every editor save cleared the transform. Fixed on the stack before this pass. If the migration turns out to be the blocker for deployment, this is the conversation to reopen — the branch is still there. @sadpandajoe your four comments here are unaddressed and still stand against this approach; they are not carried into #43870 because they are about the `tables.extra` store, which it does not have. -- 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]
