build: Vendor clickhouse-cityhash into the driver#1
Closed
khvn26 wants to merge 10 commits into
Closed
Conversation
Co-authored-by: Sergey Gorbunov <gsergey418@yandex.ru> Co-authored-by: Konstantin Lebedev <xzkostyan@users.noreply.github.com>
First clickhouse-cityhash release that builds on Python 3.13+ and ships wheels for all platforms the driver supports. beep boop
* Make columnar block-extension test deterministic test_columnar_block_extend relied on its two single-row inserts reaching the client as two blocks. A background merge between the inserts and the select lets the server send one block, silently skipping the column-extension path in QueryResult.store. Force one row per block with max_block_size so the path is always exercised. beep boop * Cover sparse serialization without relying on the server Whether a block arrives in sparse form is the server's choice and varies by version and run, so the integration tests cover SparseSerialization only when the server cooperates. Feed read_sparse and apply_sparse crafted buffers directly so every line is exercised deterministically on every job. beep boop
Store built wheels and a new sdist as workflow artifacts on tag builds, and add a publish job that releases them to PyPI using OIDC-based Trusted Publishing (no API tokens). Existing draft GitHub release uploads are unchanged.
Add a concurrency group keyed per workflow + PR so pushing a new commit cancels the in-flight run for that PR. Non-PR pushes fall back to run_id, so master and tag builds each get their own lane and never cancel one another. Scope the push trigger to master and tags. Previously every commit on a PR branch fired both push and pull_request events, running the whole matrix twice; PR branches are now validated once via pull_request. beep boop
Bring the CityHash 1.0.2 algorithm in-tree instead of depending on the external clickhouse-cityhash package. ClickHouse hashes compressed blocks with this specific old version, so the dependency could never be replaced by the current CityHash; vendoring removes it entirely. - Add clickhouse_driver/_cityhash/ with the CityHash 1.0.2 C++ sources (from clickhouse-cityhash 1.0.2.6) and a trimmed Cython wrapper exposing only CityHash128, built as an always-on C++ extension. - Commit the generated cityhash.cpp so no-Cython installs still build, matching the existing committed .c convention. - Drop clickhouse-cityhash from the lz4/zstd extras and test requirements; compression now works out of the box. - Add golden-vector tests pinning the 1.0.2 output to guard server wire compatibility. beep boop
Member
Author
|
Re-targeting to upstream mymarilyn/clickhouse-driver. |
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
1 participant
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.
Bring the CityHash 1.0.2 algorithm in-tree rather than depending on the external
clickhouse-cityhashpackage.ClickHouse hashes compressed blocks with CityHash 1.0.2 specifically, and newer CityHash releases changed the algorithm, so the dependency could never be swapped for the current
cityhashpackage. Vendoring the sources removes the external dependency entirely and lets compression work out of the box.What's here:
clickhouse_driver/_cityhash/containing the CityHash 1.0.2 C++ sources copied verbatim fromclickhouse-cityhash, and a trimmed Cython wrapper exposing onlyCityHash128.setup.py. The generatedcityhash.cppis committed so installs without Cython still build, matching the existing committed-.cconvention for the pure-C modules.clickhouse-cityhashdropped from thelz4/zstdextras and fromtestsrequire.py.Note: building from source now requires a C++ compiler (previously only a C compiler). The SSE4.2 CRC path is intentionally left out, as only
CityHash128is needed.Verification:
clickhouse-cityhashpackage across the same inputs, guarding wire compatibility with the server.Checklist:
flake8and fix issues.pytestno tests failed. See https://clickhouse-driver.readthedocs.io/en/latest/development.html.