Re: [HACKERS] Fix for pg_upgrade and invalid indexes
On 2013-03-29 19:03:05 -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.) Ah yes. Then I'd actually find it much more readable to formulate it as a NOT EXISTS(), but that might be just me. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix for pg_upgrade and invalid indexes
OK, patch applied and backpatched as far back as pg_upgrade exists in
git.
---
On Fri, Mar 29, 2013 at 11:35:13PM -0400, Bruce Momjian wrote:
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.ushttp://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;
- boolfound = false;
- charoutput_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;
- booldb_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 !=
Re: [HACKERS] Fix for pg_upgrade and invalid indexes
Bruce Momjian br...@momjian.us writes: Attached is a patch that implements the suggested pg_upgrade changes of not copying invalid indexes now that pg_dump doesn't dump them. This should be backpatched back to 8.4 to match pg_dump. It might require release note updates; not sure. Previously pg_upgrade threw an error if invalid indexes exist, but only since February, when we released the pg_upgrade fix to do this. You can see the majority of this patch is removing that check. Surely that should be LEFT JOIN not RIGHT JOIN? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix for pg_upgrade and invalid indexes
On 2013-03-29 16:57:06 -0400, Bruce Momjian wrote:
On Thu, Mar 28, 2013 at 05:27:28PM -0400, Tom Lane wrote:
Bruce Momjian br...@momjian.us writes:
Should I just patch pg_upgrade to remove the indisvalid, skip
indisvalid indexes, and backpatch it? Users should be using the
version of pg_upgrade to match new pg_dump. Is there any case where
they don't match? Do I still need to check for indisready?
Yeah, if you can just ignore !indisvalid indexes that should work fine.
I see no need to look at indisready if you're doing that.
Attached is a patch that implements the suggested pg_upgrade changes of
not copying invalid indexes now that pg_dump doesn't dump them. This
should be backpatched back to 8.4 to match pg_dump. It might require
release note updates; not sure. Previously pg_upgrade threw an error
if invalid indexes exist, but only since February, when we released the
pg_upgrade fix to do this. You can see the majority of this patch is
removing that check.
+ RIGHT 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
Those columns cannot be NULL, so using IS DISTINCT FROM seems a bit
clumsy.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix for pg_upgrade and invalid indexes
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.) regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix for pg_upgrade and invalid indexes
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.ushttp://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();
}
Re: [HACKERS] Fix for pg_upgrade status display
On Wed, Dec 5, 2012 at 10:04:53PM -0500, Bruce Momjian wrote: Pg_upgrade displays file names during copy and database names during dump/restore. Andrew Dunstan identified three bugs: * long file names were being truncated to 60 _leading_ characters, which often do not change for long file names * file names were truncated to 60 characters in log files * carriage returns were being output to log files The attached patch fixes these --- it prints 60 _trailing_ characters to the status display, and full path names without carriage returns to log files. Patch applied. It also suppresses status output to the log file unless verbose mode is used. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix for pg_upgrade status display
On Wed, Dec 5, 2012 at 10:04 PM, Bruce Momjian br...@momjian.us wrote: Pg_upgrade displays file names during copy and database names during dump/restore. Andrew Dunstan identified three bugs: * long file names were being truncated to 60 _leading_ characters, which often do not change for long file names * file names were truncated to 60 characters in log files * carriage returns were being output to log files The attached patch fixes these --- it prints 60 _trailing_ characters to the status display, and full path names without carriage returns to log files. This might be a dumb question, but why limit it to 60 characters at all instead of, say, MAXPGPATH? -- 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
Re: [HACKERS] Fix for pg_upgrade status display
Robert Haas escribió: On Wed, Dec 5, 2012 at 10:04 PM, Bruce Momjian br...@momjian.us wrote: Pg_upgrade displays file names during copy and database names during dump/restore. Andrew Dunstan identified three bugs: * long file names were being truncated to 60 _leading_ characters, which often do not change for long file names * file names were truncated to 60 characters in log files * carriage returns were being output to log files The attached patch fixes these --- it prints 60 _trailing_ characters to the status display, and full path names without carriage returns to log files. This might be a dumb question, but why limit it to 60 characters at all instead of, say, MAXPGPATH? I think this should be keyed off the terminal width, actually, no? The whole point of this is to overwrite the same line over and over, right? -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix for pg_upgrade status display
On Thu, Dec 6, 2012 at 12:43:53PM -0500, Robert Haas wrote: On Wed, Dec 5, 2012 at 10:04 PM, Bruce Momjian br...@momjian.us wrote: Pg_upgrade displays file names during copy and database names during dump/restore. Andrew Dunstan identified three bugs: * long file names were being truncated to 60 _leading_ characters, which often do not change for long file names * file names were truncated to 60 characters in log files * carriage returns were being output to log files The attached patch fixes these --- it prints 60 _trailing_ characters to the status display, and full path names without carriage returns to log files. This might be a dumb question, but why limit it to 60 characters at all instead of, say, MAXPGPATH? It is limited to 60 only for screen display, so the user knows what is being processed. If the text wraps across several lines, the \r trick to overwrite the string will not work. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix for pg_upgrade status display
On Thu, Dec 6, 2012 at 02:53:44PM -0300, Alvaro Herrera wrote: Robert Haas escribió: On Wed, Dec 5, 2012 at 10:04 PM, Bruce Momjian br...@momjian.us wrote: Pg_upgrade displays file names during copy and database names during dump/restore. Andrew Dunstan identified three bugs: * long file names were being truncated to 60 _leading_ characters, which often do not change for long file names * file names were truncated to 60 characters in log files * carriage returns were being output to log files The attached patch fixes these --- it prints 60 _trailing_ characters to the status display, and full path names without carriage returns to log files. This might be a dumb question, but why limit it to 60 characters at all instead of, say, MAXPGPATH? I think this should be keyed off the terminal width, actually, no? The whole point of this is to overwrite the same line over and over, right? That seems like overkill for a status message. It is just there so users know pg_upgrade isn't stuck, which was the complaint before the message was used. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] fix for pg_upgrade
OK, works now with the recent update. Thanks -- View this message in context: http://postgresql.1045698.n5.nabble.com/fix-for-pg-upgrade-tp3411128p5059777.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] fix for pg_upgrade
Great, thanks! -- View this message in context: http://postgresql.1045698.n5.nabble.com/fix-for-pg-upgrade-tp3411128p4856336.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] fix for pg_upgrade
Alvaro Herrera wrote: Excerpts from Bruce Momjian's message of miC3A9 sep 28 13:48:28 -0300 2011: Bruce Momjian wrote: OK, so it fails for all tables and you are using the newest version. Thanks for all your work. I am now guessing that pg_upgrade 9.1.X is just broken on Windows. Perhaps the variables set by pg_upgrade_support.so are not being passed into the server variables? I know pg_upgrade 9.0.X worked on Windows because EnterpriseDB did extensive testing recently on this. Has anyone used pg_upgrade 9.1.X on Windows? OK, I have a new theory. postmaster.c processes the -b (binary-upgrade) flag by setting a C variable: case 'b': /* Undocumented flag used for binary upgrades */ IsBinaryUpgrade = true; break; I am now wondering if this variable is not being passed down to the sessions during Win32's EXEC_BACKEND. Looking at the other postmaster settings, these set GUC variables, which I assume are passed down. Can someone confirm this? Well, you could compile it with -DEXEC_BACKEND to test it for yourself. How should this be fixed? Maybe it should be part of struct BackendParameters. Thanks. That's what I did, and tested the failure with -DEXEC_BACKEND, and the fix with the patch, which is attached. I am confident this will fix Windows as well. Applied, and backpatched to 9.1.X. Thanks for the report. The fix will be in 9.1.2. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] fix for pg_upgrade
Here are all generated log files. I just removed all other DBs except gnucash (which includes the accounts table), but the issue also emerges with other DBs. Upgraded the 9.1 instance to the new build (9.1.1.) as well but this apparently did not change anything. PG versions are (including generated logs): PostgreSQL 9.0.4, compiled by Visual C++ build 1500, 64-bit PostgreSQL 9.1.0, compiled by Visual C++ build 1500, 64-bit: http://postgresql.1045698.n5.nabble.com/file/n4848829/pg_upgrade_9.1.0.zip pg_upgrade_9.1.0.zip PostgreSQL 9.1.1, compiled by Visual C++ build 1500, 64-bit: http://postgresql.1045698.n5.nabble.com/file/n4848829/pg_upgrade_9.1.1.zip pg_upgrade_9.1.1.zip I hope that is what you meant with pg_upgrade log file. Regards, panam -- View this message in context: http://postgresql.1045698.n5.nabble.com/fix-for-pg-upgrade-tp3411128p4848829.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] fix for pg_upgrade
panam wrote:
Here are all generated log files.
I just removed all other DBs except gnucash (which includes the accounts
table), but the issue also emerges with other DBs.
Upgraded the 9.1 instance to the new build (9.1.1.) as well but this
apparently did not change anything.
PG versions are (including generated logs):
PostgreSQL 9.0.4, compiled by Visual C++ build 1500, 64-bit
PostgreSQL 9.1.0, compiled by Visual C++ build 1500, 64-bit:
http://postgresql.1045698.n5.nabble.com/file/n4848829/pg_upgrade_9.1.0.zip
pg_upgrade_9.1.0.zip
PostgreSQL 9.1.1, compiled by Visual C++ build 1500, 64-bit:
http://postgresql.1045698.n5.nabble.com/file/n4848829/pg_upgrade_9.1.1.zip
pg_upgrade_9.1.1.zip
I hope that is what you meant with pg_upgrade log file.
OK, so it fails for all tables and you are using the newest version.
Thanks for all your work. I am now guessing that pg_upgrade 9.1.X is
just broken on Windows.
Perhaps the variables set by pg_upgrade_support.so are not being passed
into the server variables? I know pg_upgrade 9.0.X worked on Windows
because EnterpriseDB did extensive testing recently on this. Has
anyone used pg_upgrade 9.1.X on Windows?
As far as a log file, you need you to use '-l log' and email me that
file.
As far as testing, I wonder if we need to load in pg_upgrade_support on
Windows, and rerun some of the pg_dumpall SQL create table statements to
see why the pg_class.oid and others are not getting set. For example,
this:
-- For binary upgrade, must preserve pg_class oids
SELECT
binary_upgrade.set_next_heap_pg_class_oid('465783'::pg_catalog.oid);
SELECT
binary_upgrade.set_next_toast_pg_class_oid('465786'::pg_catalog.oid);
SELECT
binary_upgrade.set_next_index_pg_class_oid('465788'::pg_catalog.oid);
CREATE TABLE accounts (
guid character varying(32) NOT NULL,
name character varying(2048) NOT NULL,
account_type character varying(2048) NOT NULL,
commodity_guid character varying(32),
commodity_scu integer NOT NULL,
non_std_scu integer NOT NULL,
parent_guid character varying(32),
code character varying(2048),
description character varying(2048),
hidden integer,
placeholder integer
);
should set the accounts pg_class.oid as 465783. The server will need to
be started with -b and this will disable autovacuum. Can someone on
Windows try this?
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] fix for pg_upgrade
Bruce Momjian wrote: OK, so it fails for all tables and you are using the newest version. Thanks for all your work. I am now guessing that pg_upgrade 9.1.X is just broken on Windows. Perhaps the variables set by pg_upgrade_support.so are not being passed into the server variables? I know pg_upgrade 9.0.X worked on Windows because EnterpriseDB did extensive testing recently on this. Has anyone used pg_upgrade 9.1.X on Windows? OK, I have a new theory. postmaster.c processes the -b (binary-upgrade) flag by setting a C variable: case 'b': /* Undocumented flag used for binary upgrades */ IsBinaryUpgrade = true; break; I am now wondering if this variable is not being passed down to the sessions during Win32's EXEC_BACKEND. Looking at the other postmaster settings, these set GUC variables, which I assume are passed down. Can someone confirm this? How should this be fixed? FYI, the binary-upgrade set() functions will not operate unless the -b option is enabled, which explains the failure the reporter is seeing. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] fix for pg_upgrade
Hi Bruce, here is the file you asked for: http://postgresql.1045698.n5.nabble.com/file/n4850735/pg_upgrade_logfile.txt pg_upgrade_logfile.txt I guess you are not addressing me here, right? The server will need to be started with -b and this will disable autovacuum. Can someone on Windows try this? Thanks panam -- View this message in context: http://postgresql.1045698.n5.nabble.com/fix-for-pg-upgrade-tp3411128p4850735.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] fix for pg_upgrade
panam wrote: Hi Bruce, here is the file you asked for: http://postgresql.1045698.n5.nabble.com/file/n4850735/pg_upgrade_logfile.txt pg_upgrade_logfile.txt OK, I see it using -b to pg_ctl: C:\Program Files\PostgreSQL\9.1\bin/pg_ctl -w -l nul -D D:\applications\postgres\9.1 -o -p 5432 -b start nul 21 What I have to find out is whether this is passed to the individual session processes. I guess is no. I guess you are not addressing me here, right? The server will need to be started with -b and this will disable autovacuum. Can someone on Windows try this? No, not really. I think it is a software bug and I need guidance about a solution. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] fix for pg_upgrade
Excerpts from Bruce Momjian's message of mié sep 28 13:48:28 -0300 2011: Bruce Momjian wrote: OK, so it fails for all tables and you are using the newest version. Thanks for all your work. I am now guessing that pg_upgrade 9.1.X is just broken on Windows. Perhaps the variables set by pg_upgrade_support.so are not being passed into the server variables? I know pg_upgrade 9.0.X worked on Windows because EnterpriseDB did extensive testing recently on this. Has anyone used pg_upgrade 9.1.X on Windows? OK, I have a new theory. postmaster.c processes the -b (binary-upgrade) flag by setting a C variable: case 'b': /* Undocumented flag used for binary upgrades */ IsBinaryUpgrade = true; break; I am now wondering if this variable is not being passed down to the sessions during Win32's EXEC_BACKEND. Looking at the other postmaster settings, these set GUC variables, which I assume are passed down. Can someone confirm this? Well, you could compile it with -DEXEC_BACKEND to test it for yourself. How should this be fixed? Maybe it should be part of struct BackendParameters. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] fix for pg_upgrade
Alvaro Herrera wrote: Excerpts from Bruce Momjian's message of mi?? sep 28 13:48:28 -0300 2011: Bruce Momjian wrote: OK, so it fails for all tables and you are using the newest version. Thanks for all your work. I am now guessing that pg_upgrade 9.1.X is just broken on Windows. Perhaps the variables set by pg_upgrade_support.so are not being passed into the server variables? I know pg_upgrade 9.0.X worked on Windows because EnterpriseDB did extensive testing recently on this. Has anyone used pg_upgrade 9.1.X on Windows? OK, I have a new theory. postmaster.c processes the -b (binary-upgrade) flag by setting a C variable: case 'b': /* Undocumented flag used for binary upgrades */ IsBinaryUpgrade = true; break; I am now wondering if this variable is not being passed down to the sessions during Win32's EXEC_BACKEND. Looking at the other postmaster settings, these set GUC variables, which I assume are passed down. Can someone confirm this? Well, you could compile it with -DEXEC_BACKEND to test it for yourself. How should this be fixed? Maybe it should be part of struct BackendParameters. Thanks. That's what I did, and tested the failure with -DEXEC_BACKEND, and the fix with the patch, which is attached. I am confident this will fix Windows as well. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c new file mode 100644 index 94b57fa..0a84d97 *** a/src/backend/postmaster/postmaster.c --- b/src/backend/postmaster/postmaster.c *** typedef struct *** 433,438 --- 433,439 TimestampTz PgStartTime; TimestampTz PgReloadTime; bool redirection_done; + bool IsBinaryUpgrade; #ifdef WIN32 HANDLE PostmasterHandle; HANDLE initial_signal_pipe; *** save_backend_variables(BackendParameters *** 4653,4658 --- 4654,4660 param-PgReloadTime = PgReloadTime; param-redirection_done = redirection_done; + param-IsBinaryUpgrade = IsBinaryUpgrade; #ifdef WIN32 param-PostmasterHandle = PostmasterHandle; *** restore_backend_variables(BackendParamet *** 4874,4879 --- 4876,4882 PgReloadTime = param-PgReloadTime; redirection_done = param-redirection_done; + IsBinaryUpgrade = param-IsBinaryUpgrade; #ifdef WIN32 PostmasterHandle = param-PostmasterHandle; -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] fix for pg_upgrade
Hi Bruce, here is the whole dump (old DB): http://postgresql.1045698.n5.nabble.com/file/n4844725/dump.txt dump.txt Regards, panam -- View this message in context: http://postgresql.1045698.n5.nabble.com/fix-for-pg-upgrade-tp3411128p4844725.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] fix for pg_upgrade
panam wrote:
Hi Bruce,
here is the whole dump (old DB):
http://postgresql.1045698.n5.nabble.com/file/n4844725/dump.txt dump.txt
Wow, that is interesting. I see this in the dump output:
-- For binary upgrade, must preserve relfilenodes
SELECT
binary_upgrade.set_next_heap_relfilenode('465783'::pg_catalog.oid);
SELECT
binary_upgrade.set_next_toast_relfilenode('465786'::pg_catalog.oid);
SELECT
binary_upgrade.set_next_index_relfilenode('465788'::pg_catalog.oid);
CREATE TABLE accounts (
guid character varying(32) NOT NULL,
name character varying(2048) NOT NULL,
account_type character varying(2048) NOT NULL,
commodity_guid character varying(32),
commodity_scu integer NOT NULL,
non_std_scu integer NOT NULL,
parent_guid character varying(32),
code character varying(2048),
description character varying(2048),
hidden integer,
placeholder integer
);
and it is clearly saying the oid/relfilenode should be 465783, but your
9.1 query shows:
C:\Program Files\PostgreSQL\9.1\binpsql -c select * from pg_class
where oid = 465783 or oid = 16505; -p 5433 -U postgres
relname | relnamespace | reltype | reloftype | relowner | relam |
relfilenode | reltablespace | relpages | reltuples | reltoastrelid |
reltoastidxid | relhasindex | relisshared | relpersistence | relkind | relnatts
| relchecks | relhasoids | relhaspkey | relhasrules | relhastriggers |
relhassubclass | relfrozenxid | relacl | reloptions
--+--+-+---+--+---+-+---+--+---+---+---+-+-++-+--+---+++-+++--++
accounts | 2200 | 16507 | 0 |16417 | 0 |
16505 | 0 |0 | 0 | 16508 | 0
| t | f | p | r | 11 | 0 |
f | t | f | f | f |
3934366 ||
(1 row)
and 9.0 says correctly 465783:
C:\Program Files\PostgreSQL\9.0\binpsql -c select * from pg_class
where oid = 465783 or oid = 16505; -p 5432 -U postgres
relname | relnamespace | reltype | reloftype | relowner | relam |
relfilenode | reltablespace | relpages | reltuples | reltoastrelid |
reltoastidxid | relhasindex | relisshared | relistemp | relkind | relnatts |
relchecks | relhasoids | relhaspkey | relhasexclusion | relhasrules |
relhastriggers | relhassubclass | relfrozenxid | relacl | reloptions
--+--+-+---+--+---+-+---+--+---+---+---+-+-+---+-+--+---+++-+-+++--++
accounts | 465781 | 465785 | 0 | 456619 | 0 |
465783 | 0 |3 | 122 |465786 | 0
| t | f | f | r | 11 | 0 | f
| t | f | f | f | f
| 3934366 ||
(1 row)
It is as though the system ignoring the set_next_heap_relfilenode()
call, but I don't see how that could happen. I don't see any other
'accounts' table in that dump.
My only guess at this point is that somehow the -b/IsBinaryUpgrade flag
is not being processed or regognized, and hence the binary_upgrade 'set'
routines are not working.
Is this 9.1 final or later? Can you turn on debug mode and send me the
pg_upgrade log file that is generated? I am going go look for the
pg_ctl -o '-b' flag. Are all databases/objects failing or just this
one?
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] fix for pg_upgrade
panam wrote: Hi Bruce, on the old DB I've got 465783 as oid whereas on the new one it is 16505. is not in the dump file (old db), even 16385 (i guess this is a typo here) or 16505 are not. The only line in which 465783 could be found is I need to see the lines after this. Is that enough information or should I send the whole dump? That's a bit of work as I have to expunge some sensitive schema data, or is there a meaningful way to just do the dump for a single db? You can do: pg_dump --binary-upgrade --schema-only dbname -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] fix for pg_upgrade
Hi Bruce, on the old DB I've got 465783 as oid whereas on the new one it is 16505. is not in the dump file (old db), even 16385 (i guess this is a typo here) or 16505 are not. The only line in which 465783 could be found is Is that enough information or should I send the whole dump? That's a bit of work as I have to expunge some sensitive schema data, or is there a meaningful way to just do the dump for a single db? Thanks regards, panam -- View this message in context: http://postgresql.1045698.n5.nabble.com/fix-for-pg-upgrade-tp3411128p4843289.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] fix for pg_upgrade
OK, i started once again: I hope the following is the correct way of querying the table corresponding to a relid: -- View this message in context: http://postgresql.1045698.n5.nabble.com/fix-for-pg-upgrade-tp3411128p4838427.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] fix for pg_upgrade
panam wrote:
OK, i started once again:
I hope the following is the correct way of querying the table corresponding
to a relid:
--
View this message in context:
http://postgresql.1045698.n5.nabble.com/fix-for-pg-upgrade-tp3411128p4838427.html
Yes, that is very close to what I needed. Ideally you would have
included the oid from pg_class:
select oid, * from pg_class where oid = 465783 or oid = 16505
Can you supply that?
Also can you email me privately the following output from the old
database? It should only be the schema and not your data:
pg_dumpall --schema-only --binary-upgrade
I am looking for something like this in the file:
-- For binary upgrade, must preserve pg_class oids
SELECT
binary_upgrade.set_next_heap_pg_class_oid('16385'::pg_catalog.oid);
CREATE TABLE test (
x integer
);
but for your case it would be the 'accounts' file. You can email just
those lines if you want, and that you can probably email to hackers.
Thanks.
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] fix for pg_upgrade
\panam wrote: Hi, just tried to upgrade from 9.0 to 9.1 and got this error during pg_upgrade : Mismatch of relation id: database xyz, old relid 465783, new relid 16494 It seems, I get this error on every table as I got it on another table (which I did not need and deleted) before as well. Schmemas seem to be migrated but the content is missing. I am using Windows 7 64bit (both PG servers are 64 bit as well), everthing on the same machine. Sorry for the delay in replying. It is odd you got a mismatch of relids because pg_upgrade is supposed to preserve all of those. Can you do a query to find out what table is relid of 465783 on the old cluster? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] fix for pg_upgrade
Hi, just tried to upgrade from 9.0 to 9.1 and got this error during pg_upgrade : Mismatch of relation id: database xyz, old relid 465783, new relid 16494 It seems, I get this error on every table as I got it on another table (which I did not need and deleted) before as well. Schmemas seem to be migrated but the content is missing. I am using Windows 7 64bit (both PG servers are 64 bit as well), everthing on the same machine. Any ideas? Thanks regards panam -- View this message in context: http://postgresql.1045698.n5.nabble.com/fix-for-pg-upgrade-tp3411128p4798957.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix for pg_upgrade user flag
On Sat, May 7, 2011 at 8:56 AM, Bruce Momjian br...@momjian.us wrote: The attached, applied patch checks that the pg_upgrade user specified is a super-user. It also reports the error message when the post-pg_ctl connection fails. This was prompted by a private bug report from EnterpriseDB. It strikes me that it's fairly crazy to think you're going to be able to catch all the possible errors the server might throw this way. Don't we need some way of letting the actual server errors leak out? -- 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
Re: [HACKERS] Fix for pg_upgrade user flag
On Sat, May 7, 2011 at 9:50 AM, Robert Haas robertmh...@gmail.com wrote: On Sat, May 7, 2011 at 8:56 AM, Bruce Momjian br...@momjian.us wrote: The attached, applied patch checks that the pg_upgrade user specified is a super-user. It also reports the error message when the post-pg_ctl connection fails. This was prompted by a private bug report from EnterpriseDB. It strikes me that it's fairly crazy to think you're going to be able to catch all the possible errors the server might throw this way. Don't we need some way of letting the actual server errors leak out? Or, hmm. Maybe you just did that. If so, never mind. :-) -- 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
Re: [HACKERS] Fix for pg_upgrade user flag
Robert Haas wrote: On Sat, May 7, 2011 at 9:50 AM, Robert Haas robertmh...@gmail.com wrote: On Sat, May 7, 2011 at 8:56 AM, Bruce Momjian br...@momjian.us wrote: The attached, applied patch checks that the pg_upgrade user specified is a super-user. ?It also reports the error message when the post-pg_ctl connection fails. This was prompted by a private bug report from EnterpriseDB. It strikes me that it's fairly crazy to think you're going to be able to catch all the possible errors the server might throw this way. Don't we need some way of letting the actual server errors leak out? Or, hmm. Maybe you just did that. If so, never mind. :-) What I did was to report the errors of our first database probe after we started the server --- for some reason, that code was not reporting the libpq error message, while all other failed connections did. The second change was to only run pg_upgrade as a database super-user, and hopefully that will avoid odd pg_dump error messages. One question I have is why we even bother to allow the database username to be specified? Shouldn't we just hard-code that to 'postgres'? Is there any reason to allow another username to be used? You can't drop the postgres user but you can remove super-user permissions from it so maybe we have to continue allowing it: postgres= drop user postgres; ERROR: cannot drop role postgres because it is required by the database system postgres= alter user postgres nosuperuser; ALTER ROLE -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix for pg_upgrade user flag
Bruce Momjian br...@momjian.us writes: One question I have is why we even bother to allow the database username to be specified? Shouldn't we just hard-code that to 'postgres'? Only if you want to render pg_upgrade unusable by a significant fraction of people. postgres is not the hard wired name of the bootstrap superuser. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix for pg_upgrade user flag
Tom Lane wrote: Bruce Momjian br...@momjian.us writes: One question I have is why we even bother to allow the database username to be specified? Shouldn't we just hard-code that to 'postgres'? Only if you want to render pg_upgrade unusable by a significant fraction of people. postgres is not the hard wired name of the bootstrap superuser. I was really wondering if I should be using that hard-coded name, rather than allowing the user to supply it. They have to compile in a different name, and I assume that name is accessible somewhere. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix for pg_upgrade user flag
Bruce Momjian wrote:
Tom Lane wrote:
Bruce Momjian writes:
One question I have is why we even bother to allow the database
username to be specified? Shouldn't we just hard-code that to
'postgres'?
Only if you want to render pg_upgrade unusable by a significant
fraction of people. postgres is not the hard wired name of the
bootstrap superuser.
I was really wondering if I should be using that hard-coded name,
rather than allowing the user to supply it. They have to compile in
a different name, and I assume that name is accessible somewhere.
At home, on my ubuntu machine, I built and ran initdb without
specifying a superuser. It used my OS login of kevin. Then,
test=# \du
List of roles
Role name | Attributes | Member
of
---++
---
kevin | Superuser, Create role, Create DB, Replication | {}
test=# create user xxx superuser;
CREATE ROLE
test=# \c test xxx
You are now connected to database test as user xxx.
test=# alter user kevin rename to yyy;
ALTER ROLE
test=# \du
List of roles
Role name | Attributes | Member
of
---++
---
xxx | Superuser, Replication | {}
yyy | Superuser, Create role, Create DB, Replication | {}
If I run pg_upgrade now, what will it pick? How?
-Kevin
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix for pg_upgrade user flag
On lör, 2011-05-07 at 13:50 -0400, Bruce Momjian wrote: I was really wondering if I should be using that hard-coded name, rather than allowing the user to supply it. They have to compile in a different name, and I assume that name is accessible somewhere. postgres is not compiled in. It's whatever user you run initdb under. In particular, in the regression tests, it is probably not postgres. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix for pg_upgrade user flag
Kevin Grittner wrote:
Bruce Momjian wrote:
Tom Lane wrote:
Bruce Momjian writes:
One question I have is why we even bother to allow the database
username to be specified? Shouldn't we just hard-code that to
'postgres'?
Only if you want to render pg_upgrade unusable by a significant
fraction of people. postgres is not the hard wired name of the
bootstrap superuser.
I was really wondering if I should be using that hard-coded name,
rather than allowing the user to supply it. They have to compile in
a different name, and I assume that name is accessible somewhere.
At home, on my ubuntu machine, I built and ran initdb without
specifying a superuser. It used my OS login of kevin. Then,
test=# \du
List of roles
Role name | Attributes | Member
of
---++
---
kevin | Superuser, Create role, Create DB, Replication | {}
test=# create user xxx superuser;
CREATE ROLE
test=# \c test xxx
You are now connected to database test as user xxx.
test=# alter user kevin rename to yyy;
ALTER ROLE
test=# \du
List of roles
Role name | Attributes | Member
of
---++
---
xxx | Superuser, Replication | {}
yyy | Superuser, Create role, Create DB, Replication | {}
If I run pg_upgrade now, what will it pick? How?
Good point --- you would need the user flag in pg_upgrade. Thanks.
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix for pg_upgrade user flag
Peter Eisentraut wrote: On l?r, 2011-05-07 at 13:50 -0400, Bruce Momjian wrote: I was really wondering if I should be using that hard-coded name, rather than allowing the user to supply it. They have to compile in a different name, and I assume that name is accessible somewhere. postgres is not compiled in. It's whatever user you run initdb under. In particular, in the regression tests, it is probably not postgres. Thanks. I get confused because the 'postgres' database is hardcoded in, but not the username. Not sure why I am so easily confused. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix for pg_upgrade user flag
On 05/07/2011 06:48 PM, Bruce Momjian wrote: postgres is not compiled in. It's whatever user you run initdb under. In particular, in the regression tests, it is probably not postgres. Thanks. I get confused because the 'postgres' database is hardcoded in, but not the username. Not sure why I am so easily confused. There is a requirement for a known database name, but no requirement for a known superuser name. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix for pg_upgrade discriptor leaks
Bruce Momjian br...@momjian.us writes: I have applied the attached patch to fix pg_upgrade file descriptor leaks in error paths. It seems rather pointless to spend code closing descriptors immediately before a fatal exit. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix for pg_upgrade discriptor leaks
Tom Lane wrote: Bruce Momjian br...@momjian.us writes: I have applied the attached patch to fix pg_upgrade file descriptor leaks in error paths. It seems rather pointless to spend code closing descriptors immediately before a fatal exit. Well, it is not before a fatal but rather before it returns -1, which might fatal or might not. I also had code in to close file descriptors that was a little too tricky about using a single variable to indicate two things so I cleaned it up. I got a private email report about these so obviously they were confusing. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix for pg_upgrade migrating pg_largeobject_metadata
Patch applied.
I did not backpatch to 9.0 because you can't migrate from 9.0 to 9.0
with the same catversion (because of tablespace conflict), and a pre-9.0
migration to 9.0 has not large object permissions to migrate. In
summary, it didn't seem worth the risk, and was hard to test.
---
Bruce Momjian wrote:
Bruce Momjian wrote:
Tom Lane wrote:
Bruce Momjian br...@momjian.us writes:
Tom Lane wrote:
That isn't going to work. At least not unless you start trying to
force
roles to have the same OIDs in the new installation.
If so I can use the CREATE ROLE ... SYSID clause when doing a binary
upgrade.
Oh, I had forgotten we still had that wart in the grammar.
It doesn't actually work:
else if (strcmp(defel-defname, sysid) == 0)
{
ereport(NOTICE,
(errmsg(SYSID can no longer be
specified)));
}
Not sure if it's better to try to make that work again than to add
another hack in pg_upgrade_support. On the whole that's a keyword
I'd rather see us drop someday soon.
OK, let me work on adding it to pg_upgrade_support. Glad you saw this.
I have fixed the bug by using pg_upgrade_support. It was a little
complicated because you need to install the pg_upgrade_support functions
in the super-user database so it is available when you create the users
in the first step of restoring the pg_dumpall file.
I am afraid we have to batckpatch this to fix to 9.0 for 9.0 to 9.0
upgrades. It does not apply when coming from pre-9.0 because there was
no pg_largeobject_metadata.
For testing I did this:
CREATE DATABASE lo;
\c lo
SELECT lo_import('/etc/motd');
\set loid `psql -qt -c 'select loid from pg_largeobject' lo`
CREATE ROLE user1;
CREATE ROLE user2;
-- force user2 to have a different user id on restore
DROP ROLE user1;
GRANT ALL ON LARGE OBJECT :loid TO user2;
The fixed version shows:
lo= select * from pg_largeobject_metadata;
lomowner | lomacl
--+--
10 | {postgres=rw/postgres,user2=rw/postgres}
(1 row)
In the broken version, 'user2' was a raw oid, obviously wrong.
Fortunately this was found during my testing and not reported as a bug
by a pg_upgrade user.
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
