anxkhn opened a new pull request, #4973: URL: https://github.com/apache/polaris/pull/4973
<!-- ๐ Describe what changes you're proposing, especially breaking or user-facing changes. ๐ See https://github.com/apache/polaris/blob/main/CONTRIBUTING.md for more. --> `JdbcBasePersistenceImpl.hasChildren` only needs to know whether a parent entity has at least one child, but it does far more work than that. It builds a SELECT over every entity column with no row limit, then hands the result to `DatasourceOperations.executeSelect`, which collects the entire `ResultSet` into a list (`stream.forEach(results::add)`) before the method simply returns `results != null && !results.isEmpty()`. Every matching child row is therefore fetched, transferred over the wire, and deserialized in full, including the potentially large `properties` and `internal_properties` JSON columns, only to be discarded. This path runs when dropping a catalog or a namespace (`AtomicOperationMetaStoreManager.hasChildren`). When dropping a namespace the check matches all child types (tables and views included), so dropping a namespace that holds many tables loads every one of them into memory just to decide `NAMESPACE_NOT_EMPTY`. That adds needless latency and allocation and risks `OutOfMemoryError` on large namespaces. The JDBC layer already has a bounded existence-check pattern next door: `QueryGenerator.generateEntityTableExistQuery` uses `LIMIT 1` for the table-level check. This change bounds the existence check: - Add a `LIMIT`-aware `generateSelectQuery(projections, tableName, whereClause, int limit)` overload in `QueryGenerator`. It mirrors the existing 4-arg public overload and threads an optional limit through a new private builder, so every existing caller keeps emitting no `LIMIT` (the previous private builder now delegates with `null`). The `LIMIT` is appended after any `ORDER BY`, which is valid clause order for all supported backends. - Call it from `hasChildren` with a limit of `1`. Behavior is unchanged: with at most one row returned, `!results.isEmpty()` still means "has at least one child", so the method still returns `true` when a child exists and `false` otherwise. `LIMIT` is standard SQL accepted by all supported backends (PostgreSQL, CockroachDB, H2). The projection is intentionally left unchanged: `ModelEntity` reads every column when converting a row, so narrowing the projection would need a separate converter and a larger change. `LIMIT 1` alone removes the full scan, transfer, and materialization. I kept the projection full and the interface surface additive rather than changing any existing method signature, per the project's evolution guidelines. ### Checklist - [ ] ๐ก๏ธ Don't disclose security issues! (contact [email protected]) - [ ] ๐ Clearly explained why the changes are needed, or linked related issues: Fixes # - [ ] ๐งช Added/updated tests with good coverage, or manually tested (and explained how) - [ ] ๐ก Added comments for complex logic - [ ] ๐งพ Updated `CHANGELOG.md` (if needed) - [ ] ๐ Updated documentation in `site/content/in-dev/unreleased` (if needed) Checklist notes for when this is opened (do not paste the notes, just tick boxes): - Security: N/A, tick. - Why/issue: explained above. No matching open issue exists, so the `Fixes #` line should be deleted rather than filled (see briefing). Tick the box. - Tests: added `QueryGeneratorTest.testGenerateSelectQuery_withLimitAppendsLimitClause` (asserts the generated SQL ends with `LIMIT 1`, keeps the projection/WHERE, and binds params in order). Tick. - Comments: the new overload has a Javadoc explaining the existence-check intent. Tick. - CHANGELOG: not needed. This is an internal performance change with no user-facing behavior or configuration change (`CHANGELOG.md`'s `## [Unreleased]` tracks user-facing changes). Leave unticked / N/A. - Docs: not needed, same reason. Leave unticked / N/A. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
