Fixes #27482: Fix Advanced Search between boundaries for custom numerical properties by aniruddhaadak80 · Pull Request #27486 · open-metadata/OpenMetadata

aniruddhaadak80 · 2026-04-17T14:49:02Z

Describe your changes:

I worked on QueryBuilderElasticsearchFormatUtils.js because the RAQB between boundaries (i.e. [10, 20]) were incorrectly stripped of the upper bound during buildExtensionQuery. This passes the entire bounding Array correctly, repairing Advanced Search filtering.

Type of change:

Checklist:

I have read the CONTRIBUTING document.
My PR title is Fixes <issue-number>: <short explanation>
I have commented on my code, particularly in hard-to-understand areas.
For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

Summary by Gitar

Refactored type resolution:
- Introduced getFieldTypeInfoFromOmType and lookupOmPropertyType to use registry configuration for unambiguous field mapping.
Improved precision:
- Updated field identification logic from includes() to endsWith() to prevent misclassification of properties containing dots in their names.
- Enhanced numeric value detection in buildExtensionQuery by leveraging omPropertyType metadata instead of relying solely on value-shape parsing.

_{This will update automatically on new commits.}

github-actions · 2026-04-17T14:49:26Z

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

Copilot

Pull request overview

Fixes Advanced Search formatting for custom numeric property between filters by ensuring the full boundary array is passed through to the extension/customPropertiesTyped query builder, so Elasticsearch DSL can include both gte and lte.

Changes:

Pass the complete value array (instead of value[0]) when building extension-field queries, allowing between to retain both bounds.

Copilot · 2026-04-17T14:52:41Z

    return buildExtensionQuery(
      extensionPropertyName,
      entityType,
-      hasValue ? value[0] : null,
+      hasValue ? value : null,
      op,
      not
    );


This change fixes the range-boundary bug, but there’s no regression test covering the advanced-search → Elasticsearch DSL formatting for extension numeric properties with the between operator (i.e., verifying both gte and lte are emitted and change when the second boundary changes). Please add a unit test around elasticSearchFormatForJSONLogic/buildExtensionQuery for an extension.<entity>.<property> field using between to prevent this from regressing again.

aniruddhaadak80 · 2026-04-17T14:59:04Z

hi maintainers could someone please add the safe to test label so the pipelines can run, thank you

github-actions · 2026-04-21T17:11:05Z

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

aniruddhaadak80

Added explicit unit test for advanced-search extension numeric properties with the between operator as requested. Synced up with main.

github-actions · 2026-04-21T17:21:33Z

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Copilot · 2026-04-21T17:26:26Z

@@ -0,0 +1,45 @@
+import { elasticSearchFormatForJSONLogic } from './QueryBuilderElasticsearchFormatUtils';


New test file is missing the standard Apache 2.0 license header block that other UI source/test files include (e.g., APIUtils.test.js). Please add the repository’s license header at the top of this file to satisfy license-header checks.

Copilot · 2026-04-21T17:26:27Z

+    };
+
+    // The method elasticSearchFormatForJSONLogic internally uses tree.get() 
+    const result = elasticSearchFormatForJSONLogic(tree, config);


This test calls elasticSearchFormatForJSONLogic(), but that function currently references an undeclared extendConfig (it isn’t imported/defined in QueryBuilderElasticsearchFormatUtils.js). In a Jest/module context this will throw and the catch block will return {}, making the assertions fail or the test not exercise the real formatting logic. Either switch the test to use elasticSearchFormat() (the function used by the app), or update elasticSearchFormatForJSONLogic to use extendConfigUtils.ConfigUtils.extendConfig like elasticSearchFormat does.

github-actions · 2026-04-22T05:40:23Z

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

gitar-bot · 2026-04-22T05:41:40Z

Code Review ✅ Approved 2 resolved / 2 findings

Refactored Advanced Search logic to correctly handle boundaries for numerical properties, addressing fragile string matching and adding coverage for the missing not_between operator.

✅ 2 resolved

✅ Quality: Test uses fragile string matching instead of structure assertions

📄 openmetadata-ui/src/main/resources/ui/src/utils/QueryBuilderElasticsearchFormatUtils.test.js:38-43
The test serializes the result to JSON and uses toContain on the string to verify the query structure. This is brittle — it will pass even if gte/lte appear in unexpected places in the output, and it will break if JSON key ordering or whitespace changes.

Consider asserting on the actual object structure instead, which is more readable and gives better failure messages.

✅ Edge Case: Missing test for not_between operator

📄 openmetadata-ui/src/main/resources/ui/src/utils/QueryBuilderElasticsearchFormatUtils.test.js:1-15
The not_between operator is also a range operator that uses the same array value pattern as between. It would be good to add a test case for it to ensure the negation logic works correctly with the full value array now being passed through.

Options

Display: compact → Showing less information.

Comment with these commands to change:

`Compact`
`gitar display:verbose`

_{Was this helpful? React with 👍 / 👎 | Gitar}

aniruddhaadak80 requested a review from a team as a code owner April 17, 2026 14:49

Copilot AI review requested due to automatic review settings April 17, 2026 14:49

Copilot started reviewing on behalf of aniruddhaadak80 April 17, 2026 14:49 View session

Copilot AI reviewed Apr 17, 2026

View reviewed changes

aniruddhaadak80 changed the title ~~Fix Advanced Search 'between' boundaries for custom numerical properties (#27482)~~ Fixes #27482: Fix Advanced Search between boundaries for custom numerical properties Apr 17, 2026

aniruddhaadak80 commented Apr 21, 2026

View reviewed changes

gitar-bot Bot reviewed Apr 21, 2026

View reviewed changes

Comment thread openmetadata-ui/src/main/resources/ui/src/utils/QueryBuilderElasticsearchFormatUtils.test.js Outdated

gitar-bot Bot reviewed Apr 21, 2026

View reviewed changes

Comment thread openmetadata-ui/src/main/resources/ui/src/utils/QueryBuilderElasticsearchFormatUtils.test.js Outdated

Copilot AI review requested due to automatic review settings April 21, 2026 17:21

Copilot started reviewing on behalf of aniruddhaadak80 April 21, 2026 17:21 View session

Copilot AI reviewed Apr 21, 2026

View reviewed changes

Fix Advanced Search between operator bounds (open-metadata#27482)

0e77d3d

aniruddhaadak80 force-pushed the fix-advanced-search-between-number-27482 branch from 0974b36 to 0e77d3d Compare April 22, 2026 05:40

		@@ -0,0 +1,45 @@
		import { elasticSearchFormatForJSONLogic } from './QueryBuilderElasticsearchFormatUtils';

Conversation

aniruddhaadak80 commented Apr 17, 2026 • edited by gitar-bot Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe your changes:

Type of change:

Checklist:

Summary by Gitar

Uh oh!

github-actions Bot commented Apr 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

aniruddhaadak80 commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Apr 21, 2026

Uh oh!

aniruddhaadak80 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Apr 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Apr 22, 2026

Uh oh!

gitar-bot Bot commented Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aniruddhaadak80 commented Apr 17, 2026 •

edited by gitar-bot Bot

Loading

aniruddhaadak80 commented Apr 17, 2026 •

edited

Loading

gitar-bot Bot commented Apr 22, 2026 •

edited

Loading