gabotorresruiz commented on code in PR #44999:
URL: https://github.com/apache/superset/pull/44999#discussion_r4285687705
##########
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/utils.ts:
##########
@@ -94,12 +94,27 @@ export const shouldShowTimeRangePicker = (
currentDataset: (Dataset & { column_types: GenericDataType[] }) | undefined,
): boolean => (currentDataset ? hasTemporalColumns(currentDataset) : true);
-export const doesColumnMatchFilterType = (filterType: string, column: Column)
=>
- !column.type_generic ||
- !(filterType in FILTER_SUPPORTED_TYPES) ||
- FILTER_SUPPORTED_TYPES[
- filterType as keyof typeof FILTER_SUPPORTED_TYPES
- ]?.includes(column.type_generic);
+export const doesColumnMatchFilterType = (
+ filterType: string,
+ column: Column,
+ datasourceType?: DatasourceType,
+): boolean => {
+ // Semantic range filters skip the runtime non-numeric bounds check, so an
+ // unrecognised dimension type must not be offered for >= / <= comparisons.
+ if (
+ filterType === 'filter_range' &&
+ datasourceType === DatasourceType.SemanticView
+ ) {
+ return column.type_generic === GenericDataType.Numeric;
+ }
Review Comment:
Confirmed. Reverting the pattern to its `02606ff` form turns nine of the new
`semanticViewStructure` cases red, and `integer`, `bigint`, `smallint`,
`tinyint`, `numeric` and `real` now all come back as `NUMERIC`. Thanks. One
leftover spelling in a separate comment on the regex line.
##########
superset-frontend/src/filters/components/Range/RangeFilterPlugin.tsx:
##########
@@ -331,11 +355,32 @@ export default function RangeFilterPlugin(props:
PluginFilterRangeProps) {
},
});
},
- [setDataMask],
+ [setDataMask, col, enableSingleExactValue],
);
useEffect(() => {
- if (row?.min === undefined && row?.max === undefined) {
+ if (nonNumericColumn) {
+ return;
+ }
+
+ if (manualBounds) {
+ // Invalid edits keep their draft in the inputs; external saved values
+ // and Clear All are validated without requiring a discovered domain.
+ const value: RangeValue =
+ filterState.validateStatus === 'error'
+ ? inputValue
+ : (filterState.value ?? [null, null]);
+ setInputValue(value);
+ const { isValid, errorMessage } = validateRange(
+ value,
+ undefined,
+ undefined,
+ enableEmptyFilter,
+ enableSingleValue,
+ );
Review Comment:
Confirmed. Putting the empty value checks back below the single value block
turns the three `optional semantic single-value ... accepts Clear All` cases
red, and the required variants still reject the clear. Closed from my side.
##########
superset-frontend/src/filters/components/Range/RangeFilterPlugin.tsx:
##########
@@ -262,8 +280,9 @@ export default function RangeFilterPlugin(props:
PluginFilterRangeProps) {
formData;
// Get the display mode from formData
- const rangeDisplayMode =
- formData?.rangeDisplayMode || RangeDisplayMode.SliderAndInput;
+ const rangeDisplayMode = manualBounds
+ ? RangeDisplayMode.Input
+ : formData?.rangeDisplayMode || RangeDisplayMode.SliderAndInput;
Review Comment:
Confirmed. Reverting `rangeDisplayMode` to the `manualBounds` only form
turns both `saved Slider mode falls back to usable inputs ...` cases and
`standalone SQL range keeps its empty-result inputs usable` red. I also saw it
in a browser against a real all `NULL` `DOUBLE PRECISION` column: usable
inputs, no slider, no error.
##########
superset-frontend/src/filters/components/Range/isSemanticRange.ts:
##########
@@ -0,0 +1,32 @@
+/**
+ * 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 { DatasourceKey, DatasourceType } from '@superset-ui/core';
+
+/** Identify numeric filters whose source cannot provide aggregate bounds. */
+export default function isSemanticRange(formData: {
+ datasource?: string;
+ viz_type?: string;
+ vizType?: string;
+}): boolean {
+ return (
+ (formData.viz_type ?? formData.vizType) === 'filter_range' &&
Review Comment:
Confirmed. Dropping the `filter_range` half of the predicate turns `semantic
select still requests chart data for its options` red, so the guard is pinned
now. Thanks.
--
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]
