This patch completes the transition to making inet_ops be default for
inet/cidr columns, rather than btree_gist's opclasses. Once we do
that, though, pg_upgrade has a big problem. A dump from an older
version will see btree_gist's opclasses as being default, so it will
not mention the opclass explicitly in CREATE INDEX commands, which
would cause the restore to create the indexes using inet_ops. Since
that's not compatible with what's actually in the files, havoc would
ensue.
This isn't readily fixable, because the CREATE INDEX command strings
are built by the older server's pg_get_indexdef() function; pg_dump
hasn't nearly enough knowledge to modify those strings successfully.
Even if we cared to put in the work to make that happen in pg_dump,
it would be counterproductive because the end goal here is to get
people off of these opclasses. Allowing such indexes to persist
through pg_upgrade wouldn't advance that goal.
Therefore, this patch just adds code to pg_upgrade to detect indexes
that would be problematic and refuse to upgrade.
There's another issue too: even without any indexes to worry about,
pg_dump in binary-upgrade mode will reproduce the "CREATE OPERATOR
CLASS ... DEFAULT" commands for btree_gist's opclasses, and those
will fail because now we have a built-in opclass that provides a
conflicting default. We could ask users to drop the btree_gist
extension altogether before upgrading, but that would carry very
severe penalties. It would affect perfectly-valid indexes for other
data types, and it would drop operators that might be relied on in
views or other database objects. Instead, put a hack in DefineOpClass
to ignore the DEFAULT clauses for these opclasses when in
binary-upgrade mode. This will result in installing a version of
btree_gist that isn't quite the version it claims to be, but that can
be fixed by issuing ALTER EXTENSION UPDATE afterwards.
Since we don't apply that hack when not in binary-upgrade mode,
it is now impossible to install any version of btree_gist
less than 1.9 via CREATE EXTENSION.
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Matthias van de Meent <boekewurm+postgres@gmail.com>
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://postgr.es/m/
2483812.
1754072263@sss.pgh.pa.us
</sect2>
+ <sect2 id="btree-gist-inet-indexes">
+ <title><literal>btree_gist</literal> Indexes
+ on <type>inet</type>/<type>cidr</type> Columns</title>
+
+ <para>
+ The <literal>gist_inet_ops</literal> and <literal>gist_cidr_ops</literal>
+ operator classes provided by <literal>btree_gist</literal> have been
+ shown to be unreliable: index searches may fail to find relevant rows due
+ to approximations used in creating the index entries. This is unfixable
+ without redefining the contents of indexes that use these opclasses.
+ Therefore, these opclasses are being deprecated in favor of the built-in
+ GiST <literal>inet_ops</literal> opclass, which does not share the design
+ flaw.
+ </para>
+
+ <para>
+ As a first step, <productname>PostgreSQL</productname> version 19 removes
+ the default-opclass marking from <literal>gist_inet_ops</literal>
+ and <literal>gist_cidr_ops</literal>, instead
+ marking <literal>inet_ops</literal> as default for <type>inet</type>
+ and <type>cidr</type> columns. This will result in transparently
+ substituting <literal>inet_ops</literal> for the faulty opclasses in most
+ contexts. It is still possible to create indexes using the faulty
+ opclasses, if really necessary, by explicitly specifying which opclass to
+ use; for example
+<programlisting>
+CREATE TABLE mytable (addr inet);
+CREATE INDEX dubious_index ON mytable USING GIST (addr gist_inet_ops);
+</programlisting>
+ </para>
+
+ <para>
+ However, <application>pg_upgrade</application> cannot handle this change
+ due to implementation limitations. If asked to upgrade a pre-v19
+ database that contains <literal>gist_inet_ops</literal>
+ or <literal>gist_cidr_ops</literal>
+ indexes, <application>pg_upgrade</application> will fail and tell you to
+ replace those indexes before upgrading. This would look approximately
+ like
+<programlisting>
+CREATE INDEX good_index ON mytable USING GIST (addr inet_ops);
+DROP INDEX bad_index;
+</programlisting>
+ </para>
+
+ </sect2>
+
<sect2 id="btree-gist-authors">
<title>Authors</title>
optsProcNumber, /* amoptsprocnum value */
maxProcNumber; /* amsupport value */
bool amstorage; /* amstorage flag */
+ bool isDefault = stmt->isDefault;
List *operators; /* OpFamilyMember list for operators */
List *procedures; /* OpFamilyMember list for support procs */
ListCell *l;
errmsg("operator class \"%s\" for access method \"%s\" already exists",
opcname, stmt->amname)));
+ /*
+ * HACK: if we're trying to create btree_gist's gist_inet_ops or
+ * gist_cidr_ops during a binary upgrade, avoid failure in the next stanza
+ * by silently making the new opclass non-default. Without this kluge, we
+ * would fail to upgrade databases containing pre-1.9 versions of
+ * contrib/btree_gist. We can remove it sometime in the far future when
+ * we don't expect any such databases to exist. (The result of this hack
+ * is that the installed version of btree_gist will approximate btree_gist
+ * 1.9, how closely depending on whether it's 1.8 or something older.
+ * ALTER EXTENSION UPDATE can be used to bring it up to real 1.9.)
+ */
+ if (isDefault && IsBinaryUpgrade)
+ {
+ if (amoid == GIST_AM_OID &&
+ ((typeoid == INETOID && strcmp(opcname, "gist_inet_ops") == 0) ||
+ (typeoid == CIDROID && strcmp(opcname, "gist_cidr_ops") == 0)))
+ isDefault = false;
+ }
+
/*
* If we are creating a default opclass, check there isn't one already.
* (Note we do not restrict this test to visible opclasses; this ensures
* that typcache.c can find unique solutions to its questions.)
*/
- if (stmt->isDefault)
+ if (isDefault)
{
ScanKeyData skey[1];
SysScanDesc scan;
values[Anum_pg_opclass_opcowner - 1] = ObjectIdGetDatum(GetUserId());
values[Anum_pg_opclass_opcfamily - 1] = ObjectIdGetDatum(opfamilyoid);
values[Anum_pg_opclass_opcintype - 1] = ObjectIdGetDatum(typeoid);
- values[Anum_pg_opclass_opcdefault - 1] = BoolGetDatum(stmt->isDefault);
+ values[Anum_pg_opclass_opcdefault - 1] = BoolGetDatum(isDefault);
values[Anum_pg_opclass_opckeytype - 1] = ObjectIdGetDatum(storageoid);
tup = heap_form_tuple(rel->rd_att, values, nulls);
#include "postgres_fe.h"
+#include "catalog/pg_am_d.h"
#include "catalog/pg_authid_d.h"
#include "catalog/pg_class_d.h"
#include "fe_utils/string_utils.h"
static void check_for_incompatible_polymorphics(ClusterInfo *cluster);
static void check_for_tables_with_oids(ClusterInfo *cluster);
static void check_for_not_null_inheritance(ClusterInfo *cluster);
+static void check_for_gist_inet_ops(ClusterInfo *cluster);
static void check_for_pg_role_prefix(ClusterInfo *cluster);
static void check_for_new_tablespace_dir(void);
static void check_for_user_defined_encoding_conversions(ClusterInfo *cluster);
if (GET_MAJOR_VERSION(old_cluster.major_version) <= 1800)
check_for_not_null_inheritance(&old_cluster);
+ /*
+ * The btree_gist extension contains gist_inet_ops and gist_cidr_ops
+ * opclasses that do not reliably give correct answers. We want to
+ * deprecate and eventually remove those, and as a first step v19 marks
+ * them not-opcdefault and instead marks the replacement in-core opclass
+ * "inet_ops" as opcdefault. That creates a problem for pg_upgrade: in
+ * versions where those opclasses were marked opcdefault, pg_dump will
+ * dump indexes using them with no explicit opclass specification, so that
+ * restore would create them using the inet_ops opclass. That would be
+ * incompatible with what's actually in the on-disk files. So refuse to
+ * upgrade if there are any such indexes.
+ */
+ if (GET_MAJOR_VERSION(old_cluster.major_version) <= 1800)
+ check_for_gist_inet_ops(&old_cluster);
+
/*
* Pre-PG 10 allowed tables with 'unknown' type columns and non WAL logged
* hash indexes
check_ok();
}
+/*
+ * Callback function for processing results of query for
+ * check_for_gist_inet_ops()'s UpgradeTask. If the query returned any rows
+ * (i.e., the check failed), write the details to the report file.
+ */
+static void
+process_gist_inet_ops_check(DbInfo *dbinfo, PGresult *res, void *arg)
+{
+ UpgradeTaskReport *report = (UpgradeTaskReport *) arg;
+ int ntups = PQntuples(res);
+ int i_nspname = PQfnumber(res, "nspname");
+ int i_relname = PQfnumber(res, "relname");
+
+ AssertVariableIsOfType(&process_gist_inet_ops_check, UpgradeTaskProcessCB);
+
+ if (ntups == 0)
+ return;
+
+ if (report->file == NULL &&
+ (report->file = fopen_priv(report->path, "w")) == NULL)
+ pg_fatal("could not open file \"%s\": %m", report->path);
+
+ fprintf(report->file, "In database: %s\n", dbinfo->db_name);
+
+ for (int rowno = 0; rowno < ntups; rowno++)
+ fprintf(report->file, " %s.%s\n",
+ PQgetvalue(res, rowno, i_nspname),
+ PQgetvalue(res, rowno, i_relname));
+}
+
+/*
+ * Verify that no indexes use gist_inet_ops/gist_cidr_ops, unless the
+ * opclasses have been changed to not-opcdefault (which would allow
+ * the old server to dump the index definitions with explicit opclasses).
+ */
+static void
+check_for_gist_inet_ops(ClusterInfo *cluster)
+{
+ UpgradeTaskReport report;
+ UpgradeTask *task = upgrade_task_create();
+ const char *query = "SELECT nc.nspname, cc.relname "
+ "FROM pg_catalog.pg_opclass oc, pg_catalog.pg_index i, "
+ " pg_catalog.pg_class cc, pg_catalog.pg_namespace nc "
+ "WHERE oc.opcmethod = " CppAsString2(GIST_AM_OID)
+ " AND oc.opcname IN ('gist_inet_ops', 'gist_cidr_ops')"
+ " AND oc.opcdefault"
+ " AND oc.oid = any(i.indclass)"
+ " AND i.indexrelid = cc.oid AND cc.relnamespace = nc.oid";
+
+ prep_status("Checking for uses of gist_inet_ops/gist_cidr_ops");
+
+ report.file = NULL;
+ snprintf(report.path, sizeof(report.path), "%s/%s",
+ log_opts.basedir,
+ "gist_inet_ops.txt");
+
+ upgrade_task_add_step(task, query, process_gist_inet_ops_check,
+ true, &report);
+ upgrade_task_run(task, cluster);
+ upgrade_task_free(task);
+
+ if (report.file)
+ {
+ fclose(report.file);
+ pg_log(PG_REPORT, "fatal");
+ pg_fatal("Your installation contains indexes that use btree_gist's\n"
+ "gist_inet_ops or gist_cidr_ops opclasses,\n"
+ "which cannot be binary-upgraded. Replace them with indexes\n"
+ "that use the built-in GiST inet_ops opclass.\n"
+ "A list of indexes with the problem is in the file:\n"
+ " %s", report.path);
+ }
+ else
+ check_ok();
+}
+
/*
* check_for_pg_role_prefix()
*
*/
/* yyyymmddN */
-#define CATALOG_VERSION_NO 202601071
+#define CATALOG_VERSION_NO 202601081
#endif
{ opcmethod => 'hash', opcname => 'inet_ops', opcfamily => 'hash/network_ops',
opcintype => 'inet' },
{ opcmethod => 'gist', opcname => 'inet_ops', opcfamily => 'gist/network_ops',
- opcintype => 'inet', opcdefault => 'f' },
+ opcintype => 'inet' },
{ opcmethod => 'spgist', opcname => 'inet_ops',
opcfamily => 'spgist/network_ops', opcintype => 'inet' },
{ oid => '1979', oid_symbol => 'INT2_BTREE_OPS_OID',
'drop extension if exists test_ext7');
}
+ # btree_gist inet/cidr indexes cannot be upgraded to v19
+ if ($old_version < 19)
+ {
+ if ($dbnames{"contrib_regression_btree_gist"})
+ {
+ _add_st($result, 'contrib_regression_btree_gist',
+ "drop index if exists public.inettmp_a_a1_idx");
+ _add_st($result, 'contrib_regression_btree_gist',
+ "drop index if exists public.inetidx");
+ _add_st($result, 'contrib_regression_btree_gist',
+ "drop index public.cidridx");
+ }
+ if ($dbnames{"regression_btree_gist"})
+ {
+ _add_st($result, 'regression_btree_gist',
+ "drop index if exists public.inettmp_a_a1_idx");
+ _add_st($result, 'regression_btree_gist',
+ "drop index if exists public.inetidx");
+ _add_st($result, 'regression_btree_gist',
+ "drop index public.cidridx");
+ }
+ }
+
# we removed these test-support functions in v18
if ($old_version < 18)
{
projects / users / rhaas / postgres.git / commitdiff