-
Notifications
You must be signed in to change notification settings - Fork 71
Datafusion expand scalarvalue catchall #638
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
Changes from 9 commits
58df913
68f8542
2654a8b
ee71afb
cf0dcc5
b6bf6e3
a7c5b35
f653eba
d9fcf33
d507535
ada1876
88ccf6d
c9a9d91
01cb20a
d3b9837
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -145,8 +145,13 @@ def convert( | |
| elif literal_type == "Null": | ||
| literal_type = SqlTypeName.NULL | ||
| literal_value = None | ||
| elif literal_type == "IntervalDayTime": | ||
| literal_type = SqlTypeName.INTERVAL | ||
| literal_value = rex.getIntervalDayTimeValue() | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What kind of literal_value is returned in this case? Is it a string or an integer?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i64
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In that case we might have to update the logic in sql to python value, since the conversion there seems to assume it's a string and do some computation instead.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think int should be fine as long as we update the logic here as well: https://github.com/dask-contrib/dask-sql/blob/datafusion-sql-planner/dask_sql/mappings.py#L131. I'm surprised all tests passed. Maybe none actually test the case where an interval type is passed? |
||
| else: | ||
| raise RuntimeError("Failed to determine DataFusion Type in literal.py") | ||
| raise RuntimeError( | ||
| f"Failed to map literal type {literal_type} to python type in literal.py" | ||
| ) | ||
|
|
||
| # if isinstance(literal_value, org.apache.calcite.util.Sarg): | ||
| # return SargPythonImplementation(literal_value, literal_type) | ||
|
|
||
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 should also add methods to extract this information from interval yearMonth and MonthDayNano types