Thread: Re: NOT ENFORCED constraint feature
On Tue, Oct 8, 2024, at 11:06, Amul Sul wrote: > The attached patch proposes adding the ability to define CHECK and > FOREIGN KEY constraints as NOT ENFORCED. Thanks for working on this! > Adding NOT ENFORCED to CHECK constraints is simple, see 0001 patch, I've looked at the 0001 patch and think it looks simple and straight forward. > but implementing it for FOREIGN KEY constraints requires more code due > to triggers, see 0002 - 0005 patches. I can't say that much yet about the code changes in 0002 - 0005 yet, but I've tested the patches and successfully experimented with the feature. Also think the documentation is good and sound. Only found a minor typo: - Adding a enforced <literal>CHECK</literal> or <literal>NOT NULL</literal> + Adding an enforced <literal>CHECK</literal> or <literal>NOT NULL</literal> > There are various approaches for > implementing NOT ENFORCED foreign keys, what I thought of: > > 1. When defining a NOT ENFORCED foreign key, skip the creation of > triggers used for referential integrity check, while defining an > ENFORCED foreign key, remain the same as the current behaviour. If an > existing foreign key is changed to NOT ENFORCED, the triggers are > dropped, and when switching it back to ENFORCED, the triggers are > recreated. > > 2. Another approach could be to create the NOT ENFORCED constraint > with the triggers as usual, but disable those triggers by updating the > pg_trigger catalog so that they are never executed for the check. And > enable them when the constraint is changed back to ENFORCED. > > 3. Similarly, a final approach would involve updating the logic where > trigger execution is decided and skipping the execution if the > constraint is not enforced, rather than modifying the pg_trigger > catalog. > > In the attached patch, the first approach has been implemented. This > requires more code changes but prevents unused triggers from being > left in the database and avoids the need for changes all over the > place to skip trigger execution, which could be missed in future code > additions. I also like the first approach, since I think it's nice the pg_trigger entires are inserted / deleted upon enforced / not enforced. > The ALTER CONSTRAINT operation in the patch added code to handle > dropping and recreating triggers. An alternative approach would be to > simplify the process by dropping and recreating the FK constraint, > which would automatically handle skipping or creating triggers for NOT > ENFORCED or ENFORCED FK constraints. However, I wasn't sure if this > was the right approach, as I couldn't find any existing ALTER > operations that follow this pattern. I think the current approach of dropping and recreating the triggers is best, since if we would instead be dropping and recreating the FK constraint, that would cause problems if some other future SQL feature would need to introduce dependencies on the FK constraints via pg_depend. Best regards, Joel
On 2024-10-09 We 5:14 AM, Joel Jacobson wrote:
On Tue, Oct 8, 2024, at 11:06, Amul Sul wrote:The attached patch proposes adding the ability to define CHECK and FOREIGN KEY constraints as NOT ENFORCED.Thanks for working on this!Adding NOT ENFORCED to CHECK constraints is simple, see 0001 patch,I've looked at the 0001 patch and think it looks simple and straight forward.but implementing it for FOREIGN KEY constraints requires more code due to triggers, see 0002 - 0005 patches.I can't say that much yet about the code changes in 0002 - 0005 yet, but I've tested the patches and successfully experimented with the feature. Also think the documentation is good and sound. Only found a minor typo: - Adding a enforced <literal>CHECK</literal> or <literal>NOT NULL</literal> + Adding an enforced <literal>CHECK</literal> or <literal>NOT NULL</literal>There are various approaches for implementing NOT ENFORCED foreign keys, what I thought of: 1. When defining a NOT ENFORCED foreign key, skip the creation of triggers used for referential integrity check, while defining an ENFORCED foreign key, remain the same as the current behaviour. If an existing foreign key is changed to NOT ENFORCED, the triggers are dropped, and when switching it back to ENFORCED, the triggers are recreated. 2. Another approach could be to create the NOT ENFORCED constraint with the triggers as usual, but disable those triggers by updating the pg_trigger catalog so that they are never executed for the check. And enable them when the constraint is changed back to ENFORCED. 3. Similarly, a final approach would involve updating the logic where trigger execution is decided and skipping the execution if the constraint is not enforced, rather than modifying the pg_trigger catalog. In the attached patch, the first approach has been implemented. This requires more code changes but prevents unused triggers from being left in the database and avoids the need for changes all over the place to skip trigger execution, which could be missed in future code additions.I also like the first approach, since I think it's nice the pg_trigger entires are inserted / deleted upon enforced / not enforced.
I also prefer this, as it gets us out from any possible dance with enabling / disabling triggers.
cheers
andrew
-- Andrew Dunstan EDB: https://www.enterprisedb.com
On Wed, Oct 9, 2024 at 2:44 PM Joel Jacobson <joel@compiler.org> wrote: > > On Tue, Oct 8, 2024, at 11:06, Amul Sul wrote: > > The attached patch proposes adding the ability to define CHECK and > > FOREIGN KEY constraints as NOT ENFORCED. > > Thanks for working on this! > > > Adding NOT ENFORCED to CHECK constraints is simple, see 0001 patch, > > I've looked at the 0001 patch and think it looks simple and straight forward. > Thanks for looking into it. > > but implementing it for FOREIGN KEY constraints requires more code due > > to triggers, see 0002 - 0005 patches. > > I can't say that much yet about the code changes in 0002 - 0005 yet, > but I've tested the patches and successfully experimented with the feature. > > Also think the documentation is good and sound. Only found a minor typo: > - Adding a enforced <literal>CHECK</literal> or <literal>NOT NULL</literal> > + Adding an enforced <literal>CHECK</literal> or <literal>NOT NULL</literal> > Ok, will fix it in the next version. > > There are various approaches for > > implementing NOT ENFORCED foreign keys, what I thought of: > > > > 1. When defining a NOT ENFORCED foreign key, skip the creation of > > triggers used for referential integrity check, while defining an > > ENFORCED foreign key, remain the same as the current behaviour. If an > > existing foreign key is changed to NOT ENFORCED, the triggers are > > dropped, and when switching it back to ENFORCED, the triggers are > > recreated. > > > > 2. Another approach could be to create the NOT ENFORCED constraint > > with the triggers as usual, but disable those triggers by updating the > > pg_trigger catalog so that they are never executed for the check. And > > enable them when the constraint is changed back to ENFORCED. > > > > 3. Similarly, a final approach would involve updating the logic where > > trigger execution is decided and skipping the execution if the > > constraint is not enforced, rather than modifying the pg_trigger > > catalog. > > > > In the attached patch, the first approach has been implemented. This > > requires more code changes but prevents unused triggers from being > > left in the database and avoids the need for changes all over the > > place to skip trigger execution, which could be missed in future code > > additions. > > I also like the first approach, since I think it's nice the pg_trigger > entires are inserted / deleted upon enforced / not enforced. > > > The ALTER CONSTRAINT operation in the patch added code to handle > > dropping and recreating triggers. An alternative approach would be to > > simplify the process by dropping and recreating the FK constraint, > > which would automatically handle skipping or creating triggers for NOT > > ENFORCED or ENFORCED FK constraints. However, I wasn't sure if this > > was the right approach, as I couldn't find any existing ALTER > > operations that follow this pattern. > > I think the current approach of dropping and recreating the triggers is best, > since if we would instead be dropping and recreating the FK constraint, > that would cause problems if some other future SQL feature would need to > introduce dependencies on the FK constraints via pg_depend. > Yes, that was my initial thought as well, and recreating the dependencies would be both painful and prone to bugs. Regards, Amul
On Wed, Oct 9, 2024 at 6:45 PM Andrew Dunstan <andrew@dunslane.net> wrote: > > > On 2024-10-09 We 5:14 AM, Joel Jacobson wrote: > > On Tue, Oct 8, 2024, at 11:06, Amul Sul wrote: > > The attached patch proposes adding the ability to define CHECK and > FOREIGN KEY constraints as NOT ENFORCED. > > Thanks for working on this! > > Adding NOT ENFORCED to CHECK constraints is simple, see 0001 patch, > > I've looked at the 0001 patch and think it looks simple and straight forward. > > but implementing it for FOREIGN KEY constraints requires more code due > to triggers, see 0002 - 0005 patches. > > I can't say that much yet about the code changes in 0002 - 0005 yet, > but I've tested the patches and successfully experimented with the feature. > > Also think the documentation is good and sound. Only found a minor typo: > - Adding a enforced <literal>CHECK</literal> or <literal>NOT NULL</literal> > + Adding an enforced <literal>CHECK</literal> or <literal>NOT NULL</literal> > > There are various approaches for > implementing NOT ENFORCED foreign keys, what I thought of: > > 1. When defining a NOT ENFORCED foreign key, skip the creation of > triggers used for referential integrity check, while defining an > ENFORCED foreign key, remain the same as the current behaviour. If an > existing foreign key is changed to NOT ENFORCED, the triggers are > dropped, and when switching it back to ENFORCED, the triggers are > recreated. > > 2. Another approach could be to create the NOT ENFORCED constraint > with the triggers as usual, but disable those triggers by updating the > pg_trigger catalog so that they are never executed for the check. And > enable them when the constraint is changed back to ENFORCED. > > 3. Similarly, a final approach would involve updating the logic where > trigger execution is decided and skipping the execution if the > constraint is not enforced, rather than modifying the pg_trigger > catalog. > > In the attached patch, the first approach has been implemented. This > requires more code changes but prevents unused triggers from being > left in the database and avoids the need for changes all over the > place to skip trigger execution, which could be missed in future code > additions. > > I also like the first approach, since I think it's nice the pg_trigger > entires are inserted / deleted upon enforced / not enforced. > > > > I also prefer this, as it gets us out from any possible dance with enabling / disabling triggers. > Agreed. Thanks for the inputs. Regards, Amul.