hughhhh commented on code in PR #43870:
URL: https://github.com/apache/superset/pull/43870#discussion_r4362308007
##########
superset/db_engine_specs/base.py:
##########
@@ -580,6 +580,47 @@ def normalize_custom_sql_metric(cls, expression: str) ->
str:
allows_joins = True
allows_subqueries = True
allows_alias_in_select = True
+ # Appended to a ``SELECT`` that has no ``FROM`` -- evaluating expressions
+ # and nothing else. Most engines accept a bare ``SELECT 1``; Oracle, DB2
+ # and Db2 for i require a one-row table to select it from.
+ select_without_from_suffix: str = ""
+ # Whether ``=`` on a text column compares byte-exactly by default: no case
+ # folding, no trailing-space padding, no accent insensitivity.
+ #
+ # Partition filter mapping needs this. It mirrors ``col = v`` onto
+ # ``partition_col = T(v)`` on the strength of ``col = v`` implying
+ # ``T(col) = T(v)``, which holds for value equality and not for SQL
+ # equality: under a case-insensitive collation a stored ``'us'`` satisfies
+ # a filter for ``'US'``, while the mirror ``hex('US')`` excludes the row,
+ # and the chart silently loses it. False by default, so a spec that has
+ # not said so does not mirror string equality.
+ #
+ # This speaks for the engine's *default* comparison only. A column
+ # declaring its own non-binary collation (``country COLLATE NOCASE``) is
+ # invisible to Superset -- nothing in SQLAlchemy's reflection or the engine
+ # specs exposes it -- so on such a column the assumption remains the
+ # owner's, like ``p = T(mapped_col)`` itself.
+ binary_string_comparison: bool = False
+
+ # Every text form this engine reads as a date or timestamp when one is
+ # compared against a temporal column. `strftime`/`strptime` patterns.
+ #
+ # Partition filter mapping needs this. A bucketing key such as
+ # ``to_char(:value, 'YYYYMMDD')`` is legitimately text, so the mirror
+ # ``part_date = '20260115'`` has to be allowed against a ``DATE`` partition
+ # column -- but only where the engine reads it, because comparing a
+ # temporal column against text is an implicit coercion an engine need not
+ # have. Trino has no VARCHAR-to-DATE comparison and BigQuery coerces only a
+ # canonical ``YYYY-MM-DD`` *literal*, so on those even plain ISO text fails
+ # the whole chart rather than merely losing the pruning -- which is why the
+ # ISO forms are declared here too rather than assumed universal.
+ #
+ # Empty by default, so an engine Superset has not confirmed declines the
+ # mirror and keeps the query working. Declared on concrete specs, not on a
+ # shared base: `PostgresBaseEngineSpec` is inherited by Snowflake, whose
+ # AUTO date input reads an all-digit string as epoch seconds instead.
+ temporal_literal_formats: tuple[str, ...] = ()
Review Comment:
Good catch, thanks. Fixed in
[5615540384](https://github.com/apache/superset/pull/43870/commits/5615540384).
A probed `date`/`datetime` is now written with the engine's own literal via
`convert_dttm` (the same path as the chart's own time bounds), so on Trino this
emits `dt >= DATE '2026-01-01'` for ranges, `=` and `IN`. Where the engine has
no faithful literal for the column, the mirror is declined and only pruning is
lost. Added dialect-level tests and a preview test.
##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx:
##########
@@ -1365,19 +1695,76 @@ function DatasourceEditor({
try {
const newCols = await fetchSyncedColumns(datasource, signal);
+ // Everything past the `await` reads the refs rather than the closure;
+ // see their declaration.
+ const liveColumns = currentColumnsRef.current;
+ const liveDatasource = datasourceRef.current;
+
const columnChanges = updateColumns(
- datasource.columns,
+ liveColumns,
Review Comment:
Thanks, reproduced. Fixed in
[e8935b2abe](https://github.com/apache/superset/pull/43870/commits/e8935b2abe).
The merge still runs against the live rows, but a column that comes back as
"added" and exists in `datasource.columns` is restored from its saved row
(taking the source's type), so the save updates it instead of re-creating the
name. Added a test for delete → sync → save.
##########
superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterLabel.tsx:
##########
@@ -420,7 +422,12 @@ export default function DateFilterLabel(props:
DateFilterControlProps) {
{...props}
onDescriptionHoverChange={setIsDescriptionHovered}
/>
- {popoverContent}
+ {/* The glyph sits outside the popover trigger so clicking it opens the
+ tooltip rather than the time-range editor. */}
+ <Flex align="center" gap={theme.sizeUnit}>
Review Comment:
Agreed. Fixed in
[f94ce2fe3c](https://github.com/apache/superset/pull/43870/commits/f94ce2fe3c).
The `Flex` is now rendered only when `partitionMapping` is set, so the flag-off
markup matches master. With a mapping, the trigger also gets `flex: 1;
min-width: 0`, so the box fills the row instead of shrinking. Added tests for
both cases.
--
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]
