eschutho commented on code in PR #43870:
URL: https://github.com/apache/superset/pull/43870#discussion_r4233511037


##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/components/PartitionFilterMapping/utils.ts:
##########
@@ -0,0 +1,587 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { t } from '@apache-superset/core/translation';
+import type {
+  PartitionMappingColumn,
+  PartitionMappingDatasource,
+  PartitionMappingIssue,
+  PartitionRowState,
+} from './types';
+
+/**
+ * Jinja delimiters.
+ *
+ * A transform is evaluated in a different context and at a different time from
+ * the chart query, so a template would not render the same way -- the backend
+ * rejects one outright. Shared so the check that declines to preview a Jinja
+ * transform and the check that blocks saving it cannot drift apart.
+ */
+const JINJA_PATTERN = /\{\{|\{%|\{#/;
+
+/**
+ * The column whose filters are mirrored.
+ *
+ * `partition_mapped_column` is an explicit override; `null` means "follow the
+ * default datetime column", so re-pointing that column moves the mapping with
+ * it. Mirrors `SqlaTable.partition_filter_mapping_summary` on the backend.
+ */
+export function resolveMappedColumn(
+  datasource: PartitionMappingDatasource,
+): string | null {
+  if (!datasource.partition_column) {
+    return null;
+  }
+  return datasource.partition_mapped_column || datasource.main_dttm_col || 
null;
+}
+
+/** Whether the mapped column came from `main_dttm_col` rather than an 
override. */
+export function mappedColumnIsImplicit(
+  datasource: PartitionMappingDatasource,
+): boolean {
+  return Boolean(
+    datasource.partition_column &&
+    !datasource.partition_mapped_column &&
+    datasource.main_dttm_col,
+  );
+}
+
+/**
+ * Whether a transform is worth spending a preview request on.
+ *
+ * A necessary condition, never a sufficient one: the server stays the 
authority
+ * on whether a transform is usable, and anything this cannot see -- an
+ * unparseable expression, a non-deterministic function -- still comes back 
from
+ * it. What this catches is the cases that are knowably hopeless while the 
owner
+ * is still typing, which would otherwise spend the per-user preview budget and
+ * leave none for the transform they eventually finish writing.
+ */
+export function transformCanPreview(
+  transform: string | null | undefined,
+  mappedColumn?: string | null,
+  partitionColumn?: string | null,
+): boolean {
+  const trimmed = transform?.trim();
+  if (!trimmed) {
+    return false;
+  }
+  // Without the placeholder there is no value to substitute, so the transform
+  // is inert however well-formed it is.
+  if (!/:value\b/.test(trimmed)) {
+    return false;
+  }
+  // Jinja is rejected on save, so previewing it is pointless.
+  if (JINJA_PATTERN.test(trimmed)) {
+    return false;
+  }
+  // A column cannot stand in for itself; the backend rejects this outright.
+  if (mappedColumn && partitionColumn && mappedColumn === partitionColumn) {
+    return false;
+  }
+  return true;
+}
+
+/**
+ * Whether the mapping will actually mirror anything.
+ *
+ * Drives the choice between the green "filters will automatically apply"
+ * message and the warning that nothing is mirrored yet, so "non-blank text in
+ * the box" is not a good enough test: a transform with no `:value` to
+ * substitute, or one carrying Jinja, is as inert as an empty one and the
+ * backend's own summary reports it inactive. Claiming otherwise tells an owner
+ * their queries are pruning when they are not.
+ *
+ * Shares `transformCanPreview`'s conditions rather than restating them, and is
+ * necessary rather than sufficient for the same reason: an unparseable
+ * expression or a non-deterministic function needs a SQL parser to spot, so 
the
+ * server stays the authority -- `valueTransformIssues` surfaces what it says 
at
+ * the transform field itself.
+ */
+export function mappingIsActive(
+  datasource: PartitionMappingDatasource,
+  columns: PartitionMappingColumn[],
+): boolean {
+  const mappedColumnName = resolveMappedColumn(datasource);
+  if (!mappedColumnName || mappedColumnName === datasource.partition_column) {
+    return false;
+  }
+  const mappedColumn = columns.find(
+    column => column.column_name === mappedColumnName,
+  );
+  return transformCanPreview(
+    mappedColumn?.partition_value_transform,
+    mappedColumnName,
+    datasource.partition_column,
+  );
+}
+
+/**
+ * What is wrong with one column's value transform, if anything.
+ *
+ * Split out of `partitionMappingErrors` because the two callers disagree about
+ * which text to judge. The save gate reads the committed column record; the
+ * field's own message has to describe the text on screen, which lags the 
commit
+ * by a debounce. Passing the transform separately lets both ask the same
+ * question about different values instead of reimplementing the checks.
+ */
+export function valueTransformIssues(
+  column: PartitionMappingColumn | undefined,
+  transform: string | null | undefined,
+): PartitionMappingIssue[] {
+  if (!column) {
+    return [];
+  }
+
+  if (JINJA_PATTERN.test(transform ?? '')) {
+    return [
+      {
+        field: 'partition_value_transform',
+        message: t(
+          'Jinja templating is not supported in a partition value transform. 
The transform is evaluated in a different context and at a different time from 
the chart query, so a template would not render the same way.',
+        ),
+      },
+    ];
+  }
+
+  // A temporal column falls back on the engine's own default, so leaving the 
box
+  // empty there only makes the mapping inactive -- which the dataset-level
+  // warning already says. A non-temporal column has nothing to fall back on.
+  if (!column.is_dttm && !transform?.trim()) {
+    return [
+      {
+        field: 'partition_value_transform',
+        message: t(
+          'A value transform is required on %(name)s, which is not a temporal 
column. Use :value for each value.',
+          { name: column.column_name },
+        ),
+      },
+    ];
+  }
+
+  return [];
+}
+
+/**
+ * Everything wrong with the mapping that should stop the save.
+ *
+ * The client half of `validate_partition_mapping` in
+ * `superset/connectors/sqla/partition_mapping.py`, restricted to its blocking
+ * (Tier 1) issues. Those are the ones the PUT would reject anyway, so catching
+ * them here turns a 400 toast arriving after the modal closed into a disabled
+ * Save button that says why. Tier 2 is deliberately absent: an unparseable
+ * transform, or one missing `:value`, is "saved but stays inactive until it
+ * parses" per the PRD, and must not cost the owner the rest of their edits.
+ *
+ * One check is stricter than the backend's: an empty transform on a
+ * non-temporal mapped column blocks. The backend files that as Tier 2 for 
every
+ * column, but a non-temporal column has no default transform to fall back on 
--
+ * the field carries a required marker saying exactly this -- so the marker
+ * should mean something.
+ *
+ * Callers gate on the feature flag themselves, the way the two mount sites in
+ * `DatasourceEditor` already do; a dataset carrying a stale `partition_column`
+ * on an engine that cannot use one must not block a save the backend accepts.
+ */
+export function partitionMappingErrors(
+  datasource: PartitionMappingDatasource,
+  columns: PartitionMappingColumn[],
+): PartitionMappingIssue[] {
+  const partitionColumn = datasource.partition_column;
+  if (!partitionColumn) {
+    return [];
+  }
+
+  const issues: PartitionMappingIssue[] = [];
+  const columnNames = new Set(columns.map(column => column.column_name));
+
+  if (!columnNames.has(partitionColumn)) {
+    issues.push({
+      field: 'partition_column',
+      message: t('Partition column %(name)s is not a column on this dataset.', 
{
+        name: partitionColumn,
+      }),
+    });
+  }
+
+  const override = datasource.partition_mapped_column;
+  if (override && !columnNames.has(override)) {
+    issues.push({
+      field: 'partition_mapped_column',
+      message: t('Mapped column %(name)s is not a column on this dataset.', {
+        name: override,
+      }),
+    });
+  }
+
+  const mappedColumnName = resolveMappedColumn(datasource);
+  if (mappedColumnName === partitionColumn) {
+    // Only the *explicit* self-mapping blocks the save, matching
+    // `validate_partition_mapping`: an override naming the partition column is
+    // something the owner asked for and can take back, so it is Tier 1 there.
+    //
+    // The implicit one -- no override, and `main_dttm_col` happens to equal 
the
+    // partition column -- is Tier 2 on purpose. Blocking it here disabled Save
+    // on a dataset already in that state (metadata sync used to be able to
+    // produce one) even for a description-only edit, which is exactly the
+    // unsaveable dataset the backend comment says it avoids. The owner gets 
the
+    // same message, as a warning they can save through.
+    //
+    // No transform check can follow either way: the transform is a property of
+    // the mapped column, and there is no coherent mapped column to read it 
from.
+    issues.push({
+      field: 'partition_column',
+      blocking: Boolean(datasource.partition_mapped_column),
+      message: t(
+        'The partition column cannot be mapped onto itself. %(name)s is both 
the partition column and the mapped column.',
+        { name: partitionColumn },
+      ),
+    });
+    return issues;
+  }
+
+  const mappedColumn = columns.find(
+    column => column.column_name === mappedColumnName,
+  );
+  issues.push(
+    ...valueTransformIssues(
+      mappedColumn,
+      mappedColumn?.partition_value_transform,
+    ),
+  );
+
+  return issues;
+}
+
+/** Which of the three row-expand treatments a column gets. */
+export function partitionRowState(
+  datasource: PartitionMappingDatasource,
+  columnName: string,
+): PartitionRowState {
+  if (!datasource.partition_column) {
+    return 'none';
+  }
+  if (columnName === datasource.partition_column) {
+    return 'partition';
+  }
+  return resolveMappedColumn(datasource) === columnName ? 'mapped' : 
'unmapped';
+}
+
+/**
+ * The transform to pre-fill when a column becomes the mapped one.
+ *
+ * Only temporal columns get one, and only when the engine supplies it:
+ * `unix_timestamp(:value)` is Hive-family syntax and would not parse on
+ * Postgres or BigQuery, so a wrong default is worse than none.
+ */
+export function defaultTransformFor(
+  datasource: PartitionMappingDatasource,
+  column: PartitionMappingColumn | undefined,
+): string {
+  if (!column?.is_dttm) {
+    return '';
+  }
+  return datasource.partition_value_transform_default || '';
+}
+
+/** Declared types whose values are numbers rather than strings. */
+const NUMERIC_TYPE_RE =
+  /INT|NUMERIC|DECIMAL|DOUBLE|FLOAT|REAL|NUMBER|BIGNUMERIC/i;
+
+/**
+ * Samples the preview evaluates the transform at.
+ *
+ * Temporal columns get one timestamp, because a time-range bound is what they
+ * are mapped for. Everything else gets two values, so the preview demonstrates
+ * the element-wise `IN` an ordinary categorical filter actually produces.
+ *
+ * The samples follow the column's declared type, not just whether it is
+ * temporal. A mapped integer column with `CAST(:value AS INTEGER)` is a real
+ * configuration, and evaluating it at `'US'` makes the engine reject a 
transform
+ * that an actual `IN (2025, 2026)` filter would mirror perfectly well -- the
+ * preview was reporting a fault in its own sample data.
+ */
+export function sampleValuesFor(
+  column: PartitionMappingColumn | undefined,
+): string[] {
+  if (column?.is_dttm) {
+    return ['2026-01-15 00:00:00'];

Review Comment:
   Addressed on Hugh's behalf in dee9cca8f8: `sampleValuesFor` now generates 
the sample in the column's stored representation for a numeric temporal column. 
It uses `1768435200` for `epoch_s`, `1768435200000` for `epoch_ms`, and the 
plain numeric samples for any other format, never a timestamp string. 
Non-numeric temporal columns still get the timestamp. `python_date_format` was 
added to `PartitionMappingColumn`, with unit tests in `utils.test.ts`. I 
couldn't run jest locally, so this relies on CI.



##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/components/PartitionFilterMapping/PartitionColumnFields.tsx:
##########
@@ -0,0 +1,319 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { useMemo } from 'react';
+import { t } from '@apache-superset/core/translation';
+import { css, useTheme } from '@apache-superset/core/theme';
+import { Alert } from '@apache-superset/core/components';
+import {
+  Button,
+  Flex,
+  Icons,
+  InfoTooltip,
+  Label,
+  Select,
+  Tooltip,
+  Typography,
+} from '@superset-ui/core/components';
+import {
+  mappedColumnIsImplicit,
+  mappingIsActive,
+  partitionMappingErrors,
+  resolveMappedColumn,
+  suggestedMappedColumn,
+} from './utils';
+import type {
+  PartitionMappingColumn,
+  PartitionMappingDatasource,
+  PartitionMappingIssue,
+} from './types';
+
+interface PartitionColumnFieldsProps {
+  datasource: PartitionMappingDatasource;
+  /**
+   * Physical columns: what the partition-column dropdown may offer, and where
+   * a replacement mapped column may be suggested from. A calculated column
+   * belongs in neither -- the engine cannot partition on an expression, and
+   * the mapping picker only renders on a physical column's row.
+   */
+  columns: PartitionMappingColumn[];
+  /**
+   * Every column on the dataset, physical and calculated. What *exists* is a
+   * different question from what may be picked, and validation asks the first:
+   * the backend accepts a calculated column as the mapped-column override, so
+   * checking existence against the physical columns alone reported a column
+   * that is really there as missing -- and blocked Save with no way out, since
+   * the dropdown cannot offer it back.
+   */
+  allColumns: PartitionMappingColumn[];
+  onPartitionColumnChange: (columnName: string | null) => void;
+  /** Open the given column's row expand in the Columns table. */
+  onNavigateToColumn: (columnName: string) => void;
+  /**
+   * Whether the mapped column's own preview says the transform mirrors, or
+   * `null`/undefined when no verdict is in yet. The checks this section can
+   * make are all static -- a transform the database rejects clears every one
+   * of them -- so without the preview's answer the banner claimed a speed-up
+   * the query never delivers.
+   */
+  previewMirrors?: boolean | null;
+}
+
+/**
+ * "Partition column" and the computed "Maps to partition" (wireframes 1a, 1g).
+ *
+ * "Maps to partition" is deliberately read-only. It reflects
+ * `partition_mapped_column ?? main_dttm_col`, and a peer dropdown would let it
+ * drift from the default datetime column silently. The only way to override it
+ * is from the target column's own row, where a transform has to be supplied
+ * alongside.
+ */
+export default function PartitionColumnFields({
+  datasource,
+  columns,
+  allColumns,
+  onPartitionColumnChange,
+  onNavigateToColumn,
+  previewMirrors,
+}: PartitionColumnFieldsProps) {
+  const theme = useTheme();
+
+  const options = useMemo(
+    () =>
+      columns.map(column => ({
+        value: column.column_name,
+        label: column.column_name,
+        customLabel: (
+          <Flex align="center" gap={theme.sizeUnit}>
+            <span>{column.column_name}</span>
+            {column.type && <Label>{column.type}</Label>}
+          </Flex>
+        ),
+      })),
+    [columns, theme.sizeUnit],
+  );
+
+  const mappedColumn = resolveMappedColumn(datasource);
+  const isImplicit = mappedColumnIsImplicit(datasource);
+  // The static checks are necessary, not sufficient: an unparseable expression
+  // or a function the database does not have needs the engine to spot, and the
+  // mapped column's row already asks it. Deferring to that answer where there
+  // is one is what keeps this banner and that panel from contradicting each
+  // other. A stored transform whose last probe failed says the same thing
+  // through the datasource's own summary, which is what an owner sees on
+  // reopening the editor.
+  const isActive =
+    mappingIsActive(datasource, allColumns) &&
+    previewMirrors !== false &&
+    datasource.partition_filter_mapping?.evaluable !== false;
+  const { partition_column: partitionColumn } = datasource;
+
+  // `field` is what the issues carry it for: a message about the partition
+  // column belongs under the partition column, not only in the Save button's
+  // tooltip, where an owner has to guess which of the two selects is at fault.
+  const issues = useMemo(
+    () => partitionMappingErrors(datasource, allColumns),
+    [datasource, allColumns],
+  );
+  const issueFor = (field: PartitionMappingIssue['field']) =>
+    issues.find(issue => issue.field === field)?.message;
+
+  // "Map a different column instead" normally opens the currently-mapped
+  // column's row, which is where the picker lives. When the mapped column *is*
+  // the partition column -- the self-mapping the backend rejects, and 
reachable
+  // in one click because the picker offers every column -- that row renders no
+  // mapping section at all, so the one guided way out of the broken state led
+  // nowhere. Offer a column that is not the partition column instead.
+  //
+  // Null only when there is no mapped column at all, which is also when the
+  // block below does not render -- but a dataset whose every column is the
+  // partition column would otherwise be offered a link to nothing, so the link
+  // is guarded rather than the type asserted away.
+  const differentColumnTarget =
+    mappedColumn && mappedColumn !== partitionColumn
+      ? mappedColumn
+      : (suggestedMappedColumn(columns, partitionColumn) ?? mappedColumn);
+
+  return (
+    <Flex vertical gap={theme.sizeUnit} data-test="partition-column-fields">
+      <Flex align="center" gap={theme.sizeUnit}>
+        <Typography.Text>{t('Partition column')}</Typography.Text>
+        <InfoTooltip
+          tooltip={t(
+            'The physical column the engine partitions on. Filters on the 
mapped column are mirrored onto it so the engine can prune partitions.',
+          )}
+        />
+      </Flex>
+      <Select
+        ariaLabel={t('Partition column')}
+        options={options}
+        value={partitionColumn ?? undefined}
+        onChange={value => onPartitionColumnChange((value as string) ?? null)}
+        onClear={() => onPartitionColumnChange(null)}
+        placeholder={t('None')}
+        allowClear
+        data-test="partition-column-select"
+      />
+      {issueFor('partition_column') && (
+        <Typography.Text type="danger" data-test="partition-column-error">
+          {issueFor('partition_column')}
+        </Typography.Text>
+      )}
+      <Typography.Text type="secondary">
+        {t(
+          "Column used for partition pruning on this table. Selecting one 
hides it from Explore's dimension and filter pickers by default.",
+        )}
+      </Typography.Text>
+
+      {partitionColumn && (
+        <Flex vertical gap={theme.sizeUnit} data-test="maps-to-partition">
+          <Flex align="center" gap={theme.sizeUnit}>
+            <Typography.Text type="secondary">
+              {t('Maps to partition')}
+            </Typography.Text>
+            <InfoTooltip
+              tooltip={t(
+                'The column whose filters are mirrored. It follows the default 
datetime column unless a different column holds the mapping.',
+              )}
+            />
+          </Flex>
+
+          {mappedColumn ? (
+            <>
+              <Flex align="center" gap={theme.sizeUnit}>
+                <Label>{mappedColumn}</Label>
+                {isImplicit && (
+                  <>
+                    <Typography.Text type="secondary">
+                      {t('Default datetime column')}
+                    </Typography.Text>
+                    <Tooltip
+                      title={t(
+                        'Set from the default datetime column above, so 
re-pointing that column moves the mapping with it.',
+                      )}
+                    >
+                      <Icons.LockOutlined
+                        iconSize="s"
+                        iconColor={theme.colorTextTertiary}
+                      />
+                    </Tooltip>
+                  </>
+                )}
+              </Flex>
+              {issueFor('partition_mapped_column') && (
+                <Typography.Text
+                  type="danger"
+                  data-test="partition-mapped-column-error"
+                >
+                  {issueFor('partition_mapped_column')}
+                </Typography.Text>
+              )}
+              <Typography.Text type="secondary">
+                {t(
+                  'Filters on this column are mirrored onto the partition 
column.',
+                )}
+                {differentColumnTarget && (
+                  <>
+                    {' '}
+                    <Button
+                      buttonStyle="link"
+                      onClick={() => onNavigateToColumn(differentColumnTarget)}
+                      data-test="map-a-different-column"
+                    >
+                      {t('Map a different column instead →')}
+                    </Button>
+                  </>
+                )}
+              </Typography.Text>
+              {isActive ? (
+                <Alert
+                  type="info"
+                  showIcon
+                  icon={<Icons.FilterOutlined />}
+                  message={
+                    <span>
+                      {t(
+                        'Filters on %(mapped)s will automatically apply an 
equivalent filter to %(partition)s.',
+                        { mapped: mappedColumn, partition: partitionColumn },
+                      )}{' '}
+                      <Button
+                        buttonStyle="link"
+                        onClick={() => onNavigateToColumn(mappedColumn)}
+                      >
+                        {t('Customize the value transform →')}
+                      </Button>
+                    </span>
+                  }
+                />
+              ) : (
+                <Alert
+                  type="warning"
+                  showIcon
+                  message={t(
+                    'No value transform is set on %(mapped)s, so nothing is 
mirrored yet and queries will scan every partition.',
+                    { mapped: mappedColumn },
+                  )}
+                />
+              )}
+            </>
+          ) : (
+            <>
+              <Flex align="center" gap={theme.sizeUnit}>
+                <Label
+                  css={css`
+                    color: ${theme.colorTextTertiary};
+                  `}
+                >
+                  {t('No mapping')}
+                </Label>
+                <Button
+                  buttonStyle="link"
+                  onClick={() => {
+                    const candidate = suggestedMappedColumn(
+                      columns,
+                      partitionColumn,
+                    );
+                    if (candidate) {
+                      onNavigateToColumn(candidate);
+                    }
+                  }}
+                  data-test="map-a-column"
+                >
+                  {t('Map a column →')}
+                </Button>
+              </Flex>
+              <Typography.Text type="secondary">
+                {t(
+                  'Normally the default datetime column, but none is set on 
this dataset.',
+                )}
+              </Typography.Text>
+              <Alert
+                type="warning"
+                showIcon
+                message={t(
+                  '%(partition)s is hidden from Explore, but no filter is 
mirrored onto it — queries will scan every partition until a column is mapped.',

Review Comment:
   Your suggested warning text was applied earlier. On Hugh's behalf, 
e61585070e also fixes the helper text under the selector, which still said that 
choosing a partition column hides it from Explore's pickers. It now says the 
column's Is filterable / Is dimension settings decide whether Explore offers 
it, and the existing test also asserts the old phrase is gone. Catalogs 
regenerated in d808efd1f0.



##########
superset/connectors/sqla/partition_mapping.py:
##########
@@ -0,0 +1,1314 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Partition filter mapping.
+
+Datasets on Hadoop-family engines are commonly partitioned on a *technical*
+column -- an epoch integer, a lowercased region key -- that no analyst would
+filter on. Unless a query carries a predicate on that column the engine scans
+every partition.
+
+A dataset owner names one partition column ``p``, one business column that
+filters are mirrored from, and a value transform ``T`` (a SQL expression
+containing a ``:value`` placeholder). Superset then appends an equivalent
+predicate on ``p`` to every query, so chart authors change nothing and queries
+prune.
+
+The load-bearing assumption
+---------------------------
+Everything here reasons about ``T(col) op T(v)``, but what is emitted is
+``p op T(v)`` -- a predicate on a *physically different column*. The step from
+one to the other is::
+
+    p = T(mapped_col)   for every row in the table
+
+Superset cannot verify that; it is a property of whatever ETL populates the
+partition column. If that job lags, backfills with different logic, or writes
+the partition key in a different timezone, mirrored predicates silently drop
+real rows. The mapping is only as trustworthy as the pipeline behind it.
+
+Operator safety
+---------------
+A mirrored predicate ``P2`` may only be ``AND``-ed onto a query when the
+original predicate ``P1`` *implies* it:
+
+===========================================  =============================
+Original                                     Safe when
+===========================================  =============================
+``col = v``, ``col IN (...)``                always -- ``T`` is a function
+``col >=|>|<|<= v``, ``TEMPORAL_RANGE``      only if ``T`` is monotonic
+``col != v``, ``NOT IN``, ``LIKE``, ...      never
+===========================================  =============================
+
+Negations are never safe because ``T`` need not be injective:
+``lower(:value)`` with ``country != 'US'`` mirrors to ``region_key != 'us'``,
+which wrongly excludes rows whose ``country`` is already lowercase ``'us'`` --
+rows the original filter *keeps*.
+
+Monotonicity is a property of the transform, not of the column's data type:
+``hour(:value)``, ``date_format(:value, 'dd')`` and ``dayofweek(:value)`` are
+all reasonable transforms on a ``TIMESTAMP`` column and none of them preserve
+ordering. It is therefore declared by the owner, not inferred.
+
+Time grains
+-----------
+A grained filter -- drill-to-detail, mostly -- compares the *truncated* column,
+``trunc(col) op v``, so the raw bounds it carries do not describe the rows it
+keeps. A row in the final partial bucket satisfies ``trunc(col) < until`` while
+``col < until`` excludes it.
+
+Which direction a grain rounds is not knowable from the duration alone, and the
+obvious guess is wrong: ``WEEK_ENDING_SATURDAY`` rounds *forward* on Hive and
+Presto, and Ocient's grains are ``ROUND``, i.e. to nearest. What every grain
+does satisfy is a bound on the displacement::
+
+    |trunc(ts) - ts| < width(grain)
+
+so widening *both* bounds by one bucket width is no narrower than the real
+predicate whichever way the grain rounds -- and it needs no monotonicity of
+``trunc`` itself, which is what rescues Hive's oddly-anchored ``P1W``. Width is
+a property of the grain, not of the engine, which is what makes this
+maintainable; see `grain_bucket_width`. A grain whose SQL an operator supplied
+has no known width, and does not mirror at all.
+"""
+
+from __future__ import annotations
+
+import hashlib
+import logging
+import re
+from dataclasses import dataclass
+from functools import lru_cache
+from typing import Any, cast, TYPE_CHECKING
+
+import numpy as np
+import pandas as pd
+import sqlalchemy as sa
+from dateutil.relativedelta import relativedelta
+from flask import current_app as app
+from flask_babel import lazy_gettext as _
+from sqlalchemy.engine.interfaces import Dialect
+from sqlalchemy.sql.elements import ColumnElement
+
+from superset.constants import LRU_CACHE_MAX_SIZE, TimeGrain
+from superset.exceptions import (
+    QueryClauseValidationException,
+    SupersetParseError,
+    SupersetSecurityException,
+)
+from superset.extensions import cache_manager, feature_flag_manager
+from superset.sql.parse import SQLStatement
+from superset.utils import json
+from superset.utils.core import FilterOperator
+
+if TYPE_CHECKING:
+    from superset.connectors.sqla.models import SqlaTable, TableColumn
+    from superset.models.core import Database
+
+logger = logging.getLogger(__name__)
+
+FEATURE_FLAG = "PARTITION_FILTER_MAPPING"
+
+#: Longest transform accepted anywhere. A transform is one SQL expression an
+#: owner types by hand, so this is generous; the point is that it is bounded.
+#: Every entry point enforces it -- the typed column field, the import schema
+#: and the preview request -- because `is_transform_active` parses the stored
+#: value on each Explore load, and an unbounded string would make that parse
+#: the expensive part of rendering a chart.
+MAX_TRANSFORM_LENGTH = 1024
+
+#: Placeholder the owner writes in the transform, e.g. 
``unix_timestamp(:value)``.
+#: Matched with word boundaries so ``:values`` is not mistaken for it.
+VALUE_PLACEHOLDER_RE = re.compile(r":value\b")
+
+#: Balanced Jinja blocks. The probe would render these in a different context
+#: at a different time from the chart query, so they are rejected at save time.
+JINJA_BLOCK_RE = re.compile(r"\{\{.*?\}\}|\{%.*?%\}|\{#.*?#\}", re.DOTALL)
+
+#: Substituted for ``:value`` before parsing -- sqlglot rejects a bare 
``:value``
+#: on most dialects. Mirrors the ``_JINJA_BLOCK_RE`` -> ``NULL`` trick used by
+#: ``validate_stored_expression``.
+_PARSE_STANDIN = "NULL"
+
+#: Functions whose value depends on wall-clock time or randomness. The probe
+#: runs in a different session at a different moment from the chart query and
+#: its result is then cached, so any of these freezes a snapshot of probe time
+#: into the emitted predicate.
+NON_DETERMINISTIC_FUNCTIONS = {
+    "CURRENT_DATE",
+    "CURRENT_TIME",
+    "CURRENT_TIMESTAMP",
+    "NOW",
+    "RAND",
+    "RANDOM",
+    "UUID",
+}
+
+#: Functions that mean "now" only in their zero-argument form. On Hive and
+#: Impala ``unix_timestamp()`` is the current time while ``unix_timestamp(x)``
+#: -- the canonical transform for this feature -- is pure.
+NON_DETERMINISTIC_WHEN_NILADIC = {"UNIX_TIMESTAMP"}
+
+#: Safe for any function ``T``.

Review Comment:
   Helping on Hugh's behalf. No code change in this pass, and I'm leaving this 
open for Hugh and you. Engine-level comparison semantics are covered by 
f3f49af437: `equality_mirrors_safely` withdraws `=`/`IN` on string columns 
unless the engine spec declares `binary_string_comparison`. The case you 
describe, a column with its own collation (`COLLATE NOCASE`) on an otherwise 
byte-exact engine, is still not detected, because neither SQLAlchemy reflection 
nor the engine specs expose per-column collation. It's documented as an owner 
assumption next to `p = T(mapped_col)`. Two ways to close it would be an 
explicit per-mapping opt-in for exact string equality, or reflecting collation 
where engines expose it. Both are product decisions I'd rather Hugh make than 
guess.



-- 
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]

Reply via email to