feat: Implement dedup and filter for vectors #245
Merged
Conversation
This file contains 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
evenyag
force-pushed
the
feat/dedup-vector
branch
2 times, most recently
from
59901e1
to
8502409
Compare
evenyag
changed the title
evenyag
force-pushed
the
feat/dedup-vector
branch
from
1f01dd8
to
92703af
Compare
evenyag
force-pushed
the
feat/dedup-vector
branch
2 times, most recently
from
35d07f2
to
ab3fc5d
Compare
Codecov Report
@@ Coverage Diff @@
## develop #245 +/- ##
===========================================
+ Coverage 83.33% 83.87% +0.53%
===========================================
Files 295 297 +2
Lines 25338 25737 +399
===========================================
+ Hits 21116 21587 +471
+ Misses 4222 4150 -72
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
evenyag
force-pushed
the
feat/dedup-vector
branch
4 times, most recently
from
535effa
to
f6622e1
Compare
v0y4g3r
reviewed
Sep 16, 2022
Use field val instead of tuple for variant ListValueRef::Ref to keep consistence with ListValueRef::Indexed
Also implements ScalarVectorBuilder for ListVectorBuilder, Scalar for ListValue and ScalarRef for ListValueRef
Move compute operations to VectorOp trait and acts as an super trait of Vector. So we could later put dedup/filter methods to VectorOp trait, avoid to define too many methods in Vector trait.
Move Scalar and ScalarRef trait bounds to PrimitiveElement, so for each native type which implements PrimitiveElement, its PrimitiveVector always implements ScalarVector, so we could use it as ScalarVector without adding additional trait bounds
Remove compute mod and move dedup logic to operations::dedup
Also fix NullVector::dedup and ConstantVector::dedup
Also fix TimestampVector eq not implemented.
evenyag
force-pushed
the
feat/dedup-vector
branch
from
9864253
to
43dc7c7
Compare
paomian
pushed a commit
to paomian/greptimedb
that referenced
this pull request
Oct 19, 2023
* feat: Dedup vector * refactor: Re-export Date/DateTime/Timestamp * refactor: Named field for ListValueRef::Ref Use field val instead of tuple for variant ListValueRef::Ref to keep consistence with ListValueRef::Indexed * feat: Implement ScalarVector for ListVector Also implements ScalarVectorBuilder for ListVectorBuilder, Scalar for ListValue and ScalarRef for ListValueRef * test: Add tests for ScalarVector implementation of ListVector * feat: Implement dedup using match_scalar_vector * refactor: Move dedup func to individual mod * chore: Update ListValueRef comments * refactor: Move replicate to VectorOp Move compute operations to VectorOp trait and acts as an super trait of Vector. So we could later put dedup/filter methods to VectorOp trait, avoid to define too many methods in Vector trait. * refactor: Move scalar bounds to PrimitiveElement Move Scalar and ScalarRef trait bounds to PrimitiveElement, so for each native type which implements PrimitiveElement, its PrimitiveVector always implements ScalarVector, so we could use it as ScalarVector without adding additional trait bounds * refactor: Move dedup to VectorOp Remove compute mod and move dedup logic to operations::dedup * feat: Implement VectorOp::filter * test: Move replicate test of primitive to replicate.rs * test: Add more replicate tests * test: Add tests for dedup and filter Also fix NullVector::dedup and ConstantVector::dedup * style: fix clippy * chore: Remove unused scalar.rs * test: Add more tests for VectorOp and fix failed tests Also fix TimestampVector eq not implemented. * chore: Address CR comments * chore: mention vector should be sorted in comment * refactor: slice the vector directly in replicate_primitive_with_type
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.
Changes
This PR adds some necessary compute operations for vectors in order to implement the
DedupReader.VectorOpsuper trait to provide computation support forVectordedupandfilterreplicatemethod toVectorOpScalarVectorforListVector, so all exceptNullVectorcould share the samededupimplementation based on theScalarVectortrait.ListVectornow implementsScalarVector, it could usereplicate_scalar()to implementreplicate()ListValueRef::Refbe a struct variant instead of tuple variant, for consistence betweenRefandIndexedvariantPrimitiveElementtrait and move theScalarandScalarReftrait bound to thePrimitiveElementtrait, soPrimitiveVector<T: PrimitiveElement>implies theScalarVectortrait bound