Re: [HACKERS] Fwd: [GENERAL] 4B row limit for CLOB tables

2015-04-25 Thread Bruce Momjian
On Fri, Apr 24, 2015 at 11:39:04PM -0500, Jim Nasby wrote:
 On 4/24/15 7:11 PM, Álvaro Hernández Tortosa wrote:
 On 24/04/15 05:24, Tom Lane wrote:
 ...
 TBH, I've got very little enthusiasm for fixing this given the number
 of reports of trouble from the field, which so far as I recall is zero.
 Álvaro's case came up through intentionally trying to create an
 unreasonable number of tables, not from real usage.  This thread likewise
 appears to contain lots of speculation and no reports of anyone hitting
 a problem in practice.
 
  It is certainly true that this was a very synthetic case. I
 envision, however, certain use cases where we may hit a very large
 number of tables:
 
 The original case has NOTHING to do with the number of tables and
 everything to do with the number of toasted values a table can have.
 If you have to toast 4B attributes in a single relation it will
 fail. In reality, if you get anywhere close to that things will fall
 apart due to OID conflicts.
 
 This case isn't nearly as insane as 4B tables. A table storing 10
 text fields each of which is 2K would hit this limit with only 400M
 rows. If my math is right that's only 8TB; certainly not anything
 insane space-wise or rowcount-wise.
 
 Perhaps it's still not fixing, but I think it's definitely worth
 documenting.

And it is now documented in the Postgres FAQ thanks to 'Rogerdpack',
which is where that maximum table came from:


https://wiki.postgresql.org/wiki/FAQ#What_is_the_maximum_size_for_a_row.2C_a_table.2C_and_a_database.3F

Note if you are storing a table with rows that exceed 2KB in size
(aggregate size of each row) then the Maximum number of rows in a
table may be limited to 4 Billion, see TOAST. 

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +


-- 
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] Allow SQL/plpgsql functions to accept record

2015-04-25 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Wed, Apr 22, 2015 at 6:12 PM, Jim Nasby jim.na...@bluetreble.com wrote:
 As for allowing SQL and plpgsql functions to accept a record, I think our
 JSON functionality just provided plenty of reason we should allow accepting
 them, even if you can't do much with it: you *can* hand it to row_to_json(),
 which does allow you to do something useful with it. So it seems reasonable
 to me that we should at least accept it as a function argument.

 I agree that that would be useful.  I think the problem with an
 expression like rowvar.something is that PL/pgsql cannot infer the
 type of the result, and nothing else works without that.  I doubt that
 it's practical to lift that restriction.

Well, we already support local variables of type RECORD in plpgsql, so
it's not immediately clear to me that function arguments would be much
worse.  There are a lot of deficiencies with the RECORD-local-variable
implementation: if you try to change the actual RECORD type from one call
to the next you'll probably have problems.  But it seems like we could
avoid that for function arguments by treating RECORD as a polymorphic
argument type, and thereby generating a separate set of plan trees for
each actual record type passed to the function within a given session.
So in principle it ought to work better than the local-variable case does
today.

In short I suspect that Jim is right and this has more to do with a
shortage of round tuits than any fundamental problem.

Not sure about the SQL-function case.  That might be even easier because
functions.c doesn't try to cache plans across queries; or maybe not.

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] INSERT ... ON CONFLICT syntax issues

2015-04-25 Thread Andres Freund
On 2015-04-25 11:50:59 -0700, Peter Geoghegan wrote:
 On Sat, Apr 25, 2015 at 11:24 AM, Andres Freund and...@anarazel.de wrote:
   c) Right now the UPDATE can refer to pseudo relations 'TARGET' and
  'EXCLUDED'. I think especially the latter doesn't fit anymore at
  all. How about 'CONFLICTING' and 'EXISTING'? Or even NEW and OLD?
 
  NEW and OLD are terribly misleading, since surely the NEW tuple is the
  one actually appended to the relation by the UPDATE, and the OLD one
  is the existing one (not the excluded one). Plus they have all that
  intellectual baggage from rules.
 
  What 'intellectual baggage' would that be? That they're already known to
  have been used in another place? I don't see the problem.
 
 The problem is that they make you think of rules, and they don't
 describe what's going on at all.

95% of all users will know NEW/OLD from triggers, not rules. Where NEW
is used in a quite comparable way.

  Seems pretty descriptive of the situation to me - I actually put a lot
  of thought into this. Additionally, the word is widely understood by
  non-native speakers. TARGET is also very descriptive, because it
  situationally describes either the existing tuple actually present in
  the table, or (from a RETURNING clause) the final tuple present in the
  table post-UPDATE. We use the term target for that pervasively (in
  the docs and in the code).
 
  Sorry, I don't buy either argument. EXISTING and NEW would surely at
  least as widely understood than EXCLUDE and TARGET. The latter does just
  about no sense to me; especially from a user POV. I don't think the
  existing usage of the term has much to do what it's used for here.
 
 Yes it does. The UPDATE docs refer to the target table in a way
 intended to distinguish it from any joined-to table (FROM table). It's
 clear as day.

Which means the term is used in a different way for INSERTs and UPDATEs
already.  To me it sounds like it's a remnant of your earlier syntax
proposal for UPSERT.

 Maybe EXISTING is equally well understood as a word in general, but
 it's way more ambiguous than EXCLUDED is here.

What? I'm not suggesting to replace EXCLUDED by EXISTING - that'd make
absolutely no sense. My suggesting is to have NEW refer to the tuple
specified in the INSERT and EXISTING to the, well, pre existing tuple
that the conflict is with.

  That
  it has 'morphing' characteristics imo just makes it worse, rather than
  better. Besides being confusing that it has different meanings, it's far
  from inconceivable that somebody wants to return values from the
  preexisting, new, and merged rows.
 
 This is how RETURNING works from UPDATEs in general.

And there's been a patch (which unfortunately died because it's
implementation wasn't good), to allow referring to the other versions of
the tuple. It has been wished for numerous times.


 IOW, if you do an UPDATE FROM (which is pretty similar to ON CONFLICT
 UPDATE, syntax-wise), then you can only refer to the joined table's
 tuple and the final post-update tuple from within RETURNING.

 You cannot refer to the pre-UPDATE target tuple there either -- it's
 *exactly* the same situation. Why should it be any different here? The
 situational/morphing characteristic of the alias name TARGET is
 therefore absolutely appropriate, in that it follows UPDATE.

Contrasting
 TARGET is also very descriptive, because it
 situationally describes either the existing tuple actually present in
 the table, or (from a RETURNING clause) the final tuple present in the
 table post-UPDATE. We use the term target for that pervasively (in
 the docs and in the code).

the docs say:
   Since
   literalRETURNING/ is not part of the commandUPDATE/
   auxiliary query, the special literalON CONFLICT UPDATE/ aliases
   (varnameTARGET/ and varnameEXCLUDED/) may not be
   referenced;  only the row as it exists after updating (or
   inserting) is returned.

So I don't understand that whole chain of argument. There's no such
morphing behaviour, unless I miss something?

2a5d80b27d2c5832ad26dde4651c64dd2004f401:
 The problem with this seems to be that it more or less
 necessitates making both IGNORE and UPDATE fully reserved keywords in
 order to avoid an ambiguity, which we prefer not to do

It does not. As mentioned in the thread DO UPDATE/NOTHING work without
anything like that.

Greetings,

Andres Freund


-- 
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] Shouldn't CREATE TABLE LIKE copy the relhasoids property?

2015-04-25 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 No, I just misread your email.  I thought you said you had attached
 the patch; rereading it, I see that you said you had applied the
 patch.  Silly me.

The real problem with this patch is it's wrong.  Specifically, it broke
the other case I mentioned in my original email:

regression=# create table src2 (f1 int, primary key(oid)) with oids;
ERROR:  column oid named in key does not exist
LINE 1: create table src2 (f1 int, primary key(oid)) with oids;
   ^

That works in 9.4, and was still working in HEAD as of my original email.
I think the patch's logic for attaching made-up OIDS options is actually
backwards (it's adding TRUE where it should add FALSE and vice versa),
but in any case I do not like the dependence on default_with_oids that
was introduced by the patch.  I am not sure there's any guarantee that
default_with_oids can't change between parsing and execution of a CREATE
TABLE command.

Apparently we need a few more regression tests in this area.  In the
meantime I suggest reverting and rethinking the patch.

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] Shouldn't CREATE TABLE LIKE copy the relhasoids property?

2015-04-25 Thread Bruce Momjian
On Sat, Apr 25, 2015 at 06:15:25PM -0400, Tom Lane wrote:
 Robert Haas robertmh...@gmail.com writes:
  No, I just misread your email.  I thought you said you had attached
  the patch; rereading it, I see that you said you had applied the
  patch.  Silly me.
 
 The real problem with this patch is it's wrong.  Specifically, it broke
 the other case I mentioned in my original email:
 
 regression=# create table src2 (f1 int, primary key(oid)) with oids;
 ERROR:  column oid named in key does not exist
 LINE 1: create table src2 (f1 int, primary key(oid)) with oids;
^

Wow, thanks for seeing that mistake.  I had things just fine, but then I
decided to optimize it and forgot that this code is used in non-LIKE
situations.  Reverted.

 That works in 9.4, and was still working in HEAD as of my original email.
 I think the patch's logic for attaching made-up OIDS options is actually
 backwards (it's adding TRUE where it should add FALSE and vice versa),
 but in any case I do not like the dependence on default_with_oids that
 was introduced by the patch.  I am not sure there's any guarantee that
 default_with_oids can't change between parsing and execution of a CREATE
 TABLE command.

I have changed the default value back to the function call as it should
have been all along;  patch attached.  I will revisit this for 9.6
unless I hear otherwise.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
new file mode 100644
index 1fc8c2c..40fa9d6
*** a/src/backend/parser/parse_utilcmd.c
--- b/src/backend/parser/parse_utilcmd.c
***
*** 56,61 
--- 56,62 
  #include rewrite/rewriteManip.h
  #include utils/acl.h
  #include utils/builtins.h
+ #include utils/guc.h
  #include utils/lsyscache.h
  #include utils/rel.h
  #include utils/syscache.h
*** transformCreateStmt(CreateStmt *stmt, co
*** 281,286 
--- 282,298 
  	 * Output results.
  	 */
  	stmt-tableElts = cxt.columns;
+ 	/*
+ 	 * Add WITH/WITHOUT OIDS, if necessary.  A literal statement-specified
+ 	 * WITH/WITHOUT OIDS will still take precedence because the first
+ 	 * matching oids in options is used.
+ 	 */
+ 	if (!interpretOidsOption(stmt-options, true)  cxt.hasoids)
+ 		stmt-options = lappend(stmt-options, makeDefElem(oids,
+ (Node *)makeInteger(TRUE)));
+ 	else if (interpretOidsOption(stmt-options, true)  !cxt.hasoids)
+ 		stmt-options = lappend(stmt-options, makeDefElem(oids,
+ (Node *)makeInteger(FALSE)));
  	stmt-constraints = cxt.ckconstraints;
  
  	result = lappend(cxt.blist, stmt);
*** transformTableLikeClause(CreateStmtConte
*** 849,854 
--- 861,868 
  		}
  	}
  
+ 	cxt-hasoids = relation-rd_rel-relhasoids;
+ 
  	/*
  	 * Copy CHECK constraints if requested, being careful to adjust attribute
  	 * numbers so they match the child.

-- 
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] INSERT ... ON CONFLICT syntax issues

2015-04-25 Thread Peter Geoghegan
On Sat, Apr 25, 2015 at 2:01 AM, Andres Freund and...@anarazel.de wrote:
 My problem with the WHERE being inside the parens in the above is that
 it's
 a) different from CREATE INDEX

I don't think that that's an important goal.

 b) unclear whether the WHERE belongs to colb or the whole index
expression. The equivalent for aggregates, which I bet is going to be
used less often, caused a fair amount of confusing.

I don't see those two situations as being comparable. The inference
specification does not accept aggregates. It seems obvious to me that
the predicate only ever applies to the entire table. And it's obvious
that it's part of the inference specification because it appears in
parentheses with everything else - otherwise, *users* might find this
phantom WHERE clause ambiguous/confusing.

 But I'm generally having some doubts about the syntax.

 Right now it's
 INSERT ... ON CONFLICT opt_on_conf_clause UPDATE|IGNORE.

 A couple things:

 a) Why is is 'CONFLICT? We're talking about a uniquness violation. What
if we, at some later point, also want to handle other kind of
violations? Shouldn't it be ON UNIQUE CONFLICT/ERROR/VIOLATION ...

I think that naming unique violations alone would be wrong (not to
mention ludicrously verbose). Heikki and I both feel that the CONFLICT
keyword captures the fact that this could be a dup violation, or an
exclusion violation. The syntax has been like this for some time, and
hasn't been a point of contention for a long time, so I thought this
was settled. Note that the syntax is quite similar to the SQLite
syntax of the same feature, that has ON CONFLICT IGNORE (it also has
ON CONFLICT REPLACE, but not ON CONFLICT UPDATE).

 b) For me there's a WITH before the index inference clause missing, to
have it read in 'SQL' style.

I'm not seeing it. BTW, Robert was the one who initially proposed that
the unique index inference clause follow this exact style (albeit
before it accepted a WHERE clause to infer partial indexes, which was
only added a couple of months ago).

 c) Right now the UPDATE can refer to pseudo relations 'TARGET' and
'EXCLUDED'. I think especially the latter doesn't fit anymore at
all. How about 'CONFLICTING' and 'EXISTING'? Or even NEW and OLD?

NEW and OLD are terribly misleading, since surely the NEW tuple is the
one actually appended to the relation by the UPDATE, and the OLD one
is the existing one (not the excluded one). Plus they have all that
intellectual baggage from rules.

CONFLICTING, as Greg Stark pointed out many months ago, is something
that applies to both tuples that can be referenced, which is why I
*stopped* using it months ago. They conflict with *each other*. Any
conflict must pertain to both.

Dictionary.com defines exclude as:


verb (used with object), excluded, excluding.
1.
to shut or keep out; prevent the entrance of.
2.
to shut out from consideration, privilege, etc.:
Employees and their relatives were excluded from participation in the contest.
3.
to expel and keep out; thrust out; eject:
He was excluded from the club for infractions of the rules.


Seems pretty descriptive of the situation to me - I actually put a lot
of thought into this. Additionally, the word is widely understood by
non-native speakers. TARGET is also very descriptive, because it
situationally describes either the existing tuple actually present in
the table, or (from a RETURNING clause) the final tuple present in the
table post-UPDATE. We use the term target for that pervasively (in
the docs and in the code).

 So I guess it boils down to that I think we should switch the syntax to
 be:

 INSERT ... ON UNIQUE VIOLATION [WITH (cola, colb) WHERE ...] DO 
 {NOTHING|UPDATE}

Beauty is in the eye of the beholder and all, but that seems pretty
ugly to me. Honestly, I think we should just accept that the predicate
appears in the parentheses on the odd occasion that it appears at all
- partial unique indexes are not all that common.

-- 
Peter Geoghegan


-- 
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] PL/pgSQL, RAISE and error context

2015-04-25 Thread Pavel Stehule
Hi

2015-04-24 19:16 GMT+02:00 Joel Jacobson j...@trustly.com:

 On Fri, Apr 24, 2015 at 6:07 PM, Pavel Stehule pavel.steh...@gmail.com
 wrote:
  Example:
 
  context_messages = -warning, -error, +notice
 
 
  I prefer your first proposal - and there is a precedent for plpgsql -
  plpgsql_extra_checks
 
  It is clean for anybody. +-identifiers looks like horrible httpd config.
 :)

 I have to agree on that :) Just thought this is the best we can do if
 we want to reduce the number of GUCs to a minimum.


I played with some prototype and I am thinking so we need only one GUC

plpgsql.display_context_messages = 'none'; -- compatible with current
plpgsql.display_context_messages = 'all';
plpgsql.display_context_messages = 'exception, log'; -- what I prefer

I implemented [ (WITH|WITHOUT) CONTEXT ] clause for RAISE statement

RAISE NOTICE WITH CONTEXT 'some message';
RAISE NOTICE WITH CONTEXT USING message = 'some message';
RAISE EXCEPTION WITHOUT CONTEXT 'other message';

The patch is very small with full functionality (without documentation) - I
am thinking so it can work. This patch is back compatible - and allow to
change default behave simply.

plpgsql.display_context_messages can be simplified to some like
plpgsql.display_context_min_messages

What do you think about it?

Regards

Pavel
commit cf9e23a29162ac55fcab1ac4d9e7a24492de0736
Author: Pavel Stehule pavel.steh...@gooddata.com
Date:   Sat Apr 25 22:09:28 2015 +0200

initial implementation of (WITH|WITHOUT) CONTEXT clause to plpgsql RAISE statement.

initial implementation of plpgsql GUC plpgsql.display_context_messages

diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index deefb1f..ea0dac5 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -2921,6 +2921,7 @@ exec_stmt_raise(PLpgSQL_execstate *estate, PLpgSQL_stmt_raise *stmt)
 	char	   *err_table = NULL;
 	char	   *err_schema = NULL;
 	ListCell   *lc;
+	bool			hide_ctx;
 
 	/* RAISE with no parameters: re-throw current exception */
 	if (stmt-condname == NULL  stmt-message == NULL 
@@ -3080,10 +3081,42 @@ exec_stmt_raise(PLpgSQL_execstate *estate, PLpgSQL_stmt_raise *stmt)
 			err_message = pstrdup(unpack_sql_state(err_code));
 	}
 
-	/*
-	 * Throw the error (may or may not come back)
-	 */
-	estate-err_text = raise_skip_msg;	/* suppress traceback of raise */
+	if (stmt-context_info == PLPGSQL_CONTEXT_DISPLAY)
+		hide_ctx = false;
+	else if (stmt-context_info == PLPGSQL_CONTEXT_SUPPRESS)
+	{
+		hide_ctx = true;
+	}
+	else
+	{
+		/* we display a messages via plpgsql_display_context_messages */
+		switch (stmt-elog_level)
+		{
+			case ERROR:
+hide_ctx = !(plpgsql_display_context_messages  PLPGSQL_DISPLAY_CONTEXT_ERROR);
+break;
+			case WARNING:
+hide_ctx = !(plpgsql_display_context_messages  PLPGSQL_DISPLAY_CONTEXT_WARNING);
+break;
+			case NOTICE:
+hide_ctx = !(plpgsql_display_context_messages  PLPGSQL_DISPLAY_CONTEXT_NOTICE);
+break;
+			case INFO:
+hide_ctx = !(plpgsql_display_context_messages  PLPGSQL_DISPLAY_CONTEXT_INFO);
+break;
+			case LOG:
+hide_ctx = !(plpgsql_display_context_messages  PLPGSQL_DISPLAY_CONTEXT_LOG);
+break;
+			case DEBUG1:
+hide_ctx = !(plpgsql_display_context_messages  PLPGSQL_DISPLAY_CONTEXT_DEBUG);
+break;
+			default:
+elog(ERROR, unexpected RAISE statement level);
+		}
+	}
+
+	if (hide_ctx)
+		estate-err_text = raise_skip_msg;
 
 	ereport(stmt-elog_level,
 			(err_code ? errcode(err_code) : 0,
@@ -3099,7 +3132,8 @@ exec_stmt_raise(PLpgSQL_execstate *estate, PLpgSQL_stmt_raise *stmt)
 			 (err_table != NULL) ?
 			 err_generic_string(PG_DIAG_TABLE_NAME, err_table) : 0,
 			 (err_schema != NULL) ?
-			 err_generic_string(PG_DIAG_SCHEMA_NAME, err_schema) : 0));
+			 err_generic_string(PG_DIAG_SCHEMA_NAME, err_schema) : 0,
+			 errhidecontext(hide_ctx)));
 
 	estate-err_text = NULL;	/* un-suppress... */
 
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index 4026e41..48914a7 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -259,6 +259,7 @@ static	void			check_raise_parameters(PLpgSQL_stmt_raise *stmt);
 %token keyword	K_CONSTANT
 %token keyword	K_CONSTRAINT
 %token keyword	K_CONSTRAINT_NAME
