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

compound foreign key with on delete set null vs. not null columns [CORE3037] #3418

Open
firebird-automations opened this issue Jun 10, 2010 · 12 comments

Comments

@firebird-automations
Copy link
Collaborator

Submitted by: Philip Williams (unordained)

If a compound foreign key is set to on-delete-set-null, it would be nice if it would not attempt to nullify not-null columns that may be part of other constraints; if even a single field is nullable, being set to null is sufficient to nullify the reference and allow the original deletion. If no fields are nullable, I agree that should be an error (for that matter, it could be an error at constraint-creation time.)

Nearest reports I could find (not exact match) were CORE935 and CORE77.

Test script below:

create table departments (
dept_name varchar(30) not null,
primary key (dept_name)
);

create table employees (
dept_name varchar(30) not null,
emp_name varchar(30) not null,
boss_name varchar(30),
primary key (dept_name, emp_name),
foreign key (dept_name, boss_name) references employees (dept_name, emp_name) on update cascade on delete set null
);

commit;

insert into departments (dept_name) values ('IT');
insert into employees (dept_name, emp_name, boss_name) values ('IT', 'BigBoss', null);
insert into employees (dept_name, emp_name, boss_name) values ('IT', 'Underling', 'BigBoss');

commit;

delete from employees where dept_name = 'IT' and emp_name = 'BigBoss';
-- validation error for column DEPT_NAME, value "*** null ***"
rollback;

update employees set boss_name = null where dept_name = 'IT' and emp_name = 'Underling';
-- no error
rollback;

Yes, the above could be achieved by repeating the dept_name column, but if it is a business rule that they always be the same, that means adding CHECK constraints and/or triggers to support the rule; the simpler solution (for the end-user, that is) is for the database to recognize that because a single field being NULL is enough to eliminate the relationship, it follows that CASCADE SET NULL should only require a single field to be NULLed to have done its job.

@firebird-automations
Copy link
Collaborator Author

Commented by: Sean Leyne (seanleyne)

The example usage doesn't make sense!

The FK rules dictate that if a row is deleted and related rows should have the boss_name AND *dept_name* set to null, which is exactly against the constraints.

@firebird-automations
Copy link
Collaborator Author

Commented by: Philip Williams (unordained)

They only dictate that because of the current cascade implementation, which is exactly what I'm suggesting be changed. The current situation can't be a justification for itself.

The FK rule, in theory, could just as easily be read as "when a boss is removed, make sure no employees are still referring to him" which is sufficiently accomplished by just setting the boss_name to null and leaving dept_name alone, as I do manually in the last part of the example.

I think it's also in line with what a user would expect to happen, given the table layout; they want to delete an employee, who happens to be a boss; why should they have to manually NULL fields when there's already a cascade rule that ought to do it for them, and in their experience elsewhere (non-compound keys, or compound keys not partially reused), would do exactly what they want?

The FK constraint clearly doesn't require that if boss_name is null, dept_name must also be null. So it's only the cascade trigger, not the foreign key constraint, that is in the way.

@firebird-automations
Copy link
Collaborator Author

Commented by: Philip Williams (unordained)

To go further, instead of auto-detecting which fields are nullable, the syntax could be extended:

on delete set [fieldname][, fieldname]* null

Even if 3 of 5 fields in a compound FK are nullable, the user only needs 1 nulled to void the FK reference. They could want to pick which ones to null, rather than let the DB guess. By default the syntax would be the same as it currently is.

But I have a feeling instead we'll tell people to setup the FK constraint plus a before-delete trigger to jump the gun and nullify fields before the cascade rule has a chance to fire? I dislike the concept of outsourcing what should be the FK-cascade trigger's job; isn't that why there's also a proposal to move exception messages into FK constraints (CORE736) even though in theory that could be done with a bunch of extra triggers? (But not quite as well -- built-in constraints can see into other transactions, while custom triggers can't. They're not perfectly equivalent, and that's why I'm requesting this. Maybe someone can prove that in this situation, triggers would be perfect replacements.)

@firebird-automations
Copy link
Collaborator Author

Commented by: Sean Leyne (seanleyne)

What you are looking for is outside of the SQL Standard specification. As such, your argument needs to start with the SQL Committee.

The Firebird project is generally very "conservative" when it comes to implementing features which are not based on the standard -- cross-database compatibility is a high priority for the project.

While it might mean that more "vanilla" SQL code is required for a solution, it means that this code will be portable/interoperable with many other engines, which is significant benefit for our developers/users.

@firebird-automations
Copy link
Collaborator Author

Commented by: Philip Williams (unordained)

I make no claim that it's standard (I tagged it as an improvement, not a bug), though I'm having trouble actually finding exactly what is required by the standard (sql 92, section E141, I think -- does anyone have a copy?) Would appreciate it if there were some proof that triggers, "vanilla sql" really were completely equivalent to enhanced FK triggers. I haven't seen any docs that describe in detail the order in which FB performs these triggers in complex situations (multiple child tables, multiple FK's from same child table, grandchild tables, etc.) so I can't tell what the actual effect would be.

Meta: SQL compliance isn't a hard rule for the project, obviously and thankfully, so I'll be content if this stays open as "will think about it" for years. Not everything has to get closed / won't fix.

