Re: [HACKERS] POLA violation with \c service=

[Pavel Stehule]

2015-02-20 8:22 GMT+01:00 David Fetter :

> On Fri, Feb 20, 2015 at 07:10:29AM +0100, Pavel Stehule wrote:
> > Hi
> >
> > I am happy with doc changes now.
> >
> > When I test last patch, I found sigfault bug, because host =
> > PQhost(o_conn); returns NULL. I fexed it - please, see patch 007
> >
> > If you are agree with fix, I'll mark this patch as ready for commit.
>
> Thanks for fixing the bug.  Let's go with this.
>

marked as "ready for commit"

Thank you for patch

Regards

Pavel


>
> Cheers,
> David.
> --
> David Fetter  http://fetter.org/
> Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
> Skype: davidfetter  XMPP: david.fet...@gmail.com
>
> Remember to vote!
> Consider donating to Postgres: http://www.postgresql.org/about/donate
>

Re: [HACKERS] POLA violation with \c service=

[Pavel Stehule]

The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:   tested, passed
Spec compliant:   tested, passed
Documentation:tested, passed

no issues with last 007 patch


-- 
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] POLA violation with \c service=

[David Fetter]

On Fri, Feb 20, 2015 at 07:10:29AM +0100, Pavel Stehule wrote:
> Hi
> 
> I am happy with doc changes now.
> 
> When I test last patch, I found sigfault bug, because host =
> PQhost(o_conn); returns NULL. I fexed it - please, see patch 007
> 
> If you are agree with fix, I'll mark this patch as ready for commit.

Thanks for fixing the bug.  Let's go with this.

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


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

[Etsuro Fujita]

On 2015/01/15 16:35, Etsuro Fujita wrote:
> On 2014/12/23 0:36, Tom Lane wrote:
>> Yeah, we need to do something about the PlanRowMark data structure.
>> Aside from the pre-existing issue in postgres_fdw, we need to fix it
>> to support inheritance trees in which more than one rowmark method
>> is being used.  That rte.hasForeignChildren thing is a crock,
> 
>> The idea I'd had about that was to convert the markType field into a
>> bitmask, so that a parent node's markType could represent the logical
>> OR of the rowmark methods being used by all its children.

> As I said before, that seems to me like a good idea.  So I'll update the
> patch based on that if you're okey with it.

Done based on your ideas: (a) add a field to PlanRowMark to record the
original lock strength to fix the postgres_fdw issue and (b) convert its
markType field into a bitmask to support the inheritance trees.  I think
that both work well and that (a) is useful for the other places.

Patch attached, which has been created on top of [1].

Best regards,
Etsuro Fujita

[1] http://www.postgresql.org/message-id/54bcbbf8.3020...@lab.ntt.co.jp
*** a/contrib/file_fdw/input/file_fdw.source
--- b/contrib/file_fdw/input/file_fdw.source
***
*** 148,153  EXPLAIN (VERBOSE, COSTS FALSE) SELECT * FROM agg_csv WHERE a < 0;
--- 148,167 
  SELECT * FROM agg_csv WHERE a < 0;
  RESET constraint_exclusion;
  
+ -- table inheritance tests
+ CREATE TABLE agg (a int2, b float4);
+ ALTER FOREIGN TABLE agg_csv INHERIT agg;
+ SELECT tableoid::regclass, * FROM agg;
+ SELECT tableoid::regclass, * FROM agg_csv;
+ SELECT tableoid::regclass, * FROM ONLY agg;
+ -- updates aren't supported
+ UPDATE agg SET a = 1;
+ DELETE FROM agg WHERE a = 100;
+ -- but this should be ignored
+ SELECT tableoid::regclass, * FROM agg FOR UPDATE;
+ ALTER FOREIGN TABLE agg_csv NO INHERIT agg;
+ DROP TABLE agg;
+ 
  -- privilege tests
  SET ROLE file_fdw_superuser;
  SELECT * FROM agg_text ORDER BY a;
*** a/contrib/file_fdw/output/file_fdw.source
--- b/contrib/file_fdw/output/file_fdw.source
***
*** 249,254  SELECT * FROM agg_csv WHERE a < 0;
--- 249,294 
  (0 rows)
  
  RESET constraint_exclusion;
+ -- table inheritance tests
+ CREATE TABLE agg (a int2, b float4);
+ ALTER FOREIGN TABLE agg_csv INHERIT agg;
+ SELECT tableoid::regclass, * FROM agg;
+  tableoid |  a  |b
+ --+-+-
+  agg_csv  | 100 |  99.097
+  agg_csv  |   0 | 0.09561
+  agg_csv  |  42 |  324.78
+ (3 rows)
+ 
+ SELECT tableoid::regclass, * FROM agg_csv;
+  tableoid |  a  |b
+ --+-+-
+  agg_csv  | 100 |  99.097
+  agg_csv  |   0 | 0.09561
+  agg_csv  |  42 |  324.78
+ (3 rows)
+ 
+ SELECT tableoid::regclass, * FROM ONLY agg;
+  tableoid | a | b 
+ --+---+---
+ (0 rows)
+ 
+ -- updates aren't supported
+ UPDATE agg SET a = 1;
+ ERROR:  cannot update foreign table "agg_csv"
+ DELETE FROM agg WHERE a = 100;
+ ERROR:  cannot delete from foreign table "agg_csv"
+ -- but this should be ignored
+ SELECT tableoid::regclass, * FROM agg FOR UPDATE;
+  tableoid |  a  |b
+ --+-+-
+  agg_csv  | 100 |  99.097
+  agg_csv  |   0 | 0.09561
+  agg_csv  |  42 |  324.78
+ (3 rows)
+ 
+ ALTER FOREIGN TABLE agg_csv NO INHERIT agg;
+ DROP TABLE agg;
  -- privilege tests
  SET ROLE file_fdw_superuser;
  SELECT * FROM agg_text ORDER BY a;
*** a/contrib/postgres_fdw/expected/postgres_fdw.out
--- b/contrib/postgres_fdw/expected/postgres_fdw.out
***
*** 3027,3032  NOTICE:  NEW: (13,"test triggered !")
--- 3027,3544 
  (1 row)
  
  -- ===
+ -- test inheritance features
+ -- ===
+ CREATE TABLE a (aa TEXT);
+ CREATE TABLE loct (aa TEXT, bb TEXT);
+ CREATE FOREIGN TABLE b (bb TEXT) INHERITS (a)
+   SERVER loopback OPTIONS (table_name 'loct');
+ INSERT INTO a(aa) VALUES('aaa');
+ INSERT INTO a(aa) VALUES('');
+ INSERT INTO a(aa) VALUES('a');
+ INSERT INTO a(aa) VALUES('aa');
+ INSERT INTO a(aa) VALUES('aaa');
+ INSERT INTO a(aa) VALUES('');
+ INSERT INTO b(aa) VALUES('bbb');
+ INSERT INTO b(aa) VALUES('');
+ INSERT INTO b(aa) VALUES('b');
+ INSERT INTO b(aa) VALUES('bb');
+ INSERT INTO b(aa) VALUES('bbb');
+ INSERT INTO b(aa) VALUES('');
+ SELECT relname, a.* FROM a, pg_class where a.tableoid = pg_class.oid;
+  relname |aa
+ -+--
+  a   | aaa
+  a   | 
+  a   | a
+  a   | aa
+  a   | aaa
+  a   | 
+  b   | bbb
+  b   | 
+  b   | b
+  b   | bb
+  b   | bbb
+  b   | 
+ (12 rows)
+ 
+ SELECT relname, b.* FROM b, pg_class where b.tableoid = pg_class.oid;
+  relname |aa| bb 
+ -+--+
+  b   | bbb  | 
+  b   |  | 
+  b   | b| 
+  b   | bb   | 
+  b   | 

Re: [HACKERS] Enforce creation of destination folders for source files in pg_regress (Was: pg_regress writes into source tree)

[Michael Paquier]

On Fri, Feb 20, 2015 at 5:50 AM, Peter Eisentraut  wrote:
> On 1/14/15 11:31 PM, Michael Paquier wrote:
>> pg_regress will fail with test suites using only source files if the
>> destination folders do not exist in the code tree. This is annoying
>> because this forces to maintain empty folders sql/ and expected/ with
>> a .gitignore ignoring everything.
>
> We'd still need the .gitignore files somewhere.  Do you want to move
> them one directory up?

I am not sure I am getting what you are pointing to... For extensions
that already have non-empty sql/ and expected/, they should have their
own ignore entries as sql/.gitignore and expected/.gitignore. The
point of the patch is to simplify the code tree of extensions that
need to keep empty sql/ and expected/, for example to be able to run
regression tests after a fresh repository clone for example.
-- 
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 gets attributes from tables in extensions

[Michael Paquier]

On Fri, Feb 20, 2015 at 5:33 AM, Peter Eisentraut  wrote:
> On 2/16/15 2:45 AM, Michael Paquier wrote:
>> While looking at the patch to fix pg_dump with extensions containing
>> tables referencing each other, I got surprised by the fact that
>> getTableAttrs tries to dump table attributes even for tables that are
>> part of an extension. Is that normal?
>> Attached is a patch that I think makes things right, but not dumping any
>> tables that are part of ext_member.
>
> Can you provide an example/test case?  (e.g., which publicly available
> extension contains tables with attributes?)

Sure. Attached is a simplified version of the extension I used for the
other patch on pg_dump.
$ psql -c 'create extension dump_test'
CREATE EXTENSION
$ psql -At -c '\dx+ dump_test'
table aa_tab_fkey
table bb_tab_fkey
$ pg_dump -v 2>&1 | grep "columns and types"
pg_dump: finding the columns and types of table "public"."bb_tab_fkey"
pg_dump: finding the columns and types of table "public"."aa_tab_fkey"
-- 
Michael


dump_test.tar.gz
Description: GNU Zip compressed 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] dblink: add polymorphic functions.

[Michael Paquier]

On Fri, Feb 20, 2015 at 2:50 PM, Corey Huinker  wrote:
> Thanks - completely new to this process, so I'm going to need
> walking-through of it. I promise to document what I learn and try to add
> that to the commitfest wiki. Where can I go for guidance about documentation
> format and regression tests?

Here are some guidelines about how to submit a patch:
https://wiki.postgresql.org/wiki/Submitting_a_Patch

You can have as well a look here to see how extensions like dblink are
structured:
http://www.postgresql.org/docs/devel/static/extend-pgxs.html
What you need to do in the case of your patch is to add necessary test
cases to in sql/dblink.sql for the new functions you have added and
then update the output in expected/. Be sure to not break any existing
things as well. After running the tests in your development
environment output results are available in results then pg_regress
generates a differential file in regressions.diffs.

For the documentation, updating dblink.sgml with the new function
prototypes would be sufficient. With perhaps an example(s) to show
that what you want to add is useful.
Regards,
-- 
Michael


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

[HACKERS] Question about durability and postgresql.

[Alfred Perlstein]

Hello,

We have a combination of 9.3 and 9.4 databases used for logging of data.

We do not need a strong durability guarantee, meaning it is ok if on crash a 
minute or two of data is lost from our logs.  (This is just stats for our 
internal tool).

I am looking at this page:
http://www.postgresql.org/docs/9.4/static/non-durability.html

And it's not clear which setting I should turn on.

What we do NOT want is to lose the entire table or corrupt the database.  We do 
want to gain speed though by not making DATA writes durable.

Which setting is appropriate for this use case?

At a glance it looks like a combination of
1) "Turn off synchronous_commit"
and possibly:
2)  Increase checkpoint_segments and checkpoint_timeout ; this reduces the 
frequency of checkpoints, but increases the storage requirements of /pg_xlog.
3) Turn off full_page_writes; there is no need to guard against partial page 
writes.

The point here is to never get a corrupt database, but in case of crash we 
might lose a few minutes of last transactions.

Any suggestions please?

thank you,
-Alfred

Re: [HACKERS] POLA violation with \c service=

[Pavel Stehule]

Hi

I am happy with doc changes now.

When I test last patch, I found sigfault bug, because host =
PQhost(o_conn); returns NULL. I fexed it - please, see patch 007

If you are agree with fix, I'll mark this patch as ready for commit.

Regards

Pavel

2015-02-19 23:33 GMT+01:00 David Fetter :

> On Thu, Feb 19, 2015 at 09:32:29PM +0100, Pavel Stehule wrote:
> > 2015-02-19 19:51 GMT+01:00 David Fetter :
> >
> > > On Sun, Feb 01, 2015 at 08:38:24AM +0100, Pavel Stehule wrote:
> > >
> > > I'm not sure how best to illustrate those.  Are you thinking of one
> > > example each for the URI and conninfo cases?
> > >
> >
> > some like
> >
> > "most common form is:
> >
> > "\c mydb"
> >
> > but you can use any connection format described
> > (libpq-connect.html#LIBPQ-PARAMKEYWORDS) like
> >
> > "\c postgresql://tom@localhost/mydb?application_name=myapp"
>
> Examples added.
>
> Cheers,
> David.
> --
> David Fetter  http://fetter.org/
> Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
> Skype: davidfetter  XMPP: david.fet...@gmail.com
>
> Remember to vote!
> Consider donating to Postgres: http://www.postgresql.org/about/donate
>
commit f0e71c50590b94da5dafb72a377c7c70b49ce488
Author: root 
Date:   Fri Feb 20 07:04:53 2015 +0100

fix segfault due empty variable host

diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index a637001..340a5d5 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -796,18 +796,20 @@ testdb=>
   
 
   
-\c or \connect [ dbname [ username ] [ host ] [ port ] ]
+\c or \connect  [ { [ dbname [ username ] [ host ] [ port ] ] | conninfo string | URI } ] 
 
 
 Establishes a new connection to a PostgreSQL
-server. If the new connection is successfully made, the
-previous connection is closed. If any of conninfo string, or a URI. If the new connection is
+successfully made, the
+previous connection is closed.  When using positional parameters, if any of dbname, username, host or port are omitted or specified
 as -, the value of that parameter from the
-previous connection is used. If there is no previous
+previous connection is used.  If using positional parameters and there is no previous
 connection, the libpq default for
 the parameter's value is used.
 
@@ -822,6 +824,27 @@ testdb=>
 mechanism that scripts are not accidentally acting on the
 wrong database on the other hand.
 
+
+
+Positional syntax:
+
+=> \c mydb myuser host.dom 6432
+
+
+
+
+The conninfo form detailed in  takes two forms: keyword-value pairs
+
+
+=> \c service=foo
+=> \c "host=localhost port=5432 dbname=mydb connect_timeout=10 sslmode=disable"
+
+
+and URIs:
+
+
+=> \c postgresql://tom@localhost/mydb?application_name=myapp
+
 
   
 
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 7c9f28d..dd9350e 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1607,6 +1607,8 @@ do_connect(char *dbname, char *user, char *host, char *port)
 	PGconn	   *o_conn = pset.db,
 			   *n_conn;
 	char	   *password = NULL;
+	bool		keep_password = true;
+	bool		has_connection_string = false;
 
 	if (!o_conn && (!dbname || !user || !host || !port))
 	{
@@ -1620,15 +1622,31 @@ do_connect(char *dbname, char *user, char *host, char *port)
 		return false;
 	}
 
-	if (!dbname)
-		dbname = PQdb(o_conn);
 	if (!user)
 		user = PQuser(o_conn);
+
 	if (!host)
 		host = PQhost(o_conn);
+
 	if (!port)
 		port = PQport(o_conn);
 
+	if (dbname)
+		has_connection_string = recognized_connection_string(dbname);
+
+	keep_password = (
+			(strcmp(user, PQuser(o_conn)) == 0) &&
+			(!host || strcmp(host, PQhost(o_conn)) == 0) &&
+			(strcmp(port, PQport(o_conn)) == 0) &&
+			 !has_connection_string);
+
+	/*
+	 * Unlike the previous stanzas, changing only the dbname shouldn't
+	 * trigger throwing away the password.
+	 */
+	if (!dbname)
+		dbname = PQdb(o_conn);
+
 	/*
 	 * If the user asked to be prompted for a password, ask for one now. If
 	 * not, use the password from the old connection, provided the username
@@ -1643,9 +1661,13 @@ do_connect(char *dbname, char *user, char *host, char *port)
 	{
 		password = prompt_for_password(user);
 	}
-	else if (o_conn && user && strcmp(PQuser(o_conn), user) == 0)
+	else if (o_conn && keep_password)
 	{
-		password = pg_strdup(PQpass(o_conn));
+		password = PQpass(o_conn);
+		if (password && *password)
+			password = pg_strdup(password);
+		else
+			password = NULL;
 	}
 
 	while (true)
@@ -1653,23 +1675,28 @@ do_connect(char *dbname, char *user, char *host, char *port)
 #define PARAMS_ARRAY_SIZE	8
 		const char **keywords = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*keywords));
 		const char **values = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*values));
-
-		keywords[0] = "host";
-		values[0] = host;

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

[Michael Paquier]

On Fri, Feb 20, 2015 at 5:41 AM, Peter Eisentraut wrote:
> That's cool if you want to add those assertions, but please make them
> separate statements each, like
>
> Assert(vacstmt->options & (VACOPT_FULL | VACOPT_FREEZE) || 
> vacstmt->freeze_min_age == -1);
> Assert(vacstmt->options & (VACOPT_FULL | VACOPT_FREEZE) || 
> vacstmt->freeze_table_age == -1);
> ...
>
> Besides being more readable, this will give you more useful output if
> the assertion fails.

It makes sense. When a manual VACUUM FREEZE without options specified
without parenthesis, VACOPT_FREEZE is not used in gram.y, so ISTM that
we should change them to that instead:
Assert((vacstmt->options & VACOPT_VACUUM) ||
vacstmt->multixact_freeze_table_age == -1);
At least this would check that an ANALYZE does not set those
parameters inappropriately...
-- 
Michael
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 2f3f79d..d2f5baa 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -111,6 +111,14 @@ vacuum(VacuumStmt *vacstmt, Oid relid, bool do_toast,
 	Assert(vacstmt->options & (VACOPT_VACUUM | VACOPT_ANALYZE));
 	Assert((vacstmt->options & VACOPT_VACUUM) ||
 		   !(vacstmt->options & (VACOPT_FULL | VACOPT_FREEZE)));
+	Assert((vacstmt->options & VACOPT_VACUUM) ||
+		   vacstmt->freeze_min_age == -1);
+	Assert((vacstmt->options & VACOPT_VACUUM) ||
+		   vacstmt->freeze_table_age == -1);
+	Assert((vacstmt->options & VACOPT_VACUUM) ||
+		   vacstmt->multixact_freeze_min_age == -1);
+	Assert((vacstmt->options & VACOPT_VACUUM) ||
+		   vacstmt->multixact_freeze_table_age == -1);
 	Assert((vacstmt->options & VACOPT_ANALYZE) || vacstmt->va_cols == NIL);
 
 	stmttype = (vacstmt->options & VACOPT_VACUUM) ? "VACUUM" : "ANALYZE";

-- 
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_check_dir comments and implementation mismatch

[Noah Misch]

On Mon, Feb 02, 2015 at 03:48:33PM -0500, Robert Haas wrote:
> On Sat, Jan 31, 2015 at 8:28 AM, Marco Nenciarini 
>  wrote:
> > I've attached a new version of the patch fixing the missing closedir on
> > readdir error.
> 
> If readir() fails and closedir() succeeds, the return will be -1 but
> errno will be 0.

Out of curiosity, have you seen a closedir() implementation behave that way?
It would violate C99 ("The value of errno is zero at program startup, but is
never set to zero by any library function.") and POSIX.


-- 
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] dblink: add polymorphic functions.

[Corey Huinker]

Thanks - completely new to this process, so I'm going to need
walking-through of it. I promise to document what I learn and try to add
that to the commitfest wiki. Where can I go for guidance about
documentation format and regression tests?

Author field is presently being finicky, reported that to Magnus already.

On Thu, Feb 19, 2015 at 11:00 PM, Michael Paquier  wrote:

> On Fri, Feb 20, 2015 at 7:06 AM, Corey Huinker 
> wrote:
> > Proposed patch attached.
>
> At quick glance, this patch lacks two things:
> - regression test coverage
> - documentation
> (Note: do not forget to add your name in the field "Author" when
> adding a new patch in the CF app).
> --
> Michael
>

Re: [HACKERS] Expanding the use of FLEXIBLE_ARRAY_MEMBER for declarations like foo[1]

[Michael Paquier]

On Fri, Feb 20, 2015 at 2:14 PM, Tom Lane wrote:
> Michael Paquier writes:
>> Thanks for the clarifications and the review. Attached is a new set.
>
> I've reviewed and pushed the 0001 patch (you missed a few things :-().

My apologies. I completely forgot to check for any calls of offsetof
with the structures changed...
-- 
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] Expanding the use of FLEXIBLE_ARRAY_MEMBER for declarations like foo[1]

[Tom Lane]

Michael Paquier  writes:
> Thanks for the clarifications and the review. Attached is a new set.

I've reviewed and pushed the 0001 patch (you missed a few things :-().

Let's see how unhappy the buildfarm is with this before we start on
the rest of them.

regards, tom lane


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

[HACKERS] A better translation version of Chinese for psql/po/zh_CN.po file

[tenjindesc...@gmail.com]

Hello all,
This is my first patch to postgreSQL, so I think I'd better start from a easy 
one.

I am a native Chinese speaker. A lot of words and expressions are different 
between mainsand Chinese and TaiWan/HongKong Chinese. 

For instance TaiWan Chinese will use "档案" to represent "file", but in mainland 
Chinese we use "文件". Things like that make a huge difference in 
'src/bin/psql/po/zh_CN.po', after check this file, I manually translate and 
modify the entire file to make it more native Chinese like, but I think there 
still have some space to improve.

If any of you think this version of translation is useful or have any question 
, please let me know.



Rugal Bernstein
Graduate Student @ University of Manitoba
Data Mining; Machine Learning; Neural Network


better-translation-of-psql-po.diff
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] dblink: add polymorphic functions.

[Michael Paquier]

On Fri, Feb 20, 2015 at 7:06 AM, Corey Huinker  wrote:
> Proposed patch attached.

At quick glance, this patch lacks two things:
- regression test coverage
- documentation
(Note: do not forget to add your name in the field "Author" when
adding a new patch in the CF app).
-- 
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] FDW for Oracle

[Michael Paquier]

On Fri, Feb 20, 2015 at 7:15 AM, Gilberto Castillo wrote:
> This is me problem:
> fdw_test=# SELECT oracle_diag();
>   oracle_diag
> ---
>  oracle_fdw 1.2.0, PostgreSQL 9.1.8, Oracle client 11.2.0.1.0,
> ORACLE_HOME=/opt/oracle/, TNS_ADMIN=/opt/oracle
> (1 row)
> [...]
> My error is: OCIEnvCreate
> ¿What is the solution? Please.

If you have an issue with this fdw, you had better contact the project
maintainers here:
https://github.com/laurenz/oracle_fdw
Regards,
-- 
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] Proposal : REINDEX xxx VERBOSE

[Kyotaro HORIGUCHI]

Hello, 

I showed an extreme number of examples to include *almost of all*
variations of existing syntax of option specification. And showed
what if all variations could be used for all commands. It was
almost a mess. Sorry for the confusion.

I think the issues at our hands are,

 - Options location: at-the-end or right-after-the-keyword?

 - FORCE options to be removed?

 - Decide whether to allow bare word option if the options are to
   be located right after the keyword.

Optinions or thoughts?


Rethinking from here.

At Wed, 11 Feb 2015 14:34:17 -0600, Jim Nasby  wrote 
in <54dbbcc9.1020...@bluetreble.com>
> On 2/5/15 12:01 PM, Tom Lane wrote:
...
> > Meh.  Options-at-the-end seems by far the most sensible style to me.
> > The options-right-after-the-keyword style is a mess, both logically
> > and from a parsing standpoint, and the only reason we have it at all
> > is historical artifact (ask Bruce about the origins of VACUUM ANALYZE
> > over a beer sometime).
...
> I know you were worried about accepting options anywhere because it
> leads to reserved words, but perhaps we could support it just for
> EXPLAIN and VACUUM, and then switch to trailing options if people
> think that would be better.

According to the above discussion, VACUUM and REINDEX should have
trailing options. Tom seems (to me) suggesting that SQL-style
(bare word preceded by WITH) options and Jim suggesting '()'
style options? (Anyway VACUUM gets the *third additional* option
sytle, but it is the different discussion from this)

VACUUM [tname [(cname, ...)]] [({FULL [bool]|FREEZE | [bool]|...})]

 =# VACUUM t1 (FULL, FREEZE);

VACUUM [tname [(cname, ...)]] [WITH [FULL [bool]]|[FREEZE | [bool]|...]

 =# VACUUM t1 WITH FULL;

IMHO, we are so accustomed to call by the names "VACUUM FULL" or
"VACUUM FREEZE" that the both of them look a bit uneasy.


If the new syntax above is added, REINDEX should have *only* the
trailing style.

REINDEX [{INDEX|TABLE|...}] name [(VERBOSE [bool]|...)]

 =# REINDEX TABLE t1 (VERBOSE);

REINDEX [{INDEX|TABLE|...}] name [WITH {VERBOSE [bool]|...}]

 =# REINDEX INDEX i_t1_pkey WITH VERBOSE;

Also, both of them seems to be unintuitive..


EXPLAIN.. it seems to be preferred to be as it is..


As the result, it seems the best way to go on the current syntax
for all of those commands.

Optinions?



At Wed, 18 Feb 2015 23:58:15 +0900, Sawada Masahiko  
wrote in 
> From consistency perspective,  I tend to agree with Robert to put
> option at immediately after command name as follows.
> REINDEX [(VERBOSE | FORCE)] {INDEX | ...} name;

I don't object against it if you prefer it. The remaining issue
is the choice between options-at-the-end or this
options-right-after-the-keyword mentioned above. I prefer the
more messy(:-) one..


> Btw how long will the FORCE command available?

The options is obsolete since 7.4. I think it should have been
fade away long since and it's the time to remove it. But once the
ancient option removed, the above syntax looks a bit uneasy and
the more messy syntax looks natural.


REINDEX [VERBOSE] {INDEX | ...} name;

That do you think about this?

regards,


At Tue, 17 Feb 2015 12:00:13 -0600, Jim Nasby  wrote 
in <54e381ad.1090...@bluetreble.com>
> >VACUUM [FULL] [FREEZE] ... [ANALYZE] [tname [(cname, ...)]
> > | VACUUM [({FULL [bool]|FREEZE [bool]|...}[,...])] [tname [(cname, ...)]
> > | VACUUM [tname [(cname, ...)] [[WITH ]({FULL [bool]|FREEZE
> > | [bool]|...})]
> >
> > REINDEX [{INDEX|TABLE|...}] name [[WITH] (VERBOSE [bool]|...)]
> >
> > EXPLAIN [[WITH] ({ANALYZE [bool]|VERBOSE [bool]|... [,...]})]
> > 
> >
> > For concrete examples, the lines prefixed by asterisk are in new
> > syntax.
> 
> If I could choose only one for explain, I would find it easier to be
> up front. That way you do the explain part on one line and just paste
> the query after that.
..
> >   VACUUM FULL table1;
> >   VACUUM ANALYZE table1 (col1);
> >   VACUUM (ANALYZE, VERBOSE) table1 (col1);
> > *VACUUM table1 WITH (FREEZE on)
> > *VACUUM table1 (cola) WITH (ANALYZE)
> > *VACUUM table1 WITH (ANALYZE)
> > *VACUUM table1 (FREEZE on)
> >
> > The fifth example looks quite odd.
> 
> I don't think we need to allow both () and WITH... I'd say one or the
> other, preferably ().

-- 
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] pgaudit - an auditing extension for PostgreSQL

[David Steele]

On 2/18/15 10:29 PM, Fujii Masao wrote:
> On Thu, Feb 19, 2015 at 12:25 AM, David Steele  wrote:
>>> The pg_audit doesn't log BIND parameter values when prepared statement is 
>>> used.
>>> Seems this is an oversight of the patch. Or is this intentional?
>>
>> It's actually intentional - following the model I talked about in my
>> earlier emails, the idea is to log statements only.
> 
> Is this acceptable for audit purpose in many cases? Without the values,
> I'm afraid that it's hard to analyze what table records are affected by
> the statements from the audit logs. I was thinking that identifying the
> data affected is one of important thing for the audit. If I'm malicious DBA,
> I will always use the extended protocol to prevent the values from being
> audited when I execute the statement.

I agree with you, but I wonder how much is practical at this stage.
Let me think about it and see what I can come up with.

-- 
- David Steele
da...@pgmasters.net



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] Exposing the stats snapshot timestamp to SQL

[Tom Lane]

Matt Kelly  writes:
>> Yeah.  The only use-case that's been suggested is detecting an
>> unresponsive stats collector, and the main timestamp should be plenty for
>> that.

> The problem with doing highly granular snapshots is that the postgres
> counters are monotonically increasing, but only when stats are published.
> Currently you have no option except to divide by the delta of now() between
> the polling intervals. If you poll every 2 seconds the max error is about
> .5/2 or 25%.  It makes reading those numbers a bit noisy.  Using
> (snapshot_timestamp_new
> - snapshot_timestamp_old) as the denominator in that calculation should
> help to smooth out that noise and show a clearer picture.

Ah, interesting!  Thanks for pointing out another use case.

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] Allow "snapshot too old" error, to prevent bloat

[David Steele]

On 2/19/15 1:54 PM, Kevin Grittner wrote:
> Rod Taylor  wrote:
> 
>> Would pg_dump be able to opt-out of such a restriction?
> 
> I don't see how, since vacuum would be removing recently dead
> tuples that are still visible; the alternative to getting a
> "snapshot too old" error when reading a page which could be
> affected is to return incorrect results, and nobody wants that.
> The best you could do if you wanted to run pg_dump (or similar) and
> it might take more time than your old_snapshot_threshold would be
> to increase the threshold, reload, dump, set it back to the
> "normal" setting, and reload again.

While I think pg_dump is a great solution for small to medium
installations, there are a number of excellent file-based backup options
available.  Anyone who is seriously worried about bloat (or locking)
should be looking to those solutions.

-- 
- David Steele
da...@pgmasters.net



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] Exposing the stats snapshot timestamp to SQL

[Matt Kelly]

>
> Yeah.  The only use-case that's been suggested is detecting an
> unresponsive stats collector, and the main timestamp should be plenty for
> that.

Lately, I've spent most of my time doing investigation into increasing
qps.  Turned out we've been able to triple our throughput by monitoring
experiments at highly granular time steps (1 to 2 seconds).  Effects that
were invisible with 30 second polls of the stats were obvious with 2 second
polls.

The problem with doing highly granular snapshots is that the postgres
counters are monotonically increasing, but only when stats are published.
Currently you have no option except to divide by the delta of now() between
the polling intervals. If you poll every 2 seconds the max error is about
.5/2 or 25%.  It makes reading those numbers a bit noisy.  Using
(snapshot_timestamp_new
- snapshot_timestamp_old) as the denominator in that calculation should
help to smooth out that noise and show a clearer picture.

However, I'm happy with the committed version. Thanks Tom.

- Matt K.


On Thu, Feb 19, 2015 at 9:40 PM, Tom Lane  wrote:

> Matt Kelly  writes:
> > Attached is the fixed version. (hopefully with the right mime-type and
> > wrong extension.  Alas, gmail doesn't let you set mime-types; time to
> find
> > a new email client...)
>
> Committed with a couple of changes:
>
> * I changed the function name from pg_stat_snapshot_timestamp to
> pg_stat_get_snapshot_timestamp, which seemed to me to be the style
> in general use in the stats stuff for inquiry functions.
>
> * The function should be marked stable not volatile, since its value
> doesn't change any faster than all the other stats inquiry functions.
>
> regards, tom lane
>

Re: [HACKERS] Exposing the stats snapshot timestamp to SQL

[Tom Lane]

Tomas Vondra  writes:
> Well, the patch also does this:


> *** 28,34  SELECT pg_sleep_for('2 seconds');
>   CREATE TEMP TABLE prevstats AS
>   SELECT t.seq_scan, t.seq_tup_read, t.idx_scan, t.idx_tup_fetch,
>  (b.heap_blks_read + b.heap_blks_hit) AS heap_blks,
> !(b.idx_blks_read + b.idx_blks_hit) AS idx_blks
> FROM pg_catalog.pg_stat_user_tables AS t,
>  pg_catalog.pg_statio_user_tables AS b
>WHERE t.relname='tenk2' AND b.relname='tenk2';
> --- 28,35 
>   CREATE TEMP TABLE prevstats AS
>   SELECT t.seq_scan, t.seq_tup_read, t.idx_scan, t.idx_tup_fetch,
>  (b.heap_blks_read + b.heap_blks_hit) AS heap_blks,
> !(b.idx_blks_read + b.idx_blks_hit) AS idx_blks,
> !pg_stat_snapshot_timestamp() as snap_ts
> FROM pg_catalog.pg_stat_user_tables AS t,
>  pg_catalog.pg_statio_user_tables AS b
>WHERE t.relname='tenk2' AND b.relname='tenk2';
> ***


That's merely a regression test to verify that the value appears to
advance from time to time ... I don't think it has any larger meaning.
(It will be interesting to see what this new test query reports in the
transient buildfarm failures we still occasionally see in the stats
test.)

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] Exposing the stats snapshot timestamp to SQL

[Tom Lane]

Matt Kelly  writes:
> Attached is the fixed version. (hopefully with the right mime-type and
> wrong extension.  Alas, gmail doesn't let you set mime-types; time to find
> a new email client...)

Committed with a couple of changes:

* I changed the function name from pg_stat_snapshot_timestamp to
pg_stat_get_snapshot_timestamp, which seemed to me to be the style
in general use in the stats stuff for inquiry functions.

* The function should be marked stable not volatile, since its value
doesn't change any faster than all the other stats inquiry functions.

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_upgrade and rsync

[David Steele]

On 2/19/15 11:57 AM, Bruce Momjian wrote:
> On Wed, Jan 28, 2015 at 09:26:11PM -0800, Josh Berkus wrote:
>>
>> 3. Check that the replica is not very lagged.  If it is, wait for
>> traffic to die down and for it to catch up.
> 
> Now that 9.4.1 is released, I would like to get this doc patch applied
> --- it will close the often-requested feature of how to pg_upgrade slave
> clusters.
> 
> I wasn't happy with Josh's specification above that the "replica is not
> very lagged", so I added a bullet point to check the pg_controldata
> output to verify that the primary and standby servers are synchronized.
> 
> Yes, this adds even more complication to the pg_upgrade instructions,
> but it is really more of the same complexity.  pg_upgrade really needs
> an install-aware and OS-aware tool on top of it to automate much of
> this.

#3 bothered me as well because it was not specific enough.  I like what
you've added to clarify the procedure.

-- 
- David Steele
da...@pgmasters.net



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] Exposing the stats snapshot timestamp to SQL

[Tomas Vondra]

On 20.2.2015 02:58, Tom Lane wrote:
> Tomas Vondra  writes:
>> I see the patch only works with the top-level snapshot timestamp,
>> stored in globalStats, but since 9.3 (when the stats were split
>> into per-db files) we track per-database timestamps too.
> 
>> Shouldn't we make those timestamps accessible too? It's not
>> necessary for detecting unresponsive statistics collector (if it's
>> stuck it's not writing anything, so the global timestamp will be
>> old too), but it seems more appropriate for querying database-level
>> stats to query database-level timestamp too.
> 
> I'm inclined to say not; I think that's exposing an implementation 
> detail that we might regret exposing, later. (IOW, it's not hard to 
> think of rearrangements that might mean there wasn't a per-database 
> stamp anymore.)

Fair point, but isn't the global timestamp an implementation detail too?
Although we're less likely to remove the global timestamp, no doubt
about that ...

>> But maybe that's not necessary, because to query database stats you
>> have to be connected to that particular database and that should
>> write fresh stats, so the timestamps should not be very different.
> 
> Yeah.  The only use-case that's been suggested is detecting an 
> unresponsive stats collector, and the main timestamp should be plenty
> for that.

Well, the patch also does this:


*** 28,34  SELECT pg_sleep_for('2 seconds');
  CREATE TEMP TABLE prevstats AS
  SELECT t.seq_scan, t.seq_tup_read, t.idx_scan, t.idx_tup_fetch,
 (b.heap_blks_read + b.heap_blks_hit) AS heap_blks,
!(b.idx_blks_read + b.idx_blks_hit) AS idx_blks
FROM pg_catalog.pg_stat_user_tables AS t,
 pg_catalog.pg_statio_user_tables AS b
   WHERE t.relname='tenk2' AND b.relname='tenk2';
--- 28,35 
  CREATE TEMP TABLE prevstats AS
  SELECT t.seq_scan, t.seq_tup_read, t.idx_scan, t.idx_tup_fetch,
 (b.heap_blks_read + b.heap_blks_hit) AS heap_blks,
!(b.idx_blks_read + b.idx_blks_hit) AS idx_blks,
!pg_stat_snapshot_timestamp() as snap_ts
FROM pg_catalog.pg_stat_user_tables AS t,
 pg_catalog.pg_statio_user_tables AS b
   WHERE t.relname='tenk2' AND b.relname='tenk2';
***

i.e. it adds the timestamp into queries selecting from database-level
catalogs. But OTOH we're usually using now() here, so the global
snapshot is an improvement here, and if the collector is stuck it's not
going to update of the timestamps.

So I'm OK with this patch.

-- 
Tomas Vondrahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, 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] Exposing the stats snapshot timestamp to SQL

[Tom Lane]

Tomas Vondra  writes:
> I see the patch only works with the top-level snapshot timestamp, stored
> in globalStats, but since 9.3 (when the stats were split into per-db
> files) we track per-database timestamps too.

> Shouldn't we make those timestamps accessible too? It's not necessary
> for detecting unresponsive statistics collector (if it's stuck it's not
> writing anything, so the global timestamp will be old too), but it seems
> more appropriate for querying database-level stats to query
> database-level timestamp too.

I'm inclined to say not; I think that's exposing an implementation detail
that we might regret exposing, later.  (IOW, it's not hard to think of
rearrangements that might mean there wasn't a per-database stamp anymore.)

> But maybe that's not necessary, because to query database stats you have
> to be connected to that particular database and that should write fresh
> stats, so the timestamps should not be very different.

Yeah.  The only use-case that's been suggested is detecting an
unresponsive stats collector, and the main timestamp should be plenty for
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] Allow "snapshot too old" error, to prevent bloat

[Stephen Frost]

Kevin,

* Kevin Grittner (kgri...@ymail.com) wrote:
> Stephen Frost  wrote:
> > Kevin Grittner (kgri...@ymail.com) wrote:
> >> (1)  They have a pool of connections each of which can have several
> >> long-running cursors, so the limit from that isn't just doubling
> >> the size of their database, it is limiting it to some two-or-three
> >> digit multiple of the necessary size.
> >
> > This strikes me as a bit off-the-cuff; was an analysis done which
> > deteremined that would be the result?
> 
> If it sounds like off-the-cuff it is because I am summarizing the
> mountain of data which has been accumulated over weeks of
> discussions and many rounds of multi-day tests using their actual
> software driven by a software test driver that simulates users,
> with "think time" and all.

My apologies for, unintentionally, implying that you hadn't done
sufficient research or testing.  That certainly was *not* my intent and
it's absolutely clear to me that you've spent quite a bit of time
analyzing this problem.

What I was trying to get at is that the approach which I outlined was,
perhaps, not closely analyzed.  Now, analyzing a problem based on a
solution which doesn't actually exist isn't exactly trivial, and while
it might be possible to do in this case it'd probably be easier to
simply write the code and the review the results than perform an
independent analysis.  I do feel that the solution might have been
dismissed on the basis of "doubling the database for each long-running
transaction is not acceptable" or even "having bloat because of a
long-running transaction is not acceptable," but really analyzing what
would actually happen is a much more difficult exercise.

> With the two patches I submitted, bloat stabilized at less than 5%
> except for some btree indexes which followed pattern of inserting
> at the end and deleting most (but not all) of the entries over
> time.  I've been working on a separate patch for that, but it's not
> ready to present, so I probably won't post that until the first CF
> of the next release -- unless someone's interested enough to want
> to discuss it before then.

Understood.  I'd encourage you to post your thoughts on it- I'm
certainly interested in it, even if it isn't something we'll be able to
really work on for 9.5.

> > That, really, is the crux here- they've already got support for dealing
> > with it the way Oracle did and they'd like PG to do that too.
> > Unfortunately, that, by itself, isn't a good reason for a particular
> > capability (we certainly aren't going to be trying to duplicate PL/SQL
> > in PG any time soon).
> 
> I understand that, and if the community doesn't want this style of
> limitation on bloat we can make it part of PPAS (which *does*
> duplicate PL/SQL, among other things).  We are offering it to the
> community first.

That is certainly appreciated and I'd encourage you to continue to do
so.  I do feel that the community, in general, is interested in these
kinds of knobs- it's just a question of implemenation and making sure
that it's a carefully considered solution and not a knee-jerk reaction
to match what another RDBMS does.

> >>> I'm not against having a knob like this, which is defaulted to off,
> >>
> >> Thanks!  I'm not sure that amounts to a +1, but at least it doesn't
> >> sound like a -1.  :-)
> >
> > So, at the time I wrote that, I wasn't sure if it was a +1 or not
> > myself.  I've been thinking about it since then, however, and I'm
> > leaning more towards having the capability than not, so perhaps that's a
> > +1, but it doesn't excuse the need to come up with an implementation
> > that everyone can be happy with and what you've come up with so far
> > doesn't have a lot of appeal, based on the feedback (I've only glanced
> > through it myself, but I agree with Andres and Tom that it's a larger
> > footprint than we'd want for this and the number of places having to be
> > touched is concerning as it could lead to future bugs).
> 
> This conversation has gotten a little confusing because some of my
> posts seem to have been held up in the email systems for some
> reason, and are arriving out-of-order (and at least one I don't see
> yet).  But I have been responding to these points.  For one thing I
> stated four days ago that I now feel that the metric for
> determining that a snapshot is "old" should be *time*, not
> transactions IDs consumed, and offered to modify that patch in that
> direction if people agreed.  I now see one person agreeing and
> several people arguing that I should do just that (as though they
> had not seem me propose it), so I'll try to get a new version out
> today like that.  In any event, I am sure it is possible and feel
> that it would be better.  (Having posted that now at least 4 times,
> hopefully people will pick up on it.)

I agree with others that having it be time-based would be better.  I
did see that but it's really an independent consideration from the
footprint of the patch.

> For anothe

Re: [HACKERS] CATUPDATE confusion?

[Adam Brightwell]

Peter,

Thanks for the review and feedback.

> One of the interesting behaviors (or perhaps not) is how
> > 'pg_class_aclmask' handles an invalid role id when checking permissions
> > against 'rolsuper' instead of 'rolcatupdate'.
>
> I'd get rid of that whole check, not just replace rolcatupdate by rolsuper.
>

Ah yes, that's a good point.  I have updated and attached a new version of
the patch.

Thanks,
Adam

-- 
Adam Brightwell - adam.brightw...@crunchydatasolutions.com
Database Engineer - www.crunchydatasolutions.com


remove-catupdate-v2.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] POLA violation with \c service=

[David Fetter]

On Thu, Feb 19, 2015 at 09:32:29PM +0100, Pavel Stehule wrote:
> 2015-02-19 19:51 GMT+01:00 David Fetter :
> 
> > On Sun, Feb 01, 2015 at 08:38:24AM +0100, Pavel Stehule wrote:
> >
> > I'm not sure how best to illustrate those.  Are you thinking of one
> > example each for the URI and conninfo cases?
> >
> 
> some like
> 
> "most common form is:
> 
> "\c mydb"
> 
> but you can use any connection format described
> (libpq-connect.html#LIBPQ-PARAMKEYWORDS) like
> 
> "\c postgresql://tom@localhost/mydb?application_name=myapp"

Examples added.

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index a637001..340a5d5 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -796,18 +796,20 @@ testdb=>
   
 
   
-\c or \connect [ 
dbname [ username ] [ host ] [ port ] ]
+\c or \connect  [ 
{ [ dbname [ username ] [ host ] [ port ] ] | conninfo string | URI } ] 
 
 
 Establishes a new connection to a PostgreSQL
-server. If the new connection is successfully made, the
-previous connection is closed. If any of conninfo string, or a URI. 
If the new connection is
+successfully made, the
+previous connection is closed.  When using positional parameters, if 
any of dbname, username, host or port are omitted or specified
 as -, the value of that parameter from the
-previous connection is used. If there is no previous
+previous connection is used.  If using positional parameters and there 
is no previous
 connection, the libpq default for
 the parameter's value is used.
 
@@ -822,6 +824,27 @@ testdb=>
 mechanism that scripts are not accidentally acting on the
 wrong database on the other hand.
 
+
+
+Positional syntax:
+
+=> \c mydb myuser host.dom 6432
+
+
+
+
+The conninfo form detailed in  takes 
two forms: keyword-value pairs
+
+
+=> \c service=foo
+=> \c "host=localhost port=5432 dbname=mydb connect_timeout=10 
sslmode=disable"
+
+
+and URIs:
+
+
+=> \c postgresql://tom@localhost/mydb?application_name=myapp
+
 
   
 
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 7c9f28d..ec86e52 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1607,6 +1607,8 @@ do_connect(char *dbname, char *user, char *host, char 
*port)
PGconn *o_conn = pset.db,
   *n_conn;
char   *password = NULL;
+   boolkeep_password = true;
+   boolhas_connection_string = false;
 
if (!o_conn && (!dbname || !user || !host || !port))
{
@@ -1620,15 +1622,31 @@ do_connect(char *dbname, char *user, char *host, char 
*port)
return false;
}
 
-   if (!dbname)
-   dbname = PQdb(o_conn);
if (!user)
user = PQuser(o_conn);
+
if (!host)
host = PQhost(o_conn);
+
if (!port)
port = PQport(o_conn);
 
+   if (dbname)
+   has_connection_string = recognized_connection_string(dbname);
+
+   keep_password = (
+   (strcmp(user, PQuser(o_conn)) == 0) &&
+   (strcmp(host, PQhost(o_conn)) == 0) &&
+   (strcmp(port, PQport(o_conn)) == 0) &&
+!has_connection_string);
+
+   /*
+* Unlike the previous stanzas, changing only the dbname shouldn't
+* trigger throwing away the password.
+*/
+   if (!dbname)
+   dbname = PQdb(o_conn);
+
/*
 * If the user asked to be prompted for a password, ask for one now. If
 * not, use the password from the old connection, provided the username
@@ -1643,9 +1661,13 @@ do_connect(char *dbname, char *user, char *host, char 
*port)
{
password = prompt_for_password(user);
}
-   else if (o_conn && user && strcmp(PQuser(o_conn), user) == 0)
+   else if (o_conn && keep_password)
{
-   password = pg_strdup(PQpass(o_conn));
+   password = PQpass(o_conn);
+   if (password && *password)
+   password = pg_strdup(password);
+   else
+   password = NULL;
}
 
while (true)
@@ -1653,23 +1675,28 @@ do_connect(char *dbname, char *user, char *host, char 
*port)
 #define PARAMS_ARRAY_SIZE  8
const char **keywords = pg_malloc(PARAMS_ARRAY_SIZE * 
sizeof(*keywords));
const char **values = pg_malloc(PARAMS_ARRAY_SIZE * 
sizeof(*values));
+   

Re: [HACKERS] Allow "snapshot too old" error, to prevent bloat

[Ants Aasma]

On Feb 19, 2015 10:31 PM, "Kevin Grittner"  wrote:
> > What about having the long running snapshots declare their working
> > set, and then only take them into account for global xmin for
> > relations that are in the working set? Like a SET TRANSACTION WORKING
> > SET command. This way the error is deterministic, vacuum on the high
> > churn tables doesn't have to wait for the old transaction delay to
> > expire and we avoid a hard to tune GUC (what do I need to set
> > old_snapshot_threshold to, to reduce bloat while not having "normal"
> > transactions abort).
>
> Let me make sure I understand your suggestion.  You are suggesting
> that within a transaction you can declare a list of tables which
> should get the "snapshot too old" error (or something like it) if a
> page is read which was modified after the snapshot was taken?
> Snapshots within that transaction would not constrain the effective
> global xmin for purposes of vacuuming those particular tables?

Sorry, I should have been clearer. I'm proposing that a transaction can
declare what tables it will access. After that the transaction will
constrain xmin for only those tables. Accessing any other table would give
an error immediately.

> If I'm understanding your proposal, that would help some of the
> cases the "snapshot too old" case addresses, but it would not
> handle the accidental "idle in transaction" case (e.g., start a
> repeatable read transaction, run one query, then sit idle
> indefinitely).  I don't think it would work quite as well for some
> of the other cases I've seen, but perhaps it could be made to fit
> if we could figure out the accidental "idle in transaction" case.

Accidental idle in transaction seems better handled by just terminating
transactions older than some timeout. This is already doable externally,
but an integrated solution would be nice if we could figure out how the
permissions for setting such a timeout work.

> I also think it might be hard to develop a correct global xmin for
> vacuuming a particular table with that solution.  How do you see
> that working?

I'm imagining the long running transaction would register it's xmin in a
shared map keyed by relation for each referenced relation and remove the
xmin from procarray. Vacuum would access this map by relation, determine
the minimum and use that if it's earlier than the global xmin. I'm being
deliberately vague here about the datastructure in shared memory as I don't
have a great idea what to use there. It's somewhat similar to the lock
table in that in theory the size is unbounded, but in practice it's
expected to be relatively tiny.

Regards,
Ants Aasma

Re: [HACKERS] FDW for Oracle

[Gilberto Castillo]

Hello

This is me problem:

fdw_test=# SELECT oracle_diag();
  oracle_diag
---
 oracle_fdw 1.2.0, PostgreSQL 9.1.8, Oracle client 11.2.0.1.0,
ORACLE_HOME=/opt/oracle/, TNS_ADMIN=/opt/oracle
(1 row)


In PostgreSQL:
SELECT pg_backend_pid();
5543

In a second session in the shell:
ps -p5543 -oppid=
5290

As root or PostgreSQL OS user:
cat /proc/5290/environ | xargs -0 -n1

LD_LIBRARY_PATH=/usr/lib:/opt/PostgreSQL/9.1/lib:/opt/oracle/instantclient:$LD_LIBRARY_PATH
HOME=/opt/PostgreSQL/9.1
TNS_ADMIN=/opt/oracle
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/opt/PostgreSQL/9.1/bin:/usr/lib/postgresql/9.1/bin:/opt/PostgreSQL/9.1/bin/bin
ORACLE_HOME=/opt/oracle
DYLD_LIBRARY_PATH=/opt/oracle/instantclient:$ORACLE_HOME


My error is: OCIEnvCreate

¿What is the solution? Please.


Saludos,
Gilberto Castillo
La Habana, Cuba
--- 
This message was processed by Kaspersky Mail Gateway 5.6.28/RELEASE running at 
host imx3.etecsa.cu
Visit our web-site: , 

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

[HACKERS] dblink: add polymorphic functions.

[Corey Huinker]

In the course of writing a small side project which hopefully will make its
way onto pgxn soon, I was writing functions that had a polymorphic result
set.

create function foo( p_row_type anyelement, p_param1 ...) returns setof
anyelement

Inside that function would be multiple calls to dblink() in both
synchronous and async modes. It is a requirement of the function that each
query return a result set conforming to the structure passed into
p_row_type, but there was no way for me to express that.

Unfortunately, there's no way to say


select * from dblink_get_result('connname') as ;


Instead, I had to generate the query as a string like this.

with x as (
select  a.attname || ' ' || pg_catalog.format_type(a.atttypid,
a.atttypmod) as sql_text
frompg_catalog.pg_attribute a
where   a.attrelid = pg_typeof(p_row_type)::text::regclass
and a.attisdropped is false
and a.attnum > 0
order by a.attnum )
select  format('select * from dblink_get_result($1) as
t(%s)',string_agg(x.sql_text,','))
fromx;

Moreover, I'm now executing this string dynamically, incurring reparsing
and replanning each time (and if all goes well, this would be executed many
times). Granted, I could avoid that by rewriting the stored procedure in C
and using prepared statements (not available in PL/PGSQL), but it seemed a
shame that dblink couldn't itself handle this polymorphism.

So with a little help, we were able to come up with polymorphic set
returning dblink functions.

Below is the results of the patch applied to a stock 9.4 installation.

[local]:ubuntu@dblink_test# create extension dblink;
CREATE EXTENSION
Time: 12.778 ms
[local]:ubuntu@dblink_test# \df dblink
   List of functions
 Schema |  Name  | Result data type |   Argument data types   |
 Type
++--+-+
 public | dblink | SETOF record | text|
normal
 public | dblink | SETOF anyelement | text, anyelement|
normal
 public | dblink | SETOF record | text, boolean   |
normal
 public | dblink | SETOF anyelement | text, boolean, anyelement   |
normal
 public | dblink | SETOF record | text, text  |
normal
 public | dblink | SETOF anyelement | text, text, anyelement  |
normal
 public | dblink | SETOF record | text, text, boolean |
normal
 public | dblink | SETOF anyelement | text, text, boolean, anyelement |
normal
(8 rows)

[local]:ubuntu@dblink_test# *select * from
dblink('dbname=dblink_test','select * from pg_tables order by tablename
limit 2',null::pg_tables);*
 schemaname |  tablename   | tableowner | tablespace | hasindexes |
hasrules | hastriggers
+--++++--+-
 pg_catalog | pg_aggregate | postgres   || t  | f
 | f
 pg_catalog | pg_am| postgres   || t  | f
 | f
(2 rows)

Time: 6.813 ms



Obviously, this is a trivial case, but it shows that the polymorphic
function works as expected, and the code that uses it will be a lot more
straightforward.

Proposed patch attached.
diff --git a/contrib/dblink/dblink--1.1.sql b/contrib/dblink/dblink--1.1.sql
index 8733553..bf5ddaa 100644
--- a/contrib/dblink/dblink--1.1.sql
+++ b/contrib/dblink/dblink--1.1.sql
@@ -121,6 +121,26 @@ RETURNS setof record
 AS 'MODULE_PATHNAME','dblink_record'
 LANGUAGE C STRICT;
 
+CREATE FUNCTION dblink (text, text, anyelement)
+RETURNS setof anyelement
+AS 'MODULE_PATHNAME','dblink_record'
+LANGUAGE C;
+
+CREATE FUNCTION dblink (text, text, boolean, anyelement)
+RETURNS setof anyelement
+AS 'MODULE_PATHNAME','dblink_record'
+LANGUAGE C;
+
+CREATE FUNCTION dblink (text, anyelement)
+RETURNS setof anyelement
+AS 'MODULE_PATHNAME','dblink_record'
+LANGUAGE C;
+
+CREATE FUNCTION dblink (text, boolean, anyelement)
+RETURNS setof anyelement
+AS 'MODULE_PATHNAME','dblink_record'
+LANGUAGE C;
+
 CREATE FUNCTION dblink_exec (text, text)
 RETURNS text
 AS 'MODULE_PATHNAME','dblink_exec'
@@ -188,6 +208,16 @@ RETURNS SETOF record
 AS 'MODULE_PATHNAME', 'dblink_get_result'
 LANGUAGE C STRICT;
 
+CREATE FUNCTION dblink_get_result(text, anyelement)
+RETURNS SETOF anyelement
+AS 'MODULE_PATHNAME', 'dblink_get_result'
+LANGUAGE C;
+
+CREATE FUNCTION dblink_get_result(text, bool, anyelement)
+RETURNS SETOF anyelement
+AS 'MODULE_PATHNAME', 'dblink_get_result'
+LANGUAGE C;
+
 CREATE FUNCTION dblink_get_connections()
 RETURNS text[]
 AS 'MODULE_PATHNAME', 'dblink_get_connections'
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index 9fe750e..eb7f5f9 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -680,27 +680,68 @@ dblink_record_internal(FunctionCallInfo fcinfo, bool 
is_async)
 
if (!is_async)
{
-   if (PG_NARGS() == 3)
+   if (PG_NARGS() 

[HACKERS] FDW for Oracle

[Gilberto Castillo]

Hello

This is me problem:




Saludos,
Gilberto Castillo
La Habana, Cuba


--- 
This message was processed by Kaspersky Mail Gateway 5.6.28/RELEASE running at 
host imx3.etecsa.cu
Visit our web-site: , 

-- 
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] Turning recovery.conf into GUCs

[Josh Berkus]

On 02/19/2015 12:23 PM, Peter Eisentraut wrote:
> On 1/6/15 4:22 PM, Peter Eisentraut wrote:
>> That said, there is a much simpler way to achieve that specific
>> functionality: Expose all the recovery settings as fake read-only GUC
>> variables.  See attached patch for an example.
> 
> The commit fest app has this as the patch of record.  I don't think
> there is a real updated patch under consideration here.  This item
> should probably not be in the commit fest at all.

What happened to the original patch?  Regardless of what we do, keeping
recovery.conf the way it is can't be what we go forward with.

--
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] CATUPDATE confusion?

[Peter Eisentraut]

On 12/29/14 7:16 PM, Adam Brightwell wrote:
> Given this discussion, I have attached a patch that removes CATUPDATE
> for review/discussion.
> 
> One of the interesting behaviors (or perhaps not) is how
> 'pg_class_aclmask' handles an invalid role id when checking permissions
> against 'rolsuper' instead of 'rolcatupdate'.

I'd get rid of that whole check, not just replace rolcatupdate by rolsuper.


-- 
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 "snapshot too old" error, to prevent bloat

[Andrew Dunstan]

On 02/19/2015 03:31 PM, Kevin Grittner wrote:


What about having the long running snapshots declare their working
set, and then only take them into account for global xmin for
relations that are in the working set? Like a SET TRANSACTION WORKING
SET command. This way the error is deterministic, vacuum on the high
churn tables doesn't have to wait for the old transaction delay to
expire and we avoid a hard to tune GUC (what do I need to set
old_snapshot_threshold to, to reduce bloat while not having "normal"
transactions abort).

Let me make sure I understand your suggestion.  You are suggesting
that within a transaction you can declare a list of tables which
should get the "snapshot too old" error (or something like it) if a
page is read which was modified after the snapshot was taken?
Snapshots within that transaction would not constrain the effective
global xmin for purposes of vacuuming those particular tables?



I thought it meant that the declared tables would only be vacuumed 
conservatively, so the transaction would expect not to see "snapshot too 
old" from them.


cheers

andrew





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

Re: [HACKERS] Enforce creation of destination folders for source files in pg_regress (Was: pg_regress writes into source tree)

[Peter Eisentraut]

On 1/14/15 11:31 PM, Michael Paquier wrote:
> pg_regress will fail with test suites using only source files if the
> destination folders do not exist in the code tree. This is annoying
> because this forces to maintain empty folders sql/ and expected/ with
> a .gitignore ignoring everything.

We'd still need the .gitignore files somewhere.  Do you want to move
them one directory up?



-- 
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] Strange assertion using VACOPT_FREEZE in vacuum.c

[Peter Eisentraut]

On 2/18/15 1:26 AM, Michael Paquier wrote:
> On Wed, Feb 18, 2015 at 10:06 AM, Michael Paquier wrote:
>> Yes, the existing assertion is right. My point is that it is strange
>> that we do not check the values of freeze parameters for an ANALYZE
>> query, which should be set to -1 all the time. If this is thought as
>> not worth checking, I'll drop this patch and my concerns.
> 
> Perhaps this explains better what I got in mind, aka making the
> assertion stricter:
> Assert((vacstmt->options & VACOPT_VACUUM) ||
> -  !(vacstmt->options & (VACOPT_FULL | VACOPT_FREEZE)));
> +  ((vacstmt->options & (VACOPT_FULL | VACOPT_FREEZE)) == 0 &&
> +   vacstmt->freeze_min_age < 0 &&
> +   vacstmt->freeze_table_age < 0 &&
> +   vacstmt->multixact_freeze_min_age < 0 &&
> +   vacstmt->multixact_freeze_table_age < 0));

That's cool if you want to add those assertions, but please make them
separate statements each, like

Assert(vacstmt->options & (VACOPT_FULL | VACOPT_FREEZE) || 
vacstmt->freeze_min_age == -1);
Assert(vacstmt->options & (VACOPT_FULL | VACOPT_FREEZE) || 
vacstmt->freeze_table_age == -1);
...

Besides being more readable, this will give you more useful output if
the assertion fails.



-- 
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 gets attributes from tables in extensions

[Peter Eisentraut]

On 2/16/15 2:45 AM, Michael Paquier wrote:
> While looking at the patch to fix pg_dump with extensions containing
> tables referencing each other, I got surprised by the fact that
> getTableAttrs tries to dump table attributes even for tables that are
> part of an extension. Is that normal?
> Attached is a patch that I think makes things right, but not dumping any
> tables that are part of ext_member.

Can you provide an example/test case?  (e.g., which publicly available
extension contains tables with attributes?)



-- 
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 "snapshot too old" error, to prevent bloat

[Kevin Grittner]

Ants Aasma  wrote:

> If I understood the issue correctly, you have long running snapshots
> accessing one part of the data, while you have high churn on a
> disjoint part of data. We would need to enable vacuum on the high
> churn data while still being able to run those long queries. The
> "snapshot too old" solution limits the maximum age of global xmin at
> the cost of having to abort transactions that are older than this and
> happen to touch a page that was modified after they were started.

Pretty much.  It's not quite as "black and white" regarding high
churn and stable tables, though -- at least for the customer whose
feedback drove my current work on containing bloat.  They are
willing to tolerate an occasional "snapshot too old" on a cursor,
and they can automatically pick up again by restarting the cursor
with a new "lower limit" and a new snapshot.

> What about having the long running snapshots declare their working
> set, and then only take them into account for global xmin for
> relations that are in the working set? Like a SET TRANSACTION WORKING
> SET command. This way the error is deterministic, vacuum on the high
> churn tables doesn't have to wait for the old transaction delay to
> expire and we avoid a hard to tune GUC (what do I need to set
> old_snapshot_threshold to, to reduce bloat while not having "normal"
> transactions abort).

Let me make sure I understand your suggestion.  You are suggesting
that within a transaction you can declare a list of tables which
should get the "snapshot too old" error (or something like it) if a
page is read which was modified after the snapshot was taken?
Snapshots within that transaction would not constrain the effective
global xmin for purposes of vacuuming those particular tables?

If I'm understanding your proposal, that would help some of the
cases the "snapshot too old" case addresses, but it would not
handle the accidental "idle in transaction" case (e.g., start a
repeatable read transaction, run one query, then sit idle
indefinitely).  I don't think it would work quite as well for some
of the other cases I've seen, but perhaps it could be made to fit
if we could figure out the accidental "idle in transaction" case.

I also think it might be hard to develop a correct global xmin for
vacuuming a particular table with that solution.  How do you see
that working?

--
Kevin Grittner
EDB: 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] POLA violation with \c service=

[Pavel Stehule]

2015-02-19 19:51 GMT+01:00 David Fetter :

> On Sun, Feb 01, 2015 at 08:38:24AM +0100, Pavel Stehule wrote:
> > Hi all
> >
> > I am sending a review of this patch:
> >
> > * What it does? - Allow to connect to other db by \connect uri connection
> > format
> >
> > postgres=# \c postgresql://localhost?service=old
> > psql (9.5devel, server 9.2.9)
> > You are now connected to database "postgres" as user "pavel".
> >
> > * Would we this feature? - yes, it eliminate inconsistency between cmd
> line
> > connect and \connect. It is good idea without any objections.
> >
> > * This patch is cleanly applicable, later compilation without any issues
> >
> > * All regress tests passed
> >
> > * A psql documentation is updated -- this feature (and format) is not
> > widely known, so maybe some more  examples are welcome
> >
> > * When I tested this feature, it worked as expected
> >
> > * Code respects PostgreSQL coding rules. I prefer a little bit different
> > test if keep password. Current code is little bit harder to understand.
> But
> > I can live with David's code well too.
> >
> > if
> > (!user)
> >
> > user = PQuser(o_conn);
> >
> > if
> > (!host)
> >
> > host =
> > PQhost(o_conn);
> >
> >
> > if
> > (!port)
> >
> > port =
> > PQport(o_conn);
> >
> >
> > if
> > (dbname)
> >
> > has_connection_string =
> > recognized_connection_string(dbname);
> >
> >
> >/* we should not to keep password if some connection property is
> changed
> > */
> >
> >
> >   keep_password = strcmp(user, PQuser(o_conn)) == 0 &&  strcmp(host,
> > PQhost(o_conn)) == 0
> > &&  strcmp(port, PQport(o_conn)) == 0 &&
> > !has_connection_string;
>
> Changed.  This is cleaner.
>
> > I have not any other comments.
> >
> > Possible questions:
> >   1. more examples in doc
>
> I'm not sure how best to illustrate those.  Are you thinking of one
> example each for the URI and conninfo cases?
>

some like

"most common form is:

"\c mydb"

but you can use any connection format described
(libpq-connect.html#LIBPQ-PARAMKEYWORDS) like

"\c postgresql://tom@localhost/mydb?application_name=myapp"



>
> >   2. small change how to check keep_password
>
> Done.
>
> Cheers,
> David.
> --
> David Fetter  http://fetter.org/
> Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
> Skype: davidfetter  XMPP: david.fet...@gmail.com
>
> Remember to vote!
> Consider donating to Postgres: http://www.postgresql.org/about/donate
>

Re: [HACKERS] How about to have relnamespace and relrole?

[Peter Eisentraut]

On 2/18/15 3:44 AM, Kyotaro HORIGUCHI wrote:
> Sorry, I sent the previous mail without patches by accident. The
> patches are attached to this mail.
> 
> 
> Hello, this is the patchset v2 of this feature.
> 
> 0001-Add-regrole.patch
>   Adding regrole as the name says.
> 
> 0002-Add-regnamespace.patch
>   Adding regnamespace. This depends on 0001 patch.
> 
> 0003-Check-new-reg-types-are-not-used-as-default-values.patch
>   Inhibiting the new OID aliss types from being used as constants
>   in default values, which make dependencies on the other
>   (existing) OID alias types.
> 
> 0004-Documentation-for-new-OID-alias-types.patch
>   Documentation patch for this new types.

Somehow, these patches ended up in the commit fest without an author
listed.  That should probably not be possible.



-- 
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] Turning recovery.conf into GUCs

[Peter Eisentraut]

On 1/6/15 4:22 PM, Peter Eisentraut wrote:
> That said, there is a much simpler way to achieve that specific
> functionality: Expose all the recovery settings as fake read-only GUC
> variables.  See attached patch for an example.

The commit fest app has this as the patch of record.  I don't think
there is a real updated patch under consideration here.  This item
should probably not be in the commit fest at all.



-- 
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: disallow operator "=>" and use it for named parameters

[Pavel Stehule]

2015-02-19 16:06 GMT+01:00 Petr Jelinek :

> On 19/01/15 17:14, Pavel Stehule wrote:
>
>>
>>
>> 2015-01-19 14:27 GMT+01:00 Robert Haas > >:
>>
>> On Mon, Jan 19, 2015 at 2:59 AM, Pavel Stehule
>> mailto:pavel.steh...@gmail.com>> wrote:
>> >> I think you should just remove the WARNING, not change it to an
>> error.
>> >> If somebody wants to quote the operator name to be able to continue
>> >> using it, I think that's OK.
>> >
>> > It looks so quoting doesn't help here
>> >
>> > + CREATE OPERATOR "=>" (
>> > +leftarg = int8,<--><-->-- right unary
>> > +procedure = numeric_fac
>> > + );
>> > + ERROR:  syntax error at or near "("
>> > + LINE 1: CREATE OPERATOR "=>" (
>> > +  ^
>>
>> Well then the error check is just dead code.  Either way, you don't
>> need it.
>>
>>
>> yes, I removed it
>>
>>
> I am marking this as Ready For Committer, the patch is trivial and works
> as expected, there is nothing to be added to it IMHO.
>
> The "=>" operator was deprecated for several years so it should not be too
> controversial either.
>

Thank you very much

Pavel


>
>
> --
>  Petr Jelinek  http://www.2ndQuadrant.com/
>  PostgreSQL Development, 24x7 Support, Training & Services
>

Re: [HACKERS] deparsing utility commands

[Alvaro Herrera]

Stephen Frost wrote:

> > > > Yes, I will push these unless somebody objects soon, as they seem
> > > > perfectly reasonable to me.  The only troubling thing is that there is
> > > > no regression test for this kind of thing in event triggers (i.e. verify
> > > > which command tags get support and which don't), which seems odd to me.
> > > > Not these patches's fault, though, so I'm not considering adding any 
> > > > ATM.
> > > 
> > > Ugh.  I dislike that when we say an event trigger will fire before
> > > 'GRANT' what we really mean is "GRANT when it's operating against a
> > > local object".  At the minimum we absolutely need to be very clear in
> > > the documentation about that limitation.  Perhaps there is something
> > > already which reflects that, but I don't see anything in the patch
> > > being added about that.
> > 
> > Hmm, good point, will give this some thought.  I'm thinking perhaps we
> > can add a table of which object types are supported for generic commands
> > such as GRANT, COMMENT and SECURITY LABEL.
> 
> That sounds like a good idea.

Here's a patch.  I noticed that the introduction to event trigger
already says they only act on local objects:

The ddl_command_start event occurs just before the execution of
a CREATE, ALTER, DROP, SECURITY LABEL, COMMENT, GRANT or REVOKE
command. No check whether the affected object exists or doesn't
exist is performed before the event trigger fires. As an
exception, however, this event does not occur for DDL commands
targeting shared objects — databases, roles, and tablespaces —
or for commands targeting event triggers themselves.

So adding more text to the same effect would be repetitive.  I added a
sixth column "Notes" to the table of supported command tags vs. events,
with the text "Only for local objects" next to the four commands being
added here.

I think it's fair to push this patch as is.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From b366da953a10dea2fc2ae3e172cf15ab10e1e7ad Mon Sep 17 00:00:00 2001
From: Alvaro Herrera 
Date: Thu, 19 Feb 2015 17:02:41 -0300
Subject: [PATCH] Support more commands in event triggers

COMMENT, SECURITY LABEL, and GRANT/REVOKE now also fire
ddl_command_start and ddl_command_end event triggers, when they operate
on database-local objects.

Reviewed-By: Michael Paquier, Andres Freund, Stephen Frost
---
 doc/src/sgml/event-trigger.sgml  | 125 ++-
 src/backend/commands/event_trigger.c |  34 +-
 src/backend/tcop/utility.c   |  68 +++
 src/include/commands/event_trigger.h |   1 +
 4 files changed, 211 insertions(+), 17 deletions(-)

diff --git a/doc/src/sgml/event-trigger.sgml b/doc/src/sgml/event-trigger.sgml
index 156c463..04353ea 100644
--- a/doc/src/sgml/event-trigger.sgml
+++ b/doc/src/sgml/event-trigger.sgml
@@ -36,7 +36,9 @@
 

  The ddl_command_start event occurs just before the
- execution of a CREATE, ALTER, or DROP
+ execution of a CREATE, ALTER, DROP,
+ SECURITY LABEL,
+ COMMENT, GRANT or REVOKE
  command.  No check whether the affected object exists or doesn't exist is
  performed before the event trigger fires.
  As an exception, however, this event does not occur for
@@ -123,14 +125,15 @@
 

  Event Trigger Support by Command Tag
- 
+ 
   

-command tag
+Command Tag
 ddl_command_start
 ddl_command_end
 sql_drop
 table_rewrite
+Notes

   
   
@@ -140,6 +143,7 @@
 X
 -
 -
+


 ALTER COLLATION
@@ -147,6 +151,7 @@
 X
 -
 -
+


 ALTER CONVERSION
@@ -154,6 +159,7 @@
 X
 -
 -
+


 ALTER DOMAIN
@@ -161,6 +167,7 @@
 X
 -
 -
+


 ALTER EXTENSION
@@ -168,6 +175,7 @@
 X
 -
 -
+


 ALTER FOREIGN DATA WRAPPER
@@ -175,6 +183,7 @@
 X
 -
 -
+


 ALTER FOREIGN TABLE
@@ -182,6 +191,7 @@
 X
 X
 -
+


 ALTER FUNCTION
@@ -189,6 +199,7 @@
 X
 -
 -
+


 ALTER LANGUAGE
@@ -196,6 +207,7 @@
 X
 -
 -
+


 ALTER OPERATOR
@@ -203,6 +215,7 @@
 X
 -
 -
+


 ALTER OPERATOR CLASS
@@ -210,6 +223,7 @@
 X
 -
 -
+


 ALTER OPERATOR FAMILY
@@ -217,6 +231,7 @@
 X
 -
 -
+


 ALTER POLICY
@@ -224,6 +239,7 @@
 X
 -
 

Re: [HACKERS] INSERT ... ON CONFLICT {UPDATE | IGNORE} 2.0

[Peter Geoghegan]

On Thu, Feb 19, 2015 at 11:10 AM, Heikki Linnakangas
 wrote:
>> I fully agree with your summary here. However, why should we suppose
>> that while we wait, the other backends don't both delete and then
>> re-insert their tuple? They need the pre-check to know not to
>> re-insert their tuple (seeing our tuple, immediately after we wake as
>> the preferred backend with the older XID) in order to break the race.
>> But today, exclusion constraints are optimistic in that the insert
>> happens first, and only then the check. The pre-check turns that the
>> other way around, in a limited though necessary sense.
>
> I'm not sure I understand exactly what you're saying, but AFAICS the
> pre-check doesn't completely solve that either. It's entirely possible that
> the other backend deletes its tuple, our backend then performs the
> pre-check, and the other backend re-inserts its tuple again. Sure, the
> pre-check reduces the chances, but we're talking about a rare condition to
> begin with, so I don't think it makes sense to add much code just to reduce
> the chances further.

But super deletion occurs *before* releasing the token lock, which is
the last thing we do before looping around and starting again. So iff
we're the oldest XID, the one that gets to "win" by unexpectedly
waiting on another's token in our second phase (second call to
check_exclusion_or_unique_constraint()), we will not, in fact, see
anyone else's tuple, because they'll all be forced to go through the
first phase and find our pre-existing, never-deleted tuple, so we
can't see any new tuple from them. And, because they super delete
before releasing their token, they'll definitely have super deleted
when we're woken up, so we can't see any old/existing tuple either. We
have our tuple inserted this whole time - ergo, we do, in fact, "win"
reliably.

The fly in the ointment is regular inserters, perhaps, but we've
agreed that they're not too important here, and even when that happens
we're in "deadlock land", not "livelock land", which is obviously a
nicer place to live.

Does that make sense?
-- 
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] Dead code in gin_private.h related to page split in WAL

[Heikki Linnakangas]

On 02/19/2015 05:34 AM, Michael Paquier wrote:

I noticed that the following structures are still defined in
gin_private.h but they are used nowhere since 2c03216d that has
reworked WAL format:
- ginxlogSplitEntry
- ginxlogSplitDataLeaf
- ginxlogSplitDataInternal
Attached is a trivial patch to remove them.


Removed, thanks.

- Heikki


--
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 {UPDATE | IGNORE} 2.0

[Heikki Linnakangas]

On 02/19/2015 08:16 PM, Peter Geoghegan wrote:

On Thu, Feb 19, 2015 at 5:21 AM, Heikki Linnakangas
 wrote:

Hmm. I haven't looked at your latest patch, but I don't think you need to
pre-check for this to work. To recap, the situation is that two backends
have already inserted the heap tuple, and then see that the other backend's
tuple conflicts. To avoid a livelock, it's enough that one backend
super-deletes its own tuple first, before waiting for the other to complete,
while the other other backend waits without super-deleting. No?


I fully agree with your summary here. However, why should we suppose
that while we wait, the other backends don't both delete and then
re-insert their tuple? They need the pre-check to know not to
re-insert their tuple (seeing our tuple, immediately after we wake as
the preferred backend with the older XID) in order to break the race.
But today, exclusion constraints are optimistic in that the insert
happens first, and only then the check. The pre-check turns that the
other way around, in a limited though necessary sense.


I'm not sure I understand exactly what you're saying, but AFAICS the 
pre-check doesn't completely solve that either. It's entirely possible 
that the other backend deletes its tuple, our backend then performs the 
pre-check, and the other backend re-inserts its tuple again. Sure, the 
pre-check reduces the chances, but we're talking about a rare condition 
to begin with, so I don't think it makes sense to add much code just to 
reduce the chances further.


- Heikki



--
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 "snapshot too old" error, to prevent bloat

[Kevin Grittner]

Rod Taylor  wrote:

> Would pg_dump be able to opt-out of such a restriction?

I don't see how, since vacuum would be removing recently dead
tuples that are still visible; the alternative to getting a
"snapshot too old" error when reading a page which could be
affected is to return incorrect results, and nobody wants that.
The best you could do if you wanted to run pg_dump (or similar) and
it might take more time than your old_snapshot_threshold would be
to increase the threshold, reload, dump, set it back to the
"normal" setting, and reload again.


Andrew's suggestion of setting this per table would not help here.


--
Kevin Grittner
EDB: 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] Allow "snapshot too old" error, to prevent bloat

[Claudio Freire]

On Thu, Feb 19, 2015 at 3:44 PM, Kevin Grittner  wrote:
>> I'm also interested in handling the case Stephen Frost described, where
>> a tuple is effectively dead but we don't currently have the means of
>> discovering the fact, because there is an older long running transaction
>> which is never in fact going to be able to see the tuple.
>
> Absolutely. That's one of several other issues that I've been
> looking at over the last few weeks. It sounds like people are
> already working on that one, which is great. My personal priority
> list included that, but after the two I submitted here and a patch
> to allow combining near-empty btree pages so that btree bloat is
> constrained without having to reindex periodically for the cases
> where index tuples are added in index order (at one or a few
> insertion points) and most-but-not-all are deleted. You can
> currently wind up with a worst-case of one index tuple per page
> with no action to reduce that bloat by vacuum processes.

I'd be willing to test that patch.

I have a big database that does that, and a script that fills the disk
with said bloat. That's forced me to do periodic reindexing, which
sucks.


-- 
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] POLA violation with \c service=

[David Fetter]

On Sun, Feb 01, 2015 at 08:38:24AM +0100, Pavel Stehule wrote:
> Hi all
> 
> I am sending a review of this patch:
> 
> * What it does? - Allow to connect to other db by \connect uri connection
> format
> 
> postgres=# \c postgresql://localhost?service=old
> psql (9.5devel, server 9.2.9)
> You are now connected to database "postgres" as user "pavel".
> 
> * Would we this feature? - yes, it eliminate inconsistency between cmd line
> connect and \connect. It is good idea without any objections.
> 
> * This patch is cleanly applicable, later compilation without any issues
> 
> * All regress tests passed
> 
> * A psql documentation is updated -- this feature (and format) is not
> widely known, so maybe some more  examples are welcome
> 
> * When I tested this feature, it worked as expected
> 
> * Code respects PostgreSQL coding rules. I prefer a little bit different
> test if keep password. Current code is little bit harder to understand. But
> I can live with David's code well too.
> 
> if
> (!user)
> 
> user = PQuser(o_conn);
> 
> if
> (!host)
> 
> host =
> PQhost(o_conn);
> 
> 
> if
> (!port)
> 
> port =
> PQport(o_conn);
> 
> 
> if
> (dbname)
> 
> has_connection_string =
> recognized_connection_string(dbname);
> 
> 
>/* we should not to keep password if some connection property is changed
> */
> 
> 
>   keep_password = strcmp(user, PQuser(o_conn)) == 0 &&  strcmp(host,
> PQhost(o_conn)) == 0
> &&  strcmp(port, PQport(o_conn)) == 0 &&
> !has_connection_string;

Changed.  This is cleaner.

> I have not any other comments.
> 
> Possible questions:
>   1. more examples in doc

I'm not sure how best to illustrate those.  Are you thinking of one
example each for the URI and conninfo cases?

>   2. small change how to check keep_password

Done.

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index a637001..cae6bf4 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -796,18 +796,20 @@ testdb=>
   
 
   
-\c or \connect [ 
dbname [ username ] [ host ] [ port ] ]
+\c or \connect  [ 
{ [ dbname [ username ] [ host ] [ port ] ] | conninfo string | URI } ] 
 
 
 Establishes a new connection to a PostgreSQL
-server. If the new connection is successfully made, the
-previous connection is closed. If any of conninfo string, or a URI. 
If the new connection is
+successfully made, the
+previous connection is closed.  When using positional parameters, if 
any of dbname, username, host or port are omitted or specified
 as -, the value of that parameter from the
-previous connection is used. If there is no previous
+previous connection is used.  If using positional parameters and there 
is no previous
 connection, the libpq default for
 the parameter's value is used.
 
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 7c9f28d..ec86e52 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1607,6 +1607,8 @@ do_connect(char *dbname, char *user, char *host, char 
*port)
PGconn *o_conn = pset.db,
   *n_conn;
char   *password = NULL;
+   boolkeep_password = true;
+   boolhas_connection_string = false;
 
