JRuby (please use github issues at http://bugs.jruby.org)
  1. JRuby (please use github issues at http://bugs.jruby.org)
  2. JRUBY-3026

[Derby] Allow select/delete/update conditions with comparison to NULL using '='

    Details

    • Type: Improvement Improvement
    • Status: Closed Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: JRuby 1.1.4
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None
    • Environment:
      Fedora Linux 9, Sun Java 1.6.0_07, JRuby 1.1.4, ActiveRecord-jdbc-adapter 0.8.2, activerecord 2.1.1
    • Testcase included:
      yes
    • Patch Submitted:
      Yes
    • Number of attachments :
      1

      Description

      When using conditions with parameters like

      Entry.find(:all, :conditions => ['title = ?', my_expression])

      the resulting SQL statement fails on DerbyDB (JavaDB) if the given parameter computes til nil since DerbyDB does not allow comparison with NULL using the equals operator. DerbyDB demands using "IS NULL" when comparing to NULL.

      I have attached a patch with test that allows most uses to succeed.

      Please review and comment.

        Activity

        Hide
        Martin C. Martin added a comment -

        SQL has three valued logic, where boolean expressions can be "true", "false" or "NULL" where NULL means "I don't know." It's unfortunate that they chose NULL to denote this, since every where else in computerland it means "no value."

        When you say "title = NULL", you're saying "is title equal to some value I don't know?" Of course, if you don't know what you're comparing to, you can't know whether the comparison is true or false. So the result of the expression is not "true" or "false" but NULL. If you want to test whether a value is NULL or not, you use "title is NULL," which always returns true or false.

        http://en.wikipedia.org/wiki/Null_(SQL)

        Unless I'm misunderstanding the issue...

        Show
        Martin C. Martin added a comment - SQL has three valued logic, where boolean expressions can be "true", "false" or "NULL" where NULL means "I don't know." It's unfortunate that they chose NULL to denote this, since every where else in computerland it means "no value." When you say "title = NULL", you're saying "is title equal to some value I don't know?" Of course, if you don't know what you're comparing to, you can't know whether the comparison is true or false. So the result of the expression is not "true" or "false" but NULL. If you want to test whether a value is NULL or not, you use "title is NULL," which always returns true or false. http://en.wikipedia.org/wiki/Null_(SQL ) Unless I'm misunderstanding the issue...
        Hide
        Uwe Kubosch added a comment -

        I believe you misunderstand the issue

        The construct

        Entry.find(:all, :conditions => ['title = ?', my_expression])
        

        is a common ActiveRecord usage that tries to find records in the "entries" table where the title field matches the given expression (my_expression). This construct works flawlessly on PostgreSQL (and maybe other DBMSs), but DerbyDB does not allow it. It demands the "IS NULL" construct if my_expression evaluates to nil. This is correct SQL behaviour (I expect). Nevertheless, it makes programming much harder since you need to make a special case for "nil" values for "my_expression". Like this:

        if my_expression
          Entry.find(:all, :conditions => ['title = ?', my_expression])
        else
          Entry.find(:all, :conditions => ['title IS NULL'])
        end
        

        My patch allows the first construct, avoiding treating nil as a special case. The result is less coding and a more DMBS independent ActiveRecord-JDBC usage.

        Show
        Uwe Kubosch added a comment - I believe you misunderstand the issue The construct Entry.find(:all, :conditions => ['title = ?', my_expression]) is a common ActiveRecord usage that tries to find records in the "entries" table where the title field matches the given expression (my_expression). This construct works flawlessly on PostgreSQL (and maybe other DBMSs), but DerbyDB does not allow it. It demands the "IS NULL" construct if my_expression evaluates to nil. This is correct SQL behaviour (I expect). Nevertheless, it makes programming much harder since you need to make a special case for "nil" values for "my_expression". Like this: if my_expression Entry.find(:all, :conditions => ['title = ?', my_expression]) else Entry.find(:all, :conditions => ['title IS NULL']) end My patch allows the first construct, avoiding treating nil as a special case. The result is less coding and a more DMBS independent ActiveRecord-JDBC usage.
        Hide
        Charles Oliver Nutter added a comment -

        This isn't a JRuby issue, so I'm removing the fix version. It'll get into AR-JDBC soon hopefully.

        Show
        Charles Oliver Nutter added a comment - This isn't a JRuby issue, so I'm removing the fix version. It'll get into AR-JDBC soon hopefully.
        Charles Oliver Nutter made changes -
        Field Original Value New Value
        Fix Version/s JRuby 1.1.5 [ 14528 ]
        Thomas E Enebo made changes -
        Summary Allow select/delete/update conditions with comparison to NULL using '=' [Derby] Allow select/delete/update conditions with comparison to NULL using '='
        Hide
        Thomas E Enebo added a comment -

        Uwe, There are a couple of things that it would be nice if you could fix and resubmit on this patch:

        • = NULL probably needs to be =\s*NULL
        • !=\s*NULL should become IS NOT NULL
        • INSERT GUARD needs to be added similiar to UPDATE check (and override_execute similiar to what is done in jdbc_adapter.rb)

        It is a bummer that we are post-processing what AR generates, but since the programmer themselves do this in condition string there is not much we can do. A possibly better method would be to monkey-patch AR method which processes conditions. I think that is rife with peril however. So for now your strategy seems the most manageable right now.

        Show
        Thomas E Enebo added a comment - Uwe, There are a couple of things that it would be nice if you could fix and resubmit on this patch: = NULL probably needs to be =\s*NULL !=\s*NULL should become IS NOT NULL INSERT GUARD needs to be added similiar to UPDATE check (and override_execute similiar to what is done in jdbc_adapter.rb) It is a bummer that we are post-processing what AR generates, but since the programmer themselves do this in condition string there is not much we can do. A possibly better method would be to monkey-patch AR method which processes conditions. I think that is rife with peril however. So for now your strategy seems the most manageable right now.
        Hide
        Thomas E Enebo added a comment -

        Fixed in commit 0167286. I just added the stuff I suggested to the original patch. This improves our AR unit tests to: 1819 tests, 5547 assertions, 43 failures, 161 errors (was 1819 tests, 5494 assertions, 43 failures, 183 errors)

        Show
        Thomas E Enebo added a comment - Fixed in commit 0167286. I just added the stuff I suggested to the original patch. This improves our AR unit tests to: 1819 tests, 5547 assertions, 43 failures, 161 errors (was 1819 tests, 5494 assertions, 43 failures, 183 errors)
        Thomas E Enebo made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Fix Version/s ActiveRecord-JDBC-0.9.1 [ 14819 ]
        Resolution Fixed [ 1 ]
        Assignee Thomas E Enebo [ enebo ]
        Hide
        Uwe Kubosch added a comment -

        Sorry I was slow on this. I had iteration delivery last week. Thanks for fixing it!

        Show
        Uwe Kubosch added a comment - Sorry I was slow on this. I had iteration delivery last week. Thanks for fixing it!
        Charles Oliver Nutter made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Thomas E Enebo made changes -
        Component/s ActiveRecord-JDBC [ 12786 ]
        Thomas E Enebo made changes -
        Fix Version/s ActiveRecord-JDBC-0.9.1 [ 14819 ]

          People

          • Assignee:
            Thomas E Enebo
            Reporter:
            Uwe Kubosch
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: