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

Engine::get_evaluator should be fallible #566

Open
OussamaSaoudi-db opened this issue Dec 6, 2024 · 1 comment · May be fixed by #577
Open

Engine::get_evaluator should be fallible #566

OussamaSaoudi-db opened this issue Dec 6, 2024 · 1 comment · May be fixed by #577
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@OussamaSaoudi-db
Copy link
Collaborator

Please describe why this is necessary.

One can construct an ExpressionEvaluator given an engine, input schema, expression transform, and output schema:

engine.get_expression_handler().get_evaluator(
            input_schema,
            expression,
            output_schema,
        );

An engine may eagerly perform validation to check that the columns in the expression exist in the input schema, or that the output types of the expression match the output schema. However, the engine cannot return a failure because get_evaluator(...) is infallible.

Describe the functionality you are proposing.

Change the signature of ExpressionHandler::get_evaluator to the following:

fn get_evaluator(
        &self,
        schema: SchemaRef,
        expression: Expression,
        output_type: DataType,
    ) -> DeltaResult<Arc<dyn ExpressionEvaluator>> 

Fix compiler errors at all call sites in the code and testss.

Additional context

No response

@cg-cognition
Copy link

Hey @OussamaSaoudi-db , I took a pass at fixing this issue (#577). I think there's a still a few questions around handling error propagation, would really appreciate your review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants