Repository navigation
Conversation
__hash__ hashed the bound __repr__ method object, whose hash follows the identity of the instance, so two ClickHouseVersion objects that compare equal hashed differently and sets/dicts treated them as different keys. That breaks the dedupe in get_supported_versions and in ci/tools/official_docker.py whenever the same release arrives as two independently built copies. Hash exactly the fields __eq__ compares (major, minor, patch, tweak). Hashing __repr__() instead would still be wrong: __eq__ ignores the version type, the repr does not. Adds ci/tests/test_clickhouse_version.py pinning the contract.
clickhouse-gh
Bot
added
comp-ci-infrastructure
|
|
alexey-milovidov
added
the
can be tested
Contributor
|
Workflow [PR], commit [54d9583] Summary: ❌
|
alexey-milovidov
approved these changes
Nov 15, 2026
This branch has not been deployed
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.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
CI only:
ClickHouseVersion.__hash__now hashes the fields that__eq__compares, so equal versions hash equal.Description
ClickHouseVersion.__hash__returnedhash(self.__repr__)— the hash of the bound method, which is never called. A bound method's hash is identity-derived, so two versions that compare equal under__eq__(which comparesmajor,minor,patch,tweak) generally hash differently, and set/dict deduplication or lookup across independently constructed copies silently misses.The fix returns
hash((self.major, self.minor, self.patch, self.tweak)), exactly the field set__eq__compares. Hashingself.__repr__()instead would not fix the contract:__repr__embeds the description, which comes fromversion_type, and__eq__ignoresversion_type— equal versions with different version types would still hash differently.No live failure was observed in the current call flows: the versions that reach the sets in
get_supported_versionsandofficial_docker.pymostly arrive as the same objects, so the broken hash has not produced a wrong result I could demonstrate. The defect is the contract itself — twoClickHouseVersionobjects that compare equal hash differently, so any dedupe or lookup across independently constructed copies (one parsed from a tag, one built by the release flow) silently misses. The new test pins the contract.Tests
ci/tests/test_clickhouse_version.py(5 tests): 5 passed with the fix. Against the base implementation, 4 failed and 1 passed (the distinctness control). The different-metadata test also fails against ahash(self.__repr__())variant.ci/testsrun in my sparseci/-only checkout: 55 passed; the failing set is identical to base (all intest_fuzzer_dict_codegen_deps.py, fromsrc/files the sparse checkout does not contain).ruffandpylintwith the repo configuration: clean on the changed files.Workflow [PR]
Sync PR [sync-upstream/pr/129594]