-
Notifications
You must be signed in to change notification settings - Fork 47
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
perf: Use simple null constraints to simplify queries #1381
Conversation
bigframes/core/compile/compiled.py
Outdated
@@ -16,7 +16,7 @@ | |||
import functools | |||
import itertools | |||
import typing | |||
from typing import Optional, Sequence | |||
from typing import Literal, Optional, Sequence, Tuple |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: if just use "tuple" for type annotation, then no need to import.
Sorry, something went wrong.
All reactions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed all Tuple
and typing.Tuple
for file
Sorry, something went wrong.
All reactions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to be careful about this kind of feedback. tuple
typing subscripting wasn't added until 3.9 (see: https://docs.python.org/3/library/typing.html#typing.Tuple). Since bigframes dropped 3.8 pretty early on, this is OK, but please include links to what versions of Python syntax like this is supported in in your feedback next time.
Sorry, something went wrong.
All reactions
bigframes/core/compile/compiled.py
Outdated
predicates=join_conditions, | ||
how=type, # type: ignore | ||
) | ||
combined_table = bigframes_vendored.ibis.join( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dup?
Sorry, something went wrong.
All reactions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
Sorry, something went wrong.
All reactions
Successfully merging this pull request may close these issues.
None yet
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