public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 6/6] Ignore correlation for new BRIN opclasses
2+ messages / 2 participants
[nested] [flat]
* [PATCH 6/6] Ignore correlation for new BRIN opclasses
@ 2020-09-12 13:07 Tomas Vondra <[email protected]>
0 siblings, 0 replies; 2+ messages in thread
From: Tomas Vondra @ 2020-09-12 13:07 UTC (permalink / raw)
The new BRIN opclasses (bloom and minmax-multi) are less sensitive to
poorly correlated data, so just assume the data is perfectly correlated
during costing.
Author: Tomas Vondra <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
src/backend/access/brin/brin_bloom.c | 1 +
src/backend/access/brin/brin_minmax_multi.c | 1 +
src/backend/utils/adt/selfuncs.c | 19 ++++++++++++++++++-
src/include/access/brin_internal.h | 3 +++
4 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/src/backend/access/brin/brin_bloom.c b/src/backend/access/brin/brin_bloom.c
index 000c2387ee..0137095660 100644
--- a/src/backend/access/brin/brin_bloom.c
+++ b/src/backend/access/brin/brin_bloom.c
@@ -385,6 +385,7 @@ brin_bloom_opcinfo(PG_FUNCTION_ARGS)
result = palloc0(MAXALIGN(SizeofBrinOpcInfo(1)) +
sizeof(BloomOpaque));
+ result->oi_ignore_correlation = true;
result->oi_nstored = 1;
result->oi_regular_nulls = true;
result->oi_opaque = (BloomOpaque *)
diff --git a/src/backend/access/brin/brin_minmax_multi.c b/src/backend/access/brin/brin_minmax_multi.c
index bd17da8cd5..6ef5c5f2bf 100644
--- a/src/backend/access/brin/brin_minmax_multi.c
+++ b/src/backend/access/brin/brin_minmax_multi.c
@@ -1309,6 +1309,7 @@ brin_minmax_multi_opcinfo(PG_FUNCTION_ARGS)
result = palloc0(MAXALIGN(SizeofBrinOpcInfo(1)) +
sizeof(MinmaxMultiOpaque));
+ result->oi_ignore_correlation = true;
result->oi_nstored = 1;
result->oi_regular_nulls = true;
result->oi_opaque = (MinmaxMultiOpaque *)
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index d5e61664bc..d0cc145938 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -98,6 +98,7 @@
#include <math.h>
#include "access/brin.h"
+#include "access/brin_internal.h"
#include "access/brin_page.h"
#include "access/gin.h"
#include "access/table.h"
@@ -7352,7 +7353,8 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count,
double minimalRanges;
double estimatedRanges;
double selec;
- Relation indexRel;
+ Relation indexRel = NULL;
+ TupleDesc tupdesc = NULL;
ListCell *l;
VariableStatData vardata;
@@ -7374,6 +7376,7 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count,
*/
indexRel = index_open(index->indexoid, NoLock);
brinGetStats(indexRel, &statsData);
+ tupdesc = RelationGetDescr(indexRel);
index_close(indexRel, NoLock);
/* work out the actual number of ranges in the index */
@@ -7407,6 +7410,17 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count,
{
IndexClause *iclause = lfirst_node(IndexClause, l);
AttrNumber attnum = index->indexkeys[iclause->indexcol];
+ FmgrInfo *opcInfoFn;
+ BrinOpcInfo *opcInfo;
+ Form_pg_attribute attr = TupleDescAttr(tupdesc, iclause->indexcol);
+ bool ignore_correlation;
+
+ opcInfoFn = index_getprocinfo(indexRel, iclause->indexcol + 1, BRIN_PROCNUM_OPCINFO);
+
+ opcInfo = (BrinOpcInfo *)
+ DatumGetPointer(FunctionCall1(opcInfoFn, attr->atttypid));
+
+ ignore_correlation = opcInfo->oi_ignore_correlation;
/* attempt to lookup stats in relation for this index column */
if (attnum != 0)
@@ -7477,6 +7491,9 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count,
if (sslot.nnumbers > 0)
varCorrelation = Abs(sslot.numbers[0]);
+ if (ignore_correlation)
+ varCorrelation = 1.0;
+
if (varCorrelation > *indexCorrelation)
*indexCorrelation = varCorrelation;
diff --git a/src/include/access/brin_internal.h b/src/include/access/brin_internal.h
index fdaff42722..5fbf8cf9c7 100644
--- a/src/include/access/brin_internal.h
+++ b/src/include/access/brin_internal.h
@@ -30,6 +30,9 @@ typedef struct BrinOpcInfo
/* Regular processing of NULLs in BrinValues? */
bool oi_regular_nulls;
+ /* Ignore correlation during cost estimation */
+ bool oi_ignore_correlation;
+
/* Opaque pointer for the opclass' private use */
void *oi_opaque;
--
2.26.2
--------------310A2AE1CC4C2E2E77559E3D--
^ permalink raw reply [nested|flat] 2+ messages in thread
* Re: Latches vs lwlock contention
@ 2023-09-28 09:58 Heikki Linnakangas <[email protected]>
0 siblings, 0 replies; 2+ messages in thread
From: Heikki Linnakangas @ 2023-09-28 09:58 UTC (permalink / raw)
To: Thomas Munro <[email protected]>; pgsql-hackers; +Cc: Yura Sokolov <[email protected]>; Andres Freund <[email protected]>
On 28/10/2022 06:56, Thomas Munro wrote:
> One example is heavyweight lock wakeups. If you run BEGIN; LOCK TABLE
> t; ... and then N other sessions wait in SELECT * FROM t;, and then
> you run ... COMMIT;, you'll see the first session wake all the others
> while it still holds the partition lock itself. They'll all wake up
> and begin to re-acquire the same partition lock in exclusive mode,
> immediately go back to sleep on*that* wait list, and then wake each
> other up one at a time in a chain. We could avoid the first
> double-bounce by not setting the latches until after we've released
> the partition lock. We could avoid the rest of them by not
> re-acquiring the partition lock at all, which ... if I'm reading right
> ... shouldn't actually be necessary in modern PostgreSQL? Or if there
> is another reason to re-acquire then maybe the comment should be
> updated.
ISTM that the change to not re-aqcuire the lock in ProcSleep is
independent from the other changes. Let's split that off to a separate
patch.
I agree it should be safe. Acquiring a lock just to hold off interrupts
is overkill anwyway, HOLD_INTERRUPTS() would be enough.
LockErrorCleanup() uses HOLD_INTERRUPTS() already.
There are no CHECK_FOR_INTERRUPTS() in GrantAwaitedLock(), so cancel/die
interrupts can't happen here. But could we add HOLD_INTERRUPTS(), just
pro forma, to document the assumption? It's a little awkward: you really
should hold interrupts until the caller has done "awaitedLock = NULL;".
So it's not quite enough to add a pair of HOLD_ and RESUME_INTERRUPTS()
at the end of ProcSleep(). You'd need to do the HOLD_INTERRUPTS() in
ProcSleep() and require the caller to do RESUME_INTERRUPTS(). In a
sense, ProcSleep downgrades the lock on the partition to just holding
off interrupts.
Overall +1 on this change to not re-acquire the partition lock.
--
Heikki Linnakangas
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2023-09-28 09:58 UTC | newest]
Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-09-12 13:07 [PATCH 6/6] Ignore correlation for new BRIN opclasses Tomas Vondra <[email protected]>
2023-09-28 09:58 Re: Latches vs lwlock contention Heikki Linnakangas <[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