[OPIK-5431] [BE] Fix IS_EMPTY/IS_NOT_EMPTY filters for DICTIONARY field type#6042
Merged
thiagohora merged 4 commits intomainfrom Apr 2, 2026
Merged
Conversation
…ld type
Filtering experiments (or any entity) by a metadata/dictionary field using
the IS_EMPTY or IS_NOT_EMPTY operators was returning HTTP 400 because:
1. FilterQueryBuilder had no SQL templates for those operator+type combos.
2. FiltersFactory validation rejected the filter when value was null/empty
for DICTIONARY/DICTIONARY_STATE_DB types.
3. FiltersFactory tried to URL-decode a null value, causing a NullPointerException.
Fix:
- Add JSON_EXISTS-based SQL templates to FilterQueryBuilder for both operators
on DICTIONARY and DICTIONARY_STATE_DB:
IS_EMPTY → JSON_EXISTS(col, :filterKey) = false
IS_NOT_EMPTY → JSON_EXISTS(col, :filterKey) = true
This correctly distinguishes "key not present" from "key present but empty".
- Update FIELD_TYPE_VALIDATION_MAP in FiltersFactory to accept IS_EMPTY /
IS_NOT_EMPTY with only a key (no value required).
- Guard the URL-decode step in toValidAndDecoded() to skip NO_VALUE_OPERATORS,
preventing NPE when filter.value() is null.
Add integration tests in ExperimentsResourceFindProjectExperimentsTest:
- IS_NOT_EMPTY on $.config.name returns the experiment that has the key.
- IS_EMPTY on $.config.name returns the experiment whose metadata is null
(key not present at all).
Implements OPIK-5431: [BE] Grouping experiments by configuration doesn't show
results for Undefined group
github-actions
bot
added
java
Contributor
Backend Tests - Integration Group 5 26 files - 4 26 suites - 4 2m 54s ⏱️ +20s Results for commit 340aba2. ± Comparison against base commit 94c82d8. This pull request removes 17 and adds 54 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
Backend Tests - Integration Group 11221 tests - 4 221 ✅ - 4 4m 43s ⏱️ +9s Results for commit 024cf49. ± Comparison against base commit 94c82d8. This pull request removes 6 and adds 2 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
apps/opik-backend/src/main/java/com/comet/opik/api/filter/FiltersFactory.java
Show resolved
Hide resolved
apps/opik-backend/src/main/java/com/comet/opik/domain/filter/FilterQueryBuilder.java
Show resolved
Hide resolved
Contributor
|
🔄 Test environment deployment process has started Phase 1: Deploying base version You can monitor the progress here. |
Collaborator
|
✅ Test environment is now available! To configure additional Environment variables for your environment, run [Deploy Opik AdHoc Environment workflow] (https://github.com/comet-ml/comet-deployment/actions/workflows/deploy_opik_adhoc_env.yaml) Access Information
The deployment has completed successfully and the version has been verified. |
Collaborator
|
🌙 Nightly cleanup: The test environment for this PR ( |
…dictionary-fields
…dictionary-fields
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
TS SDK E2E Tests - Node 200 tests ±0 0 ✅ ±0 0s ⏱️ ±0s For more details on these parsing errors, see this check. Results for commit 024cf49. ± Comparison against base commit 94c82d8. |
Contributor
TS SDK E2E Tests - Node 220 tests ±0 0 ✅ ±0 0s ⏱️ ±0s For more details on these parsing errors, see this check. Results for commit 024cf49. ± Comparison against base commit 94c82d8. |
Contributor
TS SDK E2E Tests - Node 180 tests ±0 0 ✅ ±0 0s ⏱️ ±0s For more details on these parsing errors, see this check. Results for commit 024cf49. ± Comparison against base commit 94c82d8. |
…andle null and empty values - IS_EMPTY now matches: key absent, empty string value, or JSON null value - IS_NOT_EMPTY now requires: key present with a non-empty, non-null value - JSON_VALUE returns the string 'null' for JSON null in ClickHouse (not SQL NULL), so the condition checks for both '' and 'null' explicitly - Updated findByFilterMetadataEmpty test to use assertExperiments - Added findByFilterMetadataEmptyWithNullAndBlankValues test covering the key-with-null-value and key-with-empty-string-value cases
...java/com/comet/opik/api/resources/v1/priv/ExperimentsResourceFindProjectExperimentsTest.java
Show resolved
Hide resolved
BorisTkachenko
approved these changes
Apr 2, 2026
thiagohora
deleted the
thiaghora/OPIK-5431-fix-is-empty-filter-for-dictionary-fields
branch
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Details
Filtering experiments by a metadata/dictionary field using the
IS_EMPTYorIS_NOT_EMPTYoperators was returning HTTP 400. This prevented the "Undefined" group from showing results when grouping experiments by configuration in the UI.Root causes:
FilterQueryBuilderhad no SQL templates for theIS_EMPTY/IS_NOT_EMPTYoperator +DICTIONARY/DICTIONARY_STATE_DBtype combinations — causing anulloperator lookup and a 400 response.FiltersFactoryvalidation rejected these filters because value wasnull/empty forDICTIONARY/DICTIONARY_STATE_DBtypes (those operators don't require a value, only a key).FiltersFactory.toValidAndDecoded()tried toURLDecoder.decode(null, ...), causing aNullPointerExceptionfor no-value operators.Fix:
JSON_EXISTS-based SQL templates inFilterQueryBuilderfor both operators:IS_EMPTY→JSON_EXISTS(col, :filterKey) = false(key not present)IS_NOT_EMPTY→JSON_EXISTS(col, :filterKey) = true(key present)JSON_EXISTScorrectly distinguishes "key not present" from "key present with empty string value".FIELD_TYPE_VALIDATION_MAPinFiltersFactoryto acceptIS_EMPTY/IS_NOT_EMPTYwith only a key (no value required) forDICTIONARYandDICTIONARY_STATE_DBtypes.FiltersFactory.toValidAndDecoded()to skipNO_VALUE_OPERATORS, preventing NPE whenfilter.value()is null.Change checklist
Issues
Testing
Added integration tests in
ExperimentsResourceFindProjectExperimentsTest#findByFilterMetadataEmpty:IS_NOT_EMPTYon key$.config.name→ returns the experiment that has the key in metadata.IS_EMPTYon key$.config.name→ returns the experiment whose metadata isnull(key not present at all).To reproduce manually:
metadata = {"config": {"name": "simulated"}}, one with no metadata.metadata.config.name— the "Undefined" group should appear and contain the experiment without the key.[{"field":"metadata","type":"dictionary","operator":"is_empty","key":"$.config.name"}]— previously returned 400, now returns the correct experiment.Documentation
N/A — internal filter system fix, no configuration or API contract changes.