+%token keyword	K_CONTEXT
 %token keyword	K_CONTINUE
 %token keyword	K_CURRENT
 %token keyword	K_CURSOR
@@ -341,6 +342,8 @@ static	void			check_raise_parameters(PLpgSQL_stmt_raise *stmt);
 %token keyword	K_WARNING
 %token keyword	K_WHEN
 %token keyword	K_WHILE
+%token keyword	K_WITH
+%token keyword	K_WITHOUT
 
 %%
 
@@ -1716,6 +1719,7 @@ stmt_raise		: K_RAISE
 		new-cmd_type	= PLPGSQL_STMT_RAISE;
 		new-lineno		= plpgsql_location_to_lineno(@1);
 		new-elog_level = ERROR;	/* default */
+		new-context_info = PLPGSQL_CONTEXT_DEFAULT;
 		new-condname	= NULL;
 		new-message	= NULL;
 		new-params		= NIL;
@@ -1773,6 +1777,21 @@ stmt_raise		: K_RAISE
 			if (tok == 0)
 

Re: [HACKERS] forward vs backward slashes in msvc build code

2015-04-25 Thread Andrew Dunstan


On 04/25/2015 08:01 PM, Michael Paquier wrote:

On Sun, Apr 26, 2015 at 2:19 AM, Andrew Dunstan and...@dunslane.net wrote:

On 04/25/2015 10:53 AM, Michael Paquier wrote:

On Sat, Apr 25, 2015 at 9:58 PM, Peter Eisentraut wrote:

On 4/24/15 12:22 AM, Michael Paquier wrote:

Now that the stuff related to the move from contrib/ to src/bin/,
modulescheck and tmp_install has been committed, shouldn't we give a
new shot at this patch? Attached is a rebased version.

done

Thanks, bowerbird is running fine:

http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=bowerbirddt=2015-04-25%2013%3A31%3A06



But currawong is not - it runs an older version of the MS build tools. See
http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=currawongdt=2015-04-25%2016%3A31%3A12:

Bad format filename 'src/backend/access/brin/brin.c'
  at src/tools/msvc/VCBuildProject.pm line 73.
 VCBuildProject::WriteFiles(VC2008Project=HASH(0x9a7274),
*Project::F) called at src/tools/msvc/Project.pm line 363
 Project::Save(VC2008Project=HASH(0x9a7274)) called at
src/tools/msvc/Solution.pm line 539
 Solution::Save(VS2008Solution=HASH(0xc00b7c)) called at
src/tools/msvc/Mkvcbuild.pm line 656
 Mkvcbuild::mkvcbuild(HASH(0xbed464)) called at build.pl line 36

Oops, attached is a patch that should make currawong happier..


OK, pushed.

cheers

andrew


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] INSERT ... ON CONFLICT syntax issues

2015-04-25 Thread Peter Geoghegan
On Sat, Apr 25, 2015 at 11:24 AM, Andres Freund and...@anarazel.de wrote:
  b) unclear whether the WHERE belongs to colb or the whole index
 expression. The equivalent for aggregates, which I bet is going to be
 used less often, caused a fair amount of confusing.

 I don't see those two situations as being comparable. The inference
 specification does not accept aggregates.

 Huh? It's pretty much entirely besides the point that inference doesn't
 accept aggregates. The point is that ORDER BY for aggregates has
 confused users because it's inside the parens.

Would any alternative cause less confusion? That's the real issue. And
I'm unconvinced that your alternative would.

  a) Why is is 'CONFLICT? We're talking about a uniquness violation. What
 if we, at some later point, also want to handle other kind of
 violations? Shouldn't it be ON UNIQUE CONFLICT/ERROR/VIOLATION ...

 I think that naming unique violations alone would be wrong (not to
 mention ludicrously verbose).

 Why?

Because, as I said, it might not be a unique violation at all. It
could be an exclusion violation.

  b) For me there's a WITH before the index inference clause missing, to
 have it read in 'SQL' style.

 I'm not seeing it. BTW, Robert was the one who initially proposed that
 the unique index inference clause follow this exact style (albeit
 before it accepted a WHERE clause to infer partial indexes, which was
 only added a couple of months ago).

 So?

So, his opinion matters if it comes down to a vote. The inference
specification syntax as implemented is exactly what he suggested (plus
I've added a predicate).

 I guess I can live with that uglyness; but I'd like somebody else to
 chime in.

Agreed.

  c) Right now the UPDATE can refer to pseudo relations 'TARGET' and
 'EXCLUDED'. I think especially the latter doesn't fit anymore at
 all. How about 'CONFLICTING' and 'EXISTING'? Or even NEW and OLD?

 NEW and OLD are terribly misleading, since surely the NEW tuple is the
 one actually appended to the relation by the UPDATE, and the OLD one
 is the existing one (not the excluded one). Plus they have all that
 intellectual baggage from rules.

 What 'intellectual baggage' would that be? That they're already known to
 have been used in another place? I don't see the problem.

The problem is that they make you think of rules, and they don't
describe what's going on at all.

 Seems pretty descriptive of the situation to me - I actually put a lot
 of thought into this. Additionally, the word is widely understood by
 non-native speakers. TARGET is also very descriptive, because it
 situationally describes either the existing tuple actually present in
 the table, or (from a RETURNING clause) the final tuple present in the
 table post-UPDATE. We use the term target for that pervasively (in
 the docs and in the code).

 Sorry, I don't buy either argument. EXISTING and NEW would surely at
 least as widely understood than EXCLUDE and TARGET. The latter does just
 about no sense to me; especially from a user POV. I don't think the
 existing usage of the term has much to do what it's used for here.

Yes it does. The UPDATE docs refer to the target table in a way
intended to distinguish it from any joined-to table (FROM table). It's
clear as day. Maybe EXISTING is equally well understood as a word in
general, but it's way more ambiguous than EXCLUDED is here.

 That
 it has 'morphing' characteristics imo just makes it worse, rather than
 better. Besides being confusing that it has different meanings, it's far
 from inconceivable that somebody wants to return values from the
 preexisting, new, and merged rows.

This is how RETURNING works from UPDATEs in general. IOW, if you do an
UPDATE FROM (which is pretty similar to ON CONFLICT UPDATE,
syntax-wise), then you can only refer to the joined table's tuple and
the final post-update tuple from within RETURNING. You cannot refer to
the pre-UPDATE target tuple there either -- it's *exactly* the same
situation. Why should it be any different here? The
situational/morphing characteristic of the alias name TARGET is
therefore absolutely appropriate, in that it follows UPDATE.

To be fair, there is one unrelated slight difference with RETURNING
and conventional UPDATEs: You cannot return the EXCLUDED tuple (in the
same way that you can reference the joined-FROM tuple within
conventional UPDATEs). This is because the pertinent information is
likely to be in the target tuple (after all, the DML statement names
the proposed-for-insertion tuples itself, directly), but more
importantly because projecting both would necessitate *always*
qualifying the RETURNING column names to resolve which tuple is
intended (UPDATE FROM will seldom be a self-join, but this will always
be like a self-join).

-- 
Peter Geoghegan


-- 
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_dump: largeobject behavior issues (possible bug)

2015-04-25 Thread Andrew Dunstan


On 04/25/2015 12:32 PM, Tom Lane wrote:

Andrew Dunstan and...@dunslane.net writes:

On 04/24/2015 06:41 PM, Tom Lane wrote:

Yeah, this was brought up when we added per-large-object metadata; it was
obvious that that patch would cause pg_dump to choke on large numbers of
large objects.  The (perhaps rather lame) argument was that you wouldn't
have that many of them.
Given that large objects don't have any individual dependencies,
one could envision fixing this by replacing the individual large-object
DumpableObjects by a single placeholder to participate in the sort phase,
and then when it's time to dump that, scan the large objects using a
cursor and create/print/delete the information separately for each one.
This would likely involve some rather painful refactoring in pg_dump
however.

I think we need to think about this some more, TBH, I'm not convinced
that the changes made back in 9.0 were well conceived. Having separate
TOC entries for each LO seems wrong in principle, although I understand
why it was done.

Perhaps.  One advantage of doing it this way is that you can get
pg_restore to extract a single LO from an archive file; though it's
debatable whether that's worth the potential resource-consumption hazards.



In my view it isn't worth it.



Another issue is that restore options such as --no-owner and
--no-privileges would not work for LOs (at least not without messy hacks)
if we go back to a scheme where all the LO information is just SQL
commands inside a single TOC object.

After further thought I realized that if we simply hack pg_dump to emit
the LOs in a streaming fashion, but keep the archive-file representation
the same as it is now, then we haven't really fixed the problem because
pg_restore is still likely to choke when it tries to read the archive's
TOC.  So my proposal above isn't enough either.



Yep, that's certainly true.



Perhaps what we need is some sort of second-level TOC which is only ever
processed in a streaming fashion, by both pg_dump and pg_restore.  This
would not support dependency resolution or re-ordering, but we don't need
those abilities for LOs.





+1, I had a similar thought, half-formed, but you've expressed it better 
than I could have.


cheers

andrew


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Bug in planner

2015-04-25 Thread Tom Lane
David Rowley dgrowle...@gmail.com writes:
 On 24 April 2015 at 21:43, Teodor Sigaev teo...@sigaev.ru wrote:
 I faced with planner error:
 ERROR:  could not find RelOptInfo for given relids

 I've done a little debugging on this too and I get the idea that in
 eqjoinsel() that min_righthand incorrectly does not have a bit set for t3

Yeah.  The short of it seems to be that initsplan.c is too optimistic
about whether antijoins can be reordered against outer joins in their RHS.
The discussion in optimizer/README says pretty clearly that they can't
(and eqjoinsel is relying on that, per the comment therein), so I think
this is basically brain fade in translating that logic to code.

regards, tom lane

diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index a7655e4..00b2625 100644
*** a/src/backend/optimizer/plan/initsplan.c
--- b/src/backend/optimizer/plan/initsplan.c
*** make_outerjoininfo(PlannerInfo *root,
*** 1165,1173 
  		 * For a lower OJ in our RHS, if our join condition does not use the
  		 * lower join's RHS and the lower OJ's join condition is strict, we
  		 * can interchange the ordering of the two OJs; otherwise we must add
! 		 * lower OJ's full syntactic relset to min_righthand.  Here, we must
! 		 * preserve ordering anyway if either the current join is a semijoin,
! 		 * or the lower OJ is either a semijoin or an antijoin.
  		 *
  		 * Here, we have to consider that our join condition includes any
  		 * clauses that syntactically appeared above the lower OJ and below
--- 1165,1173 
  		 * For a lower OJ in our RHS, if our join condition does not use the
  		 * lower join's RHS and the lower OJ's join condition is strict, we
  		 * can interchange the ordering of the two OJs; otherwise we must add
! 		 * the lower OJ's full syntactic relset to min_righthand.  Also, we
! 		 * must preserve ordering anyway if either the current join or the
! 		 * lower OJ is either a semijoin or an antijoin.
  		 *
  		 * Here, we have to consider that our join condition includes any
  		 * clauses that syntactically appeared above the lower OJ and below
*** make_outerjoininfo(PlannerInfo *root,
*** 1184,1189 
--- 1184,1190 
  		{
  			if (bms_overlap(clause_relids, otherinfo-syn_righthand) ||
  jointype == JOIN_SEMI ||
+ jointype == JOIN_ANTI ||
  otherinfo-jointype == JOIN_SEMI ||
  otherinfo-jointype == JOIN_ANTI ||
  !otherinfo-lhs_strict || otherinfo-delay_upper_joins)
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index e4f3f22..ed9ad0e 100644
*** a/src/test/regress/expected/join.out
--- b/src/test/regress/expected/join.out
*** WHERE d.f1 IS NULL;
*** 2284,2289 
--- 2284,2331 
  (3 rows)
  
  --
+ -- regression test for proper handling of outer joins within antijoins
+ --
+ create temp table tt4x(c1 int, c2 int, c3 int);
+ explain (costs off)
+ select * from tt4x t1
+ where not exists (
+   select 1 from tt4x t2
+ left join tt4x t3 on t2.c3 = t3.c1
+ left join ( select t5.c1 as c1
+ from tt4x t4 left join tt4x t5 on t4.c2 = t5.c1
+   ) a1 on t3.c2 = a1.c1
+   where t1.c1 = t2.c2
+ );
+QUERY PLAN
+ -
+  Hash Anti Join
+Hash Cond: (t1.c1 = t2.c2)
+-  Seq Scan on tt4x t1
+-  Hash
+  -  Merge Right Join
+Merge Cond: (t5.c1 = t3.c2)
+-  Merge Join
+  Merge Cond: (t4.c2 = t5.c1)
+  -  Sort
+Sort Key: t4.c2
+-  Seq Scan on tt4x t4
+  -  Sort
+Sort Key: t5.c1
+-  Seq Scan on tt4x t5
+-  Sort
+  Sort Key: t3.c2
+  -  Merge Left Join
+Merge Cond: (t2.c3 = t3.c1)
+-  Sort
+  Sort Key: t2.c3
+  -  Seq Scan on tt4x t2
+-  Sort
+  Sort Key: t3.c1
+  -  Seq Scan on tt4x t3
+ (24 rows)
+ 
+ --
  -- regression test for problems of the sort depicted in bug #3494
  --
  create temp table tt5(f1 int, f2 int);
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index d0cf0a0..5b65ea8 100644
*** a/src/test/regress/sql/join.sql
--- b/src/test/regress/sql/join.sql
*** LEFT JOIN (
*** 448,453 
--- 448,470 
  WHERE d.f1 IS NULL;
  
  --
+ -- regression test for proper handling of outer joins within antijoins
+ --
+ 
+ create temp table tt4x(c1 int, c2 int, c3 int);
+ 
+ explain (costs off)
+ select * from tt4x t1
+ where not exists (
+   select 1 from tt4x t2
+ left join tt4x t3 

Re: [HACKERS] PL/pgSQL, RAISE and error context

2015-04-25 Thread Joel Jacobson
+1


On Sat, Apr 25, 2015 at 10:23 PM, Pavel Stehule pavel.steh...@gmail.com
wrote:

 Hi

 2015-04-24 19:16 GMT+02:00 Joel Jacobson j...@trustly.com:

 On Fri, Apr 24, 2015 at 6:07 PM, Pavel Stehule pavel.steh...@gmail.com
 wrote:
  Example:
 
  context_messages = -warning, -error, +notice
 
 
  I prefer your first proposal - and there is a precedent for plpgsql -
  plpgsql_extra_checks
 
  It is clean for anybody. +-identifiers looks like horrible httpd
 config. :)

 I have to agree on that :) Just thought this is the best we can do if
 we want to reduce the number of GUCs to a minimum.


 I played with some prototype and I am thinking so we need only one GUC

 plpgsql.display_context_messages = 'none'; -- compatible with current
 plpgsql.display_context_messages = 'all';
 plpgsql.display_context_messages = 'exception, log'; -- what I prefer

 I implemented [ (WITH|WITHOUT) CONTEXT ] clause for RAISE statement

 RAISE NOTICE WITH CONTEXT 'some message';
 RAISE NOTICE WITH CONTEXT USING message = 'some message';
 RAISE EXCEPTION WITHOUT CONTEXT 'other message';

 The patch is very small with full functionality (without documentation) -
 I am thinking so it can work. This patch is back compatible - and allow to
 change default behave simply.

 plpgsql.display_context_messages can be simplified to some like
 plpgsql.display_context_min_messages

 What do you think about it?

 Regards

 Pavel




[HACKERS] Temporal extensions

2015-04-25 Thread Dave Jones
Hi all,

My apologies I couldn't directly respond to the earlier thread on this
subject
(http://www.postgresql.org/message-id/50d99278.3030...@dc.baikal.ru) but
I wasn't subscribed to the list at that point.

I've been working on a conversion of several utilities I wrote for
another engine, and was wondering if there was any interest in seeing
any of them added to contrib/ at some point in the vague undefined future?

The github repo for the source is here:

  https://github.com/waveform80/oliphant

The documentation for the extensions (currently) resides here:

  http://oliphant.readthedocs.org/en/latest/

(Obviously the docs would need conversion to docbook for any official
sort of patch; I've used docbook in the past, I mainly wrote the docs in
rst because that's what I'm most used to at the moment and it's awfully
convenient :)

The major extension is history which is intended for the tracking of
temporal data. This comprises various functions for the creation of
history tables, their associated triggers, and utility views for
providing alternate transformations of the historical data.

The other extensions (assert, auth, merge) all really exist in
support of history (to some greater or lesser degree) but I'd split
them out previously as on the original engine (DB2) they served some
purpose and perhaps they can serve some here. However, I'd have no issue
removing such dependence and simply merging the relevant code into the
history extension if that was deemed appropriate (as you can guess,
it's history I'm really interested in.).

The extensions are all written in plpgsql (no C), and there's some
rudimentary tests of their functionality under the tests/ dir (which
again would need converting/expanding in the event they were to become
official). That said, having read through the former thread
(referenced above) I get the impression there's still plenty I need to
think about.

I predict some questions are bound to arise, so I'll provide some brief
answers introductory below:

Q. Why not build on the existing work?

Honestly, I didn't think to go looking for it until I considered posting
to this list, and it's been enlightening seeing what others have done in
this space (doh!). Personally, I started tinkering with this sort of
stuff long ago, in a database engine far far away* and the stuff I had,
had fulfilled all my needs in this space. So, when it came time to move
onto postgres that's where I started (for better or worse).

* http://groups.google.com/group/comp.databases.ibm-db2/msg/e84aeb1f6ac87e6c

Q. Why are you using two timestamp fields instead of a range?

Short answer: performance. I did some tests with a relatively large
history data set using the two-field timestamp method, and a range
method before starting work on the history extension. These tests
seemed to indicate that using ranges significantly impacted performance
(both writes, and queries). It didn't look terribly difficult to convert
the code between the two systems so for the time being I pressed ahead
with the two-field method, but I'd love to find out if I was doing
something stupid with ranges. Further discussion definitely wanted!

Q. Why AFTER triggers instead of BEFORE?

Largely because on the original engine BEFORE triggers were limited in
functionality, so they had to be AFTER triggers. After reading some of
the discussion on the linked thread, this may be a decision I have to
re-visit.

Q. What about official SQL:2011 syntax? Application time, etc.?

Sure - all stuff I'd love to see in Postgres at some point, but not
stuff I'm qualified to even begin looking at (given it requires engine
alterations).


Anyway, that's probably enough for now. Questions, suggestions,
criticisms all gratefully received!

Dave.


-- 
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] inherit support for foreign tables

2015-04-25 Thread Tom Lane
Etsuro Fujita fujita.ets...@lab.ntt.co.jp writes:
 On 2015/04/16 16:05, Etsuro Fujita wrote:
 I agree with you on this point.  However, ISTM there is a bug in
 handling OIDs on foreign tables; while we now allow for ALTER SET
 WITH/WITHOUT OIDS, we still don't allow the default_with_oids parameter
 for foreign tables.  I think that since CREATE FOREIGN TABLE should be
 consistent with ALTER FOREIGN TABLE, we should also allow the parameter
 for foreign tables.  Attached is a patch for that.

 I also updated docs.  Attached is an updated version of the patch.

I believe that we intentionally did not do this, and here is why not:
existing pg_dump files assume that default_with_oids doesn't affect any
relation type except plain tables.  pg_backup_archiver.c only bothers
to change the GUC when about to dump a plain table, and otherwise leaves
it at its previous value.  That means if we apply a patch like this, it's
entirely possible that pg_dump/pg_restore will result in foreign tables
accidentally acquiring OID columns.

Since default_with_oids is really only meant as a backwards-compatibility
hack, I don't personally have a problem with restricting it to act only on
plain tables.  However, it might be a good idea to explicitly document
this interaction in a code comment to prevent anyone from re-inventing
this idea...  I'll go do 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] INSERT ... ON CONFLICT syntax issues

2015-04-25 Thread Peter Geoghegan
On Sat, Apr 25, 2015 at 11:50 AM, Peter Geoghegan p...@heroku.com wrote:
 To be fair, there is one unrelated slight difference with RETURNING
 and conventional UPDATEs: You cannot return the EXCLUDED tuple (in the
 same way that you can reference the joined-FROM tuple within
 conventional UPDATEs). This is because the pertinent information is
 likely to be in the target tuple (after all, the DML statement names
 the proposed-for-insertion tuples itself, directly), but more
 importantly because projecting both would necessitate *always*
 qualifying the RETURNING column names to resolve which tuple is
 intended (UPDATE FROM will seldom be a self-join, but this will always
 be like a self-join).


It also makes sense because this is the RETURNING clause of an INSERT,
not an UPDATE. So the general INSERT behavior is what is expected. It
ought to be irrelevant if tuples were projected by actually inserting
or updating. Otherwise, your UPSERT is probably misconceived.

-- 
Peter Geoghegan


-- 
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] INSERT ... ON CONFLICT syntax issues

2015-04-25 Thread Peter Geoghegan
On Sat, Apr 25, 2015 at 12:23 PM, Andres Freund and...@anarazel.de wrote:
 95% of all users will know NEW/OLD from triggers, not rules. Where NEW
 is used in a quite comparable way.

I don't think it's comparable.

  Seems pretty descriptive of the situation to me - I actually put a lot
  of thought into this. Additionally, the word is widely understood by
  non-native speakers. TARGET is also very descriptive, because it
  situationally describes either the existing tuple actually present in
  the table, or (from a RETURNING clause) the final tuple present in the
  table post-UPDATE. We use the term target for that pervasively (in
  the docs and in the code).
 
  Sorry, I don't buy either argument. EXISTING and NEW would surely at
  least as widely understood than EXCLUDE and TARGET. The latter does just
  about no sense to me; especially from a user POV. I don't think the
  existing usage of the term has much to do what it's used for here.

 Yes it does. The UPDATE docs refer to the target table in a way
 intended to distinguish it from any joined-to table (FROM table). It's
 clear as day.

 Which means the term is used in a different way for INSERTs and UPDATEs
 already.

No, it isn't. It both cases the table is the one involved in the parse
analysis setTargetTable() call.

 Maybe EXISTING is equally well understood as a word in general, but
 it's way more ambiguous than EXCLUDED is here.

 What? I'm not suggesting to replace EXCLUDED by EXISTING - that'd make
 absolutely no sense. My suggesting is to have NEW refer to the tuple
 specified in the INSERT and EXISTING to the, well, pre existing tuple
 that the conflict is with.

Okay, but that doesn't change my opinion.

  That
  it has 'morphing' characteristics imo just makes it worse, rather than
  better. Besides being confusing that it has different meanings, it's far
  from inconceivable that somebody wants to return values from the
  preexisting, new, and merged rows.

 This is how RETURNING works from UPDATEs in general.

 And there's been a patch (which unfortunately died because it's
 implementation wasn't good), to allow referring to the other versions of
 the tuple. It has been wished for numerous times.

Well, if that patch is ever committed, then it won't be hard to get
the behavior here too, since it is literally exactly the same code. I
don't change anything about it, and that seems to be your problem.

 the docs say:
Since
literalRETURNING/ is not part of the commandUPDATE/
auxiliary query, the special literalON CONFLICT UPDATE/ aliases
(varnameTARGET/ and varnameEXCLUDED/) may not be
referenced;  only the row as it exists after updating (or
inserting) is returned.

 So I don't understand that whole chain of argument. There's no such
 morphing behaviour, unless I miss something?

That's a documentation bug (a remnant of an earlier version). Sorry
about that. You can reference TARGET from returning. It's directly
contradicted by this much earlier statement on the INSERT doc page:

Both aliases can be used in the auxiliary query targetlist and WHERE
clause, while the TARGET alias can be used anywhere within the entire
statement (e.g., within the RETURNING clause)

I'll go fix that.

 2a5d80b27d2c5832ad26dde4651c64dd2004f401:
 The problem with this seems to be that it more or less
 necessitates making both IGNORE and UPDATE fully reserved keywords in
 order to avoid an ambiguity, which we prefer not to do

 It does not. As mentioned in the thread DO UPDATE/NOTHING work without
 anything like that.

I just mean that it couldn't work as-was in the repo at that time.
This commit message was written before your proposal of 8 hours ago.

We're going to have to agree to disagree here. I've given you my
opinion. I'm burnt out on this patch, and whatever the path of least
resistance is is the path I'll take. Frankly, the only reason that I'm
putting up any kind of argument is because I don't think that your
proposal is the path of least resistance.

-- 
Peter Geoghegan


-- 
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] INSERT ... ON CONFLICT syntax issues