@firebird-automations
Copy link
Collaborator Author

Commented by: @hvlad

Does ON DELETE SET DEFAULT suit your needs ? Just don't forget to define DEFAULT value for fields

@firebird-automations
Copy link
Collaborator Author

Commented by: Philip Williams (unordained)

Interesting thought, so I tried it:

create table departments (
dept_name varchar(30) not null,
primary key (dept_name)
);

create table employees (
dept_name varchar(30) not null,
emp_name varchar(30) not null,
boss_name varchar(30) default null,
primary key (dept_name, emp_name),
foreign key (dept_name, boss_name) references employees (dept_name, emp_name) on update cascade on delete set default
);

commit;

insert into departments (dept_name) values ('IT');
insert into employees (dept_name, emp_name, boss_name) values ('IT', 'BigBoss', null);
insert into employees (dept_name, emp_name, boss_name) values ('IT', 'Underling', 'BigBoss');

commit;

delete from employees where dept_name = 'IT' and emp_name = 'BigBoss';
-- fails, still tries to set dept_name to null

The algorithm for SET DEFAULT seems to be "for every column of the foreign key, set to column's default if specified, to null otherwise". You can't set a default on dept_name (you want it to be whatever it current is, not some single default value), you can only set it on boss_name, where DEFAULT NULL is redundant anyway.

@firebird-automations
Copy link
Collaborator Author

Commented by: @hvlad

> You can't set a default on dept_name (you want it to be whatever it current is, not some single default value)

I don't agree there. You want to leave dept_name pointing to not existing value, it is not how FK should work.

@firebird-automations
Copy link
Collaborator Author

Commented by: Philip Williams (unordained)

Vlad, are you saying that after deleting the boss (BigBoss), the employee row (dept_name, emp_name, boss_name) ('IT', 'Underline, null) is invalid? Because it's not, according to standard FK rules, and it's not, according to Firebird. You can null just one field of an FK, leave the others alone, and not have a violation, so long as the other fields validate on their own. Deleting BigBoss doesn't delete the department, so leaving dept_name = 'IT' is entirely valid -- that department still exists (i forgot to include that constraint, but I promise it wouldn't change the result.) Remember, if any field of an FK is null, the whole FK is treated as null, and validates. At no point is it "pointing to a not existing value" -- the FK is null because one field is null, the PK is valid, and the missing single-field FK (dept_name -> dept_name) would validate too. I hate to keep repeating myself, but clearly this isn't a feature of FKs that most people use, but it is standard (the semi-null part, not necessarily the on-delete-set-null-sorta part.)

@firebird-automations
Copy link
Collaborator Author

Commented by: @asfernandes

From your initial example, Oracle gives same result as Firebird:

delete from employees where dept_name = 'IT' and emp_name = 'BigBoss'
*
ERROR at line 1:
ORA-01407: cannot update ("SYS"."EMPLOYEES"."DEPT_NAME") to NULL

@firebird-automations
Copy link
Collaborator Author

Commented by: @hvlad

> Vlad, are you saying that after deleting the boss (BigBoss), the employee row (dept_name, emp_name, boss_name) ('IT', 'Underline, null) is invalid?

Ooops, i should be more attentive, sorry. You deleted from employee, not from departments.
I should note, that whole example seems wrong to me - dept_name should not be in PK of employee. This is the source of issues in your example.

But generally speaking your requst is valid, but we should act according to standard. And standard have <match type> clause, probably for what you want here :

<referential constraint definition> ::=
FOREIGN KEY <left paren> <referencing columns> <right paren>
<references specification>

<references specification> ::=
REFERENCES <referenced table and columns>
[ MATCH <match type> ]
[ <referential triggered action> ]

<match type> ::=
FULL
| PARTIAL
| SIMPLE

...

By default <match type> is SIMPLE and this is what we have implemented in Firebird.
After quick reading of standard i have impression (probably wrong) that MATCH PARTIAL is what you need, but i'm not sure.

@firebird-automations
Copy link
Collaborator Author

Commented by: Philip Williams (unordained)

Adriano is correct. I'm not saying anyone else is implementing this.

Vlad; sorry about the concocted example. Gotta start somewhere though.

My understanding of the standard was that MATCH controls the FK constraint and how it treats NULL, not how the cascade operation works (though it might do so as well.) If you have a copy of that section, I really wouldn't mind having a copy too (private email), but I think FULL and PARTIAL will try to do NULL=NULL (the way Paradox does) or "if NULL, ignore when matching" (icky!)

My request is not to change the current FK constraint behavior at all -- it should remain SIMPLE, or at the very least my request only applies to SIMPLE, and if Firebird wants to implement the others, that's fine but irrelevant. My goal is only for deleting BigBoss to cascade to the same result as if I had updated that one field (not both) to be NULL, which is itself valid under current constraint behavior, but you have to do it manually before deleting BigBoss. I don't want the constraint to start seeing partially-NULL FK's as referring to anything, or treating NULL any differently than it does now.

But again, having the standard to see the implications of MATCH would be handy. Thanks for your careful interest!

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

No branches or pull requests

1 participant