Wrench should not add `ON DELETE NO ACTION` syntax

graeme-verticalscope commented 1 year ago

Expected Behavior

When a table is created with a foreign key constraint and no foreign key action is provided, wrench should not add ON DELETE NO ACTION to the create table statement.

Current Behavior

Wrench adds ON DELETE NO ACTION, changing the statement syntax.

Steps to Reproduce

Create 2 migration scripts script 1:

CREATE TABLE Table1 (
  Table1ID STRING(MAX) NOT NULL,
) PRIMARY KEY(Table1ID);

script 2:

CREATE TABLE Table2 (
    Table2ID STRING(MAX) NOT NULL,
    Table1ID STRING(MAX) NOT NULL,
    CONSTRAINT FK_Table1Table2 FOREIGN KEY(Table1ID) REFERENCES Table1(Table1ID)
) PRIMARY KEY(Table2ID);

Create a local spanner instance with cloud spanner emulator
Apply migrations to cloud spanner emulator using wrench.
Second migration fails with the following error:
```
Foreign key referential action ON DELETE NO ACTION is not supported.
```
This error demonstrates that ON DELETE NO ACTION is being added by wrench. If it wasn't being added by wrench, then second migration would succeed.

Context (Environment)

wrench version: v1.6.0
possibly caused by dependency cloud.google.com/go/spanner upgrade from v1.47.0 to v1.49.0
- I can reproduce with wrench v1.5.0 if I upgrade cloud.google.com/go/spanner to v1.49.0.

This change breaks my local and CI environments that use the emulator:

The emulator does not accept create table statements with foreign key actions
I cannot remove the foreign key action, since it's being added by wrench.

I have already created an issue in cloud spanner emulator: https://github.com/GoogleCloudPlatform/cloud-spanner-emulator/issues/142 but I was told the fix should be on the wrench side, not the emulator side.

toga4 commented 1 year ago

Hi @graeme-verticalscope,

As you might have suspected, this behavior isn't actually due to wrench but is caused by cloud.google.com/go/spanner.

When wrench applies migrations, it uses the spansql package from cloud.google.com/go/spanner to parse and re-output the DDL. This spansql automatically adds ON DELETE NO ACTION as its default behavior when it parses a DDL that doesn't explicitly provide a foreign key action. (Regrettably, I was who implemented this behavior on https://github.com/googleapis/google-cloud-go/pull/8296. My apologies.)

I'll create a Pull Request on cloud.google.com/go/spanner to address this behavior. Once a new version of cloud.google.com/go/spanner including this fix is released, upgrading to that version should resolve the issue.

graeme-verticalscope commented 1 year ago

Great, thanks!

graeme-verticalscope commented 11 months ago

hey @toga4 do you know when the fix might get released for this?

I'm stuck on older versions of a few google dependencies until this is fixed, because they all try to upgrade google-cloud-go/spanner which breaks my CI flow.

toga4 commented 11 months ago

Hi @graeme-verticalscope, I have already created a PR at https://github.com/googleapis/google-cloud-go/pull/8968 addressing this issue. However, since I am not a maintainer of that repository, I'm unable to influence its review process directly. It would be greatly appreciated if you could comment on the PR to encourage the maintainers to review it.

graeme-verticalscope commented 11 months ago

Found a workaround to enable upgrading wrench to v1.6.0. Spanner emulator version v1.5.10 works (although older or newer spanner emulator versions don't work).

See https://github.com/GoogleCloudPlatform/cloud-spanner-emulator/issues/147#issuecomment-1839184657

cloudspannerecosystem / wrench