[OPIK-6884] [BE] refactor: SELECT DISTINCT trace_id in dataset-item filter-gate bounds#7418
Merged
Merged
Conversation
Review (andrescrz): dedupe the trace-id set in the gate bounds and their project-resolution subqueries. DISTINCT inside an IN(...) is result-equivalent (membership is unchanged) but yields a smaller prepared set.
Contributor
⏱️ pre-commit per-hook timing
⏭️ 40 skipped (no matching files changed)
|
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
Comment on lines
+351
to
+352
| AND id IN (SELECT DISTINCT trace_id FROM experiment_items WHERE workspace_id = :workspace_id AND experiment_id IN :experimentIds) | ||
| AND project_id IN (SELECT DISTINCT project_id FROM traces WHERE workspace_id = :workspace_id AND id IN (SELECT DISTINCT trace_id FROM experiment_items WHERE workspace_id = :workspace_id AND experiment_id IN :experimentIds)) |
Contributor
There was a problem hiding this comment.
CamelCase ClickHouse binds violate rule
:.agents/skills/opik-backend/clickhouse.md requires ClickHouse parameter names to be snake_case, but this query path still uses :experimentIds in the placeholder and bind sites, so it stays non-compliant — should we rename them to experiment_ids here and in the updated stats query too?
Want Baz to fix this for you? Activate Fixer You can also update your AI coding guidelines based on this comment by apply pr to [branch name]
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
apps/opik-backend/src/main/java/com/comet/opik/domain/DatasetItemDAO.java around lines
351-352 inside the SQL template SELECT_DATASET_ITEMS_WITH_EXPERIMENT_ITEMS_COUNT, the
added trace gate filters still reference :experimentIds (camelCase). Refactor this hunk
to use the snake_case ClickHouse placeholder :experiment_ids and then update all
corresponding bind sites in the Java methods that execute this query (e.g.,
getItems/getCount) to bind the parameter under the same name (experiment_ids) instead of
experimentIds. Also apply the same rename in
SELECT_DATASET_ITEMS_WITH_EXPERIMENT_ITEMS_STATS around lines 853-854 where the same
subquery pattern repeats, ensuring its execution path binds experiment_ids consistently.
Member
There was a problem hiding this comment.
It's a nit from Baz, but valid. Optionally fee free to address it.
andrescrz
approved these changes
Jul 10, 2026
Comment on lines
+351
to
+352
| AND id IN (SELECT DISTINCT trace_id FROM experiment_items WHERE workspace_id = :workspace_id AND experiment_id IN :experimentIds) | ||
| AND project_id IN (SELECT DISTINCT project_id FROM traces WHERE workspace_id = :workspace_id AND id IN (SELECT DISTINCT trace_id FROM experiment_items WHERE workspace_id = :workspace_id AND experiment_id IN :experimentIds)) |
Member
There was a problem hiding this comment.
It's a nit from Baz, but valid. Optionally fee free to address it.
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
2 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
Follow-up to the merged #7412, addressing @andrescrz's review nit: "Minor: I believe better with distinct trace_id for both cases. Applies to the changes below as well."
The
DatasetItemDAOexperiment-item filter gates (count query and stats query) added in #7412 bound thetracesscan by the experiment trace-id set:experiment_itemsholds one row per (experiment, dataset item), so a trace referenced by multiple items appears multiple times. This changes those four gate subqueries toSELECT DISTINCT trace_id, deduplicating the id set before it feeds theIN (...)bound (and the nested project-resolution).DISTINCTinside anIN (...)is result-equivalent — set membership is unchanged — so query results are identical; it only yields a smaller prepared set.Change checklist
Issues
AI-WATERMARK
AI-WATERMARK: yes
DISTINCT trace_idreview suggestion to the four gate subqueries and authored this PR description.mvn spotless:check compileruns clean locally.Testing
mvn spotless:check compile -DskipTests(offline) — clean. The change isDISTINCTinside existingIN (...)subqueries, which is result-equivalent, so the existingDatasetsResourceTest$FindDatasetItemsWithExperimentItemscoverage (56 tests, green on [OPIK-6884] [BE] perf: project/trace-id scope dataset-item + optimization enrichment scans #7412) continues to apply.Documentation
No user-facing or SDK changes; internal query-path hardening only.