On Mon, May 15, 2017 at 12:08 AM, Noah Misch <n...@leadboat.com> wrote: > The above-described topic is currently a PostgreSQL 10 open item. Robert, > since you committed the patch believed to have created it, you own this open > item. If some other commit is more relevant or if this does not belong as a > v10 open item, please let us know. Otherwise, please observe the policy on > open item ownership[1] and send a status update within three calendar days of > this message. Include a date for your subsequent status update. Testers may > discover new open items at any time, and I want to plan to get them all fixed > well in advance of shipping v10. Consequently, I will appreciate your efforts > toward speedy resolution. Thanks.
I don't believe this patch can be committed to beta1 which wraps in just a few hours; it seems to need a bit of work. I'll update again by Friday based on how review unfolds this week. I anticipate committing something on Wednesday or Thursday unless Bruce picks this one up. + /* find hash and gin indexes */ + res = executeQueryOrDie(conn, + "SELECT n.nspname, c.relname " + "FROM pg_catalog.pg_class c, " + " pg_catalog.pg_index i, " + " pg_catalog.pg_am a, " + " pg_catalog.pg_namespace n " + "WHERE i.indexrelid = c.oid AND " + " c.relam = a.oid AND " + " c.relnamespace = n.oid AND " + " a.amname = 'hash'" Comment doesn't match code. + if (!check_mode && db_used) + /* mark hash indexes as invalid */ + PQclear(executeQueryOrDie(conn, Given that we have a comment here, I'd put curly braces around this block. + snprintf(output_path, sizeof(output_path), "reindex_hash.sql"); This looks suspiciously pointless. The contents of output_path will always be precisely "reindex_hash.sql"; you could just char *output_path = "reindex_hash.sql" and get the same effect. Compare this to the logic in create_script_for_cluster_analyze, which prepends SCRIPT_PREFIX. (I am not sure why it's necessary to prepend "./" on Windows, but if it's needed in that case, maybe it's needed in this case, too.) + start_postmaster(&new_cluster, true); + old_9_6_invalidate_hash_indexes(&old_cluster, false); + stop_postmaster(false); Won't this cause the hash indexes to be invalided in the old cluster rather than the new one? This might need a visit from pgindent in one or two places, too. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
