Fixes #27482: Advanced Search between filter for number custom properties#27497
Fixes #27482: Advanced Search between filter for number custom properties#27497himanshu748 wants to merge 2 commits intoopen-metadata:mainfrom
Conversation
Prevent range operators (between/not_between) from being treated like multiselect values and ensure both bounds are passed through when building customPropertiesTyped queries. Made-with: Cursor
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
There was a problem hiding this comment.
Pull request overview
Fixes Advanced Search Elasticsearch query generation for custom property between (range) filters by preventing nested array values from being incorrectly expanded into multiple rules, ensuring both bounds are preserved.
Changes:
- Restrict nested-array expansion to multiselect/select operators only, avoiding range operator (
between) corruption. - Preserve full
[from, to]bounds when building custom property extension queries. - Add a unit test asserting
gte+lteare present for a number custom propertybetweenfilter.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| openmetadata-ui/src/main/resources/ui/src/utils/QueryBuilderElasticsearchFormatUtils.js | Adjusts nested-array handling so between values are not split into separate rules; ensures bounded range queries for custom properties. |
| openmetadata-ui/src/main/resources/ui/src/utils/QueryBuilderElasticsearchFormatUtils.test.ts | Adds unit coverage for bounded range query generation for number custom properties using between. |
| import { SearchIndex } from '../enums/search.enum'; | ||
| import { SearchOutputType } from '../generated/type/searchOutputType'; |
There was a problem hiding this comment.
SearchOutputType is imported from ../generated/type/searchOutputType, but there is no such generated file in the repo and the app code uses the enum from components/Explore/AdvanceSearchProvider/AdvanceSearchProvider.interface. This will break the test build and also risks comparing different enum instances in getTreeConfig.
| import { SearchIndex } from '../enums/search.enum'; | |
| import { SearchOutputType } from '../generated/type/searchOutputType'; | |
| import { SearchOutputType } from '../components/Explore/AdvanceSearchProvider/AdvanceSearchProvider.interface'; | |
| import { SearchIndex } from '../enums/search.enum'; |
| const config = getTreeConfig({ | ||
| searchOutputType: SearchOutputType.ElasticSearch, | ||
| searchIndex: SearchIndex.TABLE, | ||
| isExplorePage: true, | ||
| }); | ||
|
|
||
| const jsonTree = { | ||
| id: 'root', | ||
| type: 'group', | ||
| properties: { conjunction: 'AND', not: false }, | ||
| children1: { | ||
| rules: { | ||
| type: 'rule', | ||
| id: 'rules', | ||
| properties: { | ||
| field: 'extension.table.myNumberProperty', | ||
| operator: 'between', | ||
| value: [[1, 5]], | ||
| valueSrc: ['value'], |
There was a problem hiding this comment.
The base config returned by getTreeConfig() initializes fields.extension.subfields as {} (custom properties are injected later at runtime). With no extension.table.myNumberProperty field present, QbUtils.checkTree() is likely to drop/neutralize this rule, making the test unreliable. Consider injecting minimal config.fields.extension.subfields for table -> myNumberProperty (type number, operators NUMBER_FIELD_OPERATORS) before calling checkTree.
|
Hi! This PR is ready for CI. Could a maintainer please add the |
Made-with: Cursor
|
Addressed Copilot review notes in the new unit test: (1) import |
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
Code Review ✅ ApprovedAdvanced search functionality now supports range filtering for number-based custom properties, addressing the requested feature requirements. No issues found. OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
Describe your changes:
Fixes #27482
Advanced Search was incorrectly handling nested array values for range operators on custom properties. The Query Builder represents
betweenvalues as[[from, to]], but the formatter treated any nested array as a multiselect and split it into multiple rules. This dropped the upper bound and produced an effectively unbounded query.This change:
[from, to]bounds through to the customPropertiesTyped range builderTesting
gteandltefor a number custom propertybetweenfilter.Type of change:
Checklist:
Hackathon / AI disclosure
Built with Cursor (AI-assisted) and manually reviewed.
Made with Cursor