Grails

Unique constraint erroneously finds match when grouped property value is null

Details

  • Type: Bug Bug
  • Status: Closed 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. File
    UserClass.groovy
    24/Jan/12 9:20 AM
    0.2 kB
    Neill Robbins
  2. File
    UserClassTests.groovy
    24/Jan/12 9:20 AM
    2 kB
    Neill Robbins

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Neill Robbins added a comment -

Attached github pull request for fix:
https://github.com/SpringSource/grails-data-mapping/commit/93797e8066b11017a2e7cc4f2ab65f7d54251a07

Show
Neill Robbins added a comment - Attached github pull request for fix: https://github.com/SpringSource/grails-data-mapping/commit/93797e8066b11017a2e7cc4f2ab65f7d54251a07
Hide
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
Neill Robbins added a comment -

Patch needs reviewing

Show
Neill Robbins added a comment - Patch needs reviewing
Hide
Permalink
Graeme Rocher added a comment -

fixed. thanks for the contribution. Test committed here

https://github.com/SpringSource/grails-data-mapping/commit/45276f323d5836b7cc99e2cb5b1d2e6e2669d18f

Show
Graeme Rocher added a comment - fixed. thanks for the contribution. Test committed here https://github.com/SpringSource/grails-data-mapping/commit/45276f323d5836b7cc99e2cb5b1d2e6e2669d18f
Hide
Permalink
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
Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved:
    Last Reviewed: