Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow constraint name re-use in diff file after dropping (Oracle) #171

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hazardv
Copy link
Contributor

@hazardv hazardv commented Feb 27, 2024

This appears to be an issue only if you are using DBIx::Class::Schema and calling create_ddl_dir with a previous version specified. With that being said...

If you have an Oracle schema with tables/constraints identified as version 0.1:

CREATE TABLE app_category (
  id number(11) NOT NULL,
  name varchar2(256) NOT NULL,
  description varchar2(4000),
  PRIMARY KEY (id)
);

CREATE TABLE app_alert (
  id number(11) NOT NULL,
  category number(11) NOT NULL,
  name varchar2(512) NOT NULL,
  PRIMARY KEY (id)
);

ALTER TABLE app_alert ADD CONSTRAINT app_alert_category_fk FOREIGN KEY (category) REFERENCES app_category (id);

and in version 0.2 you change the app_alert_category_fk constraint to cascade deletes

ALTER TABLE app_alert ADD CONSTRAINT app_alert_category_fk FOREIGN KEY (category) REFERENCES app_category (id) ON DELETE CASCADE;

but all other things remain the same, I would expect the diff file that gets generated by $schema->create_ddl_dir() to look like this:

ALTER TABLE app_alert DROP CONSTRAINT app_alert_category_fk;

ALTER TABLE app_alert ADD CONSTRAINT app_alert_category_fk FOREIGN KEY (category) REFERENCES app_category (id) ON DELETE CASCADE;

This is the way it works when you are NOT using DBIx::Class::Schema.

When using DBIx::Class::Schema, the producer has already produced the SQL from the perl and has all of the constraint names in the %global_names hash of the Oracle producer so when it's handling the diff things get a bit messy. Instead of the expected diff above, you end up with

ALTER TABLE app_alert DROP CONSTRAINT app_alert_category_fk;

ALTER TABLE app_alert ADD CONSTRAINT app_alert_category_fk01 FOREIGN KEY (category) REFERENCES app_category (id) ON DELETE CASCADE;

This is an issue because the version 0.2 schema has

ALTER TABLE app_alert ADD CONSTRAINT app_alert_category_fk FOREIGN KEY (category) REFERENCES app_category (id) ON DELETE CASCADE;

so if you went directly to version 0.2 your constraint is named app_alert_category_fk and if you upgraded from version 0.1 to 0.2 then your constraint is named app_alert_category_fk01.

My proposed change involves decrementing the value in %global_names when a constraint is dropped so that the name can be reused.

@hazardv
Copy link
Contributor Author

hazardv commented Feb 27, 2024

I realize I don't have any tests to prove this or prove how the change fixes the issue. I have a separate super minimal project that I used to test it on. I didn't know if you wanted me to include that because it requires all of DBIx::Class.

@rabbiveesh
Copy link
Contributor

ya, let's not require DBIC for our test suite. I think you should be able to just make a test which sets up this situation artificially, with comments explaining how this would happen IRL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants