From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at> |
Cc: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: Wrong security context for deferred triggers? |
Date: | 2025-01-22 22:12:41 |
Message-ID: | 4166712.1737583961@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
I wrote:
> * It's kind of sad that we have to add yet another field to
> AfterTriggerSharedData, especially since this one will cost 8 bytes
> thanks to alignment considerations. I'm not sure if there's another
> way, but it seems like ats_relid, ats_rolid, and ats_modifiedcols
> are all going to be extremely redundant in typical scenarios.
> Maybe it's worth rethinking that data structure a bit? Or maybe
> I'm worried over nothing; perhaps normal situations have only a few
> AfterTriggerSharedDatas anyway. It might be worth trying to gather
> some statistics about that.
I poked at this a little bit by adding some debug printout and
running check-world. Since most of our test queries are designed
to not run too long, that doesn't give a representative impression
of how many trigger events get queued per query, but I think it's
probably fair as a sample of the number of distinct triggers that
might be involved in one query. Here's what I got for just the
core regression tests (the check-world numbers are bigger but
similar):
count event
1079 new entry added to 0 existing entries
342 new entry added to 1 existing entries
43 new entry added to 2 existing entries
21 new entry added to 3 existing entries
13 new entry added to 4 existing entries
10 new entry added to 5 existing entries
7 new entry added to 6 existing entries
5 new entry added to 7 existing entries
3 new entry added to 8 existing entries
2 new entry added to 9 existing entries
162 match after comparing 1 of 1 entries
30 match after comparing 1 of 2 entries
1 match after comparing 1 of 3 entries
38 match after comparing 2 of 2 entries
1 match after comparing 2 of 3 entries
1 match after comparing 2 of 4 entries
2 match after comparing 3 of 3 entries
1 match after comparing 3 of 4 entries
1 match after comparing 3 of 5 entries
1 match after comparing 4 of 4 entries
1 match after comparing 8 of 8 entries
So there are no cases with more than 10 AfterTriggerSharedDatas,
and my worry about how big those are is overblown.
However ... I noticed from the above that we seem to be hitting
the worst-case match result (matching the last entry to check)
a lot, and I realized that afterTriggerAddEvent is searching the
array backward! It goes from oldest AfterTriggerSharedData to
newest. But it really ought to do the reverse, because the newest
entry is probably from the current query while the oldest ones
might well be from prior queries in the same transaction, which
will never match anymore.
After applying the attached quickie patch, I get
count event
1079 new entry added to 0 existing entries
342 new entry added to 1 existing entries
43 new entry added to 2 existing entries
21 new entry added to 3 existing entries
13 new entry added to 4 existing entries
10 new entry added to 5 existing entries
7 new entry added to 6 existing entries
5 new entry added to 7 existing entries
3 new entry added to 8 existing entries
2 new entry added to 9 existing entries
162 match after comparing 1 of 1 entries
38 match after comparing 1 of 2 entries
2 match after comparing 1 of 3 entries
1 match after comparing 1 of 4 entries
1 match after comparing 1 of 8 entries
30 match after comparing 2 of 2 entries
1 match after comparing 2 of 3 entries
1 match after comparing 2 of 4 entries
1 match after comparing 3 of 3 entries
1 match after comparing 3 of 4 entries
1 match after comparing 3 of 5 entries
which seems less inefficient. As said, you would never
notice this effect in short queries, but for queries that
queue a lot of events it could be important.
regards, tom lane
Attachment | Content-Type | Size |
---|---|---|
reverse-AfterTriggerSharedData-search-order.patch | text/x-diff | 1.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Jeff Davis | 2025-01-22 22:42:54 | Re: Proposal: "query_work_mem" GUC, to distribute working memory to the query's individual operators |
Previous Message | Jeff Davis | 2025-01-22 22:12:35 | Re: Update Unicode data to Unicode 16.0.0 |