sql: compatibility with ORY Hydra

[glerchundi]

Is this a bug report or a feature request?

Bug report.

BUG REPORT

Please describe the issue you observed, and any steps we can take to reproduce it:

• Which version of CockroachDB are you using?

v2.0.4

• What did you do?

Try to use ORY Hydra with CockroachDB as database. When trying to apply the migrations with v1.0.0-beta.7 by doing: hydra migrate sql postgres://root:@127.0.0.1:26257/hydra?sslmode=disable it failed in the forth migration which executes these sentences:

UPDATE hydra_client SET sector_identifier_uri='', jwks='', jwks_uri='', request_uris=''
ALTER TABLE hydra_client ALTER COLUMN sector_identifier_uri SET NOT NULL
ALTER TABLE hydra_client ALTER COLUMN jwks SET NOT NULL
ALTER TABLE hydra_client ALTER COLUMN jwks_uri SET NOT NULL
ALTER TABLE hydra_client ALTER COLUMN request_uris SET NOT NULL

The first sentence failed due to sql_safe_updates was true by default, that can be solved easily. But the next ones failed with this (each one):

root@:26257/hydra> ALTER TABLE hydra_client ALTER COLUMN request_uris SET NOT NULL;
invalid syntax: statement ignored: unimplemented at or near "null"
DETAIL: source SQL:
ALTER TABLE hydra_client ALTER COLUMN request_uris SET NOT NULL;

• What did you expect to see?

To work without problems.

• What did you see instead?

$ hydra migrate sql postgres://root:@127.0.0.1:26257/hydra?sslmode=disable
Applying `oauth2` SQL migrations...
Applied 0 `oauth2` SQL migrations.
Applying `jwk` SQL migrations...
Applied 0 `jwk` SQL migrations.
Applying `consent` SQL migrations...
Applied 0 `consent` SQL migrations.
Applying `client` SQL migrations...
An error occurred while running the migrations: Could not apply `client` SQL migrations: Could not migrate sql schema, applied 0 Migrations: pq: unimplemented at or near "null" handling 4

• What was the impact?

Cannot use start using ORY Hydra which is blocker for us.

[glerchundi]

It's unsupported? https://www.cockroachlabs.com/docs/stable/alter-column.html

[glerchundi]

After inserting the migration manually I tried to do the migration again and it worked:

$ hydra migrate sql postgres://root:@127.0.0.1:26257/hydra?sslmode=disable
Applying `client` SQL migrations...
Applied 0 `client` SQL migrations.
Applying `oauth2` SQL migrations...
Applied 0 `oauth2` SQL migrations.
Applying `jwk` SQL migrations...
Applied 0 `jwk` SQL migrations.
Applying `consent` SQL migrations...
Applied 0 `consent` SQL migrations.
Migration successful! Applied a total of 0 SQL migrations.
Migration successful!

So it seems that if we can solve this issue it would work.

[axife]

I have hydra running for few weeks with cockroachdb. I had to run migration two-three times to get it working. (like described in that issue) But once it failed to start at all. I had to drop database and start from beginning.

[glerchundi]

@axife how could be this possible? Hydra executes this (ALTER TABLE hydra_client ALTER COLUMN table SET NOT NULL SQL statement which ATM isn't supported by CockroachDB.

Can you provide which versions are you using?

[axife]

@glerchundi I am not certain may be I was not passing ALTER TABLE hydra_client ALTER COLUMN table SET NOT NULL I have updated hydra today and it broke almost everything PORT -> PUBLIC_PORT env variable, command line arguments, etc BUT I did one change today hydra/client/manager_sql.go line 144-148.

			`UPDATE hydra_client SET sector_identifier_uri='', jwks='', jwks_uri='', request_uris=''`,
			`SELECT 1`, // `ALTER TABLE hydra_client ALTER COLUMN sector_identifier_uri SET NOT NULL`,
			`SELECT 1`, // `ALTER TABLE hydra_client ALTER COLUMN jwks SET NOT NULL`,
			`SELECT 1`, // `ALTER TABLE hydra_client ALTER COLUMN jwks_uri SET NOT NULL`,
			`SELECT 1`, // `ALTER TABLE hydra_client ALTER COLUMN request_uris SET NOT NULL`,

I think there are high chances that NOT NULL is not changing code flow. If database will accept NULL but code will not assign NULLs then everything should be fine in theory.

I was debating about alternative migrate routine vs such path.

[knz]

Support for ALTER TABLE ALTER COLUMN SET NOT NULL is tracked separately in #28751.

[knz]

cc @BramGruneir for triage

[glerchundi]

@knz thanks for pushing this, I can confirm that except for the ALTER COLUMN ... SET NOT NULL thing Hydra v1.0.0-beta.8 (the latest one), is working properly.

[glerchundi]

Is it possible to associate an ETA to this? It would be really helpful for us.

[knz]

The ALTER COLUMN SET TYPE NOT NULL will not come before CockroachDB 2.2, Q2 2019; possibly after.

[glerchundi]

Ok, thanks for putting in a timeframe even if it wasn’t accurate.

…

On Fri, 7 Sep 2018 at 13:06, kena ***@***.***> wrote: The ALTER COLUMN SET TYPE NOT NULL will not come before CockroachDB 2.2, Q2 2019; possibly after. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#28415 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACIPlqdYteJ-o6hxlQQBddsKR5lhkbZYks5uYlNSgaJpZM4V2XxB> .

[knz]

My pleasure! Please be in touch with our product management if you want to influence this schedule.

[glerchundi]

@lopezator did a fantastic job contributing to ORY Hydra in order to add direct support for CockroachDB, lets give him a little bit of love.

Thanks David! 👏

[lopezator]

Thank you @glerchundi !! It wouldn't have been possible without your support 👍