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

MapD bucket() : MapD backend doesn't support <class 'ibis.expr.operations.NullLiteral'> operation! #1620

Closed
565ee opened this issue Sep 11, 2018 · 14 comments
Labels
bug Incorrect behavior inside of ibis

Comments

@565ee
Copy link

565ee commented Sep 11, 2018

>>> buckets = [0, 1, 3]
>>> bucketed = table.price_col.bucket(buckets).name('bucket')
>>> bucketed.value_counts().execute()
return error :
File "/home/sw/anaconda3/lib/python3.6/site-packages/ibis/mapd/operations.py", line 365, in raise_unsupported_op_error
raise com.UnsupportedOperationError(msg.format(type(op)))
ibis.common.UnsupportedOperationError: MapD backend doesn't support <class 'ibis.expr.operations.NullLiteral'> operation!

Backend=MapD MapD=4.1.1 ibis=0.14 Python=3.6
Ubuntu=18.04 Anaconda=5.1

@cpcloud
Copy link
Member

cpcloud commented Sep 19, 2018

cc @xmnlab

@cpcloud cpcloud added this to the Future milestone Sep 19, 2018
@cpcloud cpcloud added mapd bug Incorrect behavior inside of ibis labels Sep 19, 2018
@xmnlab
Copy link
Contributor

xmnlab commented Sep 28, 2018

@randyzwitch it seems MapD doesn't allow Null literal, is it right?

ex:

mapdql> select *, NULL as "t" from tb10;
Exception: Exception occurred: org.apache.calcite.runtime.CalciteContextException: From line 1, column 11 to line 1, column 14: Illegal use of 'NULL'

@randyzwitch
Copy link
Contributor

That seems right @xmnlab, I've never seen a query try to do that :)

@xmnlab
Copy link
Contributor

xmnlab commented Sep 30, 2018

@cpcloud so this error is expected? should we add some operations in MapD unsupported_op dictionary?

@cpcloud
Copy link
Member

cpcloud commented Oct 1, 2018

That seems right @xmnlab, I've never seen a query try to do that :)

This isn't totally crazy and isn't very different from something like select *, 1 as "t" from tb10;. Does MapD plan to support NULL literals in the future?

@randyzwitch
Copy link
Contributor

I can't comment on whether we'd ever support NULL as a literal, as I don't manage the product development. Personally, I suspect we wouldn't, 1) as it doesn't seem to have a whole lot of value and 2) what type is a column of constant NULL? I suspect that's would be the bigger reason we wouldn't support it; while supporting a constant like 1 as "t" in a query might be memory inefficient, we at least know it's a smallint.

But, I'm not one of the OmniSci backend engineers either :)

@cpcloud
Copy link
Member

cpcloud commented Oct 1, 2018

what type is a column of constant NULL

Often a database will require a cast when using a null literal.

@randyzwitch
Copy link
Contributor

If it's the case that this is really a desirable operation, my suggestion would be to submit an issue and someone with the proper background (either product or engineering) can comment on the specifics:

https://github.com/omnisci/mapd-core/issues

@xmnlab
Copy link
Contributor

xmnlab commented Oct 1, 2018

done! heavyai/heavydb#269 :)

@xmnlab
Copy link
Contributor

xmnlab commented Oct 1, 2018

@cpcloud should we change anything in the code related to this issue? or could this issue be closed?

@cpcloud
Copy link
Member

cpcloud commented Oct 1, 2018

@xmnlab If you write CAST(NULL AS FLOAT64) does that work? Or whatever the syntax is for a double precision floating point value is.

@xmnlab
Copy link
Contributor

xmnlab commented Oct 1, 2018

awesome! very good idea!

mapdql> select created_at, CAST(NULL AS FLOAT) as "t" from nyc_trees_2015_683k limit 1;
created_at|t
2015-10-01|NULL

@cpcloud
Copy link
Member

cpcloud commented Oct 1, 2018

@xmnlab Can you dig into what the generated code here is and see if it's reasonable to cast? I suspect that it is.

@xmnlab
Copy link
Contributor

xmnlab commented Oct 2, 2018

sure! I am working on that :)

xmnlab added a commit to Quansight/ibis that referenced this issue Oct 2, 2018
cpcloud pushed a commit that referenced this issue Oct 30, 2018
Fixes #1619

Author: Ivan Ogasawara <ivan.ogasawara@gmail.com>
Author: Ivan <ivan.ogasawara@gmail.com>

Closes #1647 from xmnlab/add_literal_null and squashes the following commits:

86fda52 [Ivan Ogasawara] Merge from master
b8e441f [Ivan Ogasawara] Merged from master
180ea2f [Ivan Ogasawara] Fixed tests
e7c70c3 [Ivan Ogasawara] Added tests
fc9e029 [Ivan Ogasawara] Merge branch 'add_literal_null' of https://github.com/quansight/ibis into add_literal_null
155ed0a [Ivan Ogasawara] Merge branch 'upstream_master' into add_literal_null
eaa523e [Ivan] Merge branch 'master' into add_literal_null
7374d82 [Ivan Ogasawara] Merge branch 'upstream_master' into add_literal_null
4d36a15 [Ivan Ogasawara] NullLiteral moved again as unsupported
31c186e [Ivan Ogasawara] Merge remote-tracking branch 'ibis-project/master' into add_literal_null
4c4d995 [Ivan Ogasawara] Fixed #1619: Joining on different column names
e136257 [Ivan Ogasawara] Solves #1620: Added NullLiteral
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect behavior inside of ibis
Projects
None yet
Development

No branches or pull requests

4 participants