zeroshade commented on code in PR #1352:
URL: https://github.com/apache/arrow-go/pull/1352#discussion_r4197768960
##########
arrow/compute/selection.go:
##########
@@ -540,32 +540,23 @@ func structFilter(ctx *exec.KernelCtx, batch
*exec.ExecSpan, out *exec.ExecResul
// The shared dictionary is reused as-is and never compacted, so the result
may retain values
// that are no longer referenced by any index.
func dictionaryFilter(ctx *exec.KernelCtx, batch *exec.ExecSpan, out
*exec.ExecResult) error {
- // convert the filter (boolean array) to indices to take from the
dictionary array.
- indices, err := kernels.GetTakeIndices(exec.GetAllocator(ctx.Ctx),
- &batch.Values[1].Array,
ctx.State.(kernels.FilterState).NullSelection)
- if err != nil {
- return err
- }
- defer indices.Release()
-
- filter := NewDatum(indices)
- defer filter.Release()
-
- valData := batch.Values[0].Array.MakeData()
- defer valData.Release()
+ dictArr := batch.Values[0].Array.MakeArray().(*array.Dictionary)
+ defer dictArr.Release()
- vals := NewDatum(valData)
- defer vals.Release()
+ selection := batch.Values[1].Array.MakeArray()
+ defer selection.Release()
- // run 'take' on the dictionary array, which will call dictionaryTake.
- // we know the bounds are good because the indices were just created by
GetTakeIndices
- result, err := Take(ctx.Ctx, kernels.TakeOptions{BoundsCheck: false},
vals, filter)
+ filteredIndices, err := FilterArray(ctx.Ctx, dictArr.Indices(),
selection,
Review Comment:
With a nullable filter and `DropNulls`, this call is about 30% slower than
the take-index path it replaces (`nullable-random50`: 13.5 → 17.9 ms). Consider
keeping the old path for that case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
