authzed / spicedb

Open Source, Google Zanzibar-inspired permissions database to enable fine-grained authorization for customer applications
https://authzed.com/docs
Apache License 2.0
4.71k stars 250 forks source link

Fix/bulk loader nullstring #1945

Closed heissa83 closed 1 week ago

heissa83 commented 2 weeks ago

Fixes #1938 and also fixes problems like it in the future by using IS DISTINCT FROMinstead of <> to compare a potential null value field. (This will make it so that already bulk loaded relations will work correctly).

github-actions[bot] commented 2 weeks ago

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

heissa83 commented 2 weeks ago

I have read the CLA Document and I hereby sign the CLA

heissa83 commented 2 weeks ago

recheck

heissa83 commented 2 weeks ago

I added a new test that checks that caveated relations can be updated after they were added by a BulkLoad. I also did the negative test locally and both changes i made fix the problem. (The test also showed that i was using the wrong column name in the first place, i also fixed that)

josephschorr commented 1 week ago

@heissa83 Looks like there is a broken test

heissa83 commented 1 week ago

Fixed the tests. Sorry for some reason my docker daemon keeps crashing when i run the complete test suite locally.

heissa83 commented 1 week ago

@vroldanbet do you still have a change request? If not can this PR be merged as it is right now?