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

no support for OR in partition predicates #1923

Open
echai58 opened this issue Nov 29, 2023 · 1 comment
Open

no support for OR in partition predicates #1923

echai58 opened this issue Nov 29, 2023 · 1 comment
Labels
enhancement New feature or request

Comments

@echai58
Copy link

echai58 commented Nov 29, 2023

Environment

Delta-rs version: 0.13.0

Binding: python

Environment:

  • Cloud provider: n/a
  • OS: ubuntu 22.04
  • Other: testing locally via jupyter notebook

Bug

What happened:
There are inconsistencies between the python type-hints, docstring comments, and rust implementation for partition predicates.

In table.py, we see partitions: Optional[List[Tuple[str, str, Any]]] (https://github.com/delta-io/delta-rs/blob/main/python/deltalake/table.py#L704). There is also a docstring comment here referencing a DeltaTable.files_by_partitions, which seems to be a deprecated method, so I'm going to assume that is an out of date comment.

In _internal.pyi, we see Optional[FilterType] (https://github.com/delta-io/delta-rs/blob/main/python/deltalake/_internal.pyi#L86), which is defined as a DNF or Conjuction (https://github.com/delta-io/delta-rs/blob/main/python/deltalake/_internal.pyi#L717-L720).

In lib.rs, we see PartitionFilterValue (https://github.com/delta-io/delta-rs/blob/main/python/src/lib.rs#L655), which is defined as a string, or a vector of strings (https://github.com/delta-io/delta-rs/blob/main/python/src/lib.rs#L60-L63).

To my understanding, these three, which should all be defining the same partition filter type, all have different typing. The one in table.py and lib.rs are relatively congruent with each other, if we treat Any as Union[str,List[str]], which sort of makes sense.

The bigger difference is with _internal.pyi, which presents a whole different form of filter. This DNF style would allow for OR in the predicate filter, but this seems to be impossible in the python or rust typing. Would it be difficult to add support for OR in partition predicates via this DNF style or another?

What you expected to happen:
At the minimum, in the above code's current standing, I am not sure what the correct typing for predicate filters are. If the python one is incorrect, that should be fixed to provide accurate type hints and allow for correct static type checking when using python bindings.

Perhaps this should be raised in a Feature Request, but I'm also wondering why there isn't support for OR in predicate filters?

@echai58 echai58 added the bug Something isn't working label Nov 29, 2023
@ion-elgreco
Copy link
Collaborator

I think the type consistency is resolved now

@ion-elgreco ion-elgreco changed the title Partition predicate typing inconsistencies, and no support for OR in partition predicates no support for OR in partition predicates Dec 7, 2024
@ion-elgreco ion-elgreco added enhancement New feature or request and removed bug Something isn't working labels Dec 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants