sadpandajoe commented on code in PR #43870: URL: https://github.com/apache/superset/pull/43870#discussion_r3939838584
########## superset-frontend/src/explore/components/PartitionPruningIndicator/index.tsx: ########## @@ -0,0 +1,80 @@ +/** + * 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 { css, useTheme } from '@apache-superset/core/theme'; +import { Icons, Tooltip } from '@superset-ui/core/components'; +import type { PartitionFilterMapping } from '@superset-ui/chart-controls'; + +interface PartitionPruningIndicatorProps { + /** The dataset's mapping summary, straight off the datasource payload. */ + mapping?: PartitionFilterMapping | null; +} + +/** + * The glyph on a filter whose column is mirrored onto a partition column + * (wireframe 1d). + * + * Chart authors do not configure any of this and ideally never learn the word + * "partition"; the indicator exists only to explain why their query got faster, + * and to point at the generated SQL where the extra predicate is visible. + */ +export default function PartitionPruningIndicator({ + mapping, +}: PartitionPruningIndicatorProps) { + const theme = useTheme(); + + if (!mapping?.active) { + return null; + } + + return ( + <Tooltip + placement="top" + title={t( + 'This filter is also applied to a partition column for faster queries. See "View query" for the generated SQL.', + )} + > + <span data-test="partition-pruning-indicator"> + <Icons.FilterOutlined + iconSize="s" + iconColor={theme.colorSuccess} + css={css` + margin-left: ${theme.sizeUnit}px; + vertical-align: middle; + `} + /> + </span> + </Tooltip> + ); +} + +/** + * Whether a filter on `columnName` is the one being mirrored. + * + * The mapping names exactly one mapped column, so anything else on the chart -- + * including other filters on the same dataset -- gets no indicator. + */ +export function isMirroredColumn( + mapping: PartitionFilterMapping | null | undefined, + columnName: string | null | undefined, +): boolean { + return Boolean( + mapping?.active && columnName && mapping.mapped_column === columnName, + ); Review Comment: This labels every active mapped-column filter as mirrored, but the query path also rejects unsupported operators (and range filters for non-monotonic transforms). For example, a `country != 'US'` chip gets the glyph even though no partition predicate is emitted. Could this consume an applicability contract that includes the operator and monotonicity? ########## superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterLabel.tsx: ########## @@ -406,7 +408,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}> + {popoverContent} + <PartitionPruningIndicator mapping={props.partitionMapping} /> Review Comment: Agreed—the standalone time control renders any active mapping even for `No filter` or a different temporal column, so the query has no matching partition predicate. Could this gate the glyph on an actual mapped time range? -- 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]
