-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Fix: Advanced search bugs for the custom properties #25232
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| } | ||
|
|
||
| if (value && Array.isArray(value[0])) { | ||
| // Check if this is a multiselect equals operator that should use AND logic | ||
| const useAndLogic = | ||
| operator === 'multiselect_equals' || | ||
| operator === 'multiselect_not_equals'; | ||
|
|
||
| return { | ||
| bool: { | ||
| should: value[0].map((val) => | ||
| [useAndLogic ? 'must' : 'should']: value[0].map((val) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Details
The change from should to must for multiselect_equals and multiselect_not_equals operators fundamentally changes the search behavior:
- Before: When selecting multiple values (e.g., tags A and B), items matching A OR B would be returned
- After: When selecting multiple values, only items matching A AND B will be returned
This is a breaking behavioral change that may significantly impact user expectations for multiselect filters. While this might be the intended fix for one of the referenced issues, it's important to verify:
- This matches the expected behavior from the issue requirements
- Existing users/documentation reflect this change
- Consider if this should be configurable based on use case
The same change appears in both elasticSearchFormat and elasticSearchFormatForJSONLogic functions, which is consistent.
Was this helpful? React with 👍 / 👎
|
The DetailsThe failure is caused by The validation script at This is a pre-existing issue in the main branch and affects all PRs currently, not just this one. |
🔍 CI failure analysis for 84fb1fd: CI failures show 37 tests timing out on Time Interval custom properties UI interactions, along with 17 flaky tests for other custom property types (Email, Timestamp, Entity Reference, Date, DateTime) and domain/entity operations.Issue 1: Custom Property Advanced Search Test Failure (Related to PR)The playwright test Root CauseThe PR modified let subfieldsKey: string;
if (
field.type === 'array<entityReference>' ||
field.type === 'entityReference'
) {
subfieldsKey = field.name + '.displayName.keyword';
} else if (
field.type === 'number' ||
field.type === 'integer' ||
field.type === 'timestamp'
) {
subfieldsKey = field.name;
} else {
subfieldsKey = field.name + '.keyword'; // String types get .keyword suffix
}The test at But the actual output now includes the DetailsThis failure is directly related to the PR changes. The PR refactored custom property handling in advanced search and introduced logic to append The solution is to update the test expectation to include the Issue 2: Time Interval Custom Properties Tests - Widespread Timeout Failures (Likely Related to PR)37 tests failed with timeout errors (540000ms exceeded) when attempting to add Time Interval custom properties across 18 entity types (container, dashboard, database, databaseSchema, glossaryTerm, mlmodel, pipeline, searchIndex, storedProcedure, table, topic, apiCollection, apiEndpoint, dataProduct, domain, dashboardDataModel, metric, chart). Root CauseThe PR introduces significant refactoring to custom property handling in advanced search:
All 37 failures show the same pattern: This suggests the Time Interval custom property UI component is either:
DetailsThis failure pattern is likely related to the PR changes. The refactoring moves custom property handling logic from
The solution should be to investigate the
Issue 3: JSON/YAML Validation Failure (Unrelated to PR)The Root CauseThe validation script The file is documented in the test data README as:
DetailsThis failure is unrelated to the PR changes. The PR modifies advanced search functionality, utilities, and translations but does not touch The validation script currently excludes only EXCLUDED_DIRS=".vscode|great_expectations/resources"The solution is to expand the exclusion pattern to include test data directories: EXCLUDED_DIRS=".vscode|great_expectations/resources|playwright/test-data"This will allow intentionally invalid test fixtures to remain in the repository for testing purposes while still validating production JSON/YAML files. Issue 4: Flaky Tests - Custom Properties and Entity Operations (Possibly Related to PR)17 flaky tests failed intermittently across various custom property types and entity operations:
Root CauseThese tests failed with various timeout and element visibility issues:
DetailsThese failures are possibly related to the PR changes. While the error patterns suggest test flakiness, the fact that:
suggests that the refactoring may have introduced timing issues or race conditions in how custom properties are loaded and displayed in the UI. The custom property processing now happens through a different code path ( The remaining flaky tests (domain rename, data contracts, entity operations) appear to be environmental issues in the CI run, as they are unrelated to advanced search or custom property functionality modified by this PR. Code Review 👍 Approved with suggestions 1 resolved / 3 findingsComprehensive advanced search improvements for custom properties with good test coverage. The behavioral change from OR to AND logic for multiselect_equals is intentional but should be verified with product requirements.
|
| Auto-apply | Compact |
|
|
Was this helpful? React with 👍 / 👎 | Gitar
|



Fixes #17001
Fixes #21600
Fixes #22939
Fixes https://github.com/open-metadata/openmetadata-collate/issues/2539
This pull request introduces several improvements and refactors to the advanced search functionality, operator constants, and internationalization files. The main highlights are the refactoring of custom property handling in advanced search, the addition of new search operators, and the expansion of translation keys in multiple languages.
Advanced Search Refactoring and Improvements
processEntityTypeFieldsutility function, simplifying the code and improving maintainability inAdvanceSearchProvider.component.tsx. [1] [2] [3]AdvanceSearchProvider.component.tsx.Operator Constants Enhancements
is_null,is_not_null) toMULTISELECT_FIELD_OPERATORSandNUMBER_FIELD_OPERATORS, and definedNUMBER_FIELD_OPERATORSto better support null checks and range queries inAdvancedSearch.constants.ts.Type and Interface Updates
AggregationEntryandBucketinterface fromsearch.interface.ts, and importingBucketfrom theModelsmodule instead. [1] [2]Internationalization (i18n) Updates