ladon manager/sql: hard Postres 9.5 dependency

[djg2002]

ladon manager/sql uses new Postgres 9.5 upsert feature: 'ON CONFLICT DO NOTHING'.

e.g.:
INSERT INTO ladon_policy (id, description, effect, conditions) VALUES (?, ?, ?, ?) ON CONFLICT DO NOTHING

Because of this we can't bootstrap Hydra with the temporary root client - we use Postgres 9.3 in production and are unable to upgrade in the near-term.

Is there a workaround?
Or could the insert statements that use this feature be amended to make them more database/version agnostic?

[aeneasr]

While this isn't required for the policy itself (implying that it can be removed there) it is required for the other fields such as subject and action - because otherwise the whole transaction will fail if there's a duplicate in there (which is not unlikely to happen). I'm not a postgres expert so I'm not sure if there's a good workaround for this, the internet might be smarter than me here :)

[aeneasr]

Actually, it is required for the policy as well, otherwise the migration will not work (from previous ladon to this ladon). There's isn't an easy way to fix that without having to code a lot. If you're looking for a custom solution for your company, we have development power for hire, just drop us a line at hi@ory.am . Contributions in that direction are also welcome, but will need significant amount of planning beforehand so we don't lose BC.

[djg2002]

After checking a bit more I see 'ON CONFLICT DO NOTHING' isn't actually an upsert - its functionally equivalent to:

INSERT INTO ladon_policy (id, description, effect, conditions)
SELECT 'xyz', 'test_description', 'allow', 'test_conditions' 
WHERE NOT EXISTS (
		    SELECT 1
		    FROM   ladon_policy
		    WHERE  id = 'xyz'
		 )

Similarly for the migration queries:

INSERT INTO ladon_subject (id, has_regex, compiled, template)
SELECT 'abc', true, 'test_compiled', 'test_template' 
WHERE NOT EXISTS (
		    SELECT 1
		    FROM   ladon_subject
		    WHERE  id = 'abc'
		 )

INSERT INTO ladon_action (id, has_regex, compiled, template)
SELECT '123', true, 'test_compiled', 'test_template' 
WHERE NOT EXISTS (
		    SELECT 1
		    FROM   ladon_action
		    WHERE  id = '123'
		 )

INSERT INTO ladon_policy_subject_rel (policy, subject)
SELECT 'xyz', 'abc'
WHERE NOT EXISTS (
		    SELECT 1
		    FROM   ladon_policy_subject_rel
		    WHERE  policy = 'xyz' 
		       AND subject = 'abc'
		 )
etc.

Note should also work for MySQL (and most other databases), so the switch statements could be removed.

Would it be possible to use this approach instead?

[aeneasr]

Yeah looks good, would you mind doing a PR?

…

Am 17.05.2017 um 23:00 schrieb djg2002 ***@***.***>: After checking a bit more I see 'ON CONFLICT DO NOTHING' isn't actually an upsert - its functionally equivalent to: INSERT INTO ladon_policy (id, description, effect, conditions) SELECT 'xyz', 'test_description', 'allow', 'test_conditions' WHERE NOT EXISTS ( SELECT 1 FROM ladon_policy WHERE id = 'xyz' ) Similarly for the migration queries: INSERT INTO ladon_subject (id, has_regex, compiled, template) SELECT 'abc', true, 'test_compiled', 'test_template' WHERE NOT EXISTS ( SELECT 1 FROM ladon_subject WHERE id = 'abc' ) INSERT INTO ladon_action (id, has_regex, compiled, template) SELECT '123', true, 'test_compiled', 'test_template' WHERE NOT EXISTS ( SELECT 1 FROM ladon_action WHERE id = '123' ) INSERT INTO ladon_policy_subject_rel (policy, subject) SELECT 'xyz', 'abc' WHERE NOT EXISTS ( SELECT 1 FROM ladon_policy_subject_rel WHERE policy = 'xyz' AND subject = 'abc' ) etc. Note should also work for MySQL (and most other databases), so the switch statements could be removed. Would it be possible to use this approach instead? — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[djg2002]

Sure - I worked on a fix today and should be able to test and commit this weekend.

[djg2002]

Can you allow branch access to I can commit / create pull request

[aeneasr]

@djg2002 please create a fork and use that to make your pull request: https://help.github.com/articles/fork-a-repo/

[vfiebig]

It does not work for MySQL

[aeneasr]

@vfiebig could you elaborate what exactly isn't working? The patch passed the tests

[vfiebig]

"WHERE NOT EXISTS" sintaxe just doesn't work with MySQL 5.6.24 as it works with Postgres:
Could not create admin policy because Error 1064: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'WHERE NOT EXISTS (SELECT 1 FROM ladon_policy WHERE id = ?)' at line 1

[aeneasr]

Oh ok, it seems to be working with MySQL 5.7 though, which is why the tests aren't failing. As a first step, we could lower the mysql version to 4.5 or something to make it compatible with older mysql instances