public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 2/5] Allow TAP test to excecise tablespace.
38+ messages / 7 participants
[nested] [flat]
* [PATCH 2/5] Allow TAP test to excecise tablespace.
@ 2019-04-22 11:58 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 38+ messages in thread
From: Kyotaro Horiguchi @ 2019-04-22 11:58 UTC (permalink / raw)
To perform tablespace related checks, this patch lets
PostgresNode::backup have a new parameter "tablespace_mapping", and
make init_from_backup handle capable to handle a backup created using
tablespace_mapping.
---
src/test/perl/PostgresNode.pm | 45 +++++++++++++++++++++++++++++++++++++-----
src/test/perl/RecursiveCopy.pm | 38 +++++++++++++++++++++++++++++++----
2 files changed, 74 insertions(+), 9 deletions(-)
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index 76874141c5..e951acc461 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -157,6 +157,7 @@ sub new
_host => $pghost,
_basedir => "$TestLib::tmp_check/t_${testname}_${name}_data",
_name => $name,
+ _tablespaces => [],
_logfile_generation => 0,
_logfile_base => "$TestLib::log_path/${testname}_${name}",
_logfile => "$TestLib::log_path/${testname}_${name}.log"
@@ -342,6 +343,26 @@ sub backup_dir
=pod
+=item $node->make_tablespace_dir()
+
+make a tablespace directory
+
+=cut
+
+sub make_tablespace_dir
+{
+ my ($self, $name) = @_;
+ my $basedir = $self->basedir;
+
+ die "tablespace name contains '/'" if ($name =~ m#/#);
+ my $reldir = "../$name";
+ mkdir $self->data_dir() . "/". $reldir;
+ push($self->{_tablespaces}, $reldir);
+ return $reldir;
+}
+
+=pod
+
=item $node->info()
Return a string containing human-readable diagnostic information (paths, etc)
@@ -540,13 +561,27 @@ target server since it isn't done by default.
sub backup
{
- my ($self, $backup_name) = @_;
- my $backup_path = $self->backup_dir . '/' . $backup_name;
+ my ($self, $backup_name, %params) = @_;
+ my $backup_path = $self->backup_dir . '/' . $backup_name . '/' . "pgdata";
my $name = $self->name;
+ my @rest = ();
+
+ if (defined $params{tablespace_mapping})
+ {
+ foreach my $p (split /,/, $params{tablespace_mapping})
+ {
+ push(@rest, "--tablespace-mapping=$p");
+ }
+ }
+
+ foreach my $p ($self->{_tablespaces})
+ {
+ mkdir "$backup_path/$p";
+ }
print "# Taking pg_basebackup $backup_name from node \"$name\"\n";
TestLib::system_or_bail('pg_basebackup', '-D', $backup_path, '-h',
- $self->host, '-p', $self->port, '--no-sync');
+ $self->host, '-p', $self->port, '--no-sync', @rest);
print "# Backup finished\n";
return;
}
@@ -592,7 +627,7 @@ sub backup_fs_cold
sub _backup_fs
{
my ($self, $backup_name, $hot) = @_;
- my $backup_path = $self->backup_dir . '/' . $backup_name;
+ my $backup_path = $self->backup_dir . '/' . $backup_name . '/' . "pgdata";
my $port = $self->port;
my $name = $self->name;
@@ -655,7 +690,7 @@ unconditionally set to enable replication connections.
sub init_from_backup
{
my ($self, $root_node, $backup_name, %params) = @_;
- my $backup_path = $root_node->backup_dir . '/' . $backup_name;
+ my $backup_path = $root_node->backup_dir . '/' . $backup_name . '/' . "pgdata";
my $host = $self->host;
my $port = $self->port;
my $node_name = $self->name;
diff --git a/src/test/perl/RecursiveCopy.pm b/src/test/perl/RecursiveCopy.pm
index baf5d0ac63..f165db7348 100644
--- a/src/test/perl/RecursiveCopy.pm
+++ b/src/test/perl/RecursiveCopy.pm
@@ -22,6 +22,7 @@ use warnings;
use Carp;
use File::Basename;
use File::Copy;
+use TestLib;
=pod
@@ -97,14 +98,43 @@ sub _copypath_recurse
# invoke the filter and skip all further operation if it returns false
return 1 unless &$filterfn($curr_path);
- # Check for symlink -- needed only on source dir
- # (note: this will fall through quietly if file is already gone)
- croak "Cannot operate on symlink \"$srcpath\"" if -l $srcpath;
-
# Abort if destination path already exists. Should we allow directories
# to exist already?
croak "Destination path \"$destpath\" already exists" if -e $destpath;
+ # Check for symlink -- needed only on source dir
+ # (note: this will fall through quietly if file is already gone)
+ if (-l $srcpath)
+ {
+ croak "Cannot operate on symlink \"$srcpath\""
+ if ($srcpath !~ /\/(pg_tblspc\/[0-9]+)$/);
+
+ # We have mapped tablespaces. Copy them individually
+ my $linkname = $1;
+ my $rpath = my $target = readlink($srcpath);
+
+ #convert to pgdata-based if relative
+ $rpath =~ s#^\.\./##;
+
+ my $srcrealdir = "$base_src_dir/$rpath";
+ my $dstrealdir = "$base_dest_dir/$rpath";
+
+ mkdir $dstrealdir;
+ opendir(my $dh, $srcrealdir);
+ while (readdir $dh)
+ {
+ next if (/^\.\.?$/);
+ my $spath = "$srcrealdir/$_";
+ my $dpath = "$dstrealdir/$_";
+
+ copypath($spath, $dpath);
+ }
+ closedir $dh;
+
+ symlink $target, $destpath;
+ return 1;
+ }
+
# If this source path is a file, simply copy it to destination with the
# same name and we're done.
if (-f $srcpath)
--
2.16.3
----Next_Part(Wed_Apr_24_17_02_28_2019_644)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v2-0003-Add-check-for-recovery-failure-caused-by-tablespace.patch"
^ permalink raw reply [nested|flat] 38+ messages in thread
* [PATCH v2 1/2] partial revert of ff9618e82a
@ 2023-06-14 17:54 Nathan Bossart <[email protected]>
0 siblings, 0 replies; 38+ messages in thread
From: Nathan Bossart @ 2023-06-14 17:54 UTC (permalink / raw)
---
doc/src/sgml/ref/analyze.sgml | 13 ++-------
doc/src/sgml/ref/cluster.sgml | 9 +-----
doc/src/sgml/ref/lock.sgml | 24 ++++++---------
.../sgml/ref/refresh_materialized_view.sgml | 17 +++++------
doc/src/sgml/ref/reindex.sgml | 28 +++++++-----------
doc/src/sgml/ref/vacuum.sgml | 13 ++-------
doc/src/sgml/user-manag.sgml | 3 +-
src/backend/commands/cluster.c | 10 ++-----
src/backend/commands/indexcmds.c | 17 ++++-------
src/backend/commands/lockcmds.c | 8 -----
src/backend/commands/tablecmds.c | 29 +------------------
src/backend/commands/vacuum.c | 6 +---
src/include/commands/tablecmds.h | 1 -
.../expected/cluster-conflict-partition.out | 14 ++++-----
.../specs/cluster-conflict-partition.spec | 7 +++--
src/test/regress/expected/cluster.out | 3 +-
src/test/regress/expected/vacuum.out | 18 ++++++++++++
17 files changed, 74 insertions(+), 146 deletions(-)
diff --git a/doc/src/sgml/ref/analyze.sgml b/doc/src/sgml/ref/analyze.sgml
index 20c6f9939f..ecc7c884b4 100644
--- a/doc/src/sgml/ref/analyze.sgml
+++ b/doc/src/sgml/ref/analyze.sgml
@@ -183,17 +183,8 @@ ANALYZE [ VERBOSE ] [ <replaceable class="parameter">table_and_columns</replacea
<para>
To analyze a table, one must ordinarily have the <literal>MAINTAIN</literal>
- privilege on the table or be the table's owner, a superuser, or a role with
- privileges of the
- <link linkend="predefined-roles-table"><literal>pg_maintain</literal></link>
- role. However, database owners are allowed to
- analyze all tables in their databases, except shared catalogs.
- (The restriction for shared catalogs means that a true database-wide
- <command>ANALYZE</command> can only be performed by superusers and roles
- with privileges of <literal>pg_maintain</literal>.) If a role has
- permission to <command>ANALYZE</command> a partitioned table, it is also
- permitted to <command>ANALYZE</command> each of its partitions, regardless
- of whether the role has the aforementioned privileges on the partition.
+ privilege on the table. However, database owners are allowed to analyze all
+ tables in their databases, except shared catalogs.
<command>ANALYZE</command> will skip over any tables that the calling user
does not have permission to analyze.
</para>
diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml
index 29f0f1fd90..06f3d269e6 100644
--- a/doc/src/sgml/ref/cluster.sgml
+++ b/doc/src/sgml/ref/cluster.sgml
@@ -134,14 +134,7 @@ CLUSTER [VERBOSE]
<para>
To cluster a table, one must have the <literal>MAINTAIN</literal> privilege
- on the table or be the table's owner, a superuser, or a role with
- privileges of the
- <link linkend="predefined-roles-table"><literal>pg_maintain</literal></link>
- role. If a role has permission to <command>CLUSTER</command> a partitioned
- table, it is also permitted to <command>CLUSTER</command> each of its
- partitions, regardless of whether the role has the aforementioned
- privileges on the partition. <command>CLUSTER</command> will skip over any
- tables that the calling user does not have permission to cluster.
+ on the table.
</para>
<para>
diff --git a/doc/src/sgml/ref/lock.sgml b/doc/src/sgml/ref/lock.sgml
index 5b3b2b793a..d22c6f8384 100644
--- a/doc/src/sgml/ref/lock.sgml
+++ b/doc/src/sgml/ref/lock.sgml
@@ -166,21 +166,15 @@ LOCK [ TABLE ] [ ONLY ] <replaceable class="parameter">name</replaceable> [ * ]
<para>
To lock a table, the user must have the right privilege for the specified
- <replaceable class="parameter">lockmode</replaceable>, or be the table's
- owner, a superuser, or a role with privileges of the <link
- linkend="predefined-roles-table"><literal>pg_maintain</literal></link>
- role. If the user has <literal>MAINTAIN</literal>,
- <literal>UPDATE</literal>, <literal>DELETE</literal>, or
- <literal>TRUNCATE</literal> privileges on the table, any <replaceable
- class="parameter">lockmode</replaceable> is permitted. If the user has
- <literal>INSERT</literal> privileges on the table, <literal>ROW EXCLUSIVE
- MODE</literal> (or a less-conflicting mode as described in <xref
- linkend="explicit-locking"/>) is permitted. If a user has
- <literal>SELECT</literal> privileges on the table, <literal>ACCESS SHARE
- MODE</literal> is permitted. If a role has permission to lock a
- partitioned table, it is also permitted to lock each of its partitions,
- regardless of whether the role has the aforementioned privileges on the
- partition.
+ <replaceable class="parameter">lockmode</replaceable>. If the user has
+ <literal>MAINTAIN</literal>, <literal>UPDATE</literal>,
+ <literal>DELETE</literal>, or <literal>TRUNCATE</literal> privileges on the
+ table, any <replaceable class="parameter">lockmode</replaceable> is
+ permitted. If the user has <literal>INSERT</literal> privileges on the
+ table, <literal>ROW EXCLUSIVE MODE</literal> (or a less-conflicting mode as
+ described in <xref linkend="explicit-locking"/>) is permitted. If a user
+ has <literal>SELECT</literal> privileges on the table,
+ <literal>ACCESS SHARE MODE</literal> is permitted.
</para>
<para>
diff --git a/doc/src/sgml/ref/refresh_materialized_view.sgml b/doc/src/sgml/ref/refresh_materialized_view.sgml
index 4d79b6ae7f..199a577d36 100644
--- a/doc/src/sgml/ref/refresh_materialized_view.sgml
+++ b/doc/src/sgml/ref/refresh_materialized_view.sgml
@@ -31,16 +31,13 @@ REFRESH MATERIALIZED VIEW [ CONCURRENTLY ] <replaceable class="parameter">name</
<para>
<command>REFRESH MATERIALIZED VIEW</command> completely replaces the
- contents of a materialized view. To execute this command you must be the
- owner of the materialized view, have privileges of the
- <link linkend="predefined-roles-table"><literal>pg_maintain</literal></link>
- role, or have the <literal>MAINTAIN</literal>
- privilege on the materialized view. The old contents are discarded. If
- <literal>WITH DATA</literal> is specified (or defaults) the backing query
- is executed to provide the new data, and the materialized view is left in a
- scannable state. If <literal>WITH NO DATA</literal> is specified no new
- data is generated and the materialized view is left in an unscannable
- state.
+ contents of a materialized view. To execute this command you must have the
+ <literal>MAINTAIN</literal> privilege on the materialized view. The old
+ contents are discarded. If <literal>WITH DATA</literal> is specified (or
+ defaults) the backing query is executed to provide the new data, and the
+ materialized view is left in a scannable state. If
+ <literal>WITH NO DATA</literal> is specified no new data is generated and
+ the materialized view is left in an unscannable state.
</para>
<para>
<literal>CONCURRENTLY</literal> and <literal>WITH NO DATA</literal> may not
diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml
index 71455dfdc7..a64b021e66 100644
--- a/doc/src/sgml/ref/reindex.sgml
+++ b/doc/src/sgml/ref/reindex.sgml
@@ -292,25 +292,17 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { DA
</para>
<para>
- Reindexing a single index or table requires being the owner of that
- index or table, having privileges of the
+ Reindexing a single index or table requires having the
+ <literal>MAINTAIN</literal> privilege on the table. Reindexing a schema or
+ database requires being the owner of that schema or database or having
+ privileges of the
<link linkend="predefined-roles-table"><literal>pg_maintain</literal></link>
- role, or having the <literal>MAINTAIN</literal> privilege on the
- table. Reindexing a schema or database requires being the
- owner of that schema or database or having privileges of the
- <literal>pg_maintain</literal> role. Note specifically that it's thus
- possible for non-superusers to rebuild indexes of tables owned by
- other users. However, as a special exception, when
- <command>REINDEX DATABASE</command>, <command>REINDEX SCHEMA</command>
- or <command>REINDEX SYSTEM</command> is issued by a non-superuser,
- indexes on shared catalogs will be skipped unless the user owns the
- catalog (which typically won't be the case), has privileges of the
- <literal>pg_maintain</literal> role, or has the <literal>MAINTAIN</literal>
- privilege on the catalog. If a role has permission to
- <command>REINDEX</command> a partitioned table, it is also permitted to
- <command>REINDEX</command> each of its partitions, regardless of whether the
- role has the aforementioned privileges on the partition. Of course,
- superusers can always reindex anything.
+ role. Note specifically that it's thus possible for non-superusers to
+ rebuild indexes of tables owned by other users. However, as a special
+ exception, <command>REINDEX DATABASE</command>,
+ <command>REINDEX SCHEMA</command>, and <command>REINDEX SYSTEM</command>
+ will skip indexes on shared catalogs unless the user has the
+ <literal>MAINTAIN</literal> privilege on the catalog.
</para>
<para>
diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml
index 57bc4c23ec..fa2ee76e25 100644
--- a/doc/src/sgml/ref/vacuum.sgml
+++ b/doc/src/sgml/ref/vacuum.sgml
@@ -445,17 +445,8 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet
<para>
To vacuum a table, one must ordinarily have the <literal>MAINTAIN</literal>
- privilege on the table or be the table's owner, a superuser, or a role with
- privileges of the
- <link linkend="predefined-roles-table"><literal>pg_maintain</literal></link>
- role. However, database owners are allowed to
- vacuum all tables in their databases, except shared catalogs.
- (The restriction for shared catalogs means that a true database-wide
- <command>VACUUM</command> can only be performed by superusers and roles
- with privileges of <literal>pg_maintain</literal>.) If a role has
- permission to <command>VACUUM</command> a partitioned table, it is also
- permitted to <command>VACUUM</command> each of its partitions, regardless
- of whether the role has the aforementioned privileges on the partition.
+ privilege on the table. However, database owners are allowed to vacuum all
+ tables in their databases, except shared catalogs.
<command>VACUUM</command> will skip over any tables that the calling user
does not have permission to vacuum.
</para>
diff --git a/doc/src/sgml/user-manag.sgml b/doc/src/sgml/user-manag.sgml
index b6c37ccef2..e1540dd481 100644
--- a/doc/src/sgml/user-manag.sgml
+++ b/doc/src/sgml/user-manag.sgml
@@ -692,7 +692,8 @@ DROP ROLE doomed_role;
<link linkend="sql-refreshmaterializedview"><command>REFRESH MATERIALIZED VIEW</command></link>,
<link linkend="sql-reindex"><command>REINDEX</command></link>,
and <link linkend="sql-lock"><command>LOCK TABLE</command></link> on all
- relations.</entry>
+ relations, as if having <literal>MAINTAIN</literal> rights on those
+ objects, even without having it explicitly.</entry>
</row>
<row>
<entry>pg_use_reserved_connections</entry>
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 369fea7c04..38834356b9 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -1693,11 +1693,8 @@ get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid)
if (get_rel_relkind(indexrelid) != RELKIND_INDEX)
continue;
- /*
- * We already checked that the user has privileges to CLUSTER the
- * partitioned table when we locked it earlier, so there's no need to
- * check the privileges again here.
- */
+ if (!cluster_is_permitted_for_relation(relid, GetUserId()))
+ continue;
/* Use a permanent memory context for the result list */
old_context = MemoryContextSwitchTo(cluster_context);
@@ -1720,8 +1717,7 @@ get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid)
static bool
cluster_is_permitted_for_relation(Oid relid, Oid userid)
{
- if (pg_class_aclcheck(relid, userid, ACL_MAINTAIN) == ACLCHECK_OK ||
- has_partition_ancestor_privs(relid, userid, ACL_MAINTAIN))
+ if (pg_class_aclcheck(relid, userid, ACL_MAINTAIN) == ACLCHECK_OK)
return true;
ereport(WARNING,
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index a5168c9f09..ed28d13a16 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2854,8 +2854,7 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation,
/* Check permissions */
table_oid = IndexGetRelation(relId, true);
if (OidIsValid(table_oid) &&
- pg_class_aclcheck(table_oid, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK &&
- !has_partition_ancestor_privs(table_oid, GetUserId(), ACL_MAINTAIN))
+ pg_class_aclcheck(table_oid, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK)
aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_INDEX,
relation->relname);
@@ -3064,18 +3063,12 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
continue;
/*
- * The table can be reindexed if the user has been granted MAINTAIN on
- * the table or one of its partition ancestors or the user is a
- * superuser, the table owner, or the database/schema owner (but in
- * the latter case, only if it's not a shared relation).
- * pg_class_aclcheck includes the superuser case, and depending on
- * objectKind we already know that the user has permission to run
- * REINDEX on this database or schema per the permission checks at the
- * beginning of this routine.
+ * We already checked privileges on the database or schema, but we
+ * further restrict reindexing shared catalogs to roles with the
+ * MAINTAIN privilege on the relation.
*/
if (classtuple->relisshared &&
- pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK &&
- !has_partition_ancestor_privs(relid, GetUserId(), ACL_MAINTAIN))
+ pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK)
continue;
/*
diff --git a/src/backend/commands/lockcmds.c b/src/backend/commands/lockcmds.c
index 43c7d7f4bb..92662cbbc8 100644
--- a/src/backend/commands/lockcmds.c
+++ b/src/backend/commands/lockcmds.c
@@ -19,7 +19,6 @@
#include "catalog/namespace.h"
#include "catalog/pg_inherits.h"
#include "commands/lockcmds.h"
-#include "commands/tablecmds.h"
#include "miscadmin.h"
#include "nodes/nodeFuncs.h"
#include "parser/parse_clause.h"
@@ -297,12 +296,5 @@ LockTableAclCheck(Oid reloid, LOCKMODE lockmode, Oid userid)
aclresult = pg_class_aclcheck(reloid, userid, aclmask);
- /*
- * If this is a partition, check permissions of its ancestors if needed.
- */
- if (aclresult != ACLCHECK_OK &&
- has_partition_ancestor_privs(reloid, userid, ACL_MAINTAIN))
- aclresult = ACLCHECK_OK;
-
return aclresult;
}
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 4d49d70c33..29eece1c2c 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -17006,38 +17006,11 @@ RangeVarCallbackMaintainsTable(const RangeVar *relation,
errmsg("\"%s\" is not a table or materialized view", relation->relname)));
/* Check permissions */
- if (pg_class_aclcheck(relId, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK &&
- !has_partition_ancestor_privs(relId, GetUserId(), ACL_MAINTAIN))
+ if (pg_class_aclcheck(relId, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK)
aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_TABLE,
relation->relname);
}
-/*
- * If relid is a partition, returns whether userid has any of the privileges
- * specified in acl on any of its ancestors. Otherwise, returns false.
- */
-bool
-has_partition_ancestor_privs(Oid relid, Oid userid, AclMode acl)
-{
- List *ancestors;
- ListCell *lc;
-
- if (!get_rel_relispartition(relid))
- return false;
-
- ancestors = get_partition_ancestors(relid);
- foreach(lc, ancestors)
- {
- Oid ancestor = lfirst_oid(lc);
-
- if (OidIsValid(ancestor) &&
- pg_class_aclcheck(ancestor, userid, acl) == ACLCHECK_OK)
- return true;
- }
-
- return false;
-}
-
/*
* Callback to RangeVarGetRelidExtended() for TRUNCATE processing.
*/
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index a843f9ad92..2e41a31173 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -41,7 +41,6 @@
#include "catalog/pg_namespace.h"
#include "commands/cluster.h"
#include "commands/defrem.h"
-#include "commands/tablecmds.h"
#include "commands/vacuum.h"
#include "miscadmin.h"
#include "nodes/makefuncs.h"
@@ -719,13 +718,10 @@ vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
* - the role owns the relation
* - the role owns the current database and the relation is not shared
* - the role has been granted the MAINTAIN privilege on the relation
- * - the role has privileges to vacuum/analyze any of the relation's
- * partition ancestors
*----------
*/
if ((object_ownercheck(DatabaseRelationId, MyDatabaseId, GetUserId()) && !reltuple->relisshared) ||
- pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) == ACLCHECK_OK ||
- has_partition_ancestor_privs(relid, GetUserId(), ACL_MAINTAIN))
+ pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) == ACLCHECK_OK)
return true;
relname = NameStr(reltuple->relname);
diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index 17b9404937..250d89ff88 100644
--- a/src/include/commands/tablecmds.h
+++ b/src/include/commands/tablecmds.h
@@ -99,7 +99,6 @@ extern void AtEOSubXact_on_commit_actions(bool isCommit,
extern void RangeVarCallbackMaintainsTable(const RangeVar *relation,
Oid relId, Oid oldRelId,
void *arg);
-extern bool has_partition_ancestor_privs(Oid relid, Oid userid, AclMode acl);
extern void RangeVarCallbackOwnsRelation(const RangeVar *relation,
Oid relId, Oid oldRelId, void *arg);
diff --git a/src/test/isolation/expected/cluster-conflict-partition.out b/src/test/isolation/expected/cluster-conflict-partition.out
index 8d21276996..7be9e56ef1 100644
--- a/src/test/isolation/expected/cluster-conflict-partition.out
+++ b/src/test/isolation/expected/cluster-conflict-partition.out
@@ -3,7 +3,7 @@ Parsed test spec with 2 sessions
starting permutation: s1_begin s1_lock_parent s2_auth s2_cluster s1_commit s2_reset
step s1_begin: BEGIN;
step s1_lock_parent: LOCK cluster_part_tab IN SHARE UPDATE EXCLUSIVE MODE;
-step s2_auth: SET ROLE regress_cluster_part;
+step s2_auth: SET ROLE regress_cluster_part; SET client_min_messages = ERROR;
step s2_cluster: CLUSTER cluster_part_tab USING cluster_part_ind; <waiting ...>
step s1_commit: COMMIT;
step s2_cluster: <... completed>
@@ -11,7 +11,7 @@ step s2_reset: RESET ROLE;
starting permutation: s1_begin s2_auth s1_lock_parent s2_cluster s1_commit s2_reset
step s1_begin: BEGIN;
-step s2_auth: SET ROLE regress_cluster_part;
+step s2_auth: SET ROLE regress_cluster_part; SET client_min_messages = ERROR;
step s1_lock_parent: LOCK cluster_part_tab IN SHARE UPDATE EXCLUSIVE MODE;
step s2_cluster: CLUSTER cluster_part_tab USING cluster_part_ind; <waiting ...>
step s1_commit: COMMIT;
@@ -21,17 +21,15 @@ step s2_reset: RESET ROLE;
starting permutation: s1_begin s1_lock_child s2_auth s2_cluster s1_commit s2_reset
step s1_begin: BEGIN;
step s1_lock_child: LOCK cluster_part_tab1 IN SHARE UPDATE EXCLUSIVE MODE;
-step s2_auth: SET ROLE regress_cluster_part;
-step s2_cluster: CLUSTER cluster_part_tab USING cluster_part_ind; <waiting ...>
+step s2_auth: SET ROLE regress_cluster_part; SET client_min_messages = ERROR;
+step s2_cluster: CLUSTER cluster_part_tab USING cluster_part_ind;
step s1_commit: COMMIT;
-step s2_cluster: <... completed>
step s2_reset: RESET ROLE;
starting permutation: s1_begin s2_auth s1_lock_child s2_cluster s1_commit s2_reset
step s1_begin: BEGIN;
-step s2_auth: SET ROLE regress_cluster_part;
+step s2_auth: SET ROLE regress_cluster_part; SET client_min_messages = ERROR;
step s1_lock_child: LOCK cluster_part_tab1 IN SHARE UPDATE EXCLUSIVE MODE;
-step s2_cluster: CLUSTER cluster_part_tab USING cluster_part_ind; <waiting ...>
+step s2_cluster: CLUSTER cluster_part_tab USING cluster_part_ind;
step s1_commit: COMMIT;
-step s2_cluster: <... completed>
step s2_reset: RESET ROLE;
diff --git a/src/test/isolation/specs/cluster-conflict-partition.spec b/src/test/isolation/specs/cluster-conflict-partition.spec
index ae38cb4ee3..4d38a7f49a 100644
--- a/src/test/isolation/specs/cluster-conflict-partition.spec
+++ b/src/test/isolation/specs/cluster-conflict-partition.spec
@@ -23,12 +23,15 @@ step s1_lock_child { LOCK cluster_part_tab1 IN SHARE UPDATE EXCLUSIVE MODE;
step s1_commit { COMMIT; }
session s2
-step s2_auth { SET ROLE regress_cluster_part; }
+step s2_auth { SET ROLE regress_cluster_part; SET client_min_messages = ERROR; }
step s2_cluster { CLUSTER cluster_part_tab USING cluster_part_ind; }
step s2_reset { RESET ROLE; }
-# CLUSTER waits if locked, passes for all cases.
+# CLUSTER on the parent waits if locked, passes for all cases.
permutation s1_begin s1_lock_parent s2_auth s2_cluster s1_commit s2_reset
permutation s1_begin s2_auth s1_lock_parent s2_cluster s1_commit s2_reset
+
+# When taking a lock on a partition leaf, CLUSTER on the parent skips
+# the leaf, passes for all cases.
permutation s1_begin s1_lock_child s2_auth s2_cluster s1_commit s2_reset
permutation s1_begin s2_auth s1_lock_child s2_cluster s1_commit s2_reset
diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out
index 2eec483eaa..27a5dff5d4 100644
--- a/src/test/regress/expected/cluster.out
+++ b/src/test/regress/expected/cluster.out
@@ -508,6 +508,7 @@ CREATE TEMP TABLE ptnowner_oldnodes AS
JOIN pg_class AS c ON c.oid=tree.relid;
SET SESSION AUTHORIZATION regress_ptnowner;
CLUSTER ptnowner USING ptnowner_i_idx;
+WARNING: permission denied to cluster "ptnowner2", skipping it
RESET SESSION AUTHORIZATION;
SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a
JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C";
@@ -515,7 +516,7 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a
-----------+----------
ptnowner | t
ptnowner1 | f
- ptnowner2 | f
+ ptnowner2 | t
(3 rows)
DROP TABLE ptnowner;
diff --git a/src/test/regress/expected/vacuum.out b/src/test/regress/expected/vacuum.out
index 41e020cf20..4def90b805 100644
--- a/src/test/regress/expected/vacuum.out
+++ b/src/test/regress/expected/vacuum.out
@@ -442,14 +442,20 @@ ALTER TABLE vacowned_parted OWNER TO regress_vacuum;
ALTER TABLE vacowned_part1 OWNER TO regress_vacuum;
SET ROLE regress_vacuum;
VACUUM vacowned_parted;
+WARNING: permission denied to vacuum "vacowned_part2", skipping it
VACUUM vacowned_part1;
VACUUM vacowned_part2;
+WARNING: permission denied to vacuum "vacowned_part2", skipping it
ANALYZE vacowned_parted;
+WARNING: permission denied to analyze "vacowned_part2", skipping it
ANALYZE vacowned_part1;
ANALYZE vacowned_part2;
+WARNING: permission denied to analyze "vacowned_part2", skipping it
VACUUM (ANALYZE) vacowned_parted;
+WARNING: permission denied to vacuum "vacowned_part2", skipping it
VACUUM (ANALYZE) vacowned_part1;
VACUUM (ANALYZE) vacowned_part2;
+WARNING: permission denied to vacuum "vacowned_part2", skipping it
RESET ROLE;
-- Only one partition owned by other user.
ALTER TABLE vacowned_parted OWNER TO CURRENT_USER;
@@ -478,14 +484,26 @@ ALTER TABLE vacowned_parted OWNER TO regress_vacuum;
ALTER TABLE vacowned_part1 OWNER TO CURRENT_USER;
SET ROLE regress_vacuum;
VACUUM vacowned_parted;
+WARNING: permission denied to vacuum "vacowned_part1", skipping it
+WARNING: permission denied to vacuum "vacowned_part2", skipping it
VACUUM vacowned_part1;
+WARNING: permission denied to vacuum "vacowned_part1", skipping it
VACUUM vacowned_part2;
+WARNING: permission denied to vacuum "vacowned_part2", skipping it
ANALYZE vacowned_parted;
+WARNING: permission denied to analyze "vacowned_part1", skipping it
+WARNING: permission denied to analyze "vacowned_part2", skipping it
ANALYZE vacowned_part1;
+WARNING: permission denied to analyze "vacowned_part1", skipping it
ANALYZE vacowned_part2;
+WARNING: permission denied to analyze "vacowned_part2", skipping it
VACUUM (ANALYZE) vacowned_parted;
+WARNING: permission denied to vacuum "vacowned_part1", skipping it
+WARNING: permission denied to vacuum "vacowned_part2", skipping it
VACUUM (ANALYZE) vacowned_part1;
+WARNING: permission denied to vacuum "vacowned_part1", skipping it
VACUUM (ANALYZE) vacowned_part2;
+WARNING: permission denied to vacuum "vacowned_part2", skipping it
RESET ROLE;
DROP TABLE vacowned;
DROP TABLE vacowned_parted;
--
2.25.1
--EVF5PPMfhYS0aIcm
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v2-0002-convert-boolean-parameter-into-a-VACOPT_-option.patch"
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2024-11-28 19:33 Alena Rybakina <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: Alena Rybakina @ 2024-11-28 19:33 UTC (permalink / raw)
To: Ranier Vilela <[email protected]>; +Cc: Alexander Lakhin <[email protected]>; Alexander Korotkov <[email protected]>; Andrei Lepikhov <[email protected]>; jian he <[email protected]>; Robert Haas <[email protected]>; Nikolay Shaplov <[email protected]>; [email protected]; pgsql-hackers; Peter Geoghegan <[email protected]>; Marcos Pegoraro <[email protected]>; [email protected]; Peter Eisentraut <[email protected]>
On 28.11.2024 22:28, Ranier Vilela wrote:
> Em qui., 28 de nov. de 2024 às 16:03, Alena Rybakina
> <[email protected]> escreveu:
>
> Hi! Thank you for the case.
>
> On 28.11.2024 21:00, Alexander Lakhin wrote:
> > Hello Alexander,
> >
> > 21.11.2024 09:34, Alexander Korotkov wrote:
> >> I'm going to push this if no objections.
> >
> > Please look at the following query, which triggers an error after
> > ae4569161:
> > SET random_page_cost = 1;
> > CREATE TABLE tbl(u UUID);
> > CREATE INDEX idx ON tbl USING HASH (u);
> > SELECT COUNT(*) FROM tbl WHERE u =
> '00000000000000000000000000000000' OR
> > u = '11111111111111111111111111111111';
> >
> > ERROR: XX000: ScalarArrayOpExpr index qual found where not allowed
> > LOCATION: ExecIndexBuildScanKeys, nodeIndexscan.c:1625
> >
> >
> I found out what the problem is index scan method was not
> generated. We
> need to check this during OR clauses for SAOP transformation.
>
> There is a patch to fix this problem.
>
> Hi.
> Thanks for the quick fix.
>
> But I wonder if it is not possible to avoid all if the index is useless?
> Maybe moving your fix to the beginning of the function?
>
> diff --git a/src/backend/optimizer/path/indxpath.c
> b/src/backend/optimizer/path/indxpath.c
> index d827fc9f4d..5ea0b27d01 100644
> --- a/src/backend/optimizer/path/indxpath.c
> +++ b/src/backend/optimizer/path/indxpath.c
> @@ -3248,6 +3248,10 @@ match_orclause_to_indexcol(PlannerInfo *root,
> Assert(IsA(orclause, BoolExpr));
> Assert(orclause->boolop == OR_EXPR);
>
> + /* Ignore index if it doesn't support index scans */
> + if(!index->amsearcharray)
> + return NULL;
> +
Agree. I have updated the patch
> /*
> * Try to convert a list of OR-clauses to a single SAOP expression. Each
> * OR entry must be in the form: (indexkey operator constant) or
> (constant
>
> The test bug:
> EXPLAIN SELECT COUNT(*) FROM tbl WHERE u =
> '00000000000000000000000000000000' OR u =
> '11111111111111111111111111111111';
> QUERY PLAN
> ----------------------------------------------------------------------------------------------------------------------------------
> Aggregate (cost=12.46..12.47 rows=1 width=8)
> -> Bitmap Heap Scan on tbl (cost=2.14..12.41 rows=18 width=0)
> Recheck Cond: ((u =
> '00000000-0000-0000-0000-000000000000'::uuid) OR (u =
> '11111111-1111-1111-1111-111111111111'::uuid))
> -> BitmapOr (cost=2.14..2.14 rows=18 width=0)
> -> Bitmap Index Scan on idx (cost=0.00..1.07 rows=9
> width=0)
> Index Cond: (u =
> '00000000-0000-0000-0000-000000000000'::uuid)
> -> Bitmap Index Scan on idx (cost=0.00..1.07 rows=9
> width=0)
> Index Cond: (u =
> '11111111-1111-1111-1111-111111111111'::uuid)
> (8 rows)
>
Thank you
--
Regards,
Alena Rybakina
Postgres Professional
Attachments:
[text/x-patch] bugfix.diff (2.5K, ../../[email protected]/3-bugfix.diff)
download | inline diff:
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index d827fc9f4d9..5ea0b27d014 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -3248,6 +3248,10 @@ match_orclause_to_indexcol(PlannerInfo *root,
Assert(IsA(orclause, BoolExpr));
Assert(orclause->boolop == OR_EXPR);
+ /* Ignore index if it doesn't support index scans */
+ if(!index->amsearcharray)
+ return NULL;
+
/*
* Try to convert a list of OR-clauses to a single SAOP expression. Each
* OR entry must be in the form: (indexkey operator constant) or (constant
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 1b0a5f0e9e1..ef8bbf4748c 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -1233,6 +1233,23 @@ SELECT count(*) FROM tenk1 WHERE stringu1 = 'TVAAAA';
14
(1 row)
+-- Transform OR-clauses to SAOP's shouldn't be chosen
+SET random_page_cost = 1;
+EXPLAIN (COSTS OFF)
+SELECT COUNT(*) FROM tenk1 WHERE stringu1 = 'TVAAAA' OR stringu1 = 'TVAAAB';
+ QUERY PLAN
+------------------------------------------------------------------------------------
+ Aggregate
+ -> Bitmap Heap Scan on tenk1
+ Recheck Cond: ((stringu1 = 'TVAAAA'::name) OR (stringu1 = 'TVAAAB'::name))
+ -> BitmapOr
+ -> Bitmap Index Scan on hash_tuplesort_idx
+ Index Cond: (stringu1 = 'TVAAAA'::name)
+ -> Bitmap Index Scan on hash_tuplesort_idx
+ Index Cond: (stringu1 = 'TVAAAB'::name)
+(8 rows)
+
+RESET random_page_cost;
DROP INDEX hash_tuplesort_idx;
RESET maintenance_work_mem;
--
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index ddd0d9ad396..0e6529f3f3d 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -372,6 +372,11 @@ CREATE INDEX hash_tuplesort_idx ON tenk1 USING hash (stringu1 name_ops) WITH (fi
EXPLAIN (COSTS OFF)
SELECT count(*) FROM tenk1 WHERE stringu1 = 'TVAAAA';
SELECT count(*) FROM tenk1 WHERE stringu1 = 'TVAAAA';
+-- Transform OR-clauses to SAOP's shouldn't be chosen
+SET random_page_cost = 1;
+EXPLAIN (COSTS OFF)
+SELECT COUNT(*) FROM tenk1 WHERE stringu1 = 'TVAAAA' OR stringu1 = 'TVAAAB';
+RESET random_page_cost;
DROP INDEX hash_tuplesort_idx;
RESET maintenance_work_mem;
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2024-11-29 00:04 Alexander Korotkov <[email protected]>
parent: Alena Rybakina <[email protected]>
0 siblings, 2 replies; 38+ messages in thread
From: Alexander Korotkov @ 2024-11-29 00:04 UTC (permalink / raw)
To: Alena Rybakina <[email protected]>; +Cc: Ranier Vilela <[email protected]>; Alexander Lakhin <[email protected]>; Andrei Lepikhov <[email protected]>; jian he <[email protected]>; Robert Haas <[email protected]>; Nikolay Shaplov <[email protected]>; [email protected]; pgsql-hackers; Peter Geoghegan <[email protected]>; Marcos Pegoraro <[email protected]>; [email protected]; Peter Eisentraut <[email protected]>
On Thu, Nov 28, 2024 at 9:33 PM Alena Rybakina
<[email protected]> wrote:
>
> On 28.11.2024 22:28, Ranier Vilela wrote:
>
> Em qui., 28 de nov. de 2024 às 16:03, Alena Rybakina <[email protected]> escreveu:
>>
>> Hi! Thank you for the case.
>>
>> On 28.11.2024 21:00, Alexander Lakhin wrote:
>> > Hello Alexander,
>> >
>> > 21.11.2024 09:34, Alexander Korotkov wrote:
>> >> I'm going to push this if no objections.
>> >
>> > Please look at the following query, which triggers an error after
>> > ae4569161:
>> > SET random_page_cost = 1;
>> > CREATE TABLE tbl(u UUID);
>> > CREATE INDEX idx ON tbl USING HASH (u);
>> > SELECT COUNT(*) FROM tbl WHERE u = '00000000000000000000000000000000' OR
>> > u = '11111111111111111111111111111111';
>> >
>> > ERROR: XX000: ScalarArrayOpExpr index qual found where not allowed
>> > LOCATION: ExecIndexBuildScanKeys, nodeIndexscan.c:1625
>> >
>> >
>> I found out what the problem is index scan method was not generated. We
>> need to check this during OR clauses for SAOP transformation.
>>
>> There is a patch to fix this problem.
>
> Hi.
> Thanks for the quick fix.
>
> But I wonder if it is not possible to avoid all if the index is useless?
> Maybe moving your fix to the beginning of the function?
>
> diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
> index d827fc9f4d..5ea0b27d01 100644
> --- a/src/backend/optimizer/path/indxpath.c
> +++ b/src/backend/optimizer/path/indxpath.c
> @@ -3248,6 +3248,10 @@ match_orclause_to_indexcol(PlannerInfo *root,
> Assert(IsA(orclause, BoolExpr));
> Assert(orclause->boolop == OR_EXPR);
>
> + /* Ignore index if it doesn't support index scans */
> + if(!index->amsearcharray)
> + return NULL;
> +
>
> Agree. I have updated the patch
>
> /*
> * Try to convert a list of OR-clauses to a single SAOP expression. Each
> * OR entry must be in the form: (indexkey operator constant) or (constant
>
> The test bug:
> EXPLAIN SELECT COUNT(*) FROM tbl WHERE u = '00000000000000000000000000000000' OR u = '11111111111111111111111111111111';
> QUERY PLAN
> ----------------------------------------------------------------------------------------------------------------------------------
> Aggregate (cost=12.46..12.47 rows=1 width=8)
> -> Bitmap Heap Scan on tbl (cost=2.14..12.41 rows=18 width=0)
> Recheck Cond: ((u = '00000000-0000-0000-0000-000000000000'::uuid) OR (u = '11111111-1111-1111-1111-111111111111'::uuid))
> -> BitmapOr (cost=2.14..2.14 rows=18 width=0)
> -> Bitmap Index Scan on idx (cost=0.00..1.07 rows=9 width=0)
> Index Cond: (u = '00000000-0000-0000-0000-000000000000'::uuid)
> -> Bitmap Index Scan on idx (cost=0.00..1.07 rows=9 width=0)
> Index Cond: (u = '11111111-1111-1111-1111-111111111111'::uuid)
> (8 rows)
I slightly revised the fix and added similar check to
group_similar_or_args(). Could you, please, review that before
commit?
------
Regards,
Alexander Korotkov
Supabase
Attachments:
[application/octet-stream] v3-0001-Skip-not-SOAP-supported-indexes-while-transformin.patch (4.0K, ../../CAPpHfdvM4E-wPCRbYkr9eGH2sSxgOYKZ1iOUyjKYqTEyurFRLQ@mail.gmail.com/2-v3-0001-Skip-not-SOAP-supported-indexes-while-transformin.patch)
download | inline diff:
From b9fc7d52ebb051fe5f67501407c56daf0af765da Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Fri, 29 Nov 2024 01:46:43 +0200
Subject: [PATCH v3] Skip not SOAP-supported indexes while transforming an OR
clause into SAOP
There is no point in transforming OR-clauses into SAOP's if the target index
doesn't support SAOP scans anyway. This commit adds corresponding checks
to match_orclause_to_indexcol() and group_similar_or_args(). The first check
fixes the actual bug, while the second just saves some cycles.
Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/8174de69-9e1a-0827-0e81-ef97f56a5939%40gmail.com
Author: Alena Rybakina
Reviewed-by: Ranier Vilela, Alexander Korotkov
---
src/backend/optimizer/path/indxpath.c | 11 +++++++++--
src/test/regress/expected/create_index.out | 18 ++++++++++++++++++
src/test/regress/sql/create_index.sql | 6 ++++++
3 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index d827fc9f4d9..640b28a68fc 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -1354,8 +1354,11 @@ group_similar_or_args(PlannerInfo *root, RelOptInfo *rel, RestrictInfo *rinfo)
{
IndexOptInfo *index = (IndexOptInfo *) lfirst(lc2);
- /* Ignore index if it doesn't support bitmap scans */
- if (!index->amhasgetbitmap)
+ /*
+ * Ignore index if it doesn't support bitmap scans or SAOP
+ * clauses.
+ */
+ if (!index->amhasgetbitmap || !index->amsearcharray)
continue;
for (colnum = 0; colnum < index->nkeycolumns; colnum++)
@@ -3248,6 +3251,10 @@ match_orclause_to_indexcol(PlannerInfo *root,
Assert(IsA(orclause, BoolExpr));
Assert(orclause->boolop == OR_EXPR);
+ /* Ignore index if it doesn't support SAOP clauses */
+ if(!index->amsearcharray)
+ return NULL;
+
/*
* Try to convert a list of OR-clauses to a single SAOP expression. Each
* OR entry must be in the form: (indexkey operator constant) or (constant
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 1b0a5f0e9e1..1904eb65bb9 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -1233,6 +1233,24 @@ SELECT count(*) FROM tenk1 WHERE stringu1 = 'TVAAAA';
14
(1 row)
+-- OR-clauses shouldn't be transformed into SAOP because hash indexes don't
+-- support SAOP scans.
+SET enable_seqscan = off;
+EXPLAIN (COSTS OFF)
+SELECT COUNT(*) FROM tenk1 WHERE stringu1 = 'TVAAAA' OR stringu1 = 'TVAAAB';
+ QUERY PLAN
+------------------------------------------------------------------------------------
+ Aggregate
+ -> Bitmap Heap Scan on tenk1
+ Recheck Cond: ((stringu1 = 'TVAAAA'::name) OR (stringu1 = 'TVAAAB'::name))
+ -> BitmapOr
+ -> Bitmap Index Scan on hash_tuplesort_idx
+ Index Cond: (stringu1 = 'TVAAAA'::name)
+ -> Bitmap Index Scan on hash_tuplesort_idx
+ Index Cond: (stringu1 = 'TVAAAB'::name)
+(8 rows)
+
+RESET enable_seqscan;
DROP INDEX hash_tuplesort_idx;
RESET maintenance_work_mem;
--
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index ddd0d9ad396..c085e05f052 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -372,6 +372,12 @@ CREATE INDEX hash_tuplesort_idx ON tenk1 USING hash (stringu1 name_ops) WITH (fi
EXPLAIN (COSTS OFF)
SELECT count(*) FROM tenk1 WHERE stringu1 = 'TVAAAA';
SELECT count(*) FROM tenk1 WHERE stringu1 = 'TVAAAA';
+-- OR-clauses shouldn't be transformed into SAOP because hash indexes don't
+-- support SAOP scans.
+SET enable_seqscan = off;
+EXPLAIN (COSTS OFF)
+SELECT COUNT(*) FROM tenk1 WHERE stringu1 = 'TVAAAA' OR stringu1 = 'TVAAAB';
+RESET enable_seqscan;
DROP INDEX hash_tuplesort_idx;
RESET maintenance_work_mem;
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2024-11-29 05:10 Andrei Lepikhov <[email protected]>
parent: Alexander Korotkov <[email protected]>
1 sibling, 0 replies; 38+ messages in thread
From: Andrei Lepikhov @ 2024-11-29 05:10 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; Alena Rybakina <[email protected]>; +Cc: Ranier Vilela <[email protected]>; Alexander Lakhin <[email protected]>; jian he <[email protected]>; Robert Haas <[email protected]>; Nikolay Shaplov <[email protected]>; [email protected]; pgsql-hackers; Peter Geoghegan <[email protected]>; Marcos Pegoraro <[email protected]>; [email protected]; Peter Eisentraut <[email protected]>
On 11/29/24 07:04, Alexander Korotkov wrote:
> On Thu, Nov 28, 2024 at 9:33 PM Alena Rybakina
> I slightly revised the fix and added similar check to
> group_similar_or_args(). Could you, please, review that before
> commit?
LGTM,
As I see, we didn't pay attention to this option from the beginning.
Thanks for fixing it!
--
regards, Andrei Lepikhov
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2024-11-29 05:51 Alena Rybakina <[email protected]>
parent: Alexander Korotkov <[email protected]>
1 sibling, 1 reply; 38+ messages in thread
From: Alena Rybakina @ 2024-11-29 05:51 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; +Cc: Ranier Vilela <[email protected]>; Alexander Lakhin <[email protected]>; Andrei Lepikhov <[email protected]>; jian he <[email protected]>; Robert Haas <[email protected]>; Nikolay Shaplov <[email protected]>; [email protected]; pgsql-hackers; Peter Geoghegan <[email protected]>; Marcos Pegoraro <[email protected]>; [email protected]; Peter Eisentraut <[email protected]>
On 29.11.2024 03:04, Alexander Korotkov wrote:
> On Thu, Nov 28, 2024 at 9:33 PM Alena Rybakina
> <[email protected]> wrote:
>> On 28.11.2024 22:28, Ranier Vilela wrote:
>>
>> Em qui., 28 de nov. de 2024 às 16:03, Alena Rybakina <[email protected]> escreveu:
>>> Hi! Thank you for the case.
>>>
>>> On 28.11.2024 21:00, Alexander Lakhin wrote:
>>>> Hello Alexander,
>>>>
>>>> 21.11.2024 09:34, Alexander Korotkov wrote:
>>>>> I'm going to push this if no objections.
>>>> Please look at the following query, which triggers an error after
>>>> ae4569161:
>>>> SET random_page_cost = 1;
>>>> CREATE TABLE tbl(u UUID);
>>>> CREATE INDEX idx ON tbl USING HASH (u);
>>>> SELECT COUNT(*) FROM tbl WHERE u = '00000000000000000000000000000000' OR
>>>> u = '11111111111111111111111111111111';
>>>>
>>>> ERROR: XX000: ScalarArrayOpExpr index qual found where not allowed
>>>> LOCATION: ExecIndexBuildScanKeys, nodeIndexscan.c:1625
>>>>
>>>>
>>> I found out what the problem is index scan method was not generated. We
>>> need to check this during OR clauses for SAOP transformation.
>>>
>>> There is a patch to fix this problem.
>> Hi.
>> Thanks for the quick fix.
>>
>> But I wonder if it is not possible to avoid all if the index is useless?
>> Maybe moving your fix to the beginning of the function?
>>
>> diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
>> index d827fc9f4d..5ea0b27d01 100644
>> --- a/src/backend/optimizer/path/indxpath.c
>> +++ b/src/backend/optimizer/path/indxpath.c
>> @@ -3248,6 +3248,10 @@ match_orclause_to_indexcol(PlannerInfo *root,
>> Assert(IsA(orclause, BoolExpr));
>> Assert(orclause->boolop == OR_EXPR);
>>
>> + /* Ignore index if it doesn't support index scans */
>> + if(!index->amsearcharray)
>> + return NULL;
>> +
>>
>> Agree. I have updated the patch
>>
>> /*
>> * Try to convert a list of OR-clauses to a single SAOP expression. Each
>> * OR entry must be in the form: (indexkey operator constant) or (constant
>>
>> The test bug:
>> EXPLAIN SELECT COUNT(*) FROM tbl WHERE u = '00000000000000000000000000000000' OR u = '11111111111111111111111111111111';
>> QUERY PLAN
>> ----------------------------------------------------------------------------------------------------------------------------------
>> Aggregate (cost=12.46..12.47 rows=1 width=8)
>> -> Bitmap Heap Scan on tbl (cost=2.14..12.41 rows=18 width=0)
>> Recheck Cond: ((u = '00000000-0000-0000-0000-000000000000'::uuid) OR (u = '11111111-1111-1111-1111-111111111111'::uuid))
>> -> BitmapOr (cost=2.14..2.14 rows=18 width=0)
>> -> Bitmap Index Scan on idx (cost=0.00..1.07 rows=9 width=0)
>> Index Cond: (u = '00000000-0000-0000-0000-000000000000'::uuid)
>> -> Bitmap Index Scan on idx (cost=0.00..1.07 rows=9 width=0)
>> Index Cond: (u = '11111111-1111-1111-1111-111111111111'::uuid)
>> (8 rows)
> I slightly revised the fix and added similar check to
> group_similar_or_args(). Could you, please, review that before
> commit?
>
I agree with changes. Thank you!
--
Regards,
Alena Rybakina
Postgres Professional
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2024-11-29 07:54 Alexander Korotkov <[email protected]>
parent: Alena Rybakina <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: Alexander Korotkov @ 2024-11-29 07:54 UTC (permalink / raw)
To: Alena Rybakina <[email protected]>; +Cc: Ranier Vilela <[email protected]>; Alexander Lakhin <[email protected]>; Andrei Lepikhov <[email protected]>; jian he <[email protected]>; Robert Haas <[email protected]>; Nikolay Shaplov <[email protected]>; [email protected]; pgsql-hackers; Peter Geoghegan <[email protected]>; Marcos Pegoraro <[email protected]>; [email protected]; Peter Eisentraut <[email protected]>
On Fri, Nov 29, 2024 at 7:51 AM Alena Rybakina
<[email protected]> wrote:
>
> On 29.11.2024 03:04, Alexander Korotkov wrote:
> > On Thu, Nov 28, 2024 at 9:33 PM Alena Rybakina
> > <[email protected]> wrote:
> >> On 28.11.2024 22:28, Ranier Vilela wrote:
> >>
> >> Em qui., 28 de nov. de 2024 às 16:03, Alena Rybakina <[email protected]> escreveu:
> >>> Hi! Thank you for the case.
> >>>
> >>> On 28.11.2024 21:00, Alexander Lakhin wrote:
> >>>> Hello Alexander,
> >>>>
> >>>> 21.11.2024 09:34, Alexander Korotkov wrote:
> >>>>> I'm going to push this if no objections.
> >>>> Please look at the following query, which triggers an error after
> >>>> ae4569161:
> >>>> SET random_page_cost = 1;
> >>>> CREATE TABLE tbl(u UUID);
> >>>> CREATE INDEX idx ON tbl USING HASH (u);
> >>>> SELECT COUNT(*) FROM tbl WHERE u = '00000000000000000000000000000000' OR
> >>>> u = '11111111111111111111111111111111';
> >>>>
> >>>> ERROR: XX000: ScalarArrayOpExpr index qual found where not allowed
> >>>> LOCATION: ExecIndexBuildScanKeys, nodeIndexscan.c:1625
> >>>>
> >>>>
> >>> I found out what the problem is index scan method was not generated. We
> >>> need to check this during OR clauses for SAOP transformation.
> >>>
> >>> There is a patch to fix this problem.
> >> Hi.
> >> Thanks for the quick fix.
> >>
> >> But I wonder if it is not possible to avoid all if the index is useless?
> >> Maybe moving your fix to the beginning of the function?
> >>
> >> diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
> >> index d827fc9f4d..5ea0b27d01 100644
> >> --- a/src/backend/optimizer/path/indxpath.c
> >> +++ b/src/backend/optimizer/path/indxpath.c
> >> @@ -3248,6 +3248,10 @@ match_orclause_to_indexcol(PlannerInfo *root,
> >> Assert(IsA(orclause, BoolExpr));
> >> Assert(orclause->boolop == OR_EXPR);
> >>
> >> + /* Ignore index if it doesn't support index scans */
> >> + if(!index->amsearcharray)
> >> + return NULL;
> >> +
> >>
> >> Agree. I have updated the patch
> >>
> >> /*
> >> * Try to convert a list of OR-clauses to a single SAOP expression. Each
> >> * OR entry must be in the form: (indexkey operator constant) or (constant
> >>
> >> The test bug:
> >> EXPLAIN SELECT COUNT(*) FROM tbl WHERE u = '00000000000000000000000000000000' OR u = '11111111111111111111111111111111';
> >> QUERY PLAN
> >> ----------------------------------------------------------------------------------------------------------------------------------
> >> Aggregate (cost=12.46..12.47 rows=1 width=8)
> >> -> Bitmap Heap Scan on tbl (cost=2.14..12.41 rows=18 width=0)
> >> Recheck Cond: ((u = '00000000-0000-0000-0000-000000000000'::uuid) OR (u = '11111111-1111-1111-1111-111111111111'::uuid))
> >> -> BitmapOr (cost=2.14..2.14 rows=18 width=0)
> >> -> Bitmap Index Scan on idx (cost=0.00..1.07 rows=9 width=0)
> >> Index Cond: (u = '00000000-0000-0000-0000-000000000000'::uuid)
> >> -> Bitmap Index Scan on idx (cost=0.00..1.07 rows=9 width=0)
> >> Index Cond: (u = '11111111-1111-1111-1111-111111111111'::uuid)
> >> (8 rows)
> > I slightly revised the fix and added similar check to
> > group_similar_or_args(). Could you, please, review that before
> > commit?
> >
> I agree with changes. Thank you!
Andrei, Alena, thank you for the feedback. Pushed!
------
Regards,
Alexander Korotkov
Supabase
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-01-12 18:39 Alexander Korotkov <[email protected]>
parent: Alexander Korotkov <[email protected]>
0 siblings, 3 replies; 38+ messages in thread
From: Alexander Korotkov @ 2025-01-12 18:39 UTC (permalink / raw)
To: Alena Rybakina <[email protected]>; +Cc: Ranier Vilela <[email protected]>; Alexander Lakhin <[email protected]>; Andrei Lepikhov <[email protected]>; jian he <[email protected]>; Robert Haas <[email protected]>; Nikolay Shaplov <[email protected]>; [email protected]; pgsql-hackers; Peter Geoghegan <[email protected]>; Marcos Pegoraro <[email protected]>; [email protected]; Peter Eisentraut <[email protected]>
On Fri, Nov 29, 2024 at 9:54 AM Alexander Korotkov <[email protected]> wrote:
> On Fri, Nov 29, 2024 at 7:51 AM Alena Rybakina
> <[email protected]> wrote:
> >
> > On 29.11.2024 03:04, Alexander Korotkov wrote:
> > > On Thu, Nov 28, 2024 at 9:33 PM Alena Rybakina
> > > <[email protected]> wrote:
> > >> On 28.11.2024 22:28, Ranier Vilela wrote:
> > >>
> > >> Em qui., 28 de nov. de 2024 às 16:03, Alena Rybakina <[email protected]> escreveu:
> > >>> Hi! Thank you for the case.
> > >>>
> > >>> On 28.11.2024 21:00, Alexander Lakhin wrote:
> > >>>> Hello Alexander,
> > >>>>
> > >>>> 21.11.2024 09:34, Alexander Korotkov wrote:
> > >>>>> I'm going to push this if no objections.
> > >>>> Please look at the following query, which triggers an error after
> > >>>> ae4569161:
> > >>>> SET random_page_cost = 1;
> > >>>> CREATE TABLE tbl(u UUID);
> > >>>> CREATE INDEX idx ON tbl USING HASH (u);
> > >>>> SELECT COUNT(*) FROM tbl WHERE u = '00000000000000000000000000000000' OR
> > >>>> u = '11111111111111111111111111111111';
> > >>>>
> > >>>> ERROR: XX000: ScalarArrayOpExpr index qual found where not allowed
> > >>>> LOCATION: ExecIndexBuildScanKeys, nodeIndexscan.c:1625
> > >>>>
> > >>>>
> > >>> I found out what the problem is index scan method was not generated. We
> > >>> need to check this during OR clauses for SAOP transformation.
> > >>>
> > >>> There is a patch to fix this problem.
> > >> Hi.
> > >> Thanks for the quick fix.
> > >>
> > >> But I wonder if it is not possible to avoid all if the index is useless?
> > >> Maybe moving your fix to the beginning of the function?
> > >>
> > >> diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
> > >> index d827fc9f4d..5ea0b27d01 100644
> > >> --- a/src/backend/optimizer/path/indxpath.c
> > >> +++ b/src/backend/optimizer/path/indxpath.c
> > >> @@ -3248,6 +3248,10 @@ match_orclause_to_indexcol(PlannerInfo *root,
> > >> Assert(IsA(orclause, BoolExpr));
> > >> Assert(orclause->boolop == OR_EXPR);
> > >>
> > >> + /* Ignore index if it doesn't support index scans */
> > >> + if(!index->amsearcharray)
> > >> + return NULL;
> > >> +
> > >>
> > >> Agree. I have updated the patch
> > >>
> > >> /*
> > >> * Try to convert a list of OR-clauses to a single SAOP expression. Each
> > >> * OR entry must be in the form: (indexkey operator constant) or (constant
> > >>
> > >> The test bug:
> > >> EXPLAIN SELECT COUNT(*) FROM tbl WHERE u = '00000000000000000000000000000000' OR u = '11111111111111111111111111111111';
> > >> QUERY PLAN
> > >> ----------------------------------------------------------------------------------------------------------------------------------
> > >> Aggregate (cost=12.46..12.47 rows=1 width=8)
> > >> -> Bitmap Heap Scan on tbl (cost=2.14..12.41 rows=18 width=0)
> > >> Recheck Cond: ((u = '00000000-0000-0000-0000-000000000000'::uuid) OR (u = '11111111-1111-1111-1111-111111111111'::uuid))
> > >> -> BitmapOr (cost=2.14..2.14 rows=18 width=0)
> > >> -> Bitmap Index Scan on idx (cost=0.00..1.07 rows=9 width=0)
> > >> Index Cond: (u = '00000000-0000-0000-0000-000000000000'::uuid)
> > >> -> Bitmap Index Scan on idx (cost=0.00..1.07 rows=9 width=0)
> > >> Index Cond: (u = '11111111-1111-1111-1111-111111111111'::uuid)
> > >> (8 rows)
> > > I slightly revised the fix and added similar check to
> > > group_similar_or_args(). Could you, please, review that before
> > > commit?
> > >
> > I agree with changes. Thank you!
>
> Andrei, Alena, thank you for the feedback. Pushed!
I think we should give some more attention to the patch enabling OR to
SAOP transformation for joins (first time posted in [1]). I think we
tried to only work with Const and Param, because we were previously
working during parse stage. So, at that stage if we have the clause
like "a.x = 1 OR a.x = b.x OR b.x = 2", then we don't know if we
should transform it into "a.x = ANY(1, b.x) OR b.x = 2" or into "a.x
=1 OR b.x = ANY(a.x, 2)". But if we do the transformation during the
index matching, we would actually be able to try the both and select
the best.
The revised patch is attached. Most notably it revises
group_similar_or_args() to have the same notion of const-ness as
others. In that function we split potential index key and constant
early to save time on enumerating all possible index keys. But it
appears to be possible to split by relids bitmapsets: index key should
use our relid, while const shouldn't. Other that that, comments,
commit message and naming are revised.
Links.
1. https://www.postgresql.org/message-id/CAPpHfdu9QJ%3DGbua3CUUH2KKG_8urakJTen4JD47PGh9wWP%3DQxQ%40mail...
------
Regards,
Alexander Korotkov
Supabase
Attachments:
[application/octet-stream] v45-0001-Allow-usage-of-match_orclause_to_indexcol-for-jo.patch (9.6K, ../../CAPpHfdv+jtNwofg-p5z86jLYZUTt6tR17Wy00ta0dL=wHQN3ZA@mail.gmail.com/2-v45-0001-Allow-usage-of-match_orclause_to_indexcol-for-jo.patch)
download | inline diff:
From e0bc064032af36d027c497aa5e4302d4fb667950 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Wed, 9 Oct 2024 18:44:25 +0300
Subject: [PATCH v45] Allow usage of match_orclause_to_indexcol() for joins
This commit allows transformation of OR-clauses into SAOP's for index scans
within nested loop joins. That required the following changes.
1. Make match_orclause_to_indexcol() and group_similar_or_args() understand
const-ness in the same way as match_opclause_to_indexcol(). This
generally makes our approach more uniform.
2. Make match_join_clauses_to_index() pass OR-clauses to
match_clause_to_index().
---
src/backend/optimizer/path/indxpath.c | 61 +++++++++++---------
src/test/regress/expected/join.out | 9 +--
src/test/regress/expected/partition_join.out | 12 ++--
3 files changed, 43 insertions(+), 39 deletions(-)
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index 5f428e835b0..c03547a9973 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -1255,6 +1255,7 @@ group_similar_or_args(PlannerInfo *root, RelOptInfo *rel, RestrictInfo *rinfo)
ListCell *lc2;
List *orargs;
List *result = NIL;
+ Index relid = rel->relid;
Assert(IsA(rinfo->orclause, BoolExpr));
orargs = ((BoolExpr *) rinfo->orclause)->args;
@@ -1319,10 +1320,13 @@ group_similar_or_args(PlannerInfo *root, RelOptInfo *rel, RestrictInfo *rinfo)
/*
* Check for clauses of the form: (indexkey operator constant) or
* (constant operator indexkey). But we don't know a particular index
- * yet. First check for a constant, which must be Const or Param.
- * That's cheaper than search for an index key among all indexes.
+ * yet. Therefore, we try to distinguish the potential index key and
+ * constant first, then search for a matching index key among all
+ * indexes.
*/
- if (IsA(leftop, Const) || IsA(leftop, Param))
+ if (bms_is_member(relid, argrinfo->right_relids) &&
+ !bms_is_member(relid, argrinfo->left_relids) &&
+ !contain_volatile_functions(leftop))
{
opno = get_commutator(opno);
@@ -1333,7 +1337,9 @@ group_similar_or_args(PlannerInfo *root, RelOptInfo *rel, RestrictInfo *rinfo)
}
nonConstExpr = rightop;
}
- else if (IsA(rightop, Const) || IsA(rightop, Param))
+ else if (bms_is_member(relid, argrinfo->left_relids) &&
+ !bms_is_member(relid, argrinfo->right_relids) &&
+ !contain_volatile_functions(rightop))
{
nonConstExpr = leftop;
}
@@ -2435,8 +2441,8 @@ match_join_clauses_to_index(PlannerInfo *root,
/* Potentially usable, so see if it matches the index or is an OR */
if (restriction_is_or_clause(rinfo))
*joinorclauses = lappend(*joinorclauses, rinfo);
- else
- match_clause_to_index(root, rinfo, index, clauseset);
+
+ match_clause_to_index(root, rinfo, index, clauseset);
}
}
@@ -2585,10 +2591,7 @@ match_clause_to_index(PlannerInfo *root,
* (3) must match the collation of the index, if collation is relevant.
*
* Our definition of "const" is exceedingly liberal: we allow anything that
- * doesn't involve a volatile function or a Var of the index's relation
- * except for a boolean OR expression input: due to a trade-off between the
- * expected execution speedup and planning complexity, we limit or->saop
- * transformation by obvious cases when an index scan can get a profit.
+ * doesn't involve a volatile function or a Var of the index's relation.
* In particular, Vars belonging to other relations of the query are
* accepted here, since a clause of that form can be used in a
* parameterized indexscan. It's the responsibility of higher code levels
@@ -3246,7 +3249,8 @@ match_orclause_to_indexcol(PlannerInfo *root,
Oid arraytype = InvalidOid;
Oid inputcollid = InvalidOid;
bool firstTime = true;
- bool haveParam = false;
+ bool haveNonConst = false;
+ Index indexRelid = index->rel->relid;
Assert(IsA(orclause, BoolExpr));
Assert(orclause->boolop == OR_EXPR);
@@ -3258,10 +3262,9 @@ match_orclause_to_indexcol(PlannerInfo *root,
/*
* Try to convert a list of OR-clauses to a single SAOP expression. Each
* OR entry must be in the form: (indexkey operator constant) or (constant
- * operator indexkey). Operators of all the entries must match. Constant
- * might be either Const or Param. To be effective, give up on the first
- * non-matching entry. Exit is implemented as a break from the loop,
- * which is catched afterwards.
+ * operator indexkey). Operators of all the entries must match. To be
+ * effective, give up on the first non-matching entry. Exit is
+ * implemented as a break from the loop, which is catched afterwards.
*/
foreach(lc, orclause->args)
{
@@ -3312,17 +3315,21 @@ match_orclause_to_indexcol(PlannerInfo *root,
/*
* Check for clauses of the form: (indexkey operator constant) or
- * (constant operator indexkey). Determine indexkey side first, check
- * the constant later.
+ * (constant operator indexkey). See match_clause_to_indexcol's notes
+ * about const-ness.
*/
leftop = (Node *) linitial(subClause->args);
rightop = (Node *) lsecond(subClause->args);
- if (match_index_to_operand(leftop, indexcol, index))
+ if (match_index_to_operand(leftop, indexcol, index) &&
+ !bms_is_member(indexRelid, rinfo->right_relids) &&
+ !contain_volatile_functions(rightop))
{
indexExpr = leftop;
constExpr = rightop;
}
- else if (match_index_to_operand(rightop, indexcol, index))
+ else if (match_index_to_operand(rightop, indexcol, index) &&
+ !bms_is_member(indexRelid, rinfo->left_relids) &&
+ !contain_volatile_functions(leftop))
{
opno = get_commutator(opno);
if (!OidIsValid(opno))
@@ -3349,10 +3356,6 @@ match_orclause_to_indexcol(PlannerInfo *root,
if (IsA(indexExpr, RelabelType))
indexExpr = (Node *) ((RelabelType *) indexExpr)->arg;
- /* We allow constant to be Const or Param */
- if (!IsA(constExpr, Const) && !IsA(constExpr, Param))
- break;
-
/* Forbid transformation for composite types, records. */
if (type_is_rowtype(exprType(constExpr)) ||
type_is_rowtype(exprType(indexExpr)))
@@ -3389,8 +3392,12 @@ match_orclause_to_indexcol(PlannerInfo *root,
break;
}
- if (IsA(constExpr, Param))
- haveParam = true;
+ /*
+ * Check if our list of constants in match_clause_to_indexcol's
+ * understanding of const-ness have something other than Const.
+ */
+ if (!IsA(constExpr, Const))
+ haveNonConst = true;
consts = lappend(consts, constExpr);
}
@@ -3407,10 +3414,10 @@ match_orclause_to_indexcol(PlannerInfo *root,
/*
* Assemble an array from the list of constants. It seems more profitable
- * to build a const array. But in the presence of parameters, we don't
+ * to build a const array. But in the presence of other nodes, we don't
* have a specific value here and must employ an ArrayExpr instead.
*/
- if (haveParam)
+ if (haveNonConst)
{
ArrayExpr *arrayExpr = makeNode(ArrayExpr);
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 079fcf46f0d..634eff4d751 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -3849,14 +3849,11 @@ where q1 = thousand or q2 = thousand;
-> Seq Scan on q2
-> Bitmap Heap Scan on tenk1
Recheck Cond: ((q1.q1 = thousand) OR (q2.q2 = thousand))
- -> BitmapOr
- -> Bitmap Index Scan on tenk1_thous_tenthous
- Index Cond: (thousand = q1.q1)
- -> Bitmap Index Scan on tenk1_thous_tenthous
- Index Cond: (thousand = q2.q2)
+ -> Bitmap Index Scan on tenk1_thous_tenthous
+ Index Cond: (thousand = ANY (ARRAY[q1.q1, q2.q2]))
-> Hash
-> Seq Scan on int4_tbl
-(15 rows)
+(12 rows)
explain (costs off)
select * from
diff --git a/src/test/regress/expected/partition_join.out b/src/test/regress/expected/partition_join.out
index 108f9ecb445..af468682a2d 100644
--- a/src/test/regress/expected/partition_join.out
+++ b/src/test/regress/expected/partition_join.out
@@ -2533,24 +2533,24 @@ where not exists (select 1 from prtx2
-> Seq Scan on prtx1_1
Filter: ((a < 20) AND (c = 91))
-> Bitmap Heap Scan on prtx2_1
- Recheck Cond: ((b = (prtx1_1.b + 1)) OR (c = 99))
+ Recheck Cond: ((c = 99) OR (b = (prtx1_1.b + 1)))
Filter: (a = prtx1_1.a)
-> BitmapOr
- -> Bitmap Index Scan on prtx2_1_b_idx
- Index Cond: (b = (prtx1_1.b + 1))
-> Bitmap Index Scan on prtx2_1_c_idx
Index Cond: (c = 99)
+ -> Bitmap Index Scan on prtx2_1_b_idx
+ Index Cond: (b = (prtx1_1.b + 1))
-> Nested Loop Anti Join
-> Seq Scan on prtx1_2
Filter: ((a < 20) AND (c = 91))
-> Bitmap Heap Scan on prtx2_2
- Recheck Cond: ((b = (prtx1_2.b + 1)) OR (c = 99))
+ Recheck Cond: ((c = 99) OR (b = (prtx1_2.b + 1)))
Filter: (a = prtx1_2.a)
-> BitmapOr
- -> Bitmap Index Scan on prtx2_2_b_idx
- Index Cond: (b = (prtx1_2.b + 1))
-> Bitmap Index Scan on prtx2_2_c_idx
Index Cond: (c = 99)
+ -> Bitmap Index Scan on prtx2_2_b_idx
+ Index Cond: (b = (prtx1_2.b + 1))
(23 rows)
select * from prtx1
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-01-12 20:38 Alena Rybakina <[email protected]>
parent: Alexander Korotkov <[email protected]>
2 siblings, 0 replies; 38+ messages in thread
From: Alena Rybakina @ 2025-01-12 20:38 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; +Cc: Ranier Vilela <[email protected]>; Alexander Lakhin <[email protected]>; Andrei Lepikhov <[email protected]>; jian he <[email protected]>; Robert Haas <[email protected]>; Nikolay Shaplov <[email protected]>; [email protected]; pgsql-hackers; Peter Geoghegan <[email protected]>; Marcos Pegoraro <[email protected]>; [email protected]; Peter Eisentraut <[email protected]>
Hi!
On 12.01.2025 21:39, Alexander Korotkov wrote:
> On Fri, Nov 29, 2024 at 9:54 AM Alexander Korotkov <[email protected]> wrote:
>> On Fri, Nov 29, 2024 at 7:51 AM Alena Rybakina
>> <[email protected]> wrote:
>>> On 29.11.2024 03:04, Alexander Korotkov wrote:
>>>> On Thu, Nov 28, 2024 at 9:33 PM Alena Rybakina
>>>> <[email protected]> wrote:
>>>>> On 28.11.2024 22:28, Ranier Vilela wrote:
>>>>>
>>>>> Em qui., 28 de nov. de 2024 às 16:03, Alena Rybakina <[email protected]> escreveu:
>>>>>> Hi! Thank you for the case.
>>>>>>
>>>>>> On 28.11.2024 21:00, Alexander Lakhin wrote:
>>>>>>> Hello Alexander,
>>>>>>>
>>>>>>> 21.11.2024 09:34, Alexander Korotkov wrote:
>>>>>>>> I'm going to push this if no objections.
>>>>>>> Please look at the following query, which triggers an error after
>>>>>>> ae4569161:
>>>>>>> SET random_page_cost = 1;
>>>>>>> CREATE TABLE tbl(u UUID);
>>>>>>> CREATE INDEX idx ON tbl USING HASH (u);
>>>>>>> SELECT COUNT(*) FROM tbl WHERE u = '00000000000000000000000000000000' OR
>>>>>>> u = '11111111111111111111111111111111';
>>>>>>>
>>>>>>> ERROR: XX000: ScalarArrayOpExpr index qual found where not allowed
>>>>>>> LOCATION: ExecIndexBuildScanKeys, nodeIndexscan.c:1625
>>>>>>>
>>>>>>>
>>>>>> I found out what the problem is index scan method was not generated. We
>>>>>> need to check this during OR clauses for SAOP transformation.
>>>>>>
>>>>>> There is a patch to fix this problem.
>>>>> Hi.
>>>>> Thanks for the quick fix.
>>>>>
>>>>> But I wonder if it is not possible to avoid all if the index is useless?
>>>>> Maybe moving your fix to the beginning of the function?
>>>>>
>>>>> diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
>>>>> index d827fc9f4d..5ea0b27d01 100644
>>>>> --- a/src/backend/optimizer/path/indxpath.c
>>>>> +++ b/src/backend/optimizer/path/indxpath.c
>>>>> @@ -3248,6 +3248,10 @@ match_orclause_to_indexcol(PlannerInfo *root,
>>>>> Assert(IsA(orclause, BoolExpr));
>>>>> Assert(orclause->boolop == OR_EXPR);
>>>>>
>>>>> + /* Ignore index if it doesn't support index scans */
>>>>> + if(!index->amsearcharray)
>>>>> + return NULL;
>>>>> +
>>>>>
>>>>> Agree. I have updated the patch
>>>>>
>>>>> /*
>>>>> * Try to convert a list of OR-clauses to a single SAOP expression. Each
>>>>> * OR entry must be in the form: (indexkey operator constant) or (constant
>>>>>
>>>>> The test bug:
>>>>> EXPLAIN SELECT COUNT(*) FROM tbl WHERE u = '00000000000000000000000000000000' OR u = '11111111111111111111111111111111';
>>>>> QUERY PLAN
>>>>> ----------------------------------------------------------------------------------------------------------------------------------
>>>>> Aggregate (cost=12.46..12.47 rows=1 width=8)
>>>>> -> Bitmap Heap Scan on tbl (cost=2.14..12.41 rows=18 width=0)
>>>>> Recheck Cond: ((u = '00000000-0000-0000-0000-000000000000'::uuid) OR (u = '11111111-1111-1111-1111-111111111111'::uuid))
>>>>> -> BitmapOr (cost=2.14..2.14 rows=18 width=0)
>>>>> -> Bitmap Index Scan on idx (cost=0.00..1.07 rows=9 width=0)
>>>>> Index Cond: (u = '00000000-0000-0000-0000-000000000000'::uuid)
>>>>> -> Bitmap Index Scan on idx (cost=0.00..1.07 rows=9 width=0)
>>>>> Index Cond: (u = '11111111-1111-1111-1111-111111111111'::uuid)
>>>>> (8 rows)
>>>> I slightly revised the fix and added similar check to
>>>> group_similar_or_args(). Could you, please, review that before
>>>> commit?
>>>>
>>> I agree with changes. Thank you!
>> Andrei, Alena, thank you for the feedback. Pushed!
> I think we should give some more attention to the patch enabling OR to
> SAOP transformation for joins (first time posted in [1]). I think we
> tried to only work with Const and Param, because we were previously
> working during parse stage. So, at that stage if we have the clause
> like "a.x = 1 OR a.x = b.x OR b.x = 2", then we don't know if we
> should transform it into "a.x = ANY(1, b.x) OR b.x = 2" or into "a.x
> =1 OR b.x = ANY(a.x, 2)". But if we do the transformation during the
> index matching, we would actually be able to try the both and select
> the best.
>
> The revised patch is attached. Most notably it revises
> group_similar_or_args() to have the same notion of const-ness as
> others. In that function we split potential index key and constant
> early to save time on enumerating all possible index keys. But it
> appears to be possible to split by relids bitmapsets: index key should
> use our relid, while const shouldn't. Other that that, comments,
> commit message and naming are revised.
>
> Links.
> 1. https://www.postgresql.org/message-id/CAPpHfdu9QJ%3DGbua3CUUH2KKG_8urakJTen4JD47PGh9wWP%3DQxQ%40mail...
>
I like your idea. I looked at your patch and haven't noticed any bugs
yet, but my review is not finished.
I think we're missing tests here - I only noticed one difference in the
regression test related to your specific improvement.
I thought it would be possible to look at cases where q1 and q2 are not
equal to an integer constant table,
but have a more complex structure. For example, set the conditions "q1
as select (1=1)::integer" and "q2 as select (1=0)::integer".
--
Regards,
Alena Rybakina
Postgres Professional
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-01-13 00:47 Yura Sokolov <[email protected]>
parent: Alexander Korotkov <[email protected]>
2 siblings, 1 reply; 38+ messages in thread
From: Yura Sokolov @ 2025-01-13 00:47 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; +Cc: Alena Rybakina <[email protected]>; Ranier Vilela <[email protected]>; Alexander Lakhin <[email protected]>; Andrei Lepikhov <[email protected]>; jian he <[email protected]>; Robert Haas <[email protected]>; Nikolay Shaplov <[email protected]>; [email protected]; pgsql-hackers; Peter Geoghegan <[email protected]>; Marcos Pegoraro <[email protected]>; [email protected]; Peter Eisentraut <[email protected]>
<div>Вс, 12 янв. 2025 г. в 21:39, Alexander Korotkov <[email protected]>:<br></div><div><div class="gmail_quote gmail_quote_container"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">On Fri, Nov 29, 2024 at 9:54 AM Alexander Korotkov <[email protected]> wrote:
<br>> On Fri, Nov 29, 2024 at 7:51 AM Alena Rybakina
<br>> <[email protected]> wrote:
<br>> >
<br>> > On 29.11.2024 03:04, Alexander Korotkov wrote:
<br>> > > On Thu, Nov 28, 2024 at 9:33 PM Alena Rybakina
<br>> > > <[email protected]> wrote:
<br>> > >> On 28.11.2024 22:28, Ranier Vilela wrote:
<br>> > >>
<br>> > >> Em qui., 28 de nov. de 2024 às 16:03, Alena Rybakina <[email protected]> escreveu:
<br>> > >>> Hi! Thank you for the case.
<br>> > >>>
<br>> > >>> On 28.11.2024 21:00, Alexander Lakhin wrote:
<br>> > >>>> Hello Alexander,
<br>> > >>>>
<br>> > >>>> 21.11.2024 09:34, Alexander Korotkov wrote:
<br>> > >>>>> I'm going to push this if no objections.
<br>> > >>>> Please look at the following query, which triggers an error after
<br>> > >>>> ae4569161:
<br>> > >>>> SET random_page_cost = 1;
<br>> > >>>> CREATE TABLE tbl(u UUID);
<br>> > >>>> CREATE INDEX idx ON tbl USING HASH (u);
<br>> > >>>> SELECT COUNT(*) FROM tbl WHERE u = '00000000000000000000000000000000' OR
<br>> > >>>> u = '11111111111111111111111111111111';
<br>> > >>>>
<br>> > >>>> ERROR: XX000: ScalarArrayOpExpr index qual found where not allowed
<br>> > >>>> LOCATION: ExecIndexBuildScanKeys, nodeIndexscan.c:1625
<br>> > >>>>
<br>> > >>>>
<br>> > >>> I found out what the problem is index scan method was not generated. We
<br>> > >>> need to check this during OR clauses for SAOP transformation.
<br>> > >>>
<br>> > >>> There is a patch to fix this problem.
<br>> > >> Hi.
<br>> > >> Thanks for the quick fix.
<br>> > >>
<br>> > >> But I wonder if it is not possible to avoid all if the index is useless?
<br>> > >> Maybe moving your fix to the beginning of the function?
<br>> > >>
<br>> > >> diff --git a/src/backend/optimizer/path/indxpath.<wbr>c b/src/backend/optimizer/path/indxpath.c
<br>> > >> index d827fc9f4d..5ea0b27d01 100644
<br>> > >> --- a/src/backend/optimizer/path/indxpath.c
<br>> > >> +++ b/src/backend/optimizer/path/indxpath.c
<br>> > >> @@ -3248,6 +3248,10 @@ match_orclause_to_indexcol(PlannerInfo *root,
<br>> > >> Assert(IsA(orclause, BoolExpr));
<br>> > >> Assert(orclause->boolop == OR_EXPR);
<br>> > >>
<br>> > >> + /* Ignore index if it doesn't support index scans */
<br>> > >> + if(!index->amsearcharray)
<br>> > >> + return NULL;
<br>> > >> +
<br>> > >>
<br>> > >> Agree. I have updated the patch
<br>> > >>
<br>> > >> /*
<br>> > >> * Try to convert a list of OR-clauses to a single SAOP expression. Each
<br>> > >> * OR entry must be in the form: (indexkey operator constant) or (constant
<br>> > >>
<br>> > >> The test bug:
<br>> > >> EXPLAIN SELECT COUNT(*) FROM tbl WHERE u = '00000000000000000000000000000000'<wbr> OR u = '11111111111111111111111111111111';
<br>> > >> QUERY PLAN
<br>> > >> --------------------------------<wbr>--------------------------------<wbr>--------------------------------<wbr>----------------------------------
<br>> > >> Aggregate (cost=12.46..12.47 rows=1 width=8)
<br>> > >> -> Bitmap Heap Scan on tbl (cost=2.14..12.41 rows=18 width=0)
<br>> > >> Recheck Cond: ((u = '00000000-0000-0000-0000-000000000000'<wbr>::uuid) OR (u = '11111111-1111-1111-1111-111111111111'<wbr>::uuid))
<br>> > >> -> BitmapOr (cost=2.14..2.14 rows=18 width=0)
<br>> > >> -> Bitmap Index Scan on idx (cost=0.00..1.07 rows=9 width=0)
<br>> > >> Index Cond: (u = '00000000-0000-0000-0000-000000000000'<wbr>::uuid)
<br>> > >> -> Bitmap Index Scan on idx (cost=0.00..1.07 rows=9 width=0)
<br>> > >> Index Cond: (u = '11111111-1111-1111-1111-111111111111'<wbr>::uuid)
<br>> > >> (8 rows)
<br>> > > I slightly revised the fix and added similar check to
<br>> > > group_similar_or_args(). Could you, please, review that before
<br>> > > commit?
<br>> > >
<br>> > I agree with changes. Thank you!
<br>>
<br>> Andrei, Alena, thank you for the feedback. Pushed!
<br>
<br>I think we should give some more attention to the patch enabling OR to
<br>SAOP transformation for joins (first time posted in [1]). I think we
<br>tried to only work with Const and Param, because we were previously
<br>working during parse stage. So, at that stage if we have the clause
<br>like "a.x = 1 OR a.x = b.x OR b.x = 2", then we don't know if we
<br>should transform it into "a.x = ANY(1, b.x) OR b.x = 2" or into "a.x
<br>=1 OR b.x = ANY(a.x, 2)". But if we do the transformation during the
<br>index matching, we would actually be able to try the both and select
<br>the best.
</blockquote><div dir="auto"><br></div><div dir="auto">But why not “a.x = ANY(1, b.x) OR b.x = ANY(a.x, 2)” ? Looks strange, but correct ))</div><div dir="auto"><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;" dir="auto"><br>The revised patch is attached. Most notably it revises
<br>group_similar_or_args() to have the same notion of const-ness as
<br>others. In that function we split potential index key and constant
<br>early to save time on enumerating all possible index keys. But it
<br>appears to be possible to split by relids bitmapsets: index key should
<br>use our relid, while const shouldn't. Other that that, comments,
<br>commit message and naming are revised.
<br>
<br>Links.
<br>1. https://www.postgresql.org/message-<wbr>id/CAPpHfdu9QJ%3DGbua3CUUH2KKG_8urakJTen4JD47PGh9wWP%3...
<br>
<br>------
<br>Regards,
<br>Alexander Korotkov
<br>Supabase
<br></blockquote></div></div>
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-01-13 01:06 Alexander Korotkov <[email protected]>
parent: Yura Sokolov <[email protected]>
0 siblings, 0 replies; 38+ messages in thread
From: Alexander Korotkov @ 2025-01-13 01:06 UTC (permalink / raw)
To: Yura Sokolov <[email protected]>; +Cc: Alena Rybakina <[email protected]>; Ranier Vilela <[email protected]>; Alexander Lakhin <[email protected]>; Andrei Lepikhov <[email protected]>; jian he <[email protected]>; Robert Haas <[email protected]>; Nikolay Shaplov <[email protected]>; [email protected]; pgsql-hackers; Peter Geoghegan <[email protected]>; Marcos Pegoraro <[email protected]>; [email protected]; Peter Eisentraut <[email protected]>
On Mon, Jan 13, 2025 at 2:47 AM Yura Sokolov <[email protected]> wrote:
> Вс, 12 янв. 2025 г. в 21:39, Alexander Korotkov <[email protected]>:
>>
>> On Fri, Nov 29, 2024 at 9:54 AM Alexander Korotkov <[email protected]> wrote:
>> > On Fri, Nov 29, 2024 at 7:51 AM Alena Rybakina
>> > <[email protected]> wrote:
>> > >
>> > > On 29.11.2024 03:04, Alexander Korotkov wrote:
>> > > > On Thu, Nov 28, 2024 at 9:33 PM Alena Rybakina
>> > > > <[email protected]> wrote:
>> > > >> On 28.11.2024 22:28, Ranier Vilela wrote:
>> > > >>
>> > > >> Em qui., 28 de nov. de 2024 às 16:03, Alena Rybakina <[email protected]> escreveu:
>> > > >>> Hi! Thank you for the case.
>> > > >>>
>> > > >>> On 28.11.2024 21:00, Alexander Lakhin wrote:
>> > > >>>> Hello Alexander,
>> > > >>>>
>> > > >>>> 21.11.2024 09:34, Alexander Korotkov wrote:
>> > > >>>>> I'm going to push this if no objections.
>> > > >>>> Please look at the following query, which triggers an error after
>> > > >>>> ae4569161:
>> > > >>>> SET random_page_cost = 1;
>> > > >>>> CREATE TABLE tbl(u UUID);
>> > > >>>> CREATE INDEX idx ON tbl USING HASH (u);
>> > > >>>> SELECT COUNT(*) FROM tbl WHERE u = '00000000000000000000000000000000' OR
>> > > >>>> u = '11111111111111111111111111111111';
>> > > >>>>
>> > > >>>> ERROR: XX000: ScalarArrayOpExpr index qual found where not allowed
>> > > >>>> LOCATION: ExecIndexBuildScanKeys, nodeIndexscan.c:1625
>> > > >>>>
>> > > >>>>
>> > > >>> I found out what the problem is index scan method was not generated. We
>> > > >>> need to check this during OR clauses for SAOP transformation.
>> > > >>>
>> > > >>> There is a patch to fix this problem.
>> > > >> Hi.
>> > > >> Thanks for the quick fix.
>> > > >>
>> > > >> But I wonder if it is not possible to avoid all if the index is useless?
>> > > >> Maybe moving your fix to the beginning of the function?
>> > > >>
>> > > >> diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
>> > > >> index d827fc9f4d..5ea0b27d01 100644
>> > > >> --- a/src/backend/optimizer/path/indxpath.c
>> > > >> +++ b/src/backend/optimizer/path/indxpath.c
>> > > >> @@ -3248,6 +3248,10 @@ match_orclause_to_indexcol(PlannerInfo *root,
>> > > >> Assert(IsA(orclause, BoolExpr));
>> > > >> Assert(orclause->boolop == OR_EXPR);
>> > > >>
>> > > >> + /* Ignore index if it doesn't support index scans */
>> > > >> + if(!index->amsearcharray)
>> > > >> + return NULL;
>> > > >> +
>> > > >>
>> > > >> Agree. I have updated the patch
>> > > >>
>> > > >> /*
>> > > >> * Try to convert a list of OR-clauses to a single SAOP expression. Each
>> > > >> * OR entry must be in the form: (indexkey operator constant) or (constant
>> > > >>
>> > > >> The test bug:
>> > > >> EXPLAIN SELECT COUNT(*) FROM tbl WHERE u = '00000000000000000000000000000000' OR u = '11111111111111111111111111111111';
>> > > >> QUERY PLAN
>> > > >> ----------------------------------------------------------------------------------------------------------------------------------
>> > > >> Aggregate (cost=12.46..12.47 rows=1 width=8)
>> > > >> -> Bitmap Heap Scan on tbl (cost=2.14..12.41 rows=18 width=0)
>> > > >> Recheck Cond: ((u = '00000000-0000-0000-0000-000000000000'::uuid) OR (u = '11111111-1111-1111-1111-111111111111'::uuid))
>> > > >> -> BitmapOr (cost=2.14..2.14 rows=18 width=0)
>> > > >> -> Bitmap Index Scan on idx (cost=0.00..1.07 rows=9 width=0)
>> > > >> Index Cond: (u = '00000000-0000-0000-0000-000000000000'::uuid)
>> > > >> -> Bitmap Index Scan on idx (cost=0.00..1.07 rows=9 width=0)
>> > > >> Index Cond: (u = '11111111-1111-1111-1111-111111111111'::uuid)
>> > > >> (8 rows)
>> > > > I slightly revised the fix and added similar check to
>> > > > group_similar_or_args(). Could you, please, review that before
>> > > > commit?
>> > > >
>> > > I agree with changes. Thank you!
>> >
>> > Andrei, Alena, thank you for the feedback. Pushed!
>>
>> I think we should give some more attention to the patch enabling OR to
>> SAOP transformation for joins (first time posted in [1]). I think we
>> tried to only work with Const and Param, because we were previously
>> working during parse stage. So, at that stage if we have the clause
>> like "a.x = 1 OR a.x = b.x OR b.x = 2", then we don't know if we
>> should transform it into "a.x = ANY(1, b.x) OR b.x = 2" or into "a.x
>> =1 OR b.x = ANY(a.x, 2)". But if we do the transformation during the
>> index matching, we would actually be able to try the both and select
>> the best.
>
>
> But why not “a.x = ANY(1, b.x) OR b.x = ANY(a.x, 2)” ? Looks strange, but correct ))
That could probably work for a parse stage, but as you can check that
approach has a lot of other problems. As we do during index matching,
that doesn't matter. I just wanted to state that nothing in the
current approach prevent us from working the same way for joins.
------
Regards,
Alexander Korotkov
Supabase
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-01-13 03:39 Andrei Lepikhov <[email protected]>
parent: Alexander Korotkov <[email protected]>
2 siblings, 1 reply; 38+ messages in thread
From: Andrei Lepikhov @ 2025-01-13 03:39 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; Alena Rybakina <[email protected]>; +Cc: Ranier Vilela <[email protected]>; Alexander Lakhin <[email protected]>; jian he <[email protected]>; Robert Haas <[email protected]>; Nikolay Shaplov <[email protected]>; [email protected]; pgsql-hackers; Peter Geoghegan <[email protected]>; Marcos Pegoraro <[email protected]>; [email protected]; Peter Eisentraut <[email protected]>
On 1/13/25 01:39, Alexander Korotkov wrote:
> The revised patch is attached. Most notably it revises
> group_similar_or_args() to have the same notion of const-ness as
> others. In that function we split potential index key and constant
> early to save time on enumerating all possible index keys. But it
> appears to be possible to split by relids bitmapsets: index key should
> use our relid, while const shouldn't. Other that that, comments,
> commit message and naming are revised.
Hmm, I would say we should carefully review this code.
Curiously, this patch has activated the dormant problem of duplicated
clauses in joinorclauses. Look:
EXPLAIN (COSTS OFF)
SELECT * FROM bitmap_split_or t1, bitmap_split_or t2
WHERE t1.a=t2.b OR t1.a=1;
Nested Loop
-> Seq Scan on bitmap_split_or t2
-> Bitmap Heap Scan on bitmap_split_or t1
Recheck Cond: (((a = t2.b) OR (a = 1)) AND
((a = t2.b) OR (a = 1)))
-> Bitmap Index Scan on t_a_b_idx
Index Cond: ((a = ANY (ARRAY[t2.b, 1])) AND
(a = ANY (ARRAY[t2.b, 1])))
It can be resolved with a single-line change (see attached). But I need
some time to ponder over the changing behaviour when a clause may match
an index and be in joinorclauses.
--
regards, Andrei Lepikhov
Attachments:
[text/x-patch] joinorclauses-fix.diff (2.2K, ../../[email protected]/2-joinorclauses-fix.diff)
download | inline diff:
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index c03547a997..2a86298d9d 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -2440,7 +2440,7 @@ match_join_clauses_to_index(PlannerInfo *root,
/* Potentially usable, so see if it matches the index or is an OR */
if (restriction_is_or_clause(rinfo))
- *joinorclauses = lappend(*joinorclauses, rinfo);
+ *joinorclauses = list_append_unique_ptr(*joinorclauses, rinfo);
match_clause_to_index(root, rinfo, index, clauseset);
}
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 8011c141bf..47bdee1d41 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -3255,6 +3255,16 @@ CREATE INDEX t_b_c_idx ON bitmap_split_or (b, c);
CREATE STATISTICS t_a_b_stat (mcv) ON a, b FROM bitmap_split_or;
CREATE STATISTICS t_b_c_stat (mcv) ON b, c FROM bitmap_split_or;
ANALYZE bitmap_split_or;
+EXPLAIN (COSTS OFF)
+SELECT * FROM bitmap_split_or t1, bitmap_split_or t2 WHERE t1.a=t2.b OR t1.a=1;
+ QUERY PLAN
+--------------------------------------------------------
+ Nested Loop
+ -> Seq Scan on bitmap_split_or t2
+ -> Index Scan using t_a_b_idx on bitmap_split_or t1
+ Index Cond: (a = ANY (ARRAY[t2.b, 1]))
+(4 rows)
+
EXPLAIN (COSTS OFF)
SELECT * FROM bitmap_split_or WHERE a = 1 AND (b = 1 OR b = 2) AND c = 2;
QUERY PLAN
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index 068c66b95a..bc12d2f098 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -1367,6 +1367,8 @@ CREATE STATISTICS t_a_b_stat (mcv) ON a, b FROM bitmap_split_or;
CREATE STATISTICS t_b_c_stat (mcv) ON b, c FROM bitmap_split_or;
ANALYZE bitmap_split_or;
EXPLAIN (COSTS OFF)
+SELECT * FROM bitmap_split_or t1, bitmap_split_or t2 WHERE t1.a=t2.b OR t1.a=1;
+EXPLAIN (COSTS OFF)
SELECT * FROM bitmap_split_or WHERE a = 1 AND (b = 1 OR b = 2) AND c = 2;
DROP TABLE bitmap_split_or;
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-01-15 08:24 Andrei Lepikhov <[email protected]>
parent: Andrei Lepikhov <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: Andrei Lepikhov @ 2025-01-15 08:24 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; Alena Rybakina <[email protected]>; +Cc: Ranier Vilela <[email protected]>; Alexander Lakhin <[email protected]>; jian he <[email protected]>; Robert Haas <[email protected]>; Nikolay Shaplov <[email protected]>; [email protected]; pgsql-hackers; Peter Geoghegan <[email protected]>; Marcos Pegoraro <[email protected]>; [email protected]; Peter Eisentraut <[email protected]>
On 1/13/25 10:39, Andrei Lepikhov wrote:
> On 1/13/25 01:39, Alexander Korotkov wrote:
> It can be resolved with a single-line change (see attached). But I need
> some time to ponder over the changing behaviour when a clause may match
> an index and be in joinorclauses.
In addition, let me raise a couple of issues:
1. As Robert has said before, it may interfere with some short-circuit
optimisations like below:
EXPLAIN (COSTS OFF)
SELECT * FROM bitmap_split_or t1
WHERE t1.a=2 AND (t1.b=2 OR t1.b = (
SELECT sum(c1.reltuples) FROM pg_class c1, pg_class c2
WHERE c1.relpages=c2.relpages AND c1.relpages = t1.a));
Here, a user may avoid evaluating the subplan at all if t1.b=2 all the
time when t1.a=2. OR->ANY may accidentally shift this behaviour.
2. The query:
EXPLAIN (ANALYZE, COSTS OFF)
SELECT * FROM bitmap_split_or t1
WHERE t1.a=2 OR t1.a = (
SELECT sum(c1.reltuples) FROM pg_class c1, pg_class c2
WHERE c1.relpages=c2.relpages AND c1.relpages = t1.a)::integer;
causes SEGFAULT during index keys evaluation. I haven't dived into it
yet, but it seems quite a typical misstep and is not difficult to fix.
--
regards, Andrei Lepikhov
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-01-25 05:04 Alexander Korotkov <[email protected]>
parent: Andrei Lepikhov <[email protected]>
0 siblings, 2 replies; 38+ messages in thread
From: Alexander Korotkov @ 2025-01-25 05:04 UTC (permalink / raw)
To: Andrei Lepikhov <[email protected]>; +Cc: Alena Rybakina <[email protected]>; Ranier Vilela <[email protected]>; Alexander Lakhin <[email protected]>; jian he <[email protected]>; Robert Haas <[email protected]>; Nikolay Shaplov <[email protected]>; [email protected]; pgsql-hackers; Peter Geoghegan <[email protected]>; Marcos Pegoraro <[email protected]>; [email protected]; Peter Eisentraut <[email protected]>
On Wed, Jan 15, 2025 at 10:24 AM Andrei Lepikhov <[email protected]> wrote:
> On 1/13/25 10:39, Andrei Lepikhov wrote:
> > On 1/13/25 01:39, Alexander Korotkov wrote:
> > It can be resolved with a single-line change (see attached). But I need
> > some time to ponder over the changing behaviour when a clause may match
> > an index and be in joinorclauses.
> In addition, let me raise a couple of issues:
> 1. As Robert has said before, it may interfere with some short-circuit
> optimisations like below:
>
> EXPLAIN (COSTS OFF)
> SELECT * FROM bitmap_split_or t1
> WHERE t1.a=2 AND (t1.b=2 OR t1.b = (
> SELECT sum(c1.reltuples) FROM pg_class c1, pg_class c2
> WHERE c1.relpages=c2.relpages AND c1.relpages = t1.a));
>
> Here, a user may avoid evaluating the subplan at all if t1.b=2 all the
> time when t1.a=2. OR->ANY may accidentally shift this behaviour.
>
> 2. The query:
>
> EXPLAIN (ANALYZE, COSTS OFF)
> SELECT * FROM bitmap_split_or t1
> WHERE t1.a=2 OR t1.a = (
> SELECT sum(c1.reltuples) FROM pg_class c1, pg_class c2
> WHERE c1.relpages=c2.relpages AND c1.relpages = t1.a)::integer;
>
> causes SEGFAULT during index keys evaluation. I haven't dived into it
> yet, but it seems quite a typical misstep and is not difficult to fix.
Segfault appears to be caused by a typo. Patch used parent rinfo
instead of child rinfo. Fixed in the attached patch.
It appears that your first query also changed a plan after fixing
this. Could you, please, provide another example of a regression for
short-circuit optimization, which is related to this patch?
Also, I've integrated your fix from [1].
Links.
1. https://www.postgresql.org/message-id/41ba3d47-2a48-476c-88d4-6ebd889a7af2%40gmail.com
------
Regards,
Alexander Korotkov
Supabase
Attachments:
[application/octet-stream] v46-0001-Allow-usage-of-match_orclause_to_indexcol-for-jo.patch (12.9K, ../../CAPpHfdvehQbCpQG9zYr739y2nXQkE=FYsat5VE-pDoB4RDPfBw@mail.gmail.com/2-v46-0001-Allow-usage-of-match_orclause_to_indexcol-for-jo.patch)
download | inline diff:
From 2338465f4e88fbeb0c803bbe5d9318e85d26e31e Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Wed, 9 Oct 2024 18:44:25 +0300
Subject: [PATCH v46] Allow usage of match_orclause_to_indexcol() for joins
This commit allows transformation of OR-clauses into SAOP's for index scans
within nested loop joins. That required the following changes.
1. Make match_orclause_to_indexcol() and group_similar_or_args() understand
const-ness in the same way as match_opclause_to_indexcol(). This
generally makes our approach more uniform.
2. Make match_join_clauses_to_index() pass OR-clauses to
match_clause_to_index().
---
src/backend/optimizer/path/indxpath.c | 63 +++++++++++---------
src/test/regress/expected/create_index.out | 25 ++++++++
src/test/regress/expected/join.out | 9 +--
src/test/regress/expected/partition_join.out | 12 ++--
src/test/regress/sql/create_index.sql | 8 +++
5 files changed, 77 insertions(+), 40 deletions(-)
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index fa3edf60f3c..f6a0618b0a5 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -1255,6 +1255,7 @@ group_similar_or_args(PlannerInfo *root, RelOptInfo *rel, RestrictInfo *rinfo)
ListCell *lc2;
List *orargs;
List *result = NIL;
+ Index relid = rel->relid;
Assert(IsA(rinfo->orclause, BoolExpr));
orargs = ((BoolExpr *) rinfo->orclause)->args;
@@ -1319,10 +1320,13 @@ group_similar_or_args(PlannerInfo *root, RelOptInfo *rel, RestrictInfo *rinfo)
/*
* Check for clauses of the form: (indexkey operator constant) or
* (constant operator indexkey). But we don't know a particular index
- * yet. First check for a constant, which must be Const or Param.
- * That's cheaper than search for an index key among all indexes.
+ * yet. Therefore, we try to distinguish the potential index key and
+ * constant first, then search for a matching index key among all
+ * indexes.
*/
- if (IsA(leftop, Const) || IsA(leftop, Param))
+ if (bms_is_member(relid, argrinfo->right_relids) &&
+ !bms_is_member(relid, argrinfo->left_relids) &&
+ !contain_volatile_functions(leftop))
{
opno = get_commutator(opno);
@@ -1333,7 +1337,9 @@ group_similar_or_args(PlannerInfo *root, RelOptInfo *rel, RestrictInfo *rinfo)
}
nonConstExpr = rightop;
}
- else if (IsA(rightop, Const) || IsA(rightop, Param))
+ else if (bms_is_member(relid, argrinfo->left_relids) &&
+ !bms_is_member(relid, argrinfo->right_relids) &&
+ !contain_volatile_functions(rightop))
{
nonConstExpr = leftop;
}
@@ -2434,9 +2440,9 @@ match_join_clauses_to_index(PlannerInfo *root,
/* Potentially usable, so see if it matches the index or is an OR */
if (restriction_is_or_clause(rinfo))
- *joinorclauses = lappend(*joinorclauses, rinfo);
- else
- match_clause_to_index(root, rinfo, index, clauseset);
+ *joinorclauses = list_append_unique_ptr(*joinorclauses, rinfo);
+
+ match_clause_to_index(root, rinfo, index, clauseset);
}
}
@@ -2585,10 +2591,7 @@ match_clause_to_index(PlannerInfo *root,
* (3) must match the collation of the index, if collation is relevant.
*
* Our definition of "const" is exceedingly liberal: we allow anything that
- * doesn't involve a volatile function or a Var of the index's relation
- * except for a boolean OR expression input: due to a trade-off between the
- * expected execution speedup and planning complexity, we limit or->saop
- * transformation by obvious cases when an index scan can get a profit.
+ * doesn't involve a volatile function or a Var of the index's relation.
* In particular, Vars belonging to other relations of the query are
* accepted here, since a clause of that form can be used in a
* parameterized indexscan. It's the responsibility of higher code levels
@@ -3246,7 +3249,8 @@ match_orclause_to_indexcol(PlannerInfo *root,
Oid arraytype = InvalidOid;
Oid inputcollid = InvalidOid;
bool firstTime = true;
- bool haveParam = false;
+ bool haveNonConst = false;
+ Index indexRelid = index->rel->relid;
Assert(IsA(orclause, BoolExpr));
Assert(orclause->boolop == OR_EXPR);
@@ -3258,10 +3262,9 @@ match_orclause_to_indexcol(PlannerInfo *root,
/*
* Try to convert a list of OR-clauses to a single SAOP expression. Each
* OR entry must be in the form: (indexkey operator constant) or (constant
- * operator indexkey). Operators of all the entries must match. Constant
- * might be either Const or Param. To be effective, give up on the first
- * non-matching entry. Exit is implemented as a break from the loop,
- * which is catched afterwards.
+ * operator indexkey). Operators of all the entries must match. To be
+ * effective, give up on the first non-matching entry. Exit is
+ * implemented as a break from the loop, which is catched afterwards.
*/
foreach(lc, orclause->args)
{
@@ -3312,17 +3315,21 @@ match_orclause_to_indexcol(PlannerInfo *root,
/*
* Check for clauses of the form: (indexkey operator constant) or
- * (constant operator indexkey). Determine indexkey side first, check
- * the constant later.
+ * (constant operator indexkey). See match_clause_to_indexcol's notes
+ * about const-ness.
*/
leftop = (Node *) linitial(subClause->args);
rightop = (Node *) lsecond(subClause->args);
- if (match_index_to_operand(leftop, indexcol, index))
+ if (match_index_to_operand(leftop, indexcol, index) &&
+ !bms_is_member(indexRelid, subRinfo->right_relids) &&
+ !contain_volatile_functions(rightop))
{
indexExpr = leftop;
constExpr = rightop;
}
- else if (match_index_to_operand(rightop, indexcol, index))
+ else if (match_index_to_operand(rightop, indexcol, index) &&
+ !bms_is_member(indexRelid, subRinfo->left_relids) &&
+ !contain_volatile_functions(leftop))
{
opno = get_commutator(opno);
if (!OidIsValid(opno))
@@ -3349,10 +3356,6 @@ match_orclause_to_indexcol(PlannerInfo *root,
if (IsA(indexExpr, RelabelType))
indexExpr = (Node *) ((RelabelType *) indexExpr)->arg;
- /* We allow constant to be Const or Param */
- if (!IsA(constExpr, Const) && !IsA(constExpr, Param))
- break;
-
/* Forbid transformation for composite types, records. */
if (type_is_rowtype(exprType(constExpr)) ||
type_is_rowtype(exprType(indexExpr)))
@@ -3389,8 +3392,12 @@ match_orclause_to_indexcol(PlannerInfo *root,
break;
}
- if (IsA(constExpr, Param))
- haveParam = true;
+ /*
+ * Check if our list of constants in match_clause_to_indexcol's
+ * understanding of const-ness have something other than Const.
+ */
+ if (!IsA(constExpr, Const))
+ haveNonConst = true;
consts = lappend(consts, constExpr);
}
@@ -3407,10 +3414,10 @@ match_orclause_to_indexcol(PlannerInfo *root,
/*
* Assemble an array from the list of constants. It seems more profitable
- * to build a const array. But in the presence of parameters, we don't
+ * to build a const array. But in the presence of other nodes, we don't
* have a specific value here and must employ an ArrayExpr instead.
*/
- if (haveParam)
+ if (haveNonConst)
{
ArrayExpr *arrayExpr = makeNode(ArrayExpr);
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 8011c141bf8..36343a3781d 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2006,6 +2006,21 @@ SELECT * FROM tenk1
Filter: (((tenthous)::numeric = '1'::numeric) OR (tenthous = 3) OR ((tenthous)::numeric = '42'::numeric))
(2 rows)
+EXPLAIN (COSTS OFF)
+SELECT * FROM tenk1 t1
+ WHERE t1.thousand = 42 OR t1.thousand = (SELECT t2.tenthous FROM tenk1 t2 WHERE t2.thousand = t1.tenthous);
+ QUERY PLAN
+----------------------------------------------------------------
+ Seq Scan on tenk1 t1
+ Filter: ((thousand = 42) OR (thousand = (SubPlan 1)))
+ SubPlan 1
+ -> Index Only Scan using tenk1_thous_tenthous on tenk1 t2
+ Index Cond: (thousand = t1.tenthous)
+(5 rows)
+
+SELECT * FROM tenk1 t1
+ WHERE t1.thousand = 42 OR t1.thousand = (SELECT t2.tenthous FROM tenk1 t2 WHERE t2.thousand = t1.tenthous);
+ERROR: more than one row returned by a subquery used as an expression
EXPLAIN (COSTS OFF)
SELECT count(*) FROM tenk1
WHERE hundred = 42 AND (thousand = 42 OR thousand = 99);
@@ -3255,6 +3270,16 @@ CREATE INDEX t_b_c_idx ON bitmap_split_or (b, c);
CREATE STATISTICS t_a_b_stat (mcv) ON a, b FROM bitmap_split_or;
CREATE STATISTICS t_b_c_stat (mcv) ON b, c FROM bitmap_split_or;
ANALYZE bitmap_split_or;
+EXPLAIN (COSTS OFF)
+SELECT * FROM bitmap_split_or t1, bitmap_split_or t2 WHERE t1.a=t2.b OR t1.a=1;
+ QUERY PLAN
+--------------------------------------------------------
+ Nested Loop
+ -> Seq Scan on bitmap_split_or t2
+ -> Index Scan using t_a_b_idx on bitmap_split_or t1
+ Index Cond: (a = ANY (ARRAY[t2.b, 1]))
+(4 rows)
+
EXPLAIN (COSTS OFF)
SELECT * FROM bitmap_split_or WHERE a = 1 AND (b = 1 OR b = 2) AND c = 2;
QUERY PLAN
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 079fcf46f0d..634eff4d751 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -3849,14 +3849,11 @@ where q1 = thousand or q2 = thousand;
-> Seq Scan on q2
-> Bitmap Heap Scan on tenk1
Recheck Cond: ((q1.q1 = thousand) OR (q2.q2 = thousand))
- -> BitmapOr
- -> Bitmap Index Scan on tenk1_thous_tenthous
- Index Cond: (thousand = q1.q1)
- -> Bitmap Index Scan on tenk1_thous_tenthous
- Index Cond: (thousand = q2.q2)
+ -> Bitmap Index Scan on tenk1_thous_tenthous
+ Index Cond: (thousand = ANY (ARRAY[q1.q1, q2.q2]))
-> Hash
-> Seq Scan on int4_tbl
-(15 rows)
+(12 rows)
explain (costs off)
select * from
diff --git a/src/test/regress/expected/partition_join.out b/src/test/regress/expected/partition_join.out
index 108f9ecb445..af468682a2d 100644
--- a/src/test/regress/expected/partition_join.out
+++ b/src/test/regress/expected/partition_join.out
@@ -2533,24 +2533,24 @@ where not exists (select 1 from prtx2
-> Seq Scan on prtx1_1
Filter: ((a < 20) AND (c = 91))
-> Bitmap Heap Scan on prtx2_1
- Recheck Cond: ((b = (prtx1_1.b + 1)) OR (c = 99))
+ Recheck Cond: ((c = 99) OR (b = (prtx1_1.b + 1)))
Filter: (a = prtx1_1.a)
-> BitmapOr
- -> Bitmap Index Scan on prtx2_1_b_idx
- Index Cond: (b = (prtx1_1.b + 1))
-> Bitmap Index Scan on prtx2_1_c_idx
Index Cond: (c = 99)
+ -> Bitmap Index Scan on prtx2_1_b_idx
+ Index Cond: (b = (prtx1_1.b + 1))
-> Nested Loop Anti Join
-> Seq Scan on prtx1_2
Filter: ((a < 20) AND (c = 91))
-> Bitmap Heap Scan on prtx2_2
- Recheck Cond: ((b = (prtx1_2.b + 1)) OR (c = 99))
+ Recheck Cond: ((c = 99) OR (b = (prtx1_2.b + 1)))
Filter: (a = prtx1_2.a)
-> BitmapOr
- -> Bitmap Index Scan on prtx2_2_b_idx
- Index Cond: (b = (prtx1_2.b + 1))
-> Bitmap Index Scan on prtx2_2_c_idx
Index Cond: (c = 99)
+ -> Bitmap Index Scan on prtx2_2_b_idx
+ Index Cond: (b = (prtx1_2.b + 1))
(23 rows)
select * from prtx1
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index 068c66b95a5..aded693bf7b 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -781,6 +781,12 @@ EXPLAIN (COSTS OFF)
SELECT * FROM tenk1
WHERE tenthous = 1::numeric OR tenthous = 3::int4 OR tenthous = 42::numeric;
+EXPLAIN (COSTS OFF)
+SELECT * FROM tenk1 t1
+ WHERE t1.thousand = 42 OR t1.thousand = (SELECT t2.tenthous FROM tenk1 t2 WHERE t2.thousand = t1.tenthous);
+SELECT * FROM tenk1 t1
+ WHERE t1.thousand = 42 OR t1.thousand = (SELECT t2.tenthous FROM tenk1 t2 WHERE t2.thousand = t1.tenthous);
+
EXPLAIN (COSTS OFF)
SELECT count(*) FROM tenk1
WHERE hundred = 42 AND (thousand = 42 OR thousand = 99);
@@ -1367,6 +1373,8 @@ CREATE STATISTICS t_a_b_stat (mcv) ON a, b FROM bitmap_split_or;
CREATE STATISTICS t_b_c_stat (mcv) ON b, c FROM bitmap_split_or;
ANALYZE bitmap_split_or;
EXPLAIN (COSTS OFF)
+SELECT * FROM bitmap_split_or t1, bitmap_split_or t2 WHERE t1.a=t2.b OR t1.a=1;
+EXPLAIN (COSTS OFF)
SELECT * FROM bitmap_split_or WHERE a = 1 AND (b = 1 OR b = 2) AND c = 2;
DROP TABLE bitmap_split_or;
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-01-27 08:52 Andrei Lepikhov <[email protected]>
parent: Alexander Korotkov <[email protected]>
1 sibling, 1 reply; 38+ messages in thread
From: Andrei Lepikhov @ 2025-01-27 08:52 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; +Cc: Alena Rybakina <[email protected]>; Ranier Vilela <[email protected]>; Alexander Lakhin <[email protected]>; jian he <[email protected]>; Robert Haas <[email protected]>; Nikolay Shaplov <[email protected]>; [email protected]; pgsql-hackers; Peter Geoghegan <[email protected]>; Marcos Pegoraro <[email protected]>; [email protected]; Peter Eisentraut <[email protected]>
On 1/25/25 12:04, Alexander Korotkov wrote:
> On Wed, Jan 15, 2025 at 10:24 AM Andrei Lepikhov <[email protected]> wrote:
>> causes SEGFAULT during index keys evaluation. I haven't dived into it
>> yet, but it seems quite a typical misstep and is not difficult to fix.
>
> Segfault appears to be caused by a typo. Patch used parent rinfo
> instead of child rinfo. Fixed in the attached patch.
Great!
>
> It appears that your first query also changed a plan after fixing
> this. Could you, please, provide another example of a regression for
> short-circuit optimization, which is related to this patch?
Yes, it may be caused by the current lazy InitPlan evaluation strategy,
which would only happen if it was really needed.
Examples:
---------
EXPLAIN (ANALYZE, COSTS OFF, BUFFERS OFF, TIMING OFF)
SELECT * FROM bitmap_split_or t1
WHERE t1.a=2 AND (t1.b=2 OR t1.b = (
SELECT avg(x) FROM generate_series(1,1e6) AS x)::integer);
without optimisation:
Index Scan using t_a_b_idx on bitmap_split_or t1 (actual rows=1 loops=1)
Index Cond: (a = 2)
Filter: ((b = 2) OR (b = ((InitPlan 1).col1)::integer))
InitPlan 1
-> Aggregate (never executed)
-> Function Scan on generate_series x (never executed)
Planning Time: 0.564 ms
Execution Time: 0.182 ms
But having it as a part of an array, we forcedly evaluate it for (not
100% sure) more precise selectivity estimation:
Index Scan using t_a_b_idx on bitmap_split_or t1
(actual rows=1 loops=1)
Index Cond: ((a = 2) AND
(b = ANY (ARRAY[2, ((InitPlan 1).col1)::integer])))
InitPlan 1
-> Aggregate (actual rows=1 loops=1)
-> Function Scan on generate_series x
(actual rows=1000000 loops=1)
Planning Time: 0.927 ms
Execution Time: 489.933 ms
This also means that if, before the patch, we executed a query
successfully, after applying the patch, we sometimes may get the error:
'ERROR: more than one row returned by a subquery used as an expression'
because of early InitPlan evaluation. See the example below:
EXPLAIN (ANALYZE, COSTS OFF)
SELECT * FROM bitmap_split_or t1
WHERE t1.a=2 AND (t1.b=2 OR t1.b = (
SELECT random() FROM generate_series(1,1e6) AS x)::integer);
Index Scan using t_a_b_idx on bitmap_split_or t1
Index Cond: ((a = 2) AND (b = ANY (ARRAY[2, ((InitPlan
1).col1)::integer])))
InitPlan 1
-> Function Scan on generate_series x
I think optimisation should have never happened and this is another
issue, isn't it?
--
regards, Andrei Lepikhov
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-01-27 09:50 Alexander Korotkov <[email protected]>
parent: Andrei Lepikhov <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: Alexander Korotkov @ 2025-01-27 09:50 UTC (permalink / raw)
To: Andrei Lepikhov <[email protected]>; +Cc: Alena Rybakina <[email protected]>; Ranier Vilela <[email protected]>; Alexander Lakhin <[email protected]>; jian he <[email protected]>; Robert Haas <[email protected]>; Nikolay Shaplov <[email protected]>; [email protected]; pgsql-hackers; Peter Geoghegan <[email protected]>; Marcos Pegoraro <[email protected]>; [email protected]; Peter Eisentraut <[email protected]>
Hi, Andrei!
On Mon, Jan 27, 2025 at 10:52 AM Andrei Lepikhov <[email protected]> wrote:
> On 1/25/25 12:04, Alexander Korotkov wrote:
> > On Wed, Jan 15, 2025 at 10:24 AM Andrei Lepikhov <[email protected]>
wrote:
> >> causes SEGFAULT during index keys evaluation. I haven't dived into it
> >> yet, but it seems quite a typical misstep and is not difficult to fix.
> >
> > Segfault appears to be caused by a typo. Patch used parent rinfo
> > instead of child rinfo. Fixed in the attached patch.
> Great!
> >
> > It appears that your first query also changed a plan after fixing
> > this. Could you, please, provide another example of a regression for
> > short-circuit optimization, which is related to this patch?
> Yes, it may be caused by the current lazy InitPlan evaluation strategy,
> which would only happen if it was really needed.
>
> Examples:
> ---------
>
> EXPLAIN (ANALYZE, COSTS OFF, BUFFERS OFF, TIMING OFF)
> SELECT * FROM bitmap_split_or t1
> WHERE t1.a=2 AND (t1.b=2 OR t1.b = (
> SELECT avg(x) FROM generate_series(1,1e6) AS x)::integer);
>
> without optimisation:
>
> Index Scan using t_a_b_idx on bitmap_split_or t1 (actual rows=1 loops=1)
> Index Cond: (a = 2)
> Filter: ((b = 2) OR (b = ((InitPlan 1).col1)::integer))
> InitPlan 1
> -> Aggregate (never executed)
> -> Function Scan on generate_series x (never executed)
> Planning Time: 0.564 ms
> Execution Time: 0.182 ms
>
> But having it as a part of an array, we forcedly evaluate it for (not
> 100% sure) more precise selectivity estimation:
>
> Index Scan using t_a_b_idx on bitmap_split_or t1
> (actual rows=1 loops=1)
> Index Cond: ((a = 2) AND
> (b = ANY (ARRAY[2, ((InitPlan 1).col1)::integer])))
> InitPlan 1
> -> Aggregate (actual rows=1 loops=1)
> -> Function Scan on generate_series x
> (actual rows=1000000 loops=1)
> Planning Time: 0.927 ms
> Execution Time: 489.933 ms
>
> This also means that if, before the patch, we executed a query
> successfully, after applying the patch, we sometimes may get the error:
> 'ERROR: more than one row returned by a subquery used as an expression'
> because of early InitPlan evaluation. See the example below:
>
> EXPLAIN (ANALYZE, COSTS OFF)
> SELECT * FROM bitmap_split_or t1
> WHERE t1.a=2 AND (t1.b=2 OR t1.b = (
> SELECT random() FROM generate_series(1,1e6) AS x)::integer);
>
> Index Scan using t_a_b_idx on bitmap_split_or t1
> Index Cond: ((a = 2) AND (b = ANY (ARRAY[2, ((InitPlan
> 1).col1)::integer])))
> InitPlan 1
> -> Function Scan on generate_series x
>
> I think optimisation should have never happened and this is another
> issue, isn't it?
Thank you for your examples. The reason why these example works only with
the patch is that you apply the cast outside of subquery. This is
because d4378c0005 requires OR argument to be either Cost or Param, but not
a cast over the param. Consider this example on master.
# EXPLAIN (ANALYZE, COSTS OFF, BUFFERS OFF, TIMING OFF)
SELECT * FROM bitmap_split_or t1
WHERE t1.a=2 AND (t1.b=2 OR t1.b = (
SELECT avg(x)::integer FROM generate_series(1,1e6) AS x));
QUERY PLAN
--------------------------------------------------------------------------------
Index Scan using t_a_b_idx on bitmap_split_or t1 (actual rows=1 loops=1)
Index Cond: ((a = 2) AND (b = ANY (ARRAY[2, (InitPlan 1).col1])))
InitPlan 1
-> Aggregate (actual rows=1 loops=1)
-> Function Scan on generate_series x (actual rows=1000000
loops=1)
Planning Time: 0.731 ms
Execution Time: 577.953 ms
(7 rows)
I expressed my point on this in [1]. We generally greedy about index quals
and there is no logic which prevent us from using a clause and index qual
because of its cost. And there are many cases when this causes regressions
before d4378c0005. One of examples from [1].
# explain analyze select * from t where i = 0 and j = (select slowfunc());
QUERY PLAN
---------------------------------------------------------------------------------------------------
Seq Scan on t (cost=25000.01..25195.01 rows=1 width=8) (actual
time=0.806..0.807 rows=0 loops=1)
Filter: ((i = 0) AND (j = (InitPlan 1).col1))
Rows Removed by Filter: 10000
InitPlan 1
-> Result (cost=0.00..25000.01 rows=1 width=4) (never executed)
Planning Time: 0.165 ms
Execution Time: 0.843 ms
(7 rows)
Links.
1.
https://www.postgresql.org/message-id/CAPpHfdt8kowRDUkmOnO7_WJJQ1uk%2BO379JiZCk_9_Pt5AQ4%2B0w%40mail...
------
Regards,
Alexander Korotkov
Supabase
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-01-28 04:36 Andrei Lepikhov <[email protected]>
parent: Alexander Korotkov <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: Andrei Lepikhov @ 2025-01-28 04:36 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; +Cc: Alena Rybakina <[email protected]>; Ranier Vilela <[email protected]>; Alexander Lakhin <[email protected]>; jian he <[email protected]>; Robert Haas <[email protected]>; Nikolay Shaplov <[email protected]>; [email protected]; pgsql-hackers; Peter Geoghegan <[email protected]>; Marcos Pegoraro <[email protected]>; [email protected]; Peter Eisentraut <[email protected]>
On 1/27/25 16:50, Alexander Korotkov wrote:
> I expressed my point on this in [1]. We generally greedy about index
> quals and there is no logic which prevent us from using a clause and
> index qual because of its cost. And there are many cases when this
> causes regressions before d4378c0005. One of examples from [1].
Ok,
Generally, I don't concern myself with the evaluation of individual
subplans. As you mentioned, it should be a rare occurrence when this
becomes important. My main concern is the shift in frequency of
evaluations during execution for various reasons.
For example:
qsort(matches, n, sizeof(OrArgIndexMatch), or_arg_index_match_cmp);
To fit an index, the order of elements in the target array of the
`ScalarArrayOpExpr` may change compared to the initial list of OR
expressions. If there are indexes that cover the same set of columns but
in reverse order, this could potentially alter the position of a
Subplan. However, I believe this is a rare case; it is supported by the
initial OR path and should be acceptable.
So, I do not have any further objections at this time.
--
regards, Andrei Lepikhov
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-01-28 08:42 Andrei Lepikhov <[email protected]>
parent: Andrei Lepikhov <[email protected]>
0 siblings, 2 replies; 38+ messages in thread
From: Andrei Lepikhov @ 2025-01-28 08:42 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; +Cc: Alena Rybakina <[email protected]>; Ranier Vilela <[email protected]>; Alexander Lakhin <[email protected]>; jian he <[email protected]>; Robert Haas <[email protected]>; Nikolay Shaplov <[email protected]>; [email protected]; pgsql-hackers; Peter Geoghegan <[email protected]>; Marcos Pegoraro <[email protected]>; [email protected]; Peter Eisentraut <[email protected]>
On 1/28/25 11:36, Andrei Lepikhov wrote:
> On 1/27/25 16:50, Alexander Korotkov wrote:
> qsort(matches, n, sizeof(OrArgIndexMatch), or_arg_index_match_cmp);
>
> To fit an index, the order of elements in the target array of the
> `ScalarArrayOpExpr` may change compared to the initial list of OR
> expressions. If there are indexes that cover the same set of columns but
> in reverse order, this could potentially alter the position of a
> Subplan. However, I believe this is a rare case; it is supported by the
> initial OR path and should be acceptable.
I beg your pardon - I forgot that we've restricted the feature's scope
and can't combine OR clauses into ScalarArrayOpExpr if the args list
contains references to different columns.
So, my note can't be applied here.
--
regards, Andrei Lepikhov
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-01-30 13:22 Pavel Borisov <[email protected]>
parent: Andrei Lepikhov <[email protected]>
1 sibling, 1 reply; 38+ messages in thread
From: Pavel Borisov @ 2025-01-30 13:22 UTC (permalink / raw)
To: Andrei Lepikhov <[email protected]>; +Cc: Alexander Korotkov <[email protected]>; Alena Rybakina <[email protected]>; Ranier Vilela <[email protected]>; Alexander Lakhin <[email protected]>; jian he <[email protected]>; Robert Haas <[email protected]>; Nikolay Shaplov <[email protected]>; [email protected]; pgsql-hackers; Peter Geoghegan <[email protected]>; Marcos Pegoraro <[email protected]>; [email protected]; Peter Eisentraut <[email protected]>
On Tue, 28 Jan 2025 at 12:42, Andrei Lepikhov <[email protected]> wrote:
>
> On 1/28/25 11:36, Andrei Lepikhov wrote:
> > On 1/27/25 16:50, Alexander Korotkov wrote:
> > qsort(matches, n, sizeof(OrArgIndexMatch), or_arg_index_match_cmp);
> >
> > To fit an index, the order of elements in the target array of the
> > `ScalarArrayOpExpr` may change compared to the initial list of OR
> > expressions. If there are indexes that cover the same set of columns but
> > in reverse order, this could potentially alter the position of a
> > Subplan. However, I believe this is a rare case; it is supported by the
> > initial OR path and should be acceptable.
> I beg your pardon - I forgot that we've restricted the feature's scope
> and can't combine OR clauses into ScalarArrayOpExpr if the args list
> contains references to different columns.
> So, my note can't be applied here.
>
> --
> regards, Andrei Lepikhov
I've looked at the patch v46-0001
Looks good to me.
There is a test that demonstrates the behavior change. Maybe some more
cases like are also worth adding to a test.
+SELECT * FROM bitmap_split_or t1, bitmap_split_or t2 WHERE t1.a=t2.c
OR (t1.a=t2.b OR t1.a=1);
+ QUERY PLAN
+--------------------------------------------------------
+ Nested Loop
+ -> Seq Scan on bitmap_split_or t2
+ -> Index Scan using t_a_b_idx on bitmap_split_or t1
+ Index Cond: (a = ANY (ARRAY[t2.c, t2.b, 1]))
+(4 rows)
+
+EXPLAIN (COSTS OFF)
+SELECT * FROM bitmap_split_or t1, bitmap_split_or t2 WHERE t1.c=t2.b OR t1.a=1;
+ QUERY PLAN
+----------------------------------------------
+ Nested Loop
+ Join Filter: ((t1.c = t2.b) OR (t1.a = 1))
+ -> Seq Scan on bitmap_split_or t1
+ -> Materialize
+ -> Seq Scan on bitmap_split_or t2
+(5 rows)
+
+EXPLAIN (COSTS OFF)
Comment
> * Also, add any potentially usable join OR clauses to *joinorclauses
may reflect the change in v46-0001 lappend -> list_append_unique_ptr
that differs in the processing of equal clauses in the list.
Semantics mentioned in the commit message:
> 2. Make match_join_clauses_to_index() pass OR-clauses to
> match_clause_to_index().
could also be added as comments in the section just before
match_join_clauses_to_index()
Since d4378c0005e6 comment for match_clause_to_indexcol() I think
needs change. This could be as a separate commit, not regarding
current patch v46-0001.
> * NOTE: returns NULL if clause is an OR or AND clause; it is the
> * responsibility of higher-level routines to co
I think the patch can be pushed with possible additions to regression
test and comments.
Regards,
Pavel Borisov
Supabase
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-01-31 14:31 Alena Rybakina <[email protected]>
parent: Alexander Korotkov <[email protected]>
1 sibling, 1 reply; 38+ messages in thread
From: Alena Rybakina @ 2025-01-31 14:31 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; +Cc: Ranier Vilela <[email protected]>; Alexander Lakhin <[email protected]>; jian he <[email protected]>; Robert Haas <[email protected]>; Nikolay Shaplov <[email protected]>; [email protected]; pgsql-hackers; Peter Geoghegan <[email protected]>; Marcos Pegoraro <[email protected]>; [email protected]; Peter Eisentraut <[email protected]>; Andrei Lepikhov <[email protected]>
Hi!
On 25.01.2025 08:04, Alexander Korotkov wrote:
> On Wed, Jan 15, 2025 at 10:24 AM Andrei Lepikhov<[email protected]> wrote:
>> On 1/13/25 10:39, Andrei Lepikhov wrote:
>>> On 1/13/25 01:39, Alexander Korotkov wrote:
>>> It can be resolved with a single-line change (see attached). But I need
>>> some time to ponder over the changing behaviour when a clause may match
>>> an index and be in joinorclauses.
>> In addition, let me raise a couple of issues:
>> 1. As Robert has said before, it may interfere with some short-circuit
>> optimisations like below:
>>
>> EXPLAIN (COSTS OFF)
>> SELECT * FROM bitmap_split_or t1
>> WHERE t1.a=2 AND (t1.b=2 OR t1.b = (
>> SELECT sum(c1.reltuples) FROM pg_class c1, pg_class c2
>> WHERE c1.relpages=c2.relpages AND c1.relpages = t1.a));
>>
>> Here, a user may avoid evaluating the subplan at all if t1.b=2 all the
>> time when t1.a=2. OR->ANY may accidentally shift this behaviour.
>>
>> 2. The query:
>>
>> EXPLAIN (ANALYZE, COSTS OFF)
>> SELECT * FROM bitmap_split_or t1
>> WHERE t1.a=2 OR t1.a = (
>> SELECT sum(c1.reltuples) FROM pg_class c1, pg_class c2
>> WHERE c1.relpages=c2.relpages AND c1.relpages = t1.a)::integer;
>>
>> causes SEGFAULT during index keys evaluation. I haven't dived into it
>> yet, but it seems quite a typical misstep and is not difficult to fix.
> Segfault appears to be caused by a typo. Patch used parent rinfo
> instead of child rinfo. Fixed in the attached patch.
>
> It appears that your first query also changed a plan after fixing
> this. Could you, please, provide another example of a regression for
> short-circuit optimization, which is related to this patch?
>
> Also, I've integrated your fix from [1].
>
> Links.
> 1.https://www.postgresql.org/message-id/41ba3d47-2a48-476c-88d4-6ebd889a7af2%40gmail.com
I started reviewing at the patch and saw some output "ERROR" in the
output of the test and is it okay here?
SELECT * FROM tenk1 t1
WHERE t1.thousand= 42OR t1.thousand= (SELECT t2.tenthousFROM tenk1 t2
WHERE t2.thousand= t1.tenthous);
ERROR: more than one row returned by a subquery used as an expression
--
Regards,
Alena Rybakina
Postgres Professional
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-02-02 17:57 Alexander Korotkov <[email protected]>
parent: Pavel Borisov <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: Alexander Korotkov @ 2025-02-02 17:57 UTC (permalink / raw)
To: Pavel Borisov <[email protected]>; +Cc: Andrei Lepikhov <[email protected]>; Alena Rybakina <[email protected]>; Ranier Vilela <[email protected]>; Alexander Lakhin <[email protected]>; jian he <[email protected]>; Robert Haas <[email protected]>; Nikolay Shaplov <[email protected]>; [email protected]; pgsql-hackers; Peter Geoghegan <[email protected]>; Marcos Pegoraro <[email protected]>; [email protected]; Peter Eisentraut <[email protected]>
On Thu, Jan 30, 2025 at 3:23 PM Pavel Borisov <[email protected]> wrote:
> On Tue, 28 Jan 2025 at 12:42, Andrei Lepikhov <[email protected]> wrote:
> >
> > On 1/28/25 11:36, Andrei Lepikhov wrote:
> > > On 1/27/25 16:50, Alexander Korotkov wrote:
> > > qsort(matches, n, sizeof(OrArgIndexMatch), or_arg_index_match_cmp);
> > >
> > > To fit an index, the order of elements in the target array of the
> > > `ScalarArrayOpExpr` may change compared to the initial list of OR
> > > expressions. If there are indexes that cover the same set of columns but
> > > in reverse order, this could potentially alter the position of a
> > > Subplan. However, I believe this is a rare case; it is supported by the
> > > initial OR path and should be acceptable.
> > I beg your pardon - I forgot that we've restricted the feature's scope
> > and can't combine OR clauses into ScalarArrayOpExpr if the args list
> > contains references to different columns.
> > So, my note can't be applied here.
> >
> > --
> > regards, Andrei Lepikhov
>
> I've looked at the patch v46-0001
> Looks good to me.
>
> There is a test that demonstrates the behavior change. Maybe some more
> cases like are also worth adding to a test.
>
> +SELECT * FROM bitmap_split_or t1, bitmap_split_or t2 WHERE t1.a=t2.c
> OR (t1.a=t2.b OR t1.a=1);
> + QUERY PLAN
> +--------------------------------------------------------
> + Nested Loop
> + -> Seq Scan on bitmap_split_or t2
> + -> Index Scan using t_a_b_idx on bitmap_split_or t1
> + Index Cond: (a = ANY (ARRAY[t2.c, t2.b, 1]))
> +(4 rows)
> +
> +EXPLAIN (COSTS OFF)
> +SELECT * FROM bitmap_split_or t1, bitmap_split_or t2 WHERE t1.c=t2.b OR t1.a=1;
> + QUERY PLAN
> +----------------------------------------------
> + Nested Loop
> + Join Filter: ((t1.c = t2.b) OR (t1.a = 1))
> + -> Seq Scan on bitmap_split_or t1
> + -> Materialize
> + -> Seq Scan on bitmap_split_or t2
> +(5 rows)
> +
> +EXPLAIN (COSTS OFF)
Added more tests to join.sql
> Comment
> > * Also, add any potentially usable join OR clauses to *joinorclauses
> may reflect the change in v46-0001 lappend -> list_append_unique_ptr
> that differs in the processing of equal clauses in the list.
Comments in this function are revised. I also added detailed
explanation of this change to the commit message.
> Semantics mentioned in the commit message:
> > 2. Make match_join_clauses_to_index() pass OR-clauses to
> > match_clause_to_index().
> could also be added as comments in the section just before
> match_join_clauses_to_index()
Right, this is addressed too.
> Since d4378c0005e6 comment for match_clause_to_indexcol() I think
> needs change. This could be as a separate commit, not regarding
> current patch v46-0001.
> > * NOTE: returns NULL if clause is an OR or AND clause; it is the
> > * responsibility of higher-level routines to co
Good catch. This is added as a separate patch.
> I think the patch can be pushed with possible additions to regression
> test and comments.
OK, thank you!
------
Regards,
Alexander Korotkov
Supabase
Attachments:
[application/octet-stream] v47-0001-Revise-the-header-comment-for-match_clause_to_in.patch (1.3K, ../../CAPpHfdsBZmNt9qUoJBqsQFiVDX1=yCKpuVAt1YnR7JCpP=k8+A@mail.gmail.com/2-v47-0001-Revise-the-header-comment-for-match_clause_to_in.patch)
download | inline diff:
From 0fe7f1eb6447b86f8c9e8ec472333e13ed36a255 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Sat, 1 Feb 2025 16:53:32 +0200
Subject: [PATCH v47 1/2] Revise the header comment for
match_clause_to_indexcol()
Since d4378c0005e6, match_clause_to_indexcol() doesn't always return NULL
for an OR clause. This commit reflects that in the header comment.
Reported-by: Pavel Borisov <[email protected]>
---
src/backend/optimizer/path/indxpath.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index fa3edf60f3c..a58cf5bad1a 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -2639,8 +2639,9 @@ match_clause_to_index(PlannerInfo *root,
* Returns an IndexClause if the clause can be used with this index key,
* or NULL if not.
*
- * NOTE: returns NULL if clause is an OR or AND clause; it is the
- * responsibility of higher-level routines to cope with those.
+ * NOTE: This routine always returns NULL if the clause is an AND clause.
+ * Higher-level routines deal with OR and AND clauses. OR clause can be
+ * matched as a whole by match_orclause_to_indexcol() though.
*/
static IndexClause *
match_clause_to_indexcol(PlannerInfo *root,
--
2.39.5 (Apple Git-154)
[application/octet-stream] v47-0002-Allow-usage-of-match_orclause_to_indexcol-for-jo.patch (17.3K, ../../CAPpHfdsBZmNt9qUoJBqsQFiVDX1=yCKpuVAt1YnR7JCpP=k8+A@mail.gmail.com/3-v47-0002-Allow-usage-of-match_orclause_to_indexcol-for-jo.patch)
download | inline diff:
From 3522cfd0a792e77b3740ea7541826bd0fbc6e752 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Wed, 9 Oct 2024 18:44:25 +0300
Subject: [PATCH v47 2/2] Allow usage of match_orclause_to_indexcol() for joins
This commit allows transformation of OR-clauses into SAOP's for index scans
within nested loop joins. That required the following changes.
1. Make match_orclause_to_indexcol() and group_similar_or_args() understand
const-ness in the same way as match_opclause_to_indexcol(). This
generally makes our approach more uniform.
2. Make match_join_clauses_to_index() pass OR-clauses to
match_clause_to_index().
3. Also switch match_join_clauses_to_index() to use list_append_unique_ptr()
for adding clauses to *joinorclauses. That avoids possible duplicates
when processing the same clauses with different indexes. Previously such
duplicates were elimited in match_clause_to_index(), but now
group_similar_or_args() each time generates distinct copies of grouped
OR clauses.
Discussion: https://postgr.es/m/CAPpHfdv%2BjtNwofg-p5z86jLYZUTt6tR17Wy00ta0dL%3DwHQN3ZA%40mail.gmail.com
Reviewed-by: Andrei Lepikhov <[email protected]>
Reviewed-by: Alena Rybakina <[email protected]>
Reviewed-by: Pavel Borisov <[email protected]>
---
src/backend/optimizer/path/indxpath.c | 70 ++++++++++++--------
src/test/regress/expected/create_index.out | 39 +++++++++++
src/test/regress/expected/join.out | 51 ++++++++++++--
src/test/regress/expected/partition_join.out | 12 ++--
src/test/regress/sql/create_index.sql | 8 +++
src/test/regress/sql/join.sql | 18 ++++-
6 files changed, 156 insertions(+), 42 deletions(-)
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index a58cf5bad1a..6e2051efc65 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -1255,6 +1255,7 @@ group_similar_or_args(PlannerInfo *root, RelOptInfo *rel, RestrictInfo *rinfo)
ListCell *lc2;
List *orargs;
List *result = NIL;
+ Index relid = rel->relid;
Assert(IsA(rinfo->orclause, BoolExpr));
orargs = ((BoolExpr *) rinfo->orclause)->args;
@@ -1319,10 +1320,13 @@ group_similar_or_args(PlannerInfo *root, RelOptInfo *rel, RestrictInfo *rinfo)
/*
* Check for clauses of the form: (indexkey operator constant) or
* (constant operator indexkey). But we don't know a particular index
- * yet. First check for a constant, which must be Const or Param.
- * That's cheaper than search for an index key among all indexes.
+ * yet. Therefore, we try to distinguish the potential index key and
+ * constant first, then search for a matching index key among all
+ * indexes.
*/
- if (IsA(leftop, Const) || IsA(leftop, Param))
+ if (bms_is_member(relid, argrinfo->right_relids) &&
+ !bms_is_member(relid, argrinfo->left_relids) &&
+ !contain_volatile_functions(leftop))
{
opno = get_commutator(opno);
@@ -1333,7 +1337,9 @@ group_similar_or_args(PlannerInfo *root, RelOptInfo *rel, RestrictInfo *rinfo)
}
nonConstExpr = rightop;
}
- else if (IsA(rightop, Const) || IsA(rightop, Param))
+ else if (bms_is_member(relid, argrinfo->left_relids) &&
+ !bms_is_member(relid, argrinfo->right_relids) &&
+ !contain_volatile_functions(rightop))
{
nonConstExpr = leftop;
}
@@ -2414,6 +2420,7 @@ match_restriction_clauses_to_index(PlannerInfo *root,
* Identify join clauses for the rel that match the index.
* Matching clauses are added to *clauseset.
* Also, add any potentially usable join OR clauses to *joinorclauses.
+ * They also might be processed by match_clause_to_index() as a whole.
*/
static void
match_join_clauses_to_index(PlannerInfo *root,
@@ -2432,11 +2439,15 @@ match_join_clauses_to_index(PlannerInfo *root,
if (!join_clause_is_movable_to(rinfo, rel))
continue;
- /* Potentially usable, so see if it matches the index or is an OR */
+ /*
+ * Potentially usable, so see if it matches the index or is an OR. Use
+ * list_append_unique_ptr() here to avoid possible duplicates when
+ * processing the same clauses with different indexes.
+ */
if (restriction_is_or_clause(rinfo))
- *joinorclauses = lappend(*joinorclauses, rinfo);
- else
- match_clause_to_index(root, rinfo, index, clauseset);
+ *joinorclauses = list_append_unique_ptr(*joinorclauses, rinfo);
+
+ match_clause_to_index(root, rinfo, index, clauseset);
}
}
@@ -2585,10 +2596,7 @@ match_clause_to_index(PlannerInfo *root,
* (3) must match the collation of the index, if collation is relevant.
*
* Our definition of "const" is exceedingly liberal: we allow anything that
- * doesn't involve a volatile function or a Var of the index's relation
- * except for a boolean OR expression input: due to a trade-off between the
- * expected execution speedup and planning complexity, we limit or->saop
- * transformation by obvious cases when an index scan can get a profit.
+ * doesn't involve a volatile function or a Var of the index's relation.
* In particular, Vars belonging to other relations of the query are
* accepted here, since a clause of that form can be used in a
* parameterized indexscan. It's the responsibility of higher code levels
@@ -3247,7 +3255,8 @@ match_orclause_to_indexcol(PlannerInfo *root,
Oid arraytype = InvalidOid;
Oid inputcollid = InvalidOid;
bool firstTime = true;
- bool haveParam = false;
+ bool haveNonConst = false;
+ Index indexRelid = index->rel->relid;
Assert(IsA(orclause, BoolExpr));
Assert(orclause->boolop == OR_EXPR);
@@ -3259,10 +3268,9 @@ match_orclause_to_indexcol(PlannerInfo *root,
/*
* Try to convert a list of OR-clauses to a single SAOP expression. Each
* OR entry must be in the form: (indexkey operator constant) or (constant
- * operator indexkey). Operators of all the entries must match. Constant
- * might be either Const or Param. To be effective, give up on the first
- * non-matching entry. Exit is implemented as a break from the loop,
- * which is catched afterwards.
+ * operator indexkey). Operators of all the entries must match. To be
+ * effective, give up on the first non-matching entry. Exit is
+ * implemented as a break from the loop, which is catched afterwards.
*/
foreach(lc, orclause->args)
{
@@ -3313,17 +3321,21 @@ match_orclause_to_indexcol(PlannerInfo *root,
/*
* Check for clauses of the form: (indexkey operator constant) or
- * (constant operator indexkey). Determine indexkey side first, check
- * the constant later.
+ * (constant operator indexkey). See match_clause_to_indexcol's notes
+ * about const-ness.
*/
leftop = (Node *) linitial(subClause->args);
rightop = (Node *) lsecond(subClause->args);
- if (match_index_to_operand(leftop, indexcol, index))
+ if (match_index_to_operand(leftop, indexcol, index) &&
+ !bms_is_member(indexRelid, subRinfo->right_relids) &&
+ !contain_volatile_functions(rightop))
{
indexExpr = leftop;
constExpr = rightop;
}
- else if (match_index_to_operand(rightop, indexcol, index))
+ else if (match_index_to_operand(rightop, indexcol, index) &&
+ !bms_is_member(indexRelid, subRinfo->left_relids) &&
+ !contain_volatile_functions(leftop))
{
opno = get_commutator(opno);
if (!OidIsValid(opno))
@@ -3350,10 +3362,6 @@ match_orclause_to_indexcol(PlannerInfo *root,
if (IsA(indexExpr, RelabelType))
indexExpr = (Node *) ((RelabelType *) indexExpr)->arg;
- /* We allow constant to be Const or Param */
- if (!IsA(constExpr, Const) && !IsA(constExpr, Param))
- break;
-
/* Forbid transformation for composite types, records. */
if (type_is_rowtype(exprType(constExpr)) ||
type_is_rowtype(exprType(indexExpr)))
@@ -3390,8 +3398,12 @@ match_orclause_to_indexcol(PlannerInfo *root,
break;
}
- if (IsA(constExpr, Param))
- haveParam = true;
+ /*
+ * Check if our list of constants in match_clause_to_indexcol's
+ * understanding of const-ness have something other than Const.
+ */
+ if (!IsA(constExpr, Const))
+ haveNonConst = true;
consts = lappend(consts, constExpr);
}
@@ -3408,10 +3420,10 @@ match_orclause_to_indexcol(PlannerInfo *root,
/*
* Assemble an array from the list of constants. It seems more profitable
- * to build a const array. But in the presence of parameters, we don't
+ * to build a const array. But in the presence of other nodes, we don't
* have a specific value here and must employ an ArrayExpr instead.
*/
- if (haveParam)
+ if (haveNonConst)
{
ArrayExpr *arrayExpr = makeNode(ArrayExpr);
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 8011c141bf8..0ee7b2d7c61 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2006,6 +2006,35 @@ SELECT * FROM tenk1
Filter: (((tenthous)::numeric = '1'::numeric) OR (tenthous = 3) OR ((tenthous)::numeric = '42'::numeric))
(2 rows)
+EXPLAIN (COSTS OFF)
+SELECT t1.thousand FROM tenk1 t1
+ WHERE t1.thousand = 42 OR t1.thousand = (SELECT t2.tenthous FROM tenk1 t2 WHERE t2.thousand = t1.tenthous + 1 LIMIT 1);
+ QUERY PLAN
+----------------------------------------------------------------------
+ Index Only Scan using tenk1_thous_tenthous on tenk1 t1
+ Filter: ((thousand = 42) OR (thousand = (SubPlan 1)))
+ SubPlan 1
+ -> Limit
+ -> Index Only Scan using tenk1_thous_tenthous on tenk1 t2
+ Index Cond: (thousand = (t1.tenthous + 1))
+(6 rows)
+
+SELECT t1.thousand FROM tenk1 t1
+ WHERE t1.thousand = 42 OR t1.thousand = (SELECT t2.tenthous FROM tenk1 t2 WHERE t2.thousand = t1.tenthous + 1 LIMIT 1);
+ thousand
+----------
+ 42
+ 42
+ 42
+ 42
+ 42
+ 42
+ 42
+ 42
+ 42
+ 42
+(10 rows)
+
EXPLAIN (COSTS OFF)
SELECT count(*) FROM tenk1
WHERE hundred = 42 AND (thousand = 42 OR thousand = 99);
@@ -3255,6 +3284,16 @@ CREATE INDEX t_b_c_idx ON bitmap_split_or (b, c);
CREATE STATISTICS t_a_b_stat (mcv) ON a, b FROM bitmap_split_or;
CREATE STATISTICS t_b_c_stat (mcv) ON b, c FROM bitmap_split_or;
ANALYZE bitmap_split_or;
+EXPLAIN (COSTS OFF)
+SELECT * FROM bitmap_split_or t1, bitmap_split_or t2 WHERE t1.a=t2.b OR t1.a=1;
+ QUERY PLAN
+--------------------------------------------------------
+ Nested Loop
+ -> Seq Scan on bitmap_split_or t2
+ -> Index Scan using t_a_b_idx on bitmap_split_or t1
+ Index Cond: (a = ANY (ARRAY[t2.b, 1]))
+(4 rows)
+
EXPLAIN (COSTS OFF)
SELECT * FROM bitmap_split_or WHERE a = 1 AND (b = 1 OR b = 2) AND c = 2;
QUERY PLAN
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 079fcf46f0d..3ffc066b1f8 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -3849,14 +3849,11 @@ where q1 = thousand or q2 = thousand;
-> Seq Scan on q2
-> Bitmap Heap Scan on tenk1
Recheck Cond: ((q1.q1 = thousand) OR (q2.q2 = thousand))
- -> BitmapOr
- -> Bitmap Index Scan on tenk1_thous_tenthous
- Index Cond: (thousand = q1.q1)
- -> Bitmap Index Scan on tenk1_thous_tenthous
- Index Cond: (thousand = q2.q2)
+ -> Bitmap Index Scan on tenk1_thous_tenthous
+ Index Cond: (thousand = ANY (ARRAY[q1.q1, q2.q2]))
-> Hash
-> Seq Scan on int4_tbl
-(15 rows)
+(12 rows)
explain (costs off)
select * from
@@ -8239,3 +8236,45 @@ GROUP BY s.c1, s.c2;
(7 rows)
DROP TABLE group_tbl;
+--
+-- Test for a nested loop join involving index scan, transforming OR-clauses
+-- to SAOP.
+--
+EXPLAIN (COSTS OFF)
+SELECT COUNT(*) FROM tenk1 t1, tenk1 t2
+WHERE t2.thousand = t1.tenthous OR t2.thousand = t1.unique1 OR t2.thousand = t1.unique2;
+ QUERY PLAN
+-----------------------------------------------------------------------------------------
+ Aggregate
+ -> Nested Loop
+ -> Seq Scan on tenk1 t1
+ -> Index Only Scan using tenk1_thous_tenthous on tenk1 t2
+ Index Cond: (thousand = ANY (ARRAY[t1.tenthous, t1.unique1, t1.unique2]))
+(5 rows)
+
+SELECT COUNT(*) FROM tenk1 t1, tenk1 t2
+WHERE t2.thousand = t1.tenthous OR t2.thousand = t1.unique1 OR t2.thousand = t1.unique2;
+ count
+-------
+ 20000
+(1 row)
+
+EXPLAIN (COSTS OFF)
+SELECT COUNT(*) FROM onek t1 LEFT JOIN tenk1 t2
+ ON (t2.thousand = t1.tenthous OR t2.thousand = t1.thousand);
+ QUERY PLAN
+------------------------------------------------------------------------------
+ Aggregate
+ -> Nested Loop Left Join
+ -> Seq Scan on onek t1
+ -> Index Only Scan using tenk1_thous_tenthous on tenk1 t2
+ Index Cond: (thousand = ANY (ARRAY[t1.tenthous, t1.thousand]))
+(5 rows)
+
+SELECT COUNT(*) FROM onek t1 LEFT JOIN tenk1 t2
+ ON (t2.thousand = t1.tenthous OR t2.thousand = t1.thousand);
+ count
+-------
+ 19000
+(1 row)
+
diff --git a/src/test/regress/expected/partition_join.out b/src/test/regress/expected/partition_join.out
index 108f9ecb445..af468682a2d 100644
--- a/src/test/regress/expected/partition_join.out
+++ b/src/test/regress/expected/partition_join.out
@@ -2533,24 +2533,24 @@ where not exists (select 1 from prtx2
-> Seq Scan on prtx1_1
Filter: ((a < 20) AND (c = 91))
-> Bitmap Heap Scan on prtx2_1
- Recheck Cond: ((b = (prtx1_1.b + 1)) OR (c = 99))
+ Recheck Cond: ((c = 99) OR (b = (prtx1_1.b + 1)))
Filter: (a = prtx1_1.a)
-> BitmapOr
- -> Bitmap Index Scan on prtx2_1_b_idx
- Index Cond: (b = (prtx1_1.b + 1))
-> Bitmap Index Scan on prtx2_1_c_idx
Index Cond: (c = 99)
+ -> Bitmap Index Scan on prtx2_1_b_idx
+ Index Cond: (b = (prtx1_1.b + 1))
-> Nested Loop Anti Join
-> Seq Scan on prtx1_2
Filter: ((a < 20) AND (c = 91))
-> Bitmap Heap Scan on prtx2_2
- Recheck Cond: ((b = (prtx1_2.b + 1)) OR (c = 99))
+ Recheck Cond: ((c = 99) OR (b = (prtx1_2.b + 1)))
Filter: (a = prtx1_2.a)
-> BitmapOr
- -> Bitmap Index Scan on prtx2_2_b_idx
- Index Cond: (b = (prtx1_2.b + 1))
-> Bitmap Index Scan on prtx2_2_c_idx
Index Cond: (c = 99)
+ -> Bitmap Index Scan on prtx2_2_b_idx
+ Index Cond: (b = (prtx1_2.b + 1))
(23 rows)
select * from prtx1
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index 068c66b95a5..ca8a20af115 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -781,6 +781,12 @@ EXPLAIN (COSTS OFF)
SELECT * FROM tenk1
WHERE tenthous = 1::numeric OR tenthous = 3::int4 OR tenthous = 42::numeric;
+EXPLAIN (COSTS OFF)
+SELECT t1.thousand FROM tenk1 t1
+ WHERE t1.thousand = 42 OR t1.thousand = (SELECT t2.tenthous FROM tenk1 t2 WHERE t2.thousand = t1.tenthous + 1 LIMIT 1);
+SELECT t1.thousand FROM tenk1 t1
+ WHERE t1.thousand = 42 OR t1.thousand = (SELECT t2.tenthous FROM tenk1 t2 WHERE t2.thousand = t1.tenthous + 1 LIMIT 1);
+
EXPLAIN (COSTS OFF)
SELECT count(*) FROM tenk1
WHERE hundred = 42 AND (thousand = 42 OR thousand = 99);
@@ -1367,6 +1373,8 @@ CREATE STATISTICS t_a_b_stat (mcv) ON a, b FROM bitmap_split_or;
CREATE STATISTICS t_b_c_stat (mcv) ON b, c FROM bitmap_split_or;
ANALYZE bitmap_split_or;
EXPLAIN (COSTS OFF)
+SELECT * FROM bitmap_split_or t1, bitmap_split_or t2 WHERE t1.a=t2.b OR t1.a=1;
+EXPLAIN (COSTS OFF)
SELECT * FROM bitmap_split_or WHERE a = 1 AND (b = 1 OR b = 2) AND c = 2;
DROP TABLE bitmap_split_or;
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 779d56cb30f..c7349eab933 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -3016,7 +3016,6 @@ SELECT t1.a FROM skip_fetch t1 LEFT JOIN skip_fetch t2 ON t2.a = 1 WHERE t2.a IS
RESET enable_indexonlyscan;
RESET enable_seqscan;
-
-- Test BitmapHeapScan with a rescan releases resources correctly
SET enable_seqscan = off;
SET enable_indexscan = off;
@@ -3046,3 +3045,20 @@ SELECT 1 FROM group_tbl t1
GROUP BY s.c1, s.c2;
DROP TABLE group_tbl;
+
+--
+-- Test for a nested loop join involving index scan, transforming OR-clauses
+-- to SAOP.
+--
+
+EXPLAIN (COSTS OFF)
+SELECT COUNT(*) FROM tenk1 t1, tenk1 t2
+WHERE t2.thousand = t1.tenthous OR t2.thousand = t1.unique1 OR t2.thousand = t1.unique2;
+SELECT COUNT(*) FROM tenk1 t1, tenk1 t2
+WHERE t2.thousand = t1.tenthous OR t2.thousand = t1.unique1 OR t2.thousand = t1.unique2;
+
+EXPLAIN (COSTS OFF)
+SELECT COUNT(*) FROM onek t1 LEFT JOIN tenk1 t2
+ ON (t2.thousand = t1.tenthous OR t2.thousand = t1.thousand);
+SELECT COUNT(*) FROM onek t1 LEFT JOIN tenk1 t2
+ ON (t2.thousand = t1.tenthous OR t2.thousand = t1.thousand);
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-02-02 17:58 Alexander Korotkov <[email protected]>
parent: Andrei Lepikhov <[email protected]>
1 sibling, 0 replies; 38+ messages in thread
From: Alexander Korotkov @ 2025-02-02 17:58 UTC (permalink / raw)
To: Andrei Lepikhov <[email protected]>; +Cc: Alena Rybakina <[email protected]>; Ranier Vilela <[email protected]>; Alexander Lakhin <[email protected]>; jian he <[email protected]>; Robert Haas <[email protected]>; Nikolay Shaplov <[email protected]>; [email protected]; pgsql-hackers; Peter Geoghegan <[email protected]>; Marcos Pegoraro <[email protected]>; [email protected]; Peter Eisentraut <[email protected]>
On Tue, Jan 28, 2025 at 10:42 AM Andrei Lepikhov <[email protected]> wrote:
> On 1/28/25 11:36, Andrei Lepikhov wrote:
> > On 1/27/25 16:50, Alexander Korotkov wrote:
> > qsort(matches, n, sizeof(OrArgIndexMatch), or_arg_index_match_cmp);
> >
> > To fit an index, the order of elements in the target array of the
> > `ScalarArrayOpExpr` may change compared to the initial list of OR
> > expressions. If there are indexes that cover the same set of columns but
> > in reverse order, this could potentially alter the position of a
> > Subplan. However, I believe this is a rare case; it is supported by the
> > initial OR path and should be acceptable.
> I beg your pardon - I forgot that we've restricted the feature's scope
> and can't combine OR clauses into ScalarArrayOpExpr if the args list
> contains references to different columns.
> So, my note can't be applied here.
OK, thank you!
------
Regards,
Alexander Korotkov
Supabase
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-02-02 18:00 Alexander Korotkov <[email protected]>
parent: Alena Rybakina <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: Alexander Korotkov @ 2025-02-02 18:00 UTC (permalink / raw)
To: Alena Rybakina <[email protected]>; +Cc: Ranier Vilela <[email protected]>; Alexander Lakhin <[email protected]>; jian he <[email protected]>; Robert Haas <[email protected]>; Nikolay Shaplov <[email protected]>; [email protected]; pgsql-hackers; Peter Geoghegan <[email protected]>; Marcos Pegoraro <[email protected]>; [email protected]; Peter Eisentraut <[email protected]>; Andrei Lepikhov <[email protected]>
On Fri, Jan 31, 2025 at 4:31 PM Alena Rybakina
<[email protected]> wrote:
> I started reviewing at the patch and saw some output "ERROR" in the output of the test and is it okay here?
>
> SELECT * FROM tenk1 t1
> WHERE t1.thousand = 42 OR t1.thousand = (SELECT t2.tenthous FROM tenk1 t2 WHERE t2.thousand = t1.tenthous);
> ERROR: more than one row returned by a subquery used as an expression
The output is correct for this query. But the query is very
unfortunate for the regression test. I've revised query in the v47
revision [1].
Links.
1. https://www.postgresql.org/message-id/CAPpHfdsBZmNt9qUoJBqsQFiVDX1%3DyCKpuVAt1YnR7JCpP%3Dk8%2BA%40ma...
------
Regards,
Alexander Korotkov
Supabase
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-02-03 06:24 Andrei Lepikhov <[email protected]>
parent: Alexander Korotkov <[email protected]>
0 siblings, 0 replies; 38+ messages in thread
From: Andrei Lepikhov @ 2025-02-03 06:24 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; Pavel Borisov <[email protected]>; +Cc: Alena Rybakina <[email protected]>; Ranier Vilela <[email protected]>; Alexander Lakhin <[email protected]>; jian he <[email protected]>; Robert Haas <[email protected]>; Nikolay Shaplov <[email protected]>; [email protected]; pgsql-hackers; Peter Geoghegan <[email protected]>; Marcos Pegoraro <[email protected]>; [email protected]; Peter Eisentraut <[email protected]>
On 2/3/25 00:57, Alexander Korotkov wrote:
> On Thu, Jan 30, 2025 at 3:23 PM Pavel Borisov <[email protected]> wrote:
>> On Tue, 28 Jan 2025 at 12:42, Andrei Lepikhov <[email protected]> wrote:
>>>
>>> On 1/28/25 11:36, Andrei Lepikhov wrote:
>>>> On 1/27/25 16:50, Alexander Korotkov wrote:
>>>> qsort(matches, n, sizeof(OrArgIndexMatch), or_arg_index_match_cmp);
>>>>
>>>> To fit an index, the order of elements in the target array of the
>>>> `ScalarArrayOpExpr` may change compared to the initial list of OR
>>>> expressions. If there are indexes that cover the same set of columns but
>>>> in reverse order, this could potentially alter the position of a
>>>> Subplan. However, I believe this is a rare case; it is supported by the
>>>> initial OR path and should be acceptable.
>>> I beg your pardon - I forgot that we've restricted the feature's scope
>>> and can't combine OR clauses into ScalarArrayOpExpr if the args list
>>> contains references to different columns.
>>> So, my note can't be applied here.
>>>
>>> --
>>> regards, Andrei Lepikhov
>>
>> I've looked at the patch v46-0001
>> Looks good to me.
>>
>> There is a test that demonstrates the behavior change. Maybe some more
>> cases like are also worth adding to a test.
>>
>> +SELECT * FROM bitmap_split_or t1, bitmap_split_or t2 WHERE t1.a=t2.c
>> OR (t1.a=t2.b OR t1.a=1);
>> + QUERY PLAN
>> +--------------------------------------------------------
>> + Nested Loop
>> + -> Seq Scan on bitmap_split_or t2
>> + -> Index Scan using t_a_b_idx on bitmap_split_or t1
>> + Index Cond: (a = ANY (ARRAY[t2.c, t2.b, 1]))
>> +(4 rows)
>> +
>> +EXPLAIN (COSTS OFF)
>> +SELECT * FROM bitmap_split_or t1, bitmap_split_or t2 WHERE t1.c=t2.b OR t1.a=1;
>> + QUERY PLAN
>> +----------------------------------------------
>> + Nested Loop
>> + Join Filter: ((t1.c = t2.b) OR (t1.a = 1))
>> + -> Seq Scan on bitmap_split_or t1
>> + -> Materialize
>> + -> Seq Scan on bitmap_split_or t2
>> +(5 rows)
>> +
>> +EXPLAIN (COSTS OFF)
>
> Added more tests to join.sql
I have made final pass through the changes. All looks good.
Only one thing looks strange for me - multiple '42's in the output of
the test. May be reduce output by an aggregate in the target list of the
query?
--
regards, Andrei Lepikhov
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-02-03 10:22 Alena Rybakina <[email protected]>
parent: Alexander Korotkov <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: Alena Rybakina @ 2025-02-03 10:22 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; +Cc: Ranier Vilela <[email protected]>; Alexander Lakhin <[email protected]>; jian he <[email protected]>; Robert Haas <[email protected]>; Nikolay Shaplov <[email protected]>; [email protected]; pgsql-hackers; Peter Geoghegan <[email protected]>; Marcos Pegoraro <[email protected]>; [email protected]; Peter Eisentraut <[email protected]>; Andrei Lepikhov <[email protected]>
Thank you for updated version! I agree for your version of the code.
On 02.02.2025 21:00, Alexander Korotkov wrote:
> On Fri, Jan 31, 2025 at 4:31 PM Alena Rybakina
> <[email protected]> wrote:
>> I started reviewing at the patch and saw some output "ERROR" in the output of the test and is it okay here?
>>
>> SELECT * FROM tenk1 t1
>> WHERE t1.thousand = 42 OR t1.thousand = (SELECT t2.tenthous FROM tenk1 t2 WHERE t2.thousand = t1.tenthous);
>> ERROR: more than one row returned by a subquery used as an expression
> The output is correct for this query. But the query is very
> unfortunate for the regression test. I've revised query in the v47
> revision [1].
>
> Links.
> 1.https://www.postgresql.org/message-id/CAPpHfdsBZmNt9qUoJBqsQFiVDX1%3DyCKpuVAt1YnR7JCpP%3Dk8%2BA%40ma...
While analyzing the modified query plan from the regression test, I
noticed that despite using a full seqscan for table t2 in the original
plan,
its results are cached by Materialize node, and this can significantly
speed up the execution of the NestedLoop algorithm.
For example, after running the query several times, I got results that
show that the query execution time was twice as bad.
Original plan:
EXPLAIN ANALYZE SELECT * FROM bitmap_split_or t1, bitmap_split_or t2
WHERE t1.a=t2.b OR t1.a=1; QUERY PLAN
--------------------------------------------------------------------------------------------------------------------------------
Nested Loop (cost=0.00..70067.00 rows=2502499 width=24) (actual
time=0.015..1123.247 rows=2003000 loops=1) Join Filter: ((t1.a = t2.b)
OR (t1.a = 1)) Rows Removed by Join Filter: 1997000 Buffers: shared
hit=22 -> Seq Scan on bitmap_split_or t1 (cost=0.00..31.00 rows=2000
width=12) (actual time=0.006..0.372 rows=2000 loops=1) Buffers: shared
hit=11 -> Materialize (cost=0.00..41.00 rows=2000 width=12) (actual
time=0.000..0.111 rows=2000 loops=2000) Storage: Memory Maximum Storage:
110kB Buffers: shared hit=11 -> Seq Scan on bitmap_split_or t2
(cost=0.00..31.00 rows=2000 width=12) (actual time=0.003..0.188
rows=2000 loops=1) Buffers: shared hit=11 Planning Time: 0.118 ms
Execution Time: 1204.874 ms (13 rows)
Query plan after the patch:
EXPLAIN ANALYZE SELECT * FROM bitmap_split_or t1, bitmap_split_or t2
WHERE t1.a=t2.b OR t1.a=1; QUERY PLAN
-----------------------------------------------------------------------------------------------------------------------------------------------
Nested Loop (cost=0.28..56369.00 rows=2502499 width=24) (actual
time=0.121..2126.606 rows=2003000 loops=1) Buffers: shared hit=16009
read=2 -> Seq Scan on bitmap_split_or t2 (cost=0.00..31.00 rows=2000
width=12) (actual time=0.017..0.652 rows=2000 loops=1) Buffers: shared
hit=11 -> Index Scan using t_a_b_idx on bitmap_split_or t1
(cost=0.28..18.15 rows=1002 width=12) (actual time=0.044..0.627
rows=1002 loops=2000) Index Cond: (a = ANY (ARRAY[t2.b, 1])) Buffers:
shared hit=15998 read=2 Planning Time: 0.282 ms Execution Time: 2344.367
ms (9 rows)
I'm afraid that we may lose this with this optimization. Maybe this can
be taken into account somehow, what do you think?
--
Regards,
Alena Rybakina
Postgres Professional
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-02-03 11:32 Alexander Korotkov <[email protected]>
parent: Alena Rybakina <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: Alexander Korotkov @ 2025-02-03 11:32 UTC (permalink / raw)
To: Alena Rybakina <[email protected]>; +Cc: Ranier Vilela <[email protected]>; Alexander Lakhin <[email protected]>; jian he <[email protected]>; Robert Haas <[email protected]>; Nikolay Shaplov <[email protected]>; [email protected]; pgsql-hackers; Peter Geoghegan <[email protected]>; Marcos Pegoraro <[email protected]>; [email protected]; Peter Eisentraut <[email protected]>; Andrei Lepikhov <[email protected]>
On Mon, Feb 3, 2025 at 12:22 PM Alena Rybakina
<[email protected]> wrote:
>
> Thank you for updated version! I agree for your version of the code.
>
> On 02.02.2025 21:00, Alexander Korotkov wrote:
>
> On Fri, Jan 31, 2025 at 4:31 PM Alena Rybakina
> <[email protected]> wrote:
>
> I started reviewing at the patch and saw some output "ERROR" in the output of the test and is it okay here?
>
> SELECT * FROM tenk1 t1
> WHERE t1.thousand = 42 OR t1.thousand = (SELECT t2.tenthous FROM tenk1 t2 WHERE t2.thousand = t1.tenthous);
> ERROR: more than one row returned by a subquery used as an expression
>
> The output is correct for this query. But the query is very
> unfortunate for the regression test. I've revised query in the v47
> revision [1].
>
> Links.
> 1. https://www.postgresql.org/message-id/CAPpHfdsBZmNt9qUoJBqsQFiVDX1%3DyCKpuVAt1YnR7JCpP%3Dk8%2BA%40ma...
>
> While analyzing the modified query plan from the regression test, I noticed that despite using a full seqscan for table t2 in the original plan,
> its results are cached by Materialize node, and this can significantly speed up the execution of the NestedLoop algorithm.
>
> For example, after running the query several times, I got results that show that the query execution time was twice as bad.
>
> Original plan:
>
> EXPLAIN ANALYZE SELECT * FROM bitmap_split_or t1, bitmap_split_or t2 WHERE t1.a=t2.b OR t1.a=1; QUERY PLAN -------------------------------------------------------------------------------------------------------------------------------- Nested Loop (cost=0.00..70067.00 rows=2502499 width=24) (actual time=0.015..1123.247 rows=2003000 loops=1) Join Filter: ((t1.a = t2.b) OR (t1.a = 1)) Rows Removed by Join Filter: 1997000 Buffers: shared hit=22 -> Seq Scan on bitmap_split_or t1 (cost=0.00..31.00 rows=2000 width=12) (actual time=0.006..0.372 rows=2000 loops=1) Buffers: shared hit=11 -> Materialize (cost=0.00..41.00 rows=2000 width=12) (actual time=0.000..0.111 rows=2000 loops=2000) Storage: Memory Maximum Storage: 110kB Buffers: shared hit=11 -> Seq Scan on bitmap_split_or t2 (cost=0.00..31.00 rows=2000 width=12) (actual time=0.003..0.188 rows=2000 loops=1) Buffers: shared hit=11 Planning Time: 0.118 ms Execution Time: 1204.874 ms (13 rows)
>
> Query plan after the patch:
>
> EXPLAIN ANALYZE SELECT * FROM bitmap_split_or t1, bitmap_split_or t2 WHERE t1.a=t2.b OR t1.a=1; QUERY PLAN ----------------------------------------------------------------------------------------------------------------------------------------------- Nested Loop (cost=0.28..56369.00 rows=2502499 width=24) (actual time=0.121..2126.606 rows=2003000 loops=1) Buffers: shared hit=16009 read=2 -> Seq Scan on bitmap_split_or t2 (cost=0.00..31.00 rows=2000 width=12) (actual time=0.017..0.652 rows=2000 loops=1) Buffers: shared hit=11 -> Index Scan using t_a_b_idx on bitmap_split_or t1 (cost=0.28..18.15 rows=1002 width=12) (actual time=0.044..0.627 rows=1002 loops=2000) Index Cond: (a = ANY (ARRAY[t2.b, 1])) Buffers: shared hit=15998 read=2 Planning Time: 0.282 ms Execution Time: 2344.367 ms (9 rows)
>
> I'm afraid that we may lose this with this optimization. Maybe this can be taken into account somehow, what do you think?
The important aspect is that the second plan have lower cost than the
first one. So, that's the question to the cost model. The patch just
lets optimizer consider more comprehensive plurality of paths. You
can let optimizer select the first plan by tuning *_cost params. For
example, setting cpu_index_tuple_cost = 0.02 makes first plan win for
me.
Other than that the test query is quite unfortunate as t1.a=1 is very
frequent. I've adjusted the query so that nested loop with index scan
wins both in cost and execution time.
I've also adjusted another test query as proposed by Andrei.
I'm going to push this patch is there is no more notes.
Links.
1. https://www.postgresql.org/message-id/fc1017ca-877b-4f86-b491-154cf123eedd%40gmail.com
------
Regards,
Alexander Korotkov
Supabase
Attachments:
[application/octet-stream] v48-0001-Revise-the-header-comment-for-match_clause_to_in.patch (1.3K, ../../CAPpHfdumgFuUihGqrmDVR5FnzOLsAah-ZdZ269FgsUrOeNU6rw@mail.gmail.com/2-v48-0001-Revise-the-header-comment-for-match_clause_to_in.patch)
download | inline diff:
From 0fe7f1eb6447b86f8c9e8ec472333e13ed36a255 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Sat, 1 Feb 2025 16:53:32 +0200
Subject: [PATCH v48 1/2] Revise the header comment for
match_clause_to_indexcol()
Since d4378c0005e6, match_clause_to_indexcol() doesn't always return NULL
for an OR clause. This commit reflects that in the header comment.
Reported-by: Pavel Borisov <[email protected]>
---
src/backend/optimizer/path/indxpath.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index fa3edf60f3c..a58cf5bad1a 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -2639,8 +2639,9 @@ match_clause_to_index(PlannerInfo *root,
* Returns an IndexClause if the clause can be used with this index key,
* or NULL if not.
*
- * NOTE: returns NULL if clause is an OR or AND clause; it is the
- * responsibility of higher-level routines to cope with those.
+ * NOTE: This routine always returns NULL if the clause is an AND clause.
+ * Higher-level routines deal with OR and AND clauses. OR clause can be
+ * matched as a whole by match_orclause_to_indexcol() though.
*/
static IndexClause *
match_clause_to_indexcol(PlannerInfo *root,
--
2.39.5 (Apple Git-154)
[application/octet-stream] v48-0002-Allow-usage-of-match_orclause_to_indexcol-for-jo.patch (17.2K, ../../CAPpHfdumgFuUihGqrmDVR5FnzOLsAah-ZdZ269FgsUrOeNU6rw@mail.gmail.com/3-v48-0002-Allow-usage-of-match_orclause_to_indexcol-for-jo.patch)
download | inline diff:
From c35d3b71bd8db9de8941af1d07421ba2456c223b Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Wed, 9 Oct 2024 18:44:25 +0300
Subject: [PATCH v48 2/2] Allow usage of match_orclause_to_indexcol() for joins
This commit allows transformation of OR-clauses into SAOP's for index scans
within nested loop joins. That required the following changes.
1. Make match_orclause_to_indexcol() and group_similar_or_args() understand
const-ness in the same way as match_opclause_to_indexcol(). This
generally makes our approach more uniform.
2. Make match_join_clauses_to_index() pass OR-clauses to
match_clause_to_index().
3. Also switch match_join_clauses_to_index() to use list_append_unique_ptr()
for adding clauses to *joinorclauses. That avoids possible duplicates
when processing the same clauses with different indexes. Previously such
duplicates were elimited in match_clause_to_index(), but now
group_similar_or_args() each time generates distinct copies of grouped
OR clauses.
Discussion: https://postgr.es/m/CAPpHfdv%2BjtNwofg-p5z86jLYZUTt6tR17Wy00ta0dL%3DwHQN3ZA%40mail.gmail.com
Reviewed-by: Andrei Lepikhov <[email protected]>
Reviewed-by: Alena Rybakina <[email protected]>
Reviewed-by: Pavel Borisov <[email protected]>
---
src/backend/optimizer/path/indxpath.c | 70 ++++++++++++--------
src/test/regress/expected/create_index.out | 32 +++++++++
src/test/regress/expected/join.out | 51 ++++++++++++--
src/test/regress/expected/partition_join.out | 12 ++--
src/test/regress/sql/create_index.sql | 9 +++
src/test/regress/sql/join.sql | 18 ++++-
6 files changed, 150 insertions(+), 42 deletions(-)
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index a58cf5bad1a..6e2051efc65 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -1255,6 +1255,7 @@ group_similar_or_args(PlannerInfo *root, RelOptInfo *rel, RestrictInfo *rinfo)
ListCell *lc2;
List *orargs;
List *result = NIL;
+ Index relid = rel->relid;
Assert(IsA(rinfo->orclause, BoolExpr));
orargs = ((BoolExpr *) rinfo->orclause)->args;
@@ -1319,10 +1320,13 @@ group_similar_or_args(PlannerInfo *root, RelOptInfo *rel, RestrictInfo *rinfo)
/*
* Check for clauses of the form: (indexkey operator constant) or
* (constant operator indexkey). But we don't know a particular index
- * yet. First check for a constant, which must be Const or Param.
- * That's cheaper than search for an index key among all indexes.
+ * yet. Therefore, we try to distinguish the potential index key and
+ * constant first, then search for a matching index key among all
+ * indexes.
*/
- if (IsA(leftop, Const) || IsA(leftop, Param))
+ if (bms_is_member(relid, argrinfo->right_relids) &&
+ !bms_is_member(relid, argrinfo->left_relids) &&
+ !contain_volatile_functions(leftop))
{
opno = get_commutator(opno);
@@ -1333,7 +1337,9 @@ group_similar_or_args(PlannerInfo *root, RelOptInfo *rel, RestrictInfo *rinfo)
}
nonConstExpr = rightop;
}
- else if (IsA(rightop, Const) || IsA(rightop, Param))
+ else if (bms_is_member(relid, argrinfo->left_relids) &&
+ !bms_is_member(relid, argrinfo->right_relids) &&
+ !contain_volatile_functions(rightop))
{
nonConstExpr = leftop;
}
@@ -2414,6 +2420,7 @@ match_restriction_clauses_to_index(PlannerInfo *root,
* Identify join clauses for the rel that match the index.
* Matching clauses are added to *clauseset.
* Also, add any potentially usable join OR clauses to *joinorclauses.
+ * They also might be processed by match_clause_to_index() as a whole.
*/
static void
match_join_clauses_to_index(PlannerInfo *root,
@@ -2432,11 +2439,15 @@ match_join_clauses_to_index(PlannerInfo *root,
if (!join_clause_is_movable_to(rinfo, rel))
continue;
- /* Potentially usable, so see if it matches the index or is an OR */
+ /*
+ * Potentially usable, so see if it matches the index or is an OR. Use
+ * list_append_unique_ptr() here to avoid possible duplicates when
+ * processing the same clauses with different indexes.
+ */
if (restriction_is_or_clause(rinfo))
- *joinorclauses = lappend(*joinorclauses, rinfo);
- else
- match_clause_to_index(root, rinfo, index, clauseset);
+ *joinorclauses = list_append_unique_ptr(*joinorclauses, rinfo);
+
+ match_clause_to_index(root, rinfo, index, clauseset);
}
}
@@ -2585,10 +2596,7 @@ match_clause_to_index(PlannerInfo *root,
* (3) must match the collation of the index, if collation is relevant.
*
* Our definition of "const" is exceedingly liberal: we allow anything that
- * doesn't involve a volatile function or a Var of the index's relation
- * except for a boolean OR expression input: due to a trade-off between the
- * expected execution speedup and planning complexity, we limit or->saop
- * transformation by obvious cases when an index scan can get a profit.
+ * doesn't involve a volatile function or a Var of the index's relation.
* In particular, Vars belonging to other relations of the query are
* accepted here, since a clause of that form can be used in a
* parameterized indexscan. It's the responsibility of higher code levels
@@ -3247,7 +3255,8 @@ match_orclause_to_indexcol(PlannerInfo *root,
Oid arraytype = InvalidOid;
Oid inputcollid = InvalidOid;
bool firstTime = true;
- bool haveParam = false;
+ bool haveNonConst = false;
+ Index indexRelid = index->rel->relid;
Assert(IsA(orclause, BoolExpr));
Assert(orclause->boolop == OR_EXPR);
@@ -3259,10 +3268,9 @@ match_orclause_to_indexcol(PlannerInfo *root,
/*
* Try to convert a list of OR-clauses to a single SAOP expression. Each
* OR entry must be in the form: (indexkey operator constant) or (constant
- * operator indexkey). Operators of all the entries must match. Constant
- * might be either Const or Param. To be effective, give up on the first
- * non-matching entry. Exit is implemented as a break from the loop,
- * which is catched afterwards.
+ * operator indexkey). Operators of all the entries must match. To be
+ * effective, give up on the first non-matching entry. Exit is
+ * implemented as a break from the loop, which is catched afterwards.
*/
foreach(lc, orclause->args)
{
@@ -3313,17 +3321,21 @@ match_orclause_to_indexcol(PlannerInfo *root,
/*
* Check for clauses of the form: (indexkey operator constant) or
- * (constant operator indexkey). Determine indexkey side first, check
- * the constant later.
+ * (constant operator indexkey). See match_clause_to_indexcol's notes
+ * about const-ness.
*/
leftop = (Node *) linitial(subClause->args);
rightop = (Node *) lsecond(subClause->args);
- if (match_index_to_operand(leftop, indexcol, index))
+ if (match_index_to_operand(leftop, indexcol, index) &&
+ !bms_is_member(indexRelid, subRinfo->right_relids) &&
+ !contain_volatile_functions(rightop))
{
indexExpr = leftop;
constExpr = rightop;
}
- else if (match_index_to_operand(rightop, indexcol, index))
+ else if (match_index_to_operand(rightop, indexcol, index) &&
+ !bms_is_member(indexRelid, subRinfo->left_relids) &&
+ !contain_volatile_functions(leftop))
{
opno = get_commutator(opno);
if (!OidIsValid(opno))
@@ -3350,10 +3362,6 @@ match_orclause_to_indexcol(PlannerInfo *root,
if (IsA(indexExpr, RelabelType))
indexExpr = (Node *) ((RelabelType *) indexExpr)->arg;
- /* We allow constant to be Const or Param */
- if (!IsA(constExpr, Const) && !IsA(constExpr, Param))
- break;
-
/* Forbid transformation for composite types, records. */
if (type_is_rowtype(exprType(constExpr)) ||
type_is_rowtype(exprType(indexExpr)))
@@ -3390,8 +3398,12 @@ match_orclause_to_indexcol(PlannerInfo *root,
break;
}
- if (IsA(constExpr, Param))
- haveParam = true;
+ /*
+ * Check if our list of constants in match_clause_to_indexcol's
+ * understanding of const-ness have something other than Const.
+ */
+ if (!IsA(constExpr, Const))
+ haveNonConst = true;
consts = lappend(consts, constExpr);
}
@@ -3408,10 +3420,10 @@ match_orclause_to_indexcol(PlannerInfo *root,
/*
* Assemble an array from the list of constants. It seems more profitable
- * to build a const array. But in the presence of parameters, we don't
+ * to build a const array. But in the presence of other nodes, we don't
* have a specific value here and must employ an ArrayExpr instead.
*/
- if (haveParam)
+ if (haveNonConst)
{
ArrayExpr *arrayExpr = makeNode(ArrayExpr);
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 8011c141bf8..bd5f002cf20 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2006,6 +2006,27 @@ SELECT * FROM tenk1
Filter: (((tenthous)::numeric = '1'::numeric) OR (tenthous = 3) OR ((tenthous)::numeric = '42'::numeric))
(2 rows)
+EXPLAIN (COSTS OFF)
+SELECT count(*) FROM tenk1 t1
+ WHERE t1.thousand = 42 OR t1.thousand = (SELECT t2.tenthous FROM tenk1 t2 WHERE t2.thousand = t1.tenthous + 1 LIMIT 1);
+ QUERY PLAN
+----------------------------------------------------------------------------
+ Aggregate
+ -> Index Only Scan using tenk1_thous_tenthous on tenk1 t1
+ Filter: ((thousand = 42) OR (thousand = (SubPlan 1)))
+ SubPlan 1
+ -> Limit
+ -> Index Only Scan using tenk1_thous_tenthous on tenk1 t2
+ Index Cond: (thousand = (t1.tenthous + 1))
+(7 rows)
+
+SELECT count(*) FROM tenk1 t1
+ WHERE t1.thousand = 42 OR t1.thousand = (SELECT t2.tenthous FROM tenk1 t2 WHERE t2.thousand = t1.tenthous + 1 LIMIT 1);
+ count
+-------
+ 10
+(1 row)
+
EXPLAIN (COSTS OFF)
SELECT count(*) FROM tenk1
WHERE hundred = 42 AND (thousand = 42 OR thousand = 99);
@@ -3255,6 +3276,17 @@ CREATE INDEX t_b_c_idx ON bitmap_split_or (b, c);
CREATE STATISTICS t_a_b_stat (mcv) ON a, b FROM bitmap_split_or;
CREATE STATISTICS t_b_c_stat (mcv) ON b, c FROM bitmap_split_or;
ANALYZE bitmap_split_or;
+EXPLAIN (COSTS OFF)
+SELECT * FROM bitmap_split_or t1, bitmap_split_or t2
+WHERE t1.a = t2.b OR t1.a = 2;
+ QUERY PLAN
+--------------------------------------------------------
+ Nested Loop
+ -> Seq Scan on bitmap_split_or t2
+ -> Index Scan using t_a_b_idx on bitmap_split_or t1
+ Index Cond: (a = ANY (ARRAY[t2.b, 2]))
+(4 rows)
+
EXPLAIN (COSTS OFF)
SELECT * FROM bitmap_split_or WHERE a = 1 AND (b = 1 OR b = 2) AND c = 2;
QUERY PLAN
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 079fcf46f0d..3ffc066b1f8 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -3849,14 +3849,11 @@ where q1 = thousand or q2 = thousand;
-> Seq Scan on q2
-> Bitmap Heap Scan on tenk1
Recheck Cond: ((q1.q1 = thousand) OR (q2.q2 = thousand))
- -> BitmapOr
- -> Bitmap Index Scan on tenk1_thous_tenthous
- Index Cond: (thousand = q1.q1)
- -> Bitmap Index Scan on tenk1_thous_tenthous
- Index Cond: (thousand = q2.q2)
+ -> Bitmap Index Scan on tenk1_thous_tenthous
+ Index Cond: (thousand = ANY (ARRAY[q1.q1, q2.q2]))
-> Hash
-> Seq Scan on int4_tbl
-(15 rows)
+(12 rows)
explain (costs off)
select * from
@@ -8239,3 +8236,45 @@ GROUP BY s.c1, s.c2;
(7 rows)
DROP TABLE group_tbl;
+--
+-- Test for a nested loop join involving index scan, transforming OR-clauses
+-- to SAOP.
+--
+EXPLAIN (COSTS OFF)
+SELECT COUNT(*) FROM tenk1 t1, tenk1 t2
+WHERE t2.thousand = t1.tenthous OR t2.thousand = t1.unique1 OR t2.thousand = t1.unique2;
+ QUERY PLAN
+-----------------------------------------------------------------------------------------
+ Aggregate
+ -> Nested Loop
+ -> Seq Scan on tenk1 t1
+ -> Index Only Scan using tenk1_thous_tenthous on tenk1 t2
+ Index Cond: (thousand = ANY (ARRAY[t1.tenthous, t1.unique1, t1.unique2]))
+(5 rows)
+
+SELECT COUNT(*) FROM tenk1 t1, tenk1 t2
+WHERE t2.thousand = t1.tenthous OR t2.thousand = t1.unique1 OR t2.thousand = t1.unique2;
+ count
+-------
+ 20000
+(1 row)
+
+EXPLAIN (COSTS OFF)
+SELECT COUNT(*) FROM onek t1 LEFT JOIN tenk1 t2
+ ON (t2.thousand = t1.tenthous OR t2.thousand = t1.thousand);
+ QUERY PLAN
+------------------------------------------------------------------------------
+ Aggregate
+ -> Nested Loop Left Join
+ -> Seq Scan on onek t1
+ -> Index Only Scan using tenk1_thous_tenthous on tenk1 t2
+ Index Cond: (thousand = ANY (ARRAY[t1.tenthous, t1.thousand]))
+(5 rows)
+
+SELECT COUNT(*) FROM onek t1 LEFT JOIN tenk1 t2
+ ON (t2.thousand = t1.tenthous OR t2.thousand = t1.thousand);
+ count
+-------
+ 19000
+(1 row)
+
diff --git a/src/test/regress/expected/partition_join.out b/src/test/regress/expected/partition_join.out
index 108f9ecb445..af468682a2d 100644
--- a/src/test/regress/expected/partition_join.out
+++ b/src/test/regress/expected/partition_join.out
@@ -2533,24 +2533,24 @@ where not exists (select 1 from prtx2
-> Seq Scan on prtx1_1
Filter: ((a < 20) AND (c = 91))
-> Bitmap Heap Scan on prtx2_1
- Recheck Cond: ((b = (prtx1_1.b + 1)) OR (c = 99))
+ Recheck Cond: ((c = 99) OR (b = (prtx1_1.b + 1)))
Filter: (a = prtx1_1.a)
-> BitmapOr
- -> Bitmap Index Scan on prtx2_1_b_idx
- Index Cond: (b = (prtx1_1.b + 1))
-> Bitmap Index Scan on prtx2_1_c_idx
Index Cond: (c = 99)
+ -> Bitmap Index Scan on prtx2_1_b_idx
+ Index Cond: (b = (prtx1_1.b + 1))
-> Nested Loop Anti Join
-> Seq Scan on prtx1_2
Filter: ((a < 20) AND (c = 91))
-> Bitmap Heap Scan on prtx2_2
- Recheck Cond: ((b = (prtx1_2.b + 1)) OR (c = 99))
+ Recheck Cond: ((c = 99) OR (b = (prtx1_2.b + 1)))
Filter: (a = prtx1_2.a)
-> BitmapOr
- -> Bitmap Index Scan on prtx2_2_b_idx
- Index Cond: (b = (prtx1_2.b + 1))
-> Bitmap Index Scan on prtx2_2_c_idx
Index Cond: (c = 99)
+ -> Bitmap Index Scan on prtx2_2_b_idx
+ Index Cond: (b = (prtx1_2.b + 1))
(23 rows)
select * from prtx1
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index 068c66b95a5..be570da08a0 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -781,6 +781,12 @@ EXPLAIN (COSTS OFF)
SELECT * FROM tenk1
WHERE tenthous = 1::numeric OR tenthous = 3::int4 OR tenthous = 42::numeric;
+EXPLAIN (COSTS OFF)
+SELECT count(*) FROM tenk1 t1
+ WHERE t1.thousand = 42 OR t1.thousand = (SELECT t2.tenthous FROM tenk1 t2 WHERE t2.thousand = t1.tenthous + 1 LIMIT 1);
+SELECT count(*) FROM tenk1 t1
+ WHERE t1.thousand = 42 OR t1.thousand = (SELECT t2.tenthous FROM tenk1 t2 WHERE t2.thousand = t1.tenthous + 1 LIMIT 1);
+
EXPLAIN (COSTS OFF)
SELECT count(*) FROM tenk1
WHERE hundred = 42 AND (thousand = 42 OR thousand = 99);
@@ -1367,6 +1373,9 @@ CREATE STATISTICS t_a_b_stat (mcv) ON a, b FROM bitmap_split_or;
CREATE STATISTICS t_b_c_stat (mcv) ON b, c FROM bitmap_split_or;
ANALYZE bitmap_split_or;
EXPLAIN (COSTS OFF)
+SELECT * FROM bitmap_split_or t1, bitmap_split_or t2
+WHERE t1.a = t2.b OR t1.a = 2;
+EXPLAIN (COSTS OFF)
SELECT * FROM bitmap_split_or WHERE a = 1 AND (b = 1 OR b = 2) AND c = 2;
DROP TABLE bitmap_split_or;
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 779d56cb30f..c7349eab933 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -3016,7 +3016,6 @@ SELECT t1.a FROM skip_fetch t1 LEFT JOIN skip_fetch t2 ON t2.a = 1 WHERE t2.a IS
RESET enable_indexonlyscan;
RESET enable_seqscan;
-
-- Test BitmapHeapScan with a rescan releases resources correctly
SET enable_seqscan = off;
SET enable_indexscan = off;
@@ -3046,3 +3045,20 @@ SELECT 1 FROM group_tbl t1
GROUP BY s.c1, s.c2;
DROP TABLE group_tbl;
+
+--
+-- Test for a nested loop join involving index scan, transforming OR-clauses
+-- to SAOP.
+--
+
+EXPLAIN (COSTS OFF)
+SELECT COUNT(*) FROM tenk1 t1, tenk1 t2
+WHERE t2.thousand = t1.tenthous OR t2.thousand = t1.unique1 OR t2.thousand = t1.unique2;
+SELECT COUNT(*) FROM tenk1 t1, tenk1 t2
+WHERE t2.thousand = t1.tenthous OR t2.thousand = t1.unique1 OR t2.thousand = t1.unique2;
+
+EXPLAIN (COSTS OFF)
+SELECT COUNT(*) FROM onek t1 LEFT JOIN tenk1 t2
+ ON (t2.thousand = t1.tenthous OR t2.thousand = t1.thousand);
+SELECT COUNT(*) FROM onek t1 LEFT JOIN tenk1 t2
+ ON (t2.thousand = t1.tenthous OR t2.thousand = t1.thousand);
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-02-03 11:54 Alena Rybakina <[email protected]>
parent: Alexander Korotkov <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: Alena Rybakina @ 2025-02-03 11:54 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; +Cc: Ranier Vilela <[email protected]>; Alexander Lakhin <[email protected]>; jian he <[email protected]>; Robert Haas <[email protected]>; Nikolay Shaplov <[email protected]>; [email protected]; pgsql-hackers; Peter Geoghegan <[email protected]>; Marcos Pegoraro <[email protected]>; [email protected]; Peter Eisentraut <[email protected]>; Andrei Lepikhov <[email protected]>
On 03.02.2025 14:32, Alexander Korotkov wrote:
> On Mon, Feb 3, 2025 at 12:22 PM Alena Rybakina
> <[email protected]> wrote:
>> Thank you for updated version! I agree for your version of the code.
>>
>> On 02.02.2025 21:00, Alexander Korotkov wrote:
>>
>> On Fri, Jan 31, 2025 at 4:31 PM Alena Rybakina
>> <[email protected]> wrote:
>>
>> I started reviewing at the patch and saw some output "ERROR" in the output of the test and is it okay here?
>>
>> SELECT * FROM tenk1 t1
>> WHERE t1.thousand = 42 OR t1.thousand = (SELECT t2.tenthous FROM tenk1 t2 WHERE t2.thousand = t1.tenthous);
>> ERROR: more than one row returned by a subquery used as an expression
>>
>> The output is correct for this query. But the query is very
>> unfortunate for the regression test. I've revised query in the v47
>> revision [1].
>>
>> Links.
>> 1.https://www.postgresql.org/message-id/CAPpHfdsBZmNt9qUoJBqsQFiVDX1%3DyCKpuVAt1YnR7JCpP%3Dk8%2BA%40ma...
>>
>> While analyzing the modified query plan from the regression test, I noticed that despite using a full seqscan for table t2 in the original plan,
>> its results are cached by Materialize node, and this can significantly speed up the execution of the NestedLoop algorithm.
>>
>> For example, after running the query several times, I got results that show that the query execution time was twice as bad.
>>
>> Original plan:
>>
>> EXPLAIN ANALYZE SELECT * FROM bitmap_split_or t1, bitmap_split_or t2 WHERE t1.a=t2.b OR t1.a=1; QUERY PLAN -------------------------------------------------------------------------------------------------------------------------------- Nested Loop (cost=0.00..70067.00 rows=2502499 width=24) (actual time=0.015..1123.247 rows=2003000 loops=1) Join Filter: ((t1.a = t2.b) OR (t1.a = 1)) Rows Removed by Join Filter: 1997000 Buffers: shared hit=22 -> Seq Scan on bitmap_split_or t1 (cost=0.00..31.00 rows=2000 width=12) (actual time=0.006..0.372 rows=2000 loops=1) Buffers: shared hit=11 -> Materialize (cost=0.00..41.00 rows=2000 width=12) (actual time=0.000..0.111 rows=2000 loops=2000) Storage: Memory Maximum Storage: 110kB Buffers: shared hit=11 -> Seq Scan on bitmap_split_or t2 (cost=0.00..31.00 rows=2000 width=12) (actual time=0.003..0.188 rows=2000 loops=1) Buffers: shared hit=11 Planning Time: 0.118 ms Execution Time: 1204.874 ms (13 rows)
>>
>> Query plan after the patch:
>>
>> EXPLAIN ANALYZE SELECT * FROM bitmap_split_or t1, bitmap_split_or t2 WHERE t1.a=t2.b OR t1.a=1; QUERY PLAN ----------------------------------------------------------------------------------------------------------------------------------------------- Nested Loop (cost=0.28..56369.00 rows=2502499 width=24) (actual time=0.121..2126.606 rows=2003000 loops=1) Buffers: shared hit=16009 read=2 -> Seq Scan on bitmap_split_or t2 (cost=0.00..31.00 rows=2000 width=12) (actual time=0.017..0.652 rows=2000 loops=1) Buffers: shared hit=11 -> Index Scan using t_a_b_idx on bitmap_split_or t1 (cost=0.28..18.15 rows=1002 width=12) (actual time=0.044..0.627 rows=1002 loops=2000) Index Cond: (a = ANY (ARRAY[t2.b, 1])) Buffers: shared hit=15998 read=2 Planning Time: 0.282 ms Execution Time: 2344.367 ms (9 rows)
>>
>> I'm afraid that we may lose this with this optimization. Maybe this can be taken into account somehow, what do you think?
> The important aspect is that the second plan have lower cost than the
> first one. So, that's the question to the cost model. The patch just
> lets optimizer consider more comprehensive plurality of paths. You
> can let optimizer select the first plan by tuning *_cost params. For
> example, setting cpu_index_tuple_cost = 0.02 makes first plan win for
> me.
>
> Other than that the test query is quite unfortunate as t1.a=1 is very
> frequent. I've adjusted the query so that nested loop with index scan
> wins both in cost and execution time.
>
> I've also adjusted another test query as proposed by Andrei.
>
> I'm going to push this patch is there is no more notes.
>
> Links.
> 1.https://www.postgresql.org/message-id/fc1017ca-877b-4f86-b491-154cf123eedd%40gmail.com
>
Okay.I agree with your codeand have no more notes
--
Regards,
Alena Rybakina
Postgres Professional
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-02-03 12:16 Pavel Borisov <[email protected]>
parent: Alena Rybakina <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: Pavel Borisov @ 2025-02-03 12:16 UTC (permalink / raw)
To: Alena Rybakina <[email protected]>; +Cc: Alexander Korotkov <[email protected]>; Ranier Vilela <[email protected]>; Alexander Lakhin <[email protected]>; jian he <[email protected]>; Robert Haas <[email protected]>; Nikolay Shaplov <[email protected]>; [email protected]; pgsql-hackers; Peter Geoghegan <[email protected]>; Marcos Pegoraro <[email protected]>; [email protected]; Peter Eisentraut <[email protected]>; Andrei Lepikhov <[email protected]>
On Mon, 3 Feb 2025 at 15:54, Alena Rybakina <[email protected]> wrote:
>
>
> On 03.02.2025 14:32, Alexander Korotkov wrote:
>
> On Mon, Feb 3, 2025 at 12:22 PM Alena Rybakina
> <[email protected]> wrote:
>
> Thank you for updated version! I agree for your version of the code.
>
> On 02.02.2025 21:00, Alexander Korotkov wrote:
>
> On Fri, Jan 31, 2025 at 4:31 PM Alena Rybakina
> <[email protected]> wrote:
>
> I started reviewing at the patch and saw some output "ERROR" in the output of the test and is it okay here?
>
> SELECT * FROM tenk1 t1
> WHERE t1.thousand = 42 OR t1.thousand = (SELECT t2.tenthous FROM tenk1 t2 WHERE t2.thousand = t1.tenthous);
> ERROR: more than one row returned by a subquery used as an expression
>
> The output is correct for this query. But the query is very
> unfortunate for the regression test. I've revised query in the v47
> revision [1].
>
> Links.
> 1. https://www.postgresql.org/message-id/CAPpHfdsBZmNt9qUoJBqsQFiVDX1%3DyCKpuVAt1YnR7JCpP%3Dk8%2BA%40ma...
>
> While analyzing the modified query plan from the regression test, I noticed that despite using a full seqscan for table t2 in the original plan,
> its results are cached by Materialize node, and this can significantly speed up the execution of the NestedLoop algorithm.
>
> For example, after running the query several times, I got results that show that the query execution time was twice as bad.
>
> Original plan:
>
> EXPLAIN ANALYZE SELECT * FROM bitmap_split_or t1, bitmap_split_or t2 WHERE t1.a=t2.b OR t1.a=1; QUERY PLAN -------------------------------------------------------------------------------------------------------------------------------- Nested Loop (cost=0.00..70067.00 rows=2502499 width=24) (actual time=0.015..1123.247 rows=2003000 loops=1) Join Filter: ((t1.a = t2.b) OR (t1.a = 1)) Rows Removed by Join Filter: 1997000 Buffers: shared hit=22 -> Seq Scan on bitmap_split_or t1 (cost=0.00..31.00 rows=2000 width=12) (actual time=0.006..0.372 rows=2000 loops=1) Buffers: shared hit=11 -> Materialize (cost=0.00..41.00 rows=2000 width=12) (actual time=0.000..0.111 rows=2000 loops=2000) Storage: Memory Maximum Storage: 110kB Buffers: shared hit=11 -> Seq Scan on bitmap_split_or t2 (cost=0.00..31.00 rows=2000 width=12) (actual time=0.003..0.188 rows=2000 loops=1) Buffers: shared hit=11 Planning Time: 0.118 ms Execution Time: 1204.874 ms (13 rows)
>
> Query plan after the patch:
>
> EXPLAIN ANALYZE SELECT * FROM bitmap_split_or t1, bitmap_split_or t2 WHERE t1.a=t2.b OR t1.a=1; QUERY PLAN ----------------------------------------------------------------------------------------------------------------------------------------------- Nested Loop (cost=0.28..56369.00 rows=2502499 width=24) (actual time=0.121..2126.606 rows=2003000 loops=1) Buffers: shared hit=16009 read=2 -> Seq Scan on bitmap_split_or t2 (cost=0.00..31.00 rows=2000 width=12) (actual time=0.017..0.652 rows=2000 loops=1) Buffers: shared hit=11 -> Index Scan using t_a_b_idx on bitmap_split_or t1 (cost=0.28..18.15 rows=1002 width=12) (actual time=0.044..0.627 rows=1002 loops=2000) Index Cond: (a = ANY (ARRAY[t2.b, 1])) Buffers: shared hit=15998 read=2 Planning Time: 0.282 ms Execution Time: 2344.367 ms (9 rows)
>
> I'm afraid that we may lose this with this optimization. Maybe this can be taken into account somehow, what do you think?
>
> The important aspect is that the second plan have lower cost than the
> first one. So, that's the question to the cost model. The patch just
> lets optimizer consider more comprehensive plurality of paths. You
> can let optimizer select the first plan by tuning *_cost params. For
> example, setting cpu_index_tuple_cost = 0.02 makes first plan win for
> me.
>
> Other than that the test query is quite unfortunate as t1.a=1 is very
> frequent. I've adjusted the query so that nested loop with index scan
> wins both in cost and execution time.
>
> I've also adjusted another test query as proposed by Andrei.
>
> I'm going to push this patch is there is no more notes.
>
> Links.
> 1. https://www.postgresql.org/message-id/fc1017ca-877b-4f86-b491-154cf123eedd%40gmail.com
>
>
> Okay.I agree with your code and have no more notes
Hi, Alexander!
I've looked at patchset v48 and it looks good to me.
Regards,
Pavel Borisov
Supabase
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-03-24 10:10 Andrei Lepikhov <[email protected]>
parent: Pavel Borisov <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: Andrei Lepikhov @ 2025-03-24 10:10 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; +Cc: Alexander Lakhin <[email protected]>; [email protected]; Pavel Borisov <[email protected]>; a.rybakina <[email protected]>
Hi,
Playing with the feature, I found a slightly irritating permutation -
even if this code doesn't group any clauses, it may permute positions of
the quals. See:
DROP TABLE IF EXISTS main_tbl;
CREATE TABLE main_tbl(id bigint, hundred int, thousand int);
CREATE INDEX mt_hundred_ix ON main_tbl(hundred);
CREATE INDEX mt_thousand_ix ON main_tbl(thousand);
VACUUM (ANALYZE) main_tbl;
SET enable_seqscan = off;
EXPLAIN (COSTS OFF)
SELECT m.id, m.hundred, m.thousand
FROM main_tbl m WHERE (m.hundred < 2 OR m.thousand < 3);
Bitmap Heap Scan on public.main_tbl m
Output: id, hundred, thousand
Recheck Cond: ((m.thousand < 3) OR (m.hundred < 2))
-> BitmapOr
-> Bitmap Index Scan on mt_thousand_ix
Index Cond: (m.thousand < 3)
-> Bitmap Index Scan on mt_hundred_ix
Index Cond: (m.hundred < 2)
Conditions on the columns "thousand" and "hundred" changed their places
according to the initial positions defined in the user's SQL.
It isn't okay. I see that users often use the trick of "OR order" to
avoid unnecessary calculations - most frequently, Subplan evaluations.
So, it makes sense to fix.
In the attachment, I have included a quick fix for this issue. Although
many tests returned to their initial (pre-18) state, I added some tests
specifically related to this issue to make it clearer.
--
regards, Andrei Lepikhov
Attachments:
[text/x-patch] clause-permutation-fix.diff (9.1K, ../../[email protected]/2-clause-permutation-fix.diff)
download | inline diff:
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index a43ca16d68..7d8ef0c90f 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -1254,6 +1254,7 @@ group_similar_or_args(PlannerInfo *root, RelOptInfo *rel, RestrictInfo *rinfo)
ListCell *lc;
ListCell *lc2;
List *orargs;
+ bool grouping_happened = false;
List *result = NIL;
Index relid = rel->relid;
@@ -1465,13 +1466,18 @@ group_similar_or_args(PlannerInfo *root, RelOptInfo *rel, RestrictInfo *rinfo)
rinfo->incompatible_relids,
rinfo->outer_relids);
result = lappend(result, subrinfo);
+ grouping_happened = true;
}
group_start = i;
}
}
pfree(matches);
- return result;
+
+ if (grouping_happened)
+ return result;
+ else
+ return orargs;
}
/*
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index bd5f002cf2..c6f84bda64 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -3248,6 +3248,37 @@ SELECT b.relname,
(2 rows)
DROP TABLE concur_temp_tab_1, concur_temp_tab_2, reindex_temp_before;
+-- No OR-clause groupings should happen - no clause permutations in
+-- the filtering conditions we should see in the EXPLAIN.
+EXPLAIN (COSTS OFF)
+SELECT * FROM tenk1 WHERE unique1 < 1 OR hundred < 2;
+ QUERY PLAN
+--------------------------------------------------
+ Bitmap Heap Scan on tenk1
+ Recheck Cond: ((unique1 < 1) OR (hundred < 2))
+ -> BitmapOr
+ -> Bitmap Index Scan on tenk1_unique1
+ Index Cond: (unique1 < 1)
+ -> Bitmap Index Scan on tenk1_hundred
+ Index Cond: (hundred < 2)
+(7 rows)
+
+-- OR clauses on the 'unique' column is grouped. So, clause permutation happened
+-- We see it in the 'Recheck Cond' and order of BitmapOr subpaths: index scan on
+-- the 'hundred' column occupies the first position.
+EXPLAIN (COSTS OFF)
+SELECT * FROM tenk1 WHERE unique1 < 1 OR unique1 < 3 OR hundred < 2;
+ QUERY PLAN
+---------------------------------------------------------------------
+ Bitmap Heap Scan on tenk1
+ Recheck Cond: ((hundred < 2) OR ((unique1 < 1) OR (unique1 < 3)))
+ -> BitmapOr
+ -> Bitmap Index Scan on tenk1_hundred
+ Index Cond: (hundred < 2)
+ -> Bitmap Index Scan on tenk1_unique1
+ Index Cond: (unique1 < ANY ('{1,3}'::integer[]))
+(7 rows)
+
-- Check bitmap scan can consider similar OR arguments separately without
-- grouping them into SAOP.
CREATE TABLE bitmap_split_or (a int NOT NULL, b int NOT NULL, c int NOT NULL);
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index a57bb18c24..a950153d76 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -4333,20 +4333,20 @@ select * from tenk1 a join tenk1 b on
Nested Loop
Join Filter: (((a.unique1 = 1) AND (b.unique1 = 2)) OR ((a.unique2 = 3) AND (b.hundred = 4)))
-> Bitmap Heap Scan on tenk1 b
- Recheck Cond: ((hundred = 4) OR (unique1 = 2))
+ Recheck Cond: ((unique1 = 2) OR (hundred = 4))
-> BitmapOr
- -> Bitmap Index Scan on tenk1_hundred
- Index Cond: (hundred = 4)
-> Bitmap Index Scan on tenk1_unique1
Index Cond: (unique1 = 2)
+ -> Bitmap Index Scan on tenk1_hundred
+ Index Cond: (hundred = 4)
-> Materialize
-> Bitmap Heap Scan on tenk1 a
- Recheck Cond: ((unique2 = 3) OR (unique1 = 1))
+ Recheck Cond: ((unique1 = 1) OR (unique2 = 3))
-> BitmapOr
- -> Bitmap Index Scan on tenk1_unique2
- Index Cond: (unique2 = 3)
-> Bitmap Index Scan on tenk1_unique1
Index Cond: (unique1 = 1)
+ -> Bitmap Index Scan on tenk1_unique2
+ Index Cond: (unique2 = 3)
(17 rows)
explain (costs off)
@@ -4360,12 +4360,12 @@ select * from tenk1 a join tenk1 b on
Filter: ((unique1 = 2) OR (ten = 4))
-> Materialize
-> Bitmap Heap Scan on tenk1 a
- Recheck Cond: ((unique2 = 3) OR (unique1 = 1))
+ Recheck Cond: ((unique1 = 1) OR (unique2 = 3))
-> BitmapOr
- -> Bitmap Index Scan on tenk1_unique2
- Index Cond: (unique2 = 3)
-> Bitmap Index Scan on tenk1_unique1
Index Cond: (unique1 = 1)
+ -> Bitmap Index Scan on tenk1_unique2
+ Index Cond: (unique2 = 3)
(12 rows)
explain (costs off)
@@ -4377,12 +4377,12 @@ select * from tenk1 a join tenk1 b on
Nested Loop
Join Filter: (((a.unique1 = 1) AND (b.unique1 = 2)) OR (((a.unique2 = 3) OR (a.unique2 = 7)) AND (b.hundred = 4)))
-> Bitmap Heap Scan on tenk1 b
- Recheck Cond: ((hundred = 4) OR (unique1 = 2))
+ Recheck Cond: ((unique1 = 2) OR (hundred = 4))
-> BitmapOr
- -> Bitmap Index Scan on tenk1_hundred
- Index Cond: (hundred = 4)
-> Bitmap Index Scan on tenk1_unique1
Index Cond: (unique1 = 2)
+ -> Bitmap Index Scan on tenk1_hundred
+ Index Cond: (hundred = 4)
-> Materialize
-> Bitmap Heap Scan on tenk1 a
Recheck Cond: (((unique2 = 3) OR (unique2 = 7)) OR (unique1 = 1))
@@ -4403,12 +4403,12 @@ select * from tenk1 a join tenk1 b on
Nested Loop
Join Filter: (((a.unique1 = 1) AND (b.unique1 = 2)) OR (((a.unique2 = 3) OR (a.unique2 = 7)) AND (b.hundred = 4)))
-> Bitmap Heap Scan on tenk1 b
- Recheck Cond: ((hundred = 4) OR (unique1 = 2))
+ Recheck Cond: ((unique1 = 2) OR (hundred = 4))
-> BitmapOr
- -> Bitmap Index Scan on tenk1_hundred
- Index Cond: (hundred = 4)
-> Bitmap Index Scan on tenk1_unique1
Index Cond: (unique1 = 2)
+ -> Bitmap Index Scan on tenk1_hundred
+ Index Cond: (hundred = 4)
-> Materialize
-> Bitmap Heap Scan on tenk1 a
Recheck Cond: (((unique2 = 3) OR (unique2 = 7)) OR (unique1 = 1))
diff --git a/src/test/regress/expected/partition_join.out b/src/test/regress/expected/partition_join.out
index 938cedd79a..6101c8c7cf 100644
--- a/src/test/regress/expected/partition_join.out
+++ b/src/test/regress/expected/partition_join.out
@@ -2568,24 +2568,24 @@ where not exists (select 1 from prtx2
-> Seq Scan on prtx1_1
Filter: ((a < 20) AND (c = 91))
-> Bitmap Heap Scan on prtx2_1
- Recheck Cond: ((c = 99) OR (b = (prtx1_1.b + 1)))
+ Recheck Cond: ((b = (prtx1_1.b + 1)) OR (c = 99))
Filter: (a = prtx1_1.a)
-> BitmapOr
- -> Bitmap Index Scan on prtx2_1_c_idx
- Index Cond: (c = 99)
-> Bitmap Index Scan on prtx2_1_b_idx
Index Cond: (b = (prtx1_1.b + 1))
+ -> Bitmap Index Scan on prtx2_1_c_idx
+ Index Cond: (c = 99)
-> Nested Loop Anti Join
-> Seq Scan on prtx1_2
Filter: ((a < 20) AND (c = 91))
-> Bitmap Heap Scan on prtx2_2
- Recheck Cond: ((c = 99) OR (b = (prtx1_2.b + 1)))
+ Recheck Cond: ((b = (prtx1_2.b + 1)) OR (c = 99))
Filter: (a = prtx1_2.a)
-> BitmapOr
- -> Bitmap Index Scan on prtx2_2_c_idx
- Index Cond: (c = 99)
-> Bitmap Index Scan on prtx2_2_b_idx
Index Cond: (b = (prtx1_2.b + 1))
+ -> Bitmap Index Scan on prtx2_2_c_idx
+ Index Cond: (c = 99)
(23 rows)
select * from prtx1
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index be570da08a..51e030294e 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -1355,6 +1355,17 @@ SELECT b.relname,
ORDER BY 1;
DROP TABLE concur_temp_tab_1, concur_temp_tab_2, reindex_temp_before;
+-- No OR-clause groupings should happen - no clause permutations in
+-- the filtering conditions we should see in the EXPLAIN.
+EXPLAIN (COSTS OFF)
+SELECT * FROM tenk1 WHERE unique1 < 1 OR hundred < 2;
+
+-- OR clauses on the 'unique' column is grouped. So, clause permutation happened
+-- We see it in the 'Recheck Cond' and order of BitmapOr subpaths: index scan on
+-- the 'hundred' column occupies the first position.
+EXPLAIN (COSTS OFF)
+SELECT * FROM tenk1 WHERE unique1 < 1 OR unique1 < 3 OR hundred < 2;
+
-- Check bitmap scan can consider similar OR arguments separately without
-- grouping them into SAOP.
CREATE TABLE bitmap_split_or (a int NOT NULL, b int NOT NULL, c int NOT NULL);
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-03-24 10:46 Pavel Borisov <[email protected]>
parent: Andrei Lepikhov <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: Pavel Borisov @ 2025-03-24 10:46 UTC (permalink / raw)
To: Andrei Lepikhov <[email protected]>; +Cc: Alexander Korotkov <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]; a.rybakina <[email protected]>
Hi, Andrei!
On Mon, 24 Mar 2025 at 14:10, Andrei Lepikhov <[email protected]> wrote:
>
> Hi,
>
> Playing with the feature, I found a slightly irritating permutation -
> even if this code doesn't group any clauses, it may permute positions of
> the quals. See:
>
> DROP TABLE IF EXISTS main_tbl;
> CREATE TABLE main_tbl(id bigint, hundred int, thousand int);
> CREATE INDEX mt_hundred_ix ON main_tbl(hundred);
> CREATE INDEX mt_thousand_ix ON main_tbl(thousand);
> VACUUM (ANALYZE) main_tbl;
>
> SET enable_seqscan = off;
> EXPLAIN (COSTS OFF)
> SELECT m.id, m.hundred, m.thousand
> FROM main_tbl m WHERE (m.hundred < 2 OR m.thousand < 3);
>
> Bitmap Heap Scan on public.main_tbl m
> Output: id, hundred, thousand
> Recheck Cond: ((m.thousand < 3) OR (m.hundred < 2))
> -> BitmapOr
> -> Bitmap Index Scan on mt_thousand_ix
> Index Cond: (m.thousand < 3)
> -> Bitmap Index Scan on mt_hundred_ix
> Index Cond: (m.hundred < 2)
>
> Conditions on the columns "thousand" and "hundred" changed their places
> according to the initial positions defined in the user's SQL.
> It isn't okay. I see that users often use the trick of "OR order" to
> avoid unnecessary calculations - most frequently, Subplan evaluations.
> So, it makes sense to fix.
> In the attachment, I have included a quick fix for this issue. Although
> many tests returned to their initial (pre-18) state, I added some tests
> specifically related to this issue to make it clearer.
I looked at your patch and have no objections to it.
However it's clearly stated in PostgreSQL manual that nothing about
the OR order is warranted [1]. So changing OR order was (and is) ok
and any users query tricks about OR order may work and may not work.
[1] https://www.postgresql.org/docs/17/sql-expressions.html#SYNTAX-EXPRESS-EVAL
Regards,
Pavel Borisov
Supabase
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-03-24 12:46 Alena Rybakina <[email protected]>
parent: Pavel Borisov <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: Alena Rybakina @ 2025-03-24 12:46 UTC (permalink / raw)
To: Pavel Borisov <[email protected]>; Andrei Lepikhov <[email protected]>; +Cc: Alexander Korotkov <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]
On 24.03.2025 13:46, Pavel Borisov wrote:
> Hi, Andrei!
>
> On Mon, 24 Mar 2025 at 14:10, Andrei Lepikhov<[email protected]> wrote:
>> Hi,
>>
>> Playing with the feature, I found a slightly irritating permutation -
>> even if this code doesn't group any clauses, it may permute positions of
>> the quals. See:
>>
>> DROP TABLE IF EXISTS main_tbl;
>> CREATE TABLE main_tbl(id bigint, hundred int, thousand int);
>> CREATE INDEX mt_hundred_ix ON main_tbl(hundred);
>> CREATE INDEX mt_thousand_ix ON main_tbl(thousand);
>> VACUUM (ANALYZE) main_tbl;
>>
>> SET enable_seqscan = off;
>> EXPLAIN (COSTS OFF)
>> SELECT m.id, m.hundred, m.thousand
>> FROM main_tbl m WHERE (m.hundred < 2 OR m.thousand < 3);
>>
>> Bitmap Heap Scan on public.main_tbl m
>> Output: id, hundred, thousand
>> Recheck Cond: ((m.thousand < 3) OR (m.hundred < 2))
>> -> BitmapOr
>> -> Bitmap Index Scan on mt_thousand_ix
>> Index Cond: (m.thousand < 3)
>> -> Bitmap Index Scan on mt_hundred_ix
>> Index Cond: (m.hundred < 2)
>>
>> Conditions on the columns "thousand" and "hundred" changed their places
>> according to the initial positions defined in the user's SQL.
>> It isn't okay. I see that users often use the trick of "OR order" to
>> avoid unnecessary calculations - most frequently, Subplan evaluations.
>> So, it makes sense to fix.
>> In the attachment, I have included a quick fix for this issue. Although
>> many tests returned to their initial (pre-18) state, I added some tests
>> specifically related to this issue to make it clearer.
> I looked at your patch and have no objections to it.
>
> However it's clearly stated in PostgreSQL manual that nothing about
> the OR order is warranted [1]. So changing OR order was (and is) ok
> and any users query tricks about OR order may work and may not work.
>
> [1]https://www.postgresql.org/docs/17/sql-expressions.html#SYNTAX-EXPRESS-EVAL
>
I agree with Andrey's changes and think we should fix this, because
otherwise it might be inconvenient.
For example, without this changes we will have to have different test
output files for the same query for different versions of Postres in
extensions if the whole change is only related to the order of column
output for a transformation that was not applied.
--
Regards,
Alena Rybakina
Postgres Professional
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-03-27 23:18 Alexander Korotkov <[email protected]>
parent: Alena Rybakina <[email protected]>
0 siblings, 3 replies; 38+ messages in thread
From: Alexander Korotkov @ 2025-03-27 23:18 UTC (permalink / raw)
To: Alena Rybakina <[email protected]>; +Cc: Pavel Borisov <[email protected]>; Andrei Lepikhov <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]
Hi!
On Mon, Mar 24, 2025 at 2:46 PM Alena Rybakina
<[email protected]> wrote:
> I agree with Andrey's changes and think we should fix this, because otherwise it might be inconvenient.
> For example, without this changes we will have to have different test output files for the same query for different versions of Postres in extensions if the whole change is only related to the order of column output for a transformation that was not applied.
I agree with problem spotted by Andrei: it should be preferred to
preserve original order of clauses as much as possible. The approach
implemented in Andrei's patch seems fragile for me. Original order is
preserved if we didn't find any group. But once we find a single
group original order might be destroyed completely.
The attached patch changes the reordering algorithm of
group_similar_or_args() in the following way. We reorder each group
of similar clauses so that the first item of the group stays in place,
but all the other items are moved after it. So, if there are no
similar clauses, the order of clauses stays the same. When there are
some groups, only required reordering happens while the rest of the
clauses remain in their places.
------
Regards,
Alexander Korotkov
Supabase
Attachments:
[application/octet-stream] v1-0001-Make-group_similar_or_args-reorder-clause-list-as.patch (16.4K, ../../CAPpHfdu71LCrXfwaRot1u_xfx7r92VFvZNRLvQJS79B3XFmBhg@mail.gmail.com/2-v1-0001-Make-group_similar_or_args-reorder-clause-list-as.patch)
download | inline diff:
From 1829564b42450cac8d030d9942ef0a5f26ff86fe Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Fri, 28 Mar 2025 00:15:29 +0200
Subject: [PATCH v1] Make group_similar_or_args() reorder clause list as little
as possible
Currently, group_similar_or_args() permutes original positions of clauses
independently on whether it manages to find any groups of similar clauses.
While we are not providing any strict warranties on saving the original order
of OR-clauses, it is preferred that the original order be modified as little
as possible.
This commit changes the reordering algorithm of group_similar_or_args() in
the following way. We reorder each group of similar clauses so that the
first item of the group stays in place, but all the other items are moved
after it. So, if there are no similar clauses, the order of clauses stays
the same. When there are some groups, only required reordering happens while
the rest of the clauses remain in their places.
Reported-by: Andrei Lepikhov <[email protected]>
Discussion: https://postgr.es/m/3ac7c436-81e1-4191-9caf-b0dd70b51511%40gmail.com
---
src/backend/optimizer/path/indxpath.c | 60 +++++++++++++++++++-
src/test/regress/expected/create_index.out | 18 +++---
src/test/regress/expected/join.out | 52 ++++++++---------
src/test/regress/expected/partition_join.out | 12 ++--
4 files changed, 100 insertions(+), 42 deletions(-)
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index a43ca16d683..2a18bf7c7c3 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -1189,6 +1189,8 @@ typedef struct
Oid inputcollid; /* OID of the OpClause input collation */
int argindex; /* index of the clause in the list of
* arguments */
+ int groupindex; /* value of argindex for the fist clause in
+ * the group of similar clauses */
} OrArgIndexMatch;
/*
@@ -1229,6 +1231,29 @@ or_arg_index_match_cmp(const void *a, const void *b)
return 0;
}
+/*
+ * Another comparison function for OrArgIndexMatch. It sorts groups together
+ * using groupindex. The group items are then sorted by argindex.
+ */
+static int
+or_arg_index_match_cmp_group(const void *a, const void *b)
+{
+ const OrArgIndexMatch *match_a = (const OrArgIndexMatch *) a;
+ const OrArgIndexMatch *match_b = (const OrArgIndexMatch *) b;
+
+ if (match_a->groupindex < match_b->groupindex)
+ return -1;
+ else if (match_a->groupindex > match_b->groupindex)
+ return 1;
+
+ if (match_a->argindex < match_b->argindex)
+ return -1;
+ else if (match_a->argindex > match_b->argindex)
+ return 1;
+
+ return 0;
+}
+
/*
* group_similar_or_args
* Transform incoming OR-restrictinfo into a list of sub-restrictinfos,
@@ -1282,6 +1307,7 @@ group_similar_or_args(PlannerInfo *root, RelOptInfo *rel, RestrictInfo *rinfo)
i++;
matches[i].argindex = i;
+ matches[i].groupindex = i;
matches[i].indexnum = -1;
matches[i].colnum = -1;
matches[i].opno = InvalidOid;
@@ -1400,9 +1426,41 @@ group_similar_or_args(PlannerInfo *root, RelOptInfo *rel, RestrictInfo *rinfo)
return orargs;
}
- /* Sort clauses to make similar clauses go together */
+ /*
+ * Sort clauses to make similar clauses go together. But at the same
+ * time, we would like to change the order of clauses as little as
+ * possible. To have this property, we reorder each group of similar
+ * clauses so that the first item of the group stays in place, but all the
+ * other items are moved after it. So, if there are no similar clauses,
+ * the order of clauses stays the same. When there are some groups, only
+ * required reordering happens while the rest of the clauses remain in
+ * their places. That is achieved by assigning a 'groupindex' to each
+ * clause: the number of the first item in the group in the original
+ * clause list.
+ */
qsort(matches, n, sizeof(OrArgIndexMatch), or_arg_index_match_cmp);
+ /* Assign groupindex to the sorted clauses */
+ for (i = 1; i < n; i++)
+ {
+ /*
+ * When two clauses are similar and should belong to the same group,
+ * copy the 'groupindex' from the previous clause. Given we are
+ * considering clauses in direct order, all the clauses would have a
+ * 'groupindex' equal to the 'groupindex' of the first clause in the
+ * group.
+ */
+ if ((matches[i].indexnum == matches[i - 1].indexnum ||
+ matches[i].colnum == matches[i - 1].colnum ||
+ matches[i].opno == matches[i - 1].opno ||
+ matches[i].inputcollid == matches[i - 1].inputcollid) &&
+ matches[i].indexnum != -1)
+ matches[i].groupindex = matches[i - 1].groupindex;
+ }
+
+ /* Resort clauses first by groupindex then by argindex */
+ qsort(matches, n, sizeof(OrArgIndexMatch), or_arg_index_match_cmp_group);
+
/*
* Group similar clauses into single sub-restrictinfo. Side effect: the
* resulting list of restrictions will be sorted by indexnum and colnum.
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index bd5f002cf20..3bd18bbbac9 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -1899,13 +1899,13 @@ SELECT * FROM tenk1
QUERY PLAN
-------------------------------------------------------------------------------------------------------------------------------------------
Bitmap Heap Scan on tenk1
- Recheck Cond: (((thousand = 42) AND (tenthous IS NULL)) OR ((thousand = 42) AND ((tenthous = 1) OR (tenthous = 3) OR (tenthous = 42))))
+ Recheck Cond: (((thousand = 42) AND ((tenthous = 1) OR (tenthous = 3) OR (tenthous = 42))) OR ((thousand = 42) AND (tenthous IS NULL)))
Filter: ((tenthous = 1) OR (tenthous = 3) OR (tenthous = 42) OR (tenthous IS NULL))
-> BitmapOr
- -> Bitmap Index Scan on tenk1_thous_tenthous
- Index Cond: ((thousand = 42) AND (tenthous IS NULL))
-> Bitmap Index Scan on tenk1_thous_tenthous
Index Cond: ((thousand = 42) AND (tenthous = ANY ('{1,3,42}'::integer[])))
+ -> Bitmap Index Scan on tenk1_thous_tenthous
+ Index Cond: ((thousand = 42) AND (tenthous IS NULL))
(8 rows)
EXPLAIN (COSTS OFF)
@@ -1938,13 +1938,13 @@ SELECT * FROM tenk1
QUERY PLAN
-----------------------------------------------------------------------------------------------------------------------------------------------------
Bitmap Heap Scan on tenk1
- Recheck Cond: (((thousand = 42) AND ((tenthous = '3'::bigint) OR (tenthous = '42'::bigint))) OR ((thousand = 42) AND (tenthous = '1'::smallint)))
+ Recheck Cond: (((thousand = 42) AND (tenthous = '1'::smallint)) OR ((thousand = 42) AND ((tenthous = '3'::bigint) OR (tenthous = '42'::bigint))))
Filter: ((tenthous = '1'::smallint) OR (tenthous = '3'::bigint) OR (tenthous = '42'::bigint))
-> BitmapOr
- -> Bitmap Index Scan on tenk1_thous_tenthous
- Index Cond: ((thousand = 42) AND (tenthous = ANY ('{3,42}'::bigint[])))
-> Bitmap Index Scan on tenk1_thous_tenthous
Index Cond: ((thousand = 42) AND (tenthous = '1'::smallint))
+ -> Bitmap Index Scan on tenk1_thous_tenthous
+ Index Cond: ((thousand = 42) AND (tenthous = ANY ('{3,42}'::bigint[])))
(8 rows)
EXPLAIN (COSTS OFF)
@@ -2129,16 +2129,16 @@ SELECT count(*) FROM tenk1
---------------------------------------------------------------------------------------------------------------------------
Aggregate
-> Bitmap Heap Scan on tenk1
- Recheck Cond: ((hundred = 42) AND (((thousand = 99) AND (tenthous = 2)) OR ((thousand = 42) OR (thousand = 41))))
+ Recheck Cond: ((hundred = 42) AND (((thousand = 42) OR (thousand = 41)) OR ((thousand = 99) AND (tenthous = 2))))
Filter: ((thousand = 42) OR (thousand = 41) OR ((thousand = 99) AND (tenthous = 2)))
-> BitmapAnd
-> Bitmap Index Scan on tenk1_hundred
Index Cond: (hundred = 42)
-> BitmapOr
- -> Bitmap Index Scan on tenk1_thous_tenthous
- Index Cond: ((thousand = 99) AND (tenthous = 2))
-> Bitmap Index Scan on tenk1_thous_tenthous
Index Cond: (thousand = ANY ('{42,41}'::integer[]))
+ -> Bitmap Index Scan on tenk1_thous_tenthous
+ Index Cond: ((thousand = 99) AND (tenthous = 2))
(12 rows)
SELECT count(*) FROM tenk1
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index a57bb18c24f..14da5708451 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -4333,20 +4333,20 @@ select * from tenk1 a join tenk1 b on
Nested Loop
Join Filter: (((a.unique1 = 1) AND (b.unique1 = 2)) OR ((a.unique2 = 3) AND (b.hundred = 4)))
-> Bitmap Heap Scan on tenk1 b
- Recheck Cond: ((hundred = 4) OR (unique1 = 2))
+ Recheck Cond: ((unique1 = 2) OR (hundred = 4))
-> BitmapOr
- -> Bitmap Index Scan on tenk1_hundred
- Index Cond: (hundred = 4)
-> Bitmap Index Scan on tenk1_unique1
Index Cond: (unique1 = 2)
+ -> Bitmap Index Scan on tenk1_hundred
+ Index Cond: (hundred = 4)
-> Materialize
-> Bitmap Heap Scan on tenk1 a
- Recheck Cond: ((unique2 = 3) OR (unique1 = 1))
+ Recheck Cond: ((unique1 = 1) OR (unique2 = 3))
-> BitmapOr
- -> Bitmap Index Scan on tenk1_unique2
- Index Cond: (unique2 = 3)
-> Bitmap Index Scan on tenk1_unique1
Index Cond: (unique1 = 1)
+ -> Bitmap Index Scan on tenk1_unique2
+ Index Cond: (unique2 = 3)
(17 rows)
explain (costs off)
@@ -4360,12 +4360,12 @@ select * from tenk1 a join tenk1 b on
Filter: ((unique1 = 2) OR (ten = 4))
-> Materialize
-> Bitmap Heap Scan on tenk1 a
- Recheck Cond: ((unique2 = 3) OR (unique1 = 1))
+ Recheck Cond: ((unique1 = 1) OR (unique2 = 3))
-> BitmapOr
- -> Bitmap Index Scan on tenk1_unique2
- Index Cond: (unique2 = 3)
-> Bitmap Index Scan on tenk1_unique1
Index Cond: (unique1 = 1)
+ -> Bitmap Index Scan on tenk1_unique2
+ Index Cond: (unique2 = 3)
(12 rows)
explain (costs off)
@@ -4377,21 +4377,21 @@ select * from tenk1 a join tenk1 b on
Nested Loop
Join Filter: (((a.unique1 = 1) AND (b.unique1 = 2)) OR (((a.unique2 = 3) OR (a.unique2 = 7)) AND (b.hundred = 4)))
-> Bitmap Heap Scan on tenk1 b
- Recheck Cond: ((hundred = 4) OR (unique1 = 2))
+ Recheck Cond: ((unique1 = 2) OR (hundred = 4))
-> BitmapOr
- -> Bitmap Index Scan on tenk1_hundred
- Index Cond: (hundred = 4)
-> Bitmap Index Scan on tenk1_unique1
Index Cond: (unique1 = 2)
+ -> Bitmap Index Scan on tenk1_hundred
+ Index Cond: (hundred = 4)
-> Materialize
-> Bitmap Heap Scan on tenk1 a
- Recheck Cond: (((unique2 = 3) OR (unique2 = 7)) OR (unique1 = 1))
+ Recheck Cond: ((unique1 = 1) OR ((unique2 = 3) OR (unique2 = 7)))
Filter: ((unique1 = 1) OR (unique2 = 3) OR (unique2 = 7))
-> BitmapOr
- -> Bitmap Index Scan on tenk1_unique2
- Index Cond: (unique2 = ANY ('{3,7}'::integer[]))
-> Bitmap Index Scan on tenk1_unique1
Index Cond: (unique1 = 1)
+ -> Bitmap Index Scan on tenk1_unique2
+ Index Cond: (unique2 = ANY ('{3,7}'::integer[]))
(18 rows)
explain (costs off)
@@ -4403,21 +4403,21 @@ select * from tenk1 a join tenk1 b on
Nested Loop
Join Filter: (((a.unique1 = 1) AND (b.unique1 = 2)) OR (((a.unique2 = 3) OR (a.unique2 = 7)) AND (b.hundred = 4)))
-> Bitmap Heap Scan on tenk1 b
- Recheck Cond: ((hundred = 4) OR (unique1 = 2))
+ Recheck Cond: ((unique1 = 2) OR (hundred = 4))
-> BitmapOr
- -> Bitmap Index Scan on tenk1_hundred
- Index Cond: (hundred = 4)
-> Bitmap Index Scan on tenk1_unique1
Index Cond: (unique1 = 2)
+ -> Bitmap Index Scan on tenk1_hundred
+ Index Cond: (hundred = 4)
-> Materialize
-> Bitmap Heap Scan on tenk1 a
- Recheck Cond: (((unique2 = 3) OR (unique2 = 7)) OR (unique1 = 1))
+ Recheck Cond: ((unique1 = 1) OR ((unique2 = 3) OR (unique2 = 7)))
Filter: ((unique1 = 1) OR (unique2 = 3) OR (unique2 = 7))
-> BitmapOr
- -> Bitmap Index Scan on tenk1_unique2
- Index Cond: (unique2 = ANY ('{3,7}'::integer[]))
-> Bitmap Index Scan on tenk1_unique1
Index Cond: (unique1 = 1)
+ -> Bitmap Index Scan on tenk1_unique2
+ Index Cond: (unique2 = ANY ('{3,7}'::integer[]))
(18 rows)
explain (costs off)
@@ -4431,15 +4431,15 @@ select * from tenk1 a join tenk1 b on
-> Seq Scan on tenk1 b
-> Materialize
-> Bitmap Heap Scan on tenk1 a
- Recheck Cond: (((unique2 = 3) OR (unique2 = 7)) OR ((unique1 = 3) OR (unique1 = 1)) OR (unique1 < 20))
+ Recheck Cond: ((unique1 < 20) OR ((unique1 = 3) OR (unique1 = 1)) OR ((unique2 = 3) OR (unique2 = 7)))
Filter: ((unique1 < 20) OR (unique1 = 3) OR (unique1 = 1) OR (unique2 = 3) OR (unique2 = 7))
-> BitmapOr
- -> Bitmap Index Scan on tenk1_unique2
- Index Cond: (unique2 = ANY ('{3,7}'::integer[]))
- -> Bitmap Index Scan on tenk1_unique1
- Index Cond: (unique1 = ANY ('{3,1}'::integer[]))
-> Bitmap Index Scan on tenk1_unique1
Index Cond: (unique1 < 20)
+ -> Bitmap Index Scan on tenk1_unique1
+ Index Cond: (unique1 = ANY ('{3,1}'::integer[]))
+ -> Bitmap Index Scan on tenk1_unique2
+ Index Cond: (unique2 = ANY ('{3,7}'::integer[]))
(14 rows)
--
diff --git a/src/test/regress/expected/partition_join.out b/src/test/regress/expected/partition_join.out
index 938cedd79ad..6101c8c7cf1 100644
--- a/src/test/regress/expected/partition_join.out
+++ b/src/test/regress/expected/partition_join.out
@@ -2568,24 +2568,24 @@ where not exists (select 1 from prtx2
-> Seq Scan on prtx1_1
Filter: ((a < 20) AND (c = 91))
-> Bitmap Heap Scan on prtx2_1
- Recheck Cond: ((c = 99) OR (b = (prtx1_1.b + 1)))
+ Recheck Cond: ((b = (prtx1_1.b + 1)) OR (c = 99))
Filter: (a = prtx1_1.a)
-> BitmapOr
- -> Bitmap Index Scan on prtx2_1_c_idx
- Index Cond: (c = 99)
-> Bitmap Index Scan on prtx2_1_b_idx
Index Cond: (b = (prtx1_1.b + 1))
+ -> Bitmap Index Scan on prtx2_1_c_idx
+ Index Cond: (c = 99)
-> Nested Loop Anti Join
-> Seq Scan on prtx1_2
Filter: ((a < 20) AND (c = 91))
-> Bitmap Heap Scan on prtx2_2
- Recheck Cond: ((c = 99) OR (b = (prtx1_2.b + 1)))
+ Recheck Cond: ((b = (prtx1_2.b + 1)) OR (c = 99))
Filter: (a = prtx1_2.a)
-> BitmapOr
- -> Bitmap Index Scan on prtx2_2_c_idx
- Index Cond: (c = 99)
-> Bitmap Index Scan on prtx2_2_b_idx
Index Cond: (b = (prtx1_2.b + 1))
+ -> Bitmap Index Scan on prtx2_2_c_idx
+ Index Cond: (c = 99)
(23 rows)
select * from prtx1
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-03-28 10:47 Pavel Borisov <[email protected]>
parent: Alexander Korotkov <[email protected]>
2 siblings, 0 replies; 38+ messages in thread
From: Pavel Borisov @ 2025-03-28 10:47 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; +Cc: Alena Rybakina <[email protected]>; Andrei Lepikhov <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]
Hi, Alexander!
d4378c0005e61b1bb7
On Fri, 28 Mar 2025 at 03:18, Alexander Korotkov <[email protected]> wrote:
>
> Hi!
>
> On Mon, Mar 24, 2025 at 2:46 PM Alena Rybakina
> <[email protected]> wrote:
> > I agree with Andrey's changes and think we should fix this, because otherwise it might be inconvenient.
> > For example, without this changes we will have to have different test output files for the same query for different versions of Postres in extensions if the whole change is only related to the order of column output for a transformation that was not applied.
>
> I agree with problem spotted by Andrei: it should be preferred to
> preserve original order of clauses as much as possible. The approach
> implemented in Andrei's patch seems fragile for me. Original order is
> preserved if we didn't find any group. But once we find a single
> group original order might be destroyed completely.
>
> The attached patch changes the reordering algorithm of
> group_similar_or_args() in the following way. We reorder each group
> of similar clauses so that the first item of the group stays in place,
> but all the other items are moved after it. So, if there are no
> similar clauses, the order of clauses stays the same. When there are
> some groups, only required reordering happens while the rest of the
> clauses remain in their places.
With your patch, I've re-checked that there are no changes in the
order of evaluation in plans compared to d4378c0005e61b1bb7
It might be good to also include Andrei's test from his last patch. i.e:
+-- No OR-clause groupings should happen - no clause permutations in
+-- the filtering conditions we should see in the EXPLAIN.
+EXPLAIN (COSTS OFF)
+SELECT * FROM tenk1 WHERE unique1 < 1 OR hundred < 2;
+
+-- OR clauses on the 'unique' column is grouped. So, clause
permutation happened
+-- We see it in the 'Recheck Cond' and order of BitmapOr subpaths:
index scan on
+-- the 'hundred' column occupies the first position.
+EXPLAIN (COSTS OFF)
+SELECT * FROM tenk1 WHERE unique1 < 1 OR unique1 < 3 OR hundred < 2;
I propose small changes for comments:
s/To have this property,/To do so,/g
s/in place, but all the/in place, and all the/g
s/some groups, only/some groups,/g
s/Resort/Re-sort/п
The patch overall looks good to me.
Regards,
Pavel Borisov
Supabase
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-03-28 11:32 Andrei Lepikhov <[email protected]>
parent: Alexander Korotkov <[email protected]>
2 siblings, 1 reply; 38+ messages in thread
From: Andrei Lepikhov @ 2025-03-28 11:32 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; Alena Rybakina <[email protected]>; +Cc: Pavel Borisov <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]
On 3/28/25 00:18, Alexander Korotkov wrote:
> The attached patch changes the reordering algorithm of
> group_similar_or_args() in the following way. We reorder each group
> of similar clauses so that the first item of the group stays in place,
> but all the other items are moved after it. So, if there are no
> similar clauses, the order of clauses stays the same. When there are
> some groups, only required reordering happens while the rest of the
> clauses remain in their places.
The patch looks good to me from a technical perspective. But it seems
like an overkill, isn't it?
You introduce additional CPU-consuming operations in the planning OR
operations.
My point is: 1) as Pavel has mentioned, Postgres doesn't guarantee the
evaluation/output order of the clauses at all. 2) we need that to keep
regression tests stable (don't forget extensions' and forks' developers
too). But it should be done once if we have no fluidity in OR clauses
order in general.
The trade-off with tricky query writers and regression tests may be
preserving the order until OR->ANY has happened. If it has happened,
just ensure the order is determined somehow. Except that, any other
spending on CPU cycles seems too expensive.
--
regards, Andrei Lepikhov
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-03-28 11:59 Alexander Korotkov <[email protected]>
parent: Andrei Lepikhov <[email protected]>
0 siblings, 1 reply; 38+ messages in thread
From: Alexander Korotkov @ 2025-03-28 11:59 UTC (permalink / raw)
To: Andrei Lepikhov <[email protected]>; +Cc: Alena Rybakina <[email protected]>; Pavel Borisov <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]
On Fri, Mar 28, 2025 at 1:32 PM Andrei Lepikhov <[email protected]> wrote:
> On 3/28/25 00:18, Alexander Korotkov wrote:
> > The attached patch changes the reordering algorithm of
> > group_similar_or_args() in the following way. We reorder each group
> > of similar clauses so that the first item of the group stays in place,
> > but all the other items are moved after it. So, if there are no
> > similar clauses, the order of clauses stays the same. When there are
> > some groups, only required reordering happens while the rest of the
> > clauses remain in their places.
> The patch looks good to me from a technical perspective. But it seems
> like an overkill, isn't it?
> You introduce additional CPU-consuming operations in the planning OR
> operations.
I don't think this is going to be CPU-consuming. I don't think this
is going to be measurable. This patch introduces one additional pass
over array of OrArgIndexMatch'es, and qsort of them. I think I've
seen places where we spend quadratic time over the number of
OR-clauses. Even calls of match_index_to_operand() for every clause
and every index look way more expensive.
> My point is: 1) as Pavel has mentioned, Postgres doesn't guarantee the
> evaluation/output order of the clauses at all. 2) we need that to keep
> regression tests stable (don't forget extensions' and forks' developers
> too). But it should be done once if we have no fluidity in OR clauses
> order in general.
> The trade-off with tricky query writers and regression tests may be
> preserving the order until OR->ANY has happened. If it has happened,
> just ensure the order is determined somehow. Except that, any other
> spending on CPU cycles seems too expensive.
I think my patch gives better determinism too. For instance, output
order doesn't depend on order of indexes in rel->indexlist.
------
Regards,
Alexander Korotkov
Supabase
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-03-28 12:15 Andrei Lepikhov <[email protected]>
parent: Alexander Korotkov <[email protected]>
0 siblings, 0 replies; 38+ messages in thread
From: Andrei Lepikhov @ 2025-03-28 12:15 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; +Cc: Alena Rybakina <[email protected]>; Pavel Borisov <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]
On 3/28/25 12:59, Alexander Korotkov wrote:
> On Fri, Mar 28, 2025 at 1:32 PM Andrei Lepikhov <[email protected]> wrote:
> I don't think this is going to be CPU-consuming. I don't think this
> is going to be measurable. This patch introduces one additional pass
> over array of OrArgIndexMatch'es, and qsort of them. I think I've
> seen places where we spend quadratic time over the number of
> OR-clauses. Even calls of match_index_to_operand() for every clause
> and every index look way more expensive.
Ok, I have no more objections.
> I think my patch gives better determinism too. For instance, output
> order doesn't depend on order of indexes in rel->indexlist.
Nice!
--
regards, Andrei Lepikhov
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-03-28 12:23 Alena Rybakina <[email protected]>
parent: Alexander Korotkov <[email protected]>
2 siblings, 1 reply; 38+ messages in thread
From: Alena Rybakina @ 2025-03-28 12:23 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; +Cc: Pavel Borisov <[email protected]>; Andrei Lepikhov <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]
On 28.03.2025 02:18, Alexander Korotkov wrote:
> Hi!
>
> On Mon, Mar 24, 2025 at 2:46 PM Alena Rybakina
> <[email protected]> wrote:
>> I agree with Andrey's changes and think we should fix this, because otherwise it might be inconvenient.
>> For example, without this changes we will have to have different test output files for the same query for different versions of Postres in extensions if the whole change is only related to the order of column output for a transformation that was not applied.
> I agree with problem spotted by Andrei: it should be preferred to
> preserve original order of clauses as much as possible. The approach
> implemented in Andrei's patch seems fragile for me. Original order is
> preserved if we didn't find any group. But once we find a single
> group original order might be destroyed completely.
>
> The attached patch changes the reordering algorithm of
> group_similar_or_args() in the following way. We reorder each group
> of similar clauses so that the first item of the group stays in place,
> but all the other items are moved after it. So, if there are no
> similar clauses, the order of clauses stays the same. When there are
> some groups, only required reordering happens while the rest of the
> clauses remain in their places.
>
I agree with your code in general, but to be honest, double qsort
confused me a little.
I understood why it is needed - we need to sort the elements so that
they stand next to each other if they can be assigned to the same group,
and then sort the groups themselves according to the set identifier.
I may be missing something, but in the worst case we can get the
complexity of qsort O(n^2), right? And I saw the letter where you
mentioned this, but it is possible to use mergesort algorithm instead of
qsort, which in the worst case gives n * O(n) complexity?
--
Regards,
Alena Rybakina
Postgres Professional
^ permalink raw reply [nested|flat] 38+ messages in thread
* Re: POC, WIP: OR-clause support for indexes
@ 2025-03-28 12:31 Alena Rybakina <[email protected]>
parent: Alena Rybakina <[email protected]>
0 siblings, 0 replies; 38+ messages in thread
From: Alena Rybakina @ 2025-03-28 12:31 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; +Cc: Pavel Borisov <[email protected]>; Andrei Lepikhov <[email protected]>; Alexander Lakhin <[email protected]>; [email protected]
On 28.03.2025 15:23, Alena Rybakina wrote:
>
> I agree with your code in general, but to be honest, double qsort
> confused me a little.
>
> I understood why it is needed - we need to sort the elements so that
> they stand next to each other if they can be assigned to the same
> group, and then sort the groups themselves according to the set
> identifier.
>
> I may be missing something, but in the worst case we can get the
> complexity of qsort O(n^2), right? And I saw the letter where you
> mentioned this, but it is possible to use mergesort algorithm instead
> of qsort, which in the worst case gives n * O(n) complexity?
>
No, sorry, I was wrong here and it is impossible to rewrite it this way.
I apologize, I agree with your code.
--
Regards,
Alena Rybakina
Postgres Professional
^ permalink raw reply [nested|flat] 38+ messages in thread
end of thread, other threads:[~2025-03-28 12:31 UTC | newest]
Thread overview: 38+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-04-22 11:58 [PATCH 2/5] Allow TAP test to excecise tablespace. Kyotaro Horiguchi <[email protected]>
2023-06-14 17:54 [PATCH v2 1/2] partial revert of ff9618e82a Nathan Bossart <[email protected]>
2024-11-28 19:33 Re: POC, WIP: OR-clause support for indexes Alena Rybakina <[email protected]>
2024-11-29 00:04 ` Re: POC, WIP: OR-clause support for indexes Alexander Korotkov <[email protected]>
2024-11-29 05:10 ` Re: POC, WIP: OR-clause support for indexes Andrei Lepikhov <[email protected]>
2024-11-29 05:51 ` Re: POC, WIP: OR-clause support for indexes Alena Rybakina <[email protected]>
2024-11-29 07:54 ` Re: POC, WIP: OR-clause support for indexes Alexander Korotkov <[email protected]>
2025-01-12 18:39 ` Re: POC, WIP: OR-clause support for indexes Alexander Korotkov <[email protected]>
2025-01-12 20:38 ` Re: POC, WIP: OR-clause support for indexes Alena Rybakina <[email protected]>
2025-01-13 00:47 ` Re: POC, WIP: OR-clause support for indexes Yura Sokolov <[email protected]>
2025-01-13 01:06 ` Re: POC, WIP: OR-clause support for indexes Alexander Korotkov <[email protected]>
2025-01-13 03:39 ` Re: POC, WIP: OR-clause support for indexes Andrei Lepikhov <[email protected]>
2025-01-15 08:24 ` Re: POC, WIP: OR-clause support for indexes Andrei Lepikhov <[email protected]>
2025-01-25 05:04 ` Re: POC, WIP: OR-clause support for indexes Alexander Korotkov <[email protected]>
2025-01-27 08:52 ` Re: POC, WIP: OR-clause support for indexes Andrei Lepikhov <[email protected]>
2025-01-27 09:50 ` Re: POC, WIP: OR-clause support for indexes Alexander Korotkov <[email protected]>
2025-01-28 04:36 ` Re: POC, WIP: OR-clause support for indexes Andrei Lepikhov <[email protected]>
2025-01-28 08:42 ` Re: POC, WIP: OR-clause support for indexes Andrei Lepikhov <[email protected]>
2025-01-30 13:22 ` Re: POC, WIP: OR-clause support for indexes Pavel Borisov <[email protected]>
2025-02-02 17:57 ` Re: POC, WIP: OR-clause support for indexes Alexander Korotkov <[email protected]>
2025-02-03 06:24 ` Re: POC, WIP: OR-clause support for indexes Andrei Lepikhov <[email protected]>
2025-02-02 17:58 ` Re: POC, WIP: OR-clause support for indexes Alexander Korotkov <[email protected]>
2025-01-31 14:31 ` Re: POC, WIP: OR-clause support for indexes Alena Rybakina <[email protected]>
2025-02-02 18:00 ` Re: POC, WIP: OR-clause support for indexes Alexander Korotkov <[email protected]>
2025-02-03 10:22 ` Re: POC, WIP: OR-clause support for indexes Alena Rybakina <[email protected]>
2025-02-03 11:32 ` Re: POC, WIP: OR-clause support for indexes Alexander Korotkov <[email protected]>
2025-02-03 11:54 ` Re: POC, WIP: OR-clause support for indexes Alena Rybakina <[email protected]>
2025-02-03 12:16 ` Re: POC, WIP: OR-clause support for indexes Pavel Borisov <[email protected]>
2025-03-24 10:10 ` Re: POC, WIP: OR-clause support for indexes Andrei Lepikhov <[email protected]>
2025-03-24 10:46 ` Re: POC, WIP: OR-clause support for indexes Pavel Borisov <[email protected]>
2025-03-24 12:46 ` Re: POC, WIP: OR-clause support for indexes Alena Rybakina <[email protected]>
2025-03-27 23:18 ` Re: POC, WIP: OR-clause support for indexes Alexander Korotkov <[email protected]>
2025-03-28 10:47 ` Re: POC, WIP: OR-clause support for indexes Pavel Borisov <[email protected]>
2025-03-28 11:32 ` Re: POC, WIP: OR-clause support for indexes Andrei Lepikhov <[email protected]>
2025-03-28 11:59 ` Re: POC, WIP: OR-clause support for indexes Alexander Korotkov <[email protected]>
2025-03-28 12:15 ` Re: POC, WIP: OR-clause support for indexes Andrei Lepikhov <[email protected]>
2025-03-28 12:23 ` Re: POC, WIP: OR-clause support for indexes Alena Rybakina <[email protected]>
2025-03-28 12:31 ` Re: POC, WIP: OR-clause support for indexes Alena Rybakina <[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