Re: [HACKERS] how to use eclipse when debugging postgreSQL backend
Hello Hongchan, You need go to through following steps : 1. Debug Perspective = Run = Debug Configuration 2. On left side of menu you can see C/C++ Attach to Application 3. Right click on C/C++ Attach to Application and create new debug configuration, 4. Set Project, Build Configuration as Use Active , C/C++ Application as postgres executable path, 5. Start postmaster one instant of postgresql client (for creating one new postgres), 6. Click on Debug which will show current process list, 7. Select forked postgres process you want to debug 8. Put breakpoint in your function, this is working for me. Hopefully this helps you. Thanks Regards, Rajanikant Chirmade. 2009/9/28 노홍찬 falls...@cs.yonsei.ac.kr Hello hackers, I really appreciate Mr. Cecchet’s effort to establish the wiki page (working_with_eclipse, http://archives.postgresql.org/pgsql-hackers/2008-10/msg00312.php). I set up my PostgreSQL development environment with Eclipse, following the page’s instructions. However, I’m stuck to thesituation that I cannot debug the modified backend source of PostgreSQL since the gdb incorporated with Eclipse doesn’t support the debugging of the forked child processes. When I start debugging process by using the project default, it can only debug the postmaster process, since the postmaster process forks child processes each of which is actually postgres backend process responsible for the response to each client process (psql). What I’m trying to debug is the storage-related part of the backend source, so the gdb should be able to access the forked processes. I tried several ways like making the default option of gdb to be ‘set follow-fork-mode child’, but those tries didn’t work. For sure, there is another option, giving up using eclipse when debugging and using the console-mode gdb, but I prefer using graphical development environment and that’s the reason why I chose to use eclipse as a default development tool in the very first start. (This try might be regarded as silly though, I might be supposed to get familiar to console development environment, but if someone went through the same situation before and he or she can help me, it will be very time-saving) If someone can let me know how to set Eclipse or several steps needed to access the forked process while Elipse debugging, It will be very much helpful to me. Thank you for reading this *- Best Regards Hongchan (falls...@cs.yonsei.ac.kr, (02)2123-7757) -*
Re: [HACKERS] Hot Standby on git
Looking at the changes to StartupMultiXact, you're changing the locking so that both MultiXactOffsetControlLock and MultiXactMemberControlLock are acquire first before changing anything. Why? Looking at the other functions in that file, all others that access both files are happy to acquire one lock at a time, like StartupMultiXact does without the patch. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.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] Hot Standby on git
Looking at the changes to StartupMultiXact, you're changing the locking so that both MultiXactOffsetControlLock and MultiXactMemberControlLock are acquired first before changing anything. Why? Looking at the other functions in that file, all others that access both files are happy to acquire one lock at a time, like StartupMultiXact does without the patch. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.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] Review handling of MOVE and FETCH (ToDo)
On Mon, 2009-09-28 at 10:44 -0700, John Naylor wrote: + errmsg(statement FETCH returns more rows.), + errhint(Multirows fetch are not allowed in PL/pgSQL.))); This might sound better as statement FETCH returns multiple rows., errmsg should be without period. and Multirow FETCH is not allowed in PL/pgSQL. That might better be errdetail. -- 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] [PATCH] Reworks for Access Control facilities (r2311)
* KaiGai Kohei (kai...@ak.jp.nec.com) wrote: Stephen Frost wrote: The scenario you outline could happen without SE-PG, couldn't it? Specifically, if a user makes a connection, creates a temporary table, and then their rights to create temporary tables are revoked? What should happen in that instance though? Should they continue to have access to the tables they've created? Should they be dropped immediately? The permission to be checked here is ACL_USAGE, not ACL_CREATE_TEMP. So, the default PG model does not prevent to access his temporary tables, even if ACL_CREATE_TEMP is revoked after the creation. In this case, he cannot create new temporay tables any more due to the lack of ACL_CREATE_TEMP. But it does not prevent accesses to the temporary objects because these are already created. What I'm failing to understand is why SE-PG would be changing this then. In general, the pg_temp stuff is a bit of a 'wart', as I understand it, and is there mainly to be a place to put temporary tables. The fact that it's a schema, which we use in other cases to implement access controls, feels more like a side-effect of it being a schema than something really necessary. I'm not convinced this case is handled sufficiently.. We at least need to think about what we want to happen and document it accordingly. It is a special case when pg_namespace_aclmask() is called towards a temporary namespace. It always returns ACL_USAGE bit at least independently from its acl setting. (ACL_CREATE bit depends on the ACL_CREATE_TEMP privilege on the current database.) Please see the pg_namespace_aclmask(). Its source code comment describes it and it always makes its decision to allow to search temporary namespace. I guess it is the reason why pg_namespace_aclcheck() is not called towards the temporary namespace, and it's right for the default PG model. But SE-PG model concerns the assumption. The temporary namespace is just there to hold temporary tables which have been created. It's not intended to be a point of access control. I understand that there may be some cases where SE-PG wants to control access when the default PG model doesn't, but this feels like a case where the PG model doesn't because it's an implementation detail that's really intended to be hidden from the user. The reason why I commented as ac_object_drop() should be done is that SE-PG model also need to apply checks on cascaded objects, not only the original one, as you mentioned. And I expected that code only used in SE-PG will be suggested to remove from the first patch. I can understand that, and I think I even said that myself previously. Things have changed a bit since then though, we're trying to develop a generalized API. As I said before, I could go either way on this: Either drop the comment, or add the function call. I'm leaning towards adding the function call here since it can be done as part of the generalized API and doesn't add alot of complexity. Removing the comment is also an option, but that's not my preference. The dacSkip flag is a bit different issue. In some cases, cascaded deletion is not completely equivalent to the regular deletion. For example, the default PG model checks ownership of the relation when we create/drop a trigger object. The PE-PG model also follow the manner, so it also checks db_table:{setattr} permission. When we drop a table, it automatically drops triggers which depends on the table. In this case, if ac_object_drop() calls ac_trigger_drop() with dacSkip=true, SE-PG checks db_table:{setattr} first, then it also checks db_table:{drop} later. It is not incorrect, but redundant. But, it is not a fundamental differences between approaches. For example, we can skip SE-PG's check on triggers when dacSkip=true. In this case, the name of variable might be a bit strange. That was part of the reason I was suggesting changing the name to 'depDrop' for 'dependency Drop', that would clear up the confusion about it being dac or SE, etc. BTW, I wonder why ACL_EXECUTE is checked on creation of conversions, because it is equivalent to allow everyone to execute the conversion function. It seems to me ownership is more appropriate check. This is, again, something which should be discussed and dealt with separately from the ac_* model implementation. Ahn, it is just my opinion when I saw the code. I have no intention to change the behavior in this patch. Is the source code comment also unconfortable? I could probably go either way on this. In any case, it should be a separate discussion in a separate thread, if we really want to discuss it. My concern is just that there might now be an error message seen be the user first regarding the negator instead of a permissions error that would have been seen first before, for the same situation. I doubt this is a problem, really, but we should at least bring up any changes in behaviour like
Re: [HACKERS] [PATCH] Reworks for Access Control facilities (r2311)
KaiGai, * KaiGai Kohei (kai...@ak.jp.nec.com) wrote: The attached patch eliminates permission checks in FindConversion() and EnableDisableRule(), because these are nonsense or redundant. It is an separated issue from the ac_*() routines. For now, we decided not to touch these stuffs in the access control reworks patch. So, we can discuss about these fixes as a different topic. See the corresponding messages: http://archives.postgresql.org/message-id/26499.1250706...@sss.pgh.pa.us http://archives.postgresql.org/message-id/4abc136a.90...@ak.jp.nec.com Thanks. To make sure it gets picked up, you might respond to Tom's message above with this same email. Just a thought. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Review handling of MOVE and FETCH (ToDo)
Peter Eisentraut pete...@gmx.net writes: On Mon, 2009-09-28 at 10:44 -0700, John Naylor wrote: + errmsg(statement FETCH returns more rows.), + errhint(Multirows fetch are not allowed in PL/pgSQL.))); This might sound better as statement FETCH returns multiple rows., errmsg should be without period. and Multirow FETCH is not allowed in PL/pgSQL. That might better be errdetail. It got committed like this: ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg(FETCH statement cannot return multiple rows))); I didn't think the HINT was adding anything useful ... 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] TODO item: Allow more complex user/database default GUC settings
And here's the last necessary bit, which is pg_dump support for all this. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support *** src/bin/pg_dump/pg_dumpall.c 11 Jun 2009 14:49:07 - 1.126 --- src/bin/pg_dump/pg_dumpall.c 30 Sep 2009 14:32:47 - *** *** 43,50 static void dumpCreateDB(PGconn *conn); static void dumpDatabaseConfig(PGconn *conn, const char *dbname); static void dumpUserConfig(PGconn *conn, const char *username); static void makeAlterConfigCommand(PGconn *conn, const char *arrayitem, ! const char *type, const char *name); static void dumpDatabases(PGconn *conn); static void dumpTimestamp(char *msg); static void doShellQuoting(PQExpBuffer buf, const char *str); --- 43,52 static void dumpCreateDB(PGconn *conn); static void dumpDatabaseConfig(PGconn *conn, const char *dbname); static void dumpUserConfig(PGconn *conn, const char *username); + static void dumpDbRoleConfig(PGconn *conn); static void makeAlterConfigCommand(PGconn *conn, const char *arrayitem, ! const char *type, const char *name, const char *type2, ! const char *name2); static void dumpDatabases(PGconn *conn); static void dumpTimestamp(char *msg); static void doShellQuoting(PQExpBuffer buf, const char *str); *** *** 501,506 --- 503,515 /* Dump CREATE DATABASE commands */ if (!globals_only !roles_only !tablespaces_only) dumpCreateDB(conn); + + /* Dump role/database settings */ + if (!tablespaces_only) + { + if (server_version = 80500) + dumpDbRoleConfig(conn); + } } if (!globals_only !roles_only !tablespaces_only) *** *** 1325,1339 { PGresult *res; ! printfPQExpBuffer(buf, SELECT datconfig[%d] FROM pg_database WHERE datname = , count); appendStringLiteralConn(buf, dbname, conn); appendPQExpBuffer(buf, ;); res = executeQuery(conn, buf-data); ! if (!PQgetisnull(res, 0, 0)) { makeAlterConfigCommand(conn, PQgetvalue(res, 0, 0), ! DATABASE, dbname); PQclear(res); count++; } --- 1334,1357 { PGresult *res; ! if (server_version = 80500) ! printfPQExpBuffer(buf, SELECT setconfig[%d] FROM pg_db_role_setting WHERE ! setrole = 0 AND setdatabase = (SELECT oid FROM pg_database WHERE datname = , count); ! else ! printfPQExpBuffer(buf, SELECT datconfig[%d] FROM pg_database WHERE datname = , count); appendStringLiteralConn(buf, dbname, conn); + + if (server_version = 80500) + appendPQExpBuffer(buf, )); + appendPQExpBuffer(buf, ;); res = executeQuery(conn, buf-data); ! if (PQntuples(res) == 1 ! !PQgetisnull(res, 0, 0)) { makeAlterConfigCommand(conn, PQgetvalue(res, 0, 0), ! DATABASE, dbname, NULL, NULL); PQclear(res); count++; } *** *** 1362,1379 { PGresult *res; ! if (server_version = 80100) printfPQExpBuffer(buf, SELECT rolconfig[%d] FROM pg_authid WHERE rolname = , count); else printfPQExpBuffer(buf, SELECT useconfig[%d] FROM pg_shadow WHERE usename = , count); appendStringLiteralConn(buf, username, conn); res = executeQuery(conn, buf-data); if (PQntuples(res) == 1 !PQgetisnull(res, 0, 0)) { makeAlterConfigCommand(conn, PQgetvalue(res, 0, 0), ! ROLE, username); PQclear(res); count++; } --- 1380,1403 { PGresult *res; ! if (server_version = 80500) ! printfPQExpBuffer(buf, SELECT setconfig[%d] FROM pg_db_role_setting WHERE ! setdatabase = 0 AND setrole = ! (SELECT oid FROM pg_authid WHERE rolname = , count); ! else if (server_version = 80100) printfPQExpBuffer(buf, SELECT rolconfig[%d] FROM pg_authid WHERE rolname = , count); else printfPQExpBuffer(buf, SELECT useconfig[%d] FROM pg_shadow WHERE usename = , count); appendStringLiteralConn(buf, username, conn); + if (server_version = 80500) + appendPQExpBuffer(buf, )); res = executeQuery(conn, buf-data); if (PQntuples(res) == 1 !PQgetisnull(res, 0, 0)) { makeAlterConfigCommand(conn, PQgetvalue(res, 0, 0), ! ROLE, username, NULL, NULL); PQclear(res); count++; } *** *** 1388,1400 } /* * Helper function for dumpXXXConfig(). */ static void makeAlterConfigCommand(PGconn *conn, const char *arrayitem, ! const char *type, const char *name) { char *pos; char *mine; --- 1412,1458 } + /* + * Dump user-and-database-specific configuration + */ + static void + dumpDbRoleConfig(PGconn *conn) + { + PQExpBuffer buf = createPQExpBuffer(); + PGresult *res; + int i; + + printfPQExpBuffer(buf, SELECT rolname, datname, unnest(setconfig) + FROM pg_db_role_setting, pg_authid, pg_database + WHERE
Re: [HACKERS] Unicode UTF-8 table formatting for psql text output
On Tue, Sep 29, 2009 at 04:28:57PM -0400, Tom Lane wrote: Peter Eisentraut pete...@gmx.net writes: On Tue, 2009-09-29 at 12:01 -0400, Tom Lane wrote: The bigger question is exactly how we expect this stuff to interact with pg_regress' --no-locale switch. We already do clear all these variables when --no-locale is specified. I am wondering just what --locale is supposed to do, and whether selectively lobotomizing the LC stuff has any real use at all. We should do the LANG or LC_CTYPE thing only on the client, unconditionally. The --no-locale/--locale options should primarily determine what the temporary server uses. Well, that seems fairly reasonable, but it's going to require some refactoring of pg_regress. The initialize_environment function determines what happens in both the client and the temp server. Two possible approaches to fix the tests are as follows: diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index f2f9603..74cdaa2 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -711,8 +711,7 @@ initialize_environment(void) * is actually called.) */ unsetenv(LANGUAGE); - unsetenv(LC_ALL); - putenv(LC_MESSAGES=C); + putenv(LC_ALL=C); /* * Set multibyte as requested Here we just force the locale to C. This does have the disadvantage that --no-locale is made redundant, and any tests which are dependent upon locale (if any?) will be run in the C locale. diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index f2f9603..65fb49a 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -712,6 +712,7 @@ initialize_environment(void) */ unsetenv(LANGUAGE); unsetenv(LC_ALL); + putenv(LC_CTYPE=C); putenv(LC_MESSAGES=C); /* Here we set LC_CTYPE to C in addition to LC_MESSAGES (and for much the same reasons). However, if you test on non-C locales to check for issues with other locale codesets, those tests are all going to be forced to use ASCII. Is this a problem in practice? From the POV of my patch, it's working as designed: if the locale codeset is UTF-8 it's outputting UTF-8. But, due to the way the test machinery is looking at the output, this is breaking the tests. I'm not sure what I can do with my patch to make it behave differently that is both compatible with its intended use and not break the tests-- this is really just breaking an assumption in the testing code that the test output will always be ASCII. Forcing the LC_CTYPE to C will force ASCII output and work around this problem with the tests. Another approach would be to let psql know it's being run in a test environment with a PG_TEST or some other environment variable which we can check for and use to turn off UTF-8 output if set. Would that be better? Regards, Roger -- .''`. Roger Leigh : :' : Debian GNU/Linux http://people.debian.org/~rleigh/ `. `' Printing on GNU/Linux? http://gutenprint.sourceforge.net/ `-GPG Public Key: 0x25BFB848 Please GPG sign your mail. -- 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] Unicode UTF-8 table formatting for psql text output
Roger Leigh wrote: Here we just force the locale to C. This does have the disadvantage that --no-locale is made redundant, and any tests which are dependent upon locale (if any?) will be run in the C locale. That is not a solution. We have not that long ago gone to some lengths to provide for buildfarm testing in various locales. We're not going to undo that. 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] Unicode UTF-8 table formatting for psql text output
Andrew Dunstan and...@dunslane.net writes: Roger Leigh wrote: Here we just force the locale to C. This does have the disadvantage that --no-locale is made redundant, and any tests which are dependent upon locale (if any?) will be run in the C locale. That is not a solution. Right. I think you may have missed the point of what Peter was saying: it's okay to force the locale to C on the *client* side of the tests. The trick is to not change the environment that the temp server is started with. This will take some redesign inside pg_regress, I think. Probably initialize_environment needs to be split into two functions, one that's run before firing off the temp server and one that runs after. 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] Unicode UTF-8 table formatting for psql text output
On Tue, Sep 29, 2009 at 04:32:49PM -0400, Tom Lane wrote: Roger Leigh rle...@codelibre.net writes: C locale means POSIX behavior and nothing but. Indeed it does. However, making LC_CTYPE be UTF-8 rather than ASCII is both possible and still strictly conforming to the letter of the standard. There would be some collation and other restrictions (digit and other character classes would be contrained to the ASCII characters compared with other UTF-8 locales). However, any existing programs using ASCII would continue to function without any changes to their behaviour. The only observable change will be that nl_langinfo(CODESET) will return UTF-8, and it will be valid for programs to use UTF-8 encoded text in formatted print functions, etc.. I really, really don't believe that that meets either the letter or the spirit of the C standard, at least not if you are intending to silently substitute LC_CTYPE=UTF8 when the program has specified C/POSIX locale. (If this is just a matter of what the default LANG environment is, of course you can do anything.) We have spent some time reading the relevant standards documents (C, POSIX, SUSv2, SUSv3) and haven't found anything yet that would preclude this. While they all specify minimal requirements for what the C locale character set must provide (and POSIX/SUS are the most strict, specifying ASCII outright for each 0-127 codepoint), these are the minimal requirements for the locale, and implementation-specific extensions to ASCII are allowed, which would therefore permit UTF-8. Note that LC_CTYPE=C is not required to specify ASCII in any standard (though POSIX/SUS require that it must contain ASCII as a subset of the whole set). The language in SUSv2 in fact explicitly states that this is allowed. In fact, I've seen documentation that some UNIX systems such as HPUX already do have a UTF-8 C locale as an option. Regards, Roger -- .''`. Roger Leigh : :' : Debian GNU/Linux http://people.debian.org/~rleigh/ `. `' Printing on GNU/Linux? http://gutenprint.sourceforge.net/ `-GPG Public Key: 0x25BFB848 Please GPG sign your mail. -- 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] Unicode UTF-8 table formatting for psql text output
Andrew Dunstan wrote: Roger Leigh wrote: Here we just force the locale to C. This does have the disadvantage that --no-locale is made redundant, and any tests which are dependent upon locale (if any?) will be run in the C locale. That is not a solution. We have not that long ago gone to some lengths to provide for buildfarm testing in various locales. We're not going to undo that. Thinking about this some more, ISTM a much better way of approaching it would be to provide a flag for psql to turn off the fancy formatting, and have pg_regress use that flag. 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] Unicode UTF-8 table formatting for psql text output
Roger Leigh rle...@codelibre.net writes: The language in SUSv2 in fact explicitly states that this is allowed. In fact, I've seen documentation that some UNIX systems such as HPUX already do have a UTF-8 C locale as an option. I don't argue with the concept of a C.UTF8 locale --- in fact I think it sounds pretty useful. What I think is 100% broken is trying to make C locale work that way. C locale is supposed to be the traditional locale-ignorant, characters-are-bytes behavior. 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] Unicode UTF-8 table formatting for psql text output
Andrew Dunstan and...@dunslane.net writes: Thinking about this some more, ISTM a much better way of approaching it would be to provide a flag for psql to turn off the fancy formatting, and have pg_regress use that flag. Yeah, that's not a bad idea. There are likely to be other client programs that won't want this behavioral change either. 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] CommitFest 2009-09, two weeks on
Robert Haas wrote: - Hot Standby and Streaming Replication are both huge new features in this CommitFest, and there seems to be a fair amount of activity around both patches. Heikki previously expressed optimism about getting Hot Standby done this CommitFest, but I am not sure whether he is still feeling optimistic, There's a lot of small things that need fixing, but nothing major. I'm not so much optimistic, but I think we should spend the extra effort required on hot standby to force it in in this commitfest. It's a big feature and it really could use some alpha-testing earlier rather than later. It would also leave time for any extra features or tweaks to be made in the later commitfests. OTOH, I'd hate to hold the commitfest hostage for that. Perhaps it should be returned to author at this point, I should move on to other patches to get the commitfest closed ASAP, and continue reviewing and polishing that right after the commitfest. or what his feelings are about Streaming Replication, which is currently waiting on Fujii Masao for a new version. I'm undecided on whether walreceiver should be a subprocess of the startup process, or of postmaster as it was submitted. I'd appreciate if others would take a look into that too and give opinions. And then there's the small list of things I asked Fujii-san to work on. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.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] TODO item: Allow more complex user/database default GUC settings
Alvaro Herrera alvhe...@commandprompt.com writes: And here's the last necessary bit, which is pg_dump support for all this. + /* Dump role/database settings */ + if (!tablespaces_only) + { + if (server_version = 80500) + dumpDbRoleConfig(conn); + } Hmm ... I would kind of think that --roles-only should suppress this too. Otherwise you're going to be dumping commands that might refer to nonexistent databases. 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] CommitFest 2009-09, two weeks on
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: OTOH, I'd hate to hold the commitfest hostage for that. Perhaps it should be returned to author at this point, I should move on to other patches to get the commitfest closed ASAP, and continue reviewing and polishing that right after the commitfest. FWIW, I'd rather you kept focusing on those two patches while other committers work on the rest. From what I've seen you're finding a whole lot of broken or at least questionable stuff, and even if they're individually minor issues, that adds up to a lot of instability. I agree that these patches need special attention and should not be treated exactly the same as we'd treat smaller patches. 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] Hot Standby on git
Regarding this in InitStandbyDelayTimers: + /* +* If replication delay is enormously huge, just treat that as +* zero and work up from there. This prevents us from acting +* foolishly when replaying old log files. +*/ + if (*currentDelay_ms 0) + *currentDelay_ms = 0; + So we're treating restoring from an old backup the same as an up-to-date standby server. If you're restoring from say a month old base backup with WAL archive up to present day, and have max_standby_delay set to say 5 seconds, the server will wait for that 5 seconds on each conflicting query before killing it. Until it reaches the point in the archive where the delay is less than INT_MAX/1000 seconds old: at that point it switches into oh my goodness, we've fallen badly behind, let's try to catch up ASAP and kill any queries that get into the way mode. That's pretty surprising behavior, and not documented either. I propose we simply remove the above check (fixing the rest of the code so that you don't hit integer overflows), and always respect max_standby_delay. BTW, I wonder if should warn or something if we find that the timestamps in the archive are in the future? IOW, if either the master's or the standby's clock is not set correctly. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.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] TODO item: Allow more complex user/database default GUC settings
Tom Lane escribió: Alvaro Herrera alvhe...@commandprompt.com writes: And here's the last necessary bit, which is pg_dump support for all this. + /* Dump role/database settings */ + if (!tablespaces_only) + { + if (server_version = 80500) + dumpDbRoleConfig(conn); + } Hmm ... I would kind of think that --roles-only should suppress this too. Otherwise you're going to be dumping commands that might refer to nonexistent databases. Those double negatives are confusing as hell. I propose to add something like this: do_tablespaces = true; do_databases = true; do_roles = true; if (globals_only) do_databases = false; if (tablespaces_only) { do_roles = false; do_databases = false; } if (roles_only) { do_databases = false; do_tablespaces = false; } Then we can have the new block this way: /* Dump role/database settings */ if (do_databases do_roles) { if (server_version = 80500) dumpDbRoleConfig(conn); } -- Alvaro Herrerahttp://www.CommandPrompt.com/ 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] TODO item: Allow more complex user/database default GUC settings
Tom Lane escribió: Alvaro Herrera alvhe...@commandprompt.com writes: And here's the last necessary bit, which is pg_dump support for all this. + /* Dump role/database settings */ + if (!tablespaces_only) + { + if (server_version = 80500) + dumpDbRoleConfig(conn); + } Hmm ... I would kind of think that --roles-only should suppress this too. Otherwise you're going to be dumping commands that might refer to nonexistent databases. Hmm. The problem I have with this idea is that the only way to dump the per-database role settings is if you are also dumping the contents of all databases. Which seems like a pain to me because the usage I usually recommend is to backup global objects with pg_dumpall -g. I wonder if pg_dumpall should have a method for dumping database creation and settings, excluding contents (leaving that for plain pg_dump). -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- 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] CommitFest 2009-09, two weeks on
On Wed, Sep 30, 2009 at 11:36 AM, Tom Lane t...@sss.pgh.pa.us wrote: Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: OTOH, I'd hate to hold the commitfest hostage for that. Perhaps it should be returned to author at this point, I should move on to other patches to get the commitfest closed ASAP, and continue reviewing and polishing that right after the commitfest. FWIW, I'd rather you kept focusing on those two patches while other committers work on the rest. From what I've seen you're finding a whole lot of broken or at least questionable stuff, and even if they're individually minor issues, that adds up to a lot of instability. I agree that these patches need special attention and should not be treated exactly the same as we'd treat smaller patches. I tend to agree. I think it's reasonable for you (meaning Heikki) to devote far more time and effort to those patches than you would to other patches implementing more run-of-the-mill features, and it seems like there is no shortage of things to find and fix. I don't think that having you take a break to work on other patches is going to be to the overall benefit of the project (and many of the more significant remaining patches look like they are right up Tom's alley anyway). That having been said, if Hot Standby is still closer to commit than Streaming Replication, it might make sense to push Streaming Replication off until November, or at least post-CommitFest. Do you have any sense of how soon you'll feel confident to commit either patch? ...Robert -- 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] Unicode UTF-8 table formatting for psql text output
On Wed, Sep 30, 2009 at 11:11 AM, Tom Lane t...@sss.pgh.pa.us wrote: Andrew Dunstan and...@dunslane.net writes: Thinking about this some more, ISTM a much better way of approaching it would be to provide a flag for psql to turn off the fancy formatting, and have pg_regress use that flag. Yeah, that's not a bad idea. There are likely to be other client programs that won't want this behavioral change either. I'm surprised there isn't one already. I would think that any new format would default to off. ...Robert -- 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] TODO item: Allow more complex user/database default GUC settings
Alvaro Herrera alvhe...@commandprompt.com writes: Tom Lane escribió: Hmm ... I would kind of think that --roles-only should suppress this too. Otherwise you're going to be dumping commands that might refer to nonexistent databases. Hmm. The problem I have with this idea is that the only way to dump the per-database role settings is if you are also dumping the contents of all databases. Which seems like a pain to me because the usage I usually recommend is to backup global objects with pg_dumpall -g. Huh? --globals-only would still dump them, no? I wonder if pg_dumpall should have a method for dumping database creation and settings, excluding contents (leaving that for plain pg_dump). Perhaps. People keep speculating about refactoring the division of labor between pg_dump and pg_dumpall. I'd advise leaving that for a separate patch though ... 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] Unicode UTF-8 table formatting for psql text output
Robert Haas wrote: On Wed, Sep 30, 2009 at 11:11 AM, Tom Lane t...@sss.pgh.pa.us wrote: Andrew Dunstan and...@dunslane.net writes: Thinking about this some more, ISTM a much better way of approaching it would be to provide a flag for psql to turn off the fancy formatting, and have pg_regress use that flag. Yeah, that's not a bad idea. There are likely to be other client programs that won't want this behavioral change either. I'm surprised there isn't one already. I would think that any new format would default to off. I haven't looked, but if there is that's so much less work to do ;-) 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] pg_hba.conf: samehost and samenet [REVIEW]
Tom Lane wrote: I was just poking at this. Thanks for trying it out. It seems to need rather a lot of editorialization (eg to fix the lack of consistency about whether nonstandard headers have configure tests, or bother to make use of the tests that did get added). I've now added tests for sys/ioctl.h and net/if.h even though these headers seemed to be common to all the unixes investigated. The test for ifaddrs.h is to allow the test for getifaddrs() later in configure.in to work. This is how other open source projects have handled this situation, but if you'd like me to do it differently for postgres I can. However, it does actually compile and appear to work on HPUX 10.20, which is my personal benchmark for hopeless obsolescence ;-). Good news. So modulo the issue about how much we trust the system-reported netmasks, it seems we could adopt this. FWIW, there are checks for various bad netmasks. I incorporated these techniques after seeing them in the corresponding postfix code. BTW, there's also fallback code. If none of the methods work on a given OS, then the ifaddrs code just lists 127.0.0.1/8 and ::1/128. Cheers, Stef diff --git a/configure.in b/configure.in index e545a1f..8b42684 100644 *** a/configure.in --- b/configure.in *** AC_SUBST(OSSP_UUID_LIBS) *** 969,975 ## dnl sys/socket.h is required by AC_FUNC_ACCEPT_ARGTYPES ! AC_CHECK_HEADERS([crypt.h dld.h fp_class.h getopt.h ieeefp.h langinfo.h poll.h pwd.h sys/ipc.h sys/poll.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/socket.h sys/shm.h sys/tas.h sys/time.h sys/un.h termios.h ucred.h utime.h wchar.h wctype.h kernel/OS.h kernel/image.h SupportDefs.h]) # At least on IRIX, cpp test for netinet/tcp.h will fail unless # netinet/in.h is included first. --- 969,984 ## dnl sys/socket.h is required by AC_FUNC_ACCEPT_ARGTYPES ! AC_CHECK_HEADERS([crypt.h dld.h fp_class.h getopt.h ieeefp.h langinfo.h poll.h pwd.h sys/ipc.h sys/poll.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/socket.h sys/shm.h sys/tas.h sys/time.h sys/un.h termios.h ucred.h utime.h wchar.h wctype.h kernel/OS.h kernel/image.h SupportDefs.h ifaddrs.h sys/ioctl.h sys/sockio.h]) ! ! # On BSD, cpp test for net/if.h will fail unless sys/socket.h ! # is included first, it's checked above. ! AC_CHECK_HEADERS(net/if.h, [], [], ! [AC_INCLUDES_DEFAULT ! #ifdef HAVE_SYS_SOCKET_H ! #include sys/socket.h ! #endif ! ]) # At least on IRIX, cpp test for netinet/tcp.h will fail unless # netinet/in.h is included first. *** PGAC_VAR_INT_TIMEZONE *** 1148,1154 AC_FUNC_ACCEPT_ARGTYPES PGAC_FUNC_GETTIMEOFDAY_1ARG ! AC_CHECK_FUNCS([cbrt dlopen fcvt fdatasync getpeereid getpeerucred getrlimit memmove poll pstat readlink setproctitle setsid sigprocmask symlink sysconf towlower utime utimes waitpid wcstombs]) # posix_fadvise() is a no-op on Solaris, so don't incur function overhead # by calling it, 2009-04-02 --- 1157,1163 AC_FUNC_ACCEPT_ARGTYPES PGAC_FUNC_GETTIMEOFDAY_1ARG ! AC_CHECK_FUNCS([cbrt dlopen fcvt fdatasync getpeereid getpeerucred getrlimit memmove poll pstat readlink setproctitle setsid sigprocmask symlink sysconf towlower utime utimes waitpid wcstombs getifaddrs]) # posix_fadvise() is a no-op on Solaris, so don't incur function overhead # by calling it, 2009-04-02 diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml index ad4d084..e5152f4 100644 *** a/doc/src/sgml/client-auth.sgml --- b/doc/src/sgml/client-auth.sgml *** hostnossl replaceabledatabase/replac *** 244,249 --- 244,255 support for IPv6 addresses. /para + paraInstead of a replaceableCIDR-address/replaceable, you can specify +literalsamehost/literal to match any of the server's own IP addresses, +or literalsamenet/literal to match any address in a subnet that the +server belongs to. + /para + para This field only applies to literalhost/literal, literalhostssl/literal, and literalhostnossl/ records. diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index e6f7db2..702971a 100644 *** a/src/backend/libpq/hba.c --- b/src/backend/libpq/hba.c *** check_db(const char *dbname, const char *** 512,517 --- 512,608 return false; } + /* + * Check to see if a connecting IP matches the address and netmask. + */ + static bool + check_ip(SockAddr *raddr, struct sockaddr *addr, struct sockaddr *mask) + { + if (raddr-addr.ss_family == addr-sa_family) + { + /* Same address family */ + if (!pg_range_sockaddr(raddr-addr, (struct sockaddr_storage*)addr, + (struct sockaddr_storage*)mask)) + return false; + } + #ifdef HAVE_IPV6 + else if (addr-sa_family == AF_INET + raddr-addr.ss_family == AF_INET6) + { + /* + * Wrong address family. We allow only one case: if the file + * has IPv4 and the port is
Re: [HACKERS] CommitFest 2009-09, two weeks on
Robert Haas wrote: - There is one dblink pach left over from last CommitFest. Joe Conway was going to review it the weekend of July 18th-19th, but that didn't end up happening and so that patch is still waiting. We might be able to find someone else to review it, but I'm not sure whether that will help unless there is a committer other than Joe with bandwidth to do the final review and commit. I will get to it before the end of this commitfest, but I have to admit I'm not all that excited about this patch in the first place. I don't know that I agree with the need. Joe signature.asc Description: OpenPGP digital signature
Re: [HACKERS] CommitFest 2009-09, two weeks on
Robert Haas wrote: On Wed, Sep 30, 2009 at 11:36 AM, Tom Lane t...@sss.pgh.pa.us wrote: Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: OTOH, I'd hate to hold the commitfest hostage for that. Perhaps it should be returned to author at this point, I should move on to other patches to get the commitfest closed ASAP, and continue reviewing and polishing that right after the commitfest. FWIW, I'd rather you kept focusing on those two patches while other committers work on the rest. From what I've seen you're finding a whole lot of broken or at least questionable stuff, and even if they're individually minor issues, that adds up to a lot of instability. I agree that these patches need special attention and should not be treated exactly the same as we'd treat smaller patches. I tend to agree. I think it's reasonable for you (meaning Heikki) to devote far more time and effort to those patches than you would to other patches implementing more run-of-the-mill features, and it seems like there is no shortage of things to find and fix. I don't think that having you take a break to work on other patches is going to be to the overall benefit of the project (and many of the more significant remaining patches look like they are right up Tom's alley anyway). Ok, good, I'm more than happy to continue fine-combing hot standby. That having been said, if Hot Standby is still closer to commit than Streaming Replication, it might make sense to push Streaming Replication off until November, or at least post-CommitFest. Commitfest or no-commitfest, I'm planning to continue working on the streaming replication patch in any case until it's committed. Do you have any sense of how soon you'll feel confident to commit either patch? I'm bad at estimating. Not this week for sure, and next week I'm traveling and won't be able to spend as much time on it as I am right now. If no new major issues are found, and all the outstanding issues are resolved by me or Simon by then, maybe the week after that. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.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] CommitFest 2009-09, two weeks on
Joe Conway m...@joeconway.com writes: Robert Haas wrote: - There is one dblink pach left over from last CommitFest. Joe Conway was going to review it the weekend of July 18th-19th, but that didn't end up happening and so that patch is still waiting. We might be able to find someone else to review it, but I'm not sure whether that will help unless there is a committer other than Joe with bandwidth to do the final review and commit. I will get to it before the end of this commitfest, but I have to admit I'm not all that excited about this patch in the first place. I don't know that I agree with the need. Well, you're the dblink expert. If you think it should be rejected I doubt many of us will argue with you. 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] CommitFest 2009-09, two weeks on
Robert Haas robertmh...@gmail.com writes: ... (and many of the more significant remaining patches look like they are right up Tom's alley anyway). FWIW, if left to my own devices I will eventually get to everything except the dblink, ecpg, and encoding/win32 patches. I don't intend to touch any of those because there are other committers better qualified to review them. (I don't actually think we have anybody except Michael who's really familiar with ecpg.) However, if no other committers are working on it it's going to be a long commitfest ... The other problem is that most of the patches are not Ready for Committer anyway. 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] CommitFest 2009-09, two weeks on
On Wed, Sep 30, 2009 at 12:27 PM, Tom Lane t...@sss.pgh.pa.us wrote: Joe Conway m...@joeconway.com writes: Robert Haas wrote: - There is one dblink pach left over from last CommitFest. Joe Conway was going to review it the weekend of July 18th-19th, but that didn't end up happening and so that patch is still waiting. We might be able to find someone else to review it, but I'm not sure whether that will help unless there is a committer other than Joe with bandwidth to do the final review and commit. I will get to it before the end of this commitfest, but I have to admit I'm not all that excited about this patch in the first place. I don't know that I agree with the need. Well, you're the dblink expert. If you think it should be rejected I doubt many of us will argue with you. Yep. CommitFest doesn't mean commit it; it means decide whether to commit it. Things being rejected or returned with feedback for further improvement is fine; we're just trying to avoid long periods with no response at all. ...Robert -- 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] Linux LSB init script
Robert Haas robertmh...@gmail.com wrote: Peter Eisentraut pete...@gmx.net wrote: On Sun, 2009-09-20 at 22:54 -0400, Robert Haas wrote: It seems like there is some support for what this patch is trying to do, but much disagreement about the details of how to get there. Where do we go from here? I think the next step would be to outline what changes would be necessary in pg_ctl to implement LSB behavior. And then decide case by case whether it should become the default, an option, or is not appropriate for pg_ctl. Kevin apparently sort of agreed to do that when he came back. Right. It seems that, in addition to the above, there also remains some disagreement about: (1) how much checking the script should do to provide error messages and exit codes which target the specific problems versus generic I'm broken messages for problems which prevent it from getting to the point of being able to run pg_ctl, (2) whether the log functions required by the standard should be used, or whether we should assume that output to stdout and/or stderr (which the standard says may be silently discarded without showing anywhere) should be used instead, (3) whether we should provide comments of the general intent of sections of code when the implementing code is providing functionality required by the standard, versus assuming that the reader can match the code portions to the relevant sections of the standard without supporting comments. In general, I think most of the disputed points revolve around balancing strict compliance against keeping the script short. (The existing Linux script is about 100 lines of shell script; the LSB conforming proposal, without any pg_ctl changes which might make it shorter, is about 300 lines.) There's also disagreement about whether we should source /lib/lsb/init-functions -- which is required by the LSB standard, and provides the logging functions which the standard requires scripts to use. Given the lack of progress here, I'm going to move this one to Returned with Feedback for now. I think Kevin is busy with his non-PostgreSQL life, and there's always next CommitFest. Yeah, I'm back now, but given the research needed and the level of disagreement, completion in the CF seems unlikely. If all the stars align correctly and this gets completed in the next two weeks, we can always resurrect it. -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] CommitFest 2009-09, two weeks on
On Wed, Sep 30, 2009 at 12:34 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: ... (and many of the more significant remaining patches look like they are right up Tom's alley anyway). FWIW, if left to my own devices I will eventually get to everything except the dblink, ecpg, and encoding/win32 patches. I don't intend to touch any of those because there are other committers better qualified to review them. (I don't actually think we have anybody except Michael who's really familiar with ecpg.) Thanks, I think that's helpful information. However, if no other committers are working on it it's going to be a long commitfest ... That is my concern as well. The other problem is that most of the patches are not Ready for Committer anyway. I (and hopefully the people who agreed to help with patch-chasing) can work on this, but given that there are 5 that are Ready for Committer and probably as many more that are close, and further given that in the past 7 days exactly 1 patch from the CommitFest has been committed, I'm not sure there's a real problem here. If you commit/bounce all 5 of those afternoon I will spend the evening making sure you have a few more to tackle tomorrow. ...Robert -- 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] TODO item: Allow more complex user/database default GUC settings
On Wed, Sep 30, 2009 at 10:34 AM, Alvaro Herrera alvhe...@commandprompt.com wrote: And here's the last necessary bit, which is pg_dump support for all this. I think it would be helpful if you could post ONE patch with all the changes and all the new files in the diff. AIUI, the patch is now split across three separate emails. :-( ...Robert -- 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] CommitFest 2009-09, two weeks on
Robert Haas wrote: On Wed, Sep 30, 2009 at 12:34 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: ... (and many of the more significant remaining patches look like they are right up Tom's alley anyway). FWIW, if left to my own devices I will eventually get to everything except the dblink, ecpg, and encoding/win32 patches. I don't intend to touch any of those because there are other committers better qualified to review them. (I don't actually think we have anybody except Michael who's really familiar with ecpg.) Thanks, I think that's helpful information. However, if no other committers are working on it it's going to be a long commitfest ... That is my concern as well. I have been (and still am somewhat) slammed, but I can probably make space to work on Encoding issues in console and eventlog on win32 https://commitfest.postgresql.org/action/patch_view?id=148 some time in the next day or three. After that, if I still have time and nobody else has grabbed it, I'll move on to CREATE LIKE INCLUDING COMMENTS and STORAGE https://commitfest.postgresql.org/action/patch_view?id=172. 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] Unicode UTF-8 table formatting for psql text output
On Wed, 2009-09-30 at 11:03 -0400, Andrew Dunstan wrote: Thinking about this some more, ISTM a much better way of approaching it would be to provide a flag for psql to turn off the fancy formatting, and have pg_regress use that flag. Well, it might not be a bad idea, but adding a feature just to satisfy the test suite instead of fixing the test suite doesn't feel satisfying. Is there another use case? -- 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] Unicode UTF-8 table formatting for psql text output
On Wed, 2009-09-30 at 12:06 -0400, Robert Haas wrote: On Wed, Sep 30, 2009 at 11:11 AM, Tom Lane t...@sss.pgh.pa.us wrote: Andrew Dunstan and...@dunslane.net writes: Thinking about this some more, ISTM a much better way of approaching it would be to provide a flag for psql to turn off the fancy formatting, and have pg_regress use that flag. Yeah, that's not a bad idea. There are likely to be other client programs that won't want this behavioral change either. I'm surprised there isn't one already. I would think that any new format would default to off. Well, this isn't a new format, just an enhancement of an existing format. -- 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] Unicode UTF-8 table formatting for psql text output
Peter Eisentraut wrote: On Wed, 2009-09-30 at 11:03 -0400, Andrew Dunstan wrote: Thinking about this some more, ISTM a much better way of approaching it would be to provide a flag for psql to turn off the fancy formatting, and have pg_regress use that flag. Well, it might not be a bad idea, but adding a feature just to satisfy the test suite instead of fixing the test suite doesn't feel satisfying. Is there another use case? Sure, as Tom noted pg_regress probably won't be the only user. There are lots of legacy scripts out there that parse psql output, and it should be of use to them. 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] Linux LSB init script
On Wed, 2009-09-30 at 11:58 -0500, Kevin Grittner wrote: Right. It seems that, in addition to the above, there also remains some disagreement about: (1) how much checking the script should do to provide error messages and exit codes which target the specific problems versus generic I'm broken messages for problems which prevent it from getting to the point of being able to run pg_ctl, (2) whether the log functions required by the standard should be used, or whether we should assume that output to stdout and/or stderr (which the standard says may be silently discarded without showing anywhere) should be used instead, (3) whether we should provide comments of the general intent of sections of code when the implementing code is providing functionality required by the standard, versus assuming that the reader can match the code portions to the relevant sections of the standard without supporting comments. I'm not so worried about these points. They can always be adjusted later. The point about how to involve pg_ctl is more critical. -- 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] CommitFest 2009-09, two weeks on
Robert Haas wrote: On Wed, Sep 30, 2009 at 12:27 PM, Tom Lane t...@sss.pgh.pa.us wrote: Joe Conway m...@joeconway.com writes: Robert Haas wrote: - There is one dblink pach left over from last CommitFest. Joe Conway was going to review it the weekend of July 18th-19th, but that didn't end up happening and so that patch is still waiting. We might be able to find someone else to review it, but I'm not sure whether that will help unless there is a committer other than Joe with bandwidth to do the final review and commit. I will get to it before the end of this commitfest, but I have to admit I'm not all that excited about this patch in the first place. I don't know that I agree with the need. Well, you're the dblink expert. If you think it should be rejected I doubt many of us will argue with you. Yep. CommitFest doesn't mean commit it; it means decide whether to commit it. Things being rejected or returned with feedback for further improvement is fine; we're just trying to avoid long periods with no response at all. The issue is not so much technical as it is philosophical. The patch basically forces all use of libpq by dblink to be asynchronous (internally) so that a cancel can be sensed and passed down to the remote side and everything cleaned up. Possibly the right thing to do, but dblink already allows the use of async queries, and the current synchronous method uses standard libpq calls. If all of this is really necessary, doesn't every libpq client have the same issue? If so why have the synchronous libpq functions at all? So while I can vet the patch technically, and spend more time understanding the use case, and maybe explaining it better, I think other people should weigh in on the change as it is significant and points to other potential issues. Joe signature.asc Description: OpenPGP digital signature
Re: [HACKERS] pg_hba.conf: samehost and samenet [REVIEW]
At 2009-09-30 11:16:57 -0500, stef-l...@memberwebs.com wrote: I've now added tests for sys/ioctl.h and net/if.h even though these headers seemed to be common to all the unixes investigated. Thanks. I've marked this ready for committer now. FWIW, there are checks for various bad netmasks. I incorporated these techniques after seeing them in the corresponding postfix code. [...] BTW, there's also fallback code. If none of the methods work on a given OS, then the ifaddrs code just lists 127.0.0.1/8 and ::1/128. Both of these look good. + /* + * Wrong address family. We allow only one case: if the file + * has IPv4 and the port is IPv6, promote the file address to + * IPv6 and try to match that way. + */ I still think this comment should be fixed in _both_ places it now occurs, but oh well. (Note: I have not tested under Windows, and I've not tested test_ifaddrs.c. Also, there are a few lines which introduce trailing whitespace.) -- ams -- 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] Unicode UTF-8 table formatting for psql text output
Andrew Dunstan escribió: Peter Eisentraut wrote: On Wed, 2009-09-30 at 11:03 -0400, Andrew Dunstan wrote: Thinking about this some more, ISTM a much better way of approaching it would be to provide a flag for psql to turn off the fancy formatting, and have pg_regress use that flag. Well, it might not be a bad idea, but adding a feature just to satisfy the test suite instead of fixing the test suite doesn't feel satisfying. Is there another use case? Sure, as Tom noted pg_regress probably won't be the only user. There are lots of legacy scripts out there that parse psql output, and it should be of use to them. All scripts I've seen parsing psql output use unaligned, undecorated mode. I have yet to see one messing with the |'s. -- Alvaro Herrerahttp://www.CommandPrompt.com/ 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] CommitFest 2009-09, two weeks on
On Wed, Sep 30, 2009 at 18:34, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: ... (and many of the more significant remaining patches look like they are right up Tom's alley anyway). FWIW, if left to my own devices I will eventually get to everything except the dblink, ecpg, and encoding/win32 patches. I don't intend to touch any of those because there are other committers better qualified to review them. (I don't actually think we have anybody except Michael who's really familiar with ecpg.) I can certainly review the win32 encoding patch, but I was rather hoping for some comments from others on if we're interested in a win32 only solution, or if we want something more generic. Should we just go with the win32-only one for now? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] CommitFest 2009-09, two weeks on
Magnus Hagander escribió: On Wed, Sep 30, 2009 at 18:34, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: ... (and many of the more significant remaining patches look like they are right up Tom's alley anyway). FWIW, if left to my own devices I will eventually get to everything except the dblink, ecpg, and encoding/win32 patches. I don't intend to touch any of those because there are other committers better qualified to review them. (I don't actually think we have anybody except Michael who's really familiar with ecpg.) I can certainly review the win32 encoding patch, but I was rather hoping for some comments from others on if we're interested in a win32 only solution, or if we want something more generic. Should we just go with the win32-only one for now? Just a couple of days ago a question came on the spanish list because someone was getting mixed UTF8 and Latin1 output in a log file. This was in Fedora IIRC, so maybe we do want something more general. -- Alvaro Herrerahttp://www.CommandPrompt.com/ 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] Unicode UTF-8 table formatting for psql text output
On Wed, Sep 30, 2009 at 1:27 PM, Peter Eisentraut pete...@gmx.net wrote: On Wed, 2009-09-30 at 12:06 -0400, Robert Haas wrote: On Wed, Sep 30, 2009 at 11:11 AM, Tom Lane t...@sss.pgh.pa.us wrote: Andrew Dunstan and...@dunslane.net writes: Thinking about this some more, ISTM a much better way of approaching it would be to provide a flag for psql to turn off the fancy formatting, and have pg_regress use that flag. Yeah, that's not a bad idea. There are likely to be other client programs that won't want this behavioral change either. I'm surprised there isn't one already. I would think that any new format would default to off. Well, this isn't a new format, just an enhancement of an existing format. I disagree. This patch is about replacing ASCII art constructed using | and + and replacing it with box-drawing characters. I think the chances that this will not look good in one or more of the terminal emulators that I use from time to time are excellent, especially if the approach is at all similar to what pstree does on recent versions of Fedora. I'm 100% not interested in tracking down some obscure encoding/locale issue to make it show up right, and I'm 110% not interested in helping the people who file bug reports to track down the analagous issues in their crazy, broken environments. The way it works right now (and has worked for the last 5 years or more) is reliable, familiar, and, at least IME, bullet-proof. I don't even see a good case for changing the default, let alone not providing a way to retreat to the old behavior. Considering that this patch is nothing more than an exceedingly minor piece of window-dressing, taking even a small risk of breaking things for anyone who as of today can log into psql and get work done without pain seems like a bad trade-off to me. ...Robert -- 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] Unicode UTF-8 table formatting for psql text output
On Wed, Sep 30, 2009 at 01:30:17PM -0400, Andrew Dunstan wrote: Peter Eisentraut wrote: On Wed, 2009-09-30 at 11:03 -0400, Andrew Dunstan wrote: Thinking about this some more, ISTM a much better way of approaching it would be to provide a flag for psql to turn off the fancy formatting, and have pg_regress use that flag. Well, it might not be a bad idea, but adding a feature just to satisfy the test suite instead of fixing the test suite doesn't feel satisfying. Is there another use case? Sure, as Tom noted pg_regress probably won't be the only user. There are lots of legacy scripts out there that parse psql output, and it should be of use to them. The attached patch implements this feature. It adds a --no-pretty-formatting/-G option to psql (naming isn't my forté, so feel free to change it!). This is also documented in the SGML docs, and help text. Lastly, this option is used when invoking psql by pg_regress, which results in a working testsuite in a UTF-8 locale. Hopefully this is OK! I'll be happy to make any amendments required. Regards, Roger -- .''`. Roger Leigh : :' : Debian GNU/Linux http://people.debian.org/~rleigh/ `. `' Printing on GNU/Linux? http://gutenprint.sourceforge.net/ `-GPG Public Key: 0x25BFB848 Please GPG sign your mail. diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index 85e9375..95f1365 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -192,6 +192,19 @@ PostgreSQL documentation /varlistentry varlistentry + termoption-G/option/term + termoption--no-pretty-formatting/option/term + para + Only use ASCII characters to draw tables. Without this option, + box drawing characters may be used if avilable which will result + in nicer output. However, if compatibility with the output of + older psql versions, or psql used in other locales is required, it + may be desirable to disable pretty table formatting in these + situations. + /para +/varlistentry + +varlistentry termoption-h replaceable class=parameterhostname/replaceable//term termoption--host replaceable class=parameterhostname/replaceable//term listitem diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c index d498d58..d596ece 100644 --- a/src/bin/psql/help.c +++ b/src/bin/psql/help.c @@ -121,6 +121,8 @@ usage(void) set field separator (default: \%s\)\n), DEFAULT_FIELD_SEP); printf(_( -H, --html HTML table output mode\n)); + printf(_( -G, --no-pretty-formatting\n + only use ASCII characters to format tables\n)); printf(_( -P, --pset=VAR[=ARG] set printing option VAR to ARG (see \\pset command)\n)); printf(_( -R, --record-separator=STRING\n set record separator (default: newline)\n)); diff --git a/src/bin/psql/print.c b/src/bin/psql/print.c index 41d508e..e01e512 100644 --- a/src/bin/psql/print.c +++ b/src/bin/psql/print.c @@ -2290,9 +2290,10 @@ printTable(const printTableContent *cont, FILE *fout, FILE *flog) const printTextFormat *text_format = asciiformat; #if (defined(HAVE_LANGINFO_H) defined(CODESET)) - if (pg_strcasecmp(nl_langinfo(CODESET), UTF-8) == 0 || - pg_strcasecmp(nl_langinfo(CODESET), utf8) == 0 || - pg_strcasecmp(nl_langinfo(CODESET), CP65001) == 0) + if ((pg_strcasecmp(nl_langinfo(CODESET), UTF-8) == 0 || + pg_strcasecmp(nl_langinfo(CODESET), utf8) == 0 || + pg_strcasecmp(nl_langinfo(CODESET), CP65001) == 0) + cont-opt-use_text_format) text_format = utf8format; #endif diff --git a/src/bin/psql/print.h b/src/bin/psql/print.h index ea1dc97..97772a6 100644 --- a/src/bin/psql/print.h +++ b/src/bin/psql/print.h @@ -27,6 +27,7 @@ enum printFormat typedef struct printTableOpt { enum printFormat format; /* one of the above */ + bool use_text_format; /* use text formats for pretty printing */ bool expanded; /* expanded/vertical output (if supported by * output format) */ unsigned short int border; /* Print a border around the table. 0=none, diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c index 429e5d9..3fdf904 100644 --- a/src/bin/psql/startup.c +++ b/src/bin/psql/startup.c @@ -125,6 +125,7 @@ main(int argc, char *argv[]) /* We rely on unmentioned fields of pset.popt to start out 0/false/NULL */ pset.popt.topt.format = PRINT_ALIGNED; + pset.popt.topt.use_text_format = 1; pset.popt.topt.border = 1; pset.popt.topt.pager = 1; pset.popt.topt.start_table = true; @@ -317,6 +318,7 @@ parse_psql_options(int argc, char *argv[], struct adhoc_opts * options) {list, no_argument, NULL, 'l'}, {log-file, required_argument, NULL, 'L'}, {no-readline, no_argument, NULL, 'n'}, + {no-pretty-formatting, no_argument, NULL, 'G'}, {single-transaction, no_argument, NULL, '1'}, {output, required_argument,
Re: [HACKERS] Triggers on columns
On Mon, 2009-09-14 at 18:58 +0900, Itagaki Takahiro wrote: Itagaki Takahiro itagaki.takah...@oss.ntt.co.jp wrote: Ok, the attached patch implements standard-compliant version of column trigger. Here is an updated version of column-level trigger patch. I forgot to adjust pg_get_triggerdef() in the previous version. pg_dump also uses pg_get_triggerdef() instead of building CREATE TRIGGER statements to avoid duplicated codes if the server version is 8.5 or later. What is the purpose of the new pg_get_triggerdef() variant? OK, the parameter name pretty_bool gives a hint, but what does this have to do with column triggers? Maybe you could try to explain this in more detail. Ideally split the patch into two: one that deals with pg_get_triggerdef(), and one that deals with column triggers. If you want a pretty option on pg_get_triggerdef(), you could nowadays also implement that via a parameter default value instead of a second function. -- 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] Unicode UTF-8 table formatting for psql text output
On Wed, 2009-09-30 at 14:02 -0400, Alvaro Herrera wrote: All scripts I've seen parsing psql output use unaligned, undecorated mode. I have yet to see one messing with the |'s. Plus, we would have broken that with the : continuation lines. -- 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] [PATCH] DefaultACLs
Hi all, because it seems like merging privileges seems to be acceptable for most (although I am not sure I like it, but I don't have better solution for managing conflicts), I changed the patch to do just that. Also I think I addressed all other problems pointed out by Tom. Namely I added pg_dump support (hopefully) and \ddp command for psql. Still no tab competition though. I removed that GRANT DEFAULT PRIVILEGES statement, changed user facing elog()s to ereport()s, and I changed catalog name to singular. -- Regards Petr Jelinek (PJMODOS) defacl-2009-09-30.diff.gz Description: Unix tar archive -- 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] CommitFest 2009-09, two weeks on
Magnus Hagander mag...@hagander.net writes: I can certainly review the win32 encoding patch, but I was rather hoping for some comments from others on if we're interested in a win32 only solution, or if we want something more generic. Should we just go with the win32-only one for now? That was actually the only substantive comment I had about it. I don't see why it's a win32-only problem or why a win32-only solution is a good approach. 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] Unicode UTF-8 table formatting for psql text output
Robert Haas robertmh...@gmail.com writes: The way it works right now (and has worked for the last 5 years or more) is reliable, familiar, and, at least IME, bullet-proof. I don't even see a good case for changing the default, let alone not providing a way to retreat to the old behavior. This is pretty much my opinion as well. I'd vote for requiring a switch to turn it on. Those who really want the new behavior by default can set it up in their .psqlrc. 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] CommitFest 2009-09, two weeks on
On Wed, Sep 30, 2009 at 21:38, Tom Lane t...@sss.pgh.pa.us wrote: Magnus Hagander mag...@hagander.net writes: I can certainly review the win32 encoding patch, but I was rather hoping for some comments from others on if we're interested in a win32 only solution, or if we want something more generic. Should we just go with the win32-only one for now? That was actually the only substantive comment I had about it. I don't see why it's a win32-only problem or why a win32-only solution is a good approach. Yeah, that's my thought as well. If we want a complete one, we should reject this patch and ask for one that does that. If we are fine with a win32 only one, I can review this one and get it in. I'm leaning towards us wanting a general one, but I'm unsure how much work that will take. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] latest hstore patch
Andrew Gierth and...@tao11.riddles.org.uk writes: Hstore patch incorporating changes as previously discussed. In addition the requested new features of conversions to and from array formats have been added (with docs). Applied with some mostly-cosmetic editorialization. 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] TODO item: Allow more complex user/database default GUC settings
Tom Lane escribió: BTW, have we thought much about the simplest possible solution, which is to not have the view? How badly do we need it? Seems like dropping the functionality into a psql \d command might be a viable alternative. FWIW I came up with a preliminary patch for a new psql command \dus that shows settings. It takes a pattern that's used to constrain on roles. Thus there is no way to view settings for a database. If there's a need for that we could use another command, say \dls. Sample output alvherre=# \dus fo* List of settings role | database | settings --+--+--- fob | | log_duration=true foo | alvherre | work_mem=256MB : statement_timeout=10s foo | | work_mem=512MB : statement_timeout=1s (3 rows) -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support *** src/bin/psql/command.c 13 Sep 2009 22:18:22 - 1.207 --- src/bin/psql/command.c 30 Sep 2009 19:28:07 - *** *** 409,415 success = listTables(cmd[1], pattern, show_verbose, show_system); break; case 'u': ! success = describeRoles(pattern, show_verbose); break; case 'F': /* text search subsystem */ switch (cmd[2]) --- 409,418 success = listTables(cmd[1], pattern, show_verbose, show_system); break; case 'u': ! if (cmd[2] cmd[2] == 's') ! success = listRoleSettings(pattern); ! else ! success = describeRoles(pattern, show_verbose); break; case 'F': /* text search subsystem */ switch (cmd[2]) *** src/bin/psql/describe.c 29 Jul 2009 20:56:19 - 1.226 --- src/bin/psql/describe.c 30 Sep 2009 19:54:42 - *** *** 2176,2181 --- 2176,2232 appendPQExpBufferStr(buf, str); } + /* + * \dus + */ + bool + listRoleSettings(const char *pattern) + { + PQExpBufferData buf; + PGresult *res; + printQueryOpt myopt = pset.popt; + + initPQExpBuffer(buf); + + if (pset.sversion = 80500) + { + printfPQExpBuffer(buf, SELECT rolname AS role, datname AS database,\n + pg_catalog.array_to_string(setconfig, E'\\n') AS settings\n + FROM pg_db_role_setting AS s\n + LEFT JOIN pg_database ON pg_database.oid = setdatabase\n + LEFT JOIN pg_roles ON pg_roles.oid = setrole ); + processSQLNamePattern(pset.db, buf, pattern, false, false, + NULL, pg_roles.rolname, NULL, NULL); + appendPQExpBufferStr(buf, ORDER BY role, database); + } + else + return false; + + res = PSQLexec(buf.data, false); + if (!res) + return false; + + if (PQntuples(res) == 0 !pset.quiet) + { + if (pattern) + fprintf(pset.queryFout, _(No matching roles found.\n)); + else + fprintf(pset.queryFout, _(No settings found.\n)); + } + else + { + myopt.nullPrint = NULL; + myopt.title = _(List of settings); + myopt.translate_header = true; + + printQuery(res, myopt, pset.queryFout, pset.logfile); + } + + PQclear(res); + resetPQExpBuffer(buf); + return true; + } + /* * listTables() *** src/bin/psql/describe.h 21 Apr 2009 15:49:06 - 1.40 --- src/bin/psql/describe.h 30 Sep 2009 19:29:20 - *** *** 27,32 --- 27,35 /* \du, \dg */ extern bool describeRoles(const char *pattern, bool verbose); + /* \dus */ + extern bool listRoleSettings(const char *pattern); + /* \z (or \dp) */ extern bool permissionsList(const char *pattern); -- 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] TODO item: Allow more complex user/database default GUC settings
Alvaro Herrera alvhe...@commandprompt.com writes: FWIW I came up with a preliminary patch for a new psql command \dus that shows settings. It takes a pattern that's used to constrain on roles. Thus there is no way to view settings for a database. If there's a need for that we could use another command, say \dls. Why not two pattern arguments? \drds [ role-pattern [ db-pattern ]] Omitted patterns are presumed to be * 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] latest hstore patch
On Sep 30, 2009, at 12:52 PM, Tom Lane wrote: Applied with some mostly-cosmetic editorialization. And there was much rejoicing… David -- 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] CommitFest 2009-09, two weeks on
Joe Conway m...@joeconway.com writes: The issue is not so much technical as it is philosophical. The patch basically forces all use of libpq by dblink to be asynchronous (internally) so that a cancel can be sensed and passed down to the remote side and everything cleaned up. Possibly the right thing to do, but dblink already allows the use of async queries, and the current synchronous method uses standard libpq calls. If all of this is really necessary, doesn't every libpq client have the same issue? Well, only the ones that want to implement cancel and don't have access to the app's own signal handling functions. (Which suggests that a possible answer is to allow dblink to hook into the SIGINT catcher, but frankly hooks in that location scare me ...) I would argue that it's not necessarily a good idea at all: one of the typical uses for dblink is to fake autonomous transactions, and in that application I don't think you *want* a cancel to propagate to the other session. If we did put this behavior into all dblink operations, we'd need a way to turn it off. Since dblink_cancel_query is already available, people who do want cancels to propagate have the ability to do that. I'm inclined to think that this is complexity we don't need. 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] TODO item: Allow more complex user/database default GUC settings
Tom Lane escribió: Alvaro Herrera alvhe...@commandprompt.com writes: FWIW I came up with a preliminary patch for a new psql command \dus that shows settings. It takes a pattern that's used to constrain on roles. Thus there is no way to view settings for a database. If there's a need for that we could use another command, say \dls. Why not two pattern arguments? \drds [ role-pattern [ db-pattern ]] Hmm, interesting idea, patch attached. This required changing the API of processSQLNamePattern to return a bool indicating whether a clause was added; otherwise, when processing the second pattern it was impossible to figure out if we needed a WHERE or not. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. *** src/bin/pg_dump/dumputils.c 4 Aug 2009 21:56:08 - 1.48 --- src/bin/pg_dump/dumputils.c 30 Sep 2009 20:58:27 - *** *** 894,900 * * Scan a wildcard-pattern string and generate appropriate WHERE clauses * to limit the set of objects returned. The WHERE clauses are appended ! * to the already-partially-constructed query in buf. * * conn: connection query will be sent to (consulted for escaping rules). * buf: output parameter. --- 894,901 * * Scan a wildcard-pattern string and generate appropriate WHERE clauses * to limit the set of objects returned. The WHERE clauses are appended ! * to the already-partially-constructed query in buf. Returns whether ! * any clause was added. * * conn: connection query will be sent to (consulted for escaping rules). * buf: output parameter. *** *** 913,919 * Formatting note: the text already present in buf should end with a newline. * The appended text, if any, will end with one too. */ ! void processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern, bool have_where, bool force_escape, const char *schemavar, const char *namevar, --- 914,920 * Formatting note: the text already present in buf should end with a newline. * The appended text, if any, will end with one too. */ ! bool processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern, bool have_where, bool force_escape, const char *schemavar, const char *namevar, *** *** 925,933 bool inquotes; const char *cp; int i; #define WHEREAND() \ ! (appendPQExpBufferStr(buf, have_where ? AND : WHERE ), have_where = true) if (pattern == NULL) { --- 926,936 bool inquotes; const char *cp; int i; + bool added_clause = false; #define WHEREAND() \ ! (appendPQExpBufferStr(buf, have_where ? AND : WHERE ), \ ! have_where = true, added_clause = true) if (pattern == NULL) { *** *** 937,943 WHEREAND(); appendPQExpBuffer(buf, %s\n, visibilityrule); } ! return; } initPQExpBuffer(schemabuf); --- 940,946 WHEREAND(); appendPQExpBuffer(buf, %s\n, visibilityrule); } ! return added_clause; } initPQExpBuffer(schemabuf); *** *** 1094,1098 --- 1097,1102 termPQExpBuffer(schemabuf); termPQExpBuffer(namebuf); + return added_clause; #undef WHEREAND } *** src/bin/pg_dump/dumputils.h 4 Aug 2009 21:56:08 - 1.25 --- src/bin/pg_dump/dumputils.h 30 Sep 2009 20:57:42 - *** *** 36,42 const char *type, const char *acls, const char *owner, int remoteVersion, PQExpBuffer sql); ! extern void processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern, bool have_where, bool force_escape, const char *schemavar, const char *namevar, --- 36,42 const char *type, const char *acls, const char *owner, int remoteVersion, PQExpBuffer sql); ! extern bool processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern, bool have_where, bool force_escape, const char *schemavar, const char *namevar, *** src/bin/psql/command.c 13 Sep 2009 22:18:22 - 1.207 --- src/bin/psql/command.c 30 Sep 2009 20:47:53 - *** *** 408,413 --- 408,426 case 's': success = listTables(cmd[1], pattern, show_verbose, show_system); break; + case 'r': + if (cmd[2] == 'd' cmd[3] == 's') + { + char *pattern2 = NULL; + + if (pattern) + pattern2 = psql_scan_slash_option(scan_state, + OT_NORMAL, NULL, true); + success = listDbRoleSettings(pattern, pattern2); + } + else + success = PSQL_CMD_UNKNOWN; + break; case 'u': success = describeRoles(pattern, show_verbose); break; *** src/bin/psql/describe.c 29 Jul 2009 20:56:19 - 1.226 --- src/bin/psql/describe.c 30 Sep 2009 20:59:13 - *** *** 2176,2181 --- 2176,2240 appendPQExpBufferStr(buf, str);
Re: [HACKERS] latest hstore patch
David E. Wheeler da...@kineticode.com writes: On Sep 30, 2009, at 12:52 PM, Tom Lane wrote: Applied with some mostly-cosmetic editorialization. And there was much rejoicing ... except in the buildfarm. Must be some platform dependency we both missed ... 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] Unicode UTF-8 table formatting for psql text output
Peter Eisentraut pete...@gmx.net writes: On Wed, 2009-09-30 at 14:02 -0400, Alvaro Herrera wrote: All scripts I've seen parsing psql output use unaligned, undecorated mode. I have yet to see one messing with the |'s. Plus, we would have broken that with the : continuation lines. Only for data containing embedded newlines, which is not that common. 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] FSM search modes
decibel deci...@decibel.org wrote: *any* step that improves dealing with table bloat is extremely welcome, as right now you're basically stuck rebuilding the table. +1 Although, possibly more irritating than actually rebuilding it is evaluating borderline bloat situations to determine if they will work themselves out over time or whether you need to bite the bullet to do aggressive maintenance. Having some way for routine vacuums (or any other routine process, currently available or that could be scheduled) to nibble away at moderate bloat without significant performance or usability impact would make life easier for at least *some* DBAs. -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] Unicode UTF-8 table formatting for psql text output
Roger Leigh rle...@codelibre.net writes: On Wed, 2009-09-30 at 11:03 -0400, Andrew Dunstan wrote: Thinking about this some more, ISTM a much better way of approaching it would be to provide a flag for psql to turn off the fancy formatting, and have pg_regress use that flag. The attached patch implements this feature. It adds a --no-pretty-formatting/-G option to psql (naming isn't my forté, so feel free to change it!). This is also documented in the SGML docs, and help text. Lastly, this option is used when invoking psql by pg_regress, which results in a working testsuite in a UTF-8 locale. It would be a good idea to tie this to a psql magic variable (like ON_ERROR_STOP) so that it could conveniently be set in ~/.psqlrc. I'm not actually sure that we need a dedicated command line switch for it, since you could use -v varname instead. 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] latest hstore patch
Tom == Tom Lane t...@sss.pgh.pa.us writes: And there was much rejoicing Tom ... except in the buildfarm. Must be some platform dependency Tom we both missed ... oops -- Andrew (irc:RhodiumToad) -- 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] numeric_to_number() function skipping some digits
daveg da...@sonic.net wrote: On Tue, Sep 22, 2009 at 10:27:19AM +0530, Jeevan Chalke wrote: It seems that Oracle reads formatting string from right-to-left. Here are few results: ('number','format') == Oracle PG ('34,50','999,99') == 3450340 ('34,50','99,99') == 34503450 ('34,50','99,999') == Invalid Number 3450 ('34,50','999,999') == Invalid Number 340 It seems worse to to give a wrong answer silently then to throw an error. What we do now seems sort of MySqlish. I agree with David on that. Further, it seems to me that functions which are there only for Oracle compatibility (i.e., they are not part of the SQL standard and are not particularly sensible, but are popular in Oracle) should behave like the corresponding Oracle function. An argument could even be made for bug compatibility for such functions, at least to some extent; although we're not talking about that here -- the Oracle results seem more sane than current PostgreSQL behavior.. -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] numeric_to_number() function skipping some digits
Kevin Grittner kevin.gritt...@wicourts.gov writes: daveg da...@sonic.net wrote: On Tue, Sep 22, 2009 at 10:27:19AM +0530, Jeevan Chalke wrote: It seems that Oracle reads formatting string from right-to-left. It seems worse to to give a wrong answer silently then to throw an error. What we do now seems sort of MySqlish. I agree with David on that. So who is volunteering to do the presumably rather major rewrite involved? 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] numeric_to_number() function skipping some digits
Tom Lane t...@sss.pgh.pa.us wrote: Kevin Grittner kevin.gritt...@wicourts.gov writes: daveg da...@sonic.net wrote: On Tue, Sep 22, 2009 at 10:27:19AM +0530, Jeevan Chalke wrote: It seems that Oracle reads formatting string from right-to-left. It seems worse to to give a wrong answer silently then to throw an error. What we do now seems sort of MySqlish. I agree with David on that. So who is volunteering to do the presumably rather major rewrite involved? We don't typically identify an author before putting something on the TODO list. That said, this one seems like it would be within my skill set. I seem to have trouble finding tasks that are for which we can get consensus, so I just might pick this one up if we achieve such consensus on it. The biggest problem, should I take it on, would be that I don't currently have access to an Oracle database, so someone would have to supply me with accurate specs for how it should behave, or point to unambiguous documentation. -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] numeric_to_number() function skipping some digits
On Wed, 2009-09-30 at 19:08 -0500, Kevin Grittner wrote: Tom Lane t...@sss.pgh.pa.us wrote: Kevin Grittner kevin.gritt...@wicourts.gov writes: daveg da...@sonic.net wrote: On Tue, Sep 22, 2009 at 10:27:19AM +0530, Jeevan Chalke wrote: It seems that Oracle reads formatting string from right-to-left. It seems worse to to give a wrong answer silently then to throw an error. What we do now seems sort of MySqlish. I agree with David on that. So who is volunteering to do the presumably rather major rewrite involved? We don't typically identify an author before putting something on the TODO list. That said, this one seems like it would be within my skill set. I seem to have trouble finding tasks that are for which we can get consensus, so I just might pick this one up if we achieve such consensus on it. The biggest problem, should I take it on, would be that I don't currently have access to an Oracle database, so someone would have to supply me with accurate specs for how it should behave, or point to unambiguous documentation. Just download developer edition? -Kevin -- PostgreSQL.org Major Contributor Command Prompt, Inc: http://www.commandprompt.com/ - 503.667.4564 Consulting, Training, Support, Custom Development, Engineering If the world pushes look it in the eye and GRR. Then push back harder. - Salamander -- 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] numeric_to_number() function skipping some digits
Joshua D. Drake j...@commandprompt.com wrote: On Wed, 2009-09-30 at 19:08 -0500, Kevin Grittner wrote: I don't currently have access to an Oracle database Just download developer edition? [quick google search] Looks like that would do it. Thanks. -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] Triggers on columns
Peter Eisentraut pete...@gmx.net wrote: What is the purpose of the new pg_get_triggerdef() variant? OK, the parameter name pretty_bool gives a hint, but what does this have to do with column triggers? Maybe you could try to explain this in more detail. Ideally split the patch into two: one that deals with pg_get_triggerdef(), and one that deals with column triggers. It's for pg_dump. We can avoid duplicated codes if we use pg_get_triggerdef() in pg_dump. So, I think column trigger and the dump function for column trigger should be applied at once. If you want a pretty option on pg_get_triggerdef(), you could nowadays also implement that via a parameter default value instead of a second function. OK, I'll rewrite it to use default parameter. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center -- 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] CREATE LIKE INCLUDING COMMENTS and STORAGES
Alvaro Herrera alvhe...@commandprompt.com wrote: Hmm, so it works to specify LIKE t1 INCLUDING COMMENTS EXCLUDING COMMENTS? Only last specifer is applied, which is the same behavior as of now. EXCLUDING is typically useless because all of the default values are EXCLUDING, but INCLUDING ALL EXCLUDING xxx are meaningful; it copies all components except xxx. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center -- 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] [PATCH] Reworks for Access Control facilities (r2311)
Stephen Frost wrote: * KaiGai Kohei (kai...@ak.jp.nec.com) wrote: Stephen Frost wrote: The scenario you outline could happen without SE-PG, couldn't it? Specifically, if a user makes a connection, creates a temporary table, and then their rights to create temporary tables are revoked? What should happen in that instance though? Should they continue to have access to the tables they've created? Should they be dropped immediately? The permission to be checked here is ACL_USAGE, not ACL_CREATE_TEMP. So, the default PG model does not prevent to access his temporary tables, even if ACL_CREATE_TEMP is revoked after the creation. In this case, he cannot create new temporay tables any more due to the lack of ACL_CREATE_TEMP. But it does not prevent accesses to the temporary objects because these are already created. What I'm failing to understand is why SE-PG would be changing this then. In general, the pg_temp stuff is a bit of a 'wart', as I understand it, and is there mainly to be a place to put temporary tables. The fact that it's a schema, which we use in other cases to implement access controls, feels more like a side-effect of it being a schema than something really necessary. I'm not convinced this case is handled sufficiently.. We at least need to think about what we want to happen and document it accordingly. It is a special case when pg_namespace_aclmask() is called towards a temporary namespace. It always returns ACL_USAGE bit at least independently from its acl setting. (ACL_CREATE bit depends on the ACL_CREATE_TEMP privilege on the current database.) Please see the pg_namespace_aclmask(). Its source code comment describes it and it always makes its decision to allow to search temporary namespace. I guess it is the reason why pg_namespace_aclcheck() is not called towards the temporary namespace, and it's right for the default PG model. But SE-PG model concerns the assumption. The temporary namespace is just there to hold temporary tables which have been created. It's not intended to be a point of access control. I understand that there may be some cases where SE-PG wants to control access when the default PG model doesn't, but this feels like a case where the PG model doesn't because it's an implementation detail that's really intended to be hidden from the user. It is a good point. PostgreSQL implements temporary database object feature using a special schema (pg_temp_*), but it is an implementation details indeed. As you mentioned, it is a schema in the fact. It may be harmful for simplicity of the security model to use exception cases. But it is a perspective from developers, not users. For example, how many people pay mention that network layer is implemented as a pseudo filesystem in Linux? You are saying such a thing, correct? At least, I don't think it is an issue corresponding to security risk. The reason why SE-PG want to check permission to search a certain schema including temporary one is a simplicity of access control model. Yes, it is reasonable both of MAC/DAC to handle temporary schema as an exception of access controls on schemas. BTW, I wonder why ACL_EXECUTE is checked on creation of conversions, because it is equivalent to allow everyone to execute the conversion function. It seems to me ownership is more appropriate check. This is, again, something which should be discussed and dealt with separately from the ac_* model implementation. Ahn, it is just my opinion when I saw the code. I have no intention to change the behavior in this patch. Is the source code comment also unconfortable? I could probably go either way on this. In any case, it should be a separate discussion in a separate thread, if we really want to discuss it. OK, My concern is just that there might now be an error message seen be the user first regarding the negator instead of a permissions error that would have been seen first before, for the same situation. I doubt this is a problem, really, but we should at least bring up any changes in behaviour like this and get agreement that they're acceptable, even if it's just the particulars of error-messages (since they could possibly contain information that shouldn't be available...). I don't think it is a problem. The default PG model (also SE-PG model) does not support to hide existence of a certain database objects, even if client does not have access permissions. For example, a client can SELECT from the pg_class which include relations stored within a certain namespace without ACL_USAGE. The default PG model prevent to resolve the relation name in the schema, but it is different from to hide its existent. I know it doesn't hide existence of major database objects. Depending on the situation, there might be other information that could be leaked. I realize that's not the case here, but I still want to catch and document any behavioral changes, even if it's clear they
Re: [HACKERS] Triggers on columns
Peter Eisentraut pete...@gmx.net wrote: If you want a pretty option on pg_get_triggerdef(), you could nowadays also implement that via a parameter default value instead of a second function. OK, I'll rewrite it to use default parameter. I tried to remove pg_get_triggerdef_ext() and add a second argument with default value to pg_get_triggerdef(), but there is a problem. The definition of pg_get_triggerdef will be the following: DATA(insert OID = 1662 ( pg_get_triggerdefPGNSP PGUID 12 1 0 0 f f f t f s 2 1 25 26 16 _null_ _null_ _null_ ({CONST :consttype 16 :consttypmod -1 :constlen 1 :constbyval true :constisnull false :location 41 :constvalue 1 [ 0 0 0 0 0 0 0 0 ]}) pg_get_triggerdef _null_ _null_ _null_ )); The problem is in :constvalue part; It will be :constvalue 1 [ 0 0 0 0 0 0 0 0 ] on 64bit platform, but :constvalue 1 [ 0 0 0 0 ] on 32bit platform. I think we should not use default values in functions listed on pg_proc.h, so the previous patch is better than default value version. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center -- 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] CommitFest 2009-09, two weeks on
Joe Conway m...@joeconway.com wrote: The patch basically forces all use of libpq by dblink to be asynchronous (internally) so that a cancel can be sensed and passed down to the remote side and everything cleaned up. Possibly the right thing to do, but dblink already allows the use of async queries, and the current synchronous method uses standard libpq calls. The point is *memory leak* in dblink when a query is canceled or become time-out. I think it is a bug, and my patch could fix it. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center -- 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] pg_hba.conf: samehost and samenet [REVIEW]
Stef Walter stef-l...@memberwebs.com writes: [ postgres-hba-samenet-8.patch ] Applied with some mostly-cosmetic editorialization. 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
[HACKERS] Use samehost by default in pg_hba.conf?
Now that the samehost/samenet patch is in, I wonder if it wouldn't be a good idea to replace this part of the default pg_hba.conf file: # IPv4 local connections: hostall all 127.0.0.1/32 @authmethod@ # IPv6 local connections: hostall all ::1/128 @authmethod@ with: # local connections via TCP/IP: hostall all samehost @authmethod@ The advantage of this is that connections made with -h machine_name instead of -h localhost would work without customization. I can't see any disadvantage to it. Making the change now would also give us an opportunity to test the samehost/samenet implementation in the buildfarm, at least for machines without Unix sockets. (Note that you would still need a non-default setting of listen_addresses for -h machine_name to actually work.) Comments? 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] CommitFest 2009-09, two weeks on
Magnus Hagander mag...@hagander.net wrote: I can certainly review the win32 encoding patch, but I was rather hoping for some comments from others on if we're interested in a win32 only solution, or if we want something more generic. Should we just go with the win32-only one for now? Yes, because Windows is only platform that supports UTF-16 encoding natively. I believe my patch is the best solution for Windows even if we have another approach for other platforms. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center -- 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] [PATCH] Reworks for Access Control facilities (r2311)
KaiGai, * KaiGai Kohei (kai...@ak.jp.nec.com) wrote: Yes, it is reasonable both of MAC/DAC to handle temporary schema as an exception of access controls on schemas. Great. I know it doesn't hide existence of major database objects. Depending on the situation, there might be other information that could be leaked. I realize that's not the case here, but I still want to catch and document any behavioral changes, even if it's clear they shouldn't be an issue. I agree that it should be documented. Where should I document them on? I guess the purpose of the description is to inform these behavior changes for users, not only developers. The official documentation sgml? wiki.postgresql.org? or, source code comments are enough? What I would suggest is having a README or similar which accompanies the patch. This could then be included by reference in the commit message, or directly in the commit depending on what the committer prefers. Or, it could just go into the mailing list and commitfest archives. The point is to make sure the committer understands and isn't suprised when reviewing the changes and comes across places where the code changes result in a behaviour change. If the changes are significant enough (and I don't think they will be, to be honest..), they should be included by the committer in the commit message and then picked up by Bruce, et al, when the release notes are developed. I don't believe it needs to be in the formal PG documentation, unless there's something documented there today which is changing (very unlikely..). The shdepDropOwned() launched using DROP OWNED BY statement is a case that we have no DAC for each database objects but MAC should be applied on them. We can consider it as a variety of cascaded deletion, so ac_object_drop() should be also put here. Right, that makes sense to me. I agree that what the caller provides shouldn't depend on the security model. I wasn't suggesting that it would. The name-OID resolution I was referring to was for things like getting the namespace OID, not getting the OID for the new conversion being created. Does that clear up the confusion here? Sorry, confusable description. What I would like to say is something like: Create() { namespaceId = LookupCreationNamespace(); ac_xxx_create_namespace(); -- only one use it, but other doesn't use? : tablespaceId = get_tablespace_oid(); ac_xxx_create_tablespace(); -- only DAC use it? : ac_xxx_create(); -- only MAC use it? : values[ ... ] = ObjectIdGetDatum(namespaceId); values[ ... ] = ObjectIdGetDatum(tablespaceId); simple_heap_insert(); : } When we create a new object X which needs OID of namespace and tablespace, these names have to be resolved prior to updating system catalogs. If we put ac_*() routines for each name resolution, it implicitly assumes a certain security model and the ac_*() routine getting nonsense. No, no. What I was suggesting and what I think we already do in most places (but not everywhere and it's not really a policy) is this: Create() { namespaceId = LookupCreationNamespace(); tablespaceId = get_tablespace_oid(); ac_xxx_create(); : values[ ... ] = ObjectIdGetDatum(namespaceId); values[ ... ] = ObjectIdGetDatum(tablespaceId); simple_heap_insert(); : } Which I think is what you're doing with this, it just might be a change from what was done before when there were multiple permission checks done. At least, the current implementation deploys ac_*() routines as soon as the caller have enough information to provide it. Good. We should document where that changed behaviour though, but also document that this is a policy that we're going to try and stick with in the future (running ac_*() as soon as there is enough information to). This policy focuses on developers, so it is enough to be source code comments? Source code comments would be good for this. The only other place I could think of it going would be on the developer part of the wiki. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Streaming Replication patch for CommitFest 2009-09
On Thu, Sep 17, 2009 at 5:08 PM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: Walreceiver is really a slave to the startup process. The startup process decides when it's launched, and it's the startup process that then waits for it to advance. But the way it's set up at the moment, the startup process needs to ask the postmaster to start it up, and it doesn't look very robust to me. For example, if launching walreceiver fails for some reason, startup process will just hang waiting for it. I changed the postmaster to report the failure of fork of the walreceiver to the startup process by resetting WalRcv-in_progress, which prevents the startup process from getting stuck when launching walreceiver fails. http://archives.postgresql.org/pgsql-hackers/2009-09/msg01996.php Do you have another concern about the robustness? If yes, I'll address that. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- 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] Use samehost by default in pg_hba.conf?
* Tom Lane (t...@sss.pgh.pa.us) wrote: Now that the samehost/samenet patch is in, I wonder if it wouldn't be a good idea to replace this part of the default pg_hba.conf file: # IPv4 local connections: hostall all 127.0.0.1/32 @authmethod@ # IPv6 local connections: hostall all ::1/128 @authmethod@ with: # local connections via TCP/IP: hostall all samehost @authmethod@ The advantage of this is that connections made with -h machine_name instead of -h localhost would work without customization. I can't see any disadvantage to it. Making the change now would also give us an opportunity to test the samehost/samenet implementation in the buildfarm, at least for machines without Unix sockets. I'm not sure if it out-ranks the advantages of the change for buildfarm support, but the above change isn't actually without any disadvantage. Specifically, not every auth mechanism that works with -h machine_name works with -h localhost, but the first record in pg_hba which is matched is used. I could have: hostall all 127.0.0.1/32 @authmethod@ hostall all A.B.C.D/32@authmethod2@ today and a change to: hostall all samehost @authmethod@ hostall all A.B.C.D/32@authmethod2@ could override my authmethod2 and cause connections to fail, since it isn't intended to be used. Additionally, a user could be confused if they're familiar with 127.0.0.1/32 and not figure out why a change to samehost is causing problems. (Note that you would still need a non-default setting of listen_addresses for -h machine_name to actually work.) In any case, this is about the default pg_hba.conf and what I'm talking about is KRB5/GSSAPI related (127.0.0.1 may not work if it resolves to 'localhost' because KRB5/GSSAPI auth is based off getting the hostname of the machine being connected to from the reverse DNS of the IP being connected to). As such, it's entirely possible that it's completely irrelevant, but I wanted to bring up that 127.0.0.1-samehost could cause issues for some folks in some configurations. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Use samehost by default in pg_hba.conf?
On Wed, Sep 30, 2009 at 10:08 PM, Tom Lane t...@sss.pgh.pa.us wrote: The advantage of this is that connections made with -h machine_name instead of -h localhost would work without customization. I can't see any disadvantage to it. Making the change now would also give us an opportunity to test the samehost/samenet implementation in the buildfarm, at least for machines without Unix sockets. (Note that you would still need a non-default setting of listen_addresses for -h machine_name to actually work.) Comments? I don't see much advantage in this proposal, at least not immediately. If it turns out to be a wildly popular feature and doesn't turn out to introduce security vulnerabilities or breakage, we can always make this change later. ...Robert -- 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] Buffer usage in EXPLAIN and pg_stat_statements (review)
Euler Taveira de Oliveira eu...@timbira.com wrote: But there are some confusions in postgres; ReadBufferCount and BufferHitCount are used for get and hit, but heap_blks_read and heap_blks_hit are used for read and hit in pg_statio_all_tables. I see. :( I fixed the confusions of get, hit and read in your patch. longnum_hit = ReadBufferCount + ReadLocalBufferCount; longnum_read = num_hit - BufferHitCount - LocalBufferHitCount; should be longnum_get = ReadBufferCount + ReadLocalBufferCount; longnum_hit = BufferHitCount + LocalBufferHitCount; longnum_read = num_get - num_hit; ReadBufferCount means number of buffer access :( Patch attached. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center buffer_usage_20091001.patch Description: Binary data -- 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] Use samehost by default in pg_hba.conf?
On 09/30/2009 10:08 PM, Tom Lane wrote: Now that the samehost/samenet patch is in, I wonder if it wouldn't be a good idea to replace this part of the default pg_hba.conf file: # IPv4 local connections: hostall all 127.0.0.1/32 @authmethod@ # IPv6 local connections: hostall all ::1/128 @authmethod@ with: # local connections via TCP/IP: hostall all samehost @authmethod@ The advantage of this is that connections made with -h machine_name instead of -h localhost would work without customization. I can't see any disadvantage to it. Making the change now would also give us an opportunity to test the samehost/samenet implementation in the buildfarm, at least for machines without Unix sockets. (Note that you would still need a non-default setting of listen_addresses for -h machine_name to actually work.) Although there is probably no rush for it - I think this would be a great first user experience change for PostgreSQL 8.5. If it just works out of the box, this is good. In the past, my experience has been that PostgreSQL rarely works out of the box for common scenarios. I know some people are worried about it not working or creating some theoretical security problem that ends up being route caused to PostgreSQL - but I find this thinking inconsistent when I look at the default configuration of trust. I would like to see the default of trust abolished. It scares me far more than sameuser / samehost would ever scare me. Newbie users won't know to fix it, and experienced users always need to fix it. I think the default file should be something that would be most valid to most people. For example: local all all ident hostall all samehost md5 If this was the default, I think many installations would not require customization, and this would be great. Then again - maybe this will open up a huge can of worms where we debate about which configuration is more likely for the average new user :-) Anything is better than trust - even blocking access entirely! Cheers, mark -- Mark Mielkem...@mielke.cc -- 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] [PATCH] Reworks for Access Control facilities (r2311)
Stephen Frost wrote: I know it doesn't hide existence of major database objects. Depending on the situation, there might be other information that could be leaked. I realize that's not the case here, but I still want to catch and document any behavioral changes, even if it's clear they shouldn't be an issue. I agree that it should be documented. Where should I document them on? I guess the purpose of the description is to inform these behavior changes for users, not only developers. The official documentation sgml? wiki.postgresql.org? or, source code comments are enough? What I would suggest is having a README or similar which accompanies the patch. This could then be included by reference in the commit message, or directly in the commit depending on what the committer prefers. Or, it could just go into the mailing list and commitfest archives. The point is to make sure the committer understands and isn't suprised when reviewing the changes and comes across places where the code changes result in a behaviour change. If the changes are significant enough (and I don't think they will be, to be honest..), they should be included by the committer in the commit message and then picked up by Bruce, et al, when the release notes are developed. I don't believe it needs to be in the formal PG documentation, unless there's something documented there today which is changing (very unlikely..). It may be good idea to put src/backend/security/README. I'm not clear whether the commit message or release note is appropriate. (it is unnoticeable if commit message, it is too details for release note.) No, no. What I was suggesting and what I think we already do in most places (but not everywhere and it's not really a policy) is this: Create() { namespaceId = LookupCreationNamespace(); tablespaceId = get_tablespace_oid(); ac_xxx_create(); : values[ ... ] = ObjectIdGetDatum(namespaceId); values[ ... ] = ObjectIdGetDatum(tablespaceId); simple_heap_insert(); : } Which I think is what you're doing with this, it just might be a change from what was done before when there were multiple permission checks done. Ahh, it was a communication bug. we talked same thing. Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei kai...@ak.jp.nec.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] [PATCH] Reworks for Access Control facilities (r2311)
Stephen Frost wrote: KaiGai, * KaiGai Kohei (kai...@ak.jp.nec.com) wrote: The attached patch eliminates permission checks in FindConversion() and EnableDisableRule(), because these are nonsense or redundant. It is an separated issue from the ac_*() routines. For now, we decided not to touch these stuffs in the access control reworks patch. So, we can discuss about these fixes as a different topic. See the corresponding messages: http://archives.postgresql.org/message-id/26499.1250706...@sss.pgh.pa.us http://archives.postgresql.org/message-id/4abc136a.90...@ak.jp.nec.com Thanks. To make sure it gets picked up, you might respond to Tom's message above with this same email. Just a thought. The following message was my reply. http://archives.postgresql.org/pgsql-hackers/2009-08/msg01420.php Now I'm removing something with behavior change such as above patch, and eliminating comments to be discussed in other thread. I would like to quote them not to forget them away. * ACL_CREATE checks on renaming type When we rename an existing type, it checks ownership of the target type, but ACL_CREATE on the namespace in which the type is stored, although it is checked on renaming any other database objects stored within a certain namespace, such as tables, functions and so on. Is it an intended behavior? * Ownership checks on REASSIGN OWNED BY statement When we use the REASSIGN OWNER BY statement, it tries to change ownership of the database objects which are owned by the target role. It internally calls routine to change the ownership such as AlterFunctionOwner_oid(). The routine depends on the class of objects, and they have a code to check privilege to change ownership when it is actually changed as follows: But the code path corresponding to relations and types does not have such kind of checks. IMO, similar check should be deployed. -- at the AlterFunctionOwner_internal() if (procForm-proowner != newOwnerId) { Datum repl_val[Natts_pg_proc]; boolrepl_null[Natts_pg_proc]; boolrepl_repl[Natts_pg_proc]; Acl*newAcl; Datum aclDatum; boolisNull; HeapTuple newtuple; /* Superusers can always do it */ if (!superuser()) { /* Otherwise, must be owner of the existing object */ if (!pg_proc_ownercheck(procOid, GetUserId())) aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_PROC, NameStr(procForm-proname)); /* Must be able to become new owner */ check_is_member_of_role(GetUserId(), newOwnerId); /* New owner must have CREATE privilege on namespace */ aclresult = pg_namespace_aclcheck(procForm-pronamespace, newOwnerId, ACL_CREATE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, ACL_KIND_NAMESPACE, get_namespace_name(procForm-pronamespace)); } : -- OSS Platform Development Division, NEC KaiGai Kohei kai...@ak.jp.nec.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] Use samehost by default in pg_hba.conf?
Stephen Frost sfr...@snowman.net writes: I'm not sure if it out-ranks the advantages of the change for buildfarm support, but the above change isn't actually without any disadvantage. Specifically, not every auth mechanism that works with -h machine_name works with -h localhost, but the first record in pg_hba which is matched is used. I could have: hostall all 127.0.0.1/32 @authmethod@ hostall all A.B.C.D/32@authmethod2@ If you've got any such thing, you've got a *nondefault* pg_hba.conf file. Or are you opining that people who are smart enough to set that up are too stupid to replace a single samehost entry with the two entries they need? 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] [PATCH] Reworks for Access Control facilities (r2311)
* KaiGai Kohei (kai...@ak.jp.nec.com) wrote: Stephen Frost wrote: Thanks. To make sure it gets picked up, you might respond to Tom's message above with this same email. Just a thought. The following message was my reply. http://archives.postgresql.org/pgsql-hackers/2009-08/msg01420.php Right, but now there's actually a patch to go with it.. Just thinking that Tom might pick up on it more easily if the patch was sent to that thread, that's all. Of course, he seems to know all anyway, so it's entirely likely that I'm just being silly. Now I'm removing something with behavior change such as above patch, and eliminating comments to be discussed in other thread. I would like to quote them not to forget them away. That's fine. You'll start new threads with different subject lines for them, right? That way other people will see the specific issues and comment if they want to. I expect few people are actually following this very long thread at this level. :) THanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Use samehost by default in pg_hba.conf?
Robert Haas robertmh...@gmail.com writes: I don't see much advantage in this proposal, at least not immediately. If it turns out to be a wildly popular feature and doesn't turn out to introduce security vulnerabilities or breakage, we can always make this change later. The advantage is to get some testing so that we can *find out* if the code has got problems... 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] Use samehost by default in pg_hba.conf?
* Tom Lane (t...@sss.pgh.pa.us) wrote: Stephen Frost sfr...@snowman.net writes: I'm not sure if it out-ranks the advantages of the change for buildfarm support, but the above change isn't actually without any disadvantage. Specifically, not every auth mechanism that works with -h machine_name works with -h localhost, but the first record in pg_hba which is matched is used. I could have: hostall all 127.0.0.1/32 @authmethod@ hostall all A.B.C.D/32@authmethod2@ If you've got any such thing, you've got a *nondefault* pg_hba.conf file. Or are you opining that people who are smart enough to set that up are too stupid to replace a single samehost entry with the two entries they need? I was mainly just trying to address that 127.0.0.1-samehost is not a change without possible downfalls, in general. If a packager imposed or recommended such a change it could break things for end users. We do use 'host all all 127.0.0.1/32 md5' in the default Debian configuration. If that was later changed to 'samehost' and then the diff applied to some configurations (something Debian wouldn't do without asking, but it might ask if you wanted to use the maintainer's version of the file, and I know that I've done that in the past and then added back my local changes, especially if I can do so easily by just adding lines to the end of the file..), things could break. I have no idea how/if this would apply anywhere else. In general, I think the user could figure out, but running these kind of issues down can be annoying when it's not necessairly clear what's happening. My comment was primairly for Martin's benefit and could probably be resolved by just adding some commentary to the default config saying that this might override other pg_hba lines below which used to apply to connections over the local system's network IP. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Use samehost by default in pg_hba.conf?
Mark Mielke m...@mark.mielke.cc writes: I would like to see the default of trust abolished. We've been around on that point before and I don't see any reason to think that the outcome of the argument would be different now. The only reason I brought all this up is that samehost does change the terms of debate for the question of what host lines we need to provide. 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] Unicode UTF-8 table formatting for psql text output
On Wed, 2009-09-30 at 18:50 -0400, Tom Lane wrote: It would be a good idea to tie this to a psql magic variable (like ON_ERROR_STOP) so that it could conveniently be set in ~/.psqlrc. I'm not actually sure that we need a dedicated command line switch for it, since you could use -v varname instead. Right. A better yet place would be inside \pset, since that contains the other formatting options. -- 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] Use samehost by default in pg_hba.conf?
On Wed, 2009-09-30 at 22:08 -0400, Tom Lane wrote: # local connections via TCP/IP: hostall all samehost @authmethod@ The advantage of this is that connections made with -h machine_name instead of -h localhost would work without customization. I can't see any disadvantage to it. Making the change now would also give us an opportunity to test the samehost/samenet implementation in the buildfarm, at least for machines without Unix sockets. (Note that you would still need a non-default setting of listen_addresses for -h machine_name to actually work.) Which makes this proposal kind of uninteresting. Plus, with @authmethod@ being mostly trust, how much faith do we have in samehost never giving any false positives? Sounds uncomfortable to me. -- 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] Use samehost by default in pg_hba.conf?
Peter Eisentraut pete...@gmx.net writes: (Note that you would still need a non-default setting of listen_addresses for -h machine_name to actually work.) Which makes this proposal kind of uninteresting. Well, it's one less thing that has to be fixed for local connections to work smoothly. Plus, with @authmethod@ being mostly trust, how much faith do we have in samehost never giving any false positives? Having looked at the code, I think that samehost is pretty safe. I'm still worried about samenet picking up a bogusly broad netmask --- but samehost hard-wires the netmask at all-ones. Even if your network configuration is really screwed up, the kernel isn't going to send that traffic off-machine. So I think it will act as advertised. 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] Use samehost by default in pg_hba.conf?
Peter Eisentraut pete...@gmx.net writes: On Wed, 2009-09-30 at 22:08 -0400, Tom Lane wrote: (Note that you would still need a non-default setting of listen_addresses for -h machine_name to actually work.) Which makes this proposal kind of uninteresting. Although come to think of it ... is there any reason besides sheer conservatism to not make the default listen_addresses value '*'? It won't result in letting in any outside connections unless you also add pg_hba.conf entries. 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] Use samehost by default in pg_hba.conf?
On 1 okt 2009, at 06.53, Tom Lane t...@sss.pgh.pa.us wrote: Peter Eisentraut pete...@gmx.net writes: On Wed, 2009-09-30 at 22:08 -0400, Tom Lane wrote: (Note that you would still need a non-default setting of listen_addresses for -h machine_name to actually work.) Which makes this proposal kind of uninteresting. Although come to think of it ... is there any reason besides sheer conservatism to not make the default listen_addresses value '*'? It won't result in letting in any outside connections unless you also add pg_hba.conf entries. Absolutely. One less opportunity to DOS the server - it's certainly cheaper to deal with connection floods by never even answering the socket. Also, showing up in portscans for example. Now, that trust authentication is a different issue ;) /Magnus -- 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] Use samehost by default in pg_hba.conf?
On Wed, Sep 30, 2009 at 11:53 PM, Tom Lane t...@sss.pgh.pa.us wrote: Although come to think of it ... is there any reason besides sheer conservatism to not make the default listen_addresses value '*'? just my 2 cents. but i always wondered about the existence of listen_addresses at all... to me the only reason it exists is to force me to change 'localhost' to '*' after installing, something i always do almost automaticaly =) -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers