Repository navigation
Fix tests on HEAD - #3204
Merged
Merged
Fix tests on HEAD #3204
Conversation
chernser
changed the title
Client V2 CoverageCoverage Report
Class Coverage
|
|
JDBC V2 CoverageCoverage Report
Class Coverage
|
JDBC V1 CoverageCoverage Report
Class Coverage
|
Client V1 CoverageCoverage Report
Class Coverage
|
Member
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 29b553b. Configure here.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The workflow modification needs the maintainer authorization required by AGENTS.md.
1 open finding
What changed in this PR
Restructures getTables SQL to avoid a ClickHouse HEAD pruning regression and isolates module tests from dependency tests.
Changes:
- Groups table predicates into a safer nested conjunction.
- Builds dependencies without tests before testing only the selected module.
Compatibility impact is low; JDBC metadata results are intended to remain unchanged. No tests were run in this review environment.
| File | Description |
|---|---|
DatabaseMetaDataImpl.java |
Reorganizes getTables predicates. |
test_head.yml |
Separates dependency build and module testing. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
mshustov
approved these changes
Oct 14, 2026
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
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.
Summary
The breaking change was merged 3 days ago:
On October 10, ClickHouse/ClickHouse#125033 was merged into ClickHouse master. It introduced short-circuit lazy evaluation during virtual-column pruning.
That change caused the regression: when stripping virtual conjuncts from an AND(...), ClickHouse failed to rebuild the function with the reduced arity, resulting in Cannot call function and because is has 4 arguments but 3 columns were captured.
The fix was open until today:
Fix PR ClickHouse/ClickHouse#125126 (Rebuild a reduced and when pruning parts and files by virtual columns) was opened on October 11, but was held up because of another related bug on the same virtual column pruning path (#125150).
PR #125126 was merged just under an hour ago (at 2026-10-13 15:34:34 UTC, commit 50b17deb63d89d6a69ba79cbba5ba69ffd3a8bb9).
getTablesto do more safe SQL