diff --git a/docs/package.json b/docs/package.json index 33e2e132cfdf..8a5f32d732f6 100644 --- a/docs/package.json +++ b/docs/package.json @@ -64,8 +64,8 @@ "antd": "^6.6.0", "baseline-browser-mapping": "^2.11.13", "caniuse-lite": "^1.0.30001809", - "docusaurus-plugin-openapi-docs": "^5.1.3", - "docusaurus-theme-openapi-docs": "^5.1.3", + "docusaurus-plugin-openapi-docs": "^5.2.0", + "docusaurus-theme-openapi-docs": "^5.2.0", "js-yaml": "^5.2.3", "json-bigint": "^1.0.0", "prism-react-renderer": "^2.4.1", @@ -78,7 +78,7 @@ "remark-import-partial": "^0.0.2", "reselect": "^5.2.0", "storybook": "^10.5.7", - "swagger-ui-react": "^5.32.12", + "swagger-ui-react": "^5.32.13", "swc-loader": "^0.2.7", "tinycolor2": "^1.4.2", "unist-util-visit": "^5.1.0" @@ -93,7 +93,7 @@ "@typescript-eslint/parser": "^8.67.0", "eslint": "^9.39.2", "eslint-plugin-react": "^7.37.5", - "globals": "^17.9.0", + "globals": "^17.10.0", "oxfmt": "^0.63.0", "typescript": "~6.0.3", "typescript-eslint": "^8.67.0", diff --git a/docs/yarn.lock b/docs/yarn.lock index 4808dbd721a7..d61a22c4410b 100644 --- a/docs/yarn.lock +++ b/docs/yarn.lock @@ -8014,10 +8014,10 @@ doctrine@^2.1.0: dependencies: esutils "^2.0.2" -docusaurus-plugin-openapi-docs@^5.1.3: - version "5.1.3" - resolved "https://registry.yarnpkg.com/docusaurus-plugin-openapi-docs/-/docusaurus-plugin-openapi-docs-5.1.3.tgz#b8cd5f8451aaf881deb1a744a8295685f1681865" - integrity sha512-HnpblSBdXoR39VNTIW9zWERUsMJxXOpvdQoBKyaTkUBPwCM48Z76+ndo2yO2vADq+EhWjJlfxL1DUzCrgNjThQ== +docusaurus-plugin-openapi-docs@^5.2.0: + version "5.2.0" + resolved "https://registry.yarnpkg.com/docusaurus-plugin-openapi-docs/-/docusaurus-plugin-openapi-docs-5.2.0.tgz#8318ec90cd21fed023be57696211af7d72fd81db" + integrity sha512-MjrfRAMB64uvdxRVz6L9AXWe4QFjCdoBAzYs306yyI3nnXHsFj2lv2FnLA90JV9CAUZaGiYMvvkzBo2Nrkq/9w== dependencies: "@apidevtools/json-schema-ref-parser" "^15.3.3" "@redocly/openapi-core" "^2.25.2" @@ -8035,10 +8035,10 @@ docusaurus-plugin-openapi-docs@^5.1.3: swagger2openapi "^7.0.8" xml-formatter "^3.6.6" -docusaurus-theme-openapi-docs@^5.1.3: - version "5.1.3" - resolved "https://registry.yarnpkg.com/docusaurus-theme-openapi-docs/-/docusaurus-theme-openapi-docs-5.1.3.tgz#e23644a63785352abbc76e42760c0dfdff3669e1" - integrity sha512-npbD1QahtjAEmrOet/86i5fTmcJX4/rPhVT+c0qKjm7StUNbyqjwchSVBQuU1rB69T51JOA9TpT/y6QcB9Xjvw== +docusaurus-theme-openapi-docs@^5.2.0: + version "5.2.0" + resolved "https://registry.yarnpkg.com/docusaurus-theme-openapi-docs/-/docusaurus-theme-openapi-docs-5.2.0.tgz#6d93a74e2e3cf0ae77d24e1c4144bd2e74a52115" + integrity sha512-L0b80LzaMUfr76a9EQXRPCf8nxkEz8Xo6Aknnke1UeE2oXsgoiVki6U+RTE7GmJRjO8zSNKXyckGmGmqqWuHeA== dependencies: "@hookform/error-message" "^2.0.1" "@reduxjs/toolkit" "^2.8.2" @@ -8123,7 +8123,7 @@ domhandler@^5.0.2, domhandler@^5.0.3: dependencies: domelementtype "^2.3.0" -dompurify@^3.3.3, dompurify@^3.4.12: +dompurify@^3.3.3, dompurify@^3.4.13: version "3.4.13" resolved "https://registry.yarnpkg.com/dompurify/-/dompurify-3.4.13.tgz#fc28949d59f92d62e28a3a764bcbeee35897a1be" integrity sha512-2vmYIoqjze2d+kakP8S/nS5shfsl587kzwEjcGlTdiksUVgFHnFCsLYDVj/JNqJVOQZGSYBTmuycv0PodwmnMQ== @@ -9174,10 +9174,10 @@ globals@^14.0.0: resolved "https://registry.yarnpkg.com/globals/-/globals-14.0.0.tgz#898d7413c29babcf6bafe56fcadded858ada724e" integrity sha512-oahGvuMGQlPw/ivIYBjVSrWAfWLBeku5tpPE2fOPLi+WHffIWbuh2tCjhyQhTBPMf5E9jDEH4FOmTYgYwbKwtQ== -globals@^17.9.0: - version "17.9.0" - resolved "https://registry.yarnpkg.com/globals/-/globals-17.9.0.tgz#e43f252d6bbe71508da43902a1709c8895a59f70" - integrity sha512-m/MvAW61QVU5VDNF1Vj8axt016h8w7L5TU1e9zlab7XIttAT2YAlCwl75K1fOqvMM9apmD7lbCIRhpfkhmxhCg== +globals@^17.10.0: + version "17.10.0" + resolved "https://registry.yarnpkg.com/globals/-/globals-17.10.0.tgz#f9dbd847ae99e236f98b13095e2426ac3b25a45c" + integrity sha512-V0kztuWST2k8A/VbxAY8+L+7+Rgo3fyA24IHRLrZp7HOzJjV0gHSaZUjK9lpP/IrBSNite2tZ1prhRkinRu1CA== globalthis@^1.0.4: version "1.0.4" @@ -10284,10 +10284,10 @@ js-levenshtein@^1.1.6: resolved "https://registry.yarnpkg.com/js-tokens/-/js-tokens-4.0.0.tgz#19203fb59991df98e3a287050d4647cdeaf32499" integrity sha512-RdJUflcE3cUzKiMqQgsCu06FPu9UdIJO0beYbPhHN4k6apgJtifcoCtT9bcxOpYBtpD2kCM6Sbzg4CausW/PKQ== -js-yaml@4.1.0, js-yaml@=4.3.0, js-yaml@^4.1.0, js-yaml@^4.1.1, js-yaml@^4.2.0, js-yaml@^4.3.0: - version "4.3.0" - resolved "https://registry.yarnpkg.com/js-yaml/-/js-yaml-4.3.0.tgz#d1900572a7f7cf0b5f540c83673e60bad3436592" - integrity sha512-1td788aAnnZ5qs7V2QIRl1owjtYpbKt749Y3xauqQgwIIGF/xXWz1wMTEBx5O3LK3lXLVuqXPdPxj2BoFHaW9Q== +js-yaml@4.1.0, js-yaml@=4.3.1, js-yaml@^4.1.0, js-yaml@^4.1.1, js-yaml@^4.2.0, js-yaml@^4.3.0: + version "4.3.1" + resolved "https://registry.yarnpkg.com/js-yaml/-/js-yaml-4.3.1.tgz#01216c001d67f48e2cd560d708c7af21090a3848" + integrity sha512-CY6crGq313MX8GkwvB7tzgp99vjQxY1++5y10/BKN/GUfHqWaOGQMNZkBvqSzsZKWk/ijwHlWzzkLulsGHhjWQ== dependencies: argparse "^2.0.1" @@ -15103,10 +15103,10 @@ swagger-client@^3.37.8: "@swagger-api/apidom-parser-adapter-openapi-yaml-3-2" "^1.12.0" "@swagger-api/apidom-parser-adapter-yaml-1-2" "^1.12.0" -swagger-ui-react@^5.32.12: - version "5.32.12" - resolved "https://registry.yarnpkg.com/swagger-ui-react/-/swagger-ui-react-5.32.12.tgz#47525a26774eb02db0e6203af72f5b32fa6205cc" - integrity sha512-WCdkNOQyMTZDu+z356FpwVWHf1dwZgQPUjdQPh1L4r7jULaJTKKlIItXq6WsZdYeXvsHndMdxxccEQXOAroUHQ== +swagger-ui-react@^5.32.13: + version "5.32.13" + resolved "https://registry.yarnpkg.com/swagger-ui-react/-/swagger-ui-react-5.32.13.tgz#04c96140b0a2d4ea01ebec4d4cfc655d5ed9a500" + integrity sha512-XIDl+Ny6kE1N8wpSPiOFrjPfAevs4GR4XmV6BT6NLMikkMFIbIVocWbA8pnKYyYXQe8Rccfli5o2zDfySw0FnQ== dependencies: "@babel/runtime-corejs3" "^7.27.1" "@scarf/scarf" "=1.4.0" @@ -15115,11 +15115,11 @@ swagger-ui-react@^5.32.12: classnames "^2.5.1" css.escape "1.5.1" deep-extend "0.6.0" - dompurify "^3.4.12" + dompurify "^3.4.13" ieee754 "^1.2.1" immutable "^4.3.9" js-file-download "^0.4.12" - js-yaml "=4.3.0" + js-yaml "=4.3.1" lodash "^4.18.1" prop-types "^15.8.1" randexp "^0.5.3" diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index 2638131cc5fd..6b48d65a953a 100644 --- a/superset-frontend/package-lock.json +++ b/superset-frontend/package-lock.json @@ -186,7 +186,7 @@ "@storybook/test-runner": "0.24.4", "@svgr/webpack": "^8.1.0", "@swc/core": "^1.15.47", - "@swc/plugin-emotion": "^14.15.0", + "@swc/plugin-emotion": "^14.19.0", "@swc/plugin-transform-imports": "^12.5.0", "@testing-library/dom": "^10.4.1", "@testing-library/jest-dom": "^7.0.1", @@ -11808,9 +11808,9 @@ } }, "node_modules/@swc/plugin-emotion": { - "version": "14.15.0", - "resolved": "https://registry.npmjs.org/@swc/plugin-emotion/-/plugin-emotion-14.15.0.tgz", - "integrity": "sha512-nCsTO7mOOPz2UnT3N6YWb014uI0CVxeKg53A/KM/CvuSIE6H3KPkhaziJQ3q2jI3u3LfFuDKEnU5ZmB1330Dqg==", + "version": "14.19.0", + "resolved": "https://registry.npmjs.org/@swc/plugin-emotion/-/plugin-emotion-14.19.0.tgz", + "integrity": "sha512-0/q84ro0a7kdjpYpn9Wmi5/RLHYuSwYjO638lE5ZBQfIvYpSLJxbEgLsObCmdH4KPe2stoN8plVKUpCsKPggaw==", "dev": true, "license": "Apache-2.0", "dependencies": { @@ -43073,6 +43073,15 @@ "node": ">=12" } }, + "packages/superset-ui-core/node_modules/dompurify": { + "version": "3.4.13", + "resolved": "https://registry.npmjs.org/dompurify/-/dompurify-3.4.13.tgz", + "integrity": "sha512-2vmYIoqjze2d+kakP8S/nS5shfsl587kzwEjcGlTdiksUVgFHnFCsLYDVj/JNqJVOQZGSYBTmuycv0PodwmnMQ==", + "license": "(MPL-2.0 OR Apache-2.0)", + "optionalDependencies": { + "@types/trusted-types": "^2.0.7" + } + }, "packages/superset-ui-core/node_modules/react-ace": { "version": "14.0.1", "resolved": "https://registry.npmjs.org/react-ace/-/react-ace-14.0.1.tgz", diff --git a/superset-frontend/package.json b/superset-frontend/package.json index 737bad5a7022..26e253ec6587 100644 --- a/superset-frontend/package.json +++ b/superset-frontend/package.json @@ -263,7 +263,7 @@ "@storybook/test-runner": "0.24.4", "@svgr/webpack": "^8.1.0", "@swc/core": "^1.15.47", - "@swc/plugin-emotion": "^14.15.0", + "@swc/plugin-emotion": "^14.19.0", "@swc/plugin-transform-imports": "^12.5.0", "@testing-library/dom": "^10.4.1", "@testing-library/jest-dom": "^7.0.1", diff --git a/superset-frontend/packages/superset-core/src/common/index.ts b/superset-frontend/packages/superset-core/src/common/index.ts index 4519a1e154b5..b641a1c7a1e9 100644 --- a/superset-frontend/packages/superset-core/src/common/index.ts +++ b/superset-frontend/packages/superset-core/src/common/index.ts @@ -130,6 +130,7 @@ export enum GenericDataType { String = 1, Temporal = 2, Boolean = 3, + MultiValue = 4, } /** diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnTypeLabel/ColumnTypeLabel.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnTypeLabel/ColumnTypeLabel.tsx index 8ea43bdc8a68..0316ca257629 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnTypeLabel/ColumnTypeLabel.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnTypeLabel/ColumnTypeLabel.tsx @@ -28,6 +28,7 @@ import { FieldBinaryOutlined, FieldStringOutlined, NumberOutlined, + UnorderedListOutlined, } from '@ant-design/icons'; import { Icons } from '@superset-ui/core/components'; @@ -72,6 +73,10 @@ export function ColumnTypeLabel({ type }: ColumnTypeLabelProps) { typeIcon = ; } else if (type === GenericDataType.Temporal) { typeIcon = ; + } else if (type === GenericDataType.MultiValue) { + typeIcon = ( + + ); } return {typeIcon}; diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/components/ColumnTypeLabel.test.tsx b/superset-frontend/packages/superset-ui-chart-controls/test/components/ColumnTypeLabel.test.tsx index fc65cd26f86f..46fc74b49367 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/test/components/ColumnTypeLabel.test.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/test/components/ColumnTypeLabel.test.tsx @@ -64,4 +64,21 @@ describe('ColumnOption', () => { renderColumnTypeLabel({ type: GenericDataType.Temporal }); expect(screen.getByLabelText('temporal type icon')).toBeVisible(); }); + test('multi-value (array) type shows list icon', () => { + renderColumnTypeLabel({ type: GenericDataType.MultiValue }); + expect(screen.getByLabelText('multi-value type icon')).toBeVisible(); + }); +}); + +describe('GenericDataType enum parity', () => { + // These numeric values are shared with the backend enum in + // superset/utils/core.py (GenericDataType). They must stay in sync because + // the backend serializes columns using these integers. + test('values match the backend contract', () => { + expect(GenericDataType.Numeric).toBe(0); + expect(GenericDataType.String).toBe(1); + expect(GenericDataType.Temporal).toBe(2); + expect(GenericDataType.Boolean).toBe(3); + expect(GenericDataType.MultiValue).toBe(4); + }); }); diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Gantt/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Gantt/transformProps.ts index 1a9b4d565a12..5bd205a29d79 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Gantt/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Gantt/transformProps.ts @@ -390,6 +390,7 @@ export default function transformProps(chartProps: EchartsGanttChartProps) { [GenericDataType.String]: undefined, [GenericDataType.Temporal]: tooltipTimeFormatter, [GenericDataType.Boolean]: undefined, + [GenericDataType.MultiValue]: undefined, }; const echartOptions: EChartsCoreOption = { diff --git a/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx b/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx index 31cd26cc3101..738ae2214bbb 100644 --- a/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx +++ b/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx @@ -1627,9 +1627,7 @@ function DatasourceEditor({ {t( 'Default URL to redirect to when accessing from the dataset list page. Accepts relative URLs such as', )}{' '} - - /superset/dashboard/{'{id}'}/ - + /dashboard/{'{id}'}/ } control={} diff --git a/superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditor.test.tsx b/superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditor.test.tsx index 64b9d42b8f1b..74ae681285c8 100644 --- a/superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditor.test.tsx +++ b/superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditor.test.tsx @@ -71,6 +71,17 @@ test('renders Tabs', async () => { expect(screen.getByTestId('edit-dataset-tabs')).toBeInTheDocument(); }); +test('recommends a registered client route for the default URL', async () => { + await asyncRender(createProps()); + + userEvent.click(screen.getByRole('tab', { name: 'Settings' })); + + expect(await screen.findByText('/dashboard/{id}/')).toBeInTheDocument(); + expect( + screen.queryByText('/superset/dashboard/{id}/'), + ).not.toBeInTheDocument(); +}); + test('can sync columns from source', async () => { const testProps = createProps(); await asyncRender({ diff --git a/superset-frontend/src/explore/components/controls/ColumnConfigControl/constants.tsx b/superset-frontend/src/explore/components/controls/ColumnConfigControl/constants.tsx index 6822021fd400..71f860b6c06e 100644 --- a/superset-frontend/src/explore/components/controls/ColumnConfigControl/constants.tsx +++ b/superset-frontend/src/explore/components/controls/ColumnConfigControl/constants.tsx @@ -251,4 +251,11 @@ export const DEFAULT_CONFIG_FORM_LAYOUT: ColumnConfigFormLayout = { { name: 'horizontalAlign', override: { defaultValue: 'left' } }, ], ], + [GenericDataType.MultiValue]: [ + [ + 'columnWidth', + { name: 'horizontalAlign', override: { defaultValue: 'left' } }, + ], + ['truncateLongCells'], + ], }; diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopover/index.tsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopover/index.tsx index b247dc72004d..464418814569 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopover/index.tsx +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopover/index.tsx @@ -367,8 +367,22 @@ function AdhocFilterEditPopover({ ), }, - ...(datasource?.type === 'semantic_view' - ? [] + ...(datasource?.type === 'semantic_view' || + [ + Operators.ContainsAny, + Operators.ContainsAll, + Operators.IsEmpty, + Operators.IsNotEmpty, + Operators.LengthEquals, + Operators.LengthGreaterThan, + Operators.LengthLessThan, + Operators.LengthGreaterThanOrEqual, + Operators.LengthLessThanOrEqual, + ].includes(adhocFilter.operatorId as Operators) + ? // Hide the Custom SQL tab for element-level array operators: they + // have no portable SQL representation, and converting one would + // silently turn the filter into invalid raw SQL. + [] : [ { key: ExpressionTypes.Sql, diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx index ee54da8a5f8e..b86c8f526e04 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx @@ -35,6 +35,7 @@ import { } from 'src/explore/constants'; import AdhocMetric from 'src/explore/components/controls/MetricControl/AdhocMetric'; import { FeatureFlag, isFeatureEnabled } from '@superset-ui/core'; +import { GenericDataType } from '@apache-superset/core/common'; import fetchMock from 'fetch-mock'; import { TestDataset, Dataset } from '@superset-ui/chart-controls'; @@ -252,6 +253,78 @@ test('shows boolean only operators when subject is number', () => { ].map(operator => expect(isOperatorRelevant(operator, 'value')).toBe(true)); }); +test('shows array operators (tier 1 + tier 2) when subject is multi-value', () => { + const props = setup({ + adhocFilter: new AdhocFilter({ + expressionType: ExpressionTypes.Simple, + subject: 'skills', + operatorId: undefined, + operator: undefined, + comparator: undefined, + clause: undefined, + }), + datasource: { + columns: [ + { + id: 3, + column_name: 'skills', + type: 'Array(String)', + type_generic: GenericDataType.MultiValue, + }, + ], + }, + }); + const { isOperatorRelevant } = useSimpleTabFilterProps( + props as unknown as Props, + ); + // Tier 1 (whole-array) + Tier 2 (element-level) are all relevant. + [ + Operators.Equals, + Operators.NotEquals, + Operators.In, + Operators.NotIn, + Operators.IsNull, + Operators.IsNotNull, + Operators.ContainsAny, + Operators.ContainsAll, + Operators.IsEmpty, + Operators.IsNotEmpty, + ].forEach(operator => + expect(isOperatorRelevant(operator, 'skills')).toBe(true), + ); + // scalar-only operators are hidden for array columns + [Operators.GreaterThan, Operators.LessThan, Operators.Like].forEach( + operator => expect(isOperatorRelevant(operator, 'skills')).toBe(false), + ); +}); + +test('hides element-level array operators for non multi-value columns', () => { + const props = setup({ + adhocFilter: new AdhocFilter({ + expressionType: ExpressionTypes.Simple, + subject: 'value', + operatorId: undefined, + operator: undefined, + comparator: undefined, + clause: undefined, + }), + datasource: { + columns: [{ id: 3, column_name: 'value', type: 'STRING' }], + }, + }); + const { isOperatorRelevant } = useSimpleTabFilterProps( + props as unknown as Props, + ); + [ + Operators.ContainsAny, + Operators.ContainsAll, + Operators.IsEmpty, + Operators.IsNotEmpty, + ].forEach(operator => + expect(isOperatorRelevant(operator, 'value')).toBe(false), + ); +}); + test('will convert from individual comparator to array if the operator changes to multi', () => { const props = setup(); const { onOperatorChange } = useSimpleTabFilterProps( @@ -309,6 +382,49 @@ test('will convert from array to individual comparators if the operator changes ); }); +test('resets the comparator when switching between array value families', () => { + // Equal to (whole-array literal) -> Contains all (individual elements): + // the value spaces are incompatible, so the stale value must be cleared. + const wholeArrayFilter = new AdhocFilter({ + expressionType: ExpressionTypes.Simple, + subject: 'scores', + operatorId: Operators.Equals, + operator: OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.Equals].operation, + comparator: '[5,6,7]', + clause: Clauses.Where, + }); + const props = setup({ adhocFilter: wholeArrayFilter }); + const { onOperatorChange } = useSimpleTabFilterProps( + props as unknown as Props, + ); + onOperatorChange(Operators.ContainsAll); + const lastCall = + props.onChange.mock.calls[props.onChange.mock.calls.length - 1][0]; + expect(lastCall.operatorId).toEqual(Operators.ContainsAll); + expect(lastCall.comparator).toBeUndefined(); +}); + +test('keeps the value when switching within the element family', () => { + // Contains any <-> Contains all both take individual elements, so the + // selected elements should carry over. + const elementFilter = new AdhocFilter({ + expressionType: ExpressionTypes.Simple, + subject: 'scores', + operatorId: Operators.ContainsAny, + operator: OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.ContainsAny].operation, + comparator: ['5', '6'], + clause: Clauses.Where, + }); + const props = setup({ adhocFilter: elementFilter }); + const { onOperatorChange } = useSimpleTabFilterProps( + props as unknown as Props, + ); + onOperatorChange(Operators.ContainsAll); + const lastCall = + props.onChange.mock.calls[props.onChange.mock.calls.length - 1][0]; + expect(lastCall.comparator).toEqual(['5', '6']); +}); + test('passes the new adhocFilter to onChange after onComparatorChange', () => { const props = setup(); const { onComparatorChange } = useSimpleTabFilterProps( diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx index a976fd82193a..4e0e7eb21456 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx @@ -32,6 +32,7 @@ import { isDefined, SupersetClient, } from '@superset-ui/core'; +import { GenericDataType } from '@apache-superset/core/common'; import { styled, useTheme, css } from '@apache-superset/core/theme'; import { Operators, @@ -118,6 +119,8 @@ export const useSimpleTabFilterProps = (props: Props) => { const isColumnNumber = !!column && (column.type === 'INT' || column.type === 'INTEGER'); const isColumnFunction = !!column && !!column.expression; + const isColumnMultiValue = + !!column && column.type_generic === GenericDataType.MultiValue; if (operator && operator === Operators.LatestPartition) { const { partitionColumn } = props; @@ -127,6 +130,35 @@ export const useSimpleTabFilterProps = (props: Props) => { // hide the TEMPORAL_RANGE operator return false; } + // Element-level array operators only apply to multi-value columns. + const arrayElementOperators = [ + Operators.ContainsAny, + Operators.ContainsAll, + Operators.IsEmpty, + Operators.IsNotEmpty, + Operators.LengthEquals, + Operators.LengthGreaterThan, + Operators.LengthLessThan, + Operators.LengthGreaterThanOrEqual, + Operators.LengthLessThanOrEqual, + ]; + if (arrayElementOperators.includes(operator)) { + return isColumnMultiValue; + } + if (isColumnMultiValue) { + // Array columns support whole-array operators (=, !=, In, Not in, null + // checks) plus the element-level operators above. Scalar-only operators + // (Like, <, >, <=, >=) are hidden because they aren't valid on an array. + return [ + Operators.Equals, + Operators.NotEquals, + Operators.In, + Operators.NotIn, + Operators.IsNull, + Operators.IsNotNull, + ...arrayElementOperators, + ].includes(operator); + } if (operator === Operators.IsTrue || operator === Operators.IsFalse) { return isColumnBoolean || isColumnNumber || isColumnFunction; } @@ -167,9 +199,19 @@ export const useSimpleTabFilterProps = (props: Props) => { ].operation : null; if (!isDefined(operator)) { - // if operator is `null`, use the `IN` and reset the comparator. - operator = Operators.In; - operatorId = Operators.In; + // The previous operator is not relevant for the new subject; pick a + // sensible default and reset the comparator. Multi-value (array) columns + // default to "Contains any" (element membership) rather than the + // scalar-only IN. + const newColumn = props.datasource.columns?.find( + col => col.column_name === subject, + ); + const defaultOperator = + newColumn?.type_generic === GenericDataType.MultiValue + ? Operators.ContainsAny + : Operators.In; + operator = defaultOperator; + operatorId = defaultOperator; comparator = undefined; } @@ -193,10 +235,38 @@ export const useSimpleTabFilterProps = (props: Props) => { }; const onOperatorChange = (operatorId: Operators) => { const currentComparator = props.adhocFilter.comparator; + // The value space differs between operator families: element-level array + // ops (Contains any/all) take individual elements, whole-array/scalar ops + // (=, In, …) take whole arrays or scalars, Length ops take a count, and the + // unary ops take nothing. A value from one family is meaningless in another, + // so reset the value when the family changes (e.g. Equal to -> Contains all). + const comparatorKind = (op?: Operators): string => { + if (!op) return 'none'; + if (op === Operators.ContainsAny || op === Operators.ContainsAll) { + return 'element'; + } + if ( + op === Operators.LengthEquals || + op === Operators.LengthGreaterThan || + op === Operators.LengthLessThan || + op === Operators.LengthGreaterThanOrEqual || + op === Operators.LengthLessThanOrEqual + ) { + return 'length'; + } + if (DISABLE_INPUT_OPERATORS.includes(op)) return 'none'; + return 'value'; + }; + const valueFamilyChanged = + comparatorKind(props.adhocFilter.operatorId as Operators | undefined) !== + comparatorKind(operatorId); + let newComparator; - // convert between list of comparators and individual comparators - // (e.g. `in ('North America', 'Africa')` to `== 'North America'`) - if (MULTI_OPERATORS.has(operatorId)) { + if (valueFamilyChanged) { + newComparator = undefined; + } else if (MULTI_OPERATORS.has(operatorId)) { + // convert between list of comparators and individual comparators + // (e.g. `in ('North America', 'Africa')` to `== 'North America'`) newComparator = Array.isArray(currentComparator) ? currentComparator : [currentComparator].filter(element => element != null); @@ -433,19 +503,42 @@ const AdhocFilterEditPopoverSimpleTabContent: FC = props => { if (loadingComparatorSuggestions) { controller.abort(); } + // Element-level array operators (Contains any / Contains all) search + // inside the array, so suggest individual elements; whole-array + // operators (=, In, …) keep the default distinct-array suggestions. + const { operatorId } = props.adhocFilter; + const arrayElements = + operatorId === Operators.ContainsAny || + operatorId === Operators.ContainsAll; setLoadingComparatorSuggestions(true); SupersetClient.get({ signal, - endpoint: `/api/v1/datasource/${datasource.type}/${datasource.id}/column/${col}/values/`, + endpoint: `/api/v1/datasource/${datasource.type}/${datasource.id}/column/${col}/values/${ + arrayElements ? '?array_elements=true' : '' + }`, }) .then(({ json }) => { setSuggestions( - json.result.map( - (suggestion: null | number | boolean | string) => ({ - value: suggestion, - label: optionLabel(suggestion), - }), - ), + json.result.map((suggestion: unknown) => { + // Complex column values arrive as JS arrays or objects: whole + // arrays for MULTI_VALUE columns (e.g. [5, 6, 7]) and Map/Tuple + // objects for nested-container columns (e.g. {"a": ["x","y"]}). + // A raw array/object is neither a valid single-select value + // (antd collapses an array to its first element) nor renderable + // as a React child (an object throws). Render it as its literal + // string, which is also exactly what the backend's + // parse_array_literal expects for the whole-array operators. + if (suggestion !== null && typeof suggestion === 'object') { + const literal = JSON.stringify(suggestion); + return { value: literal, label: literal }; + } + return { + value: suggestion as null | number | boolean | string, + label: optionLabel( + suggestion as null | number | boolean | string, + ), + }; + }), ); setLoadingComparatorSuggestions(false); }) @@ -464,6 +557,7 @@ const AdhocFilterEditPopoverSimpleTabContent: FC = props => { }, [ props.adhocFilter.subject, props.adhocFilter.clause, + props.adhocFilter.operatorId, props.datasource, datePicker, ]); diff --git a/superset-frontend/src/explore/components/controls/FilterControl/utils/translateToSQL.ts b/superset-frontend/src/explore/components/controls/FilterControl/utils/translateToSQL.ts index 171ddbc6f528..df42774badae 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/utils/translateToSQL.ts +++ b/superset-frontend/src/explore/components/controls/FilterControl/utils/translateToSQL.ts @@ -44,6 +44,17 @@ export const OPERATORS_TO_SQL = { 'IS NULL': 'IS NULL', 'IS TRUE': 'IS TRUE', 'IS FALSE': 'IS FALSE', + // Element-level array operators (shown as filter labels; not executable SQL — + // the Custom SQL tab is hidden for these). + CONTAINS_ANY: 'CONTAINS ANY', + CONTAINS_ALL: 'CONTAINS ALL', + IS_EMPTY: 'IS EMPTY', + IS_NOT_EMPTY: 'IS NOT EMPTY', + LENGTH_EQUALS: 'LENGTH =', + LENGTH_GREATER_THAN: 'LENGTH >', + LENGTH_LESS_THAN: 'LENGTH <', + LENGTH_GREATER_THAN_OR_EQUALS: 'LENGTH >=', + LENGTH_LESS_THAN_OR_EQUALS: 'LENGTH <=', 'LATEST PARTITION': ({ datasource, }: { diff --git a/superset-frontend/src/explore/constants.ts b/superset-frontend/src/explore/constants.ts index b1c03fd98072..929b88a72098 100644 --- a/superset-frontend/src/explore/constants.ts +++ b/superset-frontend/src/explore/constants.ts @@ -45,6 +45,17 @@ export enum Operators { IsTrue = 'IS_TRUE', IsFalse = 'IS_FALSE', TemporalRange = 'TEMPORAL_RANGE', + // Element-level operators for multi-value (array) columns + ContainsAny = 'CONTAINS_ANY', + ContainsAll = 'CONTAINS_ALL', + IsEmpty = 'IS_EMPTY', + IsNotEmpty = 'IS_NOT_EMPTY', + // Length (element-count) comparison operators for array columns + LengthEquals = 'LENGTH_EQUALS', + LengthGreaterThan = 'LENGTH_GREATER_THAN', + LengthLessThan = 'LENGTH_LESS_THAN', + LengthGreaterThanOrEqual = 'LENGTH_GREATER_THAN_OR_EQUALS', + LengthLessThanOrEqual = 'LENGTH_LESS_THAN_OR_EQUALS', } export interface OperatorType { @@ -89,6 +100,39 @@ export const OPERATOR_ENUM_TO_OPERATOR_TYPE: { display: t('TEMPORAL_RANGE'), operation: 'TEMPORAL_RANGE', }, + [Operators.ContainsAny]: { + display: t('Contains any'), + operation: 'CONTAINS_ANY', + }, + [Operators.ContainsAll]: { + display: t('Contains all'), + operation: 'CONTAINS_ALL', + }, + [Operators.IsEmpty]: { display: t('Is empty'), operation: 'IS_EMPTY' }, + [Operators.IsNotEmpty]: { + display: t('Is not empty'), + operation: 'IS_NOT_EMPTY', + }, + [Operators.LengthEquals]: { + display: t('Length equals (=)'), + operation: 'LENGTH_EQUALS', + }, + [Operators.LengthGreaterThan]: { + display: t('Length greater than (>)'), + operation: 'LENGTH_GREATER_THAN', + }, + [Operators.LengthLessThan]: { + display: t('Length less than (<)'), + operation: 'LENGTH_LESS_THAN', + }, + [Operators.LengthGreaterThanOrEqual]: { + display: t('Length greater or equal (>=)'), + operation: 'LENGTH_GREATER_THAN_OR_EQUALS', + }, + [Operators.LengthLessThanOrEqual]: { + display: t('Length less or equal (<=)'), + operation: 'LENGTH_LESS_THAN_OR_EQUALS', + }, }; export const OPERATORS_OPTIONS = Object.values(Operators) as Operators[]; @@ -105,7 +149,12 @@ export const HAVING_OPERATORS = [ Operators.GreaterThan, Operators.GreaterThanOrEqual, ]; -export const MULTI_OPERATORS = new Set([Operators.In, Operators.NotIn]); +export const MULTI_OPERATORS = new Set([ + Operators.In, + Operators.NotIn, + Operators.ContainsAny, + Operators.ContainsAll, +]); // CUSTOM_OPERATORS will show operator in simple mode, // but will generate customized sqlExpression export const CUSTOM_OPERATORS = new Set([ @@ -120,6 +169,8 @@ export const DISABLE_INPUT_OPERATORS = [ Operators.LatestPartition, Operators.IsTrue, Operators.IsFalse, + Operators.IsEmpty, + Operators.IsNotEmpty, ]; export const sqlaAutoGeneratedMetricNameRegex = diff --git a/superset-frontend/src/explore/exploreUtils/getSimpleSQLExpression.test.ts b/superset-frontend/src/explore/exploreUtils/getSimpleSQLExpression.test.ts index d2319cf32688..0707e263e623 100644 --- a/superset-frontend/src/explore/exploreUtils/getSimpleSQLExpression.test.ts +++ b/superset-frontend/src/explore/exploreUtils/getSimpleSQLExpression.test.ts @@ -82,3 +82,14 @@ test('Should handle boolean true comparator as a string value', () => { "subject operator 'TRUE'", ); }); + +test('Should render array-literal comparators as-is (not quoted)', () => { + // Whole-array = filter: the pasted array literal is shown unquoted. + expect(getSimpleSQLExpression('ingredients', '=', "['1 large egg']")).toBe( + "ingredients = ['1 large egg']", + ); + // IN with multiple array literals. + expect( + getSimpleSQLExpression('ingredients', Operators.In, ["['a']", "['b']"]), + ).toBe(`ingredients ${Operators.In} (['a'], ['b'])`); +}); diff --git a/superset-frontend/src/explore/exploreUtils/index.ts b/superset-frontend/src/explore/exploreUtils/index.ts index f4fc374c143a..af9308188d7f 100644 --- a/superset-frontend/src/explore/exploreUtils/index.ts +++ b/superset-frontend/src/explore/exploreUtils/index.ts @@ -461,10 +461,15 @@ export const getSimpleSQLExpression = ( if (comparatorArray.length > 0 && showComparator) { const formattedComparators = comparatorArray .map(val => optionLabel(val)) - .map( - val => - `${quote}${isString ? String(val).replace(/'/g, "''") : val}${quote}`, - ); + .map(val => { + // Array-literal values (e.g. ['a', 'b']) are shown as-is rather than + // quoted/escaped as a string, so array-column filters read naturally. + const asString = String(val); + if (asString.startsWith('[') && asString.endsWith(']')) { + return asString; + } + return `${quote}${isString ? asString.replace(/'/g, "''") : val}${quote}`; + }); expression += ` ${prefix}${formattedComparators.join(', ')}${suffix}`; } } diff --git a/superset-frontend/src/pages/DatasetList/DatasetList.listview.test.tsx b/superset-frontend/src/pages/DatasetList/DatasetList.listview.test.tsx index 5356ebc0d383..a6d6ea9ae78b 100644 --- a/superset-frontend/src/pages/DatasetList/DatasetList.listview.test.tsx +++ b/superset-frontend/src/pages/DatasetList/DatasetList.listview.test.tsx @@ -1157,6 +1157,34 @@ test('dataset links use internal routing when PREVENT_UNSAFE_DEFAULT_URLS_ON_DAT }); }); +test('legacy dashboard default URLs use the registered client route', async () => { + const dataset = { + ...mockDatasets[0], + explore_url: '/superset/dashboard/123/?standalone=1#section', + }; + mockDatasetListEndpoints({ result: [dataset], count: 1 }); + + renderDatasetList( + mockAdminUser, + {}, + { + common: { + conf: { + PREVENT_UNSAFE_DEFAULT_URLS_ON_DATASET: true, + }, + }, + }, + ); + + const datasetLink = await screen.findByRole('link', { + name: dataset.table_name, + }); + expect(datasetLink).toHaveAttribute( + 'href', + '/dashboard/123/?standalone=1#section', + ); +}); + // Note: These delete error tests verify that the modal doesn't open when fetching // related_objects fails. The component's openDatasetDeleteModal error handler // (index.tsx:262-268) returns a string but doesn't call addDangerToast(), so no diff --git a/superset-frontend/src/pages/DatasetList/DatasetList.subdirectory.test.tsx b/superset-frontend/src/pages/DatasetList/DatasetList.subdirectory.test.tsx index 2c8717f35d42..248e8beebc5e 100644 --- a/superset-frontend/src/pages/DatasetList/DatasetList.subdirectory.test.tsx +++ b/superset-frontend/src/pages/DatasetList/DatasetList.subdirectory.test.tsx @@ -54,10 +54,18 @@ import { const APP_ROOT = '/superset'; -const renderUnderSubdirectory = () => { +const renderUnderSubdirectory = (preventUnsafeDefaultUrls = false) => { + const defaultState = createDefaultStoreState(mockAdminUser); const store = createMockStore({ - ...createDefaultStoreState(mockAdminUser), + ...defaultState, user: mockAdminUser, + common: { + ...defaultState.common, + conf: { + ...defaultState.common?.conf, + PREVENT_UNSAFE_DEFAULT_URLS_ON_DATASET: preventUnsafeDefaultUrls, + }, + }, }); return render( @@ -115,6 +123,31 @@ test('explore link is single-prefixed under a subdirectory deployment', async () expect(exploreLink.getAttribute('href')).not.toContain('/superset/superset'); }); +test('legacy dashboard default URL uses the router basename once', async () => { + // A subdirectory user pastes the full browser path, so the saved value + // carries both the application root and the legacy `/superset` prefix. + // stripAppRoot removes the root and the legacy normalization removes the + // prefix, leaving the basename to re-add the root exactly once. + const dataset = { + ...mockDatasets[0], + explore_url: `${APP_ROOT}/superset/dashboard/123/?standalone=1#section`, + }; + mockDatasetListEndpoints({ result: [dataset], count: 1 }); + + renderUnderSubdirectory(true); + + const dashboardLink = await screen.findByRole('link', { + name: dataset.table_name, + }); + expect(dashboardLink).toHaveAttribute( + 'href', + `${APP_ROOT}/dashboard/123/?standalone=1#section`, + ); + expect(dashboardLink.getAttribute('href')).not.toContain( + '/superset/superset', + ); +}); + test('external default_endpoint passes through unprefixed', async () => { const dataset = { ...mockDatasets[0], diff --git a/superset-frontend/src/pages/DatasetList/index.tsx b/superset-frontend/src/pages/DatasetList/index.tsx index 5e716af14c52..9d387ca0ea00 100644 --- a/superset-frontend/src/pages/DatasetList/index.tsx +++ b/superset-frontend/src/pages/DatasetList/index.tsx @@ -87,7 +87,6 @@ import withToasts from 'src/components/MessageToasts/withToasts'; import { Icons } from '@superset-ui/core/components/Icons'; import WarningIconWithTooltip from '@superset-ui/core/components/WarningIconWithTooltip'; import { isUserEditorOrAdmin } from 'src/dashboard/util/permissionUtils'; - import { PAGE_SIZE, SORT_BY, @@ -114,6 +113,10 @@ import type { } from 'src/types/bootstrapTypes'; import type User from 'src/types/User'; +// Keep saved Default URLs compatible with the prefix-free SPA route. +const normalizeLegacyDashboardUrl = (url: string) => + url.replace(/^\/superset(?=\/dashboard(?:\/|$))/, ''); + const SEMANTIC_LAYERS_FLAG = 'SEMANTIC_LAYERS' as FeatureFlag; type DatasetExtra = { certification?: { @@ -722,7 +725,9 @@ const DatasetList: FunctionComponent = ({ // Router basename, which re-prefixes the root — so strip it here to // avoid a doubled `/superset/superset/...`. External // `default_endpoint` URLs pass through unchanged. - const exploreTo = stripAppRoot(exploreURL); + const exploreTo = normalizeLegacyDashboardUrl( + stripAppRoot(exploreURL), + ); let titleLink: JSX.Element; if (PREVENT_UNSAFE_DEFAULT_URLS_ON_DATASET) { titleLink = ( diff --git a/superset-websocket/package-lock.json b/superset-websocket/package-lock.json index 33ec87e7c139..0cd1f07701a2 100644 --- a/superset-websocket/package-lock.json +++ b/superset-websocket/package-lock.json @@ -28,7 +28,7 @@ "@typescript-eslint/parser": "^8.67.0", "eslint": "^10.8.1", "eslint-config-prettier": "^10.1.8", - "globals": "^17.9.0", + "globals": "^17.10.0", "oxfmt": "^0.63.0", "tscw-config": "^1.1.2", "typescript": "^6.0.3", @@ -2053,9 +2053,9 @@ } }, "node_modules/globals": { - "version": "17.9.0", - "resolved": "https://registry.npmjs.org/globals/-/globals-17.9.0.tgz", - "integrity": "sha512-m/MvAW61QVU5VDNF1Vj8axt016h8w7L5TU1e9zlab7XIttAT2YAlCwl75K1fOqvMM9apmD7lbCIRhpfkhmxhCg==", + "version": "17.10.0", + "resolved": "https://registry.npmjs.org/globals/-/globals-17.10.0.tgz", + "integrity": "sha512-V0kztuWST2k8A/VbxAY8+L+7+Rgo3fyA24IHRLrZp7HOzJjV0gHSaZUjK9lpP/IrBSNite2tZ1prhRkinRu1CA==", "dev": true, "license": "MIT", "engines": { diff --git a/superset-websocket/package.json b/superset-websocket/package.json index af9d4d253805..b8349ad5aa20 100644 --- a/superset-websocket/package.json +++ b/superset-websocket/package.json @@ -36,7 +36,7 @@ "@typescript-eslint/parser": "^8.67.0", "eslint": "^10.8.1", "eslint-config-prettier": "^10.1.8", - "globals": "^17.9.0", + "globals": "^17.10.0", "oxfmt": "^0.63.0", "tscw-config": "^1.1.2", "typescript": "^6.0.3", diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index 88e148731092..8445caeb3a0b 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -957,7 +957,13 @@ def query(self, query_obj: QueryObjectDict) -> QueryResult: def get_query_str(self, query_obj: QueryObjectDict) -> str: raise NotImplementedError() - def values_for_column(self, column_name: str, limit: int = 10000) -> list[Any]: + def values_for_column( + self, + column_name: str, + limit: int = 10000, + denormalize_column: bool = False, + array_elements: bool = False, + ) -> list[Any]: raise NotImplementedError() diff --git a/superset/datasource/api.py b/superset/datasource/api.py index f04db51e8860..4e09e3edad85 100644 --- a/superset/datasource/api.py +++ b/superset/datasource/api.py @@ -133,6 +133,9 @@ def get_column_values( row_limit = apply_max_row_limit(app.config["FILTER_SELECT_ROW_LIMIT"]) denormalize_column = not datasource.normalize_columns + # Element-level operators (Contains any / Contains all) request the + # distinct array *elements* rather than distinct whole arrays. + array_elements = parse_boolean_string(request.args.get("array_elements")) # Cache distinct column-value results so a dashboard with many filters # backed by the same (often heavy) virtual dataset doesn't re-execute @@ -165,6 +168,7 @@ def get_column_values( "col": column_name, "limit": row_limit, "denorm": denormalize_column, + "elements": array_elements, "rls": security_manager.get_rls_cache_key(datasource), "changed_on": str(getattr(datasource, "changed_on", "")), }, @@ -189,6 +193,7 @@ def get_column_values( column_name=column_name, limit=row_limit, denormalize_column=denormalize_column, + array_elements=array_elements, ) except KeyError: return self.response( diff --git a/superset/db_engine_specs/base.py b/superset/db_engine_specs/base.py index ebd85bd43a41..6bf6040b8680 100644 --- a/superset/db_engine_specs/base.py +++ b/superset/db_engine_specs/base.py @@ -55,7 +55,13 @@ from sqlalchemy.engine.url import URL from sqlalchemy.ext.compiler import compiles from sqlalchemy.sql import literal_column, quoted_name, text -from sqlalchemy.sql.expression import BinaryExpression, ColumnClause, Select, TextClause +from sqlalchemy.sql.expression import ( + BinaryExpression, + ColumnClause, + ColumnElement, + Select, + TextClause, +) from sqlalchemy.types import TypeEngine from superset import db @@ -528,6 +534,11 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods time_groupby_inline = False limit_method = LimitMethod.FORCE_LIMIT supports_multivalues_insert = False + # Whether this engine supports first-class multi-value (array-typed) columns. + # When True, array columns are classified as ``GenericDataType.MULTI_VALUE`` and + # the ``array_*`` capability methods below must be implemented. Defaults to + # False so engines that have not opted in keep treating arrays as strings. + supports_multivalue_columns = False allows_joins = True allows_subqueries = True allows_alias_in_select = True @@ -2571,6 +2582,105 @@ def update_params_from_encrypted_extra( # pylint: disable=invalid-name logger.error(ex, exc_info=True) raise + @classmethod + def array_contains_any(cls, col: ColumnElement, values: list[Any]) -> ColumnElement: + """ + Build a boolean expression testing whether array column ``col`` contains + **any** of ``values`` (element-level membership, like ``IN``). Engines + that set ``supports_multivalue_columns = True`` must override this with + their native function (e.g. ClickHouse ``hasAny``). + + :param col: SQLAlchemy column element for the array column + :param values: element values to look for inside the array + :return: a SQLAlchemy boolean expression + """ + raise NotImplementedError( + f"{cls.engine} does not support multi-value (array) columns" + ) + + @classmethod + def array_contains_all(cls, col: ColumnElement, values: list[Any]) -> ColumnElement: + """ + Build a boolean expression testing whether array column ``col`` contains + **all** of ``values``. Engines that set + ``supports_multivalue_columns = True`` must override this with their + native function (e.g. ClickHouse ``hasAll``). + + :param col: SQLAlchemy column element for the array column + :param values: element values that must all be present + :return: a SQLAlchemy boolean expression + """ + raise NotImplementedError( + f"{cls.engine} does not support multi-value (array) columns" + ) + + @classmethod + def array_length(cls, col: ColumnElement) -> ColumnElement: + """ + Build a numeric expression returning the number of elements in array + column ``col``. Engines that set ``supports_multivalue_columns = True`` + must override this with their native array-length function. Used both for + the ``Length`` filter and the ``Is empty`` / ``Is not empty`` operators. + + :param col: SQLAlchemy column element for the array column + :return: a SQLAlchemy numeric expression + """ + raise NotImplementedError( + f"{cls.engine} does not support multi-value (array) columns" + ) + + @classmethod + def array_literal(cls, values: list[Any]) -> ColumnElement: + """ + Build an array-literal expression from ``values`` (e.g. ClickHouse + ``array(v1, v2)`` == ``[v1, v2]``). Used for the whole-array (column- + level) operators ``=`` / ``!=`` / ``IN`` / ``NOT IN`` where the array is + compared as a single value. Engines that set + ``supports_multivalue_columns = True`` must override this. + + :param values: element values that make up the array + :return: a SQLAlchemy array-literal expression + """ + raise NotImplementedError( + f"{cls.engine} does not support multi-value (array) columns" + ) + + @classmethod + def array_explode(cls, col: ColumnElement) -> ColumnElement: + """ + Build an expression that expands array column ``col`` into one row per + element (e.g. ClickHouse ``arrayJoin``). Used to source **element-level** + value suggestions (``SELECT DISTINCT array_explode(col)``) for the + ``Contains any`` / ``Contains all`` filter operators, so the picker offers + individual elements rather than whole arrays. Engines that set + ``supports_multivalue_columns = True`` must override this. + + :param col: SQLAlchemy column element for the array column + :return: a SQLAlchemy expression yielding one element per row + """ + raise NotImplementedError( + f"{cls.engine} does not support multi-value (array) columns" + ) + + @classmethod + def get_array_element_type( # pylint: disable=unused-argument + cls, native_type: str | None + ) -> GenericDataType | None: + """ + Return the generic type of an array column's **element** type, derived + from its native type string (e.g. ClickHouse ``Array(Int32)`` -> + ``NUMERIC``), or ``None`` when the engine has no array support or the + element type cannot be resolved. + + Callers use this to coerce filter values to the element type before + building array expressions, so, for example, a ``Contains any`` filter on + a numeric array compares against numbers rather than quoted strings. + + :param native_type: native column type string of the array column + :return: the element's :class:`GenericDataType`, or ``None`` + """ + return None + @classmethod def get_column_spec( # pylint: disable=unused-argument cls, diff --git a/superset/db_engine_specs/clickhouse.py b/superset/db_engine_specs/clickhouse.py index 41a7a27649c6..1bf29a4ea1f3 100644 --- a/superset/db_engine_specs/clickhouse.py +++ b/superset/db_engine_specs/clickhouse.py @@ -26,8 +26,9 @@ from flask_babel import gettext as __ from marshmallow import fields, Schema from marshmallow.validate import Range -from sqlalchemy import types +from sqlalchemy import func, types from sqlalchemy.engine.url import URL +from sqlalchemy.sql.expression import ColumnElement from urllib3.exceptions import NewConnectionError from superset.databases.utils import make_url_safe @@ -55,6 +56,7 @@ class ClickHouseBaseEngineSpec(BaseEngineSpec): time_groupby_inline = True supports_multivalues_insert = True + supports_multivalue_columns = True # ClickHouse doesn't support IS true/false syntax, use = true/false instead use_equality_for_boolean_filters = True @@ -128,12 +130,18 @@ def is_read_limit_error(cls, ex: Exception) -> bool: column_type_mappings = ( ( - re.compile(r".*Enum.*", re.IGNORECASE), + # Anchor to the start so only top-level arrays match. This must be + # ordered before the ``Enum`` entry below: ``Array(Enum8(...))`` is a + # real array and should classify as MULTI_VALUE, not STRING. The + # anchor also prevents over-matching nested arrays such as + # ``Map(String, Array(String))`` or ``Tuple(Array(String))``, which + # are not themselves array columns and must keep their own type. + re.compile(r"^Array\(", re.IGNORECASE), types.String(), - GenericDataType.STRING, + GenericDataType.MULTI_VALUE, ), ( - re.compile(r".*Array.*", re.IGNORECASE), + re.compile(r".*Enum.*", re.IGNORECASE), types.String(), GenericDataType.STRING, ), @@ -174,6 +182,56 @@ def is_read_limit_error(cls, ex: Exception) -> bool: ), ) + @classmethod + def array_contains_any(cls, col: ColumnElement, values: list[Any]) -> ColumnElement: + # ClickHouse: hasAny(arr, [v1, v2]) -> 1 if arr shares any element. + # func.array(*values) renders as array(v1, v2) == [v1, v2]. + return func.hasAny(col, func.array(*values)) + + @classmethod + def array_contains_all(cls, col: ColumnElement, values: list[Any]) -> ColumnElement: + # ClickHouse: hasAll(arr, [v1, v2]) -> 1 if arr contains all elements. + return func.hasAll(col, func.array(*values)) + + @classmethod + def array_length(cls, col: ColumnElement) -> ColumnElement: + # ClickHouse: length(arr) -> number of elements + return func.length(col) + + @classmethod + def array_literal(cls, values: list[Any]) -> ColumnElement: + # ClickHouse: array(v1, v2) is equivalent to the literal [v1, v2]. + return func.array(*values) + + @classmethod + def array_explode(cls, col: ColumnElement) -> ColumnElement: + # ClickHouse: arrayJoin(arr) yields one row per element, so + # SELECT DISTINCT arrayJoin(arr) returns the distinct elements. + return func.arrayJoin(col) + + # Matches the element type inside a top-level ``Array(...)`` column, e.g. + # ``Array(Int32)`` -> ``Int32``, ``Array(Nullable(String))`` -> ``String``. + _ARRAY_ELEMENT_RE = re.compile(r"^Array\((?P.+)\)$", re.IGNORECASE) + # Element-type wrappers that don't change the underlying generic type. + _ELEMENT_WRAPPER_RE = re.compile( + r"^(?:Nullable|LowCardinality)\((?P.+)\)$", re.IGNORECASE + ) + + @classmethod + def get_array_element_type(cls, native_type: str | None) -> GenericDataType | None: + if not native_type: + return None + match = cls._ARRAY_ELEMENT_RE.match(native_type.strip()) + if not match: + return None + inner = match.group("inner").strip() + # Peel wrappers (Nullable/LowCardinality) that don't alter the generic + # type so the inner scalar type drives classification. + while wrapper := cls._ELEMENT_WRAPPER_RE.match(inner): + inner = wrapper.group("inner").strip() + spec = cls.get_column_spec(inner) + return spec.generic_type if spec else None + @classmethod def epoch_to_dttm(cls) -> str: return "{col}" diff --git a/superset/mcp_service/dataset/schemas.py b/superset/mcp_service/dataset/schemas.py index 89e43a036da2..303ec71e55b7 100644 --- a/superset/mcp_service/dataset/schemas.py +++ b/superset/mcp_service/dataset/schemas.py @@ -63,6 +63,7 @@ sanitize_for_llm_context, ) from superset.mcp_service.utils.response_utils import humanize_timestamp +from superset.sql.parse import has_aggregate from superset.utils import json @@ -386,13 +387,27 @@ class CreateDatasetMetric(BaseModel): """Metric definition for dataset creation.""" metric_name: str = Field(..., description="Name of the metric") - expression: str = Field(..., description="SQL expression for the metric") + expression: str = Field( + ..., + description="Aggregate SQL expression for the metric, e.g. SUM(amount)", + ) verbose_name: str | None = None description: str | None = None metric_type: str | None = None d3format: str | None = None warning_text: str | None = None + @field_validator("expression") + @classmethod + def expression_must_aggregate(cls, value: str) -> str: + if not has_aggregate(value): + raise ValueError( + "saved metrics must aggregate rows; wrap a row-level column in " + "an aggregate such as MAX(column), or omit the saved metric and " + "use the dataset column directly" + ) + return value + class CreateDatasetCalculatedColumn(BaseModel): """Calculated column definition for dataset creation.""" diff --git a/superset/mcp_service/dataset/tool/create_virtual_dataset.py b/superset/mcp_service/dataset/tool/create_virtual_dataset.py index c134f97ddf5d..f16e31b0f002 100644 --- a/superset/mcp_service/dataset/tool/create_virtual_dataset.py +++ b/superset/mcp_service/dataset/tool/create_virtual_dataset.py @@ -21,6 +21,7 @@ from fastmcp import Context from superset_core.mcp.decorators import tool, ToolAnnotations +from superset.exceptions import SupersetGenericDBErrorException from superset.extensions import event_logger from superset.mcp_service.dataset.schemas import ( CreateVirtualDatasetRequest, @@ -67,14 +68,17 @@ def _cleanup_failed_dataset(dataset_id: int) -> None: def _update_virtual_dataset(dataset_id: int, update_props: dict[str, Any]) -> Any: - from superset.commands.dataset.exceptions import DatasetUpdateFailedError + from superset.commands.dataset.exceptions import ( + DatasetInvalidError, + DatasetUpdateFailedError, + ) from superset.commands.dataset.update import UpdateDatasetCommand try: return UpdateDatasetCommand(dataset_id, update_props).run() except Exception as exc: _cleanup_failed_dataset(dataset_id) - if not isinstance(exc, DatasetUpdateFailedError): + if not isinstance(exc, (DatasetInvalidError, DatasetUpdateFailedError)): raise DatasetUpdateFailedError() from exc raise @@ -89,7 +93,7 @@ def _update_virtual_dataset(dataset_id: int, update_props: dict[str, Any]) -> An destructiveHint=False, ), ) -async def create_virtual_dataset( +async def create_virtual_dataset( # noqa: C901 request: CreateVirtualDatasetRequest, ctx: Context ) -> CreateVirtualDatasetResponse: """Save a SQL query as a virtual dataset so it can be charted. @@ -213,6 +217,18 @@ async def create_virtual_dataset( url=None, error=f"Failed to update dataset metadata (creation rolled back): {exc}", ) + except SupersetGenericDBErrorException as exc: + logger.warning("Virtual dataset SQL validation failed", exc_info=True) + await ctx.warning(f"Virtual dataset SQL failed validation: {exc}") + return CreateVirtualDatasetResponse( + id=None, + dataset_name=request.dataset_name, + sql=request.sql, + database_id=request.database_id, + columns=[], + url=None, + error=f"Dataset SQL could not be executed: {exc}", + ) except Exception as exc: await ctx.error( f"Unexpected error creating virtual dataset: {type(exc).__name__}: {exc}" diff --git a/superset/models/helpers.py b/superset/models/helpers.py index b87427eaf41a..ebfc43e6b1bf 100644 --- a/superset/models/helpers.py +++ b/superset/models/helpers.py @@ -19,6 +19,7 @@ from __future__ import annotations +import ast import builtins import copy import dataclasses @@ -417,6 +418,52 @@ def json_to_dict(json_str: str) -> dict[Any, Any]: ) +def parse_array_literal(value: Any) -> list[Any]: + """ + Parse a user-entered array literal (e.g. ``['a', 'b']`` or ``[1, 2]``) into a + list of elements, for the whole-array (column-level) array operators. + + Accepts either an actual list/tuple, a bracketed literal string (parsed with + ``ast.literal_eval``), or a plain scalar (wrapped into a single-element list). + Falls back to a single-element list when the string is not a valid literal. + """ + if isinstance(value, (list, tuple)): + return list(value) + if isinstance(value, str): + stripped = value.strip() + if stripped.startswith("[") and stripped.endswith("]"): + try: + parsed = ast.literal_eval(stripped) + except (ValueError, SyntaxError): + parsed = None + if isinstance(parsed, (list, tuple)): + return list(parsed) + return [value] + + +def coerce_array_values( + values: list[Any], element_type: Optional[utils.GenericDataType] +) -> list[Any]: + """ + Coerce array-element ``values`` to the array column's element type so the + emitted literal matches the column. Array columns map to a SQLAlchemy + ``String`` type, so values arrive as strings and would otherwise build + string literals (e.g. ``array('5')``) that fail against a numeric array on + the server. Numeric elements are cast to numbers and boolean elements to + booleans; every other element type (string, temporal, enum, unknown) is left + untouched. + + :param values: element values entered for an array filter + :param element_type: the array's element :class:`GenericDataType`, or None + :return: the coerced values + """ + if element_type == utils.GenericDataType.NUMERIC: + return [utils.cast_to_num(v) if isinstance(v, str) else v for v in values] + if element_type == utils.GenericDataType.BOOLEAN: + return [utils.cast_to_boolean(v) if isinstance(v, str) else v for v in values] + return values + + def is_uuid_native_type(native_type: Optional[str]) -> bool: """ Return True if a native column type represents a UUID. @@ -3652,6 +3699,7 @@ def values_for_column( # pylint: disable=too-many-locals column_name: str, limit: int = 10000, denormalize_column: bool = False, + array_elements: bool = False, ) -> list[Any]: # denormalize column name before querying for values # unless disabled in the dataset configuration @@ -3666,13 +3714,25 @@ def values_for_column( # pylint: disable=too-many-locals tp = self.get_template_processor() tbl, cte = self.get_from_clause(tp) + db_engine_spec = self.database.db_engine_spec + value_expr = target_col.get_sqla_col(template_processor=tp) + # For element-level operators (Contains any / Contains all) on a + # multi-value (array) column, suggest the distinct **elements** rather + # than distinct whole arrays by expanding the array first (e.g. ClickHouse + # arrayJoin). Only when the engine supports arrays and the column is + # actually an array column; otherwise fall back to whole-value suggestions. + if array_elements and db_engine_spec.supports_multivalue_columns: + col_spec = db_engine_spec.get_column_spec(native_type=target_col.type) + if col_spec and col_spec.generic_type == GenericDataType.MULTI_VALUE: + value_expr = db_engine_spec.array_explode(value_expr) + qry = ( sa.select( # The alias (label) here is important because some dialects will # automatically add a random alias to the projection because of the # call to DISTINCT; others will uppercase the column names. This # gives us a deterministic column name in the dataframe. - target_col.get_sqla_col(template_processor=tp).label("column_values") + value_expr.label("column_values") ) .select_from(tbl) .distinct() @@ -4359,7 +4419,7 @@ def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-ma elif is_adhoc_column(flt_col): try: sqla_col, adhoc_generic_type = self.adhoc_column_to_sqla( - flt_col, + cast("AdhocColumn", flt_col), force_type_check=True, template_processor=template_processor, ) @@ -4433,9 +4493,21 @@ def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-ma sqla_col = Grouping(sqla_col) col_type = col_obj.type if col_obj else None col_spec = db_engine_spec.get_column_spec(native_type=col_type) + is_multivalue_col = bool( + col_spec and col_spec.generic_type == GenericDataType.MULTI_VALUE + ) + # Element type of an array column (e.g. Array(Int32) -> NUMERIC), + # used to coerce filter values before building array expressions. + array_element_type = ( + db_engine_spec.get_array_element_type(col_type) + if is_multivalue_col + else None + ) is_list_target = op in ( utils.FilterOperator.IN, utils.FilterOperator.NOT_IN, + utils.FilterOperator.CONTAINS_ANY, + utils.FilterOperator.CONTAINS_ALL, ) col_advanced_data_type = col_obj.advanced_data_type if col_obj else "" @@ -4490,7 +4562,56 @@ def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-ma sqla_col, op, bus_resp["values"] ) ) - elif is_list_target: + elif is_multivalue_col and op in { + utils.FilterOperator.EQUALS, + utils.FilterOperator.NOT_EQUALS, + utils.FilterOperator.IN, + utils.FilterOperator.NOT_IN, + }: + # Whole-array (column-level) comparison against array + # literal(s). The value is a pasted array literal like + # ``['a', 'b']`` (parsed into elements): ``col = ['a', 'b']`` + # for = / !=; for IN / NOT IN each entered value is one such + # array literal (``col IN (['a'], ['b'])``). + if op in { + utils.FilterOperator.EQUALS, + utils.FilterOperator.NOT_EQUALS, + }: + literal = db_engine_spec.array_literal( + coerce_array_values( + parse_array_literal(val), array_element_type + ) + ) + cond = ( + sqla_col != literal + if op == utils.FilterOperator.NOT_EQUALS + else sqla_col == literal + ) + else: + candidates: list[Any] = ( + list(val) if isinstance(val, (list, tuple)) else [val] + ) + cond = sqla_col.in_( + [ + db_engine_spec.array_literal( + coerce_array_values( + parse_array_literal(candidate), + array_element_type, + ) + ) + for candidate in candidates + ] + ) + if op == utils.FilterOperator.NOT_IN: + cond = ~cond + target_clause_list.append(cond) + elif op in { + utils.FilterOperator.IN, + utils.FilterOperator.NOT_IN, + }: + # CONTAINS_ANY/CONTAINS_ALL also produce a list ``eq`` (they + # are in ``is_list_target``), but are element-level array ops + # handled by their own branch below — not IN. assert isinstance(eq, (tuple, list)) if len(eq) == 0: raise QueryObjectValidationError( @@ -4529,6 +4650,57 @@ def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-ma target_clause_list.append( db_engine_spec.handle_null_filter(sqla_col, op) ) + elif op in { + utils.FilterOperator.IS_EMPTY, + utils.FilterOperator.IS_NOT_EMPTY, + }: + # Element-level array operators: length(col) == 0 / > 0. + if target_generic_type != GenericDataType.MULTI_VALUE: + raise QueryObjectValidationError( + _( + "The %(op)s operator is only supported for " + "multi-value (array) columns.", + op=op, + ) + ) + length_expr = db_engine_spec.array_length(sqla_col) + if op == utils.FilterOperator.IS_EMPTY: + target_clause_list.append(length_expr == 0) + else: + target_clause_list.append(length_expr > 0) + elif op in { + utils.FilterOperator.LENGTH_EQUALS, + utils.FilterOperator.LENGTH_GREATER_THAN, + utils.FilterOperator.LENGTH_LESS_THAN, + utils.FilterOperator.LENGTH_GREATER_THAN_OR_EQUALS, + utils.FilterOperator.LENGTH_LESS_THAN_OR_EQUALS, + }: + # Length filter: compare the array's element count to a + # number, e.g. length(col) > 2. + if target_generic_type != GenericDataType.MULTI_VALUE: + raise QueryObjectValidationError( + _( + "The %(op)s operator is only supported for " + "multi-value (array) columns.", + op=op, + ) + ) + number = utils.cast_to_num(eq) # type: ignore[arg-type] + if number is None: + raise QueryObjectValidationError( + _("The Length filter requires a numeric value.") + ) + length_expr = db_engine_spec.array_length(sqla_col) + length_comparisons = { + utils.FilterOperator.LENGTH_EQUALS: length_expr == number, + utils.FilterOperator.LENGTH_GREATER_THAN: length_expr > number, + utils.FilterOperator.LENGTH_LESS_THAN: length_expr < number, + utils.FilterOperator.LENGTH_GREATER_THAN_OR_EQUALS: length_expr + >= number, + utils.FilterOperator.LENGTH_LESS_THAN_OR_EQUALS: length_expr + <= number, + } + target_clause_list.append(length_comparisons[op]) elif op == utils.FilterOperator.IS_TRUE: target_clause_list.append( db_engine_spec.handle_boolean_filter(sqla_col, op, True) @@ -4586,6 +4758,38 @@ def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-ma target_clause_list.append(sqla_col.not_like(eq)) else: target_clause_list.append(sqla_col.not_ilike(eq)) + elif op in { + utils.FilterOperator.CONTAINS_ANY, + utils.FilterOperator.CONTAINS_ALL, + }: + # Element-level array membership. Enforce the target is + # actually a multi-value (array) column (only classified + # MULTI_VALUE on an array-capable engine), guarding against + # payloads that bypass the UI gating. + if target_generic_type != GenericDataType.MULTI_VALUE: + raise QueryObjectValidationError( + _( + "The %(op)s operator is only supported for " + "multi-value (array) columns.", + op=op, + ) + ) + array_values: list[Any] = coerce_array_values( + list(eq) if isinstance(eq, (list, tuple)) else [eq], + array_element_type, + ) + if op == utils.FilterOperator.CONTAINS_ANY: + target_clause_list.append( + db_engine_spec.array_contains_any( + sqla_col, array_values + ) + ) + else: + target_clause_list.append( + db_engine_spec.array_contains_all( + sqla_col, array_values + ) + ) elif ( op == utils.FilterOperator.TEMPORAL_RANGE and isinstance(eq, str) diff --git a/superset/utils/core.py b/superset/utils/core.py index 1c3b92ebd651..abfe9b221b9f 100644 --- a/superset/utils/core.py +++ b/superset/utils/core.py @@ -209,7 +209,7 @@ class GenericDataType(IntEnum): STRING = 1 TEMPORAL = 2 BOOLEAN = 3 - # ARRAY = 4 # Mapping all the complex data types to STRING for now + MULTI_VALUE = 4 # array-typed columns (e.g. ClickHouse Array, Postgres ARRAY) # JSON = 5 # and leaving these as a reminder. # MAP = 6 # ROW = 7 @@ -299,6 +299,17 @@ class FilterOperator(StrEnum): IS_TRUE = "IS TRUE" IS_FALSE = "IS FALSE" TEMPORAL_RANGE = "TEMPORAL_RANGE" + # Element-level operators for MULTI_VALUE (array) columns + CONTAINS_ANY = "CONTAINS_ANY" + CONTAINS_ALL = "CONTAINS_ALL" + IS_EMPTY = "IS_EMPTY" + IS_NOT_EMPTY = "IS_NOT_EMPTY" + # Length (element-count) comparison operators for array columns + LENGTH_EQUALS = "LENGTH_EQUALS" + LENGTH_GREATER_THAN = "LENGTH_GREATER_THAN" + LENGTH_LESS_THAN = "LENGTH_LESS_THAN" + LENGTH_GREATER_THAN_OR_EQUALS = "LENGTH_GREATER_THAN_OR_EQUALS" + LENGTH_LESS_THAN_OR_EQUALS = "LENGTH_LESS_THAN_OR_EQUALS" class FilterStringOperators(StrEnum): @@ -317,6 +328,15 @@ class FilterStringOperators(StrEnum): LATEST_PARTITION = ("LATEST_PARTITION",) IS_TRUE = ("IS_TRUE",) IS_FALSE = ("IS_FALSE",) + CONTAINS_ANY = ("CONTAINS_ANY",) + CONTAINS_ALL = ("CONTAINS_ALL",) + IS_EMPTY = ("IS_EMPTY",) + IS_NOT_EMPTY = ("IS_NOT_EMPTY",) + LENGTH_EQUALS = ("LENGTH_EQUALS",) + LENGTH_GREATER_THAN = ("LENGTH_GREATER_THAN",) + LENGTH_LESS_THAN = ("LENGTH_LESS_THAN",) + LENGTH_GREATER_THAN_OR_EQUALS = ("LENGTH_GREATER_THAN_OR_EQUALS",) + LENGTH_LESS_THAN_OR_EQUALS = ("LENGTH_LESS_THAN_OR_EQUALS",) class PostProcessingBoxplotWhiskerType(StrEnum): diff --git a/tests/integration_tests/datasource/api_tests.py b/tests/integration_tests/datasource/api_tests.py index 335f09ea47f8..4f9f7d85dcfb 100644 --- a/tests/integration_tests/datasource/api_tests.py +++ b/tests/integration_tests/datasource/api_tests.py @@ -154,8 +154,22 @@ def test_get_column_values_normalize_columns_enabled(self, values_for_column_moc column_name="col2", limit=10000, denormalize_column=False, + array_elements=False, ) + @pytest.mark.usefixtures("app_context", "virtual_dataset") + @patch("superset.models.helpers.ExploreMixin.values_for_column") + def test_get_column_values_array_elements_param(self, values_for_column_mock): + # The ?array_elements=true param (Contains any/all) is threaded through + # so array columns can suggest individual elements. + self.login(ADMIN_USERNAME) + table = self.get_virtual_dataset() + self.client.get( + f"api/v1/datasource/table/{table.id}/column/col2/values/" + "?array_elements=true" + ) + assert values_for_column_mock.call_args.kwargs["array_elements"] is True + @pytest.mark.usefixtures("app_context", "virtual_dataset") @patch("superset.db_engine_specs.base.BaseEngineSpec.denormalize_name") def test_get_column_values_not_denormalize_column(self, denormalize_name_mock): @@ -176,6 +190,7 @@ def test_get_column_values_normalize_columns_disabled(self, values_for_column_mo column_name="col2", limit=10000, denormalize_column=True, + array_elements=False, ) @pytest.mark.usefixtures("app_context", "virtual_dataset") diff --git a/tests/integration_tests/sqla_models_tests.py b/tests/integration_tests/sqla_models_tests.py index 28e970b8001b..ca46371afe93 100644 --- a/tests/integration_tests/sqla_models_tests.py +++ b/tests/integration_tests/sqla_models_tests.py @@ -1308,3 +1308,152 @@ def mock_get_df(sql, catalog=None, schema=None, mutator=None): finally: db.session.delete(table) db.session.commit() + + +def _multivalue_table() -> SqlaTable: + """A dataset with an ``Array(String)`` column, for multi-value query tests. + + Built over the example database but never executed — the tests only compile + the generated SQL, so the backing table need not physically exist. + """ + columns = [ + TableColumn(column_name="skills", type="Array(String)"), + TableColumn(column_name="city", type="VARCHAR(255)"), + ] + return SqlaTable( + table_name="test_multivalue_jobs", + database=get_example_database(), + columns=columns, + metrics=[SqlMetric(metric_name="count", expression="COUNT(*)")], + ) + + +def _multivalue_query( + *, + filters: list[dict[str, Any]] | None = None, + groupby: list[Any] | None = None, +) -> dict[str, Any]: + return { + "granularity": None, + "from_dttm": None, + "to_dttm": None, + "is_timeseries": False, + "groupby": groupby if groupby is not None else ["city"], + "metrics": ["count"], + "filter": filters or [], + "extras": {}, + } + + +def _compile(table: SqlaTable, query_obj: dict[str, Any]) -> str: + from superset.db_engine_specs.clickhouse import ClickHouseEngineSpec + + with patch.object( + SqlaTable, "db_engine_spec", property(lambda self: ClickHouseEngineSpec) + ): + sqla_query = table.get_sqla_query(**query_obj) + return table.database.compile_sqla_query(sqla_query.sqla_query).lower() + + +@pytest.mark.usefixtures("app_context") +def test_multivalue_contains_any_generates_native_sql(): + """CONTAINS_ANY compiles to ``hasAny(col, array(...))``.""" + table = _multivalue_table() + sql = _compile( + table, + _multivalue_query( + filters=[ + { + "col": "skills", + "op": FilterOperator.CONTAINS_ANY.value, + "val": ["Driver", "Cook"], + } + ] + ), + ) + assert "hasany(skills" in sql + + +@pytest.mark.usefixtures("app_context") +def test_multivalue_contains_all_generates_native_sql(): + """CONTAINS_ALL compiles to ``hasAll(col, array(...))``.""" + table = _multivalue_table() + sql = _compile( + table, + _multivalue_query( + filters=[ + { + "col": "skills", + "op": FilterOperator.CONTAINS_ALL.value, + "val": ["Driver", "Cook"], + } + ] + ), + ) + assert "hasall(skills" in sql + + +@pytest.mark.usefixtures("app_context") +def test_multivalue_is_empty_generates_native_sql(): + """IS_EMPTY compiles to ``length(col) = 0``.""" + table = _multivalue_table() + sql = _compile( + table, + _multivalue_query( + filters=[{"col": "skills", "op": FilterOperator.IS_EMPTY.value}] + ), + ) + assert "length(skills) = 0" in sql + + +@pytest.mark.usefixtures("app_context") +def test_multivalue_length_filter_generates_native_sql(): + """A LENGTH_GREATER_THAN filter compiles to ``length(col) > N``.""" + table = _multivalue_table() + sql = _compile( + table, + _multivalue_query( + filters=[ + { + "col": "skills", + "op": FilterOperator.LENGTH_GREATER_THAN.value, + "val": 2, + } + ] + ), + ) + assert "length(skills) > 2" in sql + + +@pytest.mark.usefixtures("app_context") +def test_multivalue_contains_unsupported_engine_raises(): + """CONTAINS_ANY on an engine without array support is rejected.""" + table = _multivalue_table() + query_obj = _multivalue_query( + filters=[ + { + "col": "skills", + "op": FilterOperator.CONTAINS_ANY.value, + "val": ["Driver"], + } + ] + ) + with pytest.raises(QueryObjectValidationError): + table.get_sqla_query(**query_obj) + + +@pytest.mark.usefixtures("app_context") +def test_multivalue_length_filter_unsupported_engine_raises(): + """A Length filter on an engine without array support is rejected.""" + table = _multivalue_table() + query_obj = _multivalue_query( + filters=[ + { + "col": "skills", + "op": FilterOperator.LENGTH_GREATER_THAN.value, + "val": 2, + } + ] + ) + with pytest.raises(QueryObjectValidationError): + table.get_sqla_query(**query_obj) diff --git a/tests/unit_tests/db_engine_specs/test_base.py b/tests/unit_tests/db_engine_specs/test_base.py index 01f9d46242fe..bb53f4ddc66c 100644 --- a/tests/unit_tests/db_engine_specs/test_base.py +++ b/tests/unit_tests/db_engine_specs/test_base.py @@ -1490,3 +1490,21 @@ def test_get_public_information_exposes_ansi_identifier_quote() -> None: "end": '"', "escape_by_doubling": True, } + + +def test_multivalue_columns_disabled_by_default() -> None: + """Engines must opt in to multi-value support; base defaults to off.""" + assert BaseEngineSpec.supports_multivalue_columns is False + + +@pytest.mark.parametrize( + "method", ["array_contains_any", "array_contains_all", "array_length"] +) +def test_array_capabilities_raise_when_unsupported(method: str) -> None: + """Array capability methods raise NotImplementedError unless overridden.""" + from sqlalchemy import column + + fn = getattr(BaseEngineSpec, method) + args = (column("c"), ["v"]) if "contains" in method else (column("c"),) + with pytest.raises(NotImplementedError): + fn(*args) diff --git a/tests/unit_tests/db_engine_specs/test_clickhouse.py b/tests/unit_tests/db_engine_specs/test_clickhouse.py index 94e90327adce..ac22b949cef2 100644 --- a/tests/unit_tests/db_engine_specs/test_clickhouse.py +++ b/tests/unit_tests/db_engine_specs/test_clickhouse.py @@ -143,7 +143,30 @@ def test_connect_convert_dttm( GenericDataType.STRING, False, ), - ("Array(UInt8)", String, None, GenericDataType.STRING, False), + ("Array(UInt8)", String, None, GenericDataType.MULTI_VALUE, False), + ("Array(String)", String, None, GenericDataType.MULTI_VALUE, False), + ("Array(UInt64)", String, None, GenericDataType.MULTI_VALUE, False), + ( + "Array(LowCardinality(String))", + String, + None, + GenericDataType.MULTI_VALUE, + False, + ), + # Array(Enum(...)) is a real array and must classify as MULTI_VALUE, not + # get short-circuited by the Enum rule (the anchored ^Array\( pattern is + # ordered before the Enum entry). + ( + "Array(Enum8('a' = 1, 'b' = 2))", + String, + None, + GenericDataType.MULTI_VALUE, + False, + ), + # Arrays nested inside Map/Tuple are not top-level array columns; the + # anchored pattern must not over-match them into MULTI_VALUE. + ("Map(String, Array(String))", String, None, GenericDataType.STRING, False), + ("Tuple(Array(String))", String, None, GenericDataType.STRING, False), ("Enum('hello', 'world')", String, None, GenericDataType.STRING, False), ("Enum('UInt32', 'Bool')", String, None, GenericDataType.STRING, False), ( @@ -630,3 +653,115 @@ def test_use_equality_for_boolean_filters_property() -> None: from superset.db_engine_specs.clickhouse import ClickHouseBaseEngineSpec assert ClickHouseBaseEngineSpec.use_equality_for_boolean_filters is True + + +def _compile(expr) -> str: + return str(expr.compile(compile_kwargs={"literal_binds": True})) + + +def test_clickhouse_supports_multivalue_columns() -> None: + from superset.db_engine_specs.clickhouse import ( # noqa: N813 + ClickHouseEngineSpec as spec, + ) + + assert spec.supports_multivalue_columns is True + + +def test_multivalue_contains_any_sql() -> None: + from sqlalchemy import column + + from superset.db_engine_specs.clickhouse import ( # noqa: N813 + ClickHouseEngineSpec as spec, + ) + + expr = spec.array_contains_any(column("skills"), ["Driver", "Cook"]) + assert _compile(expr) == "hasAny(skills, array('Driver', 'Cook'))" + + +def test_multivalue_contains_all_sql() -> None: + from sqlalchemy import column + + from superset.db_engine_specs.clickhouse import ( # noqa: N813 + ClickHouseEngineSpec as spec, + ) + + expr = spec.array_contains_all(column("skills"), ["Driver", "Cook"]) + assert _compile(expr) == "hasAll(skills, array('Driver', 'Cook'))" + + +def test_multivalue_contains_binds_parameters() -> None: + """Values must be bound parameters, not inlined (SQL-injection safety).""" + from sqlalchemy import column + + from superset.db_engine_specs.clickhouse import ( # noqa: N813 + ClickHouseEngineSpec as spec, + ) + + expr = spec.array_contains_any(column("skills"), ["Driver"]) + compiled = expr.compile() + assert "Driver" not in str(compiled) + assert "Driver" in compiled.params.values() + + +def test_multivalue_length_sql() -> None: + from sqlalchemy import column + + from superset.db_engine_specs.clickhouse import ( # noqa: N813 + ClickHouseEngineSpec as spec, + ) + + expr = spec.array_length(column("skills")) + assert _compile(expr) == "length(skills)" + + +@pytest.mark.parametrize( + "native_type,expected", + [ + ("Array(String)", GenericDataType.STRING), + ("Array(Int32)", GenericDataType.NUMERIC), + ("Array(UInt64)", GenericDataType.NUMERIC), + ("Array(Decimal(10, 2))", GenericDataType.NUMERIC), + ("Array(DateTime)", GenericDataType.TEMPORAL), + ("Array(Enum8('a' = 1))", GenericDataType.STRING), + # Wrappers around the element type don't change the generic type. + ("Array(Nullable(Int64))", GenericDataType.NUMERIC), + ("Array(LowCardinality(String))", GenericDataType.STRING), + # Non-array / nested-array types have no array element type. + ("String", None), + ("Map(String, Array(String))", None), + ], +) +def test_multivalue_get_array_element_type( + native_type: str, expected: GenericDataType | None +) -> None: + from superset.db_engine_specs.clickhouse import ( # noqa: N813 + ClickHouseEngineSpec as spec, + ) + + assert spec.get_array_element_type(native_type) == expected + + +def test_multivalue_array_explode_sql() -> None: + """array_explode compiles to ``arrayJoin(col)`` (element expansion).""" + from sqlalchemy import column + + from superset.db_engine_specs.clickhouse import ( # noqa: N813 + ClickHouseEngineSpec as spec, + ) + + expr = spec.array_explode(column("scores")) + assert _compile(expr) == "arrayJoin(scores)" + + +def test_multivalue_contains_any_numeric_coercion_sql() -> None: + """Numeric-array element values must render as numbers, not quoted strings.""" + from sqlalchemy import column + + from superset.db_engine_specs.clickhouse import ( # noqa: N813 + ClickHouseEngineSpec as spec, + ) + + # Simulate values already coerced to numbers (as helpers.py does via the + # element type) and confirm the emitted array literal is numeric. + expr = spec.array_contains_any(column("scores"), [5, 6]) + assert _compile(expr) == "hasAny(scores, array(5, 6))" diff --git a/tests/unit_tests/mcp_service/dataset/tool/test_dataset_tools.py b/tests/unit_tests/mcp_service/dataset/tool/test_dataset_tools.py index 5e728c1b8e60..e62a5cf303e8 100644 --- a/tests/unit_tests/mcp_service/dataset/tool/test_dataset_tools.py +++ b/tests/unit_tests/mcp_service/dataset/tool/test_dataset_tools.py @@ -1981,6 +1981,23 @@ def test_create_virtual_dataset_request_optional_fields() -> None: assert req.description == "A virtual dataset" +def test_create_virtual_dataset_rejects_non_aggregate_saved_metric() -> None: + from pydantic import ValidationError + + with pytest.raises(ValidationError, match="saved metrics must aggregate rows"): + CreateVirtualDatasetRequest( + database_id=1, + sql="SELECT needed_operators FROM staffing", + dataset_name="Staffing", + metrics=[ + { + "metric_name": "needed_operators", + "expression": "needed_operators", + } + ], + ) + + # --- Tool logic tests --- @@ -2119,6 +2136,39 @@ async def test_create_virtual_dataset_create_failed(mcp_server: object) -> None: assert "Failed to create dataset" in data["error"] +@pytest.mark.asyncio +async def test_create_virtual_dataset_sql_error_is_actionable( + mcp_server: object, +) -> None: + """Warehouse SQL errors are recoverable tool results, not adapter crashes.""" + from superset.exceptions import SupersetGenericDBErrorException + + mock_command = MagicMock() + mock_command.run.side_effect = SupersetGenericDBErrorException( + "Invalid column name 'missing_value'" + ) + + with patch( + "superset.commands.dataset.create.CreateDatasetCommand", + return_value=mock_command, + ): + async with Client(mcp_server) as client: + request = CreateVirtualDatasetRequest( + database_id=1, + sql="SELECT missing_value FROM sample_events", + dataset_name="Test", + ) + result = await client.call_tool( + "create_virtual_dataset", {"request": request.model_dump()} + ) + data = json.loads(result.content[0].text) + + assert data["id"] is None + assert data["columns"] == [] + assert data["error"] is not None + assert "Invalid column name" in data["error"] + + @pytest.mark.asyncio async def test_create_virtual_dataset_permission_denied(mcp_server: object) -> None: """SQL access denied surfaces as DatasetInvalidError with id=None.""" @@ -2289,7 +2339,13 @@ async def test_create_virtual_dataset_update_failure_rollback( if exception_to_raise == "DatasetUpdateFailedError": mock_update_instance.run.side_effect = DatasetUpdateFailedError() else: - mock_update_instance.run.side_effect = DatasetInvalidError() + from superset.commands.dataset.exceptions import ( + DatasetColumnsExistsValidationError, + ) + + invalid_error = DatasetInvalidError() + invalid_error.append(DatasetColumnsExistsValidationError()) + mock_update_instance.run.side_effect = invalid_error mock_update_cls = MagicMock(return_value=mock_update_instance) mock_delete_instance = MagicMock() @@ -2336,7 +2392,11 @@ async def test_create_virtual_dataset_update_failure_rollback( # Verify the error response data = json.loads(result.content[0].text) assert data["id"] is None - assert "creation rolled back" in data["error"] + if exception_to_raise == "DatasetInvalidError": + assert "columns" in data["error"] + assert "already exist" in data["error"] + else: + assert "creation rolled back" in data["error"] @pytest.mark.asyncio diff --git a/tests/unit_tests/models/test_multivalue_filter.py b/tests/unit_tests/models/test_multivalue_filter.py new file mode 100644 index 000000000000..675ab9003e71 --- /dev/null +++ b/tests/unit_tests/models/test_multivalue_filter.py @@ -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. +"""Element-level array filter operators (Contains Any/All, Is empty/not empty).""" + +from __future__ import annotations + +from typing import Any, cast + +import pytest +from flask import Flask +from pytest_mock import MockerFixture + +from superset.connectors.sqla.models import SqlaTable, SqlMetric, TableColumn +from superset.exceptions import QueryObjectValidationError +from superset.models.core import Database +from superset.superset_typing import QueryObjectDict +from superset.utils.core import FilterOperator + + +def _make_dataset(mocker: MockerFixture) -> SqlaTable: + database = Database(id=1, database_name="test_db", sqlalchemy_uri="sqlite://") + columns = [ + TableColumn(column_name="skills", type="Array(String)"), + TableColumn(column_name="scores", type="Array(Int32)"), + TableColumn(column_name="city", type="VARCHAR(100)"), + ] + dataset = SqlaTable( + table_name="jobs", + columns=columns, + database=database, + metrics=[SqlMetric(metric_name="count", expression="COUNT(*)")], + ) + mocker.patch( + "superset.connectors.sqla.models.security_manager.get_guest_rls_filters", + return_value=[], + ) + mocker.patch( + "superset.connectors.sqla.models.security_manager.is_guest_user", + return_value=False, + ) + return dataset + + +def _clickhouse(mocker: MockerFixture, dataset: SqlaTable) -> None: + # Imported lazily: clickhouse.py touches app.config at import time. + from superset.db_engine_specs.clickhouse import ClickHouseEngineSpec + + mocker.patch.object( + SqlaTable, "db_engine_spec", new=property(lambda self: ClickHouseEngineSpec) + ) + + +def _filter_query(filters: list[dict[str, Any]]) -> QueryObjectDict: + return cast( + QueryObjectDict, + { + "granularity": None, + "from_dttm": None, + "to_dttm": None, + "is_timeseries": False, + "groupby": ["city"], + "metrics": ["count"], + "filter": filters, + "columns": [], + }, + ) + + +def _sql(dataset: SqlaTable, filters: list[dict[str, Any]]) -> str: + return dataset.get_query_str_extended( + _filter_query(filters), mutate=False + ).sql.lower() + + +def test_contains_any_generates_hasany(mocker: MockerFixture, app: Flask) -> None: + dataset = _make_dataset(mocker) + _clickhouse(mocker, dataset) + with app.test_request_context(): + sql = _sql( + dataset, + [ + { + "col": "skills", + "op": FilterOperator.CONTAINS_ANY.value, + "val": ["Driver", "Cook"], + } + ], + ) + assert "hasany(skills, array('driver', 'cook'))" in sql + + +def test_contains_any_numeric_array_coerces_values( + mocker: MockerFixture, app: Flask +) -> None: + """Values for a numeric array must render as numbers, not quoted strings.""" + dataset = _make_dataset(mocker) + _clickhouse(mocker, dataset) + with app.test_request_context(): + sql = _sql( + dataset, + [ + { + "col": "scores", + "op": FilterOperator.CONTAINS_ANY.value, + "val": ["5", "6"], + } + ], + ) + assert "hasany(scores, array(5, 6))" in sql + assert "'5'" not in sql + + +def test_equals_numeric_array_coerces_values(mocker: MockerFixture, app: Flask) -> None: + """Whole-array equality on a numeric array coerces the parsed literal.""" + dataset = _make_dataset(mocker) + _clickhouse(mocker, dataset) + with app.test_request_context(): + sql = _sql( + dataset, + [ + { + "col": "scores", + "op": FilterOperator.EQUALS.value, + "val": "[5, 6]", + } + ], + ) + assert "scores = array(5, 6)" in sql + + +def test_contains_all_generates_hasall(mocker: MockerFixture, app: Flask) -> None: + dataset = _make_dataset(mocker) + _clickhouse(mocker, dataset) + with app.test_request_context(): + sql = _sql( + dataset, + [ + { + "col": "skills", + "op": FilterOperator.CONTAINS_ALL.value, + "val": ["Driver", "Cook"], + } + ], + ) + assert "hasall(skills, array('driver', 'cook'))" in sql + + +def test_is_empty_generates_length_zero(mocker: MockerFixture, app: Flask) -> None: + dataset = _make_dataset(mocker) + _clickhouse(mocker, dataset) + with app.test_request_context(): + sql = _sql(dataset, [{"col": "skills", "op": FilterOperator.IS_EMPTY.value}]) + assert "length(skills) = 0" in sql + + +def test_is_not_empty_generates_length_gt_zero( + mocker: MockerFixture, app: Flask +) -> None: + dataset = _make_dataset(mocker) + _clickhouse(mocker, dataset) + with app.test_request_context(): + sql = _sql( + dataset, [{"col": "skills", "op": FilterOperator.IS_NOT_EMPTY.value}] + ) + assert "length(skills) > 0" in sql + + +def test_contains_resolves_to_hasany(mocker: MockerFixture, app: Flask) -> None: + dataset = _make_dataset(mocker) + _clickhouse(mocker, dataset) + with app.test_request_context(): + sql = _sql( + dataset, + [ + { + "col": "skills", + "op": FilterOperator.CONTAINS_ANY.value, + "val": ["Driver"], + } + ], + ) + assert "hasany(skills" in sql + + +def test_element_op_on_scalar_column_raises(mocker: MockerFixture, app: Flask) -> None: + """CONTAINS_ANY on a scalar column is rejected on an array-capable engine.""" + dataset = _make_dataset(mocker) + _clickhouse(mocker, dataset) + with app.test_request_context(): # noqa: SIM117 + with pytest.raises(QueryObjectValidationError): + _sql( + dataset, + [ + { + "col": "city", + "op": FilterOperator.CONTAINS_ANY.value, + "val": ["NYC"], + } + ], + ) + + +def test_element_op_unsupported_engine_raises( + mocker: MockerFixture, app: Flask +) -> None: + """On an engine without array support (sqlite) the array op is rejected.""" + dataset = _make_dataset(mocker) + with app.test_request_context(): # noqa: SIM117 + with pytest.raises(QueryObjectValidationError): + _sql(dataset, [{"col": "skills", "op": FilterOperator.IS_EMPTY.value}]) + + +def test_equals_on_array_parses_literal(mocker: MockerFixture, app: Flask) -> None: + """A pasted array literal for = is parsed into col = array(...).""" + dataset = _make_dataset(mocker) + _clickhouse(mocker, dataset) + with app.test_request_context(): + sql = _sql( + dataset, + [ + { + "col": "skills", + "op": FilterOperator.EQUALS.value, + "val": "['Driver', 'Cook']", + } + ], + ) + assert "skills = array('driver', 'cook')" in sql + + +def test_equals_on_array_plain_value_fallback( + mocker: MockerFixture, app: Flask +) -> None: + """A plain (non-bracketed) value becomes a single-element array.""" + dataset = _make_dataset(mocker) + _clickhouse(mocker, dataset) + with app.test_request_context(): + sql = _sql( + dataset, + [{"col": "skills", "op": FilterOperator.EQUALS.value, "val": "Driver"}], + ) + assert "skills = array('driver')" in sql + + +def test_in_on_array_parses_literals(mocker: MockerFixture, app: Flask) -> None: + """Whole-array IN parses each pasted array literal into its own array.""" + dataset = _make_dataset(mocker) + _clickhouse(mocker, dataset) + with app.test_request_context(): + sql = _sql( + dataset, + [ + { + "col": "skills", + "op": FilterOperator.IN.value, + "val": ["['Driver']", "['Cook']"], + } + ], + ) + assert "skills in (array('driver'), array('cook'))" in sql diff --git a/tests/unit_tests/models/test_multivalue_length.py b/tests/unit_tests/models/test_multivalue_length.py new file mode 100644 index 000000000000..d8e2181340e6 --- /dev/null +++ b/tests/unit_tests/models/test_multivalue_length.py @@ -0,0 +1,130 @@ +# 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. +"""The Length array filter operators: length(col) compared to a number.""" + +from __future__ import annotations + +from typing import Any + +import pytest +from flask import Flask +from pytest_mock import MockerFixture + +from superset.connectors.sqla.models import SqlaTable, SqlMetric, TableColumn +from superset.exceptions import QueryObjectValidationError +from superset.models.core import Database +from superset.superset_typing import QueryObjectDict +from superset.utils.core import FilterOperator + + +def _make_dataset(mocker: MockerFixture) -> SqlaTable: + database = Database(id=1, database_name="test_db", sqlalchemy_uri="sqlite://") + columns = [ + TableColumn(column_name="skills", type="Array(String)"), + TableColumn(column_name="city", type="VARCHAR(100)"), + ] + dataset = SqlaTable( + table_name="jobs", + columns=columns, + database=database, + metrics=[SqlMetric(metric_name="count", expression="COUNT(*)")], + ) + mocker.patch( + "superset.connectors.sqla.models.security_manager.get_guest_rls_filters", + return_value=[], + ) + mocker.patch( + "superset.connectors.sqla.models.security_manager.is_guest_user", + return_value=False, + ) + return dataset + + +def _clickhouse(mocker: MockerFixture, dataset: SqlaTable) -> None: + from superset.db_engine_specs.clickhouse import ClickHouseEngineSpec + + mocker.patch.object( + SqlaTable, "db_engine_spec", new=property(lambda self: ClickHouseEngineSpec) + ) + + +def _sql(dataset: SqlaTable, op: str, val: Any, col: str = "skills") -> str: + query: QueryObjectDict = { + "granularity": None, + "from_dttm": None, + "to_dttm": None, + "is_timeseries": False, + "groupby": ["city"], + "metrics": ["count"], + "filter": [{"col": col, "op": op, "val": val}], + "columns": [], + } + return dataset.get_query_str_extended(query, mutate=False).sql.lower() + + +@pytest.mark.parametrize( + "op,expected", + [ + (FilterOperator.LENGTH_EQUALS, "length(skills) = 3"), + (FilterOperator.LENGTH_GREATER_THAN, "length(skills) > 3"), + (FilterOperator.LENGTH_LESS_THAN, "length(skills) < 3"), + (FilterOperator.LENGTH_GREATER_THAN_OR_EQUALS, "length(skills) >= 3"), + (FilterOperator.LENGTH_LESS_THAN_OR_EQUALS, "length(skills) <= 3"), + ], +) +def test_length_operators_generate_length_comparison( + mocker: MockerFixture, app: Flask, op: FilterOperator, expected: str +) -> None: + dataset = _make_dataset(mocker) + _clickhouse(mocker, dataset) + with app.test_request_context(): + sql = _sql(dataset, op.value, 3) + assert expected in sql + + +def test_length_accepts_string_number(mocker: MockerFixture, app: Flask) -> None: + """A numeric string value is coerced (e.g. '2' -> length(col) > 2).""" + dataset = _make_dataset(mocker) + _clickhouse(mocker, dataset) + with app.test_request_context(): + sql = _sql(dataset, FilterOperator.LENGTH_GREATER_THAN.value, "2") + assert "length(skills) > 2" in sql + + +def test_length_non_numeric_value_raises(mocker: MockerFixture, app: Flask) -> None: + dataset = _make_dataset(mocker) + _clickhouse(mocker, dataset) + with app.test_request_context(): # noqa: SIM117 + with pytest.raises(QueryObjectValidationError): + _sql(dataset, FilterOperator.LENGTH_EQUALS.value, "abc") + + +def test_length_on_scalar_column_raises(mocker: MockerFixture, app: Flask) -> None: + """Length on a scalar column is rejected even on an array-capable engine.""" + dataset = _make_dataset(mocker) + _clickhouse(mocker, dataset) + with app.test_request_context(): # noqa: SIM117 + with pytest.raises(QueryObjectValidationError): + _sql(dataset, FilterOperator.LENGTH_GREATER_THAN.value, 1, col="city") + + +def test_length_unsupported_engine_raises(mocker: MockerFixture, app: Flask) -> None: + """On an engine without array support (sqlite) the length op is rejected.""" + dataset = _make_dataset(mocker) + with app.test_request_context(): # noqa: SIM117 + with pytest.raises(QueryObjectValidationError): + _sql(dataset, FilterOperator.LENGTH_GREATER_THAN.value, 1)