Re: [HACKERS] inherit support for foreign tables
Hello, At Fri, 21 Feb 2014 16:33:32 +0900, Etsuro Fujita wrote > (2014/02/21 15:23), Kyotaro HORIGUCHI wrote: > NOTICE: Child foregn table child01 is affected. > NOTICE: Child foregn table child02 is affected > NOTICE: Child foregn table child03 rejected 'alter tempmin set > default' > > It says that child03 had no ability to perform the requested > > action, in this case setting a default value. It might be better > > to reject ALTER on the parent as a whole when any children > > doesn't accept any action. > > Now understood, thougn I'm not sure it's worth implementing such a > checking mechanism in the recursive altering operation... Did you mean foreign tables can sometimes silently ignore ALTER actions which it can't perform? It will cause inconsistency which operators didn't anticipate. This example uses "add column" for perspicuitly but all types of action could result like this. == =# ALTER TABLE parent ADD COLUMN x integer; ALTER TABLE =# \d parent Table "public.parent" Column | Type | Modifiers +-+ a | integer | b | integer | x | integer | Number of child tables: 2 (Use \d+ to list them.) =# \d child1 Foreign table "public.child1" Column | Type | Modifiers | FDW Options --+-+---+- a | integer | b | integer | =# (Op: Ouch!) == I think this should result as, == =# ALTER TABLE parent ADD COLUMN x integer; ERROR: Foreign child table child1 could not perform some of the actions. =# == regards, -- Kyotaro Horiguchi 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] walsender doesn't send keepalives when writes are pending
On 2014-02-21 10:08:44 +0530, Amit Kapila wrote: > On Fri, Feb 14, 2014 at 5:35 PM, Andres Freund wrote: > > Hi, > > > > In WalSndLoop() we do: > > > > wakeEvents = WL_LATCH_SET | WL_POSTMASTER_DEATH | WL_TIMEOUT | > > WL_SOCKET_READABLE; > > > > if (pq_is_send_pending()) > > wakeEvents |= WL_SOCKET_WRITEABLE; > > else if (wal_sender_timeout > 0 && !ping_sent) > > { > > ... > > if (GetCurrentTimestamp() >= timeout) > > WalSndKeepalive(true); > > ... > > > > I think those two if's should simply be separate. There's no reason not > > to ask for a ping when we're writing. On a busy server that might be the > > case most of the time. > > I think the main reason of ping is to detect n/w break sooner. > On a busy server, wouldn't WALSender can detect it when next time it > will try to send the remaining data? Well, especially on a pipelined connection, that can take a fair bit. TCP timeouts aren't fun. There's a reason we have the keepalives, and that they measure application to application performance. And detecting systems as down is important for e.g. synchronous rep. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Uninterruptable regexp_replace in 9.3.1 ?
The following snippet reveals that 9.3.1 has a bug in regexp_matches, which uninterruptably keeps CPU spinning for minutes: -8<--- \timing SET statement_timeout = 2; -- this is only to show statement_timeout is effective here SELECT count(*) from generate_series(1, 10); -- this one is uninterruptable! SELECT regexp_matches($INPUT$ /a $b$ $c$d ; $INPUT$, $REG$((?:[^'"$;]+|"[^"]*"|'(?:[^']*|'')*'|(\$[^$]*\$).*?\2)+)$REG$, 'g' ); -8<--- The above has been tested to be harmless with PostgreSQL 9.1.11 in that the regexp_matches call is interrupted, but it is NOT with PostgreSQL 9.3.1. Is it a known bug ? Please include my address in replies as I don't get notified of list activity. Thanks. --strk; () ASCII ribbon campaign -- Keep it simple ! /\ http://strk.keybit.net/rants/ascii_mails.txt -- 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] Uninterruptable regexp_replace in 9.3.1 ?
I've just tested 9.3.3 and it is _also_ affected. Should I report the regression somewhere else ? --strk; On Fri, Feb 21, 2014 at 10:17:59AM +0100, Sandro Santilli wrote: > The following snippet reveals that 9.3.1 has a bug > in regexp_matches, which uninterruptably keeps CPU > spinning for minutes: > > -8<--- > > \timing > SET statement_timeout = 2; > -- this is only to show statement_timeout is effective here > SELECT count(*) from generate_series(1, 10); > -- this one is uninterruptable! > SELECT regexp_matches($INPUT$ > > > > > > > > > /a > $b$ > $c$d > ; > $INPUT$, > $REG$((?:[^'"$;]+|"[^"]*"|'(?:[^']*|'')*'|(\$[^$]*\$).*?\2)+)$REG$, 'g' ); > > -8<--- > > The above has been tested to be harmless with PostgreSQL 9.1.11 > in that the regexp_matches call is interrupted, but it is NOT > with PostgreSQL 9.3.1. > > Is it a known bug ? > > Please include my address in replies as I don't get notified > of list activity. Thanks. > > --strk; > > () ASCII ribbon campaign -- Keep it simple ! > /\ http://strk.keybit.net/rants/ascii_mails.txt -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Proposal: IMPORT FOREIGN SCHEMA statement.
Hello, The SQL-MED specification defines the IMPORT FOREIGN SCHEMA statement. This adds discoverability to foreign servers. The structure of the statement as I understand it is simple enough: IMPORT FOREIGN SCHEMA remote_schema FROM SERVER some_server [ (LIMIT TO | EXCEPT) table_list ] INTO local_schema. Is anyone working on this? I found a reference to this from 2010 in the archive, stating that work should be focused on core functionality, but nothing more recent. This would be very useful for postgres_fdw and other RDBMS-backed fdws, but I think even file_fdw could benefit from it if it was able to create a foreign table for every csv-with-header file in a directory. I can see a simple API working for that. A new function would be added to the fdw routine, which is responsible for crafting CreateForeignTableStmt. It could have the following signature: typedef List *(*ImportForeignSchema_function) (ForeignServer *server, ImportForeignSchemaStmt * parsetree); I experimented with this idea, and came up with the attached two patches: one for the core, and the other for actually implementing the API in postgres_fdw. Maybe those can serve as a proof-of-concept for discussing the design? -- Ronan Dunklau http://dalibo.com - http://dalibo.orgdiff --git a/src/backend/commands/event_trigger.c b/src/backend/commands/event_trigger.c index 024a477..40a2540 100644 --- a/src/backend/commands/event_trigger.c +++ b/src/backend/commands/event_trigger.c @@ -250,7 +250,8 @@ check_ddl_tag(const char *tag) pg_strcasecmp(tag, "REFRESH MATERIALIZED VIEW") == 0 || pg_strcasecmp(tag, "ALTER DEFAULT PRIVILEGES") == 0 || pg_strcasecmp(tag, "ALTER LARGE OBJECT") == 0 || - pg_strcasecmp(tag, "DROP OWNED") == 0) + pg_strcasecmp(tag, "DROP OWNED") == 0 || + pg_strcasecmp(tag, "IMPORT FOREIGN SCHEMA") == 0) return EVENT_TRIGGER_COMMAND_TAG_OK; /* diff --git a/src/backend/commands/foreigncmds.c b/src/backend/commands/foreigncmds.c index 7f007d7..719c674 100644 --- a/src/backend/commands/foreigncmds.c +++ b/src/backend/commands/foreigncmds.c @@ -27,7 +27,9 @@ #include "catalog/pg_type.h" #include "catalog/pg_user_mapping.h" #include "commands/defrem.h" +#include "commands/tablecmds.h" #include "foreign/foreign.h" +#include "foreign/fdwapi.h" #include "miscadmin.h" #include "parser/parse_func.h" #include "utils/acl.h" @@ -1427,3 +1429,48 @@ CreateForeignTable(CreateForeignTableStmt *stmt, Oid relid) heap_close(ftrel, RowExclusiveLock); } + +/* + * Import a foreign schema + */ +void +ImportForeignSchema(ImportForeignSchemaStmt * stmt) +{ + Oid ownerId; + ForeignDataWrapper *fdw; + ForeignServer *server; + FdwRoutine *fdw_routine; + AclResult aclresult; + List *table_list = NULL; + ListCell *lc; + char * local_schema = strdup(stmt->local_schema); + + ownerId = GetUserId(); + server = GetForeignServerByName(stmt->servername, false); + aclresult = pg_foreign_server_aclcheck(server->serverid, ownerId, ACL_USAGE); + if (aclresult != ACLCHECK_OK) + aclcheck_error(aclresult, ACL_KIND_FOREIGN_SERVER, server->servername); + + /* Check the permissions on the schema */ + LookupCreationNamespace(local_schema); + + fdw = GetForeignDataWrapper(server->fdwid); + fdw_routine = GetFdwRoutine(fdw->fdwhandler); + if (fdw_routine->ImportForeignSchema == NULL) + { + ereport(ERROR, +(errcode(ERRCODE_FDW_NO_SCHEMAS), + errmsg("This FDW does not support schema importation"))); + } + table_list = fdw_routine->ImportForeignSchema(server, stmt); + foreach(lc, table_list) + { + CreateForeignTableStmt *create_stmt = lfirst(lc); + Oid relOid = DefineRelation((CreateStmt *) create_stmt, + RELKIND_FOREIGN_TABLE, + InvalidOid); + /* Override whatever the fdw set for the schema */ + create_stmt->base.relation->schemaname = local_schema; + CreateForeignTable(create_stmt, relOid); + } +} diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 81169a4..80c18cc 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -235,8 +235,8 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); DropAssertStmt DropTrigStmt DropRuleStmt DropCastStmt DropRoleStmt DropUserStmt DropdbStmt DropTableSpaceStmt DropFdwStmt DropForeignServerStmt DropUserMappingStmt ExplainStmt FetchStmt - GrantStmt GrantRoleStmt IndexStmt InsertStmt ListenStmt LoadStmt - LockStmt NotifyStmt ExplainableStmt PreparableStmt + GrantStmt GrantRoleStmt IndexStmt ImportForeignSchemaStmt InsertStmt + ListenStmt LoadStmt LockStmt NotifyStmt ExplainableStmt PreparableStmt CreateFunctionStmt AlterFunctionStmt ReindexStmt RemoveAggrStmt RemoveFuncStmt RemoveOperStmt RenameStmt RevokeStmt RevokeRoleStmt RuleActionStmt RuleActionStmtOrEmpty RuleStmt @@ -319,6 +319,8 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); %type defacl_privilege_target %type DefACLOption %type DefACLOptionList +%type OptImportForeignSchemaRest
Re: [HACKERS] Display oprcode and its volatility in \do+
On Thu, Jan 16, 2014 at 5:22 PM, Tom Lane wrote: > but adding > volatility here seems like probably a waste of valuable terminal width. > I think that the vast majority of operators have immutable or at worst > stable underlying functions, so this doesn't seem like the first bit > of information I'd need about the underlying function. For a data point, just today I wanted to look up the volatility of pg_trgm operators, which made me remember this patch. The \do+ output is narrow enough, I think an extra volatility column wouldn't be too bad. But even just having the function name is a huge improvement, at least that allows looking up volatility using \commands without accessing pg_operator directly. Regards, Marti -- 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] Proposal: IMPORT FOREIGN SCHEMA statement.
On Fri, Feb 21, 2014 at 4:01 PM, Ronan Dunklau wrote: > Hello, > > The SQL-MED specification defines the IMPORT FOREIGN SCHEMA statement. > > This adds discoverability to foreign servers. The structure of the > statement > as I understand it is simple enough: > > IMPORT FOREIGN SCHEMA remote_schema FROM SERVER some_server [ (LIMIT TO | > EXCEPT) table_list ] INTO local_schema. > > Is anyone working on this? I found a reference to this from 2010 in the > archive, stating that work should be focused on core functionality, but > nothing more recent. > > This would be very useful for postgres_fdw and other RDBMS-backed fdws, > but I > think even file_fdw could benefit from it if it was able to create a > foreign > table for every csv-with-header file in a directory. > > I can see a simple API working for that. A new function would be added to > the > fdw routine, which is responsible for crafting CreateForeignTableStmt. It > could have the following signature: > > typedef List *(*ImportForeignSchema_function) (ForeignServer *server, > ImportForeignSchemaStmt * parsetree); > > > I experimented with this idea, and came up with the attached two patches: > one > for the core, and the other for actually implementing the API in > postgres_fdw. > > Maybe those can serve as a proof-of-concept for discussing the design? > I havent had a look at the patch yet since I dont have a nice editor right now, but how do you handle inter operability between datatypes? Specifically, how do you handle those datatypes which have a different name from the PostgreSQL name for them and/or are stored in a different manner? Regards, Atri
Re: [HACKERS] Changeset Extraction v7.6.1
Hi, On 2014-02-19 13:01:02 -0500, Robert Haas wrote: > > I think it should be fairly easy to relax the restriction to creating a > > slot, but not getting data from it. Do you think that would that be > > sufficient? > > That would be a big improvement, for sure, and might be entirely sufficient. Turned out to be a 5 line change + tests or something... Pushed. > >> I don't think this is a very good idea. The problem with doing things > >> during error recovery that can themselves fail is that you'll lose the > >> original error, which is not cool, and maybe even blow out the error > >> stack. Many people have confuse PG_TRY()/PG_CATCH() with an > >> exception-handling system, but it's not. One way to fix this is to > >> put some of the initialization logic in ReplicationSlotCreate() just > >> prior to calling CreateSlotOnDisk(). If the work that needs to be > >> done is too complex or protracted to be done there, then I think that > >> it should be pulled out of the act of creating the replication slot > >> and made to happen as part of first use, or as a separate operation > >> like PrepareForLogicalDecoding. > > > > I think what should be done here is adding a drop_on_release flag. As > > soon as everything important is done, it gets unset. > > That might be more elegant, but I don't think it really fixes > anything, because backing stuff out from on disk can fail. If the slot is marked as "drop_on_release" during creation, and we fail during removal, it will just be dropped on the next startup. That seems ok to me? I still think it's not really important to put much effort in the "disk stuff fails" case, it's entirely hypothetical. If that fails you have *so* much huger problems, a leftover slot is the least of you problems. > AIUI, your > whole concern here is that you don't want the slot creation to fail > halfway through and leave behind the slot, but what you've got here > doesn't prevent that; it just makes it less likely. The more I think > about it, the more I think you're trying to pack stuff into slot > creation that really ought to be happening on first use. Well, having a leftover slot that never succeeded being created is going to be confusing lots of people, especially as it will not rollback or something. That's why I think it's important to make it unlikely. The typical reasons for failing are stuff like a output plugin that doesn't exist or being interrupted while initializing. I can sympathize with the "too much during init" argument, but I don't see how moving stuff to the first call would get rid of the problems. If we fail later it's going to be just as confusing. > >> ReorderBufferGetTXN() should get a comment about the performance > >> impact of this. There's a tiny bit there in ReorderBufferReturnTXN() > >> but it should be better called out. Should these call the valgrind > >> macros to make the memory inaccessible while it's being held in cache? > > > > Hm, I think it does call the valgrind stuff? > > VALGRIND_MAKE_MEM_UNDEFINED(txn, sizeof(ReorderBufferTXN)); > > VALGRIND_MAKE_MEM_DEFINED(&txn->node, sizeof(txn->node)); > > That's there in ReorderBufferReturnTXN, but don't you need something > in ReorderBufferGetTXN? Maybe not. Don't think so, it marks the memory as undefined, which allows writes, but will warn on reads. We could additionally mark the memory as inaccessible disallowing writes, but I don't really that catching much. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal: IMPORT FOREIGN SCHEMA statement.
> I havent had a look at the patch yet since I dont have a nice editor right > now, but how do you handle inter operability between datatypes? > Specifically, how do you handle those datatypes which have a different name > from the PostgreSQL name for them and/or are stored in a different manner? Do you mean in general, or for the postgres_fdw specifically ? In general, only valid column types should be accepted in the CreateForeignTableStmt. The CreateForeignTableStmt is passed through DefineRelation, which takes care of looking up the actual data types. For the postgres_fdw POC implementation, this is done by parsing the attributes type from the query result with the regtype input functions. The attribute typmod is injected too. > > Regards, > > Atri -- Ronan Dunklau http://dalibo.com - http://dalibo.org signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Changeset Extraction v7.6.1
On 2014-02-19 13:31:06 -0500, Robert Haas wrote: > TBH, as compared to what you've got now, I think this mostly boils > down to a question of quoting and escaping. I'm not really concerned > with whether we ship something that's perfectly efficient, or that has > filtering capabilities, or that has a lot of fancy bells and whistles. > What I *am* concerned about is that if the user updates a text field > that contains characters like " or ' or : or [ or ] or , that somebody > might be using as delimiters in the output format, that a program can > still parse that output format and reliably determine what the actual > change was. I don't care all that much whether we use JSON or CSV or > something custom, but the data that gets spit out should not have > SQL-injection-like vulnerabilities. If it's just that, I am *perfectly* happy to change it. What I do not want is arguments like "I don't want the type information, that's pointless" because it's actually really important for regression testing. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Public header files change
It seems to be unlikely to me, but are the changed symbols mentioned in git-log commit message 5f173040e324 supposed to be used other than internally? Snip: Avoid repeated name lookups during table and index DDL. ... to the Constraint node (FkConstraint in 8.4). Third-party code calling these functions or using the Constraint node will require updating. These seem to be pretty internal symbols to me, but I rather asking here. Is there some project we know off that is using this functions (or is there some obvious usecase for third-parties)? Pavel -- 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] Proposal: IMPORT FOREIGN SCHEMA statement.
On Fri, Feb 21, 2014 at 4:39 PM, Ronan Dunklau wrote: > > I havent had a look at the patch yet since I dont have a nice editor > right > > now, but how do you handle inter operability between datatypes? > > Specifically, how do you handle those datatypes which have a different > name > > from the PostgreSQL name for them and/or are stored in a different > manner? > > Do you mean in general, or for the postgres_fdw specifically ? > > In general, only valid column types should be accepted in the > CreateForeignTableStmt. The CreateForeignTableStmt is passed through > DefineRelation, which takes care of looking up the actual data types. > > For the postgres_fdw POC implementation, this is done by parsing the > attributes type from the query result with the regtype input functions. The > attribute typmod is injected too. > I actually meant in general. Thanks for the reply. So please help me understand here. How exactly does CreateForeignTableStmt help in type compatibility? A statement may be valid on a foreign server but may not be compatible. What am I missing here naively? Regards, Atri -- Regards, Atri *l'apprenant*
Re: [HACKERS] Public header files change
Hi, On 2014-02-21 12:11:42 +0100, Pavel Raiskup wrote: > It seems to be unlikely to me, but are the changed symbols mentioned in > git-log commit message 5f173040e324 supposed to be used other than > internally? Snip: > > These seem to be pretty internal symbols to me, but I rather asking here. > Is there some project we know off that is using this functions (or is > there some obvious usecase for third-parties)? I don't know of any public/open code, but I have seen DefineIndex() being used directly in some company's private code. And it's probably not a good idea to do so. Since such code should also adapt to be safe, it's probably a good idea if it breaks. If such code is used without recompiling against an updated PG, it's likely that it will error out with a somewhat strange message (Cache lookup failure for Oid ), but it shouldn't crash. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal: IMPORT FOREIGN SCHEMA statement.
Le vendredi 21 février 2014 16:45:20 Atri Sharma a écrit : > On Fri, Feb 21, 2014 at 4:39 PM, Ronan Dunklau wrote: > > > I havent had a look at the patch yet since I dont have a nice editor > > > > right > > > > > now, but how do you handle inter operability between datatypes? > > > Specifically, how do you handle those datatypes which have a different > > > > name > > > > > from the PostgreSQL name for them and/or are stored in a different > > > > manner? > > > > Do you mean in general, or for the postgres_fdw specifically ? > > > > In general, only valid column types should be accepted in the > > CreateForeignTableStmt. The CreateForeignTableStmt is passed through > > DefineRelation, which takes care of looking up the actual data types. > > > > For the postgres_fdw POC implementation, this is done by parsing the > > attributes type from the query result with the regtype input functions. > > The > > attribute typmod is injected too. > > I actually meant in general. Thanks for the reply. > > So please help me understand here. How exactly does CreateForeignTableStmt > help in type compatibility? I'm not sure I understand your concern. It doesn't help in type compatibility, it is still the responsibility of the FDW to convert local types to remote ones. The CreateForeignTableStmt defines the column, with their types. It is executed locally to create a new foreign table according to a remote description of the table. The only difference with a user-written CreateForeignTable statement is that the structure is crafted by the FDW instead of the parser. > A statement may be valid on a foreign server but may not be compatible. Do you mean the CreateForeignTableStmt ? It has to be valid locally, or it won't be executed. It is the FDW responsibility to build this statement in such a way that it is valid locally. > > Regards, > > Atri -- Ronan Dunklau http://dalibo.com - http://dalibo.org signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Patch: show relation and tuple infos of a lock to acquire
Hi, On 19.02.2014 08:11, Amit Kapila wrote: > I have looked into this patch. Below are some points which I > think should be improved in this patch: Thank you for your review. > 1. New context added by this patch is display at wrong place > […] > Do this patch expect to print the context at cancel request > as well? > I don't find it useful. I think the reason is that after setup of > context, if any other error happens, it will display the newly setup > context. To be honest, I don't see a problem here. If you cancel the request in most cases it is because it doesn't finish in an acceptable time. So displaying the context seems logical to me. > 2a. About the message: > LOG: process 1936 still waiting for ShareLock on transaction 842 > after 1014.000ms > CONTEXT: relation name: foo (OID 57496) > tuple (ctid (0,1)): (1, 2, abc ) > > Isn't it better to display database name as well, as if we see in > one of related messages as below, it displays database name as well. > > "LOG: process 3012 still waiting for ExclusiveLock on tuple (0,1) of > relation 57 > 499 of database 12045 after 1014.000 ms" Maybe. But then we either have duplicate infos in some cases (e.g. on a table lock) or we have to distinguish error cases and show distinct contextes. And also getting the database name from a relation is not really straight forward (according to Andres we would have to look at rel->rd_node.dbNode) and since other commands dealing with names don't do so, either, I think we should leave out the database name. > 2b. I think there is no need to display *ctid* before tuple offset, it seems > to be obvious as well as consistent with other similar message. Ok, I'll drop it. > 3. About callback I think rather than calling setup/tear down > functions from multiple places, it will be better to have wrapper > functions for XactTableLockWait() and MultiXactIdWait(). Just check > in plpgsql_exec_function(), how the errorcallback is set, can we do > something similar without to avoid palloc? OK, Attached. > 4. About the values of tuple > I think it might be useful to display some unique part of tuple for > debugging part, but what will we do incase there is no primary > key on table, so may be we can display initial few columns (2 or 3) > or just display tuple offset as is done in other similar message > shown above. Currently I simply display the whole tuple with truncating long fields. This is plain easy, no distinction necessary. I did some code reading and it seems somewhat complex to get the PK columns and it seems that we need another lock for this, too – maybe not the best idea when we are already in locking trouble, do you disagree? Also showing just a few columns when no PK is available might not be helpful since the tuple is not necessarily identified by the columns showed. And since we drop the CTID there is no alternative solution to identify the tuple. IMHO simply displaying the whole tuple is the best solution in this case, do you agree? > More to the point, I have observed that in few cases > displaying values of tuple can be confusing as well. For Example: > […] > Now here it will be bit confusing to display values with tuple, as > in session-3 statement has asked to update value (3), but we have > started waiting for value (4). Although it is right, but might not > make much sense. What do you suggest to avoid confusion? I can see what you are talking about, but I'm not sure what to do about it. Keep in mind that this patch should help debugging locking, so IMHO it is essential to be able to identify a tuple and therefore knowing its values. > Also after session-2 commits the transaction, session-3 will say > acquired lock, however still it will not be able to update it. To be honest, I don't think that this is a problem of the patch. Concurrency is not easy and it might lead to confusing situations. I don't think that we should drop the tuple values because of this, since they provide useful and precious debugging information. Attached you will find a new version of the patch, mainly using wrapper functions for XactLockTableWait() and MultiXactIdWait(). Best regards, -- Christian Kruse http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index a771ccb..71aaa93 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -107,6 +107,8 @@ static TransactionId MultiXactIdGetUpdateXid(TransactionId xmax, uint16 t_infomask); static void MultiXactIdWait(MultiXactId multi, MultiXactStatus status, int *remaining, uint16 infomask); +static void MultiXactIdWaitWithErrorContext(Relation rel, HeapTuple tup, MultiXactId multi, + MultiXactStatus status, int *remaining, uint16 infomask); static bool ConditionalMultiXactIdWait(MultiXactId multi, MultiXactStatus status, int *remaining, uint16 infomask); @@ -27
Re: [HACKERS] Proposal: IMPORT FOREIGN SCHEMA statement.
On Fri, Feb 21, 2014 at 4:56 PM, Ronan Dunklau wrote: > Le vendredi 21 février 2014 16:45:20 Atri Sharma a écrit : > > On Fri, Feb 21, 2014 at 4:39 PM, Ronan Dunklau > wrote: > > > > I havent had a look at the patch yet since I dont have a nice editor > > > > > > right > > > > > > > now, but how do you handle inter operability between datatypes? > > > > Specifically, how do you handle those datatypes which have a > different > > > > > > name > > > > > > > from the PostgreSQL name for them and/or are stored in a different > > > > > > manner? > > > > > > Do you mean in general, or for the postgres_fdw specifically ? > > > > > > In general, only valid column types should be accepted in the > > > CreateForeignTableStmt. The CreateForeignTableStmt is passed through > > > DefineRelation, which takes care of looking up the actual data types. > > > > > > For the postgres_fdw POC implementation, this is done by parsing the > > > attributes type from the query result with the regtype input functions. > > > The > > > attribute typmod is injected too. > > > > I actually meant in general. Thanks for the reply. > > > > So please help me understand here. How exactly does > CreateForeignTableStmt > > help in type compatibility? > > I'm not sure I understand your concern. It doesn't help in type > compatibility, > it is still the responsibility of the FDW to convert local types to remote > ones. > Yeah, thats what I wondered. Ok, now I get it. The responsibility of FDW shall suffice for us. > > The CreateForeignTableStmt defines the column, with their types. It is > executed locally to create a new foreign table according to a remote > description of the table. The only difference with a user-written > CreateForeignTable statement is that the structure is crafted by the FDW > instead of the parser. > Got that. > > > A statement may be valid on a foreign server but may not be compatible. > > Do you mean the CreateForeignTableStmt ? It has to be valid locally, or it > won't be executed. It is the FDW responsibility to build this statement in > such a way that it is valid locally. > Yes, I understand it now. My concerns are not valid anymore. Thanks for the detailed description. Regards, Atri -- Regards, Atri *l'apprenant*
Re: [HACKERS] Changeset Extraction v7.6.1
On 2014-02-19 13:07:11 -0500, Robert Haas wrote: > On Tue, Feb 18, 2014 at 4:07 AM, Andres Freund wrote: > >> 2. I think the snapshot-export code is fundamentally misdesigned. As > >> I said before, the idea that we're going to export one single snapshot > >> at one particular point in time strikes me as extremely short-sighted. > > > > I don't think so. It's precisely what you need to implement a simple > > replication solution. Yes, there are usecases that could benefit from > > more possibilities, but that's always the case. > > > >> For example, consider one-to-many replication where clients may join > >> or depart the replication group at any time. Whenever somebody joins, > >> we just want a pair such that they can apply all > >> changes after the LSN except for XIDs that would have been visible to > >> the snapshot. > > > > And? They need to create individual replication slots, which each > > will get a snapshot. > > So we have to wait for startup N times, and transmit the change stream > N times, instead of once? Blech. I can't get too excited about this. If we later want to add a command to clone an existing slot, sure, that's perfectly fine with me. That will then stream at exactly the same position. Easy, less than 20 LOC + docs probably. We have much more waiting e.g. in the CONCURRENTLY commands and it's not causing that many problems. Note that it'd be a *significant* overhead to contiuously be able to export snapshots that are useful for looking at normal relations. Bot for computing snapshots and for not being able to remove those rows. > >> And in fact, we don't even need any special machinery > >> for that; the client can just make a connection and *take a snapshot* > >> once decoding is initialized enough. > > > > No, they can't. Two reasons: For one the commit order between snapshots > > and WAL isn't necessarily the same. > So what? So you can't just use a plain snapshot and dump using it, without getting into inconsistencies. > > For another, clients now need logic > > to detect whether a transaction's contents has already been applied or > > has not been applied yet, that's nontrivial. > My point is, I think we should be trying to *make* that trivial, > rather than doing this. I think *this* *is* making it trivial. Maybe I've missed it, but I haven't seen any alternative that comes even *close* to being as easy to implement in a replication solution. Currently you can use it like: CREATE_REPLICATION_SLOT LOGICAL copy data using the exported snapshot START_REPLICATION SLOT LOGICAL stream changes. Where you can do the START_REPLICATION as soon as some other sesion has imported the snapshot. Really not much to worry about additionally. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal: IMPORT FOREIGN SCHEMA statement.
On Feb21, 2014, at 12:09 , Ronan Dunklau wrote: >> I havent had a look at the patch yet since I dont have a nice editor right >> now, but how do you handle inter operability between datatypes? >> Specifically, how do you handle those datatypes which have a different name >> from the PostgreSQL name for them and/or are stored in a different manner? > > For the postgres_fdw POC implementation, this is done by parsing the > attributes type from the query result with the regtype input functions. The > attribute typmod is injected too. Who says that the OIDs are the same on the local and the remove postgres instance? For user-defined types, that's certainly not going to be true... Also, why do you aggregate the lists of columns, types and oids into arrays when querying them from the remote server? Just producing a query that returns one row per table column seems much simpler, both conceptually and implementation wise. Finally, I think there are a few missing pieces. For example, you quite easily could copy over not-NULL flags, but you currently don't. Similarly, what about inheritance relationships between remote tables? There's a patch in the current CF, I believe, which adds support for inheritance to foreign tables, so all you'd have to do is to make the foreign table's inheritance structure match the remote table's. best regards, Florian Pflug -- 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] Proposal: IMPORT FOREIGN SCHEMA statement.
> Who says that the OIDs are the same on the local and the remove postgres > instance? For user-defined types, that's certainly not going to be true... That's why the the result is casted as regtype[], and parsed as such. The oid is not transmitted over the wire, but set by regtype_in. > > Also, why do you aggregate the lists of columns, types and oids into arrays > when querying them from the remote server? Just producing a query that > returns one row per table column seems much simpler, both conceptually and > implementation wise. As I said, this is a Proof-Of-Concept. It is not meant to be a fully functional, optimal implementation, but to serve as basis for discussion of the API and the feature themselves. The query could indeed be replaced by what you suggest, performing the grouping locally. I have absolutely no opinion on which implementation is better, this one seemed the most "natural" to me. > > Finally, I think there are a few missing pieces. For example, you quite > easily could copy over not-NULL flags, but you currently don't. Similarly, > what about inheritance relationships between remote tables? There's a patch > in the current CF, I believe, which adds support for inheritance to foreign > tables, so all you'd have to do is to make the foreign table's inheritance > structure match the remote table's. Duly noted, we could probably import NOT NULL flags, and maybe even the table's inheritance structure. I'll look into that if the feature and the API are deemed worthy. Thank you for the review. -- Ronan Dunklau http://dalibo.com - http://dalibo.org signature.asc Description: This is a digitally signed message part.
[HACKERS] Storing the password in .pgpass file in an encrypted format
Hi, Is there a way to store the password in ".pgpass" file in an encrypted format (for example, to be used by pg_dump). Even though, there are ways to set the permissions on .pgpass, to disallow any access to world or group, the security rules of many organizations disallow to hold any kind of passwords, as plain text. If there is no existing way to do this, shall we take up this, as a patch? Regards, Firoz EV
Re: [HACKERS] Patch: show xid and xmin in pg_stat_activity and pg_stat_replication
On 2014-02-21 13:40:59 +0100, Christian Kruse wrote: > diff --git a/src/backend/utils/adt/pgstatfuncs.c > b/src/backend/utils/adt/pgstatfuncs.c > index a4f31cf..e65b079 100644 > --- a/src/backend/utils/adt/pgstatfuncs.c > +++ b/src/backend/utils/adt/pgstatfuncs.c > @@ -536,7 +536,7 @@ pg_stat_get_activity(PG_FUNCTION_ARGS) > > oldcontext = > MemoryContextSwitchTo(funcctx->multi_call_memory_ctx); > > - tupdesc = CreateTemplateTupleDesc(14, false); > + tupdesc = CreateTemplateTupleDesc(16, false); > TupleDescInitEntry(tupdesc, (AttrNumber) 1, "datid", > OIDOID, -1, 0); > TupleDescInitEntry(tupdesc, (AttrNumber) 2, "pid", > @@ -565,6 +565,10 @@ pg_stat_get_activity(PG_FUNCTION_ARGS) > TEXTOID, -1, 0); > TupleDescInitEntry(tupdesc, (AttrNumber) 14, "client_port", > INT4OID, -1, 0); > + TupleDescInitEntry(tupdesc, (AttrNumber) 15, "backend_xid", > +XIDOID, -1, 0); > + TupleDescInitEntry(tupdesc, (AttrNumber) 16, "backend_xmin", > +XIDOID, -1, 0); > > funcctx->tuple_desc = BlessTupleDesc(tupdesc); > > @@ -588,11 +592,11 @@ pg_stat_get_activity(PG_FUNCTION_ARGS) > > for (i = 1; i <= n; i++) > { > - PgBackendStatus *be = > pgstat_fetch_stat_beentry(i); > + LocalPgBackendStatus *be = > pgstat_fetch_stat_local_beentry(i); > > if (be) > { > - if (be->st_procpid == pid) > + if (be->backendStatus.st_procpid > == pid) If we're going this route - which I am ok with - I'd suggest for doing something like: > - PgBackendStatus *be = > pgstat_fetch_stat_beentry(i); > + LocalPgBackendStatus *lbe = > pgstat_fetch_stat_local_beentry(i); > + PgBackendStatus *be = &lb->backendStatus; There seems little point in making all those lines longer and the accompanying diff noise if all it costs is a local variable. Makes sense? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Storing the password in .pgpass file in an encrypted format
On 21 February 2014 13:49, firoz e v wrote: > Hi, > > > > Is there a way to store the password in “.pgpass” file in an encrypted > format (for example, to be used by pg_dump). > > > > Even though, there are ways to set the permissions on .pgpass, to disallow > any access to world or group, the security rules of many organizations > disallow to hold any kind of passwords, as plain text. > > > > If there is no existing way to do this, shall we take up this, as a patch? > > > > Regards, > > Firoz EV > > > And where are you going to keep the passwords to decrypt these passwords (for example to be used by pg_dump)? regards, Szymon
Re: [HACKERS] Changeset Extraction v7.6.1
On Fri, Feb 21, 2014 at 6:07 AM, Andres Freund wrote: > I can sympathize with the "too much during init" argument, but I don't > see how moving stuff to the first call would get rid of the problems. If > we fail later it's going to be just as confusing. No, it isn't. If you fail during init the use will expect the slot to be gone. That's the reason for all of this complexity. If you fail on first use, the user will expect the slot to still be there. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Changeset Extraction v7.6.1
On 2014-02-21 08:16:59 -0500, Robert Haas wrote: > On Fri, Feb 21, 2014 at 6:07 AM, Andres Freund wrote: > > I can sympathize with the "too much during init" argument, but I don't > > see how moving stuff to the first call would get rid of the problems. If > > we fail later it's going to be just as confusing. > > No, it isn't. If you fail during init the use will expect the slot to > be gone. That's the reason for all of this complexity. If you fail > on first use, the user will expect the slot to still be there. The primary case for failing is a plugin that either doesn't exist or fails to initialize, or a user aborting the init. It seems odd that a created slot fails because of a bad plugin or needs to wait till it finds a suitable snapshot record. We could add an intermediary call like pg_startup_logical_slot() but that doesn't seem to have much going for it? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] walsender doesn't send keepalives when writes are pending
On Fri, Feb 21, 2014 at 2:36 PM, Andres Freund wrote: > On 2014-02-21 10:08:44 +0530, Amit Kapila wrote: >> >> I think the main reason of ping is to detect n/w break sooner. >> On a busy server, wouldn't WALSender can detect it when next time it >> will try to send the remaining data? > > Well, especially on a pipelined connection, that can take a fair > bit. TCP timeouts aren't fun. Okay, but the behaviour should be same for both keepalive message and wal data it needs to send. What I mean to say here is that if n/w is down, wal sender will detect it early by sending some data (either keepalive or wal data). Also the ping is sent only after wal_sender_timeout/2 w.r.t last reply time and wal data will be sent after each sleeptime (1 + wal_sender_timeout/10) which I think is should be lesser than the time to send ping. > There's a reason we have the keepalives, > and that they measure application to application performance. Do you mean to say the info about receiver (uphill what it has flushed..)? If yes, then won't we get this information in other reply message or sending keepalive will achieve any other purpose (may be it can get info more quickly)? > And > detecting systems as down is important for e.g. synchronous rep. If you are right, then I am missing some point which is how on a busy server sending keepalive can detect n/w break more quickly then current way. With Regards, Amit Kapila. 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] walsender doesn't send keepalives when writes are pending
On 2014-02-21 19:03:29 +0530, Amit Kapila wrote: > On Fri, Feb 21, 2014 at 2:36 PM, Andres Freund wrote: > > On 2014-02-21 10:08:44 +0530, Amit Kapila wrote: > >> > >> I think the main reason of ping is to detect n/w break sooner. > >> On a busy server, wouldn't WALSender can detect it when next time it > >> will try to send the remaining data? > > > > Well, especially on a pipelined connection, that can take a fair > > bit. TCP timeouts aren't fun. > > Okay, but the behaviour should be same for both keepalive message > and wal data it needs to send. What I mean to say here is that if n/w > is down, wal sender will detect it early by sending some data (either > keepalive or wal data). Also the ping is sent only after > wal_sender_timeout/2 w.r.t last reply time and wal data will be > sent after each sleeptime (1 + wal_sender_timeout/10) which > I think is should be lesser than the time to send ping. The danger is rather that *no* keepalive is sent (with requestReply = true triggering a reply by the client) by the walsender. Try to run pg_receivexlog against a busy server with a low walsender timeout. Since we'll never get to sending a keepalive we'll not trigger a reply by the receiver. Now > > There's a reason we have the keepalives, > > and that they measure application to application performance. > > Do you mean to say the info about receiver (uphill what it has > flushed..)? The important bit is updating walsender.c's last_reply_timestamp so we know that the standby has updated and we won't kill it. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WAL Rate Limiting
On Thu, Feb 20, 2014 at 12:07 PM, Alvaro Herrera wrote: > I think "bulk" (maintenance) operations should legitimately configured > separately from regular UPDATE etc operations. For the latter I think > it's pretty reasonable to assume that users are going to want to tweak > the GUC for each individual callsite in the application, rather than > wholesale in postgresql.conf. There's an argument to be made for that design, but the discussion upthread doesn't support the contention that the patch makes a clean and well-defined division between those things. The initial patch left VACUUM out on the dubious theory that since we have an existing system to limit its throughput by pages dirtied we don't need a way to limit the rate at which it generates WAL, and left as an open question whether this out to apply to COPY and CREATE TABLE AS SELECT (but apparently not UPDATE or DELETE, or for that matter just plain SELECT, when it does a lot of pruning). A subsequent version of the patch changed the list of commands affected, but it's not at all clear to me that we have something as tidy as "only commands where X and never commands where Y". More broadly, three committers (myself, Tom, Heikki) expressed the desire for this to be made into a more generic mechanism, and two of those people (myself and Tom) said that this was too late for 9.4 and should be postponed to 9.5. Then a month went by and Greg Stark showed up and made noises about pushing this forward as-is. So I don't want it to be forgotten that those objections were made and have not been withdrawn. I'm not dead set against changing my position on this patch, but that should happen because of work to improve the patch - which was last posted on January 17th and did not at that time even include the requested and agreed fix to measure the limit in MB/s rather than some odd unit - not because of the simple passage of time. If anything, the passage of 5 weeks without meaningful progress ought to strengthen rather than weaken the argument that this is far too late for this release. Of course, if we're talking about 9.5, then disregard the previous paragraph. But in that case perhaps we could postpone this until we don't have 40+ patches needing review and a dozen marked ready for committer. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Changeset Extraction v7.6.1
On Fri, Feb 21, 2014 at 8:27 AM, Andres Freund wrote: > On 2014-02-21 08:16:59 -0500, Robert Haas wrote: >> On Fri, Feb 21, 2014 at 6:07 AM, Andres Freund >> wrote: >> > I can sympathize with the "too much during init" argument, but I don't >> > see how moving stuff to the first call would get rid of the problems. If >> > we fail later it's going to be just as confusing. >> >> No, it isn't. If you fail during init the use will expect the slot to >> be gone. That's the reason for all of this complexity. If you fail >> on first use, the user will expect the slot to still be there. > > The primary case for failing is a plugin that either doesn't exist or > fails to initialize, or a user aborting the init. It seems odd that a > created slot fails because of a bad plugin or needs to wait till it > finds a suitable snapshot record. We could add an intermediary call like > pg_startup_logical_slot() but that doesn't seem to have much going for > it? Well, we can surely detect a plugin that fails to initialize before creating the slot on disk, right? I'm not sure what "fails to initialize" entails. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WAL Rate Limiting
It's clear now that if what Greg wants is an uncontroversial patch to commit, this is not it. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Changeset Extraction v7.6.1
On 2014-02-21 08:51:03 -0500, Robert Haas wrote: > On Fri, Feb 21, 2014 at 8:27 AM, Andres Freund wrote: > > On 2014-02-21 08:16:59 -0500, Robert Haas wrote: > >> On Fri, Feb 21, 2014 at 6:07 AM, Andres Freund > >> wrote: > >> > I can sympathize with the "too much during init" argument, but I don't > >> > see how moving stuff to the first call would get rid of the problems. If > >> > we fail later it's going to be just as confusing. > >> > >> No, it isn't. If you fail during init the use will expect the slot to > >> be gone. That's the reason for all of this complexity. If you fail > >> on first use, the user will expect the slot to still be there. > > > > The primary case for failing is a plugin that either doesn't exist or > > fails to initialize, or a user aborting the init. It seems odd that a > > created slot fails because of a bad plugin or needs to wait till it > > finds a suitable snapshot record. We could add an intermediary call like > > pg_startup_logical_slot() but that doesn't seem to have much going for > > it? > > Well, we can surely detect a plugin that fails to initialize before > creating the slot on disk, right? We could detect whether the plugin .so can be loaded and provides the required callbacks, but we can't initialize it. > I'm not sure what "fails to initialize" entails. elog(ERROR, 'hey, the tables I require are missing'); or similar. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Cost estimation in foreign data wrappers
Hello, There is a callback function in fdw's which should also set estimates for startup and total costs for each path. Assume a fdw adds only one path (e.g. in file_fdw). I am trying to understand what can go wrong if we do a bad job in estimating these costs. Since we have only one scan path here, it doesn't make a difference in choosing the best scan path. By looking at the code and doing some experiments, I think this can be significant in (1) underestimating a nested loop's cost, (2) not materializing inner table in nested loop. * Are there any other cases that this can be significant? * Assume we are not sure about the exact cost, but we know that it is in [lower_bound, upper_bound] range, where upper_bound can be 10x lower_bound Then, what value is better to choose? lower bound? upper bound? or average? Thanks, -- Hadi
Re: [HACKERS] Storing the password in .pgpass file in an encrypted format
firoz e v wrote: > Hi, > > Is there a way to store the password in ".pgpass" file in an encrypted format > (for example, to be used by pg_dump). > > Even though, there are ways to set the permissions on .pgpass, to disallow > any access to world or group, the security rules of many organizations > disallow to hold any kind of passwords, as plain text. > > If there is no existing way to do this, shall we take up this, as a patch? Maybe you can memfrob() the password to encrypt it before writing, and then memfrob() it back before applying it. Would that be secure? -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Storing the password in .pgpass file in an encrypted format
Hi, On 21/02/14 11:15, Alvaro Herrera wrote: > Maybe you can memfrob() the password to encrypt it before writing, and > then memfrob() it back before applying it. Would that be secure? From `man memfrob`: Note that this function is not a proper encryption routine as the XOR constant is fixed, and is only suitable for hiding strings. No, it is not secure. And I agree, encrypting .pgpass doesn't make sense. Either you have a known key and then encryption is useless or you have to provide a key at runtime and then .pgpass is useless. Best regards, -- Christian Kruse http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services pgpVNLWTO24xl.pgp Description: PGP signature
Re: [HACKERS] Storing the password in .pgpass file in an encrypted format
On 21-02-2014 09:49, firoz e v wrote: > Even though, there are ways to set the permissions on .pgpass, to disallow > any access to world or group, the security rules of many organizations > disallow to hold any kind of passwords, as plain text. > Is your goal hiding the password in .pgpass? You could add support to accept md5... storage format as password. -- Euler Taveira Timbira - http://www.timbira.com.br/ PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento -- 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] Cost estimation in foreign data wrappers
Hadi Moshayedi writes: > There is a callback function in fdw's which should also set estimates for > startup and total costs for each path. Assume a fdw adds only one path > (e.g. in file_fdw). I am trying to understand what can go wrong if we do a > bad job in estimating these costs. > Since we have only one scan path here, it doesn't make a difference in > choosing the best scan path. Right. But if there's more than one table in the query, it might make a difference in terms of what join plan gets chosen. I'd say that getting an accurate rowcount estimate is usually far more important, 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] Storing the password in .pgpass file in an encrypted format
Euler Taveira wrote: > On 21-02-2014 09:49, firoz e v wrote: > > Even though, there are ways to set the permissions on .pgpass, to disallow > > any access to world or group, the security rules of many organizations > > disallow to hold any kind of passwords, as plain text. > > > Is your goal hiding the password in .pgpass? You could add support to > accept md5... storage format as password. How would that work? libpq needs the straight password to send to the server, not an encrypted one. If you were to have a mechanism by which libpq can store an md5'd password (or whatever hash) and send that md5 to the server and have the server accept it to grant a connection, then the md5 has, in effect, become the unencrypted password which others can capture from the file, and you're back at square one. You could instead try to have an authentication agent that stores an encrypted password or certificate and asks the user to supply the key to decrypt it when trying to establish a connection; but that would force you to require user intervention, which in many cases you don't want. If there's policy that disallows storage of plain-text passwords, your only choice appears to be not to use .pgpass in the first place. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Storing the password in .pgpass file in an encrypted format
On 2014-02-21 12:04:47 -0300, Alvaro Herrera wrote: > You could instead try to have an authentication agent that stores an > encrypted password or certificate and asks the user to supply the key to > decrypt it when trying to establish a connection; but that would force > you to require user intervention, which in many cases you don't want. Alternatively use something like kerberos. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Uninterruptable regexp_replace in 9.3.1 ?
On 02/21/2014 05:17 PM, Sandro Santilli wrote: > The following snippet reveals that 9.3.1 has a bug > in regexp_matches, which uninterruptably keeps CPU > spinning for minutes: Huh. So it does. That's interesting. (You should generally report things to pgsql-b...@postgresql.org btw, not -hackers) Looks like it's busily looping within the regex.c code, never hitting a CHECK_FOR_INTERRUPTS. The real question IMO is why it's taking so long. It looks like cfindloop(...) is being called multiple times, with each call taking a couple of seconds. A profile of the run is attached. I don't expect to have a chance to dig into this right away, as I haven't touched the regexp code before and would need to spend a bit of time studying it to achieve anything. Hopefully the test, confirmation, and profile is useful. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services # Overhead Command Shared Object Symbol # . ... # 40.33% postgres postgres [.] longest | --- longest cdissect cdissect cdissect cdissect cdissect cdissect | |--87.25%-- cdissect | | | |--78.14%-- cdissect | | | | | |--81.92%-- cdissect | | | | | | | |--68.07%-- cdissect | | | | | | | | | |--69.11%-- pg_regexec | | | | | | | | | --30.89%-- cdissect | | | | pg_regexec | | | | | | | --31.93%-- pg_regexec | | | | | --18.08%-- pg_regexec | | | --21.86%-- pg_regexec | --12.75%-- pg_regexec 19.20% postgres postgres [.] shortest | --- shortest cdissect cdissect cdissect cdissect cdissect cdissect cdissect cdissect cdissect cdissect pg_regexec 12.76% postgres postgres [.] miss | --- miss | |--88.87%-- longest | cdissect | cdissect | cdissect | cdissect | cdissect | cdissect | | | |--93.47%-- cdissect | | | | | |--74.23%-- cdissect | | | | | | | |--90.21%-- cdissect | | | | | | | | | |--85.39%-- cdissect | | | | | | | | | | | |--91.67%-- pg_regexec | | | | | | | | | | | --8.33%-- cdissect | | | | | pg_regexec | | | | | | | | | --14.61%-- pg_regexec | | | | | | | --9.79%-- pg_regexec | | | | | --25.77%-- pg_regexec | | | --6.53%-- pg_regexec | --11.13%-- shortest cdissect cdissect cdissect cdissect cdiss
Re: [HACKERS] Storing the password in .pgpass file in an encrypted format
On Fri, Feb 21, 2014 at 7:49 AM, firoz e v wrote: > Hi, > > > > Is there a way to store the password in ".pgpass" file in an encrypted > format (for example, to be used by pg_dump). > > > > Even though, there are ways to set the permissions on .pgpass, to disallow > any access to world or group, the security rules of many organizations > disallow to hold any kind of passwords, as plain text. > > > > If there is no existing way to do this, shall we take up this, as a patch? > As observed by others, storing the password in encrypted form in .pgpass merely means that you need to store the password to decrypt .pgpass in still another file that would, again, run afoul of such security policies. There is no appetite in the community to do implementation work that is provably useless as it cannot accomplish what people imagine to accomplish. The thing you could do instead that would *look* like it is encrypted is to use a certificate (e.g. - SSL). The certificate that you'd need to put on the client still needs to be in something that is effectively plain text (however much it looks like nonsensical encrypted text). -- When confronted by a difficult problem, solve it by reducing it to the question, "How would the Lone Ranger handle this?"
[HACKERS] SPI_connect on multi-threaded app
I'm writing a pgsql extension in C, which is multithreaded. The SPI connection is global, so do I have to implement a lock to make sql queries in each thread, or can I make a connection on a per-thread basis?
Re: [HACKERS] Uninterruptable regexp_replace in 9.3.1 ?
On Feb21, 2014, at 16:46 , Craig Ringer wrote: > The real question IMO is why it's taking so long. It looks like > cfindloop(...) is being called multiple times, with each call taking a > couple of seconds. Yeah, I wondered about this too. I've shortened the example a bit - here are a few observations postgres=# select regexp_matches(' $a$b$c$', $REG$((?:[^'"$;]+|"[^"]*"|'(?:[^']*|'')*'|(\$[^$]*\$).*\2)+)$REG$, 'g'); Time: 33.026 ms postgres=# select regexp_matches(' $a$b$c$', $REG$((?:[^'"$;]+|"[^"]*"|'(?:[^']*|'')*'|(\$[^$]*\$).*\2)+)$REG$, 'g'); Time: 60.594 ms postgres=# select regexp_matches(' $a$b$c$', $REG$((?:[^'"$;]+|"[^"]*"|'(?:[^']*|'')*'|(\$[^$]*\$).*\2)+)$REG$, 'g'); Time: 114.410 ms postgres=# select regexp_matches('$a$b$c$', $REG$((?:[^'"$;]+|"[^"]*"|'(?:[^']*|'')*'|(\$[^$]*\$).*\2)+)$REG$, 'g'); Time: 227.467 ms postgres=# select regexp_matches(' $a$b$c$', $REG$((?:[^'"$;]+|"[^"]*"|'(?:[^']*|'')*'|(\$[^$]*\$).*\2)+)$REG$, 'g'); Time: 452.739 ms postgres=# select regexp_matches(' $a$b$c$', $REG$((?:[^'"$;]+|"[^"]*"|'(?:[^']*|'')*'|(\$[^$]*\$).*\2)+)$REG$, 'g'); Time: 943.098 ms postgres=# select regexp_matches(' $a$b$c$d$e$', $REG$((?:[^'"$;]+|"[^"]*"|'(?:[^']*|'')*'|(\$[^$]*\$).*\2)+)$REG$, 'g'); Time: 200.795 ms postgres=# select regexp_matches(' $a$b$c$d$e$f$', $REG$((?:[^'"$;]+|"[^"]*"|'(?:[^']*|'')*'|(\$[^$]*\$).*\2)+)$REG$, 'g'); Time: 298.264 ms postgres=# select regexp_matches(' $a$b$c$d$e$f$g$', $REG$((?:[^'"$;]+|"[^"]*"|'(?:[^']*|'')*'|(\$[^$]*\$).*\2)+)$REG$, 'g'); Time: 444.219 ms postgres=# select regexp_matches(' $a$b$c$d$e$f$g$h$', $REG$((?:[^'"$;]+|"[^"]*"|'(?:[^']*|'')*'|(\$[^$]*\$).*\2)+)$REG$, 'g'); Time: 696.137 ms postgres=# select regexp_matches(' $a$b$c$d$e$f$g$h$i$', $REG$((?:[^'"$;]+|"[^"]*"|'(?:[^']*|'')*'|(\$[^$]*\$).*\2)+)$REG$, 'g'); Time: 974.502 ms postgres=# select regexp_matches(' $a$b$c$d$e$f$g$h$i$j$', $REG$((?:[^'"$;]+|"[^"]*"|'(?:[^']*|'')*'|(\$[^$]*\$).*\2)+)$REG$, 'g'); Time: 1369.703 ms postgres=# select regexp_matches(' $a$b$c$d$e$f$g$h$i$j$', $REG$((?:[^'"$;]+|"[^"]*"|'(?:[^']*|'')*'|(\$[^$]*\$).*\2)+)$REG$, 'g'); Time: 2747.766 ms In other words, the observes runtime is roughly 2^i * 1.5^j for inputs consiting of i leading spaces (any character will probably do) followed by j substring of the form $X$ (X is an arbitrary character). best regards, Florian Pflug -- 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] Storing the password in .pgpass file in an encrypted format
On Fri, Feb 21, 2014 at 7:04 AM, Alvaro Herrera wrote: > Euler Taveira wrote: > > On 21-02-2014 09:49, firoz e v wrote: > > > Even though, there are ways to set the permissions on .pgpass, to > disallow any access to world or group, the security rules of many > organizations disallow to hold any kind of passwords, as plain text. > > > > > Is your goal hiding the password in .pgpass? You could add support to > > accept md5... storage format as password. > > How would that work? libpq needs the straight password to send to the > server, not an encrypted one. It looks like that is the way it is currently written, but it does not have to be that way, at least for "md5" rather than "password" authentication. > If you were to have a mechanism by which > libpq can store an md5'd password (or whatever hash) and send that md5 > to the server and have the server accept it to grant a connection, then > the md5 has, in effect, become the unencrypted password which others can > capture from the file, and you're back at square one. > The string in .pgpass would be enough for people to log into postgresql, true. But it would not work to log onto other things which share the same clear-text password but don't share the same salting mechanism. Cheers, Jeff
Re: [HACKERS] SPI_connect on multi-threaded app
On Feb21, 2014, at 13:44 , John Williams wrote: > I'm writing a pgsql extension in C, which is multithreaded. The SPI > connection is global, so do I have to implement a lock to make sql > queries in each thread, or can I make a connection on a per-thread basis? Postgres backends aren't multi-threaded, and offer no support for multiple threads whatsoever. AFAIK, the only safe way to use multiple threads is to either restrict calls to *any* postgres function to only one thread, or to use a single, big mutex to prevents multiple threads from accessing postgres internals simultaneously. You should also prevent auxiliary threads from executing the signal handlers that postgres installs. See pthread_sigmask. And you'll need to make sure that everything (i.e. the whole of postgres and your extension) is built with multi-threading enabled. Otherwise, things like errno might end up not being thread-local, meaning any syscall your auxiliary threads do can interfere with postgres' error handling. You might want to check whether you can run the multi-threaded parts of your extension as a separate process, and communicate with the backend via some IPC mechanism. best regards, Florian Pflugs -- 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] Storing the password in .pgpass file in an encrypted format
Jeff Janes escribió: > On Fri, Feb 21, 2014 at 7:04 AM, Alvaro Herrera > wrote: > > If you were to have a mechanism by which > > libpq can store an md5'd password (or whatever hash) and send that md5 > > to the server and have the server accept it to grant a connection, then > > the md5 has, in effect, become the unencrypted password which others can > > capture from the file, and you're back at square one. > > The string in .pgpass would be enough for people to log into postgresql, > true. But it would not work to log onto other things which share the same > clear-text password but don't share the same salting mechanism. That's true. Patches welcome to improve that. Maybe we can define that if the stored password string starts with $1$md5$ and has a just the right length then it's a md5 hash rather than cleartext, or something like that. I do fear that people are going to look at the file and say "hey, it's encrypted [sic] so it's secure! I can share the file with the world!". -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Warning in pg_backup_archiver.c
Michael, * Michael Paquier (michael.paqu...@gmail.com) wrote: > While compiling on clang, I noticed the following warning: > pg_backup_archiver.c:1950:32: warning: comparison of constant -1 with > expression of type 'ArchiveFormat' (aka 'enum _archiveFormat') is always > false > [-Wtautological-constant-out-of-range-compare] > if ((AH->format = fgetc(fh)) == EOF) > ^ ~~~ > Something like the patch attached calms down the compiler... This has been > introduced recently by commit cfa1b4a of the 9th of February. I've got a patch for this already, but it's included in a bunch of other minor cleanup stuff that I'm still playing with. I hope to commit it this weekend. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Uninterruptable regexp_replace in 9.3.1 ?
On 02/22/2014 12:04 AM, Florian Pflug wrote: > On Feb21, 2014, at 16:46 , Craig Ringer wrote: >> The real question IMO is why it's taking so long. It looks like >> cfindloop(...) is being called multiple times, with each call taking a >> couple of seconds. > > Yeah, I wondered about this too. I've shortened the example a bit - here > are a few observations > > [snip] > > In other words, the observes runtime is roughly 2^i * 1.5^j for inputs > consiting of i leading spaces (any character will probably do) followed > by j substring of the form $X$ (X is an arbitrary character). The biggest change in regexp support in the introduction of proper unicode support, but that was before 9.1. The problem report claims that the issue does not occur on 9.1, but yet: git diff REL9_1_STABLE master -- ./src/backend/utils/adt/regexp.c is utterly trivial; a copyright date line change, and 1609797c which just tweaks the includes. 9.0 has a much bigger diff. So I'd like to confirm that this issue doesn't affect 9.1. I can reproduce the issue againts 9.2. I don't have 9.1 or older lying around to test against right this second. Sandro, can you please provide the output of "SELECT version()" from a PostgreSQL version that is not slow with this query? (BTW, I'm highly amused by how the development style has changed around here. From "git blame", this is from 1997: "I haven't actually measured the speed improvement, but it `looks' a lot quicker visually when watching regression test output." Ha.) -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Storing the password in .pgpass file in an encrypted format
On 02/22/2014 12:20 AM, Alvaro Herrera wrote: > Jeff Janes escribió: >> On Fri, Feb 21, 2014 at 7:04 AM, Alvaro Herrera >> wrote: > >>> If you were to have a mechanism by which >>> libpq can store an md5'd password (or whatever hash) and send that md5 >>> to the server and have the server accept it to grant a connection, then >>> the md5 has, in effect, become the unencrypted password which others can >>> capture from the file, and you're back at square one. >> >> The string in .pgpass would be enough for people to log into postgresql, >> true. But it would not work to log onto other things which share the same >> clear-text password but don't share the same salting mechanism. > > That's true. Patches welcome to improve that. Maybe we can define that > if the stored password string starts with $1$md5$ and has a just the > right length then it's a md5 hash rather than cleartext, or something > like that. Frankly, that it's possible to just replay the md5 password says that "md5" isn't really meaningfully better than cleartext, just marginally less convenient. It should really involve a handshake, along the broad lines of: - Server sends random cookie - Client hashes password cleartext with random cookie from server - Server hashes stored (cleartext) password with random cookie - Server compares values like in the RFC 2617 DIGEST-MD5 authentication method used in SASL, or even CRAM-MD5 (RFC 2195). Both of which are imperfect, but at least not trivially replayable. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Storing the password in .pgpass file in an encrypted format
On 02/21/2014 11:52 PM, Christopher Browne wrote: > > The thing you could do instead that would *look* like it is encrypted is > to use a certificate (e.g. - SSL). The certificate that you'd need to > put on the client still needs to be in something that is effectively > plain text (however much it looks like nonsensical encrypted text). Yep, though the certificate private key may well be stored encrypted with a passphrase that must be entered via direct user interaction. It looks like doing it with OpenSSL for libpq you might be able to set a passphrase callback routine to prompt the user to decrypt a client certificate. With PgJDBC you use JSSE's keystore support. Client certificates are a *much* stronger way to do this. Another good option can be Kerberos. Either way, encrypting .pgpass seems utterly pointless. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Uninterruptable regexp_replace in 9.3.1 ?
On Feb21, 2014, at 17:29 , Craig Ringer wrote: > The problem report claims that the issue does not occur on 9.1, but yet: > > git diff REL9_1_STABLE master -- ./src/backend/utils/adt/regexp.c > > is utterly trivial; a copyright date line change, and 1609797c which > just tweaks the includes. 9.0 has a much bigger diff. On 9.1.12: postgres=# select regexp_matches(' $a$b$c$d$e$f$g$h$i$j$', $REG$((?:[^'"$;]+|"[^"]*"|'(?:[^']*|'')*'|(\$[^$]*\$).*\2)+)$REG$, 'g'); regexp_matches - {" $a$b$c$d$e$f$g$h$i$j",NULL} (1 row) Time: 1.048 ms On HEAD postgres=# select regexp_matches(' $a$b$c$d$e$f$g$h$i$j$', $REG$((?:[^'"$;]+|"[^"]*"|'(?:[^']*|'')*'|(\$[^$]*\$).*\2)+)$REG$, 'g'); regexp_matches - {" ",NULL} {a,NULL} {b,NULL} {c,NULL} {d,NULL} {e,NULL} {f,NULL} {g,NULL} {h,NULL} {i,NULL} {j,NULL} (11 rows) Time: 4787.239 ms Aha! Since we go rid of regex_flavor pre-9.1, I don't have an immediate suspect... best regards, Florian Pflug -- 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] Uninterruptable regexp_replace in 9.3.1 ?
Craig Ringer writes: > So I'd like to confirm that this issue doesn't affect 9.1. It doesn't. I suspect it has something to do with 173e29aa5 or one of the nearby commits in backend/regex/. 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] Storing the password in .pgpass file in an encrypted format
Hi, On 21 Únor 2014, 16:52, Christopher Browne wrote: > On Fri, Feb 21, 2014 at 7:49 AM, firoz e v wrote: > >> Hi, >> >> >> >> Is there a way to store the password in ".pgpass" file in an encrypted >> format (for example, to be used by pg_dump). >> >> >> >> Even though, there are ways to set the permissions on .pgpass, to >> disallow >> any access to world or group, the security rules of many organizations >> disallow to hold any kind of passwords, as plain text. >> >> >> >> If there is no existing way to do this, shall we take up this, as a >> patch? >> > > As observed by others, storing the password in encrypted form in .pgpass > merely means that you need to store the password to decrypt .pgpass in > still another file that would, again, run afoul of such security policies. > There is no appetite in the community to do implementation work that is > provably useless as it cannot accomplish what people imagine to > accomplish. Sure. If you want to log-in without any user interaction, then the password needs to be stored is a form equal to cleartext (e.g. with a key). It's mostly security by obscurity. What I think might be useful and safe at the same time is encrypted .pgpass with tools asking for the encryption key. Think of it as a simple passord wallet - not really useful if you're connecting to a single database, very useful if you have many as you only need to remember the single password. If the encrypted passwords were stored in a separate file (say .pgpass.wallet) then this should not break the current tools. The tools would do this: 1) exists .pgpass? 1.a) read .pgpass -> is there a matching record? (yes -> stop) 2) exists .pgpass.wallet? 2.a) ask for encryption key 2.b) read .pgpass using the decryption key 2.c) is there a matching record? (yes -> stop) 3) ask for connection info directly BTW yes, I know what kerberos is, but many of us are dealing with companies that don't use it. regards Tomas -- 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] Storing the password in .pgpass file in an encrypted format
On Fri, Feb 21, 2014 at 8:42 AM, Craig Ringer wrote: > On 02/22/2014 12:20 AM, Alvaro Herrera wrote: > > Jeff Janes escribió: > >> On Fri, Feb 21, 2014 at 7:04 AM, Alvaro Herrera < > alvhe...@2ndquadrant.com>wrote: > > > >>> If you were to have a mechanism by which > >>> libpq can store an md5'd password (or whatever hash) and send that md5 > >>> to the server and have the server accept it to grant a connection, then > >>> the md5 has, in effect, become the unencrypted password which others > can > >>> capture from the file, and you're back at square one. > >> > >> The string in .pgpass would be enough for people to log into postgresql, > >> true. But it would not work to log onto other things which share the > same > >> clear-text password but don't share the same salting mechanism. > > > > That's true. Patches welcome to improve that. Maybe we can define that > > if the stored password string starts with $1$md5$ and has a just the > > right length then it's a md5 hash rather than cleartext, or something > > like that. > > Frankly, that it's possible to just replay the md5 password says that > "md5" isn't really meaningfully better than cleartext, just marginally > less convenient. > > It should really involve a handshake, along the broad lines of: > > - Server sends random cookie > - Client hashes password cleartext with random cookie from server > - Server hashes stored (cleartext) password with random cookie > - Server compares values > I think that is what it does, except both the client and server use a hash of password to add the cookie to, not directly the cleartext password. The server can optionally store the 1st level hash rather than the cleartext, and then skip the first hash step (but not the second hash step). The client does not have a mechanism to start out with the hash, it currently always starts with the cleartext, but that is just an implementation detail. So it is not replayable if you just see what goes over the wire. If you see what the client starts with, then it is "replayable" but that is not really the right word for it. Cheers, Jeff
Re: [HACKERS] Storing the password in .pgpass file in an encrypted format
I think this thread deserves more attention: http://www.postgresql.org/message-id/caazkufajufddfp1_vghbdfyru0sj6msovvkrp87acq53ov6...@mail.gmail.com -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WAL Rate Limiting
On 1/16/14, 9:19 PM, Craig Ringer wrote: On 01/16/2014 11:52 PM, Tom Lane wrote: Heikki Linnakangas writes: On 01/16/2014 05:39 PM, Andres Freund wrote: Do you see a reasonable way to implement this generically for all commands? I don't. In suitable safe places, check if you've written too much WAL, and sleep if so. Call it CHECK_FOR_WAL_BUDGET(), along the lines of CHECK_FOR_INTERRUPTS(), but called less frequently. Or maybe vacuum_delay_point() is a better analogy. Hopefully you don't need to sprinkle them in too many places to be useful. We probably don't really need to implement it for "all" commands; I think everyone can agree that, say, ALTER TABLE RENAME COLUMN isn't going to emit enough WAL to really matter. My point was that we should try to hit everything that potentially *could* generate lots of WAL, rather than arbitrarily deciding that some are utility commands and some are not. Then you land up needing a configuration mechanism to control *which* commands get affected, too, to handle the original use case of "I want to rate limit vaccuum and index rebuilds, while everything else runs full tilt". Or do you propose to just do this per-session? If so, what about autovacuum? Tom was proposing per-session. For autovac, don't we already have some method of changing the GUCs it uses? If not, we should... -- Jim C. Nasby, Data Architect j...@nasby.net 512.569.9461 (cell) http://jim.nasby.net -- 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] Changeset Extraction v7.6.1
On 2/17/14, 7:31 PM, Robert Haas wrote: But do you really want to keep that snapshot around long enough to copy the entire database? I bet you don't: if the database is big, holding back xmin for long enough to copy the whole thing isn't likely to be fun. I can confirm that this would be epic fail, at least for londiste. It takes about 3 weeks for a new copy of a ~2TB database. There's no way that'd work with one snapshot. (Granted, copy performance in londiste is rather lackluster, but still...) -- Jim C. Nasby, Data Architect j...@nasby.net 512.569.9461 (cell) http://jim.nasby.net -- 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] Storing the password in .pgpass file in an encrypted format
On Fri, Feb 21, 2014 at 10:42 AM, Alvaro Herrera wrote: > I think this thread deserves more attention: > > http://www.postgresql.org/message-id/caazkufajufddfp1_vghbdfyru0sj6msovvkrp87acq53ov6...@mail.gmail.com (I wrote that mail) I'm still in interested in this idea and haven't found a good reason to rescind the general thinking there. Some of my colleagues are thinking along similar lines outside the Postgres context. They seem happy with how that is shaping up. So, if there is some will for revival, that would be grand. -- 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] Re: BUG #9210: PostgreSQL string store bug? not enforce check with correct characterSET/encoding
Noah Misch writes: > On Wed, Feb 19, 2014 at 08:22:13PM -0500, Tom Lane wrote: >> How much of this is back-patch material, do you think? > None of it. While many of the failures to validate against a character > encoding are clear bugs, applications hum along in spite of such bugs and > break when we tighten the checks. I don't see a concern to override that > here. Folks who want the tighter checking have some workarounds available. That's certainly a reasonable position to take concerning the changes for outside-a-transaction behavior. However, I think there's a case to be made for adding the additional pg_verify_mbstr() calls in the back branches. We've been promising since around 8.3 that invalidly encoded data can't get into a database, and it's disturbing to find that there are leaks in that. 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] Storing the password in .pgpass file in an encrypted format
On 02/21/2014 09:11 AM, Tomas Vondra wrote: > What I think might be useful and safe at the same time is encrypted > .pgpass with tools asking for the encryption key. Think of it as a simple > passord wallet - not really useful if you're connecting to a single > database, very useful if you have many as you only need to remember the > single password. Sounds interesting, but probably better as an external utility than as part of PostgreSQL. Call it pgWallet. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.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] Re: BUG #9210: PostgreSQL string store bug? not enforce check with correct characterSET/encoding
On Fri, Feb 21, 2014 at 05:20:06PM -0500, Tom Lane wrote: > Noah Misch writes: > > On Wed, Feb 19, 2014 at 08:22:13PM -0500, Tom Lane wrote: > >> How much of this is back-patch material, do you think? > > > None of it. While many of the failures to validate against a character > > encoding are clear bugs, applications hum along in spite of such bugs and > > break when we tighten the checks. I don't see a concern to override that > > here. Folks who want the tighter checking have some workarounds available. > > That's certainly a reasonable position to take concerning the changes for > outside-a-transaction behavior. However, I think there's a case to be > made for adding the additional pg_verify_mbstr() calls in the back > branches. We've been promising since around 8.3 that invalidly encoded > data can't get into a database, and it's disturbing to find that there > are leaks in that. I had a dark corner of an app break from the 8.4-vintage change to make E'abc\000def'::text raise an error rather than truncate the string. The old behavior was clearly wrong, but I was still glad the change arrived in a major release; the truncation happened to be harmless for that app. Adding pg_verify_mbstr() calls creates a similar situation. Anything broken by the outside-a-transaction behavior change will be in C code. I'll expect between zero and one reports of breakage from that, so whether to back-patch that is more academic. -- Noah Misch 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] Storing the password in .pgpass file in an encrypted format
On 22.2.2014 00:02, Josh Berkus wrote: > On 02/21/2014 09:11 AM, Tomas Vondra wrote: >> What I think might be useful and safe at the same time is encrypted >> .pgpass with tools asking for the encryption key. Think of it as a simple >> passord wallet - not really useful if you're connecting to a single >> database, very useful if you have many as you only need to remember the >> single password. > > Sounds interesting, but probably better as an external utility than > as part of PostgreSQL. Call it pgWallet. Depends on how you define external utility. It certainly needs to be somehow integrated with the tools using .pgpass. Do you have something particular in mind? While libsecret may look like a good choice, it kinda requires Gnome or KDE (or some other desktop environment supporting it) running, as it's just a proxy to the services provides by these environments. I'd bet most server installations won't have that installed, and in such cases it's pointless. Maybe it can be forwarded to the original machine somehow (something like what 'ssh -A' does), I'm not sure. I would prefer something self-contained, not requiring a lot of other stuff installed. What I envisioned is a simple wallet (basically encrypted .pgpass) with a simple management command-line tool. Let's call that 'pgpass', with these options pgpass list pgpass add pgpass rm I'm fully aware that writing a good / reliable / secure tool for storing passwords is tricky, and if there's something implemented and usable, let's use that. I'm also wondering how well will the existing solutions support the host/database/user/password model, with wildcards for some of the fields. I'd guess most of them use simple username/password pairs. regards Tomas -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] pg_stat_tmp files for dropped databases
Hi, I've noticed that files for dropped databases aren't removed from pg_stat_tmp. After a cluster-wide VACUUM ANALYSE, and restarting Postgres, all the old database pg_stat_tmp files remain. Shouldn't these be cleaned up? -- Thom
Re: [HACKERS] [review] PostgreSQL Service on Windows does not start if data directory given is relative path
Hi Rajeev, From: "Rajeev rastogi" Changed the patch as per your suggestion. Now only one version of make_absolute_path there defined in src/port/path.c Found one small memory leak in the existing function make_absolute_path(miscinit.c), fixed the same. Thanks for refactoring. I confirmed that the revised patch applies to HEAD cleanly, the source files built without extra warnings, and the original intended problem was solved. Please make small cosmetic changes so that make_absolute_path() follows the style of other parts. Then I'll make this ready for committer. (1) Add the function name in the comment as in: /* * make_absolute_path * * ...existing function descripton */ (2) Add errno description as in: fprintf(stderr, _("could not get current working directory: %s\n", strerror(errno))); Regards MauMau -- 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_stat_tmp files for dropped databases
Hi, On 22.2.2014 01:13, Thom Brown wrote: > Hi, > > I've noticed that files for dropped databases aren't removed from > pg_stat_tmp. After a cluster-wide VACUUM ANALYSE, and restarting > Postgres, all the old database pg_stat_tmp files remain. > > Shouldn't these be cleaned up? Yeah, that's a bug in pgstat_recv_dropdb() - instead of building path to the temporary file (in pg_stat_tmp), it builds a path to the permanent file in pg_stat. The permanent files don't exist at that moment, as the postmaster is still runnig and only writes them at shutdown. So the unlink deletes nothing (i.e. returns ENOENT), but the return value is ignored for all the unlink() in pgstat.c. Patch for master attached, needs to be backpatched to 9.3. I'd bet the files survived restart because pg_stat_tmp was kept between the restart. Postmaster walks through all databases, and moves their stats to 'pg_stat' (i.e. removes them from pg_stat_tmp). On the start it does the opposite (move pg_stat -> pg_stat_tmp). But it does not clear the pg_stat_tmp directory, i.e. the files will stay there until removed manually. IIRC it was implemented like this on purpose (what if someone pointed pg_stat_tmp to directory with other files) - only the ownership etc. is checked. So this is expected. Assuming you have just stats in the directory, it's safe to remove the contents of pg_stat_tmp before starting up the database. On systems having pg_stat_tmp pointed to tmpfs, this is basically what happens when rebooting the machine (and why I haven't noticed this on our production systems so far). regards Tomas diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c index 305d126..7c075ef 100644 --- a/src/backend/postmaster/pgstat.c +++ b/src/backend/postmaster/pgstat.c @@ -4744,7 +4744,7 @@ pgstat_recv_dropdb(PgStat_MsgDropdb *msg, int len) { char statfile[MAXPGPATH]; - get_dbstat_filename(true, false, dbid, statfile, MAXPGPATH); + get_dbstat_filename(false, false, dbid, statfile, MAXPGPATH); elog(DEBUG2, "removing %s", statfile); unlink(statfile); -- 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] Uninterruptable regexp_replace in 9.3.1 ?
I wrote: > Craig Ringer writes: >> So I'd like to confirm that this issue doesn't affect 9.1. > It doesn't. I suspect it has something to do with 173e29aa5 or one > of the nearby commits in backend/regex/. Indeed, git bisect fingers that commit as introducing the problem. What seems to be happening is that citerdissect() is trying some combinatorially large number of ways to split the input string into substrings that can satisfy the argument of the outer "+" iterator. It keeps failing on the substring starting with the first '$', and then vainly looking for other splits that dodge the problem. I'm not entirely sure how come the previous coding didn't fall into the same problem. It's quite possible Henry Spencer is smarter than I am, but there was certainly nothing there before that was obviously avoiding this hazard. Worthy of note is that I think pre-9.2 is actually giving the wrong answer --- it's claiming the whole string matches the regex, which it does not if I'm reading it right. This may be related to the problem that commit 173e29aa5 was trying to fix, ie failure to enforce backref matches in some cases. So one possible theory is that by failing to notice that it *didn't* have a valid match, the old code accidentally failed to go down the rabbit hole of trying zillions of other ways to match. 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] pg_stat_tmp files for dropped databases
On 22 February 2014 01:07, Tomas Vondra wrote: > Hi, > > On 22.2.2014 01:13, Thom Brown wrote: > > Hi, > > > > I've noticed that files for dropped databases aren't removed from > > pg_stat_tmp. After a cluster-wide VACUUM ANALYSE, and restarting > > Postgres, all the old database pg_stat_tmp files remain. > > > > Shouldn't these be cleaned up? > > Yeah, that's a bug in pgstat_recv_dropdb() - instead of building path to > the temporary file (in pg_stat_tmp), it builds a path to the permanent > file in pg_stat. > > The permanent files don't exist at that moment, as the postmaster is > still runnig and only writes them at shutdown. So the unlink deletes > nothing (i.e. returns ENOENT), but the return value is ignored for all > the unlink() in pgstat.c. > > Patch for master attached, needs to be backpatched to 9.3. > > I'd bet the files survived restart because pg_stat_tmp was kept between > the restart. Postmaster walks through all databases, and moves their > stats to 'pg_stat' (i.e. removes them from pg_stat_tmp). On the start it > does the opposite (move pg_stat -> pg_stat_tmp). > > But it does not clear the pg_stat_tmp directory, i.e. the files will > stay there until removed manually. IIRC it was implemented like this on > purpose (what if someone pointed pg_stat_tmp to directory with other > files) - only the ownership etc. is checked. So this is expected. > > Assuming you have just stats in the directory, it's safe to remove the > contents of pg_stat_tmp before starting up the database. On systems > having pg_stat_tmp pointed to tmpfs, this is basically what happens when > rebooting the machine (and why I haven't noticed this on our production > systems so far). > Thanks for confirming and for finding the explanation. -- Thom
Re: [HACKERS] Storing the password in .pgpass file in an encrypted format
On Fri, Feb 21, 2014 at 10:18 PM, Daniel Farina wrote: > I'm still in interested in this idea and haven't found a good reason > to rescind the general thinking there. It's an interesting idea. I wonder if it would be possible to make it compatible with existing tools like ssh-agent instead of inventing our own? -- greg -- 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] Storing the password in .pgpass file in an encrypted format
On 02/21/2014 03:54 PM, Tomas Vondra wrote: > Depends on how you define external utility. It certainly needs to be > somehow integrated with the tools using .pgpass. Do you have something > particular in mind? Yeah, I was thinking that the ideal would to be to make this generically pluggable, like giving the ability to use a unix socket or executable call for pgpass instead of only looking at a file. I don't think we should implement any particular wallet technology, just make it possible to call an external application. I think implementing our own wallet would be a big mistake. I'm not sure how broad the actual use case for this is -- most folks with sophisticated password needs use AD or LDAP -- but if someone wants to write the code, I'd be for accepting it. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.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] Storing the password in .pgpass file in an encrypted format
On Fri, Feb 21, 2014 at 6:15 PM, Greg Stark wrote: > On Fri, Feb 21, 2014 at 10:18 PM, Daniel Farina wrote: >> I'm still in interested in this idea and haven't found a good reason >> to rescind the general thinking there. > > It's an interesting idea. I wonder if it would be possible to make it > compatible with existing tools like ssh-agent instead of inventing our > own? I don't understand what you mean: the aesthetic of that proposal was to act as pure delegation insomuch as possible to integrate with other programs, and the supplementary programs provided that I wrote just for the purposes of demonstration are short. (https://github.com/fdr/pq-resolvers, if you want to read the program texts) -- 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] walsender doesn't send keepalives when writes are pending
On Fri, Feb 21, 2014 at 7:10 PM, Andres Freund wrote: > On 2014-02-21 19:03:29 +0530, Amit Kapila wrote: >> On Fri, Feb 21, 2014 at 2:36 PM, Andres Freund >> wrote: >> > Well, especially on a pipelined connection, that can take a fair >> > bit. TCP timeouts aren't fun. >> >> Okay, but the behaviour should be same for both keepalive message >> and wal data it needs to send. What I mean to say here is that if n/w >> is down, wal sender will detect it early by sending some data (either >> keepalive or wal data). Also the ping is sent only after >> wal_sender_timeout/2 w.r.t last reply time and wal data will be >> sent after each sleeptime (1 + wal_sender_timeout/10) which >> I think is should be lesser than the time to send ping. > > The danger is rather that *no* keepalive is sent (with requestReply = > true triggering a reply by the client) by the walsender. Try to run > pg_receivexlog against a busy server with a low walsender timeout. Since > we'll never get to sending a keepalive we'll not trigger a reply by the > receiver. Now Looking at code of pg_receivexlog in function HandleCopyStream(), it seems that it can only happen if user has not configured --status-interval in pg_receivexlog. Code referred is as below: HandleCopyStream() { .. /* * Potentially send a status message to the master */ now = localGetCurrentTimestamp(); if (still_sending && standby_message_timeout > 0 && localTimestampDifferenceExceeds(last_status, now, standby_message_timeout)) { /* Time to send feedback! */ if (!sendFeedback(conn, blockpos, now, false)) goto error; last_status = now; } Even if this is not happening due to some reason, shouldn't this be anyway the responsibility of pg_receivexlog to send the status from time to time rather than sending when server asked for it? With Regards, Amit Kapila. 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] [bug fix] postgres.exe fails to start on Windows Server 2012 due to ASLR
From: "Magnus Hagander" Does somebody want to look at backpatching this to 9.1 and earlier, or should we just say that it's not fully supported on those Windows versions unless you apply the registry workaround? Please use the attached patch. It applies cleanly to both 9.1 and 9.0. We don't need to consider 8.4, because ASLR became enabled by default in Visual Studio 2008 and 8.4 doesn't support building with 2008. I tested with Visual Studio 2008 Express. You can check if ASLR is disabled using dumpbin. "dumpbin /headers " outputs the following lines. If the "Dynamic base" line is displayed, ASLR is enabled. If this line disappears, ASLR is disabled. 8140 DLL characteristics Dynamic base NX compatible Terminal Server Aware Regards MauMau disable_ASLR_pg90_91.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] Patch: show relation and tuple infos of a lock to acquire
On Fri, Feb 21, 2014 at 4:55 PM, Christian Kruse wrote: > Hi, >> 1. New context added by this patch is display at wrong place >> [...] >> Do this patch expect to print the context at cancel request >> as well? > > To be honest, I don't see a problem here. If you cancel the request in > most cases it is because it doesn't finish in an acceptable time. So > displaying the context seems logical to me. Isn't it a matter of consistency, we might not be able to display it on all long running updates, rather only when it goes for update on tuple on which some other transaction is operating. Also though this functionality is mainly going to be useful for the case when log_lock_waits = on, but it will display newly added context even without that. Is it possible that we set callback to error_context_stack, only when we go for displaying this message. The way to do could be that rather than forming callback in local variable in fuction XactLockTableWaitWithErrorCallback(), store it in global variable and then use that at appropriate place to set error_context_stack. In general, why I am suggesting to restrict display of newly added context for the case it is added to ensure that it doesn't get started displaying in other cases where it might make sense or might not make sense. I think if it is too tedious to do it or there are some problems due to which we cannot restrict it for case it has been added, then we can think of going with it. >> 2a. About the message: >> LOG: process 1936 still waiting for ShareLock on transaction 842 >> after 1014.000ms >> CONTEXT: relation name: foo (OID 57496) >> tuple (ctid (0,1)): (1, 2, abc ) >> >> Isn't it better to display database name as well, as if we see in >> one of related messages as below, it displays database name as well. >> >> "LOG: process 3012 still waiting for ExclusiveLock on tuple (0,1) of >> relation 57 >> 499 of database 12045 after 1014.000 ms" > > Maybe. But then we either have duplicate infos in some cases (e.g. on > a table lock) or we have to distinguish error cases and show distinct > contextes. I think if we go with the way suggested above, then this should not be problem, or do you think it can still create duplicate info problem. > And also getting the database name from a relation is not > really straight forward I think there should not be any complication, we can do something like get_database_name(rel->rd_node.dbNode); > (according to Andres we would have to look at > rel->rd_node.dbNode) and since other commands dealing with names don't > do so, either, I think we should leave out the database name. For this case as I have shown that in similar message, we already display database oid, it should not be a problem to display here as well. It will be more information to user. >> 2b. I think there is no need to display *ctid* before tuple offset, it seems >> to be obvious as well as consistent with other similar message. > > Ok, I'll drop it. I have asked to just remove *ctid* from info not the actual info of tuple location. It should look like tuple (0,1). The way you have currently changed the code, it is crashing the backend. Another thing related to this is that below code should also not use *ctid* + if (geterrlevel() >= ERROR) + { + errcontext("tuple ctid (%u,%u)", + BlockIdGetBlockNumber(&(info->tuple->t_self.ip_blkid)), + info->tuple->t_self.ip_posid); + } >> 3. About callback I think rather than calling setup/tear down >> functions from multiple places, it will be better to have wrapper >> functions for XactTableLockWait() and MultiXactIdWait(). Just check >> in plpgsql_exec_function(), how the errorcallback is set, can we do >> something similar without to avoid palloc? > > OK, Attached. It looks better than previous, one minor suggestion: Function name XactLockTableWaitWithErrorCallback() looks bit awkward to me, shall we name something like XactLockTableWaitWithInfo()? >> 4. About the values of tuple >> I think it might be useful to display some unique part of tuple for >> debugging part, but what will we do incase there is no primary >> key on table, so may be we can display initial few columns (2 or 3) >> or just display tuple offset as is done in other similar message >> shown above. > > Currently I simply display the whole tuple with truncating long > fields. This is plain easy, no distinction necessary. I did some code > reading and it seems somewhat complex to get the PK columns and it > seems that we need another lock for this, too - maybe not the best > idea when we are already in locking trouble, do you disagree? I don't think you need to take another lock for this, it must already have appropriate lock by that time. There should not be any problem in calling relationHasPrimaryKey except that currently it is static, you need to expose it. > Also showing just a few columns when no PK is available might not be > helpful since the tuple is not necessarily identified by the columns > showed. And since we