Re: [HACKERS] inherit support for foreign tables

2014-02-21 Thread Kyotaro HORIGUCHI
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

2014-02-21 Thread Andres Freund
On 2014-02-21 10:08:44 +0530, Amit Kapila wrote:
 On Fri, Feb 14, 2014 at 5:35 PM, Andres Freund and...@2ndquadrant.com 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 ?

2014-02-21 Thread Sandro Santilli
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 ?

2014-02-21 Thread Sandro Santilli
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.

2014-02-21 Thread Ronan Dunklau
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 ival	defacl_privilege_target
 %type defelt	DefACLOption
 %type list	DefACLOptionList
+%type node	OptImportForeignSchemaRestriction
+%type ival	

Re: [HACKERS] Display oprcode and its volatility in \do+

2014-02-21 Thread Marti Raudsepp
On Thu, Jan 16, 2014 at 5:22 PM, Tom Lane t...@sss.pgh.pa.us 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.

2014-02-21 Thread Atri Sharma
On Fri, Feb 21, 2014 at 4:01 PM, Ronan Dunklau ronan.dunk...@dalibo.comwrote:

 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

2014-02-21 Thread Andres Freund
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.

2014-02-21 Thread Ronan Dunklau
 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

2014-02-21 Thread Andres Freund
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

2014-02-21 Thread Pavel Raiskup
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.

2014-02-21 Thread Atri Sharma
On Fri, Feb 21, 2014 at 4:39 PM, Ronan Dunklau ronan.dunk...@dalibo.comwrote:

  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

2014-02-21 Thread Andres Freund
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 ptr as uint32), 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.

2014-02-21 Thread Ronan Dunklau
Le vendredi 21 février 2014 16:45:20 Atri Sharma a écrit :
 On Fri, Feb 21, 2014 at 4:39 PM, Ronan Dunklau 
ronan.dunk...@dalibo.comwrote:
   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