if (!o_conn && (!dbname || !user || !host || !port))
{
@@ -1620,15 +1622,31 @@ do_connect(char *dbname, char *user, char *host, char 
*port)
return false;
}
 
-   if (!dbname)
-   dbname = PQdb(o_conn);
if (!user)
user = PQuser(o_conn);
+
if (!host)
host = PQhost(o_conn);
+
if (!port)
port = PQport(o_conn);
 
+   if (dbname)
+   has_connection_string = recognized_connection_string(dbname);
+
+   keep_password = (
+   (strcmp(user, PQuser(o_conn)) == 0) &&
+   (strcmp(host, PQhost(o_conn)) == 0) &&
+   (strcmp(port, PQport(o_conn)) == 0) &&
+!has_connection_string);
+
+   /*
+* Unlike the previous stanzas, changing only the dbname shouldn't
+* trigger throwing away the password.
+*/
+   if (!dbname)
+   dbname = PQdb(o_conn);
+
/*
 * If the user asked to be prompted for a password, ask for one now. If
 * not, use the password from the old connection, provided the username
@@ -1643,9 +1661,13 @@ do_connect(char *dbname, char *user, char *host, char 
*port)
{
password = prompt_for_password(user);

Re: [HACKERS] Allow "snapshot too old" error, to prevent bloat

[Kevin Grittner]

Andrew Dunstan  wrote:
> On 02/19/2015 09:44 AM, Kevin Grittner wrote:

> I understand why this make people nervous. I wonder if it might be more
> palatable if there were a per-table setting that could enable it? If we
> could ensure that this was only applied to high churn queue tables, say,
> while tables touched by the report writer would not have it applied,
> that would calm a lot of my fears.

That's an interesting idea.  I think I should switch the unit of
measure for "too old" to a time-based value first, since there
seems to be universal agreement that it would be better than number
of transactions.  Once I've cleared that hurdle I'll see what this
would take.

> I'm also interested in handling the case Stephen Frost described, where
> a tuple is effectively dead but we don't currently have the means of
> discovering the fact, because there is an older long running transaction
> which is never in fact going to be able to see the tuple.

Absolutely. That's one of several other issues that I've been
looking at over the last few weeks. It sounds like people are
already working on that one, which is great. My personal priority
list included that, but after the two I submitted here and a patch
to allow combining near-empty btree pages so that btree bloat is
constrained without having to reindex periodically for the cases
where index tuples are added in index order (at one or a few
insertion points) and most-but-not-all are deleted. You can
currently wind up with a worst-case of one index tuple per page
with no action to reduce that bloat by vacuum processes.

--
Kevin Grittner
EDB: 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] Allow "snapshot too old" error, to prevent bloat

[Ants Aasma]

On Thu, Feb 19, 2015 at 6:01 PM, Kevin Grittner  wrote:
>> I can see how they would be, provided we can be confident that we're
>> going to actually throw an error when the snapshot is out of date and
>> not end up returning incorrect results.  We need to be darn sure of
>> that, both now and in a few years from now when many of us may have
>> forgotten about this knob.. ;)
>
> I get that this is not zero-cost to maintain, and that it might not
> be of interest to the community largely for that reason.  If you
> have any ideas about how to improve that, I'm all ears.  But do
> consider the actual scope of what was needed for the btree coverage
> (above).  (Note that the index-only scan issue doesn't look like
> anything AM-specific; if something is needed for that it would be
> in the pruning/vacuum area.)

If I understood the issue correctly, you have long running snapshots
accessing one part of the data, while you have high churn on a
disjoint part of data. We would need to enable vacuum on the high
churn data while still being able to run those long queries. The
"snapshot too old" solution limits the maximum age of global xmin at
the cost of having to abort transactions that are older than this and
happen to touch a page that was modified after they were started.

What about having the long running snapshots declare their working
set, and then only take them into account for global xmin for
relations that are in the working set? Like a SET TRANSACTION WORKING
SET command. This way the error is deterministic, vacuum on the high
churn tables doesn't have to wait for the old transaction delay to
expire and we avoid a hard to tune GUC (what do I need to set
old_snapshot_threshold to, to reduce bloat while not having "normal"
transactions abort).

Regards,
Ants Aasma
-- 
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt
Web: http://www.postgresql-support.de


-- 
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 "snapshot too old" error, to prevent bloat

[Rod Taylor]

On Wed, Feb 18, 2015 at 4:57 PM, Kevin Grittner  wrote:

> >> But max_standby_streaming_delay, max_standby_archive_delay and
> >> hot_standby_feedback are among the most frequent triggers for
> >> questions and complaints that I/we see.
> >>
> > Agreed.
> > And a really bad one used to be vacuum_defer_cleanup_age, because
> > of confusing units amongst other things. Which in terms seems
> > fairly close to Kevins suggestions, unfortunately.
>
> Particularly my initial suggestion, which was to base snapshot
> "age" it on the number of transaction IDs assigned.  Does this look
> any better to you if it is something that can be set to '20min' or
> '1h'?  Just to restate, that would not automatically cancel the
> snapshots past that age; it would allow vacuum of any tuples which
> became "dead" that long ago, and would cause a "snapshot too old"
> message for any read of a page modified more than that long ago
> using a snapshot which was older than that.
>
>
I like this thought. One of the first things I do in a new Pg environment
is setup a cronjob that watches pg_stat_activity and terminates most
backends over N minutes in age (about 5x the length of normal work) with an
exception for a handful of accounts doing backups and other maintenance
operations.  This prevents a stuck client from jamming up the database.

Would pg_dump be able to opt-out of such a restriction?

regards,

Rod

Re: [HACKERS] Add min and max execute statement time in pg_stat_statement

[David G. Johnston]

On Thu, Feb 19, 2015 at 11:10 AM, David Fetter  wrote:

> On Wed, Feb 18, 2015 at 08:31:09PM -0700, David G. Johnston wrote:
> > On Wed, Feb 18, 2015 at 6:50 PM, Andrew Dunstan 
> wrote:
> > > On 02/18/2015 08:34 PM, David Fetter wrote:
> > >
> > >> On Tue, Feb 17, 2015 at 08:21:32PM -0500, Peter Eisentraut wrote:
> > >>
> > >>> On 1/20/15 6:32 PM, David G Johnston wrote:
> > >>>
> >  In fact, as far as the database knows, the values provided to this
> >  function do represent an entire population and such a correction
> >  would be unnecessary.  I guess it boils down to whether "future"
> >  queries are considered part of the population or whether the
> >  population changes upon each query being run and thus we are
> >  calculating the ever-changing population variance.
> >
> > > I think we should be calculating the population variance.
> >
> > >> Why population variance and not sample variance?  In distributions
> > >> where the second moment about the mean exists, it's an unbiased
> > >> estimator of the variance.  In this, it's different from the
> > >> population variance.
> >
> > > Because we're actually measuring the whole population, and not a
> sample?
>
> We're not.  We're taking a sample, which is to say past measurements,
> and using it to make inferences about the population, which includes
> all queries in the future.
>
>
​"All past measurements" does not qualify as a "random sample" of a
population made up of all past measurements and any potential members that
may be added in the future.  Without the "random sample" aspect you throw
away all pretense of avoiding bias so you might as well just call the
totality of the past measurements the population, describe them using
population descriptive statistics, and call it a day.

For large populations it isn't going to matter anyway but for small
populations the difference of one in the divisor seems like it would make
the past performance look worse than it actually was.

David J.

Re: [HACKERS] INSERT ... ON CONFLICT {UPDATE | IGNORE} 2.0

[Peter Geoghegan]

On Thu, Feb 19, 2015 at 5:21 AM, Heikki Linnakangas
 wrote:
> Hmm. I haven't looked at your latest patch, but I don't think you need to
> pre-check for this to work. To recap, the situation is that two backends
> have already inserted the heap tuple, and then see that the other backend's
> tuple conflicts. To avoid a livelock, it's enough that one backend
> super-deletes its own tuple first, before waiting for the other to complete,
> while the other other backend waits without super-deleting. No?

I fully agree with your summary here. However, why should we suppose
that while we wait, the other backends don't both delete and then
re-insert their tuple? They need the pre-check to know not to
re-insert their tuple (seeing our tuple, immediately after we wake as
the preferred backend with the older XID) in order to break the race.
But today, exclusion constraints are optimistic in that the insert
happens first, and only then the check. The pre-check turns that the
other way around, in a limited though necessary sense.

Granted, it's unlikely that we'd livelock due to one session always
deleting and then nullifying that by re-inserting in time, but the
theoretical risk seems real. Therefore, I'm not inclined to bother
committing something without a pre-check, particularly since we're not
really interested in fixing unprincipled deadlocks for ordinary
exclusion constraint inserters (useful to know that you also think
that doesn't matter, BTW). Does that make sense?

This is explained within "livelock insurance" new-to-V2.3 comments in
check_exclusion_or_unique_constraint().  (Not that I think that
explanation is easier to follow than this one).

> It might be easier to provoke the livelocks with a GiST opclass that's
> unusually slow. I wrote the attached opclass for the purpose of testing this
> a while ago, but I haven't actually gotten around to do much with it. It's
> called "useless_gist", because it's a GiST opclass for integers, like
> btree_gist, but the penalty and picksplit functions are totally dumb. The
> result is that the tuples are put to the index in pretty much random order,
> and every scan has to scan the whole index. I'm posting it here, in the hope
> that it happens to be useful, but I don't really know if it is.

Thanks. I'll try and use this for testing. Haven't been able to break
exclusion constraints with the jjanes_upsert test suite in a long
time, now.

-- 
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] Add min and max execute statement time in pg_stat_statement

[David Fetter]

On Wed, Feb 18, 2015 at 08:31:09PM -0700, David G. Johnston wrote:
> On Wed, Feb 18, 2015 at 6:50 PM, Andrew Dunstan  wrote:
> > On 02/18/2015 08:34 PM, David Fetter wrote:
> >
> >> On Tue, Feb 17, 2015 at 08:21:32PM -0500, Peter Eisentraut wrote:
> >>
> >>> On 1/20/15 6:32 PM, David G Johnston wrote:
> >>>
>  In fact, as far as the database knows, the values provided to this
>  function do represent an entire population and such a correction
>  would be unnecessary.  I guess it boils down to whether "future"
>  queries are considered part of the population or whether the
>  population changes upon each query being run and thus we are
>  calculating the ever-changing population variance.
> 
> > I think we should be calculating the population variance.
> 
> >> Why population variance and not sample variance?  In distributions
> >> where the second moment about the mean exists, it's an unbiased
> >> estimator of the variance.  In this, it's different from the
> >> population variance.
> 
> > Because we're actually measuring the whole population, and not a sample?

We're not.  We're taking a sample, which is to say past measurements,
and using it to make inferences about the population, which includes
all queries in the future.

> The key incorrect word in David Fetter's statement is "estimator".  We are
> not estimating anything but rather providing descriptive statistics for a
> defined population.

See above.

> Users extrapolate that the next member added to the population would be
> expected to conform to this statistical description without bias (though
> see below).  We can also then define the new population by including this
> new member and generating new descriptive statistics (which allows for
> evolution to be captured in the statistics).
> 
> Currently (I think) we allow the end user to kill off the entire population
> and build up from scratch so that while, in the short term, the ability to
> predict the attributes of future members is limited once the population has
> reached a statistically significant level ​new predictions will no longer
> be skewed by population members who attributes were defined in a older and
> possibly significantly different environment.  In theory it would be nice
> to be able to give the user the ability to specify - by time or percentage
> - a subset of the population to leave alive.

I agree that stale numbers can fuzz things in a way we don't like, but
let's not creep too much feature in here.

> Actual time-weighted sampling would be an alternative but likely one
> significantly more difficult to accomplish.

Right.

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


-- 
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] deparsing utility commands

[Alvaro Herrera]

Stephen Frost wrote:
> * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> > > > Now, we probably don't want to hack *all* the utility commands to return
> > > > ObjectAddress instead of OID, because it many cases that's just not
> > > > going to be convenient (not to speak of the code churn); so I think for
> > > > most objtypes the ProcessUtilitySlow stanza would look like this:
> > 
> > > That'd be fine with me, though for my 2c, I wouldn't object to changing
> > > them all to return ObjectAddress either.  I agree that it'd cause a fair
> > > bit of code churn to do so, but there's a fair bit of code churn
> > > happening here anyway (looking at what 0008 does to ProcessUtilitySlow
> > > anyway).
> > 
> > Well, that would make my life easier I think (even if it's a bit more
> > work), so unless there are objections I will do things this way.  It's a
> > bit of a pity that Robert and Dimitri went to huge lengths to have these
> > functions return OID and we're now changing it all to ObjAddress
> > instead, but oh well.
> 
> Not sure that I see it as that huge a deal..  They're still returning an
> Oid, it's just embedded in the ObjAddress to provide a complete
> statement of what the object is.

I've been looking at this idea some more.  There's some churn, but it's
not that bad.  For instance, here's the patch to have RENAME return
ObjectAddress rather than OIDs.

For instance, the get_objtype_catalog_id() function, provided in a later
patch in the series, will no longer be necessary; instead, each
execution code path in the src/backend/command code must set the correct
catalog ID in the ObjectAddress it returns.  So the event triggers code
will have the catalog ID at the ready, without having to figure it out
from the ObjectType it gets from the parse node.

> btw, the hunk in 0026 which adds a 'break;' into standard_ProcessUtility
> caught me by surprise.  Looks like that 'break;' was missing from 0003
> (for T_GrantStmt).

Thanks for pointing this out -- that was broken rebase.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 69d0b62787c2a2a514f70247ccde98c4c43d70a0 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera 
Date: Thu, 19 Feb 2015 14:38:09 -0300
Subject: [PATCH] Have RENAME routines return ObjectAddress rather than OID

This lets them include an objectSubId when appropriate (i.e. when
renaming relation columns and composite type attributes), and
additionally they make the catalog OID available for further processing.
The object OID that was previously returned is still available in the
objectId field of the returned ObjectAddress.

This isn't terribly exciting in itself but it supports future event
trigger changes.

Discussion: 20150218213255.gc6...@tamriel.snowman.net
Reviewed-By: Stephen Frost
---
 src/backend/catalog/dependency.c|  6 ++-
 src/backend/catalog/objectaddress.c |  6 +++
 src/backend/commands/alter.c| 14 +--
 src/backend/commands/dbcommands.c   |  7 +++-
 src/backend/commands/policy.c   |  7 +++-
 src/backend/commands/schemacmds.c   |  7 +++-
 src/backend/commands/tablecmds.c| 77 +
 src/backend/commands/tablespace.c   |  7 +++-
 src/backend/commands/trigger.c  |  7 +++-
 src/backend/commands/typecmds.c |  6 ++-
 src/backend/commands/user.c |  7 +++-
 src/backend/rewrite/rewriteDefine.c |  7 +++-
 src/include/catalog/objectaddress.h | 16 
 src/include/commands/alter.h|  3 +-
 src/include/commands/dbcommands.h   |  3 +-
 src/include/commands/policy.h   |  2 +-
 src/include/commands/schemacmds.h   |  2 +-
 src/include/commands/tablecmds.h|  9 +++--
 src/include/commands/tablespace.h   |  3 +-
 src/include/commands/trigger.h  |  3 +-
 src/include/commands/typecmds.h |  2 +-
 src/include/commands/user.h |  2 +-
 src/include/rewrite/rewriteDefine.h |  3 +-
 23 files changed, 155 insertions(+), 51 deletions(-)

diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index bacb242..25ff326 100644
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -2269,8 +2269,9 @@ free_object_addresses(ObjectAddresses *addrs)
 ObjectClass
 getObjectClass(const ObjectAddress *object)
 {
-	/* only pg_class entries can have nonzero objectSubId */
-	if (object->classId != RelationRelationId &&
+	/* only pg_class and pg_type entries can have nonzero objectSubId */
+	if ((object->classId != RelationRelationId &&
+		 object->classId != TypeRelationId) &&
 		object->objectSubId != 0)
 		elog(ERROR, "invalid non-zero objectSubId for object class %u",
 			 object->classId);
@@ -2285,6 +2286,7 @@ getObjectClass(const ObjectAddress *object)
 			return OCLASS_PROC;
 
 		case TypeRelationId:
+			/* caller must check objectSubId */
 			return OCLASS_TYPE;
 
 		case CastRelationId:
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddres

Re: [HACKERS] Allow "snapshot too old" error, to prevent bloat

[Andrew Dunstan]

On 02/19/2015 09:44 AM, Kevin Grittner wrote:


On the 15th I said this:

| What this discussion has made me reconsider is the metric for
| considering a transaction "too old".  The number of transaction IDs
| consumed seems inferior as the unit of measure for that to LSN or
| time.
|
| It looks to me to be pretty trivial (on the order of maybe 30 lines
| of code) to specify this GUC in minutes rather than transaction
| IDs.  At first glance this seems like it would be vulnerable to the
| usual complaints about mismanaged clocks, but those are easily
| answered by using a cached time in shared memory that we populate
| in such a way that it can never move backward.  Done right, this
| could even allow the GUC to be changeable on reload rather than
| only at restart.  A badly mismanaged system clock could not cause a
| query to generate incorrect results; the worst that could happen is
| that this feature would fail to control bloat as much as expected
| or reads of modified data could generate the "snapshot too old"
| error around the time of the clock adjustment.
|
| As before, this would default to a magic value to mean that you
| want the historical PostgreSQL behavior.
|
| If that makes the idea more palatable to anyone, I can submit a
| patch to that effect within the next 24 hours.

Until yesterday I didn't get any feedback suggesting that such a
change would make the patch more palatable.  Now that I have had,
I'll try to post a patch to that effect today.

Thanks!




I understand why this make people nervous. I wonder if it might be more 
palatable if there were a per-table setting that could enable it? If we 
could ensure that this was only applied to high churn queue tables, say, 
while tables touched by the report writer would not have it applied, 
that would calm a lot of my fears.


I'm also interested in handling the case Stephen Frost described, where 
a tuple is effectively dead but we don't currently have the means of 
discovering the fact, because there is an older long running transaction 
which is never in fact going to be able to see the tuple.


cheers

andrew



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

Re: [HACKERS] pg_upgrade and rsync

[Bruce Momjian]

On Wed, Jan 28, 2015 at 09:26:11PM -0800, Josh Berkus wrote:
> 
> > So, for my 2c, I'm on the fence about it.  On the one hand, I agree,
> > it's a bit of a complex process to get right.  On the other hand, it's
> > far better if we put something out there along the lines of "if you
> > really want to, this is how to do it" than having folks try to fumble
> > through to find the correct steps themselves.
> 
> So, here's the correct steps for Bruce, because his current doc does not
> cover all of these.  I really think this should go in as a numbered set
> of steps; the current doc has some steps as steps, and other stuff
> buried in paragraphs.
> 
> 1. Install the new version binaries on both servers, alongside the old
> version.
> 
> 2. If not done by the package install, initdb the new version's data
> directory.
> 
> 3. Check that the replica is not very lagged.  If it is, wait for
> traffic to die down and for it to catch up.

Now that 9.4.1 is released, I would like to get this doc patch applied
--- it will close the often-requested feature of how to pg_upgrade slave
clusters.

I wasn't happy with Josh's specification above that the "replica is not
very lagged", so I added a bullet point to check the pg_controldata
output to verify that the primary and standby servers are synchronized.

Yes, this adds even more complication to the pg_upgrade instructions,
but it is really more of the same complexity.  pg_upgrade really needs
an install-aware and OS-aware tool on top of it to automate much of
this.

Patch attached.

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

  + Everyone has their own god. +
diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
new file mode 100644
index 07ca0dc..e25e0d0
*** a/doc/src/sgml/backup.sgml
--- b/doc/src/sgml/backup.sgml
*** tar -cf backup.tar /usr/local/pgsql/data
*** 438,445 
 Another option is to use rsync to perform a file
 system backup.  This is done by first running rsync
 while the database server is running, then shutting down the database
!server just long enough to do a second rsync.  The
!second rsync will be much quicker than the first,
 because it has relatively little data to transfer, and the end result
 will be consistent because the server was down.  This method
 allows a file system backup to be performed with minimal downtime.
--- 438,447 
 Another option is to use rsync to perform a file
 system backup.  This is done by first running rsync
 while the database server is running, then shutting down the database
!server long enough to do an rsync --checksum.
!(--checksum is necessary because rsync only
!has file modification-time granularity of one second.)  The
!second rsync will be quicker than the first,
 because it has relatively little data to transfer, and the end result
 will be consistent because the server was down.  This method
 allows a file system backup to be performed with minimal downtime.
diff --git a/doc/src/sgml/pgupgrade.sgml b/doc/src/sgml/pgupgrade.sgml
new file mode 100644
index e1cd260..d1c26df
*** a/doc/src/sgml/pgupgrade.sgml
--- b/doc/src/sgml/pgupgrade.sgml
*** NET STOP postgresql-8.4
*** 315,320 
--- 315,324 
  NET STOP postgresql-9.0
  
  
+ 
+ 
+  Log-shipping standby servers can remain running until a later step.
+ 
 
  
 
*** pg_upgrade.exe
*** 399,404 
--- 403,525 
 
  
 
+ Upgrade any Log-Shipping Standby Servers
+ 
+ 
+  If you have Log-Shipping Standby Servers (), follow these steps to upgrade them (before
+  starting any servers):
+ 
+ 
+ 
+ 
+  
+   Install the new PostgreSQL binaries on standby servers
+ 
+   
+Make sure the new binaries and support files are installed on all
+standby servers.
+   
+  
+ 
+  
+   Make sure the new standby data directories do not
+   exist
+ 
+   
+Make sure the new standby data directories do not
+exist or are empty.  If initdb was run, delete
+the standby server data directories.
+   
+  
+ 
+  
+   Install custom shared object files
+ 
+   
+Install the same custom shared object files on the new standbys
+that you installed in the new master cluster.
+   
+  
+ 
+  
+   Stop standby servers
+ 
+   
+If the standby servers are still running, stop them now using the
+above instructions.
+   
+  
+ 
+  
+   Verify Standby Servers
+ 
+   
+To prevent old standby servers from being modified, run
+pg_controldata against the primary and standby
+clusters and verify that the Latest checkpoint location
+values match in all clusters.  (This requires the standbys to be
+shut down after the primary.)
+   
+  
+ 
+

Re: [HACKERS] Allow "snapshot too old" error, to prevent bloat

[Kevin Grittner]

Stephen Frost  wrote:
> Kevin Grittner (kgri...@ymail.com) wrote:
>> Stephen Frost  wrote:

>>> In the end, with a single long-running transaction, the worst bloat you
>>> would have is double the size of the system at the time the long-running
>>> transaction started.
>>
>> I agree that limiting bloat to one dead tuple for every live one
>> for each old snapshot is a limit that has value, and it was unfair
>> of me to characterize that as not being a limit.  Sorry for that.
>>
>> This possible solution was discussed with the user whose feedback
>> caused me to write this patch, but there were several reasons they
>> dismissed that as a viable total solution for them, two of which I
>> can share:
>>
>> (1)  They have a pool of connections each of which can have several
>> long-running cursors, so the limit from that isn't just doubling
>> the size of their database, it is limiting it to some two-or-three
>> digit multiple of the necessary size.
>
> This strikes me as a bit off-the-cuff; was an analysis done which
> deteremined that would be the result?

If it sounds like off-the-cuff it is because I am summarizing the
mountain of data which has been accumulated over weeks of
discussions and many rounds of multi-day tests using their actual
software driven by a software test driver that simulates users,
with "think time" and all.

To be usable, we need to run on a particular set of hardware with a
simulated load of 3000 users.  In a two-day test without the
patches I'm proposing, their actual, working production software
running against PostgreSQL bloated the database by 37% and was on a
linear rate of increase.  Unpatched, CPU time consumed at a linear
rate due to the bloat until in the second day it saturated the CPUs
and the driver was unable to get acceptable user performance.
Because of that we haven't moved on to test what the bloat rate
would be like with 3000 users, but I assume it would be higher than
with 300 users.

With the two patches I submitted, bloat stabilized at less than 5%
except for some btree indexes which followed pattern of inserting
at the end and deleting most (but not all) of the entries over
time.  I've been working on a separate patch for that, but it's not
ready to present, so I probably won't post that until the first CF
of the next release -- unless someone's interested enough to want
to discuss it before then.

>> (2)  They are already prepared to deal with "snapshot too old"
>> errors on queries that run more than about 20 minutes and which
>> access tables which are modified.  They would rather do that than
>> suffer the bloat beyond that point.
>
> That, really, is the crux here- they've already got support for dealing
> with it the way Oracle did and they'd like PG to do that too.
> Unfortunately, that, by itself, isn't a good reason for a particular
> capability (we certainly aren't going to be trying to duplicate PL/SQL
> in PG any time soon).

I understand that, and if the community doesn't want this style of
limitation on bloat we can make it part of PPAS (which *does*
duplicate PL/SQL, among other things).  We are offering it to the
community first.

> That said, there are capabilities in other RDBMS's which are
> valuable and which we *do* want, so the fact that Oracle does this
> also isn't a reason to not include it.

:-)

