Hi hackers.
While working on jsonbstatistics, I found the following bug:
an empty jsonb array is considered to be lesser than any scalar,
but it is expected that objects > arrays > scalars.
# select '[]'::jsonb < 'null'::jsonb;
?column?
----------
t
(1 row)
Attached patch contains:
1. bug fix (added the missing "else" in compareJsonbContainers())
2. regression test
3. pg_upgrade: invalidation of btree indexes on jsonb columns and
REINDEX-script generation
--
Nikita Glukhov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
diff --git a/src/backend/utils/adt/jsonb_util.c b/src/backend/utils/adt/jsonb_util.c
index ddc34ce..43934bf 100644
--- a/src/backend/utils/adt/jsonb_util.c
+++ b/src/backend/utils/adt/jsonb_util.c
@@ -232,7 +232,7 @@ compareJsonbContainers(JsonbContainer *a, JsonbContainer *b)
*/
if (va.val.array.rawScalar != vb.val.array.rawScalar)
res = (va.val.array.rawScalar) ? -1 : 1;
- if (va.val.array.nElems != vb.val.array.nElems)
+ else if (va.val.array.nElems != vb.val.array.nElems)
res = (va.val.array.nElems > vb.val.array.nElems) ? 1 : -1;
break;
case jbvObject:
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 42bf499..81c1616 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -115,6 +115,11 @@ check_and_dump_old_cluster(bool live_check)
if (GET_MAJOR_VERSION(old_cluster.major_version) <= 804)
new_9_0_populate_pg_largeobject_metadata(&old_cluster, true);
+ /* Pre-PG 10.0 had bug in jsonb comparison operator */
+ if (GET_MAJOR_VERSION(old_cluster.major_version) <= 906 &&
+ GET_MAJOR_VERSION(old_cluster.major_version) >= 904)
+ old_9_6_invalidate_jsonb_btree_indexes(&old_cluster, true);
+
/*
* While not a check option, we do this now because this is the only time
* the old server is running.
@@ -166,11 +171,26 @@ report_clusters_compatible(void)
void
issue_warnings(void)
{
- /* Create dummy large object permissions for old < PG 9.0? */
- if (GET_MAJOR_VERSION(old_cluster.major_version) <= 804)
+ bool need_new_9_0_populate_pg_largeobject_metadata =
+ GET_MAJOR_VERSION(old_cluster.major_version) <= 804;
+
+ bool need_old_9_6_invalidate_jsonb_btree_indexes =
+ GET_MAJOR_VERSION(old_cluster.major_version) <= 906 &&
+ GET_MAJOR_VERSION(old_cluster.major_version) >= 904;
+
+ if (need_new_9_0_populate_pg_largeobject_metadata ||
+ need_old_9_6_invalidate_jsonb_btree_indexes)
{
start_postmaster(&new_cluster, true);
- new_9_0_populate_pg_largeobject_metadata(&new_cluster, false);
+
+ /* Create dummy large object permissions for old < PG 9.0? */
+ if (need_new_9_0_populate_pg_largeobject_metadata)
+ new_9_0_populate_pg_largeobject_metadata(&new_cluster, false);
+
+ /* invalidate jsonb btree indexes for old < PG 10.0 */
+ if (need_old_9_6_invalidate_jsonb_btree_indexes)
+ old_9_6_invalidate_jsonb_btree_indexes(&new_cluster, false);
+
stop_postmaster(false);
}
}
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index 19dca83..07e0ca6 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -442,6 +442,8 @@ void pg_putenv(const char *var, const char *val);
void new_9_0_populate_pg_largeobject_metadata(ClusterInfo *cluster,
bool check_mode);
void old_9_3_check_for_line_data_type_usage(ClusterInfo *cluster);
+void old_9_6_invalidate_jsonb_btree_indexes(ClusterInfo *cluster,
+ bool check_mode);
/* parallel.c */
void parallel_exec_prog(const char *log_file, const char *opt_log_file,
diff --git a/src/bin/pg_upgrade/version.c b/src/bin/pg_upgrade/version.c
index 3c7c5fa..b1a3b89 100644
--- a/src/bin/pg_upgrade/version.c
+++ b/src/bin/pg_upgrade/version.c
@@ -10,6 +10,7 @@
#include "postgres_fe.h"
#include "pg_upgrade.h"
+#include "catalog/pg_type.h"
#include "fe_utils/string_utils.h"
@@ -185,3 +186,116 @@ old_9_3_check_for_line_data_type_usage(ClusterInfo *cluster)
else
check_ok();
}
+
+/*
+ * old_9_6_invalidate_jsonb_btree_indexes()
+ * 9.4-9.6 -> 10.0
+ * Btree index ordering for jsonb had been fixed in 10.0
+ */
+void
+old_9_6_invalidate_jsonb_btree_indexes(ClusterInfo *cluster, bool check_mode)
+{
+ int dbnum;
+ FILE *script = NULL;
+ bool found = false;
+ char output_path[MAXPGPATH];
+
+ prep_status("Checking for jsonb btree indexes existence");
+
+ snprintf(output_path, sizeof(output_path), "reindex_jsonb_btree.sql");
+
+ for (dbnum = 0; dbnum < cluster->dbarr.ndbs; dbnum++)
+ {
+ PGresult *res;
+ bool db_used = false;
+ int ntups;
+ int rowno;
+ int i_nspname,
+ i_relname;
+ DbInfo *active_db = &cluster->dbarr.dbs[dbnum];
+ PGconn *conn = connectToServer(cluster, active_db->db_name);
+
+ /* find jsonb btree indexes */
+ res = executeQueryOrDie(conn,
+ "SELECT DISTINCT n.nspname, c.relname "
+ "FROM pg_catalog.pg_class c, "
+ " pg_catalog.pg_index i, "
+ " pg_catalog.pg_am am, "
+ " pg_catalog.pg_namespace n, "
+ " pg_catalog.pg_attribute a "
+ "WHERE i.indexrelid = c.oid AND "
+ " c.relam = am.oid AND "
+ " c.relnamespace = n.oid AND "
+ " a.attrelid = c.oid AND "
+ " a.atttypid = " CppAsString2(JSONBOID) " AND "
+ " am.amname = 'btree'");
+
+ ntups = PQntuples(res);
+ i_nspname = PQfnumber(res, "nspname");
+ i_relname = PQfnumber(res, "relname");
+
+ for (rowno = 0; rowno < ntups; rowno++)
+ {
+ found = true;
+ if (!check_mode)
+ {
+ if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
+ pg_fatal("could not open file \"%s\": %s\n", output_path, strerror(errno));
+ if (!db_used)
+ {
+ fprintf(script, "\\connect %s\n",
+ quote_identifier(active_db->db_name));
+ db_used = true;
+ }
+ fprintf(script, "REINDEX INDEX %s.%s;\n",
+ quote_identifier(PQgetvalue(res, rowno, i_nspname)),
+ quote_identifier(PQgetvalue(res, rowno, i_relname)));
+ }
+ }
+
+ PQclear(res);
+
+ if (!check_mode && found)
+ /* mark jsonb btree indexes as invalid */
+ PQclear(executeQueryOrDie(conn,
+ "UPDATE pg_catalog.pg_index i "
+ "SET indisvalid = false "
+ "FROM pg_catalog.pg_class c, "
+ " pg_catalog.pg_am am, "
+ " pg_catalog.pg_namespace n, "
+ " pg_catalog.pg_attribute a "
+ "WHERE i.indexrelid = c.oid AND "
+ " c.relam = am.oid AND "
+ " c.relnamespace = n.oid AND "
+ " a.attrelid = c.oid AND "
+ " a.atttypid = " CppAsString2(JSONBOID) " AND "
+ " am.amname = 'btree'"));
+
+ PQfinish(conn);
+ }
+
+ if (script)
+ fclose(script);
+
+ if (found)
+ {
+ report_status(PG_WARNING, "warning");
+ if (check_mode)
+ pg_log(PG_WARNING, "\n"
+ "Your installation contains jsonb btree indexes. These indexes have\n"
+ "different comparison rules between your old and new clusters, so they\n"
+ "must be reindexed with the REINDEX command. After upgrading, you will\n"
+ "be given REINDEX instructions.\n\n");
+ else
+ pg_log(PG_WARNING, "\n"
+ "Your installation contains jsonb btree indexes. These indexes have\n"
+ "different comparison rules between your old and new clusters, so they\n"
+ "must be reindexed with the REINDEX command. The file:\n"
+ " %s\n"
+ "when executed by psql by the database superuser will recreate all invalid\n"
+ "indexes; until then, none of these indexes will be used.\n\n",
+ output_path);
+ }
+ else
+ check_ok();
+}
diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out
index e2cb08a..203088d 100644
--- a/src/test/regress/expected/jsonb.out
+++ b/src/test/regress/expected/jsonb.out
@@ -2256,6 +2256,13 @@ SELECT count(*) FROM testjsonb WHERE j = '{"pos":98, "line":371, "node":"CBA", "
1
(1 row)
+-- bug in comparison of [] and scalars
+SELECT '[]'::jsonb > 'null'::jsonb;
+ ?column?
+----------
+ t
+(1 row)
+
--gin path opclass
DROP INDEX jidx;
CREATE INDEX jidx ON testjsonb USING gin (j jsonb_path_ops);
diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql
index 6b4c796..2d37c4f 100644
--- a/src/test/regress/sql/jsonb.sql
+++ b/src/test/regress/sql/jsonb.sql
@@ -580,6 +580,9 @@ SET enable_seqscan = off;
SELECT count(*) FROM testjsonb WHERE j > '{"p":1}';
SELECT count(*) FROM testjsonb WHERE j = '{"pos":98, "line":371, "node":"CBA", "indexed":true}';
+-- bug in comparison of [] and scalars
+SELECT '[]'::jsonb > 'null'::jsonb;
+
--gin path opclass
DROP INDEX jidx;
CREATE INDEX jidx ON testjsonb USING gin (j jsonb_path_ops);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
