On Fri, Mar 29, 2013 at 07:03:05PM -0400, Tom Lane wrote:
> Andres Freund <and...@2ndquadrant.com> writes:
> > Those columns cannot be NULL, so using IS DISTINCT FROM seems a bit
> > clumsy.
>
> That was what I started to write, too, but actually I think the IS
> DISTINCT is correct and the RIGHT JOIN should be a LEFT JOIN. Note
> that the query appears to be intended to collect regular tables as
> well as indexes. (As patched, that's totally broken, so I infer
> Bruce hasn't tested it yet.)
Yes, I only ran my simple tests so far --- I wanted to at least get some
eyes on it. I was wondering if we ever need to use parentheses for
queries that mix normal and outer joins? I am unclear on that.
Attached is a fixed patch that uses LEFT JOIN. I went back and looked
at the patch that added this test and I think the patch is now complete.
I would like to apply it tomorrow/Saturday so it will be ready for
Monday's packaging, and get some buildfarm time on it.
--
Bruce Momjian <br...@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
diff --git a/contrib/pg_upgrade/check.c b/contrib/pg_upgrade/check.c
new file mode 100644
index 65fb548..35783d0
*** a/contrib/pg_upgrade/check.c
--- b/contrib/pg_upgrade/check.c
*************** static void check_is_super_user(ClusterI
*** 20,26 ****
static void check_for_prepared_transactions(ClusterInfo *cluster);
static void check_for_isn_and_int8_passing_mismatch(ClusterInfo *cluster);
static void check_for_reg_data_type_usage(ClusterInfo *cluster);
- static void check_for_invalid_indexes(ClusterInfo *cluster);
static void get_bin_version(ClusterInfo *cluster);
static char *get_canonical_locale_name(int category, const char *locale);
--- 20,25 ----
*************** check_and_dump_old_cluster(bool live_che
*** 97,103 ****
check_is_super_user(&old_cluster);
check_for_prepared_transactions(&old_cluster);
check_for_reg_data_type_usage(&old_cluster);
- check_for_invalid_indexes(&old_cluster);
check_for_isn_and_int8_passing_mismatch(&old_cluster);
/* old = PG 8.3 checks? */
--- 96,101 ----
*************** check_for_reg_data_type_usage(ClusterInf
*** 952,1046 ****
" %s\n\n", output_path);
}
else
- check_ok();
- }
-
-
- /*
- * check_for_invalid_indexes()
- *
- * CREATE INDEX CONCURRENTLY can create invalid indexes if the index build
- * fails. These are dumped as valid indexes by pg_dump, but the
- * underlying files are still invalid indexes. This checks to make sure
- * no invalid indexes exist, either failed index builds or concurrent
- * indexes in the process of being created.
- */
- static void
- check_for_invalid_indexes(ClusterInfo *cluster)
- {
- int dbnum;
- FILE *script = NULL;
- bool found = false;
- char output_path[MAXPGPATH];
-
- prep_status("Checking for invalid indexes from concurrent index builds");
-
- snprintf(output_path, sizeof(output_path), "invalid_indexes.txt");
-
- 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);
-
- res = executeQueryOrDie(conn,
- "SELECT n.nspname, c.relname "
- "FROM pg_catalog.pg_class c, "
- " pg_catalog.pg_namespace n, "
- " pg_catalog.pg_index i "
- "WHERE (i.indisvalid = false OR "
- " i.indisready = false) AND "
- " i.indexrelid = c.oid AND "
- " c.relnamespace = n.oid AND "
- /* we do not migrate these, so skip them */
- " n.nspname != 'pg_catalog' AND "
- " n.nspname != 'information_schema' AND "
- /* indexes do not have toast tables */
- " n.nspname != 'pg_toast'");
-
- ntups = PQntuples(res);
- i_nspname = PQfnumber(res, "nspname");
- i_relname = PQfnumber(res, "relname");
- for (rowno = 0; rowno < ntups; rowno++)
- {
- found = true;
- if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
- pg_log(PG_FATAL, "Could not open file \"%s\": %s\n",
- output_path, getErrorText(errno));
- if (!db_used)
- {
- fprintf(script, "Database: %s\n", active_db->db_name);
- db_used = true;
- }
- fprintf(script, " %s.%s\n",
- PQgetvalue(res, rowno, i_nspname),
- PQgetvalue(res, rowno, i_relname));
- }
-
- PQclear(res);
-
- PQfinish(conn);
- }
-
- if (script)
- fclose(script);
-
- if (found)
- {
- pg_log(PG_REPORT, "fatal\n");
- pg_log(PG_FATAL,
- "Your installation contains invalid indexes due to failed or\n"
- "currently running CREATE INDEX CONCURRENTLY operations. You\n"
- "cannot upgrade until these indexes are valid or removed. A\n"
- "list of the problem indexes is in the file:\n"
- " %s\n\n", output_path);
- }
- else
check_ok();
}
--- 950,955 ----
diff --git a/contrib/pg_upgrade/info.c b/contrib/pg_upgrade/info.c
new file mode 100644
index a5aa40f..c5c3698
*** a/contrib/pg_upgrade/info.c
--- b/contrib/pg_upgrade/info.c
*************** get_rel_infos(ClusterInfo *cluster, DbIn
*** 282,288 ****
--- 282,294 ----
"CREATE TEMPORARY TABLE info_rels (reloid) AS SELECT c.oid "
"FROM pg_catalog.pg_class c JOIN pg_catalog.pg_namespace n "
" ON c.relnamespace = n.oid "
+ "LEFT OUTER JOIN pg_catalog.pg_index i "
+ " ON c.oid = i.indexrelid "
"WHERE relkind IN ('r', 'm', 'i'%s) AND "
+ /* pg_dump only dumps valid indexes; testing indisready is
+ * necessary in 9.2, and harmless in earlier/later versions. */
+ " i.indisvalid IS DISTINCT FROM false AND "
+ " i.indisready IS DISTINCT FROM false AND "
/* exclude possible orphaned temp tables */
" ((n.nspname !~ '^pg_temp_' AND "
" n.nspname !~ '^pg_toast_temp_' AND "
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
