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

Properly cast reductions when necessary #1130

Closed
wants to merge 14 commits into from
Closed

Properly cast reductions when necessary #1130

wants to merge 14 commits into from

Conversation

cpcloud
Copy link
Member

@cpcloud cpcloud commented Jun 12, 2015

closes #1050

also restricts the scope of most reductions (notable exceptions are count and nelements)
to only work on sqlalchemy.elements.ColumnElements, rather than the generic ClauseElement.

@cpcloud cpcloud self-assigned this Jun 12, 2015
@cpcloud cpcloud added this to the 0.8.1 milestone Jun 12, 2015
@cpcloud
Copy link
Member Author

cpcloud commented Jun 12, 2015

cc @llllllllll can you try this out on a few things on your side? this touches some often hit code paths, i want to make sure nothing major is borked.

@cpcloud
Copy link
Member Author

cpcloud commented Jun 12, 2015

ugh it looks like spark 1.2 doesn't allow casting to anything but DECIMAL

@cpcloud
Copy link
Member Author

cpcloud commented Jun 15, 2015

@llllllllll i'd like to get this in, any chance you could review this tomorrow and/or try it out to make sure there are no major breakages? i've run it on some simple redshift queries (this was partly the impetus for this change) and things are working

@llllllllll
Copy link
Member

Sure thing, I will test this tomorrow morning at work

@cpcloud
Copy link
Member Author

cpcloud commented Jun 15, 2015

awesome thx!

Best,
Phillip Cloud

On Sun, Jun 14, 2015 at 11:04 PM, Joe Jevnik notifications@github.com
wrote:

Sure thing, I will test this tomorrow morning at work


Reply to this email directly or view it on GitHub
#1130 (comment).

@llllllllll
Copy link
Member

To test, I created a table with a single bool column in postgres; however, I could not cast bools to floats

ProgrammingError: (psycopg2.ProgrammingError) function sum(boolean) does not exist
LINE 1: SELECT sum(test.c) AS c_sum 
               ^
HINT:  No function matches the given name and argument types. You might need to add explicit type casts.
 [SQL: 'SELECT sum(test.c) AS c_sum \nFROM test']

@llllllllll
Copy link
Member

bz=# select cast(False as int);
 int4 
------
    0
(1 row)

bz=# select cast(False as float);
ERROR:  cannot cast type boolean to double precision
LINE 1: select cast(False as float);
               ^
bz=# select cast(False as int);
 int4 
------
    0
(1 row)

bz=# select cast(cast(False as int) as decimal);
 numeric 
---------
       0
(1 row)

It looks like we can actually cast to int an then to numeric though

@cpcloud
Copy link
Member Author

cpcloud commented Jun 15, 2015

i think casting to int is probably okay for now

@cpcloud
Copy link
Member Author

cpcloud commented Jun 17, 2015

closing in favor of #1137

@cpcloud cpcloud closed this Jun 17, 2015
@cpcloud cpcloud deleted the cast-sql-reductions branch June 17, 2015 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sum(boolean) doesn't work on sql but it's exposed as an attribute on bool columns
2 participants