feat: filter() function return remain_columns and deleted_columns for statistics

[lichuang]

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

Summary

feat: filter() function return remain_columns and deleted_columns for statistics

Closes #8766

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Updated
databend	⬜️ Ignored (Inspect)		Nov 13, 2022 at 11:17AM (UTC)

[sundy-li]

This will make filter queries work slower than before which is unacceptable.

If you want to have the deleted column, there are some other ways like using Series::take

[lichuang]

This will make filter queries work slower than before which is unacceptable.

If you want to have the deleted column , there are some other ways like using Series::take

filter() use BooleanColumn as its filter args, i have make some poc that can get set and unset bits indices from BooleanColumn, and use Series::take to get remain and delete columns respectively, but it will loop the columns twice, if refactar filter() to return remain and deleted tuple, it will only need to be loop once.

[BohuTANG]

I prefer not to add the deleted to the filter function, it's a common function and is a hot path.
Could you give more information on how we use the filter function in your case? cc @lichuang

[sundy-li]

it will loop the columns twice, if refactar filter() to return remain and deleted tuple, it will only need to be loop once.

An extra memory copy for the column value is much more costable than an extra loop.

[lichuang]

I prefer not to add the deleted to the filter function, it's a common function and is a hot path. Could you give more information on how we use the filter function in your case? cc @lichuang

i agree that filter() is the hot path, and extra memory copy is expensive, so can we add a arg like Option<()> to indicate that if or not return deleted_column? cc @sundy-li

@sundy-li I have another question, if Series::take can filter all the column by BooleanColumn, then why all the sub-type of Column need to impl filter() function, why just use Series::take instead?

[sundy-li]

filter can use vectorized selection to improve performance.

mask_chunks
        .by_ref()
        .enumerate()
        .for_each(|(mask_index, mut mask)| {
            while mask != 0 {
                let n = mask.trailing_zeros() as usize;
                let i = mask_index * CHUNK_SIZE + n + start_index;
                builder.push(c.get_data(i));
                mask = mask & (mask - 1);
            }
        });

take is used to take value by random.

so can we add a arg like Option<()> to indicate that if or not return deleted_column?

I don't think we should add extra arg to do that. Let's make the filter method work as its name.

The simplest way to have is
deleted_columns = column.filter(Neg::neg(bitmap) )