2015-04-25 Thread Peter Geoghegan
On Sat, Apr 25, 2015 at 12:35 PM, Peter Geoghegan p...@heroku.com wrote:
  That
  it has 'morphing' characteristics imo just makes it worse, rather than
  better. Besides being confusing that it has different meanings, it's far
  from inconceivable that somebody wants to return values from the
  preexisting, new, and merged rows.

 This is how RETURNING works from UPDATEs in general.

 And there's been a patch (which unfortunately died because it's
 implementation wasn't good), to allow referring to the other versions of
 the tuple. It has been wished for numerous times.

 Well, if that patch is ever committed, then it won't be hard to get
 the behavior here too, since it is literally exactly the same code. I
 don't change anything about it, and that seems to be your problem.


I withdraw this remark. Even in a world where this patch is committed,
it still makes sense for the INSERT returning behavior to not be
altered (and to project only TARGET tuples even if they come from the
auxiliary UPDATE). The join is within the auxiliary UPDATE, not the
INSERT, and it should be no more possible to project intermediate
tuples (like EXCLUDED.*) from the INSERT's RETURNING than it is to
project CTE scan tuples from an INSERT ... RETURNING with a CTE.

-- 
Peter Geoghegan


-- 
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] forward vs backward slashes in msvc build code

2015-04-25 Thread Andrew Dunstan


On 04/25/2015 10:53 AM, Michael Paquier wrote:

On Sat, Apr 25, 2015 at 9:58 PM, Peter Eisentraut wrote:

On 4/24/15 12:22 AM, Michael Paquier wrote:

Now that the stuff related to the move from contrib/ to src/bin/,
modulescheck and tmp_install has been committed, shouldn't we give a
new shot at this patch? Attached is a rebased version.

done

Thanks, bowerbird is running fine:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=bowerbirddt=2015-04-25%2013%3A31%3A06



But currawong is not - it runs an older version of the MS build tools. 
See 
http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=currawongdt=2015-04-25%2016%3A31%3A12:


   Bad format filename 'src/backend/access/brin/brin.c'
 at src/tools/msvc/VCBuildProject.pm line 73.
VCBuildProject::WriteFiles(VC2008Project=HASH(0x9a7274), *Project::F) 
called at src/tools/msvc/Project.pm line 363
Project::Save(VC2008Project=HASH(0x9a7274)) called at 
src/tools/msvc/Solution.pm line 539
Solution::Save(VS2008Solution=HASH(0xc00b7c)) called at 
src/tools/msvc/Mkvcbuild.pm line 656
Mkvcbuild::mkvcbuild(HASH(0xbed464)) called at build.pl line 36

cheers

andrew



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] forward vs backward slashes in msvc build code

2015-04-25 Thread Michael Paquier
On Sun, Apr 26, 2015 at 2:19 AM, Andrew Dunstan and...@dunslane.net wrote:

 On 04/25/2015 10:53 AM, Michael Paquier wrote:

 On Sat, Apr 25, 2015 at 9:58 PM, Peter Eisentraut wrote:

 On 4/24/15 12:22 AM, Michael Paquier wrote:

 Now that the stuff related to the move from contrib/ to src/bin/,
 modulescheck and tmp_install has been committed, shouldn't we give a
 new shot at this patch? Attached is a rebased version.

 done

 Thanks, bowerbird is running fine:

 http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=bowerbirddt=2015-04-25%2013%3A31%3A06



 But currawong is not - it runs an older version of the MS build tools. See
 http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=currawongdt=2015-04-25%2016%3A31%3A12:

Bad format filename 'src/backend/access/brin/brin.c'
  at src/tools/msvc/VCBuildProject.pm line 73.
 VCBuildProject::WriteFiles(VC2008Project=HASH(0x9a7274),
 *Project::F) called at src/tools/msvc/Project.pm line 363
 Project::Save(VC2008Project=HASH(0x9a7274)) called at
 src/tools/msvc/Solution.pm line 539
 Solution::Save(VS2008Solution=HASH(0xc00b7c)) called at
 src/tools/msvc/Mkvcbuild.pm line 656
 Mkvcbuild::mkvcbuild(HASH(0xbed464)) called at build.pl line 36

Oops, attached is a patch that should make currawong happier..
-- 
Michael
diff --git a/src/tools/msvc/VCBuildProject.pm b/src/tools/msvc/VCBuildProject.pm
index 513cfb5..dda662f 100644
--- a/src/tools/msvc/VCBuildProject.pm
+++ b/src/tools/msvc/VCBuildProject.pm
@@ -71,7 +71,7 @@ EOF
 	foreach my $fileNameWithPath (sort keys %{ $self-{files} })
 	{
 		confess Bad format filename '$fileNameWithPath'\n
-		  unless ($fileNameWithPath =~ /^(.*)\\([^\\]+)\.(c|cpp|y|l|rc)$/);
+		  unless ($fileNameWithPath =~ m!^(.*)/([^/]+)\.(c|cpp|y|l|rc)$!);
 		my $dir  = $1;
 		my $file = $2;
 

-- 
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] Shouldn't CREATE TABLE LIKE copy the relhasoids property?

2015-04-25 Thread David G. Johnston
On Saturday, April 25, 2015, Tom Lane t...@sss.pgh.pa.us wrote:


 It's perhaps debatable whether it should act that way, but in the absence
 of complaints from the field, I'm hesitant to change these cases.  It
 might be better if the effective behavior were table gets OIDs if
 default_with_oids = true or WITH OIDS is given or base table has OIDs.


+1



 Still another case that needs to be thought about is create table likeit
 (like base) without oids where base does have OIDs.  Probably the right
 thing here is to let the WITHOUT OIDS spec override what we see in base.


Why are oids special in this manner?  No other inherited column can be
omitted from the child table.  Though I guess unlike inherits there is no
reason to mandate the final result be identical to the base table - though
here is something to be said for pointing out the inconsistency and
requiring the user to alter table if indeed they want to have the oid-ness
changed.

David J.


Re: [HACKERS] parallel mode and parallel contexts

2015-04-25 Thread Amit Kapila
On Tue, Apr 21, 2015 at 11:38 PM, Robert Haas robertmh...@gmail.com wrote:


 After thinking about it a bit more, I realized that even if we settle
 on some solution to that problem, there's another issues: the
 wait-edges created by this system don't really have the same semantics
 as regular lock waits.  Suppose A is waiting on a lock held by B and B
 is waiting for a lock held by A; that's a deadlock.  But if A is
 waiting for B to write to a tuple queue and B is waiting for A to read
 from a tuple queue, that's not a deadlock if the queues in question
 are the same.  If they are different queues, it might be a deadlock,
 but then again maybe not.  It may be that A is prepared to accept B's
 message from one queue, and that upon fully receiving it, it will do
 some further work that will lead it to write a tuple into the other
 queue.  If so, we're OK; if not, it's a deadlock.

I agree that the way deadlock detector works for locks, it might not
be same as it needs to work for communication buffers (tuple queues).
Here I think we also need to devise some different way to remove resources
from wait/resource queue, it might not be a good idea to do it similar to
locks
as locks are released at transaction end whereas this new resources
(communication buffers) don't operate at transaction boundary.

 I'm not sure
 whether you'll want to argue that that is an implausible scenario, but
 I'm not too sure it is.  The worker could be saying hey, I need some
 additional piece of your backend-local state in order to finish this
 computation, and the master could then provide it.  I don't have any
 plans like that, but it's been suggested previously by others, so it's
 not an obviously nonsensical thing to want to do.


If such a thing is not possible today and also it seems that is a not a
good design to solve any problem, then why to spend too much effort
in trying to find ways to detect deadlocks for the same.

 A further difference in the semantics of these wait-edges is that if
 process A is awaiting AccessExclusiveLock on a resource held by B, C,
 and D at AccessShareLock, it needs to wait for all of those processes
 to release their locks before it can do anything else.  On the other
 hand, if process A is awaiting tuples from B, C, and D, it just needs
 ONE of those processes to emit tuples in order to make progress.  Now
 maybe that doesn't make any difference in practice, because even if
 two of those processes are making lively progress and A is receiving
 tuples from them and processing them like gangbusters, that's probably
 not going to help the third one get unstuck.  If we adopt the approach
 of discounting that possibility, then as long as the parallel leader
 can generate tuples locally (that is, local_scan_done = false) we
 don't report the deadlock, but as soon as it can no longer do that
 (local_scan_done = true) then we do, even though we could still
 theoretically read more tuples from the non-stuck workers.  So then
 you have to wonder why we're not solving problem #1, because the
 deadlock was just as certain before we generated the maximum possible
 number of tuples locally as it was afterwards.


I think the deadlock detection code should run if anytime we have to
wait for more than deadlock timeout.  Now I think the important point
here is when to actually start waiting, as per current parallel_seqscan
patch we wait only after checking the tuples from all queues, we could
have done it other way (wait if we can't fetch from one queue) as well.
It seems both has pros and cons, so we can proceed assuming current
way is okay and we can consider to change it in future once we find some
reason for the same.

Having said above, I feel this is not the problem that we should try to
solve at this point unless there is any scenario where we could hit
deadlock due to communication buffers.  I think such a solution would
be required for advanced form of parallelism (like intra-query parallelism).
By the way, why are we trying to solve this problem, is there any way
with which we can hit it for parallel sequential scan?


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


Re: [HACKERS] Shouldn't CREATE TABLE LIKE copy the relhasoids property?

2015-04-25 Thread Tom Lane
Bruce Momjian br...@momjian.us writes:
 I have changed the default value back to the function call as it should
 have been all along;  patch attached.  I will revisit this for 9.6
 unless I hear otherwise.

I still don't like this patch one bit.  I don't think that this code
should be modifying stmt-options that way.  Also, you have not addressed
whether this is even the right semantics.  In particular, currently
default_with_oids will force an OID column to exist regardless of whether
the LIKE-referenced table has them:

regression=# create table base (f1 int);
CREATE TABLE
regression=# set default_with_oids = true;
SET
regression=# create table likeit (like base);
CREATE TABLE
regression=# \d+ base
 Table public.base
 Column |  Type   | Modifiers | Storage | Stats target | Description 
+-+---+-+--+-
 f1 | integer |   | plain   |  | 

regression=# \d+ likeit
Table public.likeit
 Column |  Type   | Modifiers | Storage | Stats target | Description 
+-+---+-+--+-
 f1 | integer |   | plain   |  | 
Has OIDs: yes

Another variant is create table likeit (like base) with oids.

It's perhaps debatable whether it should act that way, but in the absence
of complaints from the field, I'm hesitant to change these cases.  It
might be better if the effective behavior were table gets OIDs if
default_with_oids = true or WITH OIDS is given or base table has OIDs.

Still another case that needs to be thought about is create table likeit
(like base) without oids where base does have OIDs.  Probably the right
thing here is to let the WITHOUT OIDS spec override what we see in base.

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] a fast bloat measurement tool (was Re: Measuring relation free space)

2015-04-25 Thread Amit Kapila
On Fri, Apr 24, 2015 at 8:05 PM, Tomas Vondra tomas.von...@2ndquadrant.com
wrote:



 On 04/24/15 14:58, Amit Kapila wrote:

 On Fri, Apr 24, 2015 at 8:46 AM, Abhijit Menon-Sen a...@2ndquadrant.com
 mailto:a...@2ndquadrant.com wrote:
  
   At 2015-04-24 08:35:40 +0530, amit.kapil...@gmail.com
 mailto:amit.kapil...@gmail.com wrote:
   
 Just stick a PG_RETURN_NULL() at the end?
   
That should also work.
  
   OK, updated patch attached with just that one change.
  

 Patch looks good to me, will mark it as Ready for Committer if Tomas
 or anyone else doesn't have more to add in terms of review.


 FWIW, I'm OK with the patch as is. Your reviews were spot on.


Thanks, I have marked this patch as Ready For Committer.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


Re: [HACKERS] Shouldn't CREATE TABLE LIKE copy the relhasoids property?

2015-04-25 Thread Tom Lane
David G. Johnston david.g.johns...@gmail.com writes:
 On Saturday, April 25, 2015, Tom Lane t...@sss.pgh.pa.us wrote:
 Still another case that needs to be thought about is create table likeit
 (like base) without oids where base does have OIDs.  Probably the right
 thing here is to let the WITHOUT OIDS spec override what we see in base.

 Why are oids special in this manner?  No other inherited column can be
 omitted from the child table.

Hm, good point; INHERITS will silently override such a specification:

regression=# create table base1 (f1 int) with oids;
CREATE TABLE
regression=# create table c2 () inherits (base1) without oids;
CREATE TABLE
regression=# \d+ c2
  Table public.c2
 Column |  Type   | Modifiers | Storage | Stats target | Description 
+-+---+-+--+-
 f1 | integer |   | plain   |  | 
Inherits: base1
Has OIDs: yes

 Though I guess unlike inherits there is no
 reason to mandate the final result be identical to the base table - though
 here is something to be said for pointing out the inconsistency and
 requiring the user to alter table if indeed they want to have the oid-ness
 changed.

Yeah, LIKE doesn't necessarily have to behave the same as INHERITS;
but probably we should follow that precedent unless we have a specific
argument not to.  Which I don't.

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] adding more information about process(es) cpu and memory usage

2015-04-25 Thread Julien Rouhaud
Le 24/04/2015 21:11, Jim Nasby a écrit :
 On 4/24/15 6:29 AM, Robert Haas wrote:
 On Thu, Apr 23, 2015 at 9:28 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 The reason nobody's gotten around to that in the last fifteen years is
 that per-process rusage isn't actually all that interesting; there's
 too much that happens in background daemons, for instance.

 There's *some* stuff that happens in background daemons, but if you
 want to measure user and system time consume by a particularly query,
 this would actually be a pretty handy way to do that, I think.
 
 I more often am wondering what a running backend is doing OS-wise, but
 being able to see what happened when it finished would definitely be
 better than what's available now.

There are at least two projects that provides this kind of statistics
for backends: pg_proctab (https://github.com/markwkm/pg_proctab) and
pg_stat_kcache (https://github.com/dalibo/pg_stat_kcache).  Michael also
wrote an article on this topic some weeks ago
(http://michael.otacoo.com/postgresql-2/postgres-calculate-cpu-usage-process/).

Regards
-- 
Julien Rouhaud
http://dalibo.com - http://dalibo.org


-- 
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] Row security violation error is misleading

2015-04-25 Thread Dean Rasheed
On 25 April 2015 at 01:52, Stephen Frost sfr...@snowman.net wrote:
 * Dean Rasheed (dean.a.rash...@gmail.com) wrote:
 The second patch [2] is the one that is actually relevant to this
 thread. This patch is primarily to apply the RLS checks earlier,
 before an update is attempted, more like a regular permissions check.
 This adds a new enum to classify the kinds of WCO, a side benefit of
 which is that it allows different error messages when RLS checks are
 violated, as opposed to WITH CHECK OPTIONs on views.

 I've gone ahead and pushed this, please take a look and test it and let
 me know if you see any issues or have any questions or concerns.


Brilliant, thanks! I gave it a quick read-through and all the changes
look good, so thanks for all your work on this.

Regards,
Dean


-- 
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 in planner

2015-04-25 Thread David Rowley
On 24 April 2015 at 21:43, Teodor Sigaev teo...@sigaev.ru wrote:

 Hi!

 I faced with planner error:
 ERROR:  could not find RelOptInfo for given relids


Good find!

I've simplified your query a bit, the following still shows the issue
(using your schema):

SELECT *
FROM t1
WHERE NOT EXISTS (SELECT t2.c2 AS c1
  FROM t2
  LEFT OUTER JOIN t3 ON t2.c3 = t3.c1
  LEFT OUTER JOIN (SELECT t5.c1 AS c1
   FROM t4
   LEFT OUTER JOIN t5 ON t4.c2 = t5.c1 --offset 0
  ) a1 ON a1.c1 = t3.c2
  WHERE t1.c1 = t2.c2
);

I've done a little debugging on this too and I get the idea that in
eqjoinsel() that min_righthand incorrectly does not have a bit set for t3

When the failing call is made to find_join_rel() with the above query
relids being searched for has a decimal value of 388 (binary 11100 i.e
t5, t4, t2)

find_join_rel makes a pass over join_rel_list to search for the 388 valued
relids.

 join_rel_list contains the following:

1 - 396  (110001100) t5, t4, t3, t2
2 - 384  (11000) t5, t4
3 - 392  (110001000) t5, t4, t3
4 - 396  (110001100) t5, t4, t3, t2

I looked up simple_rte_array to determine which bits are for which relation.

simple_rte_array:
1 - t1
2 - t2
3 - t3
4 - join
5 - a1
6 - join
7 - t4
8 - t5


I'd imagine that the find_join_input_rel() search should actually be for
relids 396. I need to spend more time in this area to get a better grasp of
what's actually meant to be happening, but I think the problem lies
in make_outerjoininfo() when it determines what min_righthand should be set
to with the following:

/*
 * Similarly for required RHS.  But here, we must also include any lower
 * inner joins, to ensure we don't try to commute with any of them.
 */
min_righthand = bms_int_members(bms_union(clause_relids, inner_join_rels),
right_rels);

I think the problem seems to be down to the fact that inner_join_rels and
clause_relids are built from deconstruct_jointree() which I'd imagine does
not get modified when the subquery for t4 and t5 is pulled up, therefore is
out-of-date. /theory

I've attached a patch which appears to fix the problem, but this is more
for the purposes of a demonstration of where the problem lies. I don't have
enough knowledge of what's meant to be happening here, I'll need to spend
more time reading code and debugging.

On a side note, I just discovered another join removal opportunity:

explain select * from t1 where not exists(select 1 from t2 left join t3 on
t2.c1 = t3.c1 where t1.c1=t2.c1);

The join to t3 here is useless, as since it's a left join, the join could
only cause duplication of t2 rows, and this does not matter as we're
performing an anti join anyway (same applies for semi join).

Regards

David Rowley


anti_join_with_pulledup_outer_joins_fix_attempt.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] Idea: closing the loop for pg_ctl reload

2015-04-25 Thread Jan de Visser
On April 22, 2015 06:04:42 PM Payal Singh wrote:
 The following review has been posted through the commitfest application:
 make installcheck-world:  not tested
 Implements feature:   tested, failed
 Spec compliant:   not tested
 Documentation:tested, failed
 
 Error in postgresql.conf gives the expected result on pg_ctl reload,
 although errors in pg_hba.conf file don't. Like before, reload completes
 fine without any information that pg_hba failed to load and only
 information is present in the log file. I'm assuming pg_ctl reload should
 prompt user if file fails to load irrespective of which file it is -
 postgresql.conf or pg_hba.conf.

Will fix. Not hard, just move the code that writes the .pid file to after the 
pg_hba reload.

 
 There is no documentation change so far, but I guess that's not yet
 necessary.

I will update the page for pg_ctl. At least the behaviour of -w/-W in relation 
to the reload command needs explained.

 
 gmake check passed all tests.
 
 The new status of this patch is: Waiting on Author

jan



-- 
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] improving speed of make check-world

2015-04-25 Thread Michael Paquier
On Sat, Apr 25, 2015 at 7:59 AM, Peter Eisentraut pete...@gmx.net wrote:
 On 4/23/15 1:22 PM, Jeff Janes wrote:
 Something about this commit (dcae5faccab64776376d354d) broke make
 check in parallel conditions when started from a clean directory.  It
 fails with a different error each time, one example:

 make -j4 check  /dev/null

 In file included from gram.y:14515:
 scan.c: In function 'yy_try_NUL_trans':
 scan.c:10307: warning: unused variable 'yyg'
 /usr/bin/ld: tab-complete.o: No such file: No such file or directory
 collect2: ld returned 1 exit status
 make[3]: *** [psql] Error 1
 make[2]: *** [all-psql-recurse] Error 2
 make[2]: *** Waiting for unfinished jobs
 make[1]: *** [all-bin-recurse] Error 2
 make: *** [all-src-recurse] Error 2
 make: *** Waiting for unfinished jobs

 I think the problem is that check depends on all, but now also
 depends on temp-install, which in turn runs install and all.  With a
 sufficient amount of parallelism, you end up running two alls on top
 of each other.

 It seems this can be fixed by removing the check: all dependency.  Try
 removing that in the top-level GNUmakefile.in and see if the problem
 goes away.  For completeness, we should then also remove it in the other
 makefiles.

Yep. I spent some time yesterday and today poking at that, but I
clearly missed that dependency.. Attached is a patch fixing the
problem. I tested check and check-world with some parallel jobs and
both worked. In the case of check, the amount of logs is very reduced
because all the install process is done by temp-install which
redirects everything into tmp_install/log/install.log.
-- 
Michael
diff --git a/GNUmakefile.in b/GNUmakefile.in
index 361897a..42aeaa6 100644
--- a/GNUmakefile.in
+++ b/GNUmakefile.in
@@ -62,7 +62,7 @@ distclean maintainer-clean:
 # Garbage from autoconf:
 	@rm -rf autom4te.cache/
 
-check check-tests: all
+check check-tests:
 
 check check-tests installcheck installcheck-parallel installcheck-tests:
 	$(MAKE) -C src/test/regress $@
diff --git a/contrib/test_decoding/Makefile b/contrib/test_decoding/Makefile
index 613e9c3..e47ebab 100644
--- a/contrib/test_decoding/Makefile
+++ b/contrib/test_decoding/Makefile
@@ -39,7 +39,7 @@ submake-test_decoding:
 
 REGRESSCHECKS=ddl rewrite toast permissions decoding_in_xact decoding_into_rel binary prepared
 
-regresscheck: all | submake-regress submake-test_decoding temp-install
+regresscheck: | submake-regress submake-test_decoding temp-install
 	$(MKDIR_P) regression_output
 	$(pg_regress_check) \
 	--temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \
@@ -53,7 +53,7 @@ regresscheck-install-force: | submake-regress submake-test_decoding temp-install
 
 ISOLATIONCHECKS=mxact delayed_startup ondisk_startup concurrent_ddl_dml
 
-isolationcheck: all | submake-isolation submake-test_decoding temp-install
+isolationcheck: | submake-isolation submake-test_decoding temp-install
 	$(MKDIR_P) isolation_output
 	$(pg_isolation_regress_check) \
 	--temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \
diff --git a/src/bin/initdb/Makefile b/src/bin/initdb/Makefile
index fc809a0..d479788 100644
--- a/src/bin/initdb/Makefile
+++ b/src/bin/initdb/Makefile
@@ -58,7 +58,7 @@ clean distclean maintainer-clean:
 # ensure that changes in datadir propagate into object file
 initdb.o: initdb.c $(top_builddir)/src/Makefile.global
 
-check: all
+check:
 	$(prove_check)
 
 installcheck:
diff --git a/src/bin/pg_basebackup/Makefile b/src/bin/pg_basebackup/Makefile
index 58f8b66..0d8421a 100644
--- a/src/bin/pg_basebackup/Makefile
+++ b/src/bin/pg_basebackup/Makefile
@@ -50,7 +50,7 @@ clean distclean maintainer-clean:
 		$(OBJS)
 	rm -rf tmp_check
 
-check: all
+check:
 	$(prove_check)
 
 installcheck:
diff --git a/src/bin/pg_config/Makefile b/src/bin/pg_config/Makefile
index 71ce236..dbc9899 100644
--- a/src/bin/pg_config/Makefile
+++ b/src/bin/pg_config/Makefile
@@ -49,7 +49,7 @@ clean distclean maintainer-clean:
 	rm -f pg_config$(X) $(OBJS)
 	rm -rf tmp_check
 
-check: all
+check:
 	$(prove_check)
 
 installcheck:
diff --git a/src/bin/pg_controldata/Makefile b/src/bin/pg_controldata/Makefile
index f7a4010..fd7399b 100644
--- a/src/bin/pg_controldata/Makefile
+++ b/src/bin/pg_controldata/Makefile
@@ -35,7 +35,7 @@ clean distclean maintainer-clean:
 	rm -f pg_controldata$(X) $(OBJS)
 	rm -rf tmp_check
 
-check: all
+check:
 	$(prove_check)
 
 installcheck:
diff --git a/src/bin/pg_ctl/Makefile b/src/bin/pg_ctl/Makefile
index 525e1d4..37eb482 100644
--- a/src/bin/pg_ctl/Makefile
+++ b/src/bin/pg_ctl/Makefile
@@ -38,7 +38,7 @@ clean distclean maintainer-clean:
 	rm -f pg_ctl$(X) $(OBJS)
 	rm -rf tmp_check
 
-check: all
+check:
 	$(prove_check)
 
 installcheck:
diff --git a/src/bin/scripts/Makefile b/src/bin/scripts/Makefile
index 8b6f54c..c831716 100644
--- a/src/bin/scripts/Makefile
+++ b/src/bin/scripts/Makefile
@@ -69,7 +69,7 @@ clean distclean maintainer-clean:
 	rm -f dumputils.c print.c mbprint.c kwlookup.c 

Re: [HACKERS] collate.linux.utf8 test coverage

2015-04-25 Thread Michael Paquier
On Sat, Apr 25, 2015 at 4:51 AM, Andrew Dunstan and...@dunslane.net wrote:
 The optional buildfarm module that runs this test was broken by commit
 dcae5faccab64776376d354decda0017c648bb53

 Since nobody has responded to my complaint about this, I have disabled it on
 crake, the only buildfarm machine that has actually been running the test,
 so we now have no buildfarm coverage for it.

Commit dcae5fac has switched in pg_regress --temp-install to
--temp-instance, --psqldir to --bindir, and has removed --top-builddir
and --extra-install. After looking at TestCollateLinuxUTF8.pm, I think
that you need to use the one-liner attached.
Regards,
-- 
Michael
diff --git a/PGBuild/Modules/TestCollateLinuxUTF8.pm b/PGBuild/Modules/TestCollateLinuxUTF8.pm
index 465a1cd..19333f1 100644
--- a/PGBuild/Modules/TestCollateLinuxUTF8.pm
+++ b/PGBuild/Modules/TestCollateLinuxUTF8.pm
@@ -82,7 +82,7 @@ sub installcheck
 my $logpos = -s $installdir/logfile || 0;
 
 my @checklog;
