public inbox for [email protected]help / color / mirror / Atom feed
[PATCH 1/8] introduce bsearch_arg 5+ messages / 5 participants [nested] [flat]
* [PATCH 1/8] introduce bsearch_arg @ 2021-03-04 23:16 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Tomas Vondra @ 2021-03-04 23:16 UTC (permalink / raw) --- src/backend/statistics/extended_stats.c | 31 -------------- src/include/port.h | 5 +++ .../statistics/extended_stats_internal.h | 5 --- src/port/Makefile | 1 + src/port/bsearch_arg.c | 40 +++++++++++++++++++ src/tools/msvc/Mkvcbuild.pm | 2 +- 6 files changed, 47 insertions(+), 37 deletions(-) create mode 100644 src/port/bsearch_arg.c diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c index a030ea3653..fa42851fd5 100644 --- a/src/backend/statistics/extended_stats.c +++ b/src/backend/statistics/extended_stats.c @@ -659,37 +659,6 @@ compare_datums_simple(Datum a, Datum b, SortSupport ssup) return ApplySortComparator(a, false, b, false, ssup); } -/* simple counterpart to qsort_arg */ -void * -bsearch_arg(const void *key, const void *base, size_t nmemb, size_t size, - int (*compar) (const void *, const void *, void *), - void *arg) -{ - size_t l, - u, - idx; - const void *p; - int comparison; - - l = 0; - u = nmemb; - while (l < u) - { - idx = (l + u) / 2; - p = (void *) (((const char *) base) + (idx * size)); - comparison = (*compar) (key, p, arg); - - if (comparison < 0) - u = idx; - else if (comparison > 0) - l = idx + 1; - else - return (void *) p; - } - - return NULL; -} - /* * build_attnums_array * Transforms a bitmap into an array of AttrNumber values. diff --git a/src/include/port.h b/src/include/port.h index 227ef4b148..82f63de325 100644 --- a/src/include/port.h +++ b/src/include/port.h @@ -508,6 +508,11 @@ typedef int (*qsort_arg_comparator) (const void *a, const void *b, void *arg); extern void qsort_arg(void *base, size_t nel, size_t elsize, qsort_arg_comparator cmp, void *arg); +extern void *bsearch_arg(const void *key, const void *base, + size_t nmemb, size_t size, + int (*compar) (const void *, const void *, void *), + void *arg); + /* port/chklocale.c */ extern int pg_get_encoding_from_locale(const char *ctype, bool write_message); diff --git a/src/include/statistics/extended_stats_internal.h b/src/include/statistics/extended_stats_internal.h index c849bd57c0..a0a3cf5b0f 100644 --- a/src/include/statistics/extended_stats_internal.h +++ b/src/include/statistics/extended_stats_internal.h @@ -85,11 +85,6 @@ extern int multi_sort_compare_dims(int start, int end, const SortItem *a, extern int compare_scalars_simple(const void *a, const void *b, void *arg); extern int compare_datums_simple(Datum a, Datum b, SortSupport ssup); -extern void *bsearch_arg(const void *key, const void *base, - size_t nmemb, size_t size, - int (*compar) (const void *, const void *, void *), - void *arg); - extern AttrNumber *build_attnums_array(Bitmapset *attrs, int *numattrs); extern SortItem *build_sorted_items(int numrows, int *nitems, HeapTuple *rows, diff --git a/src/port/Makefile b/src/port/Makefile index e41b005c4f..52dbf5783f 100644 --- a/src/port/Makefile +++ b/src/port/Makefile @@ -40,6 +40,7 @@ LIBS += $(PTHREAD_LIBS) OBJS = \ $(LIBOBJS) \ $(PG_CRC32C_OBJS) \ + bsearch_arg.o \ chklocale.o \ erand48.o \ inet_net_ntop.o \ diff --git a/src/port/bsearch_arg.c b/src/port/bsearch_arg.c new file mode 100644 index 0000000000..d24dc4b7c4 --- /dev/null +++ b/src/port/bsearch_arg.c @@ -0,0 +1,40 @@ +/* + * bsearch_arg.c: bsearch variant with a user-supplied pointer + * + * src/port/bsearch_arg.c + */ + + +#include "c.h" + + +/* simple counterpart to qsort_arg */ +void * +bsearch_arg(const void *key, const void *base, size_t nmemb, size_t size, + int (*compar) (const void *, const void *, void *), + void *arg) +{ + size_t l, + u, + idx; + const void *p; + int comparison; + + l = 0; + u = nmemb; + while (l < u) + { + idx = (l + u) / 2; + p = (void *) (((const char *) base) + (idx * size)); + comparison = (*compar) (key, p, arg); + + if (comparison < 0) + u = idx; + else if (comparison > 0) + l = idx + 1; + else + return (void *) p; + } + + return NULL; +} diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm index 49614106dc..20221c1ae3 100644 --- a/src/tools/msvc/Mkvcbuild.pm +++ b/src/tools/msvc/Mkvcbuild.pm @@ -101,7 +101,7 @@ sub mkvcbuild dirent.c dlopen.c getopt.c getopt_long.c link.c pread.c preadv.c pwrite.c pwritev.c pg_bitutils.c pg_strong_random.c pgcheckdir.c pgmkdirp.c pgsleep.c pgstrcasecmp.c - pqsignal.c mkdtemp.c qsort.c qsort_arg.c quotes.c system.c + pqsignal.c mkdtemp.c qsort.c qsort_arg.c bsearch_arg.c quotes.c system.c strerror.c tar.c thread.c win32env.c win32error.c win32security.c win32setlocale.c win32stat.c); -- 2.26.2 --------------556A1DC61262AF15641DB204 Content-Type: text/x-patch; charset=UTF-8; name="0002-Pass-all-scan-keys-to-BRIN-consistent-func-20210308b.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0002-Pass-all-scan-keys-to-BRIN-consistent-func-20210308b.pa"; filename*1="tch" ^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Trigger violates foreign key constraint @ 2023-10-02 13:49 Tom Lane <[email protected]> 2023-10-03 07:24 ` Re: Trigger violates foreign key constraint Laurenz Albe <[email protected]> 2023-10-08 18:17 ` Re: Trigger violates foreign key constraint Noah Misch <[email protected]> 0 siblings, 2 replies; 5+ messages in thread From: Tom Lane @ 2023-10-02 13:49 UTC (permalink / raw) To: Laurenz Albe <[email protected]>; +Cc: pgsql-hackers Laurenz Albe <[email protected]> writes: > CREATE FUNCTION silly() RETURNS trigger LANGUAGE plpgsql AS 'BEGIN RETURN NULL; END;'; > CREATE TRIGGER silly BEFORE DELETE ON child FOR EACH ROW EXECUTE FUNCTION silly(); > The trigger function cancels the cascaded delete on "child", and we are left with > a row in "child" that references no row in "parent". Yes. This is by design: triggers operate at a lower level than foreign keys, so an ill-conceived trigger can break an FK constraint. That's documented somewhere, though maybe not visibly enough. There are good reasons to want triggers to be able to see and react to FK-driven updates, so it's unlikely that we'd want to revisit that design decision, even if it hadn't already stood for decades. regards, tom lane ^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Trigger violates foreign key constraint 2023-10-02 13:49 Re: Trigger violates foreign key constraint Tom Lane <[email protected]> @ 2023-10-03 07:24 ` Laurenz Albe <[email protected]> 2023-10-30 21:50 ` Re: Trigger violates foreign key constraint David G. Johnston <[email protected]> 1 sibling, 1 reply; 5+ messages in thread From: Laurenz Albe @ 2023-10-03 07:24 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: pgsql-hackers On Mon, 2023-10-02 at 09:49 -0400, Tom Lane wrote: > This is by design: triggers operate at a lower level than > foreign keys, so an ill-conceived trigger can break an FK constraint. > That's documented somewhere, though maybe not visibly enough. Not having found any documentation, I propose the attached caution. Yours, Laurenz Albe Attachments: [text/x-patch] 0001-Document-foreign-key-internals.patch (1.4K, ../../[email protected]/2-0001-Document-foreign-key-internals.patch) download | inline diff: From b6abd7dfdf1e25515ead092489efde0d239f7053 Mon Sep 17 00:00:00 2001 From: Laurenz Albe <[email protected]> Date: Tue, 3 Oct 2023 09:20:54 +0200 Subject: [PATCH] Document foreign key internals Warn the user that foreign keys are implemented as triggers, and that user-defined triggers can interact with them and break referential integrity. Discussion: https://postgr.es/m/b81fe38fcc25a81be6e2e5b3fc1ff624130762fa.camel%40cybertec.at --- doc/src/sgml/ddl.sgml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml index 075ff32991..8016b9fd81 100644 --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml @@ -1224,6 +1224,18 @@ CREATE TABLE posts ( syntax in the reference documentation for <xref linkend="sql-createtable"/>. </para> + + <note> + <para> + Foreign key constraints are implemented as system triggers in PostgreSQL. + As such, they are subject to the trigger firing rules described in + <xref linkend="trigger-definition"/>. In particular, other triggers + defined on the referencing table can cancel or modify the effects of + cascading delete or update, thereby breaking referential integrity. + This is not considered a bug, and it is the responsibility of the user + to write triggers so that such problems are avoided. + </para> + </note> </sect2> <sect2 id="ddl-constraints-exclusion"> -- 2.41.0 ^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Trigger violates foreign key constraint 2023-10-02 13:49 Re: Trigger violates foreign key constraint Tom Lane <[email protected]> 2023-10-03 07:24 ` Re: Trigger violates foreign key constraint Laurenz Albe <[email protected]> @ 2023-10-30 21:50 ` David G. Johnston <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: David G. Johnston @ 2023-10-30 21:50 UTC (permalink / raw) To: Laurenz Albe <[email protected]>; +Cc: Tom Lane <[email protected]>; pgsql-hackers On Tue, Oct 3, 2023 at 12:52 AM Laurenz Albe <[email protected]> wrote: > On Mon, 2023-10-02 at 09:49 -0400, Tom Lane wrote: > > This is by design: triggers operate at a lower level than > > foreign keys, so an ill-conceived trigger can break an FK constraint. > > That's documented somewhere, though maybe not visibly enough. > > Not having found any documentation, I propose the attached caution. > > I dislike scaring the user like this without providing any context on what conditions or actions are problematic. The ON DELETE and ON UPDATE clauses of foreign keys are implemented as system triggers on the referenced table that invoke additional delete or update commands on the referencing table. The final outcome of these additional commands are not checked - it is the responsibility of the DBA to ensure that the user triggers on the referencing table actually remove the rows they are requested to remove, or update to NULL any referencing foreign key columns. In particular, before row triggers that return NULL will prevent the delete/update from occurring and thus result in a violated foreign key constraint. Add sgml as needed, note the original patch missed adding "<productname>" to PostgreSQL. David J. ^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Trigger violates foreign key constraint 2023-10-02 13:49 Re: Trigger violates foreign key constraint Tom Lane <[email protected]> @ 2023-10-08 18:17 ` Noah Misch <[email protected]> 1 sibling, 0 replies; 5+ messages in thread From: Noah Misch @ 2023-10-08 18:17 UTC (permalink / raw) To: Laurenz Albe <[email protected]>; Tom Lane <[email protected]>; +Cc: pgsql-hackers On Mon, Oct 02, 2023 at 09:49:53AM -0400, Tom Lane wrote: > Laurenz Albe <[email protected]> writes: > > CREATE FUNCTION silly() RETURNS trigger LANGUAGE plpgsql AS 'BEGIN RETURN NULL; END;'; > > CREATE TRIGGER silly BEFORE DELETE ON child FOR EACH ROW EXECUTE FUNCTION silly(); > > > The trigger function cancels the cascaded delete on "child", and we are left with > > a row in "child" that references no row in "parent". > > Yes. This is by design: triggers operate at a lower level than > foreign keys, so an ill-conceived trigger can break an FK constraint. > That's documented somewhere, though maybe not visibly enough. > > There are good reasons to want triggers to be able to see and > react to FK-driven updates, I agree with that, but I also think it's a bug that other triggers can invalidate the constraint, without even going out of their way to do so. Ideally, triggers would be able to react, yet when all non-superuser-defined code settles, the constraint would still hold. While UNIQUE indexes over expressions aren't that strict, at least for those you need to commit the clear malfeasance of redefining an IMMUTABLE function. On Mon, Oct 02, 2023 at 12:02:17PM +0200, Laurenz Albe wrote: > Perhaps it would be enough to run "RI_FKey_noaction_del" after > "RI_FKey_cascade_del", although that would impact the performance. Yes. A cure that doubles the number of heap fetches would be worse than the disease, but a more-optimized version of this idea could work. The FK system could use a broader optimization-oriented rewrite, to deal with the unbounded memory usage and redundant I/O. If that happens, it could be a good time to plan for closing the trigger hole. ^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2023-10-30 21:50 UTC | newest] Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2021-03-04 23:16 [PATCH 1/8] introduce bsearch_arg Tomas Vondra <[email protected]> 2023-10-02 13:49 Re: Trigger violates foreign key constraint Tom Lane <[email protected]> 2023-10-03 07:24 ` Re: Trigger violates foreign key constraint Laurenz Albe <[email protected]> 2023-10-30 21:50 ` Re: Trigger violates foreign key constraint David G. Johnston <[email protected]> 2023-10-08 18:17 ` Re: Trigger violates foreign key constraint Noah Misch <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox