public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v13 02/18] f! progress reporting
2+ messages / 2 participants
[nested] [flat]
* [PATCH v13 02/18] f! progress reporting
@ 2021-02-15 00:31 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 2+ messages in thread
From: Justin Pryzby @ 2021-02-15 00:31 UTC (permalink / raw)
---
src/backend/commands/indexcmds.c | 33 +++++++-------------------------
1 file changed, 7 insertions(+), 26 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 4ac1dacd7d..d6567ec231 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -1638,40 +1638,20 @@ reindex_invalid_child_indexes(Oid indexRelationId)
.options = REINDEXOPT_CONCURRENTLY
};
- MemoryContext ind_context = AllocSetContextCreate(PortalContext, "CREATE INDEX",
- ALLOCSET_DEFAULT_SIZES);
- MemoryContext oldcontext;
- List *childs = find_inheritance_children(indexRelationId, ShareLock);
- List *partitions = NIL;
-
PreventInTransactionBlock(true, "REINDEX INDEX");
- foreach (lc, childs)
+ foreach (lc, find_inheritance_children(indexRelationId, ShareLock))
{
Oid partoid = lfirst_oid(lc);
- /* XXX: need to retrofit progress reporting into it */
- // pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_DONE,
- // npart++);
-
- if (get_index_isvalid(partoid) ||
- !RELKIND_HAS_STORAGE(get_rel_relkind(partoid)))
- continue;
+ if (!get_index_isvalid(partoid) &&
+ RELKIND_HAS_STORAGE(get_rel_relkind(partoid)))
+ ReindexRelationConcurrently(partoid, ¶ms);
- /* Save partition OID */
- oldcontext = MemoryContextSwitchTo(ind_context);
- partitions = lappend_oid(partitions, partoid);
- MemoryContextSwitchTo(oldcontext);
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_DONE,
+ npart++);
}
- /*
- * Process each partition listed in a separate transaction. Note that
- * this commits and then starts a new transaction immediately.
- * XXX: since this is done in 2*N transactions, it could just as well
- * call ReindexRelationConcurrently directly
- */
- ReindexMultipleInternal(partitions, ¶ms);
-
/*
* CIC needs to mark a partitioned index as VALID, which itself
* requires setting READY, which is unset for CIC (even though
@@ -1680,6 +1660,7 @@ reindex_invalid_child_indexes(Oid indexRelationId)
* partitions.
* See also: validatePartitionedIndex().
*/
+ CommandCounterIncrement();
index_set_state_flags(indexRelationId, INDEX_CREATE_SET_READY);
CommandCounterIncrement();
index_set_state_flags(indexRelationId, INDEX_CREATE_SET_VALID);
--
2.17.0
--QTprm0S8XgL7H0Dt
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v13-0003-WIP-Add-SKIPVALID-flag-for-more-integration.patch"
^ permalink raw reply [nested|flat] 2+ messages in thread
* Re: [PATCH] Add support function for containment operators
@ 2023-07-06 16:15 Laurenz Albe <[email protected]>
0 siblings, 0 replies; 2+ messages in thread
From: Laurenz Albe @ 2023-07-06 16:15 UTC (permalink / raw)
To: Kim Johan Andersson <[email protected]>; [email protected]
On Sat, 2023-04-29 at 17:07 +0200, Kim Johan Andersson wrote:
> I had noticed that performance wasn't great when using the @> or <@
> operators when examining if an element is contained in a range.
> Based on the discussion in [1] I would like to suggest the following
> changes:
>
> This patch attempts to improve the row estimation, as well as opening
> the possibility of using a btree index scan when using the containment
> operators.
>
> This is done via a new support function handling the following 2 requests:
>
> * SupportRequestIndexCondition
> find_index_quals will build an operator clause, given at least one
> finite RangeBound.
>
> * SupportRequestSimplify
> find_simplified_clause will rewrite the containment operator into a
> clause using inequality operators from the btree family (if available
> for the element type).
>
> A boolean constant is returned if the range is either empty or has no
> bounds.
>
> Performing the rewrite here lets the clausesel machinery provide the
> same estimates as for normal scalar inequalities.
>
> In both cases build_bound_expr is used to build the operator clauses
> from RangeBounds.
I think that this is a small, but useful improvement.
The patch applies and builds without warning and passes "make installcheck-world"
with the (ample) new regression tests.
Some comments:
- About the regression tests:
You are using EXPLAIN (ANALYZE, SUMMARY OFF, TIMING OFF, COSTS OFF).
While that returns stable results, I don't see the added value.
I think you should use EXPLAIN (COSTS OFF). You don't need to test the
actual number of result rows; we can trust that the executor processes
>= and < correctly.
Plain EXPLAIN would speed up the regression tests, which is a good thing.
- About the implementation:
You implement both "SupportRequestIndexCondition" and "SupportRequestSimplify",
but when I experimented, the former was never called. That does not
surprise me, since any expression of the shape "expr <@ range constant"
can be simplified. Is the "SupportRequestIndexCondition" branch dead code?
If not, do you have an example that triggers it?
- About the code:
+static Node *
+find_index_quals(Const *rangeConst, Expr *otherExpr, Oid opfamily)
+{
[...]
+
+ if (!(lower.infinite && upper.infinite))
+ {
[...]
+ }
+
+ return NULL;
To avoid deep indentation and to make the code more readable, I think
it would be better to write
if (!(lower.infinite && upper.infinite))
return NULL;
and unindent the rest of the code
+static Node *
+match_support_request(Node *rawreq)
+{
[...]
+ switch (req->funcid)
+ {
+ case F_ELEM_CONTAINED_BY_RANGE:
[...]
+ case F_RANGE_CONTAINS_ELEM:
[...]
+ default:
+ return NULL;
+ }
(This code appears twice.)
The default clause should not be reachable, right?
I think that there should be an Assert() to verify that.
Perhaps something like
Assert(req->funcid == F_ELEM_CONTAINED_BY_RANGE ||
req->funcid == F_RANGE_CONTAINS_ELEM);
if (req->funcid == F_ELEM_CONTAINED_BY_RANGE)
{
[...]
}
else if (req->funcid == F_RANGE_CONTAINS_ELEM)
{
[...]
}
Yours,
Laurenz Albe
^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2023-07-06 16:15 UTC | newest]
Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-02-15 00:31 [PATCH v13 02/18] f! progress reporting Justin Pryzby <[email protected]>
2023-07-06 16:15 Re: [PATCH] Add support function for containment operators Laurenz Albe <[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