Skip to content
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

New Fastfield Trait with Nullhandling #1678

Closed
PSeitz opened this issue Nov 12, 2022 · 3 comments
Closed

New Fastfield Trait with Nullhandling #1678

PSeitz opened this issue Nov 12, 2022 · 3 comments

Comments

@PSeitz
Copy link
Contributor

PSeitz commented Nov 12, 2022

OptionalColumn should be the default, so all consumers would need to handle Option<T>, or try to convert to full column

pub trait OptionalColumn<T: PartialOrd = u64>: Send + Sync {

    fn get_val(&self, idx: u64) -> Option<T>;

    // TODO: Should output be `Option<T>` or `T`
    fn get_range(&self, start: u64, output: &mut [T]); 

    /// Return the positions of values which are in the provided range.
        fn get_positions_for_value_range(
        &self,
        value_range: RangeInclusive<T>,
        doc_id_range: Range<u32>,
        positions: &mut Vec<u32>,
    );

    fn min_value(&self) -> Option<T>;
    fn max_value(&self) -> Option<T>;

    fn num_vals(&self) -> u32;

    /// Returns a iterator over the data
    fn iter<'a>(&'a self) -> Box<dyn Iterator<Item = Option<T>> + 'a>; 

    /// return full column with default_val if value is None
    fn or(&self, default_val: T) -> Arc<dyn Column<T>>;
    
    /// return full column if all values are set and is not empty
    fn into_full(&self) -> Option<Arc<dyn Column<T>>>;

    /// maybe docset, if we can utilize it
    fn docset_with_value(&self) -> Box<dyn DocSet>;
}
@fulmicoton
Copy link
Collaborator

It looks good!

We can forget about docset_with_value initially.
into_full should probably be named differently if it does not consume self.

Weird question: do you think there is a world where we can have one trait and use GAT to deal with Optional / Required / Multi cardinality?

@PSeitz
Copy link
Contributor Author

PSeitz commented Nov 15, 2022

I did some tests with GAT, but did run quickly into issues, because they are not object safe currently.

pub trait GATColumn: Send + Sync {
    type T: PartialOrd;
    type Item<T>;
    fn get_val(&self, idx: u64) -> Self::Item<Self::T>;
    fn into_full(
        &self,
        idx: u64,
    ) -> Arc<dyn GATColumn<Value = Self::T, Item<Self::T> = Self::T>>;
}

error[E0038]: the trait `GATColumn` cannot be made into an object
  --> src/lib.rs:10:14
   |
10 |     ) -> Arc<dyn GATColumn<Value = Self::Value, Item<Self::Value> = Self::Value>>;
   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `GATColumn` cannot be made into an object

rust-lang/rust#81823

@fulmicoton
Copy link
Collaborator

fulmicoton commented Nov 15, 2022

haha. I tried to too. It is not GAT, but

trait Column<T> {
   type ItemContainer = Into<Option<T>>;
   fn get(&self, idx: u64) -> Self::ItemContainer;
}

You then manipulate `Arc<dyn Column, ItemContainer=T>

The benefit is that you can use the same code for both containers and get monomorphization.
(hopefully the code for ItemContainer = T will be as fast as if you did not play with Options.)

You still have to do some juggling/manual dispatch to build the collector though, and manipulate it.

trait Column<T> {
   type ItemContainer = Into<Iterator<Item=T>>;
   fn get(&self, idx: u64) -> Self::ItemContainer;
}

is even more tempting, as it might make it possible to work for Optional/Required/Multivalued altogether.
Unfortunately, I don't know how to work with Multivalued stuff without passing a vec buffer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants