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

bug: Figure out plan for supporting Datums / new Arrow APIs #783

Open
bjchambers opened this issue Sep 29, 2023 · 1 comment
Open

bug: Figure out plan for supporting Datums / new Arrow APIs #783

bjchambers opened this issue Sep 29, 2023 · 1 comment
Labels
bug Something isn't working

Comments

@bjchambers
Copy link
Collaborator

Description
Arrow is introducing a Datum type which makes it easier to support arrays and scalar values in a variety of methods. They are gradually implementing new kernels based on the Datum type and deprecating the old methods. We should figure out what this means for our expression evaluators.

@bjchambers bjchambers added the bug Something isn't working label Sep 29, 2023
@bjchambers
Copy link
Collaborator Author

At a high level, this is nice -- it makes it easier to work with Literals.

We currently intend to distinguish between add and add_scalar instructions. This allows us to look at the compiled plan and see if we have any literal instructions remaining, which (inefficiently) cause the conversion of a scalar value to an array. During plan generation, we would try to replace add(x, 1) with add_scalar(x, 1).

Generally, we think is still likely the best path, from the terms of simplicity of executing the plans and supporting instructions like field_ref which need to statically know the type of the field name.

Alternative:

  1. Use const in instruction signatures to indicate this argument must be const (eg., field ref)
  2. Allow arguments to be Some(index) or None (indicating a literal).

This could open the possibility of having a None in the arguments that doesn't have a corresponding literal value, as well as making it less clear where literals may occur.

bjchambers added a commit that referenced this issue Sep 30, 2023
This updates many places to use the new `Datum` backed APIs. But, it
suppresses a few cases and references #783. Specifically, some
expression code needs to be figured out, some runtime code (likely to
be deleted) and some other places.
github-merge-queue bot pushed a commit that referenced this issue Oct 2, 2023
This updates many places to use the new `Datum` backed APIs. But, it
suppresses a few cases and references #783. Specifically, some
expression code needs to be figured out, some runtime code (likely to be
deleted) and some other places.

Seems to fix the behavior of time-delta comparisons to literals (and
possibly other logical types represented by numeric primitives).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant