diff --git a/doc/src/sgml/btree-gist.sgml b/doc/src/sgml/btree-gist.sgml index a4c1b99be1f..cc09ec83733 100644 --- a/doc/src/sgml/btree-gist.sgml +++ b/doc/src/sgml/btree-gist.sgml @@ -108,6 +108,53 @@ INSERT 0 1 + + <literal>btree_gist</literal> Indexes + on <type>inet</type>/<type>cidr</type> Columns + + + The gist_inet_ops and gist_cidr_ops + operator classes provided by btree_gist 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 inet_ops opclass, which does not share the design + flaw. + + + + As a first step, PostgreSQL version 19 removes + the default-opclass marking from gist_inet_ops + and gist_cidr_ops, instead + marking inet_ops as default for inet + and cidr columns. This will result in transparently + substituting inet_ops 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 + +CREATE TABLE mytable (addr inet); +CREATE INDEX dubious_index ON mytable USING GIST (addr gist_inet_ops); + + + + + However, pg_upgrade cannot handle this change + due to implementation limitations. If asked to upgrade a pre-v19 + database that contains gist_inet_ops + or gist_cidr_ops + indexes, pg_upgrade will fail and tell you to + replace those indexes before upgrading. This would look approximately + like + +CREATE INDEX good_index ON mytable USING GIST (addr inet_ops); +DROP INDEX bad_index; + + + + + Authors diff --git a/src/backend/commands/opclasscmds.c b/src/backend/commands/opclasscmds.c index 4130aca411e..7493a9ccc06 100644 --- a/src/backend/commands/opclasscmds.c +++ b/src/backend/commands/opclasscmds.c @@ -343,6 +343,7 @@ DefineOpClass(CreateOpClassStmt *stmt) 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; @@ -610,12 +611,31 @@ DefineOpClass(CreateOpClassStmt *stmt) 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; @@ -661,7 +681,7 @@ DefineOpClass(CreateOpClassStmt *stmt) 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); diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c index a47d553b809..64805fef0eb 100644 --- a/src/bin/pg_upgrade/check.c +++ b/src/bin/pg_upgrade/check.c @@ -9,6 +9,7 @@ #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" @@ -24,6 +25,7 @@ static void check_for_user_defined_postfix_ops(ClusterInfo *cluster); 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); @@ -681,6 +683,21 @@ check_and_dump_old_cluster(void) 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 @@ -1721,6 +1738,82 @@ check_for_not_null_inheritance(ClusterInfo *cluster) 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() * diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index 0ebaba0867e..ab38a69f0f8 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -57,6 +57,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 202601071 +#define CATALOG_VERSION_NO 202601081 #endif diff --git a/src/include/catalog/pg_opclass.dat b/src/include/catalog/pg_opclass.dat index 0eddd0298f8..df170b80840 100644 --- a/src/include/catalog/pg_opclass.dat +++ b/src/include/catalog/pg_opclass.dat @@ -57,7 +57,7 @@ { 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', diff --git a/src/test/perl/PostgreSQL/Test/AdjustUpgrade.pm b/src/test/perl/PostgreSQL/Test/AdjustUpgrade.pm index de6908159cc..b8e641cc1cb 100644 --- a/src/test/perl/PostgreSQL/Test/AdjustUpgrade.pm +++ b/src/test/perl/PostgreSQL/Test/AdjustUpgrade.pm @@ -112,6 +112,29 @@ sub adjust_database_contents '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) {