-my $cmd =./pg_regress --psqldir=$installdir/bin --dlpath=. 
+my $cmd =./pg_regress --bindir=$installdir/bin --dlpath=. 
   .$inputdir --port=$buildport collate.linux.utf8;
 @checklog = `cd $pgsql/src/test/regress  $cmd 21`;
 

-- 
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] Ignoring some binaries generated in src/test

2015-04-25 Thread Michael Paquier
On Sat, Apr 25, 2015 at 6:14 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Alvaro Herrera alvhe...@2ndquadrant.com writes:
 Tom Lane wrote:
 The current logic in src/test/Makefile, particularly the way that
 the modules subdirectory is handled, seems pretty ugly/convoluted
 anyway.  I wonder why it was done that way rather than just ensuring
 that modules/ doesn't do anything for make install?

 Because we do want to have the Makefile in src/test/modules to install
 the modules if make install is invoked there.  That way, you can run
 make -C src/test/modules install installcheck, and it works.

 OK.  I still wonder if there isn't a better way to get that effect, but
 I left it alone for now.  I committed Michael's new .gitignore files and
 fixed the Makefiles so that make clean and friends clean up properly.

Thanks.
-- 
Michael


-- 
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] forward vs backward slashes in msvc build code

2015-04-25 Thread Michael Paquier
On Sat, Apr 25, 2015 at 9:58 PM, Peter Eisentraut wrote:
 On 4/24/15 12:22 AM, Michael Paquier wrote:
 Now that the stuff related to the move from contrib/ to src/bin/,
 modulescheck and tmp_install has been committed, shouldn't we give a
 new shot at this patch? Attached is a rebased version.

 done

Thanks, bowerbird is running fine:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=bowerbirddt=2015-04-25%2013%3A31%3A06
-- 
Michael


-- 
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_dump: largeobject behavior issues (possible bug)

2015-04-25 Thread Andrew Dunstan


On 04/24/2015 06:41 PM, Tom Lane wrote:

Andrew Dunstan and...@dunslane.net writes:

On 04/23/2015 04:04 PM, Andrew Gierth wrote:

The relevant code is getBlobs in pg_dump.c, which queries the whole of
pg_largeobject_metadata without using a cursor (so the PGresult is
already huge thanks to having 100 million rows), and then mallocs a
BlobInfo array and populates it from the PGresult, also using pg_strdup
for the oid string, owner name, and ACL if any.

I'm surprised this hasn't come up before. I have a client that I
persuaded to convert all their LOs to bytea fields because of problems
with pg_dump handling millions of LOs, and kept them on an older
postgres version until they made that change.

Yeah, this was brought up when we added per-large-object metadata; it was
obvious that that patch would cause pg_dump to choke on large numbers of
large objects.  The (perhaps rather lame) argument was that you wouldn't
have that many of them.

Given that large objects don't have any individual dependencies,
one could envision fixing this by replacing the individual large-object
DumpableObjects by a single placeholder to participate in the sort phase,
and then when it's time to dump that, scan the large objects using a
cursor and create/print/delete the information separately for each one.
This would likely involve some rather painful refactoring in pg_dump
however.



I think we need to think about this some more, TBH, I'm not convinced 
that the changes made back in 9.0 were well conceived. Having separate 
TOC entries for each LO seems wrong in principle, although I understand 
why it was done. For now, my advice would be to avoid use of 
pg_dump/pg_restore if you have large numbers of LOs. The good news is 
that these days there are alternative methods of doing backup / restore, 
albeit not 100% equivalent with pg_dump / pg_restore.


One useful thing might be to provide pg_dump with 
--no-blobs/--blobs-only switches so you could at least easily segregate 
the blobs into their own dump file. That would be in addition to dealing 
with the memory problems pg_dump has with millions of LOs, of course.



cheers

andrew


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Feedback on getting rid of VACUUM FULL

2015-04-25 Thread Simon Riggs
On 24 April 2015 at 22:36, Jim Nasby jim.na...@bluetreble.com wrote:


 Instead of adding forcefsm, I think it would be more useful to accept a
 target block number. That way we can actually control where the new tuple
 goes. For this particular case we'd presumably go with normal FSM page
 selection logic, but someone could chose to to do something more
 sophisticated if they wanted.

 [1] http://postgresql.org/message-id/3409.1253147...@sss.pgh.pa.us
 [2] http://postgresql.org/message-id/3631.1253149...@sss.pgh.pa.us


I don't think specifying exact blocks will help, it will get us in more
trouble in the long run.

I think we need to be able to specify these update placement strategies

* TARGBLOCK_SAME - try to put the update on the same block if possible -
default
* TARGBLOCK_NEW - always force the update to go on a new block, to shrink
table rapidly

and these new block selection strategies

* FSM_ANY - Any block from FSM - default, as now
* FSM_NEAR - A block near the current one to maintain clustering as much as
possible - set automatically if table is clustered
* FSM_SHRINK - A block as near to block 0 as possible, while still handing
out different blocks to each backend by reselecting a block if we
experience write contention

I would suggest that if VACUUM finds the table is bloated beyond a specific
threshold it automatically puts it in FSM_SHRINK mode, and resets it back
to FSM_ANY once the bloat has reduced. That will naturally avoid bloat.

fsm modes can also be set manually to enforce bloat minimization.

We can also design a utility to actively use TARGBLOCK_NEW and FSM_SHRINK
to reduce table size without blocking writes.

But this is all stuff for 9.6...

-- 
Simon Riggshttp://www.2ndQuadrant.com/
http://www.2ndquadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training  Services


Re: [HACKERS] Can pg_dump make use of CURRENT/SESSION_USER

2015-04-25 Thread Fabrízio de Royes Mello
On Wed, Mar 18, 2015 at 2:24 PM, Robert Haas robertmh...@gmail.com wrote:

 On Wed, Mar 18, 2015 at 1:23 PM, Fabrízio de Royes Mello
 fabriziome...@gmail.com wrote:
   If we ever implement something like
  
   COMMENT ON CURRENT_DATABASE IS ...
  
   it will be useful, because you will be able to restore a dump into
   another database and have the comment apply to the target database.
 
  I think it's simple to implement, but how about pg_dump... we need to
add
  new option (like --use-current-database) or am I missing something ?

 I think we'd just change it to use the new syntax, full stop.  I see
 no need for an option.


I'm returning on this...

What's the reasonable syntaxes?

COMMENT ON CURRENT DATABASE IS 'text';

or

COMMENT ON DATABASE { CURRENT_DATABASE | object_name } IS 'text';


   (Also, I wonder about
   ALTER USER foo IN DATABASE current_database ...
   because that will let us dump per-database user options too.)
 
  +1 for both of those ideas.
 


ALTER ROLE { role_specification | ALL } [ IN CURRENT DATABASE ] SET/RESET
...

or

ALTER ROLE { role_specification | ALL } [ IN { CURRENT DATABASE | DATABASE
database_name } ] SET/RESET ...


Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
 Timbira: http://www.timbira.com.br
 Blog: http://fabriziomello.github.io
 Linkedin: http://br.linkedin.com/in/fabriziomello
 Twitter: http://twitter.com/fabriziomello
 Github: http://github.com/fabriziomello


Re: [HACKERS] forward vs backward slashes in msvc build code

2015-04-25 Thread Peter Eisentraut
On 4/24/15 12:22 AM, Michael Paquier wrote:
 On Fri, Mar 13, 2015 at 6:04 PM, Michael Paquier
 michael.paqu...@gmail.com wrote:
 On Fri, Mar 13, 2015 at 6:20 AM, Alvaro Herrera wrote:
 Peter Eisentraut wrote:
 This is contrib/chkpass not finding the crypt symbol, which is
 presumably in some library.  But I can't see how it would normally find
 it, without my patch.

 It seems crypt is provided by libpgport.  So chkpass should be mentioned
 in @contrib_uselibpgport, but isn't.  Maybe the fix is just to add it
 there?

 I had a closer look at this patch, and yes indeed, the problem was
 exactly that. Now honestly I cannot understand why this dependency
 with libpgport was not necessary before... In any case, attached is a
 patch rebased on HEAD that builds correctly with MSVC.
 
 Now that the stuff related to the move from contrib/ to src/bin/,
 modulescheck and tmp_install has been committed, shouldn't we give a
 new shot at this patch? Attached is a rebased version.

done



-- 
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_dump: largeobject behavior issues (possible bug)

2015-04-25 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes:
 On 04/24/2015 06:41 PM, Tom Lane wrote:
 Yeah, this was brought up when we added per-large-object metadata; it was
 obvious that that patch would cause pg_dump to choke on large numbers of
 large objects.  The (perhaps rather lame) argument was that you wouldn't
 have that many of them.

 Given that large objects don't have any individual dependencies,
 one could envision fixing this by replacing the individual large-object
 DumpableObjects by a single placeholder to participate in the sort phase,
 and then when it's time to dump that, scan the large objects using a
 cursor and create/print/delete the information separately for each one.
 This would likely involve some rather painful refactoring in pg_dump
 however.

 I think we need to think about this some more, TBH, I'm not convinced 
 that the changes made back in 9.0 were well conceived. Having separate 
 TOC entries for each LO seems wrong in principle, although I understand 
 why it was done.

Perhaps.  One advantage of doing it this way is that you can get
pg_restore to extract a single LO from an archive file; though it's
debatable whether that's worth the potential resource-consumption hazards.
Another issue is that restore options such as --no-owner and
--no-privileges would not work for LOs (at least not without messy hacks)
if we go back to a scheme where all the LO information is just SQL
commands inside a single TOC object.

After further thought I realized that if we simply hack pg_dump to emit
the LOs in a streaming fashion, but keep the archive-file representation
the same as it is now, then we haven't really fixed the problem because
pg_restore is still likely to choke when it tries to read the archive's
TOC.  So my proposal above isn't enough either.

Perhaps what we need is some sort of second-level TOC which is only ever
processed in a streaming fashion, by both pg_dump and pg_restore.  This
would not support dependency resolution or re-ordering, but we don't need
those abilities for LOs.

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