aminghadersohi commented on code in PR #43870:
URL: https://github.com/apache/superset/pull/43870#discussion_r4185252045
##########
superset/models/helpers.py:
##########
@@ -4290,36 +4297,283 @@ def dttm_sql_literal(self, dttm: datetime, col:
"TableColumn") -> str:
return f"""'{dttm.strftime("%Y-%m-%d %H:%M:%S.%f")}'"""
- def get_time_filter( # pylint: disable=too-many-arguments # noqa: C901
+ def _mirror_probe_value(self, dttm: datetime, col:
Optional["TableColumn"]) -> Any:
+ """
+ ``dttm`` in the mapped column's own stored representation.
+
+ The predicate this mirror stands in for compares the column against
+ `dttm_sql_literal(dttm, col)`, so the transform has to be probed with
+ the same value. On a column stored as an epoch integer, ``T`` is a
+ function of an integer: probing it with a `datetime` either returns
+ something unrelated to the partition keys or raises, and a raise costs
+ the dataset its pruning silently.
+
+ Only the ``python_date_format`` branches of `dttm_sql_literal` are
+ reproduced, because only those produce a *value*. `convert_dttm` yields
+ engine-specific SQL text, which cannot be bound as a parameter; its own
+ divergence from the probe is handled by widening the bounds instead.
+ """
+ if col is None:
+ return dttm
+
+ tf = col.python_date_format
+ if not tf and self.db_extra:
+ tf = self.db_extra.get("python_date_format_by_column_name",
{}).get(
+ col.column_name
+ )
+ if not tf:
+ return dttm
+
+ if tf in EPOCH_FORMATS:
+ dttm_tz_aware = dttm
+ if dttm_tz_aware.tzinfo is None:
+ dttm_tz_aware = dttm_tz_aware.replace(tzinfo=timezone.utc)
+ return int(dttm_tz_aware.timestamp()) * EPOCH_FORMATS[tf]
+ return dttm.strftime(tf)
+
+ def _engine_drops_subseconds(
+ self, dttm: datetime, col: Optional["TableColumn"]
+ ) -> bool:
+ """
+ Whether this engine's literal for ``dttm`` loses the sub-second part.
+
+ SQLite renders a timestamp with ``timespec="seconds"``, so the real
+ predicate compares a floored bound while the probe is handed the full
+ precision -- which makes the mirror *narrower* than the predicate it
+ stands in for, and narrower means dropped rows.
+
+ Detected rather than enumerated per engine: two instants that differ
+ only below the second render as the same text exactly when the engine
+ has thrown the remainder away.
+
+ Both probes carry a non-zero sub-second part on purpose. Comparing
+ against ``dttm`` truncated to the second looks like the obvious test
+ and is wrong, because truncating can cross a formatting boundary as
+ well as a precision one -- SQLite renders a DATE column at exactly
+ midnight as a bare date, so the two texts differ for a reason that has
+ nothing to do with precision and the truncation goes unnoticed.
+
+ Note this only distinguishes second-level precision. An engine that
+ rendered no time part at all would need the bounds widened to the whole
+ day, which this does not do; no supported engine does that for a value
Review Comment:
Presto, Trino, BigQuery and ~20 other specs render a DATE bound with no
time, so this happens: a 10:00 start gives `event_date >= DATE '2026-01-01'`
but `dt_epoch >= 1767261600`, dropping Jan 1 rows the filter keeps. No
suggestion: the fix spans this detector and the rounding.
##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx:
##########
@@ -617,100 +674,160 @@ function ColumnCollectionTable({
/>
);
- return (
- <CollectionTable
- tableColumns={tableColumns}
- sortColumns={tableColumns}
- allowDeletes
- allowAddItem={allowAddItem}
- itemGenerator={itemGenerator}
- collection={columns}
- columnLabelTooltips={columnLabelTooltips}
- filterTerm={filterTerm}
- filterFields={filterFields}
- stickyHeader
- expandFieldset={
- <FormContainer>
- <Fieldset compact>
- {showExpression && (
- <Field
- fieldKey="expression"
- label={t('SQL expression')}
- control={
- <TextAreaControl
- language="sql"
- offerEditInModal={false}
- maxLines={25}
- debounceDelay={300}
- />
- }
- />
- )}
- <Field
- fieldKey="verbose_name"
- label={t('Label')}
- control={
- <TextControl
- controlId="verbose_name"
- placeholder={t('Label')}
- />
- }
+ const partitionMappingEnabled =
+ isFeatureEnabled(FeatureFlag.PartitionFilterMapping) &&
Boolean(datasource);
+ const partitionColumn = partitionMappingEnabled
+ ? datasource?.partition_column
+ : null;
+
+ // The two `itemRenderers` variants below differ only in which widget edits
+ // the name, so the certified badge and the PARTITION tag are shared here
+ // rather than written out four times.
+ const renderColumnName =
+ (EditControl: 'editableTitle' | 'textControl') =>
+ (
+ v: unknown,
+ onItemChange: (value: any) => void,
+ _: unknown,
+ record: Column,
+ ): ReactNode => (
+ <StyledLabelWrapper>
+ {record.is_certified && (
+ <CertifiedBadge
+ certifiedBy={record.certified_by}
+ details={record.certification_details}
+ />
+ )}
+ {editableColumnName ? (
+ EditControl === 'editableTitle' ? (
+ <EditableTitle
+ canEdit
+ title={v as string}
+ onSaveTitle={onItemChange}
/>
- <Field
- fieldKey="description"
- label={t('Description')}
- control={
- <TextControl
- controlId="description"
- placeholder={t('Description')}
+ ) : (
+ <TextControl value={v as string} onChange={onItemChange} />
+ )
+ ) : (
+ (v as ReactNode)
+ )}
+ {partitionColumn === record.column_name && (
+ <Tooltip
+ title={t(
+ 'Filters on the mapped column are mirrored onto this column so
the engine can prune partitions.',
+ )}
+ >
+ <Label data-test="partition-tag">
+ <Icons.FilterOutlined iconSize="s" /> {t('PARTITION')}
+ </Label>
+ </Tooltip>
+ )}
+ </StyledLabelWrapper>
+ );
+
+ return (
+ <StyledColumnsTableWrapper>
+ <CollectionTable
+ tableColumns={tableColumns}
+ sortColumns={tableColumns}
+ allowDeletes
+ allowAddItem={allowAddItem}
+ itemGenerator={itemGenerator}
+ collection={columns}
+ columnLabelTooltips={columnLabelTooltips}
+ filterTerm={filterTerm}
+ filterFields={filterFields}
+ rowClassName={record =>
Review Comment:
This `rowClassName` is now ignored: the merge took master's CollectionTable
whole, dropping its `rowClassName` prop and `<Table>` passthrough, so the
partition row is never muted. No suggestion: the fix is in
CollectionTable/index.tsx, outside the diff.
--
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]