>>> I'm not against having a knob like this, which is defaulted to off,
>>
>> Thanks!  I'm not sure that amounts to a +1, but at least it doesn't
>> sound like a -1.  :-)
>
> So, at the time I wrote that, I wasn't sure if it was a +1 or not
> myself.  I've been thinking about it since then, however, and I'm
> leaning more towards having the capability than not, so perhaps that's a
> +1, but it doesn't excuse the need to come up with an implementation
> that everyone can be happy with and what you've come up with so far
> doesn't have a lot of appeal, based on the feedback (I've only glanced
> through it myself, but I agree with Andres and Tom that it's a larger
> footprint than we'd want for this and the number of places having to be
> touched is concerning as it could lead to future bugs).

This conversation has gotten a little confusing because some of my
posts seem to have been held up in the email systems for some
reason, and are arriving out-of-order (and at least one I don't see
yet).  But I have been responding to these points.  For one thing I
stated four days ago that I now feel that the metric for
determining that a snapshot is "old" should be *time*, not
transactions IDs consumed, and offered to modify that patch in that
direction if people agreed.  I now see one person agreeing and
several people arguing that I should do just that (as though they
had not seem me propose it), so I'll try to get a new version out
today like that.  In any event, I am sure it is possible and feel
that it would be better.  (Having posted that now at least 4 times,
hopefully people will pick up on it.)

For another thing, as mentioned earlier, this is the

[HACKERS] Precedence of standard comparison operators

[Tom Lane]

My Salesforce colleagues have been bugging me about this topic, and
since I see in a nearby thread that we may be about to break backwards
compatibility on "=>", maybe it's time to do something about this too.
To wit, that the precedence of <= >= and <> is neither sane nor standards
compliant.

Up to now, Postgres has had special precedence rules for = < and >,
but their multi-character brethren just get treated as general Op
tokens.  This has assorted surprising consequences; for example you
can do this:

regression=# select * from testjsonb where j->>'space' < j->>'node';

but not this:

regression=# select * from testjsonb where j->>'space' <= j->>'node';
ERROR:  operator does not exist: text <= jsonb
LINE 1: select * from testjsonb where j->>'space' <= j->>'node';
  ^
HINT:  No operator matches the given name and argument type(s). You might need 
to add explicit type casts.

Of course the latter happens because ->> and <= have identical
precedence so the construct is parsed as
((j ->> 'space') <= j) ->> 'node'
whereas < has lower precedence than user-defined operators so
the first case parses in the expected fashion.

I claim that this behavior is contrary to spec as well as being
unintuitive.  Following the grammar productions in SQL99:

  ::= WHERE 

  ::=
  

  ::=

  |  OR 

  ::=

  |  AND 

  ::=
  [ NOT ] 

  ::=
   [ IS [ NOT ]  ]

  ::=
TRUE
  | FALSE
  | UNKNOWN

  ::=

  | 
  | 

  ::=


  ::=

  | 
  | 
  | 
  | 
  | 
  | 
  | 
  | 
  | 
  | 
  | 
  | 


  ::=


  ::=

  | 
  | 
  | 
  | 
  | 


  ::=

  | 

  ::=

  | 

  ::=

  | 


So both the examples I gave should be understood as  value expressions related by s.  This line of reasoning
says that any non-boolean operator should bind tighter than the six
standard comparison operators, because it will necessarily be part of a
 component of a boolean expression.

We have that right for = < > but not for the other three standard-mandated
comparison operators.  I think we should change the grammar so that all
six act like < > do now, that is, they should have %nonassoc precedence
just above NOT.

Another thought, looking at this closely, is that we have the precedence
of IS tests (IS NOT NULL etc) wrong as well: they should bind less tightly
than user-defined ops, not more so.  I'm less excited about changing that,
but there's certainly room to argue that it's wrong per spec.  It's
definitely weird that the IS tests bind more tightly than multicharacter
Ops but less tightly than + - * /.

I've not really experimented with this at all; it would be useful for
example to see how many regression tests break as a gauge for how
troublesome such changes would be.  I thought I'd ask whether there's
any chance at all of such a change getting accepted before doing any
serious work on it.

Thoughts?

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] proposal: disallow operator "=>" and use it for named parameters

[Petr Jelinek]

On 19/01/15 17:14, Pavel Stehule wrote:



2015-01-19 14:27 GMT+01:00 Robert Haas mailto:robertmh...@gmail.com>>:

On Mon, Jan 19, 2015 at 2:59 AM, Pavel Stehule
mailto:pavel.steh...@gmail.com>> wrote:
>> I think you should just remove the WARNING, not change it to an error.
>> If somebody wants to quote the operator name to be able to continue
>> using it, I think that's OK.
>
> It looks so quoting doesn't help here
>
> + CREATE OPERATOR "=>" (
> +leftarg = int8,<--><-->-- right unary
> +procedure = numeric_fac
> + );
> + ERROR:  syntax error at or near "("
> + LINE 1: CREATE OPERATOR "=>" (
> +  ^

Well then the error check is just dead code.  Either way, you don't
need it.


yes, I removed it



I am marking this as Ready For Committer, the patch is trivial and works 
as expected, there is nothing to be added to it IMHO.


The "=>" operator was deprecated for several years so it should not be 
too controversial either.



--
 Petr Jelinek  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] Fetch zero result rows when executing a query?

[Shay Rojansky]

Sorry to revive this thread, I just had one additional thought...

To those advocating the deprecation of the max_rows parameter of Execute,
there's another argument to consider. max_rows isn't just there in order to
fetch, say, a single row of the result set and discard the rest (which is
what I originally asked about). There's also the function of retrieving the
resultset in chunks: getting 5 rows, then 10, etc. etc. Deprecating
max_rows would leave the user/driver only with the option of retrieving the
entire resultset in one go (for example, no option for the interleaving of
rows from several resultsets). And the lack of the ability to execute and
retrieve 0 rows hurts this scenario as well.

Just wanted to put it out there as another argument against deprecation.

On Wed, Feb 11, 2015 at 2:05 AM, Shay Rojansky  wrote:

> Thanks for understanding Robert, that's more or less what I had in mind, I
> was mainly wondering if I were missing some deeper explanation for the
> absence of the possibility of requesting 0 rows.
>
> Regardless of all of the above, it's really no big deal. I'll go ahead and
> use max_rows=1 for now, hopefully you guys don't decide to deprecate it.
>
> Shay
>
> On Tue, Feb 10, 2015 at 3:00 PM, Robert Haas 
> wrote:
>
>> On Sun, Feb 8, 2015 at 3:56 AM, Shay Rojansky  wrote:
>> > Just to be precise: what is strange to me is that the max_rows feature
>> > exists
>> > but has no 0 value. You and Marko are arguing that the whole feature
>> should
>> > be
>> > deprecated (i.e. always return all rows).
>>
>> I think the fact that it has no zero value is probably just a
>> historical accident; most likely, whoever designed it originally
>> (probably twenty years ago) didn't think about queries with
>> side-effects and therefore didn't consider that wanting 0 rows would
>> ever be sensible.  Meanwhile, a sentinel value was needed to request
>> all rows, so they used 0.  If they'd thought of it, they might have
>> picked -1 and we'd not be having this discussion.
>>
>> FWIW, I'm in complete agreement that it would be good if we had this
>> feature.  I believe this is not the first report we've had of
>> PostgreSQL doing things in ways that mesh nicely with standardized
>> driver interfaces.  Whether we think those interfaces are
>> well-designed or not, they are standardized.  When people use $OTHERDB
>> and have a really great driver, and then they move to PostgreSQL and
>> get one with more warts, it does not encourage them to stick with
>> PostgreSQL.
>>
>> .NET is not some fringe user community that we can dismiss as
>> irrelevant.  We need users of all languages to want to use PostgreSQL,
>> not just users of languages any one of us happens to personally like.
>>
>> --
>> Robert Haas
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>

