aminghadersohi commented on code in PR #43870: URL: https://github.com/apache/superset/pull/43870#discussion_r4168251296
########## superset/migrations/versions/2026-09-23_00-01_a7f3c2e91d84_add_partition_filter_mapping.py: ########## @@ -0,0 +1,101 @@ +# 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. +"""add partition filter mapping + +Adds the four columns behind the ``PARTITION_FILTER_MAPPING`` feature: + +- ``tables.partition_column`` -- the physical column the engine partitions on +- ``tables.partition_mapped_column`` -- explicit override for the column whose + filters are mirrored; NULL means "follow ``main_dttm_col``" +- ``table_columns.partition_value_transform`` -- the ``:value`` expression +- ``table_columns.partition_transform_is_monotonic`` -- gates range operators + +The Continuum shadow tables get the same columns so dataset version history and +restore keep working. + +Revision ID: a7f3c2e91d84 +Revises: 95d8a99c822e +Create Date: 2026-08-31 22:30:00.000000 + +""" + +import sqlalchemy as sa + +from superset.migrations.shared.utils import add_columns, drop_columns + +# revision identifiers, used by Alembic. +revision = "a7f3c2e91d84" +down_revision = "95d8a99c822e" Review Comment: Master's #43232 added `884a2115ebd3` on the same parent, so the merge has two Alembic heads and `superset db upgrade` fails on every DB leg. Re-parent onto it (and the `Revises:` line at :31). ```suggestion down_revision = "884a2115ebd3" ``` ########## superset-frontend/src/components/Datasource/components/DatasourceEditor/components/PartitionFilterMapping/PartitionColumnFields.tsx: ########## @@ -0,0 +1,274 @@ +/** + * 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; + columns: PartitionMappingColumn[]; + onPartitionColumnChange: (columnName: string | null) => void; + /** Open the given column's row expand in the Columns table. */ + onNavigateToColumn: (columnName: string) => void; +} + +/** + * "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, + onPartitionColumnChange, + onNavigateToColumn, +}: 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); + const isActive = mappingIsActive(datasource, columns); + 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, columns), + [datasource, columns], + ); + 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. + 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.', + )}{' '} + <Button + buttonStyle="link" + onClick={() => onNavigateToColumn(differentColumnTarget)} Review Comment: `tsc` fails here (TS2345): `differentColumnTarget` is `string | null`. `mappedColumn` is narrowed in this branch, so the fallback is type-only. ```suggestion onClick={() => onNavigateToColumn(differentColumnTarget ?? mappedColumn) } ``` -- 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]
