- Notifications
You must be signed in to change notification settings - Fork63
refactor: fix ops.StrftimeOp, ops.ToDatetimeOp, ops.ToTimestampOp in sqlglot compiler#2297
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
546607a toc99acc9Compare| ifexpr.dtype!=dtypes.INT_DTYPE: | ||
| value=sge.Cast(this=value,to=sge.DataType(this="INT64")) |
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.
Should we remove this branch? "to_timedelta" only supports int, float and timedelta inputs:
python-bigquery-dataframes/bigframes/operations/timedelta_ops.py
Lines 28 to 35 ind1ecc61
| defoutput_type(self,*input_types:dtypes.ExpressionType)->dtypes.ExpressionType: | |
| ifinput_types[0]in ( | |
| dtypes.INT_DTYPE, | |
| dtypes.FLOAT_DTYPE, | |
| dtypes.TIMEDELTA_DTYPE, | |
| ): | |
| returndtypes.TIMEDELTA_DTYPE | |
| raiseTypeError("expected integer or float input") |
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.
Got it. Yeah, the logic are same but I just refactored it for more readable, after having these information. Thanks
9e0f70b intomainUh oh!
There was an error while loading.Please reload this page.
This change aims to fix the
to_datetimerelated tests failing in#2248.Fixes internal issue 417774347 🦕