#5457 Arel/AR prepending 'AND' unconditionally after newlines in string literals in RC2 - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#5457 ✓resolved

Arel/AR prepending 'AND' unconditionally after newlines in string literals in RC2

Reported by Brian Lopez | August 25th, 2010 @ 05:57 PM | in 3.0.2

Not sure if that title makes sense, but an easy way to reproduce it is:

SomeModel.where("varchar_test IS NULL\nor varchar_test IS NOT NULL")

Without the newline I get:

"SELECT mysql2_test.* FROM mysql2_test WHERE (varchar_test IS NULL or varchar_test IS NOT NULL)"

With the newline I get:

"SELECT mysql2_test.* FROM mysql2_test WHERE (varchar_test IS NULL\n AND or varchar_test IS NOT NULL)"

Which is invalid.

Trying to track down where this is happening, if I find it I'll try to come up with a patch but I figure you'd have a better idea of where it's happening.

Comments and changes to this ticket

Brian Lopez August 25th, 2010 @ 05:59 PM
- Tag changed from “pink warrior rc2 arel” to “pink warrior, arel, rc2”
José Valim August 25th, 2010 @ 06:01 PM
- Milestone cleared.
- State changed from “new” to “open”
- Importance changed from “” to “High”
You flagged this item as spam.
Dim August 25th, 2010 @ 06:13 PM
Got it, I found the issue!

In http://github.com/rails/arel/blob/master/lib/arel/algebra/relations... the clause is passed to Array(clause), therefore:
```
 > Array(%("table"."some_column" = 'some\nvalue'))
=> ["\"table\".\"some_column\" = 'some\n", "value'"]
```
Brian Lopez August 25th, 2010 @ 06:21 PM
Wow, didn't realize Array() would treat newlines like that...
José Valim August 25th, 2010 @ 06:27 PM
Yup, it does! This is why in Rails we usually use Array.wrap.
Brian Lopez August 25th, 2010 @ 06:28 PM
For reference, this was the commit that changed to using Array(): http://github.com/rails/arel/commit/bef0d30e30b99c8e56325645a9f643b...

Dim August 25th, 2010 @ 06:29 PM

Just to confirm:

ruby-1.9.2-rc2 > Array("a\nb")
 => ["a\nb"] 
ree-1.8.7-2010.02 > Array("a\nb")
 => ["a\n", "b"]

Xavier Noria August 25th, 2010 @ 06:43 PM
You could copy http://edgeguides.rubyonrails.org/active_support_core_extensions.ht...
You flagged this item as spam.
Dim August 25th, 2010 @ 06:52 PM
Or just go with [clause].flatten (not to duplicate code for just one single Array() instance)
Brian Lopez August 25th, 2010 @ 07:01 PM
If I remember correctly, Aaron made this change for performance reasons so whatever solution we end up figuring out should hopefully be performant; if at all possible.
Xavier Noria August 25th, 2010 @ 07:18 PM
Yeah I was in a hurry before. I mean, Array() has that gotcha and AS implements Array.wrap for that reason. If it makes sense it's there. If there's only one place where it is used then some simple Ruby may be better. (But the next guy may wonder why Array() wasn't used, so perhaps with a comment?)
Aaron Patterson August 25th, 2010 @ 10:10 PM
I'm on it. I'll have a fix for this later today. I'm super jet lagged right now. :'(
Aaron Patterson August 25th, 2010 @ 10:22 PM
- State changed from “open” to “resolved”
Should be fixed here:

http://github.com/rails/arel/commit/6333a2bd526cc4d09833aaf94c8cf1e...
Jeremy Kemper October 15th, 2010 @ 11:02 PM
- Milestone set to “3.0.2”
[bulk edit]