Grails
  1. Grails
  2. GRAILS-8656

Unique constraint erroneously finds match when grouped property value is null

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0 final
    • Fix Version/s: 2.0.1
    • Component/s: None
    • Labels:
      None
    • Patch Submitted:
      Yes
    • Flagged:
      Impediment

      Description

      I have a user class where I would like the user id to be unique for records marked as not deleted
      As a simplification I have the rstriction uniqueness on date deleted (2 deleted records at the same time should also not have the same user id):

      class User {
      String userId
      Date dateDeleted

      static constraints =

      { userId blank: false, unique: 'dateDeleted' dateDeleted nullable: true }

      }

      However, if I save a user with userId = "User1" and a not null date deleted value
      Then validate a 2nd user with userId = "User1" and a null date deleted, the unique constrain kicks in.

      The problem appears in to be in org.grails.datastore.gorm.validation.constraints.UniqueConstraint

      line 69:

       final existing = constraintOwningClass.createCriteria().get {
                          eq constraintPropertyName, propertyValue
                          for(prop in group) {
                              eq prop, target[prop]
                          }
                      }
      

      evaluates to:

       final existing = User.createCriteria().get {
                          eq 'userId', 'User1'
                          eq 'dateDeleted', null
                      }
      

      Which is returning a row, even though the saved user has a not null dateDeleted. This is causing the uniqueness to fire

      1. UserClass.groovy
        0.2 kB
        Neill Robbins
      2. UserClassTests.groovy
        2 kB
        Neill Robbins

        Activity

        Show
        Neill Robbins added a comment - Attached github pull request for fix: https://github.com/SpringSource/grails-data-mapping/commit/93797e8066b11017a2e7cc4f2ab65f7d54251a07
        Hide
        Neill Robbins added a comment - - edited

        Hm - Still not what I want - the isNull is getting ignored because the column is not String ? I'm a little stumped...

        I guess you could check to see if the type of the constraining property (dateDeleted in my exam
        le) is String and do an isNull if it is.

        Otherwise remove it all together and then filter the result set on the grails side rather than in the Criteria expression?

        A bit long winded though - have I missed something?

        Show
        Neill Robbins added a comment - - edited Hm - Still not what I want - the isNull is getting ignored because the column is not String ? I'm a little stumped... I guess you could check to see if the type of the constraining property (dateDeleted in my exam le) is String and do an isNull if it is. Otherwise remove it all together and then filter the result set on the grails side rather than in the Criteria expression? A bit long winded though - have I missed something?
        Hide
        Graeme Rocher added a comment -

        your pull request contains no tests, perhaps if you wrote a test expressing the behavior you are trying to achieve we could better understand the issue

        Show
        Graeme Rocher added a comment - your pull request contains no tests, perhaps if you wrote a test expressing the behavior you are trying to achieve we could better understand the issue
        Hide
        Neill Robbins added a comment - - edited

        Attached a domain class (UserClass) and unit test class (UserClassTests) that demonstrate the issue. the last test fails and it shouldn't

        Show
        Neill Robbins added a comment - - edited Attached a domain class (UserClass) and unit test class (UserClassTests) that demonstrate the issue. the last test fails and it shouldn't
        Hide
        Neill Robbins added a comment - - edited

        Ok now updated the pull request to contain an additional commit that passes the tests and fixes the issue:
        https://github.com/SpringSource/grails-data-mapping/commit/2bcbc87ea520ecc23efe432b3db965af7aec5e44

        I think possibly it could be improved though.

        The fix resorts to using criteria.list to get all the persisted rows that match the constraint on not null properties, and then does a find to see if there is one in that result set that matches all the null properties. If there is, the uniqueness is violated, otherwise all is good.

        It could be improved by checking the data type of the columns that are null, and if they are String, using a criteria isNull restriction. that way the only properties being filtered by Grails rather than GORM are the null properties where the data type is not String. Cuts down the cases where criteria.list has to be used.

        Show
        Neill Robbins added a comment - - edited Ok now updated the pull request to contain an additional commit that passes the tests and fixes the issue: https://github.com/SpringSource/grails-data-mapping/commit/2bcbc87ea520ecc23efe432b3db965af7aec5e44 I think possibly it could be improved though. The fix resorts to using criteria.list to get all the persisted rows that match the constraint on not null properties, and then does a find to see if there is one in that result set that matches all the null properties. If there is, the uniqueness is violated, otherwise all is good. It could be improved by checking the data type of the columns that are null, and if they are String, using a criteria isNull restriction. that way the only properties being filtered by Grails rather than GORM are the null properties where the data type is not String. Cuts down the cases where criteria.list has to be used.
        Hide
        Neill Robbins added a comment -

        Patch needs reviewing

        Show
        Neill Robbins added a comment - Patch needs reviewing
        Hide
        Graeme Rocher added a comment -
        Show
        Graeme Rocher added a comment - fixed. thanks for the contribution. Test committed here https://github.com/SpringSource/grails-data-mapping/commit/45276f323d5836b7cc99e2cb5b1d2e6e2669d18f
        Hide
        Wynand Mooy added a comment -

        There still seems to be a problem with unique constraints containing a null column.
        I ran into the problem that validation passes, but the insertion into the (Oracle) database fails due to unique constraint validation (as it should)

        I can reproduce with these example classes.
        The unit tests work fine, but when I run-app the UserClass it behaves differently.

        1) New UserClass "A", no dataDeleted
        This works fine, of course.
        2) New Userclass, same parameters "A", no dateDeleted
        This works fine too, but it shouldn't (seems like the in-memory database doesn't care about the duplicate insert, but that must be 'bug/feature' of the db implementation. Oracle will throw an exception here in a 'real' application)

        In the sql logging you can see the root of the problem:

        The validation creates the sql query (twice actually, for some reason):
        DEBUG hibernate.SQL - select this_.id as id0_0_, this_.version as version0_0_, this_.date_deleted as date3_0_0_, this_.user_id as user4_0_0_ from user_class this_ where this_.user_id=? and this_.date_deleted=?

        date_deleted=? is not going to find any matches for NULL.

        So, for some reason it does not get into the code that was added above at all.

        I am currently working around this bug by using a customvalidator to do my own duplicate checking

        Show
        Wynand Mooy added a comment - There still seems to be a problem with unique constraints containing a null column. I ran into the problem that validation passes, but the insertion into the (Oracle) database fails due to unique constraint validation (as it should) I can reproduce with these example classes. The unit tests work fine, but when I run-app the UserClass it behaves differently. 1) New UserClass "A", no dataDeleted This works fine, of course. 2) New Userclass, same parameters "A", no dateDeleted This works fine too, but it shouldn't (seems like the in-memory database doesn't care about the duplicate insert, but that must be 'bug/feature' of the db implementation. Oracle will throw an exception here in a 'real' application) In the sql logging you can see the root of the problem: The validation creates the sql query (twice actually, for some reason): DEBUG hibernate.SQL - select this_.id as id0_0_, this_.version as version0_0_, this_.date_deleted as date3_0_0_, this_.user_id as user4_0_0_ from user_class this_ where this_.user_id=? and this_.date_deleted=? date_deleted=? is not going to find any matches for NULL. So, for some reason it does not get into the code that was added above at all. I am currently working around this bug by using a customvalidator to do my own duplicate checking

          People

          • Assignee:
            Graeme Rocher
            Reporter:
            Neill Robbins
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:
              Last Reviewed:

              Development