Skip to content

Refactor ClickHouseChain for efficient slice selects #51

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 14, 2022

Conversation

michaelosthege
Copy link
Member

@michaelosthege michaelosthege commented Aug 14, 2022

  • Returning empty arrays (of correct shape and dtype) was needed because slices can result in an empty selection.
  • Increased coverage of the slicing test cases, including empty slices, reverse direction slices and dynamically shaped variables.
  • Implemented WHERE clause for the ClickHouse queries to download only the needed elements. This uses >=, < and a modulo operator.

The first CI run was ❌ just because of a too-low coverage.

To align behaviors, the `ClickHouseChain` may now return empty
arrays from empty chains or selections.
@codecov-commenter
Copy link

codecov-commenter commented Aug 14, 2022

Codecov Report

Merging #51 (0c70c3b) into main (1b2fe3b) will increase coverage by 0.12%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #51      +/-   ##
==========================================
+ Coverage   95.80%   95.93%   +0.12%     
==========================================
  Files           8        8              
  Lines         572      590      +18     
==========================================
+ Hits          548      566      +18     
  Misses         24       24              
Impacted Files Coverage Δ
mcbackend/backends/clickhouse.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@michaelosthege michaelosthege added the enhancement New feature or request label Aug 14, 2022
@michaelosthege michaelosthege self-assigned this Aug 14, 2022
@michaelosthege michaelosthege merged commit 8faf25e into main Aug 14, 2022
@michaelosthege michaelosthege deleted the slice-performance branch August 14, 2022 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants