pg.ddx.io pgsql-committers@postgresql.org mailing list archive
help / color / mirror / Atom feedpgsql: Transform OR clauses to ANY expression
7+ messages / 5 participants
[nested] [flat]
Commits from this thread:
2024-04-30 96c7381c4c Fix error message in check_partition_bounds_for_split_range()
2024-04-08 b453a7a16a Fix the wording of or_to_any_transform_limit description
2024-04-07 beabea6c20 Fix usage of same ListCell transform_or_to_any()'s in nested loops
* pgsql: Transform OR clauses to ANY expression
@ 2024-04-07 22:28 Alexander Korotkov <akorotkov@postgresql.org>
0 siblings, 2 replies; 7+ messages in thread
From: Alexander Korotkov @ 2024-04-07 22:28 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Transform OR clauses to ANY expression
Replace (expr op C1) OR (expr op C2) ... with expr op ANY(ARRAY[C1, C2, ...])
on the preliminary stage of optimization when we are still working with the
expression tree.
Here Cn is a n-th constant expression, 'expr' is non-constant expression, 'op'
is an operator which returns boolean result and has a commuter (for the case
of reverse order of constant and non-constant parts of the expression,
like 'Cn op expr').
Sometimes it can lead to not optimal plan. This is why there is a
or_to_any_transform_limit GUC. It specifies a threshold value of length of
arguments in an OR expression that triggers the OR-to-ANY transformation.
Generally, more groupable OR arguments mean that transformation will be more
likely to win than to lose.
Discussion: https://postgr.es/m/567ED6CA.2040504%40sigaev.ru
Author: Alena Rybakina <lena.ribackina@yandex.ru>
Author: Andrey Lepikhov <a.lepikhov@postgrespro.ru>
Reviewed-by: Peter Geoghegan <pg@bowt.ie>
Reviewed-by: Ranier Vilela <ranier.vf@gmail.com>
Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com>
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Reviewed-by: Jian He <jian.universality@gmail.com>
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/72bd38cc99a15da6f97373fae98027c908c398ea
Modified Files
--------------
doc/src/sgml/config.sgml | 57 ++++
src/backend/nodes/queryjumblefuncs.c | 27 ++
src/backend/optimizer/prep/prepqual.c | 399 +++++++++++++++++++++++++-
src/backend/utils/misc/guc_tables.c | 12 +
src/backend/utils/misc/postgresql.conf.sample | 1 +
src/include/nodes/queryjumble.h | 1 +
src/include/optimizer/optimizer.h | 2 +
src/test/regress/expected/create_index.out | 159 ++++++++++
src/test/regress/expected/join.out | 50 ++++
src/test/regress/expected/partition_prune.out | 37 +--
src/test/regress/sql/create_index.sql | 45 +++
src/test/regress/sql/join.sql | 11 +
src/test/regress/sql/partition_prune.sql | 2 +
src/tools/pgindent/typedefs.list | 2 +
14 files changed, 785 insertions(+), 20 deletions(-)
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: pgsql: Transform OR clauses to ANY expression
@ 2024-04-07 22:35 Melanie Plageman <melanieplageman@gmail.com>
parent: Alexander Korotkov <akorotkov@postgresql.org>
1 sibling, 1 reply; 7+ messages in thread
From: Melanie Plageman @ 2024-04-07 22:35 UTC (permalink / raw)
To: Alexander Korotkov <akorotkov@postgresql.org>; +Cc: pgsql-committers@lists.postgresql.org
On Sun, Apr 7, 2024 at 6:28 PM Alexander Korotkov
<akorotkov@postgresql.org> wrote:
>
> Transform OR clauses to ANY expression
>
> Replace (expr op C1) OR (expr op C2) ... with expr op ANY(ARRAY[C1, C2, ...])
> on the preliminary stage of optimization when we are still working with the
> expression tree.
>
> Here Cn is a n-th constant expression, 'expr' is non-constant expression, 'op'
> is an operator which returns boolean result and has a commuter (for the case
> of reverse order of constant and non-constant parts of the expression,
> like 'Cn op expr').
>
> Sometimes it can lead to not optimal plan. This is why there is a
> or_to_any_transform_limit GUC. It specifies a threshold value of length of
> arguments in an OR expression that triggers the OR-to-ANY transformation.
> Generally, more groupable OR arguments mean that transformation will be more
> likely to win than to lose.
I'm getting this warning now
/src/backend/optimizer/prep/prepqual.c:582:33: warning: declaration of
‘lc__state’ shadows a previous local [-Wshadow=compatible-local]
582 | foreach(lc, entry->consts)
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: pgsql: Transform OR clauses to ANY expression
@ 2024-04-07 22:38 Alexander Korotkov <aekorotkov@gmail.com>
parent: Melanie Plageman <melanieplageman@gmail.com>
0 siblings, 1 reply; 7+ messages in thread
From: Alexander Korotkov @ 2024-04-07 22:38 UTC (permalink / raw)
To: Melanie Plageman <melanieplageman@gmail.com>; +Cc: Alexander Korotkov <akorotkov@postgresql.org>; pgsql-committers@lists.postgresql.org
On Mon, Apr 8, 2024 at 1:35 AM Melanie Plageman
<melanieplageman@gmail.com> wrote:
> On Sun, Apr 7, 2024 at 6:28 PM Alexander Korotkov
> <akorotkov@postgresql.org> wrote:
> >
> > Transform OR clauses to ANY expression
> >
> > Replace (expr op C1) OR (expr op C2) ... with expr op ANY(ARRAY[C1, C2, ...])
> > on the preliminary stage of optimization when we are still working with the
> > expression tree.
> >
> > Here Cn is a n-th constant expression, 'expr' is non-constant expression, 'op'
> > is an operator which returns boolean result and has a commuter (for the case
> > of reverse order of constant and non-constant parts of the expression,
> > like 'Cn op expr').
> >
> > Sometimes it can lead to not optimal plan. This is why there is a
> > or_to_any_transform_limit GUC. It specifies a threshold value of length of
> > arguments in an OR expression that triggers the OR-to-ANY transformation.
> > Generally, more groupable OR arguments mean that transformation will be more
> > likely to win than to lose.
>
> I'm getting this warning now
>
> /src/backend/optimizer/prep/prepqual.c:582:33: warning: declaration of
> ‘lc__state’ shadows a previous local [-Wshadow=compatible-local]
> 582 | foreach(lc, entry->consts)
Thank you for catching. I'm fixing this now.
------
Regards,
Alexander Korotkov
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: pgsql: Transform OR clauses to ANY expression
@ 2024-04-08 05:46 Kyotaro Horiguchi <horikyota.ntt@gmail.com>
parent: Alexander Korotkov <akorotkov@postgresql.org>
1 sibling, 1 reply; 7+ messages in thread
From: Kyotaro Horiguchi @ 2024-04-08 05:46 UTC (permalink / raw)
To: akorotkov@postgresql.org; +Cc: pgsql-committers@lists.postgresql.org
At Sun, 07 Apr 2024 22:28:06 +0000, Alexander Korotkov <akorotkov@postgresql.org> wrote in
> Transform OR clauses to ANY expression
This commit introduces a message like this:
> gettext_noop("Set the minimum length of the list of OR clauses to attempt the OR-to-ANY transformation."),
Unlike the usual phrasing of similar messages in this context, which
use the form "Sets the minimum length of...", this message uses an
imperative form ("Set"). I believe it would be better to align the
tone of this message with the others by changing "Set" to "Sets".
regards.
inline patch 1: src/backend/utils/misc/guc_tables.c (+1/-1 lines) download inline-1.patch
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 83e3a59d7e..a527ffe0b0 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -3670,7 +3670,7 @@ struct config_int ConfigureNamesInt[] =
{
{"or_to_any_transform_limit", PGC_USERSET, QUERY_TUNING_OTHER,
- gettext_noop("Set the minimum length of the list of OR clauses to attempt the OR-to-ANY transformation."),
+ gettext_noop("Sets the minimum length of the list of OR clauses to attempt the OR-to-ANY transformation."),
gettext_noop("Once the limit is reached, the planner will try to replace expression like "
"'x=c1 OR x=c2 ..' to the expression 'x = ANY(ARRAY[c1,c2,..])'"),
GUC_EXPLAIN
* Re: pgsql: Transform OR clauses to ANY expression
@ 2024-04-08 06:24 Kyotaro Horiguchi <horikyota.ntt@gmail.com>
parent: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
0 siblings, 1 reply; 7+ messages in thread
From: Kyotaro Horiguchi @ 2024-04-08 06:24 UTC (permalink / raw)
To: akorotkov@postgresql.org; +Cc: pgsql-committers@lists.postgresql.org
At Mon, 08 Apr 2024 14:46:57 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in
> At Sun, 07 Apr 2024 22:28:06 +0000, Alexander Korotkov <akorotkov@postgresql.org> wrote in
> > Transform OR clauses to ANY expression
>
> This commit introduces a message like this:
>
> > gettext_noop("Set the minimum length of the list of OR clauses to attempt the OR-to-ANY transformation."),
>
> Unlike the usual phrasing of similar messages in this context, which
> use the form "Sets the minimum length of...", this message uses an
> imperative form ("Set"). I believe it would be better to align the
> tone of this message with the others by changing "Set" to "Sets".
>
> regards.
>
>
> diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
> index 83e3a59d7e..a527ffe0b0 100644
> --- a/src/backend/utils/misc/guc_tables.c
> +++ b/src/backend/utils/misc/guc_tables.c
> @@ -3670,7 +3670,7 @@ struct config_int ConfigureNamesInt[] =
>
> {
> {"or_to_any_transform_limit", PGC_USERSET, QUERY_TUNING_OTHER,
> - gettext_noop("Set the minimum length of the list of OR clauses to attempt the OR-to-ANY transformation."),
> + gettext_noop("Sets the minimum length of the list of OR clauses to attempt the OR-to-ANY transformation."),
> gettext_noop("Once the limit is reached, the planner will try to replace expression like "
> "'x=c1 OR x=c2 ..' to the expression 'x = ANY(ARRAY[c1,c2,..])'"),
> GUC_EXPLAIN
Sorry for the sequential posts, but I found the following contruct in
the same patch to be quite untranslatable.
> errmsg("%s bound of partition \"%s\" is %s %s bound of split partition",
> first ? "lower" : "upper",
> relname,
> defaultPart ? (first ? "less than" : "greater than") : "not equals to",
> first ? "lower" : "upper"),
> parser_errposition(pstate, datum->location)));
I might be misunderstanding this, but if the expressions are correct,
the message could be divided into four fixed messages based on "first"
and "defaultPart". Alternatively, it might be better to provide
simpler explation of the situation.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: pgsql: Transform OR clauses to ANY expression
@ 2024-04-08 07:36 Alexander Korotkov <aekorotkov@gmail.com>
parent: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
0 siblings, 0 replies; 7+ messages in thread
From: Alexander Korotkov @ 2024-04-08 07:36 UTC (permalink / raw)
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; +Cc: akorotkov@postgresql.org; pgsql-committers@lists.postgresql.org
On Mon, Apr 8, 2024 at 9:24 AM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:
> At Mon, 08 Apr 2024 14:46:57 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in
> > At Sun, 07 Apr 2024 22:28:06 +0000, Alexander Korotkov <akorotkov@postgresql.org> wrote in
> > > Transform OR clauses to ANY expression
> >
> > This commit introduces a message like this:
> >
> > > gettext_noop("Set the minimum length of the list of OR clauses to attempt the OR-to-ANY transformation."),
> >
> > Unlike the usual phrasing of similar messages in this context, which
> > use the form "Sets the minimum length of...", this message uses an
> > imperative form ("Set"). I believe it would be better to align the
> > tone of this message with the others by changing "Set" to "Sets".
> >
> > regards.
> >
> >
> > diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
> > index 83e3a59d7e..a527ffe0b0 100644
> > --- a/src/backend/utils/misc/guc_tables.c
> > +++ b/src/backend/utils/misc/guc_tables.c
> > @@ -3670,7 +3670,7 @@ struct config_int ConfigureNamesInt[] =
> >
> > {
> > {"or_to_any_transform_limit", PGC_USERSET, QUERY_TUNING_OTHER,
> > - gettext_noop("Set the minimum length of the list of OR clauses to attempt the OR-to-ANY transformation."),
> > + gettext_noop("Sets the minimum length of the list of OR clauses to attempt the OR-to-ANY transformation."),
> > gettext_noop("Once the limit is reached, the planner will try to replace expression like "
> > "'x=c1 OR x=c2 ..' to the expression 'x = ANY(ARRAY[c1,c2,..])'"),
> > GUC_EXPLAIN
>
> Sorry for the sequential posts, but I found the following contruct in
> the same patch to be quite untranslatable.
No worries. But these are distinct patches.
> > errmsg("%s bound of partition \"%s\" is %s %s bound of split partition",
> > first ? "lower" : "upper",
> > relname,
> > defaultPart ? (first ? "less than" : "greater than") : "not equals to",
> > first ? "lower" : "upper"),
> > parser_errposition(pstate, datum->location)));
>
> I might be misunderstanding this, but if the expressions are correct,
> the message could be divided into four fixed messages based on "first"
> and "defaultPart". Alternatively, it might be better to provide
> simpler explation of the situation.
Yes, splitting this into multiple messages helps both translating and
readability. Will be fixed in the next couple of days.
------
Regards,
Alexander Korotkov
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: pgsql: Transform OR clauses to ANY expression
@ 2024-04-08 15:14 Jelte Fennema-Nio <me@jeltef.nl>
parent: Alexander Korotkov <aekorotkov@gmail.com>
0 siblings, 0 replies; 7+ messages in thread
From: Jelte Fennema-Nio @ 2024-04-08 15:14 UTC (permalink / raw)
To: Alexander Korotkov <aekorotkov@gmail.com>; +Cc: Melanie Plageman <melanieplageman@gmail.com>; Alexander Korotkov <akorotkov@postgresql.org>; pgsql-committers@lists.postgresql.org
On Mon, 8 Apr 2024 at 17:10, Alexander Korotkov <aekorotkov@gmail.com> wrote:
>
> On Mon, Apr 8, 2024 at 1:35 AM Melanie Plageman
> <melanieplageman@gmail.com> wrote:
> > /src/backend/optimizer/prep/prepqual.c:582:33: warning: declaration of
> > ‘lc__state’ shadows a previous local [-Wshadow=compatible-local]
> > 582 | foreach(lc, entry->consts)
>
> Thank you for catching. I'm fixing this now.
I noticed the fix in question, and I wanted to say that this whole
issue could've been avoided if the new foreach_ptr macros were used
(and thus arguably would have been a better way to fix this). Then
there wouldn't have been any ListCell shadowing, because no ListCell
would have been declared at all.
^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2024-04-08 15:14 UTC | newest]
Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-04-07 22:28 pgsql: Transform OR clauses to ANY expression Alexander Korotkov <akorotkov@postgresql.org>
2024-04-07 22:35 ` Melanie Plageman <melanieplageman@gmail.com>
2024-04-07 22:38 ` Alexander Korotkov <aekorotkov@gmail.com>
2024-04-08 15:14 ` Jelte Fennema-Nio <me@jeltef.nl>
2024-04-08 05:46 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2024-04-08 06:24 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2024-04-08 07:36 ` Alexander Korotkov <aekorotkov@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox