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

Refactor to make it easy to add new functions #6

Closed
cswinter opened this Issue Jul 9, 2018 · 3 comments

Comments

Projects
None yet
1 participant
@cswinter
Copy link
Owner

cswinter commented Jul 9, 2018

I envision a world where adding a new function to LocustDB looks something like this:

// subtraction.rs
struct Subtract;

impl<T: Integer, U: Integer> Op2<T, U, i64> for Subtract {
    fn op(x: T, y: U) -> i64 { x.to_i64() - y.to_i64() }

    // optional properties used by query engine/optimiser
    fn is_commutative() -> bool { false }
    fn is_order_preserving() -> bool { true }
    // ...

    fn display_name() -> &'static str { "-" }
    fn display_infix() -> bool { true }
}
fn init_function_registry() {
    // ...
    register_function(
        Box::new(Subtract),
        specialize_int_int_i64!(Subtract),
    );
    // ...
}

There are a number of obstacles that have to be removed to make that happen.

Generating specializations

Look at the factory methods defined under impl VecOperator { and despair. We should have some macros that can automatically generate all that crap.

E.g. specialize_int_int_i64! would return something like a dictionary that maps (EncodingType, EncodingType) to a function that can be called to create a new BoxedOperator implementing subtraction for the corresponding types and includes implementations for for vector and scalar inputs. We will need to have a small number of actual VecOperator implementations to make this possible, similar to but slightly more general than e.g. the VecConstBoolOperator.

As an added bonus, this will also make it much easier to add support for new types and have them be supported by all existing functions.

Query planner

The create_query_plan function currently contains large (mostly copy-pasted) case for each individual function. It should be possible to replace this with a single piece of code that works for any function by using a table that maps each function to its properties and factory methods. This table is populated by the register_function in the example above.

Parser

It should not be necessary to modify the parser, at the most we should have to add the function name to a single list of valid names. Also see #4.

Open the floodgates

Add ALL the functions.

@cswinter cswinter referenced this issue Oct 13, 2018

Closed

Basic functions/operators #33

14 of 14 tasks complete

@cswinter cswinter self-assigned this Nov 6, 2018

@cswinter

This comment has been minimized.

Copy link
Owner

cswinter commented Nov 6, 2018

One piece of the puzzle: #53

@cswinter

This comment has been minimized.

Copy link
Owner

cswinter commented Nov 18, 2018

Further progress in #73

@cswinter

This comment has been minimized.

Copy link
Owner

cswinter commented Dec 2, 2018

#79 eliminates some more annoyances.
This will do for now.

@cswinter cswinter closed this Dec 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment