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

Virtual blocks fail silently #979

Closed
janko opened this issue Mar 22, 2015 · 2 comments
Closed

Virtual blocks fail silently #979

janko opened this issue Mar 22, 2015 · 2 comments

Comments

@janko
Copy link
Contributor

janko commented Mar 22, 2015

I wanted to do regex mathching inside a virtual block, and I wrote this:

DB[:questions].where{title =~ /#{query}/i}

The query didn't work, and after reading the section about virtual blocks I saw that there is no operator for matching regexes inside of virtual blocks. What surprised me is that Sequel didn't raise any errors. What it did is simply ignore that WHERE clause in the final query:

SELECT * FROM "questions"

Is this an intentional part of the design? Is it possible to make Sequel raise an error for mistakes like these? Because this way writing virtual blocks is very brittle.

@jeremyevans
Copy link
Owner

You don't need a special operator for regexp matching in virtual row blocks, since they can be specified via a hash: DB[:questions].where(:title=>/#{query}/i). Your example is not specific to virtual row blocks, it applies to Sequel generally: DB[:questions].where(Sequel.expr(:title) =~ /#{query}/i)

It's possible to support what you want fairly easily:

class Sequel::SQL::GenericExpression
  def =~(regexp)
    Sequel.expr(self=>regexp)
  end
end

I'm not sure what the backwards compatibility effects would be, but I'm guessing they would be minimal, so that's something I would consider. If you think that should be added, please open up a thread on the sequel-talk Google Group and see what the communities' thoughts are.

Sequel doesn't raise an error in this case, because the =~ call returns nil, so what you are doing is: DB[:questions].where{nil}, and Sequel treats where(nil) as a no-op.

Virtual row blocks don't add any features to Sequel, they are purely a way to make code less verbose. In general, they just offer a way to easily create Sequel::SQL::Identifier, Sequel::SQL::QualifiedIdentifier, and Sequel::SQL::Function instances. They don't override standard ruby behavior. Your issue here is you are calling the =~ method on an object that doesn't override the default ruby Object#=~ behavior of returning nil.

@janko
Copy link
Contributor Author

janko commented Mar 22, 2015

Wow, I did not know that =~ is defined on Object. Then I totally get why this behaviour happens, I thought it was something general. Thank you for the explanation!

Yeah, I might suggest adding =~ to Sequel::SQL::GenericExpression. I would normally use a simple hash: where(title: /#{query}/i), but I want to use the virtual row because I need to write an OR statement.

jeremyevans added a commit that referenced this issue Mar 23, 2015
…to a hash (Fixes #979)

This makes it simpler to use equality/inclusion/pattern match
operations in virtual row blocks.  On ruby 1.9, it makes it
significantly simpler to include inverted conditions inside more
complex expressions, which previously required Sequel.~.

This does change behavior, since =~ and !~ were already defined,
since they are defined in Object.  However, it is unlikely that
anyone was relying on the previous behavior, since =~ returned
nil and !~ returned true.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants