Quote integers in postgresql #172

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
@lukaszx0
Member

lukaszx0 commented Mar 12, 2013

Related to ActiveRecord issue: rails/rails#9170

Quote integers in postgres
Related to ActiveRecord issue: rails/rails#9170

@lukaszx0 lukaszx0 referenced this pull request in rails/rails Mar 12, 2013

Merged

Cast number to string in Postgres #9686

@ernie

This comment has been minimized.

Show comment
Hide comment
@ernie

ernie Mar 13, 2013

Collaborator

Seems like this should really be in to_sql. If comparing on a numeric column we wouldn't quote, if not, we would. Related: #162. I'm 👍 on both of these (well, this but in to_sql, and with tests). @tenderlove, what say you?

Collaborator

ernie commented Mar 13, 2013

Seems like this should really be in to_sql. If comparing on a numeric column we wouldn't quote, if not, we would. Related: #162. I'm 👍 on both of these (well, this but in to_sql, and with tests). @tenderlove, what say you?

@lukaszx0

This comment has been minimized.

Show comment
Hide comment
@lukaszx0

lukaszx0 Mar 13, 2013

Member

Thanks @ernie.

You're right. This is actually in to_sql right already: https://github.com/rails/arel/blob/master/lib/arel/visitors/to_sql.rb#L556-L557 but it's not quoted.

I've putted fix into PostgreSQL as I wasn't sure (and I'm still) not how this will affect other DBMS.

I wasn't aware of the #162 though. There's proper solution to my problem. I think this PR can be closed and it would be awesome to merge it as it's needed to solve Rails issue fully. Sorry for the mess ;)

Thanks.

Member

lukaszx0 commented Mar 13, 2013

Thanks @ernie.

You're right. This is actually in to_sql right already: https://github.com/rails/arel/blob/master/lib/arel/visitors/to_sql.rb#L556-L557 but it's not quoted.

I've putted fix into PostgreSQL as I wasn't sure (and I'm still) not how this will affect other DBMS.

I wasn't aware of the #162 though. There's proper solution to my problem. I think this PR can be closed and it would be awesome to merge it as it's needed to solve Rails issue fully. Sorry for the mess ;)

Thanks.

@lukaszx0 lukaszx0 closed this Mar 13, 2013

@ernie

This comment has been minimized.

Show comment
Hide comment
@ernie

ernie Mar 13, 2013

Collaborator

Without a change that quotes these two types, the visit in #162 wouldn't change anything, since the literal visitation method passes them through unchanged.

Collaborator

ernie commented Mar 13, 2013

Without a change that quotes these two types, the visit in #162 wouldn't change anything, since the literal visitation method passes them through unchanged.

@lukaszx0

This comment has been minimized.

Show comment
Hide comment
@lukaszx0

lukaszx0 Mar 13, 2013

Member

Yes, but in #162 there's a change from literal to quoted for there two types: https://github.com/rails/arel/pull/162/files#L0R588

Member

lukaszx0 commented Mar 13, 2013

Yes, but in #162 there's a change from literal to quoted for there two types: https://github.com/rails/arel/pull/162/files#L0R588

@ernie

This comment has been minimized.

Show comment
Hide comment
@ernie

ernie Mar 13, 2013

Collaborator

Oh. My bad. Forgot.

Collaborator

ernie commented Mar 13, 2013

Oh. My bad. Forgot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment