fix(jdbi3): chunk IN-list batch queries to stay under DB parameter limit (#28752) - #28777
Merged
Conversation
…mit (#28752) * fix(jdbi3): chunk IN-list batch queries to stay under DB parameter limit Bulk subtree delete/restore fans an entire tree level's ids into a single IN (...) query. JDBI @BindList emits one bind parameter per element, so a level with ~100k entities (e.g. a service with 100k tables) exceeds PostgreSQL's 65,535 parameter ceiling ("Given query has 100,002 parameters") and the operation fails. The reported findToBatchAllTypes crash was only the first uncaught offender: getExtensionsBatch hits the same limit earlier but is swallowed by populateRelationFields' per-entity fallback, and ~13 other findToBatch*/findFromBatch* and tag batches were vulnerable in other paths. Add EntityDAO.queryInChunks / updateInChunks helpers (chunk at MAX_IN_LIST_CHUNK_SIZE, pass straight through for small lists so single-query behavior is preserved) and route every @BindList batch method through them: findToBatch*/findFromBatch*, entity_extension deleteAllBatch / getExtensionsBatch / getExtensionBatch, and tag_usage getTagsInternalBatch / getCertTagsInternalBatch. Also extract the duplicated deletedCondition(include) clause builder. Adds InListChunkingTest covering the chunk-size cap, exact input coverage (no dropped/duplicated ids), and per-chunk result aggregation. * fix(jdbi3): dedup ids before chunking IN-list batches queryInChunks / updateInChunks split a large id list into chunks, so a duplicate value landing in two different chunks would be queried (or deleted) twice — duplicating result rows / issuing a redundant statement, unlike a single IN (...) which ignores duplicate values. De-duplicate (encounter order preserved) before chunking, matching the existing findEntitiesByIds pattern. Adds InListChunkingTest cases covering duplicates split across a chunk boundary for both the query and update helpers. * fix(jdbi3): chunk three more unbounded IN-list paths Audit of the remaining @BindList paths surfaced three more methods that fan a caller-controlled, catalog-scaled list into one IN (...) and can hit the same 65,535 parameter ceiling. All routed through the queryInChunks/updateInChunks helpers: - EntityRelationshipDAO.bulkUpdateFromId — reparenting a glossary term updates every nested descendant's relationship row in one UPDATE; a subtree with >65k terms blew the cap. Now chunks toIds via updateInChunks. - TagUsageDAO.deleteTagsByTargets — clearing column tags on a bulk table update/import passes every column FQN across the batch (wide/nested tables x ~100 per batch). Now chunks via updateInChunks. - EntityTimeSeriesDAO.getLatestExtensionBatch — listing ingestion pipelines with fields=pipelineStatuses and a large limit (capped at 1,000,000) binds one FQN-hash per row. Now chunks via queryInChunks. Adds InListChunkingTest cases for all three. * test(jdbi3): cover IN-list chunking pass-through with duplicate ids The existing tests all feed an over-limit list, leaving queryInChunks / updateInChunks' else branch (size <= MAX_IN_LIST_CHUNK_SIZE) untested. Add two cases proving a within-limit list with duplicates issues exactly one query/statement and is passed through verbatim (no dedup) — the database collapses the duplicates via IN(...) set semantics. Complements the existing over-limit dedup tests, which assert against the distinct set. * fix(jdbi3): chunk five more catalog-scaled IN-list paths Follow-up to the IN-list chunking work — five more @BindList paths fan a catalog-scaled list into one IN (...) and can still hit the 65,535 parameter ceiling. All routed through the existing queryInChunks/updateInChunks helpers. - EntityRelationshipDAO.bulkRemoveTo / bulkRemoveFrom: the 'remove' siblings of bulkUpdateFromId (already chunked). Reached via the bulkRemove{To,From}Relationship wrappers (their only callers) from DataProductRepository.batchMigrateAssetDomains (all assets of a type under one data product), TeamRepository, DomainRepository. - UsageDAO.getLatestUsageBatch: GET /v1/tables?fields=usageSummary (and dashboards/pipelines/topics/mlmodels/...) resolves the field over the whole page via setFieldsInBulk; list limit is @max(1000000). - DataQualityDataTimeSeriesDAO.getLatestRecordBatch: GET /v1/dataQuality/testCases?fields=testCaseResult, same whole-page resolution. - TestCaseResultTimeSeriesDAO.listResultSummariesForTestSuites: GET /v1/dataQuality/testSuites?fields=summary, binds one id per executable test suite. Adds InListChunkingTest cases for all five (16 tests pass). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Sriharsha Chintalapani <harsha@getcollate.io> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Sriharsha Chintalapani <harshach@users.noreply.github.com> [1.13 backport] Dropped the findToBatchAllTypes(List<Integer> relations, Include) chunking hunk and its findToBatchAllTypesWithRelationsCondition delegate: that overload originates in temporal lineage (#28426), which is not on the 1.13 branch. The two queryInChunks dedup / pass-through tests that exercised it were retargeted to the existing int-relation findToBatchAllTypes overload (same queryInChunks path). All 16 InListChunkingTest cases pass. (cherry picked from commit 5af1ba5)
Contributor
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
github-actions
Bot
added
backend
safe to test
Code Review ✅ ApprovedImplements OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
Contributor
|
The Python checkstyle failed. Please run You can install the pre-commit hooks with |
|
Contributor
🟡 Playwright Results — all passed (18 flaky)✅ 3890 passed · ❌ 0 failed · 🟡 18 flaky · ⏭️ 80 skipped
🟡 18 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
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.
Cherry-pick of #28752 onto the
1.13branch.What
Bulk subtree delete/restore fans an entire tree level's ids into a single
IN (...)query. JDBI@BindListemits one bind parameter per element, so a level with ~100k entities (e.g. a service with 100k tables) exceeds PostgreSQL's 65,535 parameter ceiling and the operation fails. AddsEntityDAO.queryInChunks/updateInChunkshelpers (chunk atMAX_IN_LIST_CHUNK_SIZE = 30_000, pass small lists straight through, dedup before chunking) and routes every@BindListbatch method through them.1.13 backport adaptation
findToBatchAllTypes(List<Integer> relations, Include)chunking hunk and itsfindToBatchAllTypesWithRelationsConditiondelegate — that overload originates in temporal lineage (Add temporal lineage traversal support #28426), which is not on1.13. It has no callers on this branch.queryInChunksdedup / pass-through tests that exercised that overload to the existingint-relationfindToBatchAllTypesoverload (samequeryInChunkscode path), preserving coverage.Verification
openmetadata-servicecompiles (main + test) under Java 21.InListChunkingTest— 16/16 pass (0 failures, 0 errors).mvn spotless:applyclean.🤖 Generated with Claude Code