Re: [HACKERS] Allow "snapshot too old" error, to prevent bloat

[Kevin Grittner]

Greg Stark  wrote:
> On Sun, Feb 15, 2015 at 8:27 PM, Tom Lane  wrote:

>> There might be something in that, but again it's not much like
>> this patch.  The key point I think we're both making is that
>> nondeterministic failures are bad, especially when you're
>> talking about long-running, expensive-to-retry transactions.
>>
> But I think Kevin would agree with you there. That's why he's
> interested in having the errors not occur if you don't read from
> the volatile tables. Ie, your reporting query reading from
> read-only tables would be guaranteed to succeed even if some
> other table had had some rows vacuumed away.
>
> I'm not sure that's really going to work because of things like
> hint bit updates or hot pruning. That is, say your table is
> read-only now but last week some of the records were updated. You
> might reasonably expect those rows to be safe and indeed the rows
> themselves will still be in your report. But the earlier versions
> of the rows could still be sitting on some pages invisible to
> every query but not vacuumable quite yet and then suddenly they
> get vacuumed away and the LSN on the page gets bumped.

That's a very interesting point.  In the case that the reporting
tables are like that (versus being created right before the run,
for the report), it would argue for either very aggressive
autovacuum settings or an explicit vacuum on those tables before
starting the report.

> Fwiw I would strongly suggest that instead of having a number of
> transactions to have a time difference.

On the 15th I said this:

| What this discussion has made me reconsider is the metric for
| considering a transaction "too old".  The number of transaction IDs
| consumed seems inferior as the unit of measure for that to LSN or
| time.
|
| It looks to me to be pretty trivial (on the order of maybe 30 lines
| of code) to specify this GUC in minutes rather than transaction
| IDs.  At first glance this seems like it would be vulnerable to the
| usual complaints about mismanaged clocks, but those are easily
| answered by using a cached time in shared memory that we populate
| in such a way that it can never move backward.  Done right, this
| could even allow the GUC to be changeable on reload rather than
| only at restart.  A badly mismanaged system clock could not cause a
| query to generate incorrect results; the worst that could happen is
| that this feature would fail to control bloat as much as expected
| or reads of modified data could generate the "snapshot too old"
| error around the time of the clock adjustment.
|
| As before, this would default to a magic value to mean that you
| want the historical PostgreSQL behavior.
|
| If that makes the idea more palatable to anyone, I can submit a
| patch to that effect within the next 24 hours.

Until yesterday I didn't get any feedback suggesting that such a
change would make the patch more palatable.  Now that I have had,
I'll try to post a patch to that effect today.

Thanks!

--
Kevin Grittner
EDB: 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] INSERT ... ON CONFLICT {UPDATE | IGNORE} 2.0

[Heikki Linnakangas]

On 02/16/2015 11:31 AM, Andres Freund wrote:

On 2015-02-16 10:00:24 +0200, Heikki Linnakangas wrote:

I'm starting to think that we should bite the bullet and consume an infomask
bit for this. The infomask bits are a scarce resource, but we should use
them when it makes sense. It would be good for forensic purposes too, to
leave a trace that a super-deletion happened.


Well, we IIRC don't have any left right now. We could reuse
MOVED_IN|MOVED_OUT, as that never was set in the past. But it'd
essentially use two infomask bits forever...


t_infomask is all used, but t_infomask2 has two bits left:


/*
 * information stored in t_infomask2:
 */
#define HEAP_NATTS_MASK 0x07FF  /* 11 bits for number of 
attributes */
/* bits 0x1800 are available */
#define HEAP_KEYS_UPDATED   0x2000  /* tuple was updated and key 
cols

 * modified, or tuple deleted */
#define HEAP_HOT_UPDATED0x4000  /* tuple was HOT-updated */
#define HEAP_ONLY_TUPLE 0x8000  /* this is heap-only tuple */

#define HEAP2_XACT_MASK 0xE000  /* visibility-related bits */


- Heikki



--
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 {UPDATE | IGNORE} 2.0

[Heikki Linnakangas]

On 02/18/2015 11:43 PM, Peter Geoghegan wrote:

Heikki seemed to think that the deadlock problems were not really
worth fixing independently of ON CONFLICT UPDATE support, but rather
represented a useful way of committing code incrementally. Do I have
that right?


Yes.


The way I chose to break the livelock (what I call "livelock
insurance") does indeed involve comparing XIDs, which Heikki thought
most promising. But it also involves having the oldest XID wait on
another session's speculative token in the second phase, which
ordinarily does not occur in the second
phase/check_exclusion_or_unique_constraint() call. The idea is that
one session is guaranteed to be the waiter that has a second iteration
within its second-phase check_exclusion_or_unique_constraint() call,
where (following the super deletion of conflict TIDs by the other
conflicting session or sessions) reliably finds that it can proceed
(those other sessions are denied the opportunity to reach their second
phase by our second phase waiter's still-not-super-deleted tuple).

However, it's difficult to see how to map this on to general exclusion
constraint insertion + enforcement. In Heikki's recent sketch of this
[1], there is no pre-check, since that is considered an "UPSERT thing"
deferred until a later patch, and therefore my scheme here cannot work
(recall that for plain inserts with exclusion constraints, we insert
first and check last). I have a hard time justifying adding the
pre-check for plain exclusion constraint inserters given the total
lack of complaints from the field regarding unprincipled deadlocks,
and given the fact that it would probably make the code more
complicated than it needs to be. It is critical that there be a
pre-check to prevent livelock, though, because otherwise the
restarting sessions can go straight to their "second" phase
(check_exclusion_or_unique_constraint() call), without ever realizing
that they should not do so.


Hmm. I haven't looked at your latest patch, but I don't think you need 
to pre-check for this to work. To recap, the situation is that two 
backends have already inserted the heap tuple, and then see that the 
other backend's tuple conflicts. To avoid a livelock, it's enough that 
one backend super-deletes its own tuple first, before waiting for the 
other to complete, while the other other backend waits without 
super-deleting. No?



It seems like the livelock insurance is pretty close to or actually
free, and doesn't imply that a "second phase wait for token" needs to
happen too often. With heavy contention on a small number of possible
tuples (100), and 8 clients deleting and inserting, I can see it
happening only once every couple of hundred milliseconds on my laptop.
It's not hard to imagine why the code didn't obviously appear to be
broken before now, as the window for an actual livelock must have been
small. Also, deadlocks bring about more deadlocks (since the deadlock
detector kicks in), whereas livelocks do not bring about more
livelocks.


It might be easier to provoke the livelocks with a GiST opclass that's 
unusually slow. I wrote the attached opclass for the purpose of testing 
this a while ago, but I haven't actually gotten around to do much with 
it. It's called "useless_gist", because it's a GiST opclass for 
integers, like btree_gist, but the penalty and picksplit functions are 
totally dumb. The result is that the tuples are put to the index in 
pretty much random order, and every scan has to scan the whole index. 
I'm posting it here, in the hope that it happens to be useful, but I 
don't really know if it is.


- Heikki



useless_gist.tar.gz
Description: application/gzip

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

[HACKERS] xpath changes in the recent back branches

[Marko Tiikkaja]

Hi,

Commit 79af9a1d2668c9edc8171f03c39e7fed571eeb98 changed xpath handling 
with regard to namespaces, and it seems to be fixing an actual issue. 
However, it was also backpatched to all branches despite it breaking for 
example code like this:


do $$
declare
_x xml;
begin
_x := (xpath('/x:Foo/x:Bar', xml 'xmlns="teh:urn">12', 
array[['x','teh:urn']]))[1];

raise notice '%', xpath('/Bar/Baz/text()', _x);
raise notice '%', xpath('/Bar/Bat/text()', _x);
end
$$;

The problem is that there's no way to write the code like this in such a 
way that it would work on both versions.  If I add the namespace, it's 
broken on 9.1.14.  Without it it's broken on 9.1.15.


I'm now thinking of adding a workaround which strips namespaces, but 
that doesn't seem to be easy to do, even with PL/Perl.  Is there a 
better workaround here that I'm not seeing?


I'm not sure how changing behavior like this in a minor release was 
considered acceptable.



.m


--
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_basebackup may fail to send feedbacks.

[Fujii Masao]

On Wed, Feb 18, 2015 at 5:34 PM, Kyotaro HORIGUCHI
 wrote:
> Hello, this is the last patch for pg_basebackup/pg_receivexlog on
> master (9.5). Preor versions don't have this issue.
>
> 4. basebackup_reply_fix_mst_v2.patch
>   receivelog.c patch applyable on master.
>
> This is based on the same design with
> walrcv_reply_fix_91_v2.patch in the aspect of gettimeofday().

Thanks for updating the patches! But I'm still not sure if the idea depending
on the frequent calls of gettimeofday() for each WAL receive is good or not.
Some users may complain about the performance impact by such frequent calls
and we may want to get rid of them from walreceiver loop in the future.
If we adopt your idea now, I'm afraid that it would tie our hands in that case.

How much impact can such frequent calls of gettimeofday() have on replication
performance? If it's not negligible, probably we should remove them at first
and find out another idea to fix the problem you pointed. ISTM that it's not so
difficult to remove them. Thought? Do you have any numbers which can prove
that such frequent gettimeofday() has only ignorable impact on the performance?

Regards,

-- 
Fujii Masao


-- 
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 UPDATE and logical decoding

[Andres Freund]

Hi,

On 2015-02-18 16:35:14 -0800, Peter Geoghegan wrote:
> Andres pointed out that the INSERT ... ON CONFLICT UPDATE patch
> doesn't work well with logical decoding.

Just to make that clear: I didn't actually test it, but it ddidn't look
good.

> I guess that the best way of fixing this is exposing to output plugins
> that a "super deletion" is a
> REORDER_BUFFER_CHANGE_INTERNAL_SUPERDELETE.

I'm pretty much dead set against exposing anything like this output
plugins. The output plugin shouldn't care that a insertion was a
upsert. For one they won't get it right since they will be really
infrequent, for another it'll be hard to replay such an event on another
node.

> This is kind of an
> internal ReorderBufferChangeType constant, because it means that the
> output plugin should probably just omit the tuple just inserted and
> now deleted.

An output plugin can't just go back and call a tuple that was already
sent to a client back.

> I tend to think that it would be best if the fact that a
> speculative insertion was a speculative insertion is not exposed. We
> just formalize that a REORDER_BUFFER_CHANGE_INTERNAL_SUPERDELETE is a
> variant of REORDER_BUFFER_CHANGE_DELETE...it's implicit that things
> like triggers are not fired here (which may or may not matter,
> depending on the use case, I suppose).

I can't see that working. How such a event - that obviously causes
unique conflicts! - be replayed on another node?

> Would that be sufficiently flexible for the various logical
> replication use cases? I guess we are somewhat forced to push that
> kind of thing into output plugins, because doing so lets them decide
> how to handle this.

I don't see what benefits that'd bring.

> It's a little unfortunate that this implementation
> detail is exposed to output plugins, though, which is why I'd be
> willing to believe that it'd be better to have transaction reassembly
> normalize the records such that a super deleted tuple was never
> exposed to output plugins in the first place...

Yes.

> they'd only see a
> REORDER_BUFFER_CHANGE_INSERT when that was the definitive outcome of
> an UPSERT, or alternatively a REORDER_BUFFER_CHANGE_UPDATE when that
> was the definitive outcome. No need for output plugins to consider
> REORDER_BUFFER_CHANGE_INTERNAL_SUPERDELETE at all.

Yes. It'd be easiest if the only the final insert/update were actually
WAL logged as full actions. IIUC we can actually otherwise end with a,
theoretically, arbitrarily large chain of insert/super deletions.

> Thoughts? Can't say that I've given conflict resolution for
> multi-master systems a great deal of thought before now, so I might be
> quite off the mark here.

I don't think conflict resolution actually plays a role here. This is
about consistency inside a single system, not consistency across
systems.

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] Expanding the use of FLEXIBLE_ARRAY_MEMBER for declarations like foo[1]

[Andres Freund]

On 2015-02-18 21:00:43 -0500, Tom Lane wrote:
> Michael Paquier  writes:
> > 3) heapam.c in three places with HeapTupleHeaderData:
> > struct
> > {
> > HeapTupleHeaderData hdr;
> > chardata[MaxHeapTupleSize];
> > }   tbuf;
> 
> And this, though I'm not sure if we'd have to change the size of the
> padding data[] member.

I don't think so.
/*
 * MaxHeapTupleSize is the maximum allowed size of a heap tuple, including
 * header and MAXALIGN alignment padding.  Basically it's BLCKSZ minus the
 * other stuff that has to be on a disk page.  Since heap pages use no
 * "special space", there's no deduction for that.
...
#define MaxHeapTupleSize  (BLCKSZ - MAXALIGN(SizeOfPageHeaderData + 
sizeof(ItemIdData)))

> > 5) reorderbuffer.h with its use of HeapTupleHeaderData:
> 
> Hmm.  Andres will have to answer for that one ;-)

That should be fairly uncomplicated to replace.
...
/* tuple, stored sequentially */
HeapTupleData tuple;
HeapTupleHeaderData header;
chardata[MaxHeapTupleSize];

probably can just be replaced by a union of data and header part - as
quoted above MaxHeapTupleSize actually contains space for the
header. It's a bit annoying because potentially some output plugin might
reference .header - but they can just be changed to reference
tuple.t_data instead.

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] Expanding the use of FLEXIBLE_ARRAY_MEMBER for declarations like foo[1]

[Andres Freund]

On 2015-02-18 17:29:27 -0500, Tom Lane wrote:
> Michael Paquier  writes:
> > On Wed, Feb 18, 2015 at 10:09 PM, Andres Freund  
> > wrote:
> >> The compiler will complain if you use a FLEXIBLE_ARRAY_MEMBER in the
> >> middle of a struct but not when when you embed a struct that uses it
> >> into the middle another struct. At least gcc doesn't and I think it'd be
> >> utterly broken if another compiler did that. If there's a compiler that
> >> does so, we need to make it define FLEXIBLE_ARRAY_MEMBER to 1.
> 
> > clang does complain on my OSX laptop regarding that ;)
> 
> I'm a bit astonished that gcc doesn't consider this an error.  Sure seems
> like it should.

Why? The flexible arrary stuff tells the compiler that it doesn't have
to worry about space for the array - it seems alright that it actually
doesn't. There's pretty much no way you can do that sensibly if the
variable length array itself is somewhere in the middle of a struct -
but if you embed the whole struct somewhere you have to take care
yourself. And e.g. the varlena cases Michael has shown do just that?

> (Has anyone tried it on recent gcc?)

Yes.

> Moreover, if we have any code that is assuming such cases are okay, it
> probably needs a second look.  Isn't this situation effectively assuming
> that a variable-length array is fixed-length?

Not really. If you have
struct varlena hdr;
chardata[TOAST_MAX_CHUNK_SIZE]; /* make struct big 
enough */
the variable length part is preallocated in the data?

You're right that many of these structs could just be replaced with a
union though.

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] Odd behavior of updatable security barrier views on foreign tables

[Dean Rasheed]

On 18 February 2015 at 16:22, Stephen Frost  wrote:
> Here's the patch against master.  I'm still fiddling with the comment
> wording and the commit message a bit, but barring objections these
> patches are what I'm planning to move forward with.
>

Yes, that matches what I had in mind.

While you're tweaking comments, you might want to look at the comment
in the block above which also relates to this new code, and says that
"we will end up locking all rows which pass the securityQuals". That's
not really accurate, I think it wants to say something like more like
"we won't necessarily be able to push user-defined quals down into the
subquery since they may include untrusted functions, and that means
that we may end up locking rows that don't pass the user-defined
quals.  In the worst case, we may end up locking all rows which pass
the securityQuals...".

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