2014-02-21 Thread Christian Kruse
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);
@@ -2703,8 +2705,8 @@ l1:
 		if (infomask  

Re: [HACKERS] Proposal: IMPORT FOREIGN SCHEMA statement.

2014-02-21 Thread Atri Sharma
On Fri, Feb 21, 2014 at 4:56 PM, Ronan Dunklau ronan.dunk...@dalibo.comwrote:

 Le vendredi 21 février 2014 16:45:20 Atri Sharma a écrit :
  On Fri, Feb 21, 2014 at 4:39 PM, Ronan Dunklau
 ronan.dunk...@dalibo.comwrote:
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

2014-02-21 Thread Andres Freund
On 2014-02-19 13:07:11 -0500, Robert Haas wrote:
 On Tue, Feb 18, 2014 at 4:07 AM, Andres Freund and...@2ndquadrant.com 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 snapshot, LSN 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 name LOGICAL
copy data using the exported snapshot
START_REPLICATION SLOT name 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.

2014-02-21 Thread Florian Pflug
On Feb21, 2014, at 12:09 , Ronan Dunklau ronan.dunk...@dalibo.com 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.

2014-02-21 Thread Ronan Dunklau

 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

2014-02-21 Thread firoz e v
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

2014-02-21 Thread Andres Freund
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

2014-02-21 Thread Szymon Guz
On 21 February 2014 13:49, firoz e v firoz...@huawei.com 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

2014-02-21 Thread Robert Haas
On Fri, Feb 21, 2014 at 6:07 AM, Andres Freund and...@2ndquadrant.com 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

2014-02-21 Thread Andres Freund
On 2014-02-21 08:16:59 -0500, Robert Haas wrote:
 On Fri, Feb 21, 2014 at 6:07 AM, Andres Freund and...@2ndquadrant.com 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

2014-02-21 Thread Amit Kapila
On Fri, Feb 21, 2014 at 2:36 PM, Andres Freund and...@2ndquadrant.com 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

2014-02-21 Thread Andres Freund
On 2014-02-21 19:03:29 +0530, Amit Kapila wrote:
 On Fri, Feb 21, 2014 at 2:36 PM, Andres Freund and...@2ndquadrant.com 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

2014-02-21 Thread Robert Haas
On Thu, Feb 20, 2014 at 12:07 PM, Alvaro Herrera
alvhe...@2ndquadrant.com 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

2014-02-21 Thread Robert Haas
On Fri, Feb 21, 2014 at 8:27 AM, Andres Freund and...@2ndquadrant.com wrote:
 On 2014-02-21 08:16:59 -0500, Robert Haas wrote:
 On Fri, Feb 21, 2014 at 6:07 AM, Andres Freund and...@2ndquadrant.com 
 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

2014-02-21 Thread Alvaro Herrera
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

2014-02-21 Thread Andres Freund
On 2014-02-21 08:51:03 -0500, Robert Haas wrote:
 On Fri, Feb 21, 2014 at 8:27 AM, Andres Freund and...@2ndquadrant.com wrote:
  On 2014-02-21 08:16:59 -0500, Robert Haas wrote:
  On Fri, Feb 21, 2014 at 6:07 AM, Andres Freund and...@2ndquadrant.com 
  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

2014-02-21 Thread Hadi Moshayedi
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

2014-02-21 Thread Alvaro Herrera
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

2014-02-21 Thread Christian Kruse
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

2014-02-21 Thread Euler Taveira
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

2014-02-21 Thread Tom Lane
Hadi Moshayedi h...@moshayedi.net 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

2014-02-21 Thread Alvaro Herrera
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

2014-02-21 Thread Andres Freund
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 ?

2014-02-21 Thread Craig Ringer
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
  cdissect

Re: [HACKERS] Storing the password in .pgpass file in an encrypted format

2014-02-21 Thread Christopher Browne
On Fri, Feb 21, 2014 at 7:49 AM, firoz e v firoz...@huawei.com 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

2014-02-21 Thread John Williams
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 ?

2014-02-21 Thread Florian Pflug
On Feb21, 2014, at 16:46 , Craig Ringer cr...@2ndquadrant.com 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

2014-02-21 Thread Jeff Janes
On Fri, Feb 21, 2014 at 7:04 AM, Alvaro Herrera alvhe...@2ndquadrant.comwrote:

 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

2014-02-21 Thread Florian Pflug
On Feb21, 2014, at 13:44 , John Williams jdwilliams1...@gmail.com 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

2014-02-21 Thread Alvaro Herrera
Jeff Janes escribió:
 On Fri, Feb 21, 2014 at 7:04 AM, Alvaro Herrera 
 alvhe...@2ndquadrant.comwrote:

   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

2014-02-21 Thread Stephen Frost
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 ?

2014-02-21 Thread Craig Ringer
On 02/22/2014 12:04 AM, Florian Pflug wrote:
 On Feb21, 2014, at 16:46 , Craig Ringer cr...@2ndquadrant.com 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

2014-02-21 Thread Craig Ringer
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.comwrote:
 
  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

2014-02-21 Thread Craig Ringer
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 ?

2014-02-21 Thread Florian Pflug
On Feb21, 2014, at 17:29 , Craig Ringer cr...@2ndquadrant.com 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 ?

2014-02-21 Thread Tom Lane
Craig Ringer cr...@2ndquadrant.com 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

2014-02-21 Thread Tomas Vondra
Hi,

On 21 Únor 2014, 16:52, Christopher Browne wrote:
 On Fri, Feb 21, 2014 at 7:49 AM, firoz e v firoz...@huawei.com 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

2014-02-21 Thread Jeff Janes
On Fri, Feb 21, 2014 at 8:42 AM, Craig Ringer cr...@2ndquadrant.com 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.comwrote:
 
   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

2014-02-21 Thread Alvaro Herrera
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

2014-02-21 Thread Jim Nasby

On 1/16/14, 9:19 PM, Craig Ringer wrote:

On 01/16/2014 11:52 PM, Tom Lane wrote:

Heikki Linnakangas hlinnakan...@vmware.com 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

2014-02-21 Thread Jim Nasby

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

2014-02-21 Thread Daniel Farina
On Fri, Feb 21, 2014 at 10:42 AM, Alvaro Herrera
alvhe...@2ndquadrant.com 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

2014-02-21 Thread Tom Lane
Noah Misch n...@leadboat.com 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

2014-02-21 Thread Josh Berkus
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

2014-02-21 Thread Noah Misch
On Fri, Feb 21, 2014 at 05:20:06PM -0500, Tom Lane wrote:
 Noah Misch n...@leadboat.com 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

2014-02-21 Thread Tomas Vondra
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

2014-02-21 Thread Thom Brown
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

2014-02-21 Thread MauMau

Hi Rajeev,

From: Rajeev rastogi rajeev.rast...@huawei.com
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

2014-02-21 Thread Tomas Vondra
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 ?

2014-02-21 Thread Tom Lane
I wrote:
 Craig Ringer cr...@2ndquadrant.com 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

2014-02-21 Thread Thom Brown
On 22 February 2014 01:07, Tomas Vondra t...@fuzzy.cz 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

2014-02-21 Thread Greg Stark
On Fri, Feb 21, 2014 at 10:18 PM, Daniel Farina dan...@heroku.com 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

2014-02-21 Thread Josh Berkus
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

2014-02-21 Thread Daniel Farina
On Fri, Feb 21, 2014 at 6:15 PM, Greg Stark st...@mit.edu wrote:
 On Fri, Feb 21, 2014 at 10:18 PM, Daniel Farina dan...@heroku.com 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

2014-02-21 Thread Amit Kapila
On Fri, Feb 21, 2014 at 7:10 PM, Andres Freund and...@2ndquadrant.com wrote:
 On 2014-02-21 19:03:29 +0530, Amit Kapila wrote:
 On Fri, Feb 21, 2014 at 2:36 PM, Andres Freund and...@2ndquadrant.com 
 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

2014-02-21 Thread MauMau

From: Magnus Hagander mag...@hagander.net

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 EXE_or_DLL_file 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

2014-02-21 Thread Amit Kapila
On Fri, Feb 21, 2014 at 4:55 PM, Christian Kruse
christ...@2ndquadrant.com 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 drop the CTID there is no alternative solution to
 identify the tuple.