public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v2 1/2] Be strict in numeric parameters on command line
18+ messages / 8 participants
[nested] [flat]
* [PATCH v2 1/2] Be strict in numeric parameters on command line
@ 2021-07-08 06:08 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 18+ messages in thread
From: Kyotaro Horiguchi @ 2021-07-08 06:08 UTC (permalink / raw)
Some numeric command line parameters are tolerant of valid values
followed by garbage like "123xyz". Be strict to reject such invalid
values. Do the same for psql meta command parameters.
---
src/bin/pg_amcheck/pg_amcheck.c | 6 ++-
src/bin/pg_basebackup/pg_basebackup.c | 13 +++--
src/bin/pg_basebackup/pg_receivewal.c | 18 +++++--
src/bin/pg_basebackup/pg_recvlogical.c | 17 +++++--
src/bin/pg_checksums/pg_checksums.c | 7 ++-
src/bin/pg_ctl/pg_ctl.c | 18 ++++++-
src/bin/pg_dump/pg_dump.c | 39 ++++++++-------
src/bin/pg_dump/pg_restore.c | 17 ++++---
src/bin/pg_upgrade/option.c | 21 ++++++--
src/bin/pgbench/pgbench.c | 66 ++++++++++++++++----------
src/bin/psql/command.c | 52 ++++++++++++++++++--
src/bin/scripts/reindexdb.c | 10 ++--
src/bin/scripts/vacuumdb.c | 23 +++++----
13 files changed, 219 insertions(+), 88 deletions(-)
diff --git a/src/bin/pg_amcheck/pg_amcheck.c b/src/bin/pg_amcheck/pg_amcheck.c
index 4bde16fb4b..71a82f9b75 100644
--- a/src/bin/pg_amcheck/pg_amcheck.c
+++ b/src/bin/pg_amcheck/pg_amcheck.c
@@ -17,6 +17,7 @@
#include "catalog/pg_am_d.h"
#include "catalog/pg_namespace_d.h"
#include "common/logging.h"
+#include "common/string.h"
#include "common/username.h"
#include "fe_utils/cancel.h"
#include "fe_utils/option_utils.h"
@@ -326,8 +327,9 @@ main(int argc, char *argv[])
append_btree_pattern(&opts.exclude, optarg, encoding);
break;
case 'j':
- opts.jobs = atoi(optarg);
- if (opts.jobs < 1)
+ errno = 0;
+ opts.jobs = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE || opts.jobs < 1)
{
pg_log_error("number of parallel jobs must be at least 1");
exit(1);
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 8bb0acf498..29be95b96a 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -2287,6 +2287,8 @@ main(int argc, char **argv)
while ((c = getopt_long(argc, argv, "CD:F:r:RS:T:X:l:nNzZ:d:c:h:p:U:s:wWkvP",
long_options, &option_index)) != -1)
{
+ char *endptr;
+
switch (c)
{
case 'C':
@@ -2371,8 +2373,10 @@ main(int argc, char **argv)
#endif
break;
case 'Z':
- compresslevel = atoi(optarg);
- if (compresslevel < 0 || compresslevel > 9)
+ errno = 0;
+ compresslevel = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE ||
+ compresslevel < 0 || compresslevel > 9)
{
pg_log_error("invalid compression level \"%s\"", optarg);
exit(1);
@@ -2409,8 +2413,9 @@ main(int argc, char **argv)
dbgetpassword = 1;
break;
case 's':
- standby_message_timeout = atoi(optarg) * 1000;
- if (standby_message_timeout < 0)
+ errno = 0;
+ standby_message_timeout = strtoint(optarg, &endptr, 10) * 1000;
+ if (*endptr || errno == ERANGE || standby_message_timeout < 0)
{
pg_log_error("invalid status interval \"%s\"", optarg);
exit(1);
diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c
index c1334fad35..7fef925b99 100644
--- a/src/bin/pg_basebackup/pg_receivewal.c
+++ b/src/bin/pg_basebackup/pg_receivewal.c
@@ -22,6 +22,7 @@
#include "access/xlog_internal.h"
#include "common/file_perm.h"
#include "common/logging.h"
+#include "common/string.h"
#include "getopt_long.h"
#include "libpq-fe.h"
#include "receivelog.h"
@@ -520,6 +521,8 @@ main(int argc, char **argv)
while ((c = getopt_long(argc, argv, "D:d:E:h:p:U:s:S:nwWvZ:",
long_options, &option_index)) != -1)
{
+ char *endptr;
+
switch (c)
{
case 'D':
@@ -532,7 +535,9 @@ main(int argc, char **argv)
dbhost = pg_strdup(optarg);
break;
case 'p':
- if (atoi(optarg) <= 0)
+ errno = 0;
+ if (strtoint(optarg, &endptr, 10) <= 0 ||
+ *endptr || errno == ERANGE)
{
pg_log_error("invalid port number \"%s\"", optarg);
exit(1);
@@ -549,8 +554,9 @@ main(int argc, char **argv)
dbgetpassword = 1;
break;
case 's':
- standby_message_timeout = atoi(optarg) * 1000;
- if (standby_message_timeout < 0)
+ errno = 0;
+ standby_message_timeout = strtoint(optarg, &endptr, 10) * 1000;
+ if (*endptr || errno == ERANGE || standby_message_timeout < 0)
{
pg_log_error("invalid status interval \"%s\"", optarg);
exit(1);
@@ -574,8 +580,10 @@ main(int argc, char **argv)
verbose++;
break;
case 'Z':
- compresslevel = atoi(optarg);
- if (compresslevel < 0 || compresslevel > 9)
+ errno = 0;
+ compresslevel = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE ||
+ compresslevel < 0 || compresslevel > 9)
{
pg_log_error("invalid compression level \"%s\"", optarg);
exit(1);
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 76bd153fac..7be932d025 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -23,6 +23,7 @@
#include "common/fe_memutils.h"
#include "common/file_perm.h"
#include "common/logging.h"
+#include "common/string.h"
#include "getopt_long.h"
#include "libpq-fe.h"
#include "libpq/pqsignal.h"
@@ -732,6 +733,8 @@ main(int argc, char **argv)
while ((c = getopt_long(argc, argv, "E:f:F:nvtd:h:p:U:wWI:o:P:s:S:",
long_options, &option_index)) != -1)
{
+ char *endptr;
+
switch (c)
{
/* general options */
@@ -739,8 +742,9 @@ main(int argc, char **argv)
outfile = pg_strdup(optarg);
break;
case 'F':
- fsync_interval = atoi(optarg) * 1000;
- if (fsync_interval < 0)
+ errno = 0;
+ fsync_interval = strtoint(optarg, &endptr, 10) * 1000;
+ if (*endptr || errno == ERANGE || fsync_interval < 0)
{
pg_log_error("invalid fsync interval \"%s\"", optarg);
exit(1);
@@ -763,7 +767,9 @@ main(int argc, char **argv)
dbhost = pg_strdup(optarg);
break;
case 'p':
- if (atoi(optarg) <= 0)
+ errno = 0;
+ if (strtoint(optarg, &endptr, 10) <= 0 ||
+ *endptr || errno == ERANGE)
{
pg_log_error("invalid port number \"%s\"", optarg);
exit(1);
@@ -820,8 +826,9 @@ main(int argc, char **argv)
plugin = pg_strdup(optarg);
break;
case 's':
- standby_message_timeout = atoi(optarg) * 1000;
- if (standby_message_timeout < 0)
+ errno = 0;
+ standby_message_timeout = strtoint(optarg, &endptr, 10) * 1000;
+ if (*endptr || errno == ERANGE || standby_message_timeout < 0)
{
pg_log_error("invalid status interval \"%s\"", optarg);
exit(1);
diff --git a/src/bin/pg_checksums/pg_checksums.c b/src/bin/pg_checksums/pg_checksums.c
index 3c326906e2..1c4e5b9d85 100644
--- a/src/bin/pg_checksums/pg_checksums.c
+++ b/src/bin/pg_checksums/pg_checksums.c
@@ -24,6 +24,7 @@
#include "common/file_perm.h"
#include "common/file_utils.h"
#include "common/logging.h"
+#include "common/string.h"
#include "getopt_long.h"
#include "pg_getopt.h"
#include "storage/bufpage.h"
@@ -506,6 +507,8 @@ main(int argc, char *argv[])
while ((c = getopt_long(argc, argv, "cD:deNPf:v", long_options, &option_index)) != -1)
{
+ char *endptr;
+
switch (c)
{
case 'c':
@@ -518,7 +521,9 @@ main(int argc, char *argv[])
mode = PG_MODE_ENABLE;
break;
case 'f':
- if (atoi(optarg) == 0)
+ errno = 0;
+ if (strtoint(optarg, &endptr, 10) == 0
+ || *endptr || errno == ERANGE)
{
pg_log_error("invalid filenode specification, must be numeric: %s", optarg);
exit(1);
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 7985da0a94..d6a39182cf 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -2331,6 +2331,8 @@ main(int argc, char **argv)
/* process command-line options */
while (optind < argc)
{
+ char *endptr;
+
while ((c = getopt_long(argc, argv, "cD:e:l:m:N:o:p:P:sS:t:U:wW",
long_options, &option_index)) != -1)
{
@@ -2396,7 +2398,13 @@ main(int argc, char **argv)
#endif
break;
case 't':
- wait_seconds = atoi(optarg);
+ errno = 0;
+ wait_seconds = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE || wait_seconds < 1)
+ {
+ pg_log_error("invalid timeout value \"%s\", use --no-wait to finish without waiting", optarg);
+ exit(1);
+ }
wait_seconds_arg = true;
break;
case 'U':
@@ -2459,7 +2467,13 @@ main(int argc, char **argv)
}
ctl_command = KILL_COMMAND;
set_sig(argv[++optind]);
- killproc = atol(argv[++optind]);
+ errno = 0;
+ killproc = strtol(argv[++optind], &endptr, 10);
+ if (*endptr || errno == ERANGE || killproc < 0)
+ {
+ pg_log_error("invalid process ID \"%s\"", argv[optind]);
+ exit(1);
+ }
}
#ifdef WIN32
else if (strcmp(argv[optind], "register") == 0)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 321152151d..793f4b3509 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -54,6 +54,7 @@
#include "catalog/pg_trigger_d.h"
#include "catalog/pg_type_d.h"
#include "common/connect.h"
+#include "common/string.h"
#include "dumputils.h"
#include "fe_utils/string_utils.h"
#include "getopt_long.h"
@@ -486,7 +487,19 @@ main(int argc, char **argv)
break;
case 'j': /* number of dump jobs */
- numWorkers = atoi(optarg);
+ errno = 0;
+ numWorkers = strtoint(optarg, &endptr, 10);
+ /*
+ * On Windows we can only have at most MAXIMUM_WAIT_OBJECTS
+ * (= 64 usually) parallel jobs because that's the maximum
+ * limit for the WaitForMultipleObjects() call.
+ */
+ if (*endptr || errno == ERANGE || numWorkers <= 0
+#ifdef WIN32
+ || numWorkers > MAXIMUM_WAIT_OBJECTS
+#endif
+ )
+ fatal("invalid number of parallel jobs %s", optarg);
break;
case 'n': /* include schema(s) */
@@ -549,8 +562,10 @@ main(int argc, char **argv)
break;
case 'Z': /* Compression Level */
- compressLevel = atoi(optarg);
- if (compressLevel < 0 || compressLevel > 9)
+ errno = 0;
+ compressLevel = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE ||
+ compressLevel < 0 || compressLevel > 9)
{
pg_log_error("compression level must be in range 0..9");
exit_nicely(1);
@@ -587,8 +602,10 @@ main(int argc, char **argv)
case 8:
have_extra_float_digits = true;
- extra_float_digits = atoi(optarg);
- if (extra_float_digits < -15 || extra_float_digits > 3)
+ errno = 0;
+ extra_float_digits = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE ||
+ extra_float_digits < -15 || extra_float_digits > 3)
{
pg_log_error("extra_float_digits must be in range -15..3");
exit_nicely(1);
@@ -719,18 +736,6 @@ main(int argc, char **argv)
if (!plainText)
dopt.outputCreateDB = 1;
- /*
- * On Windows we can only have at most MAXIMUM_WAIT_OBJECTS (= 64 usually)
- * parallel jobs because that's the maximum limit for the
- * WaitForMultipleObjects() call.
- */
- if (numWorkers <= 0
-#ifdef WIN32
- || numWorkers > MAXIMUM_WAIT_OBJECTS
-#endif
- )
- fatal("invalid number of parallel jobs");
-
/* Parallel backup only in the directory archive format so far */
if (archiveFormat != archDirectory && numWorkers > 1)
fatal("parallel backup only supported by the directory format");
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index 589b4aed53..285a09aaac 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -39,6 +39,7 @@
*-------------------------------------------------------------------------
*/
#include "postgres_fe.h"
+#include "common/string.h"
#include <ctype.h>
#ifdef HAVE_TERMIOS_H
@@ -151,6 +152,8 @@ main(int argc, char **argv)
while ((c = getopt_long(argc, argv, "acCd:ef:F:h:I:j:lL:n:N:Op:P:RsS:t:T:U:vwWx1",
cmdopts, NULL)) != -1)
{
+ char *endptr;
+
switch (c)
{
case 'a': /* Dump data only */
@@ -181,7 +184,13 @@ main(int argc, char **argv)
break;
case 'j': /* number of restore jobs */
- numWorkers = atoi(optarg);
+ errno = 0;
+ numWorkers = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE || numWorkers <= 0)
+ {
+ pg_log_error("invalid number of parallel jobs");
+ exit(1);
+ }
break;
case 'l': /* Dump the TOC summary */
@@ -344,12 +353,6 @@ main(int argc, char **argv)
exit_nicely(1);
}
- if (numWorkers <= 0)
- {
- pg_log_error("invalid number of parallel jobs");
- exit(1);
- }
-
/* See comments in pg_dump.c */
#ifdef WIN32
if (numWorkers > MAXIMUM_WAIT_OBJECTS)
diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index 64bbda5650..f96f0d1e2a 100644
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -104,6 +104,8 @@ parseCommandLine(int argc, char *argv[])
while ((option = getopt_long(argc, argv, "d:D:b:B:cj:ko:O:p:P:rs:U:v",
long_options, &optindex)) != -1)
{
+ char *endptr;
+
switch (option)
{
case 'b':
@@ -127,7 +129,12 @@ parseCommandLine(int argc, char *argv[])
break;
case 'j':
- user_opts.jobs = atoi(optarg);
+ errno = 0;
+ user_opts.jobs = strtoint(optarg, &endptr, 10);
+ /**/
+ if (*endptr || errno == ERANGE || user_opts.jobs < 1)
+ pg_fatal("invalid number of jobs %s\n", optarg);
+
break;
case 'k':
@@ -166,13 +173,17 @@ parseCommandLine(int argc, char *argv[])
* supported on all old/new versions (added in PG 9.2).
*/
case 'p':
- if ((old_cluster.port = atoi(optarg)) <= 0)
- pg_fatal("invalid old port number\n");
+ errno = 0;
+ if ((old_cluster.port = strtoint(optarg, &endptr, 10)) <= 0 ||
+ *endptr || errno == ERANGE)
+ pg_fatal("invalid old port number %s\n", optarg);
break;
case 'P':
- if ((new_cluster.port = atoi(optarg)) <= 0)
- pg_fatal("invalid new port number\n");
+ errno = 0;
+ if ((new_cluster.port = strtoint(optarg, &endptr, 10)) <= 0 ||
+ *endptr || errno == ERANGE)
+ pg_fatal("invalid new port number %s\n", optarg);
break;
case 'r':
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 4aeccd93af..4020347585 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -5838,6 +5838,7 @@ main(int argc, char **argv)
while ((c = getopt_long(argc, argv, "iI:h:nvp:dqb:SNc:j:Crs:t:T:U:lf:D:F:M:P:R:L:", long_options, &optindex)) != -1)
{
char *script;
+ char *endptr;
switch (c)
{
@@ -5869,8 +5870,9 @@ main(int argc, char **argv)
break;
case 'c':
benchmarking_option_set = true;
- nclients = atoi(optarg);
- if (nclients <= 0)
+ errno = 0;
+ nclients = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE || nclients <= 0)
{
pg_log_fatal("invalid number of clients: \"%s\"", optarg);
exit(1);
@@ -5896,8 +5898,9 @@ main(int argc, char **argv)
break;
case 'j': /* jobs */
benchmarking_option_set = true;
- nthreads = atoi(optarg);
- if (nthreads <= 0)
+ errno = 0;
+ nthreads = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE || nthreads <= 0)
{
pg_log_fatal("invalid number of threads: \"%s\"", optarg);
exit(1);
@@ -5920,8 +5923,9 @@ main(int argc, char **argv)
break;
case 's':
scale_given = true;
- scale = atoi(optarg);
- if (scale <= 0)
+ errno = 0;
+ scale = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE || scale <= 0)
{
pg_log_fatal("invalid scaling factor: \"%s\"", optarg);
exit(1);
@@ -5929,8 +5933,9 @@ main(int argc, char **argv)
break;
case 't':
benchmarking_option_set = true;
- nxacts = atoi(optarg);
- if (nxacts <= 0)
+ errno = 0;
+ nxacts = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE || nxacts <= 0)
{
pg_log_fatal("invalid number of transactions: \"%s\"", optarg);
exit(1);
@@ -5938,8 +5943,9 @@ main(int argc, char **argv)
break;
case 'T':
benchmarking_option_set = true;
- duration = atoi(optarg);
- if (duration <= 0)
+ errno = 0;
+ duration = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE || duration <= 0)
{
pg_log_fatal("invalid duration: \"%s\"", optarg);
exit(1);
@@ -6001,8 +6007,10 @@ main(int argc, char **argv)
break;
case 'F':
initialization_option_set = true;
- fillfactor = atoi(optarg);
- if (fillfactor < 10 || fillfactor > 100)
+ errno = 0;
+ fillfactor = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE ||
+ fillfactor < 10 || fillfactor > 100)
{
pg_log_fatal("invalid fillfactor: \"%s\"", optarg);
exit(1);
@@ -6021,8 +6029,9 @@ main(int argc, char **argv)
break;
case 'P':
benchmarking_option_set = true;
- progress = atoi(optarg);
- if (progress <= 0)
+ errno = 0;
+ progress = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE || progress <= 0)
{
pg_log_fatal("invalid thread progress delay: \"%s\"", optarg);
exit(1);
@@ -6031,11 +6040,13 @@ main(int argc, char **argv)
case 'R':
{
/* get a double from the beginning of option value */
- double throttle_value = atof(optarg);
+ double throttle_value;
+ errno = 0;
+ throttle_value = strtod(optarg, &endptr);
benchmarking_option_set = true;
- if (throttle_value <= 0.0)
+ if (*endptr || errno == ERANGE || throttle_value <= 0.0)
{
pg_log_fatal("invalid rate limit: \"%s\"", optarg);
exit(1);
@@ -6046,9 +6057,12 @@ main(int argc, char **argv)
break;
case 'L':
{
- double limit_ms = atof(optarg);
+ double limit_ms;
- if (limit_ms <= 0.0)
+ errno = 0;
+ limit_ms = strtod(optarg, &endptr);
+
+ if (*endptr || errno == ERANGE || limit_ms <= 0.0)
{
pg_log_fatal("invalid latency limit: \"%s\"", optarg);
exit(1);
@@ -6071,8 +6085,10 @@ main(int argc, char **argv)
break;
case 4: /* sampling-rate */
benchmarking_option_set = true;
- sample_rate = atof(optarg);
- if (sample_rate <= 0.0 || sample_rate > 1.0)
+ errno = 0;
+ sample_rate = strtod(optarg, &endptr);
+ if (*endptr || errno == ERANGE ||
+ sample_rate <= 0.0 || sample_rate > 1.0)
{
pg_log_fatal("invalid sampling rate: \"%s\"", optarg);
exit(1);
@@ -6080,8 +6096,9 @@ main(int argc, char **argv)
break;
case 5: /* aggregate-interval */
benchmarking_option_set = true;
- agg_interval = atoi(optarg);
- if (agg_interval <= 0)
+ errno = 0;
+ agg_interval = strtod(optarg, &endptr);
+ if (*endptr || errno == ERANGE || agg_interval <= 0)
{
pg_log_fatal("invalid number of seconds for aggregation: \"%s\"", optarg);
exit(1);
@@ -6117,8 +6134,9 @@ main(int argc, char **argv)
break;
case 11: /* partitions */
initialization_option_set = true;
- partitions = atoi(optarg);
- if (partitions < 0)
+ errno = 0;
+ partitions = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE || partitions < 0)
{
pg_log_fatal("invalid number of partitions: \"%s\"", optarg);
exit(1);
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 543401c6d6..aaed986ae1 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1039,8 +1039,11 @@ exec_command_edit(PsqlScanState scan_state, bool active_branch,
}
if (ln)
{
- lineno = atoi(ln);
- if (lineno < 1)
+ char *endptr;
+
+ errno = 0;
+ lineno = strtoint(ln, &endptr, 10);
+ if (*endptr || errno == ERANGE || lineno < 1)
{
pg_log_error("invalid line number: %s", ln);
status = PSQL_CMD_ERROR;
@@ -4283,7 +4286,21 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
else if (strcmp(param, "border") == 0)
{
if (value)
- popt->topt.border = atoi(value);
+ {
+ char *endptr;
+ int new_value;
+
+ errno = 0;
+ new_value = strtoint(value, &endptr, 10);
+ if (*endptr || errno == ERANGE ||
+ new_value < 0 || new_value > 65535)
+ {
+ pg_log_error("\\pset: border is invalid or out of range");
+ return false;
+ }
+
+ popt->topt.border = new_value;
+ }
}
/* set expanded/vertical mode */
@@ -4439,7 +4456,20 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
else if (strcmp(param, "pager_min_lines") == 0)
{
if (value)
- popt->topt.pager_min_lines = atoi(value);
+ {
+ char *endptr;
+ int new_value;
+
+ errno = 0;
+ new_value = strtoint(value, &endptr, 10);
+ if (*endptr || errno == ERANGE || new_value < 0)
+ {
+ pg_log_error("\\pset: pager_min_lines is invalid or out of range");
+ return false;
+ }
+
+ popt->topt.pager_min_lines = new_value;
+ }
}
/* disable "(x rows)" footer */
@@ -4455,7 +4485,19 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
else if (strcmp(param, "columns") == 0)
{
if (value)
- popt->topt.columns = atoi(value);
+ {
+ char *endptr;
+ int new_value;
+
+ errno = 0;
+ new_value = strtoint(value, &endptr, 10);
+ if (*endptr || errno == ERANGE || new_value < 0)
+ {
+ pg_log_error("\\pset: column is invalid or out of range");
+ return false;
+ }
+ popt->topt.columns = new_value;
+ }
}
else
{
diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c
index fc0681538a..baa68d58d8 100644
--- a/src/bin/scripts/reindexdb.c
+++ b/src/bin/scripts/reindexdb.c
@@ -15,6 +15,7 @@
#include "common.h"
#include "common/connect.h"
#include "common/logging.h"
+#include "common/string.h"
#include "fe_utils/cancel.h"
#include "fe_utils/option_utils.h"
#include "fe_utils/parallel_slot.h"
@@ -109,6 +110,8 @@ main(int argc, char *argv[])
/* process command-line options */
while ((c = getopt_long(argc, argv, "h:p:U:wWeqS:d:ast:i:j:v", long_options, &optindex)) != -1)
{
+ char *endptr;
+
switch (c)
{
case 'h':
@@ -151,10 +154,11 @@ main(int argc, char *argv[])
simple_string_list_append(&indexes, optarg);
break;
case 'j':
- concurrentCons = atoi(optarg);
- if (concurrentCons <= 0)
+ errno = 0;
+ concurrentCons = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE || concurrentCons <= 0)
{
- pg_log_error("number of parallel jobs must be at least 1");
+ pg_log_error("number of parallel jobs must be at least 1: %s", optarg);
exit(1);
}
break;
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index 61974baa78..93b563998a 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -17,6 +17,7 @@
#include "common.h"
#include "common/connect.h"
#include "common/logging.h"
+#include "common/string.h"
#include "fe_utils/cancel.h"
#include "fe_utils/option_utils.h"
#include "fe_utils/parallel_slot.h"
@@ -141,6 +142,8 @@ main(int argc, char *argv[])
while ((c = getopt_long(argc, argv, "h:p:U:wWeqd:zZFat:fvj:P:", long_options, &optindex)) != -1)
{
+ char *endptr;
+
switch (c)
{
case 'h':
@@ -192,16 +195,18 @@ main(int argc, char *argv[])
vacopts.verbose = true;
break;
case 'j':
- concurrentCons = atoi(optarg);
- if (concurrentCons <= 0)
+ errno = 0;
+ concurrentCons = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE || concurrentCons <= 0)
{
pg_log_error("number of parallel jobs must be at least 1");
exit(1);
}
break;
case 'P':
- vacopts.parallel_workers = atoi(optarg);
- if (vacopts.parallel_workers < 0)
+ errno = 0;
+ vacopts.parallel_workers = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE || vacopts.parallel_workers < 0)
{
pg_log_error("parallel workers for vacuum must be greater than or equal to zero");
exit(1);
@@ -220,16 +225,18 @@ main(int argc, char *argv[])
vacopts.skip_locked = true;
break;
case 6:
- vacopts.min_xid_age = atoi(optarg);
- if (vacopts.min_xid_age <= 0)
+ errno = 0;
+ vacopts.min_xid_age = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE || vacopts.min_xid_age <= 0)
{
pg_log_error("minimum transaction ID age must be at least 1");
exit(1);
}
break;
case 7:
- vacopts.min_mxid_age = atoi(optarg);
- if (vacopts.min_mxid_age <= 0)
+ errno = 0;
+ vacopts.min_mxid_age = strtoint(optarg, &endptr, 10);
+ if (*endptr || errno == ERANGE || vacopts.min_mxid_age <= 0)
{
pg_log_error("minimum multixact ID age must be at least 1");
exit(1);
--
2.27.0
----Next_Part(Fri_Jul__9_16_50_28_2021_228)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v2-0002-Make-complain-for-invalid-numeirc-values-in-envir.patch"
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher
@ 2023-01-09 20:03 Andres Freund <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Andres Freund @ 2023-01-09 20:03 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Önder Kalacı <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; Amit Kapila <[email protected]>; PostgreSQL Hackers <[email protected]>
Hi,
On 2023-01-07 13:50:04 -0500, Tom Lane wrote:
> I think we should either live within the constraints set by this
> overarching design, or else nuke execReplication.c from orbit and
> start using the real planner and executor. Perhaps the foreign
> key enforcement mechanisms could be a model --- although if you
> don't want to buy into using SPI as well, you probably should look
> at Amit L's work at [1].
I don't think using the full executor for every change is feasible from an
overhead perspective. But it might make sense to bail out to using the full
executor in a bunch of non-fastpath paths.
I think this is basically similar to COPY not using the full executor.
But that doesn't mean that all of this has to be open coded in
execReplication.c. Abstracting pieces so that COPY, logical rep and perhaps
even nodeModifyTable.c can share code makes sense.
> Also ... maybe I am missing something, but is REPLICA IDENTITY FULL
> sanely defined in the first place? It looks to me that
> RelationFindReplTupleSeq assumes without proof that there is a unique
> full-tuple match, but that is only reasonable to assume if there is at
> least one unique index (and maybe not even then, if nulls are involved).
If the table definition match between publisher and standby, it doesn't matter
which tuple is updated, if all columns are used to match. Since there's
nothing distinguishing two rows with all columns being equal, it doesn't
matter which we update.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher
@ 2023-01-09 20:37 Tom Lane <[email protected]>
parent: Andres Freund <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Tom Lane @ 2023-01-09 20:37 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Önder Kalacı <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; Amit Kapila <[email protected]>; PostgreSQL Hackers <[email protected]>
Andres Freund <[email protected]> writes:
> On 2023-01-07 13:50:04 -0500, Tom Lane wrote:
>> Also ... maybe I am missing something, but is REPLICA IDENTITY FULL
>> sanely defined in the first place? It looks to me that
>> RelationFindReplTupleSeq assumes without proof that there is a unique
>> full-tuple match, but that is only reasonable to assume if there is at
>> least one unique index (and maybe not even then, if nulls are involved).
> If the table definition match between publisher and standby, it doesn't matter
> which tuple is updated, if all columns are used to match. Since there's
> nothing distinguishing two rows with all columns being equal, it doesn't
> matter which we update.
Yeah, but the point here is precisely that they might *not* match;
for example there could be extra columns in the subscriber's table.
This may be largely a documentation problem, though --- I think my
beef is mainly that there's nothing in our docs explaining the
semantic pitfalls of FULL, we only say "it's slow".
Anyway, to get back to the point at hand: if we do have a REPLICA IDENTITY
FULL situation then we can make use of any unique index over a subset of
the transmitted columns, and if there's more than one candidate index
it's unlikely to matter which one we pick. Given your comment I guess
we have to also compare the non-indexed columns, so we can't completely
convert the FULL case to the straight index case. But still it doesn't
seem to me to be appropriate to use the planner to find a suitable index.
regards, tom lane
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher
@ 2023-01-20 12:35 Marco Slot <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Marco Slot @ 2023-01-20 12:35 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Andres Freund <[email protected]>; Önder Kalacı <[email protected]>; [email protected]; [email protected]; Amit Kapila <[email protected]>; PostgreSQL Hackers <[email protected]>
On Mon, Jan 9, 2023 at 11:37 PM Tom Lane <[email protected]> wrote:
> Anyway, to get back to the point at hand: if we do have a REPLICA IDENTITY
> FULL situation then we can make use of any unique index over a subset of
> the transmitted columns, and if there's more than one candidate index
> it's unlikely to matter which one we pick. Given your comment I guess
> we have to also compare the non-indexed columns, so we can't completely
> convert the FULL case to the straight index case. But still it doesn't
> seem to me to be appropriate to use the planner to find a suitable index.
The main purpose of REPLICA IDENTITY FULL seems to be to enable logical
replication for tables that may have duplicates and therefore cannot have a
unique index that can be used as a replica identity.
For those tables the user currently needs to choose between update/delete
erroring (bad) or doing a sequential scan on the apply side per
updated/deleted tuple (often worse). This issue currently prevents a lot of
automation around logical replication, because users need to decide whether
and when they are willing to accept partial downtime. The current REPLICA
IDENTITY FULL implementation can work in some cases, but applying the
effects of an update that affected a million rows through a million
sequential scans will certainly not end well.
This patch solves the problem by allowing the apply side to pick a
non-unique index to find any matching tuple instead of always using a
sequential scan, but that either requires some planning/costing logic to
avoid picking a lousy index, or allowing the user to manually preselect the
index to use, which is less convenient.
An alternative might be to construct prepared statements and using the
regular planner. If applied uniformly that would also be nice from the
extensibility point-of-view, since there is currently no way for an
extension to augment the apply side. However, I assume the current approach
of using low-level functions in the common case was chosen for performance
reasons.
I suppose the options are:
1. use regular planner uniformly
2. use regular planner only when there's no replica identity (or
configurable?)
3. only use low-level functions
4. keep using sequential scans for every single updated row
5. introduce a hidden logical row identifier in the heap that is guaranteed
unique within a table and can be used as a replica identity when no unique
index exists
Any thoughts?
cheers,
Marco
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher
@ 2023-01-27 13:02 Önder Kalacı <[email protected]>
parent: Marco Slot <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Önder Kalacı @ 2023-01-27 13:02 UTC (permalink / raw)
To: Marco Slot <[email protected]>; +Cc: Tom Lane <[email protected]>; Andres Freund <[email protected]>; [email protected]; [email protected]; Amit Kapila <[email protected]>; PostgreSQL Hackers <[email protected]>
Hi Marco, Tom,
> But still it doesn't seem to me to be appropriate to use the planner to
find a suitable index.
As Marco noted, here we are trying to pick an index that is non-unique. We
could pick the index based on information extracted from pg_index (or
such), but then, it'd be a premature selection. Before sending the patch to
pgsql-hackers, I initially tried to find a suitable one with such an
approach.
But then, I still ended up using costing functions (and some other low
level functions). Overall, it felt like the planner is the module that
makes this decision best. Why would we try to invent another immature way
of doing this? With that reasoning, I ended up using the related planner
functions directly.
However, I assume the current approach of using low-level functions in the
> common case was chosen for performance reasons.
>
That's partially the reason. If you look at the patch, we use the planner
(or the low level functions) infrequently. It is only called when the
logical replication relation cache is rebuilt. As far as I can see, that
happens with (auto) ANALYZE or DDLs etc. I expect these are infrequent
operations. Still, I wanted to make sure we do not create too much overhead
even if there are frequent invalidations.
The main reason for using the low level functions over the planner itself
is to have some more control over the decision. For example, due to the
execution limitations, we currently cannot allow an index that consists of
only expressions (similar to pkey restriction). With the current approach,
we can easily filter those out.
Also, another minor reason is that, if we use planner, we'd get a
PlannedStmt back. It also felt weird to check back the index used from a
PlannedStmt. In the current patch, we iterate over Paths, which seems more
intuitive to me.
> I suppose the options are:
> 1. use regular planner uniformly
> 2. use regular planner only when there's no replica identity (or
> configurable?)
> 3. only use low-level functions
> 4. keep using sequential scans for every single updated row
> 5. introduce a hidden logical row identifier in the heap that is
> guaranteed unique within a table and can be used as a replica identity when
> no unique index exists
>
One other option I considered was to ask the index explicitly on the
subscriber side from the user when REPLICA IDENTITY is FULL. But, it is a
pretty hard choice for any user, even a planner sometimes fails to pick the
right index :) Also, it is probably controversial to change any of the
APIs for this purpose?
I'd be happy to hear from more experienced hackers on the trade-offs for
the above, and I'd be open to work on that if there is a clear winner. For
me (3) is a decent solution for the problem.
Thanks,
Onder
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher
@ 2023-01-30 13:16 Amit Kapila <[email protected]>
parent: Önder Kalacı <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Amit Kapila @ 2023-01-30 13:16 UTC (permalink / raw)
To: Önder Kalacı <[email protected]>; +Cc: Marco Slot <[email protected]>; Tom Lane <[email protected]>; Andres Freund <[email protected]>; [email protected]; [email protected]; PostgreSQL Hackers <[email protected]>
On Fri, Jan 27, 2023 at 6:32 PM Önder Kalacı <[email protected]> wrote:
>>
>> I suppose the options are:
>> 1. use regular planner uniformly
>> 2. use regular planner only when there's no replica identity (or configurable?)
>> 3. only use low-level functions
>> 4. keep using sequential scans for every single updated row
>> 5. introduce a hidden logical row identifier in the heap that is guaranteed unique within a table and can be used as a replica identity when no unique index exists
>
>
> One other option I considered was to ask the index explicitly on the subscriber side from the user when REPLICA IDENTITY is FULL. But, it is a pretty hard choice for any user, even a planner sometimes fails to pick the right index :) Also, it is probably controversial to change any of the APIs for this purpose?
>
I agree that it won't be a very convenient option for the user but how
about along with asking for an index from the user (when the user
didn't provide an index), we also allow to make use of any unique
index over a subset of the transmitted columns, and if there's more
than one candidate index pick any one. Additionally, we can allow
disabling the use of an index scan for this particular case. If we are
too worried about API change for allowing users to specify the index
then we can do that later or as a separate patch.
> I'd be happy to hear from more experienced hackers on the trade-offs for the above, and I'd be open to work on that if there is a clear winner. For me (3) is a decent solution for the problem.
>
From the discussion above it is not very clear that adding maintenance
costs in this area is worth it even though that can give better
results as far as this feature is concerned.
--
With Regards,
Amit Kapila.
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher
@ 2023-02-02 08:33 Önder Kalacı <[email protected]>
parent: Amit Kapila <[email protected]>
0 siblings, 4 replies; 18+ messages in thread
From: Önder Kalacı @ 2023-02-02 08:33 UTC (permalink / raw)
To: Amit Kapila <[email protected]>; +Cc: Marco Slot <[email protected]>; Tom Lane <[email protected]>; Andres Freund <[email protected]>; [email protected]; [email protected]; PostgreSQL Hackers <[email protected]>
Hi all,
Thanks for the feedback!
I agree that it won't be a very convenient option for the user but how
> about along with asking for an index from the user (when the user
> didn't provide an index), we also allow to make use of any unique
> index over a subset of the transmitted columns,
Tbh, I cannot follow why you would use REPLICA IDENTITY FULL if you can
already
create a unique index? Aren't you supposed to use REPLICA IDENTITY .. USING
INDEX
in that case (if not simply pkey)?
That seems like a potential expansion of this patch, but I don't consider
it as essential. Given it
is hard to get even small commits in, I'd rather wait to see what you think
before doing such
a change.
> and if there's more
> than one candidate index pick any one. Additionally, we can allow
> disabling the use of an index scan for this particular case. If we are
> too worried about API change for allowing users to specify the index
> then we can do that later or as a separate patch.
>
>
On v23, I dropped the planner support for picking the index. Instead, it
simply
iterates over the indexes and picks the first one that is suitable.
I'm currently thinking on how to enable users to override this decision.
One option I'm leaning towards is to add a syntax like the following:
*ALTER SUBSCRIPTION .. ALTER TABLE ... SET INDEX ...*
Though, that should probably be a seperate patch. I'm going to work
on that, but still wanted to share v23 given picking the index sounds
complementary, not strictly required at this point.
Thanks,
Onder
Attachments:
[application/octet-stream] v23_0001_use_index_on_subs_when_pub_rep_ident_full.patch (63.6K, ../../CACawEhUN=+vjY0+4q416-rAYx6pw-nZMHQYsJZCftf9MjoPN3w@mail.gmail.com/3-v23_0001_use_index_on_subs_when_pub_rep_ident_full.patch)
download | inline diff:
From 25ab136c6ebcb3a2bcfc938989b3c6f2e9fb1163 Mon Sep 17 00:00:00 2001
From: Onder Kalaci <[email protected]>
Date: Tue, 17 May 2022 10:47:39 +0200
Subject: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full
on the publisher
Using `REPLICA IDENTITY FULL` on the publication leads to a full table
scan per tuple change on the subscription. This makes `REPLICA
IDENTITY FULL` impracticable -- probably other than some small number
of use cases.
With this patch, I'm proposing the following change: If there is any
index on the subscriber, let the planner sub-modules compare the costs
of index versus sequential scan and choose the cheapest. The index
should be a btree index, not a partial index, and it should have at
least one column reference (e.g., cannot consist of only expressions).
The majority of the logic on the subscriber side already exists in
the code. The subscriber is already capable of doing (unique) index
scans. With this patch, we are allowing the index to iterate over the
tuples fetched and only act when tuples are equal. Anyone familiar
with this part of the code might recognize that the sequential scan
code on the subscriber already implements the `tuples_equal()`
function. In short, the changes on the subscriber are mostly
combining parts of (unique) index scan and sequential scan codes.
The decision on whether to use an index (or which index) is mostly
derived from planner infrastructure. The idea is that on the
subscriber we have all the columns. So, construct all the
`Path`s with the restrictions on all columns, such as
`col_1 = $1 AND col_2 = $2 ... AND col_n = $N`. Finally, let the
planner sub-module -- `make_one_rel()` -- to give us the relevant
index `Path`s. On top of that, add the sequential scan `Path` as
well. Finally, pick the cheapest `Path` among.
From the performance point of view, there are a few things to note.
First, the patch aims not to change the behavior when PRIMARY KEY
or UNIQUE INDEX is used. Second, when REPLICA IDENTITY FULL is on
the publisher and an index is used on the subscriber, the
difference mostly comes down to `index scan` vs `sequential scan`.
That's why it is hard to claim certain number of improvements.
It mostly depends on the data size, index and the data distribution.
Still, below I try to showcase the potential improvements using an
index on the subscriber `pgbench_accounts(bid)`. With the index,
all the changes are replicated within ~5 seconds. When the index
is dropped, the same operation takes around ~300 seconds.
// init source db
pgbench -i -s 100 -p 5432 postgres
psql -c "ALTER TABLE pgbench_accounts DROP CONSTRAINT pgbench_accounts_pkey;" -p 5432 postgres
psql -c "CREATE INDEX i1 ON pgbench_accounts(aid);" -p 5432 postgres
psql -c "ALTER TABLE pgbench_accounts REPLICA IDENTITY FULL;" -p 5432 postgres
psql -c "CREATE PUBLICATION pub_test_1 FOR TABLE pgbench_accounts;" -p 5432 postgres
// init target db, drop existing primary key
pgbench -i -p 9700 postgres
psql -c "TRUNCATE pgbench_accounts;" -p 9700 postgres
psql -c "ALTER TABLE pgbench_accounts DROP CONSTRAINT pgbench_accounts_pkey;" -p 9700 postgres
psql -c "CREATE SUBSCRIPTION sub_test_1 CONNECTION 'host=localhost port=5432 user=onderkalaci dbname=postgres' PUBLICATION pub_test_1;" -p 9700 postgres
// create one index, even on a low cardinality column
psql -c "CREATE INDEX i2 ON pgbench_accounts(bid);" -p 9700 postgres
// now, run some pgbench tests and observe replication
pgbench -t 500 -b tpcb-like -p 5432 postgres
---
doc/src/sgml/logical-replication.sgml | 8 +-
src/backend/executor/execReplication.c | 149 ++-
src/backend/replication/logical/relation.c | 178 ++++
src/backend/replication/logical/worker.c | 75 +-
src/include/replication/logicalrelation.h | 3 +
src/test/subscription/meson.build | 1 +
.../subscription/t/032_subscribe_use_index.pl | 867 ++++++++++++++++++
7 files changed, 1217 insertions(+), 64 deletions(-)
create mode 100644 src/test/subscription/t/032_subscribe_use_index.pl
diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index 1bd5660c87..96ab80b21a 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -132,8 +132,12 @@
certain additional requirements) can also be set to be the replica
identity. If the table does not have any suitable key, then it can be set
to replica identity <quote>full</quote>, which means the entire row becomes
- the key. This, however, is very inefficient and should only be used as a
- fallback if no other solution is possible. If a replica identity other
+ the key. If replica identity <quote>full</quote> is used, indexes can be
+ used on the subscriber side for seaching the rows. The index should be
+ btree, non-partial and have at least one column reference (e.g.,
+ should not consists of only expressions). If there are no suitable indexes,
+ the search on the subscriber side is very inefficient and should only be
+ used as a fallback if no other solution is possible. If a replica identity other
than <quote>full</quote> is set on the publisher side, a replica identity
comprising the same or fewer columns must also be set on the subscriber
side. See <xref linkend="sql-altertable-replica-identity"/> for details on
diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c
index c484f5c301..b678106ca6 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -25,6 +25,9 @@
#include "nodes/nodeFuncs.h"
#include "parser/parse_relation.h"
#include "parser/parsetree.h"
+#ifdef USE_ASSERT_CHECKING
+#include "replication/logicalrelation.h"
+#endif
#include "storage/bufmgr.h"
#include "storage/lmgr.h"
#include "utils/builtins.h"
@@ -37,28 +40,29 @@
#include "utils/typcache.h"
+static bool tuples_equal(TupleTableSlot *slot1, TupleTableSlot *slot2,
+ TypeCacheEntry **eq);
+
/*
* Setup a ScanKey for a search in the relation 'rel' for a tuple 'key' that
* is setup to match 'rel' (*NOT* idxrel!).
*
- * Returns whether any column contains NULLs.
+ * Returns how many columns should be used for the index scan.
*
- * This is not generic routine, it expects the idxrel to be replication
- * identity of a rel and meet all limitations associated with that.
+ * This is not a generic routine - it expects the idxrel to be an index
+ * that planner would choose if the searchslot includes all the columns
+ * (e.g., REPLICA IDENTITY FULL on the source).
*/
-static bool
+static int
build_replindex_scan_key(ScanKey skey, Relation rel, Relation idxrel,
TupleTableSlot *searchslot)
{
- int attoff;
+ int index_attoff;
+ int skey_attoff = 0;
bool isnull;
Datum indclassDatum;
oidvector *opclass;
int2vector *indkey = &idxrel->rd_index->indkey;
- bool hasnulls = false;
-
- Assert(RelationGetReplicaIndex(rel) == RelationGetRelid(idxrel) ||
- RelationGetPrimaryKeyIndex(rel) == RelationGetRelid(idxrel));
indclassDatum = SysCacheGetAttr(INDEXRELID, idxrel->rd_indextuple,
Anum_pg_index_indclass, &isnull);
@@ -66,20 +70,54 @@ build_replindex_scan_key(ScanKey skey, Relation rel, Relation idxrel,
opclass = (oidvector *) DatumGetPointer(indclassDatum);
/* Build scankey for every attribute in the index. */
- for (attoff = 0; attoff < IndexRelationGetNumberOfKeyAttributes(idxrel); attoff++)
+ for (index_attoff = 0; index_attoff < IndexRelationGetNumberOfKeyAttributes(idxrel);
+ index_attoff++)
{
Oid operator;
Oid opfamily;
+ Oid optype = get_opclass_input_type(opclass->values[index_attoff]);
RegProcedure regop;
- int pkattno = attoff + 1;
- int mainattno = indkey->values[attoff];
- Oid optype = get_opclass_input_type(opclass->values[attoff]);
+ int table_attno = indkey->values[index_attoff];
+
+ if (!AttributeNumberIsValid(table_attno))
+ {
+ /*
+ * This attribute is an expression, and
+ * SuitableIndexPathsForRepIdentFull() was called earlier when the
+ * index for subscriber was selected. There, the indexes
+ * comprising *only* expressions have already been eliminated.
+ *
+ * We sanity check this now.
+ */
+#ifdef USE_ASSERT_CHECKING
+ IndexInfo *indexInfo = BuildIndexInfo(idxrel);
+
+ Assert(!IsIndexOnlyOnExpression(indexInfo));
+#endif
+
+ /*
+ * Furthermore, because primary key and unique key indexes can't
+ * include expressions we also sanity check the index is neither
+ * of those kinds.
+ */
+ Assert(RelationGetReplicaIndex(rel) != RelationGetRelid(idxrel) &&
+ RelationGetPrimaryKeyIndex(rel) != RelationGetRelid(idxrel));
+
+ /*
+ * XXX: For a non-primary/unique index with an additional
+ * expression, we do not have to continue at this point. However,
+ * the below code assumes the index scan is only done for simple
+ * column references. If we can relax the assumption in the below
+ * code-block, we can also remove the continue.
+ */
+ continue;
+ }
/*
* Load the operator info. We need this to get the equality operator
* function for the scan key.
*/
- opfamily = get_opclass_family(opclass->values[attoff]);
+ opfamily = get_opclass_family(opclass->values[index_attoff]);
operator = get_opfamily_member(opfamily, optype,
optype,
@@ -91,23 +129,44 @@ build_replindex_scan_key(ScanKey skey, Relation rel, Relation idxrel,
regop = get_opcode(operator);
/* Initialize the scankey. */
- ScanKeyInit(&skey[attoff],
- pkattno,
+ ScanKeyInit(&skey[skey_attoff],
+ index_attoff + 1,
BTEqualStrategyNumber,
regop,
- searchslot->tts_values[mainattno - 1]);
+ searchslot->tts_values[table_attno - 1]);
- skey[attoff].sk_collation = idxrel->rd_indcollation[attoff];
+ skey[skey_attoff].sk_collation = idxrel->rd_indcollation[index_attoff];
/* Check for null value. */
- if (searchslot->tts_isnull[mainattno - 1])
- {
- hasnulls = true;
- skey[attoff].sk_flags |= SK_ISNULL;
- }
+ if (searchslot->tts_isnull[table_attno - 1])
+ skey[skey_attoff].sk_flags |= (SK_ISNULL | SK_SEARCHNULL);
+
+ skey_attoff++;
}
- return hasnulls;
+ /* There should always be at least one attribute for the index scan. */
+ Assert(skey_attoff > 0);
+
+ return skey_attoff;
+}
+
+/*
+ * Given a relation and OID of an index, returns true if
+ * the index is relation's primary key's index or
+ * relaton's replica identity index.
+ *
+ * Returns false otherwise.
+ */
+static bool
+IdxIsRelationIdentityOrPK(Relation rel, Oid idxoid)
+{
+ Assert(OidIsValid(idxoid));
+
+ if (RelationGetReplicaIndex(rel) == idxoid ||
+ RelationGetPrimaryKeyIndex(rel) == idxoid)
+ return true;
+
+ return false;
}
/*
@@ -123,33 +182,55 @@ RelationFindReplTupleByIndex(Relation rel, Oid idxoid,
TupleTableSlot *outslot)
{
ScanKeyData skey[INDEX_MAX_KEYS];
+ int skey_attoff;
IndexScanDesc scan;
SnapshotData snap;
TransactionId xwait;
Relation idxrel;
bool found;
+ TypeCacheEntry **eq = NULL; /* only used when the index is not repl. ident
+ * or pkey */
+ bool idxIsRelationIdentityOrPK;
/* Open the index. */
idxrel = index_open(idxoid, RowExclusiveLock);
- /* Start an index scan. */
+ idxIsRelationIdentityOrPK = IdxIsRelationIdentityOrPK(rel, idxoid);
+
InitDirtySnapshot(snap);
- scan = index_beginscan(rel, idxrel, &snap,
- IndexRelationGetNumberOfKeyAttributes(idxrel),
- 0);
/* Build scan key. */
- build_replindex_scan_key(skey, rel, idxrel, searchslot);
+ skey_attoff = build_replindex_scan_key(skey, rel, idxrel, searchslot);
+
+ /* Start an index scan. */
+ scan = index_beginscan(rel, idxrel, &snap, skey_attoff, 0);
retry:
found = false;
- index_rescan(scan, skey, IndexRelationGetNumberOfKeyAttributes(idxrel), NULL, 0);
+ index_rescan(scan, skey, skey_attoff, NULL, 0);
/* Try to find the tuple */
- if (index_getnext_slot(scan, ForwardScanDirection, outslot))
+ while (index_getnext_slot(scan, ForwardScanDirection, outslot))
{
- found = true;
+ /*
+ * Avoid expensive equality check if the index is primary key or
+ * replica identity index.
+ */
+ if (!idxIsRelationIdentityOrPK)
+ {
+ /*
+ * We only need to allocate once. This is allocated within per
+ * tuple context -- ApplyMessageContext -- hence no need to
+ * explicitly pfree().
+ */
+ if (eq == NULL)
+ eq = palloc0(sizeof(*eq) * outslot->tts_tupleDescriptor->natts);
+
+ if (!tuples_equal(outslot, searchslot, eq))
+ continue;
+ }
+
ExecMaterializeSlot(outslot);
xwait = TransactionIdIsValid(snap.xmin) ?
@@ -164,6 +245,10 @@ retry:
XactLockTableWait(xwait, NULL, NULL, XLTW_None);
goto retry;
}
+
+ /* Found our tuple and it's not locked */
+ found = true;
+ break;
}
/* Found tuple, try to lock it in the lockmode. */
diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index ca88ae171c..07ad6dc2b0 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -17,13 +17,16 @@
#include "postgres.h"
+#include "access/genam.h"
#include "access/table.h"
#include "catalog/namespace.h"
+#include "catalog/pg_am_d.h"
#include "catalog/pg_subscription_rel.h"
#include "executor/executor.h"
#include "nodes/makefuncs.h"
#include "replication/logicalrelation.h"
#include "replication/worker_internal.h"
+#include "optimizer/cost.h"
#include "utils/inval.h"
@@ -50,6 +53,9 @@ typedef struct LogicalRepPartMapEntry
LogicalRepRelMapEntry relmapentry;
} LogicalRepPartMapEntry;
+static Oid FindLogicalRepUsableIndex(Relation localrel,
+ LogicalRepRelation *remoterel);
+
/*
* Relcache invalidation callback for our relation map cache.
*/
@@ -438,6 +444,14 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode)
*/
logicalrep_rel_mark_updatable(entry);
+ /*
+ * Finding a usable index is an infrequent task. It occurs when an
+ * operation is first performed on the relation, or after invalidation
+ * of the relation cache entry (such as ANALYZE or CREATE/DROP index
+ * on the relation).
+ */
+ entry->usableIndexOid = FindLogicalRepUsableIndex(entry->localrel, remoterel);
+
entry->localrelvalid = true;
}
@@ -696,6 +710,14 @@ logicalrep_partition_open(LogicalRepRelMapEntry *root,
/* Set if the table's replica identity is enough to apply update/delete. */
logicalrep_rel_mark_updatable(entry);
+ /*
+ * Finding a usable index is an infrequent task. It occurs when an
+ * operation is first performed on the relation, or after invalidation of
+ * of the relation cache entry (such as ANALYZE or CREATE/DROP index on
+ * the relation).
+ */
+ entry->usableIndexOid = FindLogicalRepUsableIndex(partrel, remoterel);
+
entry->localrelvalid = true;
/* state and statelsn are left set to 0. */
@@ -703,3 +725,159 @@ logicalrep_partition_open(LogicalRepRelMapEntry *root,
return entry;
}
+
+/*
+ * Returns true if the given index consists only of expressions such as:
+ * CREATE INDEX idx ON table(foo(col));
+ *
+ * Returns false even if there is one column reference:
+ * CREATE INDEX idx ON table(foo(col), col_2);
+ */
+bool
+IsIndexOnlyOnExpression(IndexInfo *indexInfo)
+{
+ for (int i = 0; i < indexInfo->ii_NumIndexKeyAttrs; i++)
+ {
+ AttrNumber attnum = indexInfo->ii_IndexAttrNumbers[i];
+
+ if (AttributeNumberIsValid(attnum))
+ return false;
+ }
+
+ return true;
+}
+
+
+/*
+ * Returns an index oid if the planner submodules pick index scans
+ * over sequential scan.
+ *
+ * Otherwise, returns InvalidOid.
+ *
+ * Note that this is not a generic function, it expects REPLICA
+ * IDENTITY FULL for the remote relation.
+ */
+static Oid
+FindUsableIndexForReplicaIdentityFull(Relation localrel)
+{
+ MemoryContext usableIndexContext;
+ MemoryContext oldctx;
+ Oid usableIndex;
+ Oid idxoid;
+ List *indexlist;
+ ListCell *lc;
+ Relation indexRelation;
+ IndexInfo *indexInfo;
+ bool is_btree;
+ bool is_partial;
+ bool is_only_on_expression;
+
+ usableIndexContext = AllocSetContextCreate(CurrentMemoryContext,
+ "usableIndexContext",
+ ALLOCSET_DEFAULT_SIZES);
+ oldctx = MemoryContextSwitchTo(usableIndexContext);
+
+ /* Get index list of the local relation */
+ indexlist = RelationGetIndexList(localrel);
+ Assert(indexlist != NIL);
+
+ usableIndex = InvalidOid;
+ idxoid = InvalidOid;
+ foreach(lc, indexlist)
+ {
+ idxoid = lfirst_oid(lc);
+ indexRelation = index_open(idxoid, AccessShareLock);
+ indexInfo = BuildIndexInfo(indexRelation);
+
+ is_btree = (indexInfo->ii_Am == BTREE_AM_OID);
+ is_partial = (indexInfo->ii_Predicate != NIL);
+ is_only_on_expression = IsIndexOnlyOnExpression(indexInfo);
+ index_close(indexRelation, AccessShareLock);
+
+ if (is_btree && !is_partial && !is_only_on_expression)
+ {
+ /* we found one eligible index, don't need to continue */
+ usableIndex = idxoid;
+ break;
+ }
+ }
+
+ MemoryContextSwitchTo(oldctx);
+
+ MemoryContextDelete(usableIndexContext);
+
+ return usableIndex;
+}
+
+/*
+ * Get replica identity index or if it is not defined a primary key.
+ *
+ * If neither is defined, returns InvalidOid
+ */
+static Oid
+GetRelationIdentityOrPK(Relation rel)
+{
+ Oid idxoid;
+
+ idxoid = RelationGetReplicaIndex(rel);
+
+ if (!OidIsValid(idxoid))
+ idxoid = RelationGetPrimaryKeyIndex(rel);
+
+ return idxoid;
+}
+
+/*
+ * Returns an index oid if we can use an index for subscriber. If not,
+ * returns InvalidOid.
+ */
+static Oid
+FindLogicalRepUsableIndex(Relation localrel, LogicalRepRelation *remoterel)
+{
+ Oid idxoid;
+
+ /*
+ * We never need index oid for partitioned tables, always rely on leaf
+ * partition's index.
+ */
+ if (localrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+ return InvalidOid;
+
+ /*
+ * Simple case, we already have a primary key or a replica identity index.
+ *
+ * Note that we do not use index scans below when enable_indexscan is
+ * false. Allowing primary key or replica identity even when index scan is
+ * disabled is the legacy behaviour. So we hesitate to move the below
+ * enable_indexscan check to be done earlier in this function.
+ */
+ idxoid = GetRelationIdentityOrPK(localrel);
+ if (OidIsValid(idxoid))
+ return idxoid;
+
+ /* If index scans are disabled, use a sequential scan */
+ if (!enable_indexscan)
+ return InvalidOid;
+
+ if (remoterel->replident == REPLICA_IDENTITY_FULL &&
+ RelationGetIndexList(localrel) != NIL)
+ {
+ /*
+ * If we had a primary key or relation identity with a unique index,
+ * we would have already found and returned that oid. At this point,
+ * the remote relation has replica identity full and we have at least
+ * one local index defined.
+ *
+ * We are looking for one more opportunity for using an index. If
+ * there are any indexes defined on the local relation, try to pick
+ * the cheapest index.
+ *
+ * The index selection safely assumes that all the columns are going
+ * to be available for the index scan given that remote relation has
+ * replica identity full.
+ */
+ return FindUsableIndexForReplicaIdentityFull(localrel);
+ }
+
+ return InvalidOid;
+}
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index cfb2ab6248..75eb2ba7f1 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -397,6 +397,8 @@ static void apply_handle_commit_internal(LogicalRepCommitData *commit_data);
static void apply_handle_insert_internal(ApplyExecutionData *edata,
ResultRelInfo *relinfo,
TupleTableSlot *remoteslot);
+static Oid get_usable_indexoid(ApplyExecutionData *edata,
+ ResultRelInfo *relinfo);
static void apply_handle_update_internal(ApplyExecutionData *edata,
ResultRelInfo *relinfo,
TupleTableSlot *remoteslot,
@@ -406,6 +408,7 @@ static void apply_handle_delete_internal(ApplyExecutionData *edata,
TupleTableSlot *remoteslot);
static bool FindReplTupleInLocalRel(EState *estate, Relation localrel,
LogicalRepRelation *remoterel,
+ Oid localidxoid,
TupleTableSlot *remoteslot,
TupleTableSlot **localslot);
static void apply_handle_tuple_routing(ApplyExecutionData *edata,
@@ -2348,24 +2351,6 @@ apply_handle_type(StringInfo s)
logicalrep_read_typ(s, &typ);
}
-/*
- * Get replica identity index or if it is not defined a primary key.
- *
- * If neither is defined, returns InvalidOid
- */
-static Oid
-GetRelationIdentityOrPK(Relation rel)
-{
- Oid idxoid;
-
- idxoid = RelationGetReplicaIndex(rel);
-
- if (!OidIsValid(idxoid))
- idxoid = RelationGetPrimaryKeyIndex(rel);
-
- return idxoid;
-}
-
/*
* Check that we (the subscription owner) have sufficient privileges on the
* target relation to perform the given operation.
@@ -2511,11 +2496,8 @@ check_relation_updatable(LogicalRepRelMapEntry *rel)
if (rel->updatable)
return;
- /*
- * We are in error mode so it's fine this is somewhat slow. It's better to
- * give user correct error.
- */
- if (OidIsValid(GetRelationIdentityOrPK(rel->localrel)))
+ /* Give user more precise error if possible. */
+ if (OidIsValid(rel->usableIndexOid))
{
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
@@ -2655,12 +2637,14 @@ apply_handle_update_internal(ApplyExecutionData *edata,
TupleTableSlot *localslot;
bool found;
MemoryContext oldctx;
+ Oid usableIndexOid = get_usable_indexoid(edata, relinfo);
EvalPlanQualInit(&epqstate, estate, NULL, NIL, -1);
ExecOpenIndices(relinfo, false);
found = FindReplTupleInLocalRel(estate, localrel,
&relmapentry->remoterel,
+ usableIndexOid,
remoteslot, &localslot);
ExecClearTuple(remoteslot);
@@ -2793,11 +2777,12 @@ apply_handle_delete_internal(ApplyExecutionData *edata,
EPQState epqstate;
TupleTableSlot *localslot;
bool found;
+ Oid usableIndexOid = get_usable_indexoid(edata, relinfo);
EvalPlanQualInit(&epqstate, estate, NULL, NIL, -1);
ExecOpenIndices(relinfo, false);
- found = FindReplTupleInLocalRel(estate, localrel, remoterel,
+ found = FindReplTupleInLocalRel(estate, localrel, remoterel, usableIndexOid,
remoteslot, &localslot);
/* If found delete it. */
@@ -2828,20 +2813,50 @@ apply_handle_delete_internal(ApplyExecutionData *edata,
EvalPlanQualEnd(&epqstate);
}
+/*
+ * Decide whether we can pick an index for the relinfo (e.g., the relation)
+ * we're actually deleting/updating from. If it is a child partition of
+ * edata->targetRelInfo, find the index on the partition.
+ *
+ * Note that if the corresponding relmapentry has invalid usableIndexOid,
+ * the function returns InvalidOid.
+ */
+static Oid
+get_usable_indexoid(ApplyExecutionData *edata, ResultRelInfo *relinfo)
+{
+ ResultRelInfo *targetResultRelInfo = edata->targetRelInfo;
+ LogicalRepRelMapEntry *relmapentry = edata->targetRel;
+
+ char targetrelkind = targetResultRelInfo->ri_RelationDesc->rd_rel->relkind;
+
+ if (targetrelkind == RELKIND_PARTITIONED_TABLE)
+ {
+ /* Target is a partitioned table, so find relmapentry of the partition */
+ TupleConversionMap *map = ExecGetRootToChildMap(relinfo, edata->estate);
+ AttrMap *attrmap = map ? map->attrMap : NULL;
+
+ relmapentry =
+ logicalrep_partition_open(relmapentry, relinfo->ri_RelationDesc,
+ attrmap);
+ }
+
+ return relmapentry->usableIndexOid;
+}
+
/*
* Try to find a tuple received from the publication side (in 'remoteslot') in
* the corresponding local relation using either replica identity index,
- * primary key or if needed, sequential scan.
+ * primary key, index or if needed, sequential scan.
*
* Local tuple, if found, is returned in '*localslot'.
*/
static bool
FindReplTupleInLocalRel(EState *estate, Relation localrel,
LogicalRepRelation *remoterel,
+ Oid localidxoid,
TupleTableSlot *remoteslot,
TupleTableSlot **localslot)
{
- Oid idxoid;
bool found;
/*
@@ -2852,12 +2867,11 @@ FindReplTupleInLocalRel(EState *estate, Relation localrel,
*localslot = table_slot_create(localrel, &estate->es_tupleTable);
- idxoid = GetRelationIdentityOrPK(localrel);
- Assert(OidIsValid(idxoid) ||
+ Assert(OidIsValid(localidxoid) ||
(remoterel->replident == REPLICA_IDENTITY_FULL));
- if (OidIsValid(idxoid))
- found = RelationFindReplTupleByIndex(localrel, idxoid,
+ if (OidIsValid(localidxoid))
+ found = RelationFindReplTupleByIndex(localrel, localidxoid,
LockTupleExclusive,
remoteslot, *localslot);
else
@@ -2978,6 +2992,7 @@ apply_handle_tuple_routing(ApplyExecutionData *edata,
/* Get the matching local tuple from the partition. */
found = FindReplTupleInLocalRel(estate, partrel,
&part_entry->remoterel,
+ part_entry->usableIndexOid,
remoteslot_part, &localslot);
if (!found)
{
diff --git a/src/include/replication/logicalrelation.h b/src/include/replication/logicalrelation.h
index 9c34054bb7..9433122198 100644
--- a/src/include/replication/logicalrelation.h
+++ b/src/include/replication/logicalrelation.h
@@ -13,6 +13,7 @@
#define LOGICALRELATION_H
#include "access/attmap.h"
+#include "catalog/index.h"
#include "replication/logicalproto.h"
typedef struct LogicalRepRelMapEntry
@@ -31,6 +32,7 @@ typedef struct LogicalRepRelMapEntry
Relation localrel; /* relcache entry (NULL when closed) */
AttrMap *attrmap; /* map of local attributes to remote ones */
bool updatable; /* Can apply updates/deletes? */
+ Oid usableIndexOid; /* which index to use, or InvalidOid if none */
/* Sync state. */
char state;
@@ -46,5 +48,6 @@ extern LogicalRepRelMapEntry *logicalrep_partition_open(LogicalRepRelMapEntry *r
Relation partrel, AttrMap *map);
extern void logicalrep_rel_close(LogicalRepRelMapEntry *rel,
LOCKMODE lockmode);
+extern bool IsIndexOnlyOnExpression(IndexInfo *indexInfo);
#endif /* LOGICALRELATION_H */
diff --git a/src/test/subscription/meson.build b/src/test/subscription/meson.build
index 3db0fdfd96..f85bf92b6f 100644
--- a/src/test/subscription/meson.build
+++ b/src/test/subscription/meson.build
@@ -38,6 +38,7 @@ tests += {
't/029_on_error.pl',
't/030_origin.pl',
't/031_column_list.pl',
+ 't/032_subscribe_use_index.pl',
't/100_bugs.pl',
],
},
diff --git a/src/test/subscription/t/032_subscribe_use_index.pl b/src/test/subscription/t/032_subscribe_use_index.pl
new file mode 100644
index 0000000000..3fb05f3ab7
--- /dev/null
+++ b/src/test/subscription/t/032_subscribe_use_index.pl
@@ -0,0 +1,867 @@
+# Copyright (c) 2021-2022, PostgreSQL Global Development Group
+
+# Test logical replication behavior with subscriber uses available index
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# create publisher node
+my $node_publisher = PostgreSQL::Test::Cluster->new('publisher');
+$node_publisher->init(allows_streaming => 'logical');
+$node_publisher->start;
+
+# create subscriber node
+my $node_subscriber = PostgreSQL::Test::Cluster->new('subscriber');
+$node_subscriber->init(allows_streaming => 'logical');
+$node_subscriber->append_conf('postgresql.conf',
+ "wal_retrieve_retry_interval = 1ms");
+
+# we don't want planner to pick bitmap scans instead of index scans
+# this is to make the tests consistent
+$node_subscriber->append_conf('postgresql.conf',
+ "enable_bitmapscan = off");
+$node_subscriber->start;
+
+my $publisher_connstr = $node_publisher->connstr . ' dbname=postgres';
+my $appname = 'tap_sub';
+
+# ====================================================================
+# Testcase start: SUBSCRIPTION USES INDEX
+#
+# Basic test where the subscriber uses index
+# and only updates 1 row and deletes
+# 1 other row
+#
+
+# create tables pub and sub
+$node_publisher->safe_psql('postgres',
+ "CREATE TABLE test_replica_id_full (x int)");
+$node_publisher->safe_psql('postgres',
+ "ALTER TABLE test_replica_id_full REPLICA IDENTITY FULL;");
+$node_subscriber->safe_psql('postgres',
+ "CREATE TABLE test_replica_id_full (x int)");
+$node_subscriber->safe_psql('postgres',
+ "CREATE INDEX test_replica_id_full_idx ON test_replica_id_full(x)");
+
+# insert some initial data
+$node_publisher->safe_psql('postgres',
+ "INSERT INTO test_replica_id_full SELECT i FROM generate_series(0,21)i;");
+
+# create pub/sub
+$node_publisher->safe_psql('postgres',
+ "CREATE PUBLICATION tap_pub_rep_full FOR TABLE test_replica_id_full");
+$node_subscriber->safe_psql('postgres',
+ "CREATE SUBSCRIPTION tap_sub_rep_full CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub_rep_full"
+);
+
+# wait for initial table synchronization to finish
+$node_subscriber->wait_for_subscription_sync;
+
+$node_publisher->safe_psql('postgres',
+ "UPDATE test_replica_id_full SET x = x + 1 WHERE x = 15;");
+$node_publisher->wait_for_catchup($appname);
+
+# wait until the index is used on the subscriber
+$node_subscriber->poll_query_until(
+ 'postgres', q{select (idx_scan = 1) from pg_stat_all_indexes where indexrelname = 'test_replica_id_full_idx';}
+) or die "Timed out while waiting for check subscriber tap_sub_rep_full updates one row via index";
+
+$node_publisher->safe_psql('postgres',
+ "DELETE FROM test_replica_id_full WHERE x = 20;");
+$node_publisher->wait_for_catchup($appname);
+
+# wait until the index is used on the subscriber
+$node_subscriber->poll_query_until(
+ 'postgres', q{select (idx_scan = 2) from pg_stat_all_indexes where indexrelname = 'test_replica_id_full_idx';}
+) or die "Timed out while waiting for check subscriber tap_sub_rep_full deletes one row via index";
+
+# make sure that the subscriber has the correct data
+my $result = $node_subscriber->safe_psql('postgres',
+ "SELECT count(DISTINCT x) FROM test_replica_id_full");
+is($result, qq(20), 'ensure subscriber has the correct data at the end of the test');
+
+# cleanup pub
+$node_publisher->safe_psql('postgres', "DROP PUBLICATION tap_pub_rep_full");
+$node_publisher->safe_psql('postgres', "DROP TABLE test_replica_id_full");
+# cleanup sub
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub_rep_full");
+$node_subscriber->safe_psql('postgres', "DROP TABLE test_replica_id_full");
+
+# Testcase end: SUBSCRIPTION USES INDEX
+# ====================================================================
+
+# ====================================================================
+# Testcase start: SUBSCRIPTION RE-CALCULATES INDEX AFTER CREATE/DROP INDEX
+#
+# The subscription should react if an index is dropped or recreated.
+# This test ensures that after CREATE INDEX, the subscriber can automatically
+# use one of the indexes (provided that it fulfils the requirements).
+# Similarly, after DROP index, the subscriber can automatically switch to
+# sequential scan
+
+# create tables pub and sub
+$node_publisher->safe_psql('postgres',
+ "CREATE TABLE test_replica_id_full (x int NOT NULL, y int)");
+$node_publisher->safe_psql('postgres',
+ "ALTER TABLE test_replica_id_full REPLICA IDENTITY FULL;");
+$node_subscriber->safe_psql('postgres',
+ "CREATE TABLE test_replica_id_full (x int NOT NULL, y int)");
+
+# insert some initial data
+$node_publisher->safe_psql('postgres',
+ "INSERT INTO test_replica_id_full SELECT i, i FROM generate_series(0,2100)i;");
+
+# create pub/sub
+$node_publisher->safe_psql('postgres',
+ "CREATE PUBLICATION tap_pub_rep_full FOR TABLE test_replica_id_full");
+$node_subscriber->safe_psql('postgres',
+ "CREATE SUBSCRIPTION tap_sub_rep_full CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub_rep_full"
+);
+
+# wait for initial table synchronization to finish
+$node_subscriber->wait_for_subscription_sync;
+
+# now, create index and see that the index is used
+$node_subscriber->safe_psql('postgres',
+ "CREATE INDEX test_replica_id_full_idx ON test_replica_id_full(x)");
+
+# wait until the index is created
+$node_subscriber->poll_query_until(
+ 'postgres', q{select count(*)=1 from pg_stat_all_indexes where indexrelname = 'test_replica_id_full_idx';}
+) or die "Timed out while waiting for creating index test_replica_id_full_idx";
+
+$node_publisher->safe_psql('postgres',
+ "UPDATE test_replica_id_full SET x = x + 1 WHERE x = 15;");
+$node_publisher->wait_for_catchup($appname);
+
+
+# wait until the index is used on the subscriber
+$node_subscriber->poll_query_until(
+ 'postgres', q{select (idx_scan = 1) from pg_stat_all_indexes where indexrelname = 'test_replica_id_full_idx';}
+) or die "Timed out while waiting for check subscriber tap_sub_rep_full updates one row via index";
+
+
+# now, ingest more data and create index on column y which has higher cardinality
+# so that the future commands use the index on column y
+$node_publisher->safe_psql('postgres',
+ "INSERT INTO test_replica_id_full SELECT 50, i FROM generate_series(0,3100)i;");
+$node_subscriber->safe_psql('postgres',
+ "CREATE INDEX test_replica_id_full_idy ON test_replica_id_full(y)");
+
+# wait until the index is created
+$node_subscriber->poll_query_until(
+ 'postgres', q{select count(*)=1 from pg_stat_all_indexes where indexrelname = 'test_replica_id_full_idy';}
+) or die "Timed out while waiting for creating index test_replica_id_full_idy";
+
+# now, the update should use the test_replica_id_full_idy index
+$node_publisher->safe_psql('postgres',
+ "UPDATE test_replica_id_full SET y = y + 1 WHERE y = 3000;");
+$node_publisher->wait_for_catchup($appname);
+
+# wait until the index is used on the subscriber
+$node_subscriber->poll_query_until(
+ 'postgres', q{select count(idx_scan) = 2 from pg_stat_all_indexes where indexrelname IN ('test_replica_id_full_idy', 'test_replica_id_full_idx');}
+) or die "Timed out while waiting for check subscriber tap_sub_rep_full deletes one row via index";
+
+# let's also test dropping test_replica_id_full_idy and show that
+# it triggers re-calculation of the index, hence use test_replica_id_full_idx
+$node_subscriber->safe_psql('postgres',
+ "DROP INDEX test_replica_id_full_idy;");
+$node_publisher->safe_psql('postgres',
+ "UPDATE test_replica_id_full SET x = x + 1 WHERE x = 25;");
+$node_publisher->wait_for_catchup($appname);
+
+# make sure that the subscriber has the correct data
+$result = $node_subscriber->safe_psql('postgres',
+ "SELECT count(*) FROM test_replica_id_full WHERE x = 15 OR x = 25 OR y = 3000;");
+is($result, qq(0), 'ensure subscriber has the correct data at the end of the test');
+
+# cleanup pub
+$node_publisher->safe_psql('postgres', "DROP PUBLICATION tap_pub_rep_full");
+$node_publisher->safe_psql('postgres', "DROP TABLE test_replica_id_full");
+# cleanup sub
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub_rep_full");
+$node_subscriber->safe_psql('postgres', "DROP TABLE test_replica_id_full");
+
+# Testcase end: SUBSCRIPTION RE-CALCULATES INDEX AFTER CREATE/DROP INDEX
+# ====================================================================
+
+# ====================================================================
+# Testcase start: SUBSCRIPTION USES INDEX UPDATEs MULTIPLE ROWS
+#
+# Basic test where the subscriber uses index
+# and updates 50 rows
+#
+
+# create tables pub and sub
+$node_publisher->safe_psql('postgres',
+ "CREATE TABLE test_replica_id_full (x int)");
+$node_publisher->safe_psql('postgres',
+ "ALTER TABLE test_replica_id_full REPLICA IDENTITY FULL;");
+$node_subscriber->safe_psql('postgres',
+ "CREATE TABLE test_replica_id_full (x int)");
+$node_subscriber->safe_psql('postgres',
+ "CREATE INDEX test_replica_id_full_idx ON test_replica_id_full(x)");
+
+# insert some initial data within the range 0-19
+$node_publisher->safe_psql('postgres',
+ "INSERT INTO test_replica_id_full SELECT i%20 FROM generate_series(0,1000)i;"
+);
+
+# create pub/sub
+$node_publisher->safe_psql('postgres',
+ "CREATE PUBLICATION tap_pub_rep_full FOR TABLE test_replica_id_full");
+$node_subscriber->safe_psql('postgres',
+ "CREATE SUBSCRIPTION tap_sub_rep_full CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub_rep_full"
+);
+
+# wait for initial table synchronization to finish
+$node_subscriber->wait_for_subscription_sync;
+
+# updates 50 rows
+$node_publisher->safe_psql('postgres',
+ "UPDATE test_replica_id_full SET x = x + 1 WHERE x = 15;");
+$node_publisher->wait_for_catchup($appname);
+
+# wait until the index is used on the subscriber
+$node_subscriber->poll_query_until(
+ 'postgres', q{select (idx_scan = 50) from pg_stat_all_indexes where indexrelname = 'test_replica_id_full_idx';}
+) or die "Timed out while waiting for check subscriber tap_sub_rep_full updates 50 rows via index";
+
+# make sure that the subscriber has the correct data
+$result = $node_subscriber->safe_psql('postgres',
+ "select count(*) from test_replica_id_full where x = 15;");
+is($result, qq(0), 'ensure subscriber has the correct data at the end of the test');
+
+# cleanup pub
+$node_publisher->safe_psql('postgres', "DROP PUBLICATION tap_pub_rep_full");
+$node_publisher->safe_psql('postgres', "DROP TABLE test_replica_id_full");
+# cleanup sub
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub_rep_full");
+$node_subscriber->safe_psql('postgres', "DROP TABLE test_replica_id_full");
+
+# Testcase end: SUBSCRIPTION USES INDEX UPDATEs MULTIPLE ROWS
+# ====================================================================
+
+
+# ====================================================================
+# Testcase start: SUBSCRIPTION USES INDEX WITH MULTIPLE COLUMNS
+#
+# Basic test where the subscriber uses index
+# and deletes 200 rows
+#
+
+# create tables pub and sub
+$node_publisher->safe_psql('postgres',
+ "CREATE TABLE test_replica_id_full (x int, y text)");
+$node_publisher->safe_psql('postgres',
+ "ALTER TABLE test_replica_id_full REPLICA IDENTITY FULL;");
+$node_subscriber->safe_psql('postgres',
+ "CREATE TABLE test_replica_id_full (x int, y text)");
+$node_subscriber->safe_psql('postgres',
+ "CREATE INDEX test_replica_id_full_idx ON test_replica_id_full(x,y)");
+
+# insert some initial data within the range 0-9
+$node_publisher->safe_psql('postgres',
+ "INSERT INTO test_replica_id_full SELECT (i%10), (i%10)::text FROM generate_series(0,1000)i;"
+);
+
+# create pub/sub
+$node_publisher->safe_psql('postgres',
+ "CREATE PUBLICATION tap_pub_rep_full FOR TABLE test_replica_id_full");
+$node_subscriber->safe_psql('postgres',
+ "CREATE SUBSCRIPTION tap_sub_rep_full CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub_rep_full"
+);
+
+# wait for initial table synchronization to finish
+$node_subscriber->wait_for_subscription_sync;
+
+# deletes 200 rows
+$node_publisher->safe_psql('postgres',
+ "DELETE FROM test_replica_id_full WHERE x IN (5, 6);");
+
+# wait until the index is used on the subscriber
+$node_subscriber->poll_query_until(
+ 'postgres', q{select (idx_scan = 200) from pg_stat_all_indexes where indexrelname = 'test_replica_id_full_idx';}
+) or die "Timed out while waiting for check subscriber tap_sub_rep_full deletes 200 rows via index";
+
+# make sure that the subscriber has the correct data
+$result = $node_subscriber->safe_psql('postgres',
+ "select count(*) from test_replica_id_full where x in (5, 6);");
+is($result, qq(0), 'ensure subscriber has the correct data at the end of the test');
+
+# cleanup pub
+$node_publisher->safe_psql('postgres', "DROP PUBLICATION tap_pub_rep_full");
+$node_publisher->safe_psql('postgres', "DROP TABLE test_replica_id_full");
+# cleanup sub
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub_rep_full");
+$node_subscriber->safe_psql('postgres', "DROP TABLE test_replica_id_full");
+
+# Testcase end: SUBSCRIPTION USES INDEX WITH MULTIPLE COLUMNS
+# ====================================================================
+
+# ====================================================================
+# Testcase start: SUBSCRIPTION USES INDEX WITH DROPPED COLUMNS
+#
+# Basic test where the subscriber uses index
+# and updates multiple rows with a table that has
+# dropped columns
+#
+
+# create tables pub and sub
+$node_publisher->safe_psql('postgres',
+ "CREATE TABLE test_replica_id_full (drop_1 jsonb, x int, drop_2 point, y text, drop_3 timestamptz)"
+);
+$node_publisher->safe_psql('postgres',
+ "ALTER TABLE test_replica_id_full DROP COLUMN drop_1");
+$node_publisher->safe_psql('postgres',
+ "ALTER TABLE test_replica_id_full DROP COLUMN drop_2");
+$node_publisher->safe_psql('postgres',
+ "ALTER TABLE test_replica_id_full DROP COLUMN drop_3");
+$node_publisher->safe_psql('postgres',
+ "ALTER TABLE test_replica_id_full REPLICA IDENTITY FULL;");
+
+$node_subscriber->safe_psql('postgres',
+ "CREATE TABLE test_replica_id_full (drop_1 jsonb, x int, drop_2 point, y text, drop_3 timestamptz)"
+);
+$node_subscriber->safe_psql('postgres',
+ "ALTER TABLE test_replica_id_full DROP COLUMN drop_1");
+$node_subscriber->safe_psql('postgres',
+ "ALTER TABLE test_replica_id_full DROP COLUMN drop_2");
+$node_subscriber->safe_psql('postgres',
+ "ALTER TABLE test_replica_id_full DROP COLUMN drop_3");
+
+$node_subscriber->safe_psql('postgres',
+ "CREATE INDEX test_replica_id_full_idx ON test_replica_id_full(x,y)");
+
+# insert some initial data within the range 0-9
+$node_publisher->safe_psql('postgres',
+ "INSERT INTO test_replica_id_full SELECT (i%10), (i%10)::text FROM generate_series(0,1000)i;"
+);
+
+# create pub/sub
+$node_publisher->safe_psql('postgres',
+ "CREATE PUBLICATION tap_pub_rep_full FOR TABLE test_replica_id_full");
+$node_subscriber->safe_psql('postgres',
+ "CREATE SUBSCRIPTION tap_sub_rep_full CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub_rep_full"
+);
+
+# wait for initial table synchronization to finish
+$node_subscriber->wait_for_subscription_sync;
+
+# updates 200 rows
+$node_publisher->safe_psql('postgres',
+ "UPDATE test_replica_id_full SET x = x + 1 WHERE x IN (5, 6);");
+
+# wait until the index is used on the subscriber
+$node_subscriber->poll_query_until(
+ 'postgres', q{select (idx_scan = 200) from pg_stat_all_indexes where indexrelname = 'test_replica_id_full_idx';}
+) or die "Timed out while waiting for check subscriber tap_sub_rep_full updates 200 rows via index";
+
+# make sure that the subscriber has the correct data
+$result = $node_subscriber->safe_psql('postgres',
+ "select sum(x+y::int) from test_replica_id_full;");
+is($result, qq(9200), 'ensure subscriber has the correct data at the end of the test');
+
+# cleanup pub
+$node_publisher->safe_psql('postgres', "DROP PUBLICATION tap_pub_rep_full");
+$node_publisher->safe_psql('postgres', "DROP TABLE test_replica_id_full");
+# cleanup sub
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub_rep_full");
+$node_subscriber->safe_psql('postgres', "DROP TABLE test_replica_id_full");
+
+# Testcase end: SUBSCRIPTION USES INDEX WITH DROPPED COLUMNS
+# ====================================================================
+
+# ====================================================================
+# Testcase start: SUBSCRIPTION USES INDEX ON PARTITIONED TABLES
+
+# create tables pub and sub
+$node_publisher->safe_psql('postgres',
+ "CREATE TABLE users_table_part(user_id bigint, value_1 int, value_2 int) PARTITION BY RANGE (value_1);"
+);
+$node_publisher->safe_psql('postgres',
+ "CREATE TABLE users_table_part_0 PARTITION OF users_table_part FOR VALUES FROM (0) TO (10);"
+);
+$node_publisher->safe_psql('postgres',
+ "CREATE TABLE users_table_part_1 PARTITION OF users_table_part FOR VALUES FROM (10) TO (20);"
+);
+
+$node_publisher->safe_psql('postgres',
+ "ALTER TABLE users_table_part REPLICA IDENTITY FULL;");
+$node_publisher->safe_psql('postgres',
+ "ALTER TABLE users_table_part_0 REPLICA IDENTITY FULL;");
+$node_publisher->safe_psql('postgres',
+ "ALTER TABLE users_table_part_1 REPLICA IDENTITY FULL;");
+
+$node_subscriber->safe_psql('postgres',
+ "CREATE TABLE users_table_part(user_id bigint, value_1 int, value_2 int) PARTITION BY RANGE (value_1);"
+);
+$node_subscriber->safe_psql('postgres',
+ "CREATE TABLE users_table_part_0 PARTITION OF users_table_part FOR VALUES FROM (0) TO (10);"
+);
+$node_subscriber->safe_psql('postgres',
+ "CREATE TABLE users_table_part_1 PARTITION OF users_table_part FOR VALUES FROM (10) TO (20);"
+);
+$node_subscriber->safe_psql('postgres',
+ "CREATE INDEX users_table_part_idx ON users_table_part(user_id, value_1)"
+);
+
+# insert some initial data
+$node_publisher->safe_psql('postgres',
+ "INSERT INTO users_table_part SELECT (i%100), (i%20), i FROM generate_series(0,1000)i;"
+);
+
+# create pub/sub
+$node_publisher->safe_psql('postgres',
+ "CREATE PUBLICATION tap_pub_rep_full FOR TABLE users_table_part");
+$node_subscriber->safe_psql('postgres',
+ "CREATE SUBSCRIPTION tap_sub_rep_full CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub_rep_full"
+);
+
+# wait for initial table synchronization to finish
+$node_subscriber->wait_for_subscription_sync;
+
+# updates rows and moves between partitions
+$node_publisher->safe_psql('postgres',
+ "UPDATE users_table_part SET value_1 = 0 WHERE user_id = 4;");
+
+# wait until the index is used on the subscriber
+$node_subscriber->poll_query_until(
+ 'postgres', q{select sum(idx_scan)=10 from pg_stat_all_indexes where indexrelname ilike 'users_table_part_%';}
+) or die "Timed out while waiting for updates on partitioned table with index";
+
+# deletes rows and moves between partitions
+$node_publisher->safe_psql('postgres',
+ "DELETE FROM users_table_part WHERE user_id = 1 and value_1 = 1;");
+$node_publisher->safe_psql('postgres',
+ "DELETE FROM users_table_part WHERE user_id = 12 and value_1 = 12;");
+
+# wait until the index is used on the subscriber
+$node_subscriber->poll_query_until(
+ 'postgres', q{select sum(idx_scan)=30 from pg_stat_all_indexes where indexrelname ilike 'users_table_part_%';}
+) or die "Timed out while waiting for check subscriber tap_sub_rep_full updates partitioned table";
+
+# make sure that the subscriber has the correct data
+$result = $node_subscriber->safe_psql('postgres',
+ "select sum(user_id+value_1+value_2) from users_table_part;");
+is($result, qq(550070), 'ensure subscriber has the correct data at the end of the test');
+$result = $node_subscriber->safe_psql('postgres',
+ "select count(DISTINCT(user_id,value_1, value_2)) from users_table_part;");
+is($result, qq(981), 'ensure subscriber has the correct data at the end of the test');
+
+# cleanup pub
+$node_publisher->safe_psql('postgres', "DROP PUBLICATION tap_pub_rep_full");
+$node_publisher->safe_psql('postgres', "DROP TABLE users_table_part");
+
+# cleanup sub
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub_rep_full");
+$node_subscriber->safe_psql('postgres', "DROP TABLE users_table_part");
+
+# Testcase end: SUBSCRIPTION USES INDEX ON PARTITIONED TABLES
+# ====================================================================
+
+# ====================================================================
+# Testcase start: SUBSCRIPTION DOES NOT USE PARTIAL INDEX
+
+# create tables pub and sub
+$node_publisher->safe_psql('postgres',
+ "CREATE TABLE test_replica_id_full_part_index (x int);");
+$node_publisher->safe_psql('postgres',
+ "ALTER TABLE test_replica_id_full_part_index REPLICA IDENTITY FULL;");
+$node_subscriber->safe_psql('postgres',
+ "CREATE TABLE test_replica_id_full_part_index (x int);");
+$node_subscriber->safe_psql('postgres',
+ "CREATE INDEX test_replica_id_full_part_idx ON test_replica_id_full_part_index(x) WHERE (x = 5);");
+
+# insert some initial data
+$node_publisher->safe_psql('postgres',
+ "INSERT INTO test_replica_id_full_part_index SELECT i FROM generate_series(0,21)i;");
+
+# create pub/sub
+$node_publisher->safe_psql('postgres',
+ "CREATE PUBLICATION tap_pub_rep_full FOR TABLE test_replica_id_full_part_index");
+$node_subscriber->safe_psql('postgres',
+ "CREATE SUBSCRIPTION tap_sub_rep_full CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub_rep_full"
+);
+
+# wait for initial table synchronization to finish
+$node_subscriber->wait_for_subscription_sync;
+
+# update 2 rows, one of them is indexed
+$node_publisher->safe_psql('postgres',
+ "UPDATE test_replica_id_full_part_index SET x = x + 1 WHERE x = 5;");
+$node_publisher->safe_psql('postgres',
+ "UPDATE test_replica_id_full_part_index SET x = x + 1 WHERE x = 15;");
+$node_publisher->wait_for_catchup($appname);
+
+# make sure that the index is not used
+$result = $node_subscriber->safe_psql('postgres',
+ "select idx_scan from pg_stat_all_indexes where indexrelname = 'test_replica_id_full_part_idx'");
+is($result, qq(0), 'ensure subscriber tap_sub_rep_full updates one row via seq. scan with with partial index');
+
+# make sure that the subscriber has the correct data
+$result = $node_subscriber->safe_psql('postgres',
+ "SELECT count(*) FROM test_replica_id_full_part_index;");
+is($result, qq(22), 'ensure subscriber has the correct data at the end of the test');
+
+$result = $node_subscriber->safe_psql('postgres',
+ "SELECT count(DISTINCT x) FROM test_replica_id_full_part_index;");
+is($result, qq(20), 'ensure subscriber has the correct data at the end of the test');
+
+# cleanup pub
+$node_publisher->safe_psql('postgres', "DROP PUBLICATION tap_pub_rep_full");
+$node_publisher->safe_psql('postgres', "DROP TABLE test_replica_id_full_part_index");
+# cleanup sub
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub_rep_full");
+$node_subscriber->safe_psql('postgres', "DROP TABLE test_replica_id_full_part_index");
+
+# Testcase end: SUBSCRIPTION DOES NOT USE PARTIAL INDEX
+# ====================================================================
+
+
+# ====================================================================
+# Testcase start: SUBSCRIPTION DOES NOT USE INDEXES WITH ONLY EXPRESSIONS
+
+# create tables pub and sub
+$node_publisher->safe_psql('postgres',
+ "CREATE TABLE people (firstname text, lastname text);");
+$node_publisher->safe_psql('postgres',
+ "ALTER TABLE people REPLICA IDENTITY FULL;");
+$node_subscriber->safe_psql('postgres',
+ "CREATE TABLE people (firstname text, lastname text);");
+$node_subscriber->safe_psql('postgres',
+ "CREATE INDEX people_names ON people ((firstname || ' ' || lastname));");
+
+# insert some initial data
+$node_publisher->safe_psql('postgres',
+ "INSERT INTO people SELECT 'first_name_' || i::text, 'last_name_' || i::text FROM generate_series(0,200)i;");
+
+# create pub/sub
+$node_publisher->safe_psql('postgres',
+ "CREATE PUBLICATION tap_pub_rep_full FOR TABLE people");
+$node_subscriber->safe_psql('postgres',
+ "CREATE SUBSCRIPTION tap_sub_rep_full CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub_rep_full"
+);
+
+# wait for initial table synchronization to finish
+$node_subscriber->wait_for_subscription_sync;
+
+# update 2 rows
+$node_publisher->safe_psql('postgres',
+ "UPDATE people SET firstname = 'Nan' WHERE firstname = 'first_name_1';");
+$node_publisher->safe_psql('postgres',
+ "UPDATE people SET firstname = 'Nan' WHERE firstname = 'first_name_2' AND lastname = 'last_name_2';");
+
+# make sure the index is not used on the subscriber
+$result = $node_subscriber->safe_psql('postgres',
+ "select idx_scan from pg_stat_all_indexes where indexrelname = 'people_names'");
+is($result, qq(0), 'ensure subscriber tap_sub_rep_full updates two rows via seq. scan with index on expressions');
+
+$node_publisher->safe_psql('postgres',
+ "DELETE FROM people WHERE firstname = 'first_name_3';");
+$node_publisher->safe_psql('postgres',
+ "DELETE FROM people WHERE firstname = 'first_name_4' AND lastname = 'last_name_4';");
+
+# make sure the index is not used on the subscriber
+$result = $node_subscriber->safe_psql('postgres',
+ "select idx_scan from pg_stat_all_indexes where indexrelname = 'people_names'");
+is($result, qq(0), 'ensure subscriber tap_sub_rep_full updates two rows via seq. scan with index on expressions');
+
+# make sure that the subscriber has the correct data
+$result = $node_subscriber->safe_psql('postgres',
+ "SELECT count(*) FROM people;");
+is($result, qq(199), 'ensure subscriber has the correct data at the end of the test');
+
+# cleanup pub
+$node_publisher->safe_psql('postgres', "DROP PUBLICATION tap_pub_rep_full");
+$node_publisher->safe_psql('postgres', "DROP TABLE people");
+# cleanup sub
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub_rep_full");
+$node_subscriber->safe_psql('postgres', "DROP TABLE people");
+
+# Testcase end: SUBSCRIPTION DOES NOT USE INDEXES WITH ONLY EXPRESSIONS
+# ====================================================================
+
+# ====================================================================
+# Testcase start: SUBSCRIPTION CAN USE INDEXES WITH EXPRESSIONS AND COLUMNS
+
+# create tables pub and sub
+$node_publisher->safe_psql('postgres',
+ "CREATE TABLE people (firstname text, lastname text);");
+$node_publisher->safe_psql('postgres',
+ "ALTER TABLE people REPLICA IDENTITY FULL;");
+$node_subscriber->safe_psql('postgres',
+ "CREATE TABLE people (firstname text, lastname text);");
+$node_subscriber->safe_psql('postgres',
+ "CREATE INDEX people_names ON people (firstname, lastname, (firstname || ' ' || lastname));");
+
+# insert some initial data
+$node_publisher->safe_psql('postgres',
+ "INSERT INTO people SELECT 'first_name_' || i::text, 'last_name_' || i::text FROM generate_series(0, 200)i;");
+
+# create pub/sub
+$node_publisher->safe_psql('postgres',
+ "CREATE PUBLICATION tap_pub_rep_full FOR TABLE people");
+$node_subscriber->safe_psql('postgres',
+ "CREATE SUBSCRIPTION tap_sub_rep_full CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub_rep_full"
+);
+
+# wait for initial table synchronization to finish
+$node_subscriber->wait_for_subscription_sync;
+
+# update 2 rows
+$node_publisher->safe_psql('postgres',
+ "UPDATE people SET firstname = 'Nan' WHERE firstname = 'first_name_1';");
+$node_publisher->safe_psql('postgres',
+ "UPDATE people SET firstname = 'Nan' WHERE firstname = 'first_name_3' AND lastname = 'last_name_3';");
+
+# wait until the index is used on the subscriber
+$node_subscriber->poll_query_until(
+ 'postgres', q{select idx_scan=2 from pg_stat_all_indexes where indexrelname = 'people_names';}
+) or die "Timed out while waiting for check subscriber tap_sub_rep_full updates two rows via index scan with index on expressions and columns";
+
+$node_publisher->safe_psql('postgres',
+ "DELETE FROM people WHERE firstname = 'Nan';");
+
+# wait until the index is used on the subscriber
+$node_subscriber->poll_query_until(
+ 'postgres', q{select idx_scan=4 from pg_stat_all_indexes where indexrelname = 'people_names';}
+) or die "Timed out while waiting for check subscriber tap_sub_rep_full deletes two rows via index scan with index on expressions and columns";
+
+# make sure that the subscriber has the correct data
+$result = $node_subscriber->safe_psql('postgres',
+ "SELECT count(*) FROM people;");
+is($result, qq(199), 'ensure subscriber has the correct data at the end of the test');
+
+$result = $node_subscriber->safe_psql('postgres',
+ "SELECT count(*) FROM people WHERE firstname = 'NaN';");
+is($result, qq(0), 'ensure subscriber has the correct data at the end of the test');
+
+# cleanup pub
+$node_publisher->safe_psql('postgres', "DROP PUBLICATION tap_pub_rep_full");
+$node_publisher->safe_psql('postgres', "DROP TABLE people");
+# cleanup sub
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub_rep_full");
+$node_subscriber->safe_psql('postgres', "DROP TABLE people");
+
+# Testcase end: SUBSCRIPTION CAN USE INDEXES WITH EXPRESSIONS AND COLUMNS
+# ====================================================================
+
+# ====================================================================
+# Testcase start: Some NULL values
+
+$node_publisher->safe_psql('postgres',
+ "CREATE TABLE test_replica_id_full (x int);"
+);
+
+$node_publisher->safe_psql('postgres',
+ "ALTER TABLE test_replica_id_full REPLICA IDENTITY FULL;");
+
+$node_subscriber->safe_psql('postgres',
+ "CREATE TABLE test_replica_id_full (x int, y int);"
+);
+
+$node_subscriber->safe_psql('postgres',
+ "CREATE INDEX test_replica_id_full_idx ON test_replica_id_full(x,y);"
+);
+
+# create pub/sub
+$node_publisher->safe_psql('postgres',
+ "CREATE PUBLICATION tap_pub_rep_full FOR TABLE test_replica_id_full");
+
+$node_subscriber->safe_psql('postgres',
+ "CREATE SUBSCRIPTION tap_sub_rep_full CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub_rep_full"
+);
+
+# wait for initial table synchronization to finish
+$node_subscriber->wait_for_subscription_sync;
+
+$node_publisher->safe_psql('postgres',
+ "INSERT INTO test_replica_id_full VALUES (1), (2), (3);");
+$node_publisher->safe_psql('postgres',
+ "UPDATE test_replica_id_full SET x = x + 1 WHERE x = 1;");
+$node_publisher->safe_psql('postgres',
+ "UPDATE test_replica_id_full SET x = x + 1 WHERE x = 3;");
+
+# check if the index is used even when the index has NULL values
+$node_subscriber->poll_query_until(
+ 'postgres', q{select idx_scan=2 from pg_stat_all_indexes where indexrelname = 'test_replica_id_full_idx';}
+) or die "Timed out while waiting for check subscriber tap_sub_rep_full updates test_replica_id_full table";
+
+# make sure that the subscriber has the correct data
+$result = $node_subscriber->safe_psql('postgres',
+ "select sum(x) from test_replica_id_full WHERE y IS NULL;");
+is($result, qq(8), 'ensure subscriber has the correct data at the end of the test');
+
+# make sure that the subscriber has the correct data
+$result = $node_subscriber->safe_psql('postgres',
+ "select count(*) from test_replica_id_full WHERE y IS NULL;");
+is($result, qq(3), 'ensure subscriber has the correct data at the end of the test');
+
+# cleanup pub
+$node_publisher->safe_psql('postgres', "DROP PUBLICATION tap_pub_rep_full");
+$node_publisher->safe_psql('postgres', "DROP TABLE test_replica_id_full");
+
+# cleanup sub
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub_rep_full");
+$node_subscriber->safe_psql('postgres', "DROP TABLE test_replica_id_full");
+
+# Testcase end: Some NULL values
+# ====================================================================
+
+# ====================================================================
+# Testcase start: Unique index that is not primary key or replica identity
+
+$node_publisher->safe_psql('postgres',
+ "CREATE TABLE test_replica_id_full (x int, y int);"
+);
+
+$node_publisher->safe_psql('postgres',
+ "ALTER TABLE test_replica_id_full REPLICA IDENTITY FULL;");
+
+$node_subscriber->safe_psql('postgres',
+ "CREATE TABLE test_replica_id_full (x int, y int);"
+);
+
+$node_subscriber->safe_psql('postgres',
+ "CREATE UNIQUE INDEX test_replica_id_full_unique_idx ON test_replica_id_full(x);"
+);
+
+# create pub/sub
+$node_publisher->safe_psql('postgres',
+ "CREATE PUBLICATION tap_pub_rep_full FOR TABLE test_replica_id_full");
+
+$node_subscriber->safe_psql('postgres',
+ "CREATE SUBSCRIPTION tap_sub_rep_full CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub_rep_full"
+);
+
+# wait for initial table synchronization to finish
+$node_subscriber->wait_for_subscription_sync;
+
+$node_publisher->safe_psql('postgres',
+ "INSERT INTO test_replica_id_full (x, y) VALUES (NULL, 1), (NULL, 2), (NULL, 3);");
+$node_publisher->safe_psql('postgres',
+ "UPDATE test_replica_id_full SET x = 1 WHERE y = 2;");
+
+# check if the index is used even when the index has NULL values
+$node_subscriber->poll_query_until(
+ 'postgres', q{select idx_scan=1 from pg_stat_all_indexes where indexrelname = 'test_replica_id_full_unique_idx';}
+) or die "Timed out while waiting for check subscriber tap_sub_rep_full updates test_replica_id_full table";
+
+# make sure that the subscriber has the correct data
+$result = $node_subscriber->safe_psql('postgres',
+ "select sum(y) from test_replica_id_full;");
+is($result, qq(6), 'ensure subscriber has the correct data at the end of the test');
+
+# cleanup pub
+$node_publisher->safe_psql('postgres', "DROP PUBLICATION tap_pub_rep_full");
+$node_publisher->safe_psql('postgres', "DROP TABLE test_replica_id_full");
+
+# cleanup sub
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub_rep_full");
+$node_subscriber->safe_psql('postgres', "DROP TABLE test_replica_id_full");
+
+# Testcase end: Unique index that is not primary key or replica identity
+# ====================================================================
+
+
+
+# ====================================================================
+# Testcase start: SUBSCRIPTION BEHAVIOR WITH ENABLE_INDEXSCAN
+#
+# Even if enable_indexscan = false, we do use the primary keys, this
+# is the legacy behavior. However, we do not use non-primary/non replica
+# identity columns.
+#
+
+# create tables pub and sub
+$node_publisher->safe_psql('postgres',
+ "CREATE TABLE test_replica_id_full (x int NOT NULL)");
+$node_publisher->safe_psql('postgres',
+ "ALTER TABLE test_replica_id_full REPLICA IDENTITY FULL;");
+$node_subscriber->safe_psql('postgres',
+ "CREATE TABLE test_replica_id_full (x int NOT NULL)");
+$node_subscriber->safe_psql('postgres',
+ "CREATE INDEX test_replica_id_full_idx ON test_replica_id_full(x)");
+$node_subscriber->safe_psql('postgres',
+ "ALTER SYSTEM SET enable_indexscan TO off;");
+$node_subscriber->safe_psql('postgres',
+ "SELECT pg_reload_conf();");
+
+# insert some initial data
+$node_publisher->safe_psql('postgres',
+ "INSERT INTO test_replica_id_full SELECT i FROM generate_series(0,21)i;");
+
+# create pub/sub
+$node_publisher->safe_psql('postgres',
+ "CREATE PUBLICATION tap_pub_rep_full FOR TABLE test_replica_id_full");
+$node_subscriber->safe_psql('postgres',
+ "CREATE SUBSCRIPTION tap_sub_rep_full CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub_rep_full"
+);
+
+# wait for initial table synchronization to finish
+$node_subscriber->wait_for_subscription_sync;
+
+$node_publisher->safe_psql('postgres',
+ "UPDATE test_replica_id_full SET x = x + 10000 WHERE x = 15;");
+$node_publisher->wait_for_catchup($appname);
+
+# show that index is not used even when enable_indexscan=false
+$result = $node_subscriber->safe_psql('postgres',
+ "select idx_scan from pg_stat_all_indexes where indexrelname = 'test_replica_id_full_idx'");
+is($result, qq(0), 'ensure subscriber has not used index with enable_indexscan=false');
+
+# we are done with this index, drop to simplify the tests
+$node_subscriber->safe_psql('postgres',
+ "DROP INDEX test_replica_id_full_idx");
+
+# now, create a unique index and set the replica
+$node_publisher->safe_psql('postgres',
+ "CREATE UNIQUE INDEX test_replica_id_full_unique ON test_replica_id_full(x);");
+$node_subscriber->safe_psql('postgres',
+ "CREATE UNIQUE INDEX test_replica_id_full_unique ON test_replica_id_full(x);");
+
+$node_publisher->safe_psql('postgres',
+ "ALTER TABLE test_replica_id_full REPLICA IDENTITY USING INDEX test_replica_id_full_unique;");
+$node_subscriber->safe_psql('postgres',
+ "ALTER TABLE test_replica_id_full REPLICA IDENTITY USING INDEX test_replica_id_full_unique;");
+
+# wait for the synchronization to finish
+$node_subscriber->wait_for_subscription_sync;
+
+$node_publisher->safe_psql('postgres',
+ "UPDATE test_replica_id_full SET x = x + 10000 WHERE x = 14;");
+$node_publisher->wait_for_catchup($appname);
+
+# show that the unique index on replica identity is used even when enable_indexscan=false
+# this is a legacy behavior
+$node_subscriber->poll_query_until(
+ 'postgres', q{select (idx_scan=1) from pg_stat_all_indexes where indexrelname = 'test_replica_id_full_unique'}
+) or die "Timed out while waiting ensuring subscriber used unique index as replica identity even with enable_indexscan=false";
+
+$result = $node_subscriber->safe_psql('postgres',
+ "SELECT count(*) FROM test_replica_id_full WHERE x IN (14,15)");
+is($result, qq(0), 'ensure the results are accurate even with enable_indexscan=false');
+
+# cleanup pub
+$node_publisher->safe_psql('postgres', "DROP PUBLICATION tap_pub_rep_full");
+$node_publisher->safe_psql('postgres', "DROP TABLE test_replica_id_full");
+# cleanup sub
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub_rep_full");
+$node_subscriber->safe_psql('postgres', "DROP TABLE test_replica_id_full");
+
+$node_subscriber->safe_psql('postgres',
+ "ALTER SYSTEM RESET enable_indexscan;");
+$node_subscriber->safe_psql('postgres',
+ "SELECT pg_reload_conf();");
+
+# Testcase end: SUBSCRIPTION BEHAVIOR WITH ENABLE_INDEXSCAN
+# ====================================================================
+
+$node_subscriber->stop('fast');
+$node_publisher->stop('fast');
+
+done_testing();
--
2.34.1
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher
@ 2023-02-04 11:24 Amit Kapila <[email protected]>
parent: Önder Kalacı <[email protected]>
3 siblings, 1 reply; 18+ messages in thread
From: Amit Kapila @ 2023-02-04 11:24 UTC (permalink / raw)
To: Önder Kalacı <[email protected]>; +Cc: Marco Slot <[email protected]>; Tom Lane <[email protected]>; Andres Freund <[email protected]>; [email protected]; [email protected]; PostgreSQL Hackers <[email protected]>
On Thu, Feb 2, 2023 at 2:03 PM Önder Kalacı <[email protected]> wrote:
>
>>
>> and if there's more
>> than one candidate index pick any one. Additionally, we can allow
>> disabling the use of an index scan for this particular case. If we are
>> too worried about API change for allowing users to specify the index
>> then we can do that later or as a separate patch.
>>
>
> On v23, I dropped the planner support for picking the index. Instead, it simply
> iterates over the indexes and picks the first one that is suitable.
>
> I'm currently thinking on how to enable users to override this decision.
> One option I'm leaning towards is to add a syntax like the following:
>
> ALTER SUBSCRIPTION .. ALTER TABLE ... SET INDEX ...
>
> Though, that should probably be a seperate patch. I'm going to work
> on that, but still wanted to share v23 given picking the index sounds
> complementary, not strictly required at this point.
>
I agree that it could be a separate patch. However, do you think we
need some way to disable picking the index scan? This is to avoid
cases where sequence scan could be better or do we think there won't
exist such a case?
--
With Regards,
Amit Kapila.
^ permalink raw reply [nested|flat] 18+ messages in thread
* RE: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher
@ 2023-02-13 11:00 [email protected] <[email protected]>
parent: Önder Kalacı <[email protected]>
3 siblings, 1 reply; 18+ messages in thread
From: [email protected] @ 2023-02-13 11:00 UTC (permalink / raw)
To: Önder Kalacı <[email protected]>; Amit Kapila <[email protected]>; +Cc: Marco Slot <[email protected]>; Tom Lane <[email protected]>; Andres Freund <[email protected]>; [email protected] <[email protected]>; PostgreSQL Hackers <[email protected]>
On Thu, Feb 2, 2023 4:34 PM Önder Kalacı <[email protected]> wrote:
>
>>
>> and if there's more
>> than one candidate index pick any one. Additionally, we can allow
>> disabling the use of an index scan for this particular case. If we are
>> too worried about API change for allowing users to specify the index
>> then we can do that later or as a separate patch.
>>
>
> On v23, I dropped the planner support for picking the index. Instead, it simply
> iterates over the indexes and picks the first one that is suitable.
>
> I'm currently thinking on how to enable users to override this decision.
> One option I'm leaning towards is to add a syntax like the following:
>
> ALTER SUBSCRIPTION .. ALTER TABLE ... SET INDEX ...
>
> Though, that should probably be a seperate patch. I'm going to work
> on that, but still wanted to share v23 given picking the index sounds
> complementary, not strictly required at this point.
>
Thanks for your patch. Here are some comments.
1.
I noticed that get_usable_indexoid() is called in apply_handle_update_internal()
and apply_handle_delete_internal() to get the usable index. Could usableIndexOid
be a parameter of these two functions? Because we have got the
LogicalRepRelMapEntry when calling them and if we do so, we can get
usableIndexOid without get_usable_indexoid(). Otherwise for partitioned tables,
logicalrep_partition_open() is called in get_usable_indexoid() and searching
the entry via hash_search() will increase cost.
2.
+ * This attribute is an expression, and
+ * SuitableIndexPathsForRepIdentFull() was called earlier when the
+ * index for subscriber was selected. There, the indexes
+ * comprising *only* expressions have already been eliminated.
The comment looks need to be updated:
SuitableIndexPathsForRepIdentFull
->
FindUsableIndexForReplicaIdentityFull
3.
/* Build scankey for every attribute in the index. */
- for (attoff = 0; attoff < IndexRelationGetNumberOfKeyAttributes(idxrel); attoff++)
+ for (index_attoff = 0; index_attoff < IndexRelationGetNumberOfKeyAttributes(idxrel);
+ index_attoff++)
{
Should the comment be changed? Because we skip the attributes that are expressions.
4.
+ Assert(RelationGetReplicaIndex(rel) != RelationGetRelid(idxrel) &&
+ RelationGetPrimaryKeyIndex(rel) != RelationGetRelid(idxrel));
Maybe we can call the new function idxIsRelationIdentityOrPK()?
Regards,
Shi Yu
^ permalink raw reply [nested|flat] 18+ messages in thread
* RE: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher
@ 2023-02-14 09:35 [email protected] <[email protected]>
parent: [email protected] <[email protected]>
0 siblings, 0 replies; 18+ messages in thread
From: [email protected] @ 2023-02-14 09:35 UTC (permalink / raw)
To: [email protected] <[email protected]>; Önder Kalacı <[email protected]>; Amit Kapila <[email protected]>; +Cc: Marco Slot <[email protected]>; Tom Lane <[email protected]>; Andres Freund <[email protected]>; [email protected] <[email protected]>; PostgreSQL Hackers <[email protected]>
On Mon, Feb 13, 2023 7:01 PM [email protected] <[email protected]> wrote:
>
> On Thu, Feb 2, 2023 4:34 PM Önder Kalacı <[email protected]> wrote:
> >
> >>
> >> and if there's more
> >> than one candidate index pick any one. Additionally, we can allow
> >> disabling the use of an index scan for this particular case. If we are
> >> too worried about API change for allowing users to specify the index
> >> then we can do that later or as a separate patch.
> >>
> >
> > On v23, I dropped the planner support for picking the index. Instead, it simply
> > iterates over the indexes and picks the first one that is suitable.
> >
> > I'm currently thinking on how to enable users to override this decision.
> > One option I'm leaning towards is to add a syntax like the following:
> >
> > ALTER SUBSCRIPTION .. ALTER TABLE ... SET INDEX ...
> >
> > Though, that should probably be a seperate patch. I'm going to work
> > on that, but still wanted to share v23 given picking the index sounds
> > complementary, not strictly required at this point.
> >
>
> Thanks for your patch. Here are some comments.
>
Hi,
Here are some comments on the test cases.
1. in test case "SUBSCRIPTION RE-CALCULATES INDEX AFTER CREATE/DROP INDEX"
+# now, ingest more data and create index on column y which has higher cardinality
+# so that the future commands use the index on column y
+$node_publisher->safe_psql('postgres',
+ "INSERT INTO test_replica_id_full SELECT 50, i FROM generate_series(0,3100)i;");
+$node_subscriber->safe_psql('postgres',
+ "CREATE INDEX test_replica_id_full_idy ON test_replica_id_full(y)");
We don't pick the cheapest index in the current patch, so should we modify this
part of the test?
BTW, the following comment in FindLogicalRepUsableIndex() need to be changed,
too.
+ * We are looking for one more opportunity for using an index. If
+ * there are any indexes defined on the local relation, try to pick
+ * the cheapest index.
2. Is there any reasons why we need the test case "SUBSCRIPTION USES INDEX WITH
DROPPED COLUMNS"? Has there been a problem related to dropped columns before?
3. in test case "SUBSCRIPTION USES INDEX ON PARTITIONED TABLES"
+# deletes rows and moves between partitions
+$node_publisher->safe_psql('postgres',
+ "DELETE FROM users_table_part WHERE user_id = 1 and value_1 = 1;");
+$node_publisher->safe_psql('postgres',
+ "DELETE FROM users_table_part WHERE user_id = 12 and value_1 = 12;");
"moves between partitions" in the comment seems wrong.
4. in test case "SUBSCRIPTION DOES NOT USE INDEXES WITH ONLY EXPRESSIONS"
+# update 2 rows
+$node_publisher->safe_psql('postgres',
+ "UPDATE people SET firstname = 'Nan' WHERE firstname = 'first_name_1';");
+$node_publisher->safe_psql('postgres',
+ "UPDATE people SET firstname = 'Nan' WHERE firstname = 'first_name_2' AND lastname = 'last_name_2';");
+
+# make sure the index is not used on the subscriber
+$result = $node_subscriber->safe_psql('postgres',
+ "select idx_scan from pg_stat_all_indexes where indexrelname = 'people_names'");
+is($result, qq(0), 'ensure subscriber tap_sub_rep_full updates two rows via seq. scan with index on expressions');
+
I think it would be better to call wait_for_catchup() before the check because
we want to check the index is NOT used. Otherwise the check may pass because the
rows have not yet been updated on subscriber.
5. in test case "SUBSCRIPTION BEHAVIOR WITH ENABLE_INDEXSCAN"
+# show that index is not used even when enable_indexscan=false
+$result = $node_subscriber->safe_psql('postgres',
+ "select idx_scan from pg_stat_all_indexes where indexrelname = 'test_replica_id_full_idx'");
+is($result, qq(0), 'ensure subscriber has not used index with enable_indexscan=false');
Should we remove the word "even" in the comment?
6.
In each test case we re-create publications, subscriptions, and tables. Could we
create only one publication and one subscription at the beginning, and use them
in all test cases? I think this can save some time running the test file.
Regards,
Shi Yu
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher
@ 2023-02-15 02:16 Peter Smith <[email protected]>
parent: Önder Kalacı <[email protected]>
3 siblings, 0 replies; 18+ messages in thread
From: Peter Smith @ 2023-02-15 02:16 UTC (permalink / raw)
To: Önder Kalacı <[email protected]>; +Cc: Amit Kapila <[email protected]>; Marco Slot <[email protected]>; Tom Lane <[email protected]>; Andres Freund <[email protected]>; [email protected]; [email protected]; PostgreSQL Hackers <[email protected]>
Here are some review comments for patch v23.
======
General
1.
IIUC the previous logic for checking "cost" comparisons and selecting
the "cheapest" strategy is no longer present in the latest patch.
In that case, I think there are some leftover stale comments that need
changing. For example,
1a. Commit message:
"let the planner sub-modules compare the costs of index versus
sequential scan and choose the cheapest."
~
1b. Commit message:
"Finally, pick the cheapest `Path` among."
~
1c. FindLogicalRepUsableIndex function:
+ * We are looking for one more opportunity for using an index. If
+ * there are any indexes defined on the local relation, try to pick
+ * the cheapest index.
======
doc/src/sgml/logical-replication.sgml
If replica identity "full" is used, indexes can be used on the
subscriber side for seaching the rows. The index should be btree,
non-partial and have at least one column reference (e.g., should not
consists of only expressions). If there are no suitable indexes, the
search on the subscriber side is very inefficient and should only be
used as a fallback if no other solution is possible
2a.
Fixed typo "seaching", and minor rewording.
SUGGESTION
When replica identity "full" is specified, indexes can be used on the
subscriber side for searching the rows. These indexes should be btree,
non-partial and have at least one column reference (e.g., should not
consist of only expressions). If there are no such suitable indexes,
the search on the subscriber side can be very inefficient, therefore
replica identity "full" should only be used as a fallback if no other
solution is possible.
~
2b.
I know you are just following some existing text here, but IMO this
should probably refer to replica identity <literal>FULL</literal>
instead of "full".
======
src/backend/executor/execReplication.c
3. IdxIsRelationIdentityOrPK
+/*
+ * Given a relation and OID of an index, returns true if
+ * the index is relation's primary key's index or
+ * relaton's replica identity index.
+ *
+ * Returns false otherwise.
+ */
+static bool
+IdxIsRelationIdentityOrPK(Relation rel, Oid idxoid)
+{
+ Assert(OidIsValid(idxoid));
+
+ if (RelationGetReplicaIndex(rel) == idxoid ||
+ RelationGetPrimaryKeyIndex(rel) == idxoid)
+ return true;
+
+ return false;
3a.
Comment typo "relaton"
~
3b.
Code could be written using single like below if you wish (but see #2c)
return RelationGetReplicaIndex(rel) == idxoid ||
RelationGetPrimaryKeyIndex(rel) == idxoid;
~
3c.
Actually, RelationGetReplicaIndex and RelationGetPrimaryKeyIndex
implementations are very similar so it seemed inefficient to be
calling both of them. IMO it might be better to just make a new
relcache function IdxIsRelationIdentityOrPK(Relation rel, Oid idxoid).
This implementation will be similar to those others, but now you need
only to call the workhorse RelationGetIndexList *one* time.
~~~
4. RelationFindReplTupleByIndex
bool found;
+ TypeCacheEntry **eq = NULL; /* only used when the index is not repl. ident
+ * or pkey */
+ bool idxIsRelationIdentityOrPK;
If you change the comment to say "RI" instead of "repl. Ident" then it
can all fit on one line, which would be an improvement.
======
src/backend/replication/logical/relation.c
5.
#include "replication/logicalrelation.h"
#include "replication/worker_internal.h"
+#include "optimizer/cost.h"
#include "utils/inval.h"
Can that #include be added in alphabetical order like the others or not?
~~~
6. logicalrep_partition_open
+ /*
+ * Finding a usable index is an infrequent task. It occurs when an
+ * operation is first performed on the relation, or after invalidation of
+ * of the relation cache entry (such as ANALYZE or CREATE/DROP index on
+ * the relation).
+ */
+ entry->usableIndexOid = FindLogicalRepUsableIndex(partrel, remoterel);
+
Typo "of of the relation"
~~~
7. FindUsableIndexForReplicaIdentityFull
+static Oid
+FindUsableIndexForReplicaIdentityFull(Relation localrel)
+{
+ MemoryContext usableIndexContext;
+ MemoryContext oldctx;
+ Oid usableIndex;
+ Oid idxoid;
+ List *indexlist;
+ ListCell *lc;
+ Relation indexRelation;
+ IndexInfo *indexInfo;
+ bool is_btree;
+ bool is_partial;
+ bool is_only_on_expression;
It looks like some of these variables are only used within the scope
of the foreach loop, so I think that is where they should be declared.
~~~
8.
+ usableIndex = InvalidOid;
Might as well do that assignment at the declaration.
~~~
9. FindLogicalRepUsableIndex
+ /*
+ * Simple case, we already have a primary key or a replica identity index.
+ *
+ * Note that we do not use index scans below when enable_indexscan is
+ * false. Allowing primary key or replica identity even when index scan is
+ * disabled is the legacy behaviour. So we hesitate to move the below
+ * enable_indexscan check to be done earlier in this function.
+ */
+ idxoid = GetRelationIdentityOrPK(localrel);
+ if (OidIsValid(idxoid))
+ return idxoid;
+
+ /* If index scans are disabled, use a sequential scan */
+ if (!enable_indexscan)
+ return InvalidOid;
~
IMO that "Note" really belongs with the if (!enable)indexscan) more like this:
SUGGESTION
/*
* Simple case, we already have a primary key or a replica identity index.
*/
idxoid = GetRelationIdentityOrPK(localrel);
if (OidIsValid(idxoid))
return idxoid;
/*
* If index scans are disabled, use a sequential scan.
*
* Note we hesitate to move this check to earlier in this function
* because allowing primary key or replica identity even when index scan
* is disabled is the legacy behaviour.
*/
if (!enable_indexscan)
return InvalidOid;
======
src/backend/replication/logical/worker.c
10. get_usable_indexoid
+/*
+ * Decide whether we can pick an index for the relinfo (e.g., the relation)
+ * we're actually deleting/updating from. If it is a child partition of
+ * edata->targetRelInfo, find the index on the partition.
+ *
+ * Note that if the corresponding relmapentry has invalid usableIndexOid,
+ * the function returns InvalidOid.
+ */
"(e.g., the relation)" --> "(i.e. the relation)"
------
Kind Regards,
Peter Smith.
Fujitsu Australia
^ permalink raw reply [nested|flat] 18+ messages in thread
* RE: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher
@ 2023-02-15 03:53 [email protected] <[email protected]>
parent: Amit Kapila <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: [email protected] @ 2023-02-15 03:53 UTC (permalink / raw)
To: Amit Kapila <[email protected]>; Önder Kalacı <[email protected]>; +Cc: Marco Slot <[email protected]>; Tom Lane <[email protected]>; Andres Freund <[email protected]>; [email protected] <[email protected]>; PostgreSQL Hackers <[email protected]>
On Sat, Feb 4, 2023 7:24 PM Amit Kapila <[email protected]> wrote:
>
> On Thu, Feb 2, 2023 at 2:03 PM Önder Kalacı <[email protected]> wrote:
> >
> >>
> >> and if there's more
> >> than one candidate index pick any one. Additionally, we can allow
> >> disabling the use of an index scan for this particular case. If we are
> >> too worried about API change for allowing users to specify the index
> >> then we can do that later or as a separate patch.
> >>
> >
> > On v23, I dropped the planner support for picking the index. Instead, it simply
> > iterates over the indexes and picks the first one that is suitable.
> >
> > I'm currently thinking on how to enable users to override this decision.
> > One option I'm leaning towards is to add a syntax like the following:
> >
> > ALTER SUBSCRIPTION .. ALTER TABLE ... SET INDEX ...
> >
> > Though, that should probably be a seperate patch. I'm going to work
> > on that, but still wanted to share v23 given picking the index sounds
> > complementary, not strictly required at this point.
> >
>
> I agree that it could be a separate patch. However, do you think we
> need some way to disable picking the index scan? This is to avoid
> cases where sequence scan could be better or do we think there won't
> exist such a case?
>
I think such a case exists. I tried the following cases based on v23 patch.
# Step 1.
Create publication, subscription and tables.
-- on publisher
create table tbl (a int);
alter table tbl replica identity full;
create publication pub for table tbl;
-- on subscriber
create table tbl (a int);
create index idx_a on tbl(a);
create subscription sub connection 'dbname=postgres port=5432' publication pub;
# Step 2.
Setup synchronous replication.
# Step 3.
Execute SQL query on publisher.
-- case 1 (All values are duplicated)
truncate tbl;
insert into tbl select 1 from generate_series(0,10000)i;
update tbl set a=a+1;
-- case 2
truncate tbl;
insert into tbl select i%3 from generate_series(0,10000)i;
update tbl set a=a+1;
-- case 3
truncate tbl;
insert into tbl select i%5 from generate_series(0,10000)i;
update tbl set a=a+1;
-- case 4
truncate tbl;
insert into tbl select i%10 from generate_series(0,10000)i;
update tbl set a=a+1;
-- case 5
truncate tbl;
insert into tbl select i%100 from generate_series(0,10000)i;
update tbl set a=a+1;
-- case 6
truncate tbl;
insert into tbl select i%1000 from generate_series(0,10000)i;
update tbl set a=a+1;
-- case 7 (No duplicate value)
truncate tbl;
insert into tbl select i from generate_series(0,10000)i;
update tbl set a=a+1;
# Result
The time executing update (the average of 3 runs is taken, the unit is
milliseconds):
+--------+---------+---------+
| | patched | master |
+--------+---------+---------+
| case 1 | 3933.68 | 1298.32 |
| case 2 | 1803.46 | 1294.42 |
| case 3 | 1380.82 | 1299.90 |
| case 4 | 1042.60 | 1300.20 |
| case 5 | 691.69 | 1297.51 |
| case 6 | 578.50 | 1300.69 |
| case 7 | 566.45 | 1302.17 |
+--------+---------+---------+
In case 1~3, there's an overhead after applying the patch. In other cases, the
patch improved the performance. As more duplicate values, the greater the
overhead after applying the patch.
Regards,
Shi Yu
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher
@ 2023-02-15 04:37 Amit Kapila <[email protected]>
parent: [email protected] <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Amit Kapila @ 2023-02-15 04:37 UTC (permalink / raw)
To: [email protected] <[email protected]>; +Cc: Önder Kalacı <[email protected]>; Marco Slot <[email protected]>; Tom Lane <[email protected]>; Andres Freund <[email protected]>; [email protected] <[email protected]>; PostgreSQL Hackers <[email protected]>
On Wed, Feb 15, 2023 at 9:23 AM [email protected]
<[email protected]> wrote:
>
> On Sat, Feb 4, 2023 7:24 PM Amit Kapila <[email protected]> wrote:
> >
> > On Thu, Feb 2, 2023 at 2:03 PM Önder Kalacı <[email protected]> wrote:
> > >
> > > On v23, I dropped the planner support for picking the index. Instead, it simply
> > > iterates over the indexes and picks the first one that is suitable.
> > >
> > > I'm currently thinking on how to enable users to override this decision.
> > > One option I'm leaning towards is to add a syntax like the following:
> > >
> > > ALTER SUBSCRIPTION .. ALTER TABLE ... SET INDEX ...
> > >
> > > Though, that should probably be a seperate patch. I'm going to work
> > > on that, but still wanted to share v23 given picking the index sounds
> > > complementary, not strictly required at this point.
> > >
> >
> > I agree that it could be a separate patch. However, do you think we
> > need some way to disable picking the index scan? This is to avoid
> > cases where sequence scan could be better or do we think there won't
> > exist such a case?
> >
>
> I think such a case exists. I tried the following cases based on v23 patch.
>
...
> # Result
> The time executing update (the average of 3 runs is taken, the unit is
> milliseconds):
>
> +--------+---------+---------+
> | | patched | master |
> +--------+---------+---------+
> | case 1 | 3933.68 | 1298.32 |
> | case 2 | 1803.46 | 1294.42 |
> | case 3 | 1380.82 | 1299.90 |
> | case 4 | 1042.60 | 1300.20 |
> | case 5 | 691.69 | 1297.51 |
> | case 6 | 578.50 | 1300.69 |
> | case 7 | 566.45 | 1302.17 |
> +--------+---------+---------+
>
> In case 1~3, there's an overhead after applying the patch. In other cases, the
> patch improved the performance. As more duplicate values, the greater the
> overhead after applying the patch.
>
I think this overhead seems to be mostly due to the need to perform
tuples_equal multiple times for duplicate values. I don't know if
there is any simple way to avoid this without using the planner stuff
as was used in the previous approach. So, this brings us to the
question of whether just providing a way to disable/enable the use of
index scan for such cases is sufficient or if we need any other way.
Tom, Andres, or others, do you have any suggestions on how to move
forward with this patch?
--
With Regards,
Amit Kapila.
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher
@ 2023-02-17 06:57 Peter Smith <[email protected]>
parent: Önder Kalacı <[email protected]>
3 siblings, 0 replies; 18+ messages in thread
From: Peter Smith @ 2023-02-17 06:57 UTC (permalink / raw)
To: Önder Kalacı <[email protected]>; +Cc: Amit Kapila <[email protected]>; Marco Slot <[email protected]>; Tom Lane <[email protected]>; Andres Freund <[email protected]>; [email protected]; [email protected]; PostgreSQL Hackers <[email protected]>
FYI, I accidentally left this (v23) patch's TAP test
t/032_subscribe_use_index.pl still lurking even after removing all
other parts of this patch.
In this scenario, the t/032 test gets stuck (build of latest HEAD)
IIUC the patch is only meant to affect performance, so I expected this
032 test to work regardless of whether the rest of the patch is
applied.
Anyway, it hangs every time for me. I didn't dig looking for the
cause, but if it requires patched code for this new test to pass, I
thought it indicates something wrong either with the test or something
more sinister the new test has exposed. Maybe I am mistaken
------
Kind Regards,
Peter Smith.
Fujitsu Australia
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher
@ 2023-02-21 14:25 Önder Kalacı <[email protected]>
parent: Amit Kapila <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Önder Kalacı @ 2023-02-21 14:25 UTC (permalink / raw)
To: Amit Kapila <[email protected]>; +Cc: [email protected] <[email protected]>; Marco Slot <[email protected]>; Tom Lane <[email protected]>; Andres Freund <[email protected]>; [email protected] <[email protected]>; PostgreSQL Hackers <[email protected]>
Hi Amit, all
Amit Kapila <[email protected]>, 15 Şub 2023 Çar, 07:37 tarihinde
şunu yazdı:
> On Wed, Feb 15, 2023 at 9:23 AM [email protected]
> <[email protected]> wrote:
> >
> > On Sat, Feb 4, 2023 7:24 PM Amit Kapila <[email protected]> wrote:
> > >
> > > On Thu, Feb 2, 2023 at 2:03 PM Önder Kalacı <[email protected]>
> wrote:
> > > >
> > > > On v23, I dropped the planner support for picking the index.
> Instead, it simply
> > > > iterates over the indexes and picks the first one that is suitable.
> > > >
> > > > I'm currently thinking on how to enable users to override this
> decision.
> > > > One option I'm leaning towards is to add a syntax like the following:
> > > >
> > > > ALTER SUBSCRIPTION .. ALTER TABLE ... SET INDEX ...
> > > >
> > > > Though, that should probably be a seperate patch. I'm going to work
> > > > on that, but still wanted to share v23 given picking the index sounds
> > > > complementary, not strictly required at this point.
> > > >
> > >
> > > I agree that it could be a separate patch. However, do you think we
> > > need some way to disable picking the index scan? This is to avoid
> > > cases where sequence scan could be better or do we think there won't
> > > exist such a case?
> > >
> >
> > I think such a case exists. I tried the following cases based on v23
> patch.
> >
> ...
> > # Result
> > The time executing update (the average of 3 runs is taken, the unit is
> > milliseconds):
> >
> > +--------+---------+---------+
> > | | patched | master |
> > +--------+---------+---------+
> > | case 1 | 3933.68 | 1298.32 |
> > | case 2 | 1803.46 | 1294.42 |
> > | case 3 | 1380.82 | 1299.90 |
> > | case 4 | 1042.60 | 1300.20 |
> > | case 5 | 691.69 | 1297.51 |
> > | case 6 | 578.50 | 1300.69 |
> > | case 7 | 566.45 | 1302.17 |
> > +--------+---------+---------+
> >
> > In case 1~3, there's an overhead after applying the patch. In other
> cases, the
> > patch improved the performance. As more duplicate values, the greater the
> > overhead after applying the patch.
> >
>
> I think this overhead seems to be mostly due to the need to perform
> tuples_equal multiple times for duplicate values. I don't know if
> there is any simple way to avoid this without using the planner stuff
> as was used in the previous approach. So, this brings us to the
> question of whether just providing a way to disable/enable the use of
> index scan for such cases is sufficient or if we need any other way.
>
> Tom, Andres, or others, do you have any suggestions on how to move
> forward with this patch?
>
>
Thanks for the feedback and testing. Due to personal circumstances,
I could not reply the thread in the last 2 weeks, but I'll be more active
going forward.
I also agree that we should have a way to control the behavior.
I created another patch (v24_0001_optionally_disable_index.patch) which can
be applied
on top of v23_0001_use_index_on_subs_when_pub_rep_ident_full.patch.
The new patch adds a new *subscription_parameter* for both CREATE and ALTER
subscription
named: *enable_index_scan*. The setting is valid only when REPLICA IDENTITY
is full.
What do you think about such a patch to control the behavior? It does not
give a per-relation
level of control, but still useful for many cases.
(Note that I'll be working on the other feedback in the email thread,
wanted to send this earlier
to hear some early thoughts on v24_0001_optionally_disable_index.patch).
Attachments:
[application/octet-stream] v24_0001_optionally_disable_index.patch (51.3K, ../../CACawEhV8X88dDxL+LYs9X=qGh+en1nVjNSevXcQPVuSnN+d2Pg@mail.gmail.com/3-v24_0001_optionally_disable_index.patch)
download | inline diff:
From c2a1efdba52da5e071e1f063978649d0cc07b79d Mon Sep 17 00:00:00 2001
From: Onder Kalaci <[email protected]>
Date: Tue, 21 Feb 2023 13:42:14 +0300
Subject: [PATCH] Optionally disable index scan when replica identity is full
When replica identitiy is full, the logical replication apply workers
for subscription is capable of using non-unique index scans. However,
index scans has some overhead, especially when the data on the indexed
column(s) is duplicated.
We add a new option to subscription_parameter for both CREATE and ALTER
subscription commands such that the user can control the behavior.
---
doc/src/sgml/catalogs.sgml | 10 ++
doc/src/sgml/ref/alter_subscription.sgml | 3 +-
doc/src/sgml/ref/create_subscription.sgml | 12 ++
src/backend/catalog/pg_subscription.c | 1 +
src/backend/catalog/system_views.sql | 3 +-
src/backend/commands/subscriptioncmds.c | 27 +++-
src/backend/replication/logical/worker.c | 5 +
src/bin/pg_dump/pg_dump.c | 15 +-
src/bin/pg_dump/pg_dump.h | 1 +
src/bin/psql/describe.c | 8 +-
src/bin/psql/tab-complete.c | 6 +-
src/include/catalog/catversion.h | 2 +-
src/include/catalog/pg_subscription.h | 4 +
src/test/regress/expected/subscription.out | 144 +++++++++---------
.../subscription/t/032_subscribe_use_index.pl | 97 ++++++++++++
15 files changed, 253 insertions(+), 85 deletions(-)
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index c1e4048054..0545b3dc60 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -7948,6 +7948,16 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
</para></entry>
</row>
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>subenableindexscan</structfield> <type>bool</type>
+ </para>
+ <para>
+ If true, the subscription is allowed to use indexes that are
+ not primary key or replica identity.
+ </para></entry>
+ </row>
+
<row>
<entry role="catalog_table_entry"><para role="column_definition">
<structfield>subconninfo</structfield> <type>text</type>
diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml
index 964fcbb8ff..94530bd227 100644
--- a/doc/src/sgml/ref/alter_subscription.sgml
+++ b/doc/src/sgml/ref/alter_subscription.sgml
@@ -213,7 +213,8 @@ ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> RENAME TO <
are <literal>slot_name</literal>,
<literal>synchronous_commit</literal>,
<literal>binary</literal>, <literal>streaming</literal>,
- <literal>disable_on_error</literal>, and
+ <literal>enable_index_scan</literal>,
+ <literal>disable_on_error</literal>,and
<literal>origin</literal>.
</para>
</listitem>
diff --git a/doc/src/sgml/ref/create_subscription.sgml b/doc/src/sgml/ref/create_subscription.sgml
index 51c45f17c7..bf0af67646 100644
--- a/doc/src/sgml/ref/create_subscription.sgml
+++ b/doc/src/sgml/ref/create_subscription.sgml
@@ -330,6 +330,18 @@ CREATE SUBSCRIPTION <replaceable class="parameter">subscription_name</replaceabl
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>enable_index_scan</literal> (<type>boolean</type>)</term>
+ <listitem>
+ <para>
+ Specifies whether the subscription should automatically use
+ any indexes during data replication from the publisher when
+ replica identity is full. The default is
+ <literal>true</literal>.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><literal>origin</literal> (<type>string</type>)</term>
<listitem>
diff --git a/src/backend/catalog/pg_subscription.c b/src/backend/catalog/pg_subscription.c
index a56ae311c3..e05ae5027c 100644
--- a/src/backend/catalog/pg_subscription.c
+++ b/src/backend/catalog/pg_subscription.c
@@ -71,6 +71,7 @@ GetSubscription(Oid subid, bool missing_ok)
sub->stream = subform->substream;
sub->twophasestate = subform->subtwophasestate;
sub->disableonerr = subform->subdisableonerr;
+ sub->enableidxscan = subform->subenableidxscan;
/* Get conninfo */
datum = SysCacheGetAttr(SUBSCRIPTIONOID,
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 34ca0e739f..20df5a282a 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1316,7 +1316,8 @@ REVOKE ALL ON pg_replication_origin_status FROM public;
REVOKE ALL ON pg_subscription FROM public;
GRANT SELECT (oid, subdbid, subskiplsn, subname, subowner, subenabled,
subbinary, substream, subtwophasestate, subdisableonerr,
- subslotname, subsynccommit, subpublications, suborigin)
+ subenableidxscan, subslotname, subsynccommit,
+ subpublications, suborigin)
ON pg_subscription TO public;
CREATE VIEW pg_stat_subscription_stats AS
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 464db6d247..aa26cc2164 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -66,6 +66,7 @@
#define SUBOPT_DISABLE_ON_ERR 0x00000400
#define SUBOPT_LSN 0x00000800
#define SUBOPT_ORIGIN 0x00001000
+#define SUBOPT_ENABLE_INDEX_SCAN 0x00002000
/* check if the 'val' has 'bits' set */
#define IsSet(val, bits) (((val) & (bits)) == (bits))
@@ -88,6 +89,7 @@ typedef struct SubOpts
char streaming;
bool twophase;
bool disableonerr;
+ bool enableidxscan;
char *origin;
XLogRecPtr lsn;
} SubOpts;
@@ -144,6 +146,8 @@ parse_subscription_options(ParseState *pstate, List *stmt_options,
opts->twophase = false;
if (IsSet(supported_opts, SUBOPT_DISABLE_ON_ERR))
opts->disableonerr = false;
+ if (IsSet(supported_opts, SUBOPT_ENABLE_INDEX_SCAN))
+ opts->enableidxscan = true;
if (IsSet(supported_opts, SUBOPT_ORIGIN))
opts->origin = pstrdup(LOGICALREP_ORIGIN_ANY);
@@ -274,6 +278,15 @@ parse_subscription_options(ParseState *pstate, List *stmt_options,
opts->specified_opts |= SUBOPT_DISABLE_ON_ERR;
opts->disableonerr = defGetBoolean(defel);
}
+ else if (IsSet(supported_opts, SUBOPT_ENABLE_INDEX_SCAN) &&
+ strcmp(defel->defname, "enable_index_scan") == 0)
+ {
+ if (IsSet(opts->specified_opts, SUBOPT_ENABLE_INDEX_SCAN))
+ errorConflictingDefElem(defel, pstate);
+
+ opts->specified_opts |= SUBOPT_ENABLE_INDEX_SCAN;
+ opts->enableidxscan = defGetBoolean(defel);
+ }
else if (IsSet(supported_opts, SUBOPT_ORIGIN) &&
strcmp(defel->defname, "origin") == 0)
{
@@ -560,7 +573,8 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt,
SUBOPT_SLOT_NAME | SUBOPT_COPY_DATA |
SUBOPT_SYNCHRONOUS_COMMIT | SUBOPT_BINARY |
SUBOPT_STREAMING | SUBOPT_TWOPHASE_COMMIT |
- SUBOPT_DISABLE_ON_ERR | SUBOPT_ORIGIN);
+ SUBOPT_DISABLE_ON_ERR | SUBOPT_ENABLE_INDEX_SCAN |
+ SUBOPT_ORIGIN);
parse_subscription_options(pstate, stmt->options, supported_opts, &opts);
/*
@@ -649,6 +663,7 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt,
publicationListToArray(publications);
values[Anum_pg_subscription_suborigin - 1] =
CStringGetTextDatum(opts.origin);
+ values[Anum_pg_subscription_subenableidxscan - 1] = BoolGetDatum(opts.enableidxscan);
tup = heap_form_tuple(RelationGetDescr(rel), values, nulls);
@@ -1054,7 +1069,7 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
supported_opts = (SUBOPT_SLOT_NAME |
SUBOPT_SYNCHRONOUS_COMMIT | SUBOPT_BINARY |
SUBOPT_STREAMING | SUBOPT_DISABLE_ON_ERR |
- SUBOPT_ORIGIN);
+ SUBOPT_ENABLE_INDEX_SCAN | SUBOPT_ORIGIN);
parse_subscription_options(pstate, stmt->options,
supported_opts, &opts);
@@ -1111,6 +1126,14 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
= true;
}
+ if (IsSet(opts.specified_opts, SUBOPT_ENABLE_INDEX_SCAN))
+ {
+ values[Anum_pg_subscription_subenableidxscan - 1]
+ = BoolGetDatum(opts.enableidxscan);
+ replaces[Anum_pg_subscription_subenableidxscan - 1]
+ = true;
+ }
+
if (IsSet(opts.specified_opts, SUBOPT_ORIGIN))
{
values[Anum_pg_subscription_suborigin - 1] =
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 75eb2ba7f1..7f67f9389e 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -2870,6 +2870,11 @@ FindReplTupleInLocalRel(EState *estate, Relation localrel,
Assert(OidIsValid(localidxoid) ||
(remoterel->replident == REPLICA_IDENTITY_FULL));
+ /* Override the selected index if user disabled the index scan */
+ if (!MySubscription->enableidxscan &&
+ remoterel->replident == REPLICA_IDENTITY_FULL)
+ localidxoid = InvalidOid;
+
if (OidIsValid(localidxoid))
found = RelationFindReplTupleByIndex(localrel, localidxoid,
LockTupleExclusive,
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 527c7651ab..0c4c1c593f 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -4488,6 +4488,7 @@ getSubscriptions(Archive *fout)
int i_substream;
int i_subtwophasestate;
int i_subdisableonerr;
+ int i_subenableidxscan;
int i_suborigin;
int i_subconninfo;
int i_subslotname;
@@ -4546,9 +4547,13 @@ getSubscriptions(Archive *fout)
LOGICALREP_TWOPHASE_STATE_DISABLED);
if (fout->remoteVersion >= 160000)
- appendPQExpBufferStr(query, " s.suborigin\n");
+ appendPQExpBufferStr(query,
+ " s.suborigin,\n"
+ " s.subenableidxscan\n");
else
- appendPQExpBuffer(query, " '%s' AS suborigin\n", LOGICALREP_ORIGIN_ANY);
+ appendPQExpBuffer(query, " '%s' AS suborigin,\n"
+ " false AS subenableidxscan\n",
+ LOGICALREP_ORIGIN_ANY);
appendPQExpBufferStr(query,
"FROM pg_subscription s\n"
@@ -4575,6 +4580,7 @@ getSubscriptions(Archive *fout)
i_substream = PQfnumber(res, "substream");
i_subtwophasestate = PQfnumber(res, "subtwophasestate");
i_subdisableonerr = PQfnumber(res, "subdisableonerr");
+ i_subenableidxscan = PQfnumber(res, "subenableidxscan");
i_suborigin = PQfnumber(res, "suborigin");
subinfo = pg_malloc(ntups * sizeof(SubscriptionInfo));
@@ -4605,6 +4611,8 @@ getSubscriptions(Archive *fout)
pg_strdup(PQgetvalue(res, i, i_subtwophasestate));
subinfo[i].subdisableonerr =
pg_strdup(PQgetvalue(res, i, i_subdisableonerr));
+ subinfo[i].subenableidxscan =
+ pg_strdup(PQgetvalue(res, i, i_subenableidxscan));
subinfo[i].suborigin = pg_strdup(PQgetvalue(res, i, i_suborigin));
/* Decide whether we want to dump it */
@@ -4681,6 +4689,9 @@ dumpSubscription(Archive *fout, const SubscriptionInfo *subinfo)
if (strcmp(subinfo->subdisableonerr, "t") == 0)
appendPQExpBufferStr(query, ", disable_on_error = true");
+ if (strcmp(subinfo->subenableidxscan, "f") == 0)
+ appendPQExpBufferStr(query, ", enable_index_scan = false");
+
if (pg_strcasecmp(subinfo->suborigin, LOGICALREP_ORIGIN_ANY) != 0)
appendPQExpBuffer(query, ", origin = %s", subinfo->suborigin);
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index e7cbd8d7ed..8c60102f63 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -659,6 +659,7 @@ typedef struct _SubscriptionInfo
char *substream;
char *subtwophasestate;
char *subdisableonerr;
+ char *subenableidxscan;
char *suborigin;
char *subsynccommit;
char *subpublications;
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index c8a0bb7b3a..b705f6c611 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -6472,7 +6472,7 @@ describeSubscriptions(const char *pattern, bool verbose)
PGresult *res;
printQueryOpt myopt = pset.popt;
static const bool translate_columns[] = {false, false, false, false,
- false, false, false, false, false, false, false, false};
+ false, false, false, false, false, false, false, false, false};
if (pset.sversion < 100000)
{
@@ -6529,8 +6529,10 @@ describeSubscriptions(const char *pattern, bool verbose)
if (pset.sversion >= 160000)
appendPQExpBuffer(&buf,
- ", suborigin AS \"%s\"\n",
- gettext_noop("Origin"));
+ ", suborigin AS \"%s\"\n"
+ ", subenableidxscan AS \"%s\"\n",
+ gettext_noop("Origin"),
+ gettext_noop("Enable index scan"));
appendPQExpBuffer(&buf,
", subsynccommit AS \"%s\"\n"
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 5e1882eaea..a1353172b7 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1925,7 +1925,7 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH("(", "PUBLICATION");
/* ALTER SUBSCRIPTION <name> SET ( */
else if (HeadMatches("ALTER", "SUBSCRIPTION", MatchAny) && TailMatches("SET", "("))
- COMPLETE_WITH("binary", "disable_on_error", "origin", "slot_name",
+ COMPLETE_WITH("binary", "disable_on_error", "enable_index_scan", "origin", "slot_name",
"streaming", "synchronous_commit");
/* ALTER SUBSCRIPTION <name> SKIP ( */
else if (HeadMatches("ALTER", "SUBSCRIPTION", MatchAny) && TailMatches("SKIP", "("))
@@ -3268,8 +3268,8 @@ psql_completion(const char *text, int start, int end)
/* Complete "CREATE SUBSCRIPTION <name> ... WITH ( <opt>" */
else if (HeadMatches("CREATE", "SUBSCRIPTION") && TailMatches("WITH", "("))
COMPLETE_WITH("binary", "connect", "copy_data", "create_slot",
- "disable_on_error", "enabled", "origin", "slot_name",
- "streaming", "synchronous_commit", "two_phase");
+ "disable_on_error", "enable_index_scan", "enabled", "origin",
+ "slot_name", "streaming", "synchronous_commit", "two_phase");
/* CREATE TRIGGER --- is allowed inside CREATE SCHEMA, so use TailMatches */
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index 9c298cb125..e7f9a20eec 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -57,6 +57,6 @@
*/
/* yyyymmddN */
-#define CATALOG_VERSION_NO 202302111
+#define CATALOG_VERSION_NO 202302211
#endif
diff --git a/src/include/catalog/pg_subscription.h b/src/include/catalog/pg_subscription.h
index b0f2a1705d..4fec93fe4f 100644
--- a/src/include/catalog/pg_subscription.h
+++ b/src/include/catalog/pg_subscription.h
@@ -88,6 +88,9 @@ CATALOG(pg_subscription,6100,SubscriptionRelationId) BKI_SHARED_RELATION BKI_ROW
bool subdisableonerr; /* True if a worker error should cause the
* subscription to be disabled */
+ bool subenableidxscan; /* True if a worker should use indexes when
+ * replica identity is full */
+
#ifdef CATALOG_VARLEN /* variable-length fields start here */
/* Connection string to the publisher */
text subconninfo BKI_FORCE_NOT_NULL;
@@ -131,6 +134,7 @@ typedef struct Subscription
bool disableonerr; /* Indicates if the subscription should be
* automatically disabled if a worker error
* occurs */
+ bool enableidxscan; /* Allow replica identitiy full to use indexes */
char *conninfo; /* Connection string to the publisher */
char *slotname; /* Name of the replication slot */
char *synccommit; /* Synchronous commit setting for worker */
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 3f99b14394..c2a3df2e35 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -114,18 +114,18 @@ CREATE SUBSCRIPTION regress_testsub4 CONNECTION 'dbname=regress_doesnotexist' PU
WARNING: subscription was created, but is not connected
HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
\dRs+ regress_testsub4
- List of subscriptions
- Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Synchronous commit | Conninfo | Skip LSN
-------------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+--------------------+-----------------------------+----------
- regress_testsub4 | regress_subscription_user | f | {testpub} | f | off | d | f | none | off | dbname=regress_doesnotexist | 0/0
+ List of subscriptions
+ Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Enable index scan | Synchronous commit | Conninfo | Skip LSN
+------------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+-------------------+--------------------+-----------------------------+----------
+ regress_testsub4 | regress_subscription_user | f | {testpub} | f | off | d | f | none | t | off | dbname=regress_doesnotexist | 0/0
(1 row)
ALTER SUBSCRIPTION regress_testsub4 SET (origin = any);
\dRs+ regress_testsub4
- List of subscriptions
- Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Synchronous commit | Conninfo | Skip LSN
-------------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+--------------------+-----------------------------+----------
- regress_testsub4 | regress_subscription_user | f | {testpub} | f | off | d | f | any | off | dbname=regress_doesnotexist | 0/0
+ List of subscriptions
+ Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Enable index scan | Synchronous commit | Conninfo | Skip LSN
+------------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+-------------------+--------------------+-----------------------------+----------
+ regress_testsub4 | regress_subscription_user | f | {testpub} | f | off | d | f | any | t | off | dbname=regress_doesnotexist | 0/0
(1 row)
DROP SUBSCRIPTION regress_testsub3;
@@ -143,10 +143,10 @@ ALTER SUBSCRIPTION regress_testsub CONNECTION 'foobar';
ERROR: invalid connection string syntax: missing "=" after "foobar" in connection info string
\dRs+
- List of subscriptions
- Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Synchronous commit | Conninfo | Skip LSN
------------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+--------------------+-----------------------------+----------
- regress_testsub | regress_subscription_user | f | {testpub} | f | off | d | f | any | off | dbname=regress_doesnotexist | 0/0
+ List of subscriptions
+ Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Enable index scan | Synchronous commit | Conninfo | Skip LSN
+-----------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+-------------------+--------------------+-----------------------------+----------
+ regress_testsub | regress_subscription_user | f | {testpub} | f | off | d | f | any | t | off | dbname=regress_doesnotexist | 0/0
(1 row)
ALTER SUBSCRIPTION regress_testsub SET PUBLICATION testpub2, testpub3 WITH (refresh = false);
@@ -163,10 +163,10 @@ ERROR: unrecognized subscription parameter: "create_slot"
-- ok
ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/12345');
\dRs+
- List of subscriptions
- Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Synchronous commit | Conninfo | Skip LSN
------------------+---------------------------+---------+---------------------+--------+-----------+------------------+------------------+--------+--------------------+------------------------------+----------
- regress_testsub | regress_subscription_user | f | {testpub2,testpub3} | f | off | d | f | any | off | dbname=regress_doesnotexist2 | 0/12345
+ List of subscriptions
+ Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Enable index scan | Synchronous commit | Conninfo | Skip LSN
+-----------------+---------------------------+---------+---------------------+--------+-----------+------------------+------------------+--------+-------------------+--------------------+------------------------------+----------
+ regress_testsub | regress_subscription_user | f | {testpub2,testpub3} | f | off | d | f | any | t | off | dbname=regress_doesnotexist2 | 0/12345
(1 row)
-- ok - with lsn = NONE
@@ -175,10 +175,10 @@ ALTER SUBSCRIPTION regress_testsub SKIP (lsn = NONE);
ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/0');
ERROR: invalid WAL location (LSN): 0/0
\dRs+
- List of subscriptions
- Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Synchronous commit | Conninfo | Skip LSN
------------------+---------------------------+---------+---------------------+--------+-----------+------------------+------------------+--------+--------------------+------------------------------+----------
- regress_testsub | regress_subscription_user | f | {testpub2,testpub3} | f | off | d | f | any | off | dbname=regress_doesnotexist2 | 0/0
+ List of subscriptions
+ Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Enable index scan | Synchronous commit | Conninfo | Skip LSN
+-----------------+---------------------------+---------+---------------------+--------+-----------+------------------+------------------+--------+-------------------+--------------------+------------------------------+----------
+ regress_testsub | regress_subscription_user | f | {testpub2,testpub3} | f | off | d | f | any | t | off | dbname=regress_doesnotexist2 | 0/0
(1 row)
BEGIN;
@@ -210,10 +210,10 @@ ALTER SUBSCRIPTION regress_testsub_foo SET (synchronous_commit = foobar);
ERROR: invalid value for parameter "synchronous_commit": "foobar"
HINT: Available values: local, remote_write, remote_apply, on, off.
\dRs+
- List of subscriptions
- Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Synchronous commit | Conninfo | Skip LSN
----------------------+---------------------------+---------+---------------------+--------+-----------+------------------+------------------+--------+--------------------+------------------------------+----------
- regress_testsub_foo | regress_subscription_user | f | {testpub2,testpub3} | f | off | d | f | any | local | dbname=regress_doesnotexist2 | 0/0
+ List of subscriptions
+ Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Enable index scan | Synchronous commit | Conninfo | Skip LSN
+---------------------+---------------------------+---------+---------------------+--------+-----------+------------------+------------------+--------+-------------------+--------------------+------------------------------+----------
+ regress_testsub_foo | regress_subscription_user | f | {testpub2,testpub3} | f | off | d | f | any | t | local | dbname=regress_doesnotexist2 | 0/0
(1 row)
-- rename back to keep the rest simple
@@ -247,19 +247,19 @@ CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist' PUB
WARNING: subscription was created, but is not connected
HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
\dRs+
- List of subscriptions
- Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Synchronous commit | Conninfo | Skip LSN
------------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+--------------------+-----------------------------+----------
- regress_testsub | regress_subscription_user | f | {testpub} | t | off | d | f | any | off | dbname=regress_doesnotexist | 0/0
+ List of subscriptions
+ Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Enable index scan | Synchronous commit | Conninfo | Skip LSN
+-----------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+-------------------+--------------------+-----------------------------+----------
+ regress_testsub | regress_subscription_user | f | {testpub} | t | off | d | f | any | t | off | dbname=regress_doesnotexist | 0/0
(1 row)
ALTER SUBSCRIPTION regress_testsub SET (binary = false);
ALTER SUBSCRIPTION regress_testsub SET (slot_name = NONE);
\dRs+
- List of subscriptions
- Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Synchronous commit | Conninfo | Skip LSN
------------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+--------------------+-----------------------------+----------
- regress_testsub | regress_subscription_user | f | {testpub} | f | off | d | f | any | off | dbname=regress_doesnotexist | 0/0
+ List of subscriptions
+ Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Enable index scan | Synchronous commit | Conninfo | Skip LSN
+-----------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+-------------------+--------------------+-----------------------------+----------
+ regress_testsub | regress_subscription_user | f | {testpub} | f | off | d | f | any | t | off | dbname=regress_doesnotexist | 0/0
(1 row)
DROP SUBSCRIPTION regress_testsub;
@@ -271,27 +271,27 @@ CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist' PUB
WARNING: subscription was created, but is not connected
HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
\dRs+
- List of subscriptions
- Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Synchronous commit | Conninfo | Skip LSN
------------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+--------------------+-----------------------------+----------
- regress_testsub | regress_subscription_user | f | {testpub} | f | on | d | f | any | off | dbname=regress_doesnotexist | 0/0
+ List of subscriptions
+ Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Enable index scan | Synchronous commit | Conninfo | Skip LSN
+-----------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+-------------------+--------------------+-----------------------------+----------
+ regress_testsub | regress_subscription_user | f | {testpub} | f | on | d | f | any | t | off | dbname=regress_doesnotexist | 0/0
(1 row)
ALTER SUBSCRIPTION regress_testsub SET (streaming = parallel);
\dRs+
- List of subscriptions
- Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Synchronous commit | Conninfo | Skip LSN
------------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+--------------------+-----------------------------+----------
- regress_testsub | regress_subscription_user | f | {testpub} | f | parallel | d | f | any | off | dbname=regress_doesnotexist | 0/0
+ List of subscriptions
+ Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Enable index scan | Synchronous commit | Conninfo | Skip LSN
+-----------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+-------------------+--------------------+-----------------------------+----------
+ regress_testsub | regress_subscription_user | f | {testpub} | f | parallel | d | f | any | t | off | dbname=regress_doesnotexist | 0/0
(1 row)
ALTER SUBSCRIPTION regress_testsub SET (streaming = false);
ALTER SUBSCRIPTION regress_testsub SET (slot_name = NONE);
\dRs+
- List of subscriptions
- Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Synchronous commit | Conninfo | Skip LSN
------------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+--------------------+-----------------------------+----------
- regress_testsub | regress_subscription_user | f | {testpub} | f | off | d | f | any | off | dbname=regress_doesnotexist | 0/0
+ List of subscriptions
+ Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Enable index scan | Synchronous commit | Conninfo | Skip LSN
+-----------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+-------------------+--------------------+-----------------------------+----------
+ regress_testsub | regress_subscription_user | f | {testpub} | f | off | d | f | any | t | off | dbname=regress_doesnotexist | 0/0
(1 row)
-- fail - publication already exists
@@ -306,10 +306,10 @@ ALTER SUBSCRIPTION regress_testsub ADD PUBLICATION testpub1, testpub2 WITH (refr
ALTER SUBSCRIPTION regress_testsub ADD PUBLICATION testpub1, testpub2 WITH (refresh = false);
ERROR: publication "testpub1" is already in subscription "regress_testsub"
\dRs+
- List of subscriptions
- Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Synchronous commit | Conninfo | Skip LSN
------------------+---------------------------+---------+-----------------------------+--------+-----------+------------------+------------------+--------+--------------------+-----------------------------+----------
- regress_testsub | regress_subscription_user | f | {testpub,testpub1,testpub2} | f | off | d | f | any | off | dbname=regress_doesnotexist | 0/0
+ List of subscriptions
+ Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Enable index scan | Synchronous commit | Conninfo | Skip LSN
+-----------------+---------------------------+---------+-----------------------------+--------+-----------+------------------+------------------+--------+-------------------+--------------------+-----------------------------+----------
+ regress_testsub | regress_subscription_user | f | {testpub,testpub1,testpub2} | f | off | d | f | any | t | off | dbname=regress_doesnotexist | 0/0
(1 row)
-- fail - publication used more than once
@@ -324,10 +324,10 @@ ERROR: publication "testpub3" is not in subscription "regress_testsub"
-- ok - delete publications
ALTER SUBSCRIPTION regress_testsub DROP PUBLICATION testpub1, testpub2 WITH (refresh = false);
\dRs+
- List of subscriptions
- Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Synchronous commit | Conninfo | Skip LSN
------------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+--------------------+-----------------------------+----------
- regress_testsub | regress_subscription_user | f | {testpub} | f | off | d | f | any | off | dbname=regress_doesnotexist | 0/0
+ List of subscriptions
+ Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Enable index scan | Synchronous commit | Conninfo | Skip LSN
+-----------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+-------------------+--------------------+-----------------------------+----------
+ regress_testsub | regress_subscription_user | f | {testpub} | f | off | d | f | any | t | off | dbname=regress_doesnotexist | 0/0
(1 row)
DROP SUBSCRIPTION regress_testsub;
@@ -363,10 +363,10 @@ CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist' PUB
WARNING: subscription was created, but is not connected
HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
\dRs+
- List of subscriptions
- Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Synchronous commit | Conninfo | Skip LSN
------------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+--------------------+-----------------------------+----------
- regress_testsub | regress_subscription_user | f | {testpub} | f | off | p | f | any | off | dbname=regress_doesnotexist | 0/0
+ List of subscriptions
+ Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Enable index scan | Synchronous commit | Conninfo | Skip LSN
+-----------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+-------------------+--------------------+-----------------------------+----------
+ regress_testsub | regress_subscription_user | f | {testpub} | f | off | p | f | any | t | off | dbname=regress_doesnotexist | 0/0
(1 row)
--fail - alter of two_phase option not supported.
@@ -375,10 +375,10 @@ ERROR: unrecognized subscription parameter: "two_phase"
-- but can alter streaming when two_phase enabled
ALTER SUBSCRIPTION regress_testsub SET (streaming = true);
\dRs+
- List of subscriptions
- Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Synchronous commit | Conninfo | Skip LSN
------------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+--------------------+-----------------------------+----------
- regress_testsub | regress_subscription_user | f | {testpub} | f | on | p | f | any | off | dbname=regress_doesnotexist | 0/0
+ List of subscriptions
+ Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Enable index scan | Synchronous commit | Conninfo | Skip LSN
+-----------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+-------------------+--------------------+-----------------------------+----------
+ regress_testsub | regress_subscription_user | f | {testpub} | f | on | p | f | any | t | off | dbname=regress_doesnotexist | 0/0
(1 row)
ALTER SUBSCRIPTION regress_testsub SET (slot_name = NONE);
@@ -388,10 +388,10 @@ CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist' PUB
WARNING: subscription was created, but is not connected
HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
\dRs+
- List of subscriptions
- Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Synchronous commit | Conninfo | Skip LSN
------------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+--------------------+-----------------------------+----------
- regress_testsub | regress_subscription_user | f | {testpub} | f | on | p | f | any | off | dbname=regress_doesnotexist | 0/0
+ List of subscriptions
+ Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Enable index scan | Synchronous commit | Conninfo | Skip LSN
+-----------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+-------------------+--------------------+-----------------------------+----------
+ regress_testsub | regress_subscription_user | f | {testpub} | f | on | p | f | any | t | off | dbname=regress_doesnotexist | 0/0
(1 row)
ALTER SUBSCRIPTION regress_testsub SET (slot_name = NONE);
@@ -404,18 +404,18 @@ CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist' PUB
WARNING: subscription was created, but is not connected
HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
\dRs+
- List of subscriptions
- Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Synchronous commit | Conninfo | Skip LSN
------------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+--------------------+-----------------------------+----------
- regress_testsub | regress_subscription_user | f | {testpub} | f | off | d | f | any | off | dbname=regress_doesnotexist | 0/0
+ List of subscriptions
+ Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Enable index scan | Synchronous commit | Conninfo | Skip LSN
+-----------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+-------------------+--------------------+-----------------------------+----------
+ regress_testsub | regress_subscription_user | f | {testpub} | f | off | d | f | any | t | off | dbname=regress_doesnotexist | 0/0
(1 row)
ALTER SUBSCRIPTION regress_testsub SET (disable_on_error = true);
\dRs+
- List of subscriptions
- Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Synchronous commit | Conninfo | Skip LSN
------------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+--------------------+-----------------------------+----------
- regress_testsub | regress_subscription_user | f | {testpub} | f | off | d | t | any | off | dbname=regress_doesnotexist | 0/0
+ List of subscriptions
+ Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Enable index scan | Synchronous commit | Conninfo | Skip LSN
+-----------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+-------------------+--------------------+-----------------------------+----------
+ regress_testsub | regress_subscription_user | f | {testpub} | f | off | d | t | any | t | off | dbname=regress_doesnotexist | 0/0
(1 row)
ALTER SUBSCRIPTION regress_testsub SET (slot_name = NONE);
diff --git a/src/test/subscription/t/032_subscribe_use_index.pl b/src/test/subscription/t/032_subscribe_use_index.pl
index 3fb05f3ab7..777d81dd36 100644
--- a/src/test/subscription/t/032_subscribe_use_index.pl
+++ b/src/test/subscription/t/032_subscribe_use_index.pl
@@ -92,6 +92,103 @@ $node_subscriber->safe_psql('postgres', "DROP TABLE test_replica_id_full");
# Testcase end: SUBSCRIPTION USES INDEX
# ====================================================================
+# ====================================================================
+# Testcase start: SUBSCRIPTION DISABLED INDEX SCAN
+#
+# Show that enable_index_scan option for CREATE SUBSCRIPTION and
+# ALTER SUBSCRIPTION works as intended
+#
+
+# create tables pub and sub
+$node_publisher->safe_psql('postgres',
+ "CREATE TABLE test_replica_id_full (x int)");
+$node_publisher->safe_psql('postgres',
+ "ALTER TABLE test_replica_id_full REPLICA IDENTITY FULL;");
+$node_subscriber->safe_psql('postgres',
+ "CREATE TABLE test_replica_id_full (x int)");
+$node_subscriber->safe_psql('postgres',
+ "CREATE INDEX test_replica_id_full_idx ON test_replica_id_full(x)");
+
+# insert some initial data
+$node_publisher->safe_psql('postgres',
+ "INSERT INTO test_replica_id_full SELECT i FROM generate_series(0,21)i;");
+
+# create pub/sub
+$node_publisher->safe_psql('postgres',
+ "CREATE PUBLICATION tap_pub_rep_full FOR TABLE test_replica_id_full");
+$node_subscriber->safe_psql('postgres',
+ "CREATE SUBSCRIPTION tap_sub_rep_full CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub_rep_full WITH (enable_index_scan = false)"
+);
+
+# wait for initial table synchronization to finish
+$node_subscriber->wait_for_subscription_sync;
+
+$node_publisher->safe_psql('postgres',
+ "UPDATE test_replica_id_full SET x = x + 1 WHERE x = 15;");
+$node_publisher->wait_for_catchup($appname);
+
+# show that index is not used for UPDATE
+$node_subscriber->poll_query_until(
+ 'postgres', q{select (idx_scan = 0) from pg_stat_all_indexes where indexrelname = 'test_replica_id_full_idx';}
+) or die "Timed out while waiting for check subscriber tap_sub_rep_full updates one row via index";
+
+$node_publisher->safe_psql('postgres',
+ "DELETE FROM test_replica_id_full WHERE x = 20;");
+$node_publisher->wait_for_catchup($appname);
+
+# show that index is not used for DELETE
+$node_subscriber->poll_query_until(
+ 'postgres', q{select (idx_scan = 0) from pg_stat_all_indexes where indexrelname = 'test_replica_id_full_idx';}
+) or die "Timed out while waiting for check subscriber tap_sub_rep_full deletes one row via index";
+
+# now, enable the index scan via ALTER SUBSCRIPTION command
+# and show that we can control the behavior of using index
+$node_subscriber->safe_psql('postgres',
+ "ALTER SUBSCRIPTION tap_sub_rep_full SET (enable_index_scan = true)"
+);
+
+$node_publisher->safe_psql('postgres',
+ "UPDATE test_replica_id_full SET x = x + 1 WHERE x = 5;");
+$node_publisher->wait_for_catchup($appname);
+
+# show that index is used for UPDATE as we changed enable_index_scan to true
+$node_subscriber->poll_query_until(
+ 'postgres', q{select (idx_scan = 1) from pg_stat_all_indexes where indexrelname = 'test_replica_id_full_idx';}
+) or die "Timed out while waiting for check subscriber tap_sub_rep_full updates one row via index";
+
+
+# now, disable the index scan via ALTER SUBSCRIPTION command
+# and show that we can control the behavior of using index
+$node_subscriber->safe_psql('postgres',
+ "ALTER SUBSCRIPTION tap_sub_rep_full SET (enable_index_scan = false)"
+);
+
+$node_publisher->safe_psql('postgres',
+ "UPDATE test_replica_id_full SET x = x + 1 WHERE x = 8;");
+$node_publisher->wait_for_catchup($appname);
+
+# show that index is not used for UPDATE as we changed enable_index_scan to false
+$node_subscriber->poll_query_until(
+ 'postgres', q{select (idx_scan = 1) from pg_stat_all_indexes where indexrelname = 'test_replica_id_full_idx';}
+) or die "Timed out while waiting for check subscriber tap_sub_rep_full updates one row via index";
+
+
+# make sure that the subscriber has the correct data
+$result = $node_subscriber->safe_psql('postgres',
+ "SELECT count(DISTINCT x) FROM test_replica_id_full");
+is($result, qq(18), 'ensure subscriber has the correct data at the end of the test');
+
+# cleanup pub
+$node_publisher->safe_psql('postgres', "DROP PUBLICATION tap_pub_rep_full");
+$node_publisher->safe_psql('postgres', "DROP TABLE test_replica_id_full");
+# cleanup sub
+$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub_rep_full");
+$node_subscriber->safe_psql('postgres', "DROP TABLE test_replica_id_full");
+
+# Testcase end: SUBSCRIPTION DISABLED INDEX SCAN
+# ====================================================================
+
+
# ====================================================================
# Testcase start: SUBSCRIPTION RE-CALCULATES INDEX AFTER CREATE/DROP INDEX
#
--
2.34.1
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher
@ 2023-02-25 10:30 Amit Kapila <[email protected]>
parent: Önder Kalacı <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Amit Kapila @ 2023-02-25 10:30 UTC (permalink / raw)
To: Önder Kalacı <[email protected]>; +Cc: [email protected] <[email protected]>; Marco Slot <[email protected]>; Tom Lane <[email protected]>; Andres Freund <[email protected]>; [email protected] <[email protected]>; PostgreSQL Hackers <[email protected]>
On Tue, Feb 21, 2023 at 7:55 PM Önder Kalacı <[email protected]> wrote:
>
>>
>> I think this overhead seems to be mostly due to the need to perform
>> tuples_equal multiple times for duplicate values. I don't know if
>> there is any simple way to avoid this without using the planner stuff
>> as was used in the previous approach. So, this brings us to the
>> question of whether just providing a way to disable/enable the use of
>> index scan for such cases is sufficient or if we need any other way.
>>
>> Tom, Andres, or others, do you have any suggestions on how to move
>> forward with this patch?
>>
>
> Thanks for the feedback and testing. Due to personal circumstances,
> I could not reply the thread in the last 2 weeks, but I'll be more active
> going forward.
>
> I also agree that we should have a way to control the behavior.
>
> I created another patch (v24_0001_optionally_disable_index.patch) which can be applied
> on top of v23_0001_use_index_on_subs_when_pub_rep_ident_full.patch.
>
> The new patch adds a new subscription_parameter for both CREATE and ALTER subscription
> named: enable_index_scan. The setting is valid only when REPLICA IDENTITY is full.
>
> What do you think about such a patch to control the behavior? It does not give a per-relation
> level of control, but still useful for many cases.
>
Wouldn't a table-level option like 'apply_index_scan' be better than a
subscription-level option with a default value as false? Anyway, the
bigger point is that we don't see a better way to proceed here than to
introduce some option to control this behavior.
I see this as a way to provide this feature for users but I would
prefer to proceed with this if we can get some more buy-in from senior
community members (at least one more committer) and some user(s) if
possible. So, I once again request others to chime in and share their
opinion.
--
With Regards,
Amit Kapila.
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher
@ 2023-02-27 07:05 Önder Kalacı <[email protected]>
parent: Amit Kapila <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Önder Kalacı @ 2023-02-27 07:05 UTC (permalink / raw)
To: Amit Kapila <[email protected]>; +Cc: [email protected] <[email protected]>; Marco Slot <[email protected]>; Tom Lane <[email protected]>; Andres Freund <[email protected]>; [email protected] <[email protected]>; PostgreSQL Hackers <[email protected]>
Hi Amit, all
Wouldn't a table-level option like 'apply_index_scan' be better than a
> subscription-level option with a default value as false? Anyway, the
> bigger point is that we don't see a better way to proceed here than to
> introduce some option to control this behavior.
>
What would be a good API for adding such an option for table-level?
To be more specific, I cannot see any table level sub/pub options in the
docs.
My main motivation for doing it for subscription-level is that (a) it might
be
too much work for users to control the behavior if it is table-level (b) I
couldn't
find a good API for table-level, and inventing a new one seemed
like a big change.
Overall, I think it makes sense to disable the feature by default. It is
enabled by default, and that's good for test coverage for now, but
let me disable it when I push a version next time.
>
> I see this as a way to provide this feature for users but I would
> prefer to proceed with this if we can get some more buy-in from senior
> community members (at least one more committer) and some user(s) if
> possible. So, I once again request others to chime in and share their
> opinion.
>
>
Agreed, it would be great to hear some other perspectives on this.
Thanks,
Onder
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher
@ 2023-02-27 08:05 Amit Kapila <[email protected]>
parent: Önder Kalacı <[email protected]>
0 siblings, 0 replies; 18+ messages in thread
From: Amit Kapila @ 2023-02-27 08:05 UTC (permalink / raw)
To: Önder Kalacı <[email protected]>; +Cc: [email protected] <[email protected]>; Marco Slot <[email protected]>; Tom Lane <[email protected]>; Andres Freund <[email protected]>; [email protected] <[email protected]>; PostgreSQL Hackers <[email protected]>
On Mon, Feb 27, 2023 at 12:35 PM Önder Kalacı <[email protected]> wrote:
>
>
>> Wouldn't a table-level option like 'apply_index_scan' be better than a
>> subscription-level option with a default value as false? Anyway, the
>> bigger point is that we don't see a better way to proceed here than to
>> introduce some option to control this behavior.
>
>
> What would be a good API for adding such an option for table-level?
> To be more specific, I cannot see any table level sub/pub options in the docs.
>
I was thinking something along the lines of "Storage Parameters" [1]
for a table. See parameters like autovacuum_enabled that decide the
autovacuum behavior for a table. These can be set via CREATE/ALTER
TABLE commands.
[1] - https://www.postgresql.org/docs/devel/sql-createtable.html#SQL-CREATETABLE-STORAGE-PARAMETERS
--
With Regards,
Amit Kapila.
^ permalink raw reply [nested|flat] 18+ messages in thread
end of thread, other threads:[~2023-02-27 08:05 UTC | newest]
Thread overview: 18+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-07-08 06:08 [PATCH v2 1/2] Be strict in numeric parameters on command line Kyotaro Horiguchi <[email protected]>
2023-01-09 20:03 Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher Andres Freund <[email protected]>
2023-01-09 20:37 ` Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher Tom Lane <[email protected]>
2023-01-20 12:35 ` Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher Marco Slot <[email protected]>
2023-01-27 13:02 ` Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher Önder Kalacı <[email protected]>
2023-01-30 13:16 ` Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher Amit Kapila <[email protected]>
2023-02-02 08:33 ` Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher Önder Kalacı <[email protected]>
2023-02-04 11:24 ` Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher Amit Kapila <[email protected]>
2023-02-15 03:53 ` RE: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher [email protected] <[email protected]>
2023-02-15 04:37 ` Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher Amit Kapila <[email protected]>
2023-02-21 14:25 ` Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher Önder Kalacı <[email protected]>
2023-02-25 10:30 ` Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher Amit Kapila <[email protected]>
2023-02-27 07:05 ` Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher Önder Kalacı <[email protected]>
2023-02-27 08:05 ` Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher Amit Kapila <[email protected]>
2023-02-13 11:00 ` RE: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher [email protected] <[email protected]>
2023-02-14 09:35 ` RE: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher [email protected] <[email protected]>
2023-02-15 02:16 ` Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher Peter Smith <[email protected]>
2023-02-17 06:57 ` Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher Peter Smith <[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