Skip to content

Conversation

@dantengsky
Copy link
Member

@dantengsky dantengsky commented Jun 13, 2025

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/

Summary

Modifies the ExprBloomFilter implementation to accept a reference to BinaryFuse16 instead of taking ownership of the filter. This optimization eliminates unnecessary cloning when applying runtime bloom filters during data deserialization in data sources.

Key changes:

  • Add lifetime parameter to ExprBloomFilter struct
  • Change filter field from owned BinaryFuse16 to a reference
  • Update constructor to accept and store references
  • Remove redundant clone() calls in native and parquet deserializers

Tests

  • Unit Test
  • Logic Test
  • Benchmark Test
  • No Test - Explain why

Type of change

  • Bug Fix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Breaking Change (fix or feature that could cause existing functionality not to work as expected)
  • Documentation Update
  • Refactoring
  • Performance Improvement
  • Other (please describe):

This change is Reviewable

@dantengsky dantengsky added the ci-benchmark-cloud Benchmark: run only cloud tests for tpch/hits label Jun 13, 2025
@github-actions github-actions bot added the pr-refactor this PR changes the code base without new features or bugfix label Jun 13, 2025
@github-actions
Copy link
Contributor

Docker Image for PR

  • tag: pr-18157-ee3b432-1749801796

note: this image tag is only available for internal use.

@github-actions
Copy link
Contributor

@dantengsky dantengsky added the ci-cloud Build docker image for cloud test label Aug 19, 2025
@github-actions
Copy link
Contributor

Docker Image for PR

  • tag: pr-18157-4317483-1755597166

note: this image tag is only available for internal use.

@github-actions
Copy link
Contributor

… clones

This change modifies the ExprBloomFilter implementation to accept a reference to BinaryFuse16
instead of taking ownership of the filter. This optimization eliminates unnecessary cloning
when applying runtime bloom filters during data deserialization in data sources.

Key changes:
- Add lifetime parameter to ExprBloomFilter struct
- Change filter field from owned BinaryFuse16 to a reference
- Update constructor to accept and store references
- Remove redundant clone() calls in native and parquet deserializers
@dantengsky dantengsky changed the title refactor: optimize ExprBloomFilter to use references instead of clones chore: optimize ExprBloomFilter to use references instead of clones Nov 13, 2025
@dantengsky dantengsky added the pr-chore this PR only has small changes that no need to record, like coding styles. label Nov 13, 2025
@dantengsky dantengsky force-pushed the refactor-optimize-exprbloomFilter branch from 8700a02 to 7c41e9d Compare November 13, 2025 09:02
@github-actions
Copy link
Contributor

Docker Image for PR

  • tag: pr-18157-e5bbf97-1763031809

note: this image tag is only available for internal use.

@github-actions
Copy link
Contributor

@dantengsky dantengsky marked this pull request as ready for review November 13, 2025 13:58
@dantengsky dantengsky merged commit 34565b0 into databendlabs:main Nov 14, 2025
207 of 256 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-benchmark-cloud Benchmark: run only cloud tests for tpch/hits ci-cloud Build docker image for cloud test pr-chore this PR only has small changes that no need to record, like coding styles. pr-refactor this PR changes the code base without new features or bugfix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants