Re: [HACKERS] Upgrading postmaster's log messages about bind/listen errors

2017-03-10 Thread Craig Ringer
On 11 March 2017 at 03:24, Peter Eisentraut
 wrote:
> On 3/9/17 23:43, Tom Lane wrote:
>> However, if we're measuring this on a scale of usefulness to the average
>> DBA, I would argue that it's of more interest than any of these messages
>> that currently appear by default:
>>
>> 2017-03-09 23:40:12.334 EST [19335] LOG:  MultiXact member wraparound 
>> protections are now enabled
>> 2017-03-09 23:40:12.335 EST [19339] LOG:  autovacuum launcher started
>> 2017-03-09 23:40:12.336 EST [19341] LOG:  logical replication launcher 
>> started
>>
>> The first of those is surely past its sell-by date.  As for the other two,
>> we should log *failure* to start, but not the normal case.
>
> I'm OK with removing these.  No news is good news.

Right, and it doesn't tell us that autovacuum will actually do
anything useful. The user could've set ridiculous thresholds/delays,
per-table overrides, etc.

So it's not much use for remote support/diagnostics, and might as well go away.


-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


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


Re: [HACKERS] [PATCH] Transaction traceability - txid_status(bigint)

2017-03-10 Thread Craig Ringer
On 11 March 2017 at 05:09, Robert Haas  wrote:

> On the other
> hand, there really are two separate notions of the "oldest" XID.
> There's the oldest XID that we can safely look up, and then there's
> the oldest XID that we can't reuse.  These two are the same when no
> truncation is in progress, but when a truncation is in progress then
> they're different: the oldest XID that's safe to look up is the first
> one after whatever we're truncating away, but the oldest XID that we
> can't reuse is the newest one preceding the stuff that we're busy
> truncating.

Right.

My view here is that the oldest xid we cannot reuse is already guarded
by xidWrapLimit, which we advance after clog truncation. Whether as
this advances at the same time as or after we advance oldestXid and
truncate clog doesn't actually matter, we must just ensure that it
never advances _before_.

So tracking a second copy of oldestXid whose only purpose is to
recalculate xidWrapLimit serves no real purpose. It's redundant except
during vac_truncate_clog, during which time local state is sufficient
*if* we add oldestXid to the clog truncation xlog record, which we
must do anyway because:

Any number of locking hoop-jumping schemes fail to solve the problem
of outdated oldestXid information on standbys. Right now we truncate
clog and xlog the truncation before we write the new oldestXid limit
to xlog. In fact, we don't write the new xid limit to xlog until the
next checkpoint. So the standby has a huge window where its idea of
oldestXid is completely wrong, and unless we at least add the new
oldestXid to the clog truncation xlog record we can't fix that.

We only get away with this now because there's no way to look up an
arbitrary xid's status.

No locking scheme on the master can solve this, because the locks on
the master do not affect the standby or vice versa.

Therefore, we _must_ advance oldestXid (or a copy of it used only for
"oldest xid still in clog) before truncating clog.

If we're going to do that we might as well just make sure the
standby's xid limits are updated correctly when we truncate clog
rather than doing it lazily at checkpoints. Advance oldestXid before
truncating clog away, and record the new xid in the clog truncation
xlog record. On redo after master crash, and on standbys, we're
guaranteed to re-do the whole clog truncation operation - advance
oldestXid, truncate clog, advance xidWrapLimit etc - and everything
stays consistent.

I'll extract this part of the patch so it can be looked at separately,
it'll be clearer that way.

I think of it as slightly contracting then slightly expanding the xid
range window during clog truncation. Advance the oldest xid slightly
before the xidWrapLimit, so temporarily the range of xids is narrower
than 2^31. xlog it first so we ensure it's all redone on crash and on
standby. Because no lock is held throughout all of vac_truncate_clog,
make sure the ordering of the different phases between concurrent
vac_truncate_xlog runs doesn't matter.

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


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


[HACKERS] Logical replication initial feedback

2017-03-10 Thread Mark Kirkwood

Hi,

Thought I'd take a look at how this works (checking out current master). 
You guys have manged to get this to the point where it is *ridiculously* 
easy to set up a basic replication scenario - awesome i must say:


- change a few parameters on the master 
(max_wal_senders,max_replication_slots,wal_level)


- create a publication on the master

- pg_dump/restore relevant tables

- create a subscription of the slave


..and done. Very nice!


best wishes

Mark



--
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] New CORRESPONDING clause design

2017-03-10 Thread Pavel Stehule
Hi

2017-03-10 13:49 GMT+01:00 Pavel Stehule :

> Hi
>
> 2017-03-10 12:55 GMT+01:00 Pavel Stehule :
>
>>
>>
>> 2017-03-10 10:13 GMT+01:00 Surafel Temesgen :
>>
>>> Yes, you are correct it should to work on CORRESPONDING clause case. SQL
>>> 20nn standard draft only said each query to be of the same degree in a case
>>> of set operation without corresponding clause. The attached patch is
>>> corrected as such .I add those new test case to regression test too
>>>
>>
>> Thank you - I will recheck it.
>>
>
>  Fast check - it looks well
>

I am sending minor update - cleaning formatting and white spaces, error
messages + few more tests

It is working very well.

Maybe correspondingClause needs own node type with attached location. Then
context can be much better positioned.

Regards

Pavel


>
> Regards
>
> Pavel
>
diff --git a/doc/src/sgml/queries.sgml b/doc/src/sgml/queries.sgml
index 30792f45f1..c3cdee54ad 100644
--- a/doc/src/sgml/queries.sgml
+++ b/doc/src/sgml/queries.sgml
@@ -1601,6 +1601,9 @@ SELECT DISTINCT ON (expression , EXCEPT
   
   
+   CORRESPONDING
+  
+  
set union
   
   
@@ -1617,9 +1620,9 @@ SELECT DISTINCT ON (expression , 
-query1 UNION ALL query2
-query1 INTERSECT ALL query2
-query1 EXCEPT ALL query2
+query1 UNION ALL CORRESPONDING BY query2
+query1 INTERSECT ALL CORRESPONDING BY query2
+query1 EXCEPT ALL CORRESPONDING BY query2
 
query1 and
query2 are queries that can use any of
@@ -1659,11 +1662,22 @@ SELECT DISTINCT ON (expression , 
 
   
-   In order to calculate the union, intersection, or difference of two
-   queries, the two queries must be union compatible,
-   which means that they return the same number of columns and
-   the corresponding columns have compatible data types, as
-   described in .
+   EXCEPT returns all rows that are in the result of
+   query1 but not in the result of
+   query2.  (This is sometimes called the
+   difference between two queries.)  Again, duplicates
+   are eliminated unless EXCEPT ALL is used.
+  
+
+  
+   CORRESPONDING returns all columns that are in both 
+   query1 and query2 with the same name.
+  
+
+  
+   CORRESPONDING BY returns all columns in the column list 
+   that are also in both query1 and 
+   query2 with the same name.
   
  
 
diff --git a/doc/src/sgml/sql.sgml b/doc/src/sgml/sql.sgml
index 57396d7c24..f98c22e696 100644
--- a/doc/src/sgml/sql.sgml
+++ b/doc/src/sgml/sql.sgml
@@ -859,7 +859,7 @@ SELECT [ ALL | DISTINCT [ ON ( expressioncondition ]
 [ GROUP BY expression [, ...] ]
 [ HAVING condition [, ...] ]
-[ { UNION | INTERSECT | EXCEPT } [ ALL ] select ]
+[ { UNION | INTERSECT | EXCEPT } [ ALL ] [ CORRESPONDING [ BY ( expression ) ] ] select ]
 [ ORDER BY expression [ ASC | DESC | USING operator ] [ NULLS { FIRST | LAST } ] [, ...] ]
 [ LIMIT { count | ALL } ]
 [ OFFSET start ]
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index ac8e50ef1d..71e06e5c2e 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -2952,6 +2952,7 @@ _copySelectStmt(const SelectStmt *from)
 	COPY_NODE_FIELD(withClause);
 	COPY_SCALAR_FIELD(op);
 	COPY_SCALAR_FIELD(all);
+	COPY_NODE_FIELD(correspondingClause);
 	COPY_NODE_FIELD(larg);
 	COPY_NODE_FIELD(rarg);
 
@@ -2967,6 +2968,7 @@ _copySetOperationStmt(const SetOperationStmt *from)
 	COPY_SCALAR_FIELD(all);
 	COPY_NODE_FIELD(larg);
 	COPY_NODE_FIELD(rarg);
+	COPY_NODE_FIELD(correspondingColumns);
 	COPY_NODE_FIELD(colTypes);
 	COPY_NODE_FIELD(colTypmods);
 	COPY_NODE_FIELD(colCollations);
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 54e9c983a0..77bedc4b23 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -1041,6 +1041,7 @@ _equalSelectStmt(const SelectStmt *a, const SelectStmt *b)
 	COMPARE_NODE_FIELD(withClause);
 	COMPARE_SCALAR_FIELD(op);
 	COMPARE_SCALAR_FIELD(all);
+	COMPARE_NODE_FIELD(correspondingClause);
 	COMPARE_NODE_FIELD(larg);
 	COMPARE_NODE_FIELD(rarg);
 
@@ -1054,6 +1055,7 @@ _equalSetOperationStmt(const SetOperationStmt *a, const SetOperationStmt *b)
 	COMPARE_SCALAR_FIELD(all);
 	COMPARE_NODE_FIELD(larg);
 	COMPARE_NODE_FIELD(rarg);
+	COMPARE_NODE_FIELD(correspondingColumns);
 	COMPARE_NODE_FIELD(colTypes);
 	COMPARE_NODE_FIELD(colTypmods);
 	COMPARE_NODE_FIELD(colCollations);
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index 6e52eb7231..7102ea96c2 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -3444,6 +3444,8 @@ raw_expression_tree_walker(Node *node,
 	return true;
 if (walker(stmt->lockingClause, context))
 	return true;
+if (walker(stmt->correspondingClause, context))
+	return true;
 if (walker(stmt->withClause, context))
 	return true;
 if (walker(stmt->larg, context))
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c

Re: [HACKERS] Write Ahead Logging for Hash Indexes

2017-03-10 Thread Amit Kapila
On Thu, Mar 9, 2017 at 3:11 AM, Robert Haas  wrote:
> On Tue, Mar 7, 2017 at 6:41 PM, Robert Haas  wrote:
 Great, thanks.  0001 looks good to me now, so committed.
>>>
>>> Committed 0002.
>>
>> Here are some initial review thoughts on 0003 based on a first read-through.
>
> More thoughts on the main patch:
>
>  /*
> + * Change the shared buffer state in critical section,
> + * otherwise any error could make it unrecoverable after
> + * recovery.
> + */
> +START_CRIT_SECTION();
> +
> +/*
>   * Insert tuple on new page, using _hash_pgaddtup to ensure
>   * correct ordering by hashkey.  This is a tad inefficient
>   * since we may have to shuffle itempointers repeatedly.
>   * Possible future improvement: accumulate all the items for
>   * the new page and qsort them before insertion.
>   */
>  (void) _hash_pgaddtup(rel, nbuf, itemsz, new_itup);
>
> +END_CRIT_SECTION();
>
> No way.  You have to start the critical section before making any page
> modifications and keep it alive until all changes have been logged.
>

I think what we need to do here is to accumulate all the tuples that
need to be added to new bucket page till either that page has no more
space or there are no more tuples remaining in an old bucket.  Then in
a critical section, add them to the page using _hash_pgaddmultitup and
log the entire new bucket page contents as is currently done in patch
log_split_page().  Now, here we can choose to log the individual
tuples as well instead of a complete page, however not sure if there
is any benefit for doing the same because XLogRecordAssemble() will
anyway remove the empty space from the page.  Let me know if you have
something else in mind.

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


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


Re: [HACKERS] How to get the 'ctid' from a record type?

2017-03-10 Thread Eric Ridge
>
> I suspect the tuple at (0,1) has been the subject of a failed update.
>

Yep.


> Your problem here is that you're mistaking the t_ctid field of a tuple
> header for the tuple's address.  It is not that; it's really just garbage
> normally, and is only useful to link forward to the next version of the
> row from an outdated tuple.  I think we do initialize it to the tuple's
> own address during an INSERT, but either a completed or failed UPDATE
> would change it.
>

Thanks.  That helps clarify the comments in htup_details.h, actually.


> I do not think there is any way to get the true address of a heap tuple
> out of a composite Datum manufactured from the tuple.  Most of the other
> system columns can't be gotten from a composite Datum either, because of
> the field overlay in HeapTupleHeaderData's union t_choice.


Well shoot.  That kinda spoils my plans.

What about this?  Is the tuple currently being evaluated (I suppose in the
case of a sequential scan) available in the context of a function call?

Thanks again for your time!  It's much appreciated.

eric


Re: [HACKERS] Bizarre choice of case for RELKIND_PARTITIONED_TABLE

2017-03-10 Thread Bruce Momjian
On Thu, Mar  9, 2017 at 11:06:56PM -0300, Alvaro Herrera wrote:
> Tom Lane wrote:
> 
> > (And no, I don't especially
> > approve of RELKIND_SEQUENCE being 'S' either, but it's far too late to
> > change that.)
> 
> FWIW the reason SEQUENCE uses S instead of 's' is that the latter was
> taken for "special" relations, which we removed a few releases ago
> (commit 3a694bb0a1).

Ah!  I knew there was a reason for 'S', but couldn't remember it.  :-)

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

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +


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


[HACKERS] [GSOC 17] Eliminate O(N^2) scaling from rw-conflict tracking in serializable transactions

2017-03-10 Thread Mengxing Liu
Hi, all 

My name is Mengxing Liu. I am interested in the project "Eliminate O(N^2) 
scaling from rw-conflict tracking in serializable transactions”. After 
discussing with Kevin off-list, I think it's time to post discussion here. I am 
afraid that there are two things that I need your help. Thank you very much.

> 
> > So the task is clear. We can use a tree-like or hash-like data
> > structure to speed up this function.
> 
> Right -- especially with a large number of connections holding a
> large number of conflicts.  In one paper with high concurrency, they
> found over 50% of the CPU time for PostgreSQL was going to these
> functions (including functions called by them).  This seems to me to
> be due to the O(N^2) (or possibly worse) performance from the number
> of connections.

Anyone knows the title of this paper? I want to reproduce its workloads.

>
> Remember, I think most of the work here is going to be in
> benchmarking.  We not only need to show improvements in simple test
> cases using readily available tools like pgbench, but think about
> what types of cases might be worst for the approach taken and show
> that it still does well -- or at least not horribly.  It can be OK
> to have some slight regression in an unusual case if the common
> cases improve a lot, but any large regression needs to be addressed
> before the patch can be accepted.  There are some community members
> who are truly diabolical in their ability to devise "worst case"
> tests, and we don't want to be blind-sided by a bad result from one
> of them late in the process.
> 

Are there any documents or links introducing how to test and benchmark 
PostgreSQL? I may need some time to learn about it.

Thanks.

--
Mengxing Liu










-- 
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] make check-world output

2017-03-10 Thread Peter Eisentraut
On 3/10/17 19:26, Jeff Janes wrote:
> and there will be an exit code.
> 
> 
> True.  But I generally don't rely on that, unless the docs explicitly
> tell me to.
> 
> 
> If we show no output, then other people will complain that they can't
> tell whether it's hanging.
> 
> 
> Isn't that what stdout is for?

I haven't ever looked at the difference between stdout and stderr output
from the build process.  I rely on the exit code to tell me what
happened.  I don't mind changing things if it helps.

-- 
Peter Eisentraut  http://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] make check-world output

2017-03-10 Thread Tom Lane
Alvaro Herrera  writes:
> Jeff Janes wrote:
>> There was some recent discussion about making "make check-world" faster.
>> I'm all for that, but how about making it quieter?  On both machines I've
>> run it on (CentOS6.8 and Ubuntu 16.04.2), it dumps some gibberish to
>> stderr, example attached.

> I think you're complaining about the test code added by commit
> fcd15f13581f.  Together with behavior introduced by 2f227656076a, it is
> certainly annoying.  I would vote for redirecting that output to a log
> file which can be ignored/removed unless there is a failure.

What about just reverting 2f227656076a?

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] SERIALIZABLE with parallel query

2017-03-10 Thread Thomas Munro
On Wed, Feb 22, 2017 at 2:01 PM, Robert Haas  wrote:
> I don't think I know enough about the serializable code to review
> this, or at least not quickly, but it seems very cool if it works.
> Have you checked what effect it has on shared memory consumption?

I'm not sure how to test that.  Kevin, can you provide any pointers to
the test workloads you guys used when developing SSI?  In theory shmem
usage shouldn't change, since the predicate locks are shared by the
cooperating backends.  It might be able to use a bit more by creating
finer grain locks in worker A that are already covered by coarse
grained locks acquired by worker B or something like that, but it
seems unlikely if they tend to scan disjoint sets of pages.

Here is a rebased patch.

I should explain the included isolation test.  It's almost the same as
the SERIALIZABLE variant that I submitted for
https://commitfest.postgresql.org/13/949/.  That is a useful test here
because it's a serialisation anomaly that involves a read-only query.
In this version we run that query (s3r) in a parallel worker, and the
query plan comes out like this:

Gather  (cost=1013.67..1013.87 rows=2 width=64)
  Workers Planned: 1
  Single Copy: true
  ->  Sort  (cost=13.67..13.67 rows=2 width=64)
Sort Key: id
->  Bitmap Heap Scan on bank_account  (cost=8.32..13.66 rows=2 width=64)
  Recheck Cond: (id = ANY ('{X,Y}'::text[]))
  ->  Bitmap Index Scan on bank_account_pkey
(cost=0.00..8.32 rows=2 width=0)
Index Cond: (id = ANY ('{X,Y}'::text[]))

A dangerous cycle is detected, confirming that reads done by the
worker participate correctly in predicate locking and conflict
detection:

step s2wx: UPDATE bank_account SET balance = -11 WHERE id = 'X';
ERROR:  could not serialize access due to read/write dependencies
among transactions

It's probably too late for this WIP patch to get the kind of review
and testing it needs for PostgreSQL 10.  That's OK, but think it might
be a strategically good idea to get parallel SSI support (whether with
this or some other approach) into the tree before people start showing
up with parallel write patches.

-- 
Thomas Munro
http://www.enterprisedb.com


ssi-parallel-v4.patch
Description: Binary data

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


Re: [HACKERS] [PATCH] Enabling atomics on ARM64

2017-03-10 Thread Andres Freund
On 2017-03-10 18:14:13 -0800, Roman Shaposhnik wrote:
> On Fri, Mar 10, 2017 at 11:20 AM, Andres Freund  wrote:
> > Hi,
> >
> > On 2017-03-08 19:18:04 -0800, Roman Shaposhnik wrote:
> >> I'd like to offer a forward port from a change I'm contributing
> >> the Greenplum code base over here
> >
> > I think the change makes sense.  Any chance we could convince you to
> > bring up an arm64 buildfarm animal (our test infrastructure) up, so we
> > can be sure arm64 stays working?  See https://buildfarm.postgresql.org/
> 
> What does it take to be a your build farm slave? Can it be done via
> a Docker container that calls out to your infrastructure? I can probably
> run that as part of the Apache Bigtop CI.

Yes, it can.  The buildfarm stuff is all push from the test machine's
side, not pull.  You basically configure a cron job as described in
https://wiki.postgresql.org/wiki/PostgreSQL_Buildfarm_Howto and then
register your animal with the form on
https://buildfarm.postgresql.org/cgi-bin/register-form.pl

Would be great. We have a bunch of 32bit arm animal, but no 64bit ones:
https://buildfarm.postgresql.org/cgi-bin/show_members.pl
https://buildfarm.postgresql.org/cgi-bin/show_status.pl

Regards,

Andres


-- 
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] make check-world output

2017-03-10 Thread Alvaro Herrera
Jeff Janes wrote:
> There was some recent discussion about making "make check-world" faster.
> I'm all for that, but how about making it quieter?  On both machines I've
> run it on (CentOS6.8 and Ubuntu 16.04.2), it dumps some gibberish to
> stderr, example attached.

I think you're complaining about the test code added by commit
fcd15f13581f.  Together with behavior introduced by 2f227656076a, it is
certainly annoying.  I would vote for redirecting that output to a log
file which can be ignored/removed unless there is a failure.

-- 
Álvaro Herrerahttps://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] [PATCH] Enabling atomics on ARM64

2017-03-10 Thread Roman Shaposhnik
On Fri, Mar 10, 2017 at 11:20 AM, Andres Freund  wrote:
> Hi,
>
> On 2017-03-08 19:18:04 -0800, Roman Shaposhnik wrote:
>> I'd like to offer a forward port from a change I'm contributing
>> the Greenplum code base over here
>
> I think the change makes sense.  Any chance we could convince you to
> bring up an arm64 buildfarm animal (our test infrastructure) up, so we
> can be sure arm64 stays working?  See https://buildfarm.postgresql.org/

What does it take to be a your build farm slave? Can it be done via
a Docker container that calls out to your infrastructure? I can probably
run that as part of the Apache Bigtop CI.

> Pushed, thanks for your contribution.

Thanks for taking care of it so quickly!

Thanks,
Roman.


-- 
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] Need a builtin way to run all tests faster manner

2017-03-10 Thread Tom Lane
Peter Eisentraut  writes:
 make check-world -j2 seems to run fine for me.

> I have been pounding it a bit, and every so often the test_decoding
> tests fail in mysterious ways, but otherwise it seems to work fine.  I'm
> curious what you are seeing.

For me, it's quite unreliable at make parallelism above -j2 or so.

I believe the core problem is that contrib/test_decoding's regresscheck
and isolationcheck targets each want to use ./tmp_check as their
--temp-instance.  make has no reason to believe it shouldn't run those
two sub-jobs in parallel, but when it does, we get two postmasters trying
to share the same directory.  This looks reasonably straightforward to
solve, but I'm not entirely familiar with the code here, and am not
sure what is the least ugly way to fix it.

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] Indirect assignment code for array slices is dead code?

2017-03-10 Thread Andres Freund
Hi,

On 2017-03-10 20:59:48 -0500, Tom Lane wrote:
> Andres Freund  writes:
> > In the context of my expression evaluation patch, I was trying to
> > increase test coverage of execQual.c.  I'm a bit confused about
> > $subject.  ExecEvalArrayRef() has the following codepath:
> 
> I think it may indeed be unreachable at present, because we don't support
> something like this:
> 
> regression=# create table tt (f1 complex[]);
> CREATE TABLE
> regression=# update tt set f1[2:3].r = array[7,11];
> ERROR:  cannot assign to field "r" of column "f1" because its type complex[] 
> is not a composite type
> LINE 1: update tt set f1[2:3].r = array[7,11];
>   ^
> 
> regression=# update tt set f1[2:3].r = array[7,11];
> ERROR:  cannot assign to field "r" of column "f1" because its type complex[] 
> is not a composite type
> LINE 1: update tt set f1[2:3].r = 42;

Yea, that's where I got too.


> I would not like to remove it though.  This particular bit of the executor
> has no business making assumptions about how array and field references
> can be nested.

Not planning to - I am just trying to make sure I get the corner cases
right - if there had been a way to reach that bit of code, I'd have like
to understand how, before screwing around.

Thanks for looking,

Andres


-- 
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] Indirect assignment code for array slices is dead code?

2017-03-10 Thread Tom Lane
Andres Freund  writes:
> In the context of my expression evaluation patch, I was trying to
> increase test coverage of execQual.c.  I'm a bit confused about
> $subject.  ExecEvalArrayRef() has the following codepath:

I think it may indeed be unreachable at present, because we don't support
something like this:

regression=# create table tt (f1 complex[]);
CREATE TABLE
regression=# update tt set f1[2:3].r = array[7,11];
ERROR:  cannot assign to field "r" of column "f1" because its type complex[] is 
not a composite type
LINE 1: update tt set f1[2:3].r = array[7,11];
  ^

regression=# update tt set f1[2:3].r = array[7,11];
ERROR:  cannot assign to field "r" of column "f1" because its type complex[] is 
not a composite type
LINE 1: update tt set f1[2:3].r = 42;
  ^

I would not like to remove it though.  This particular bit of the executor
has no business making assumptions about how array and field references
can be nested.

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] How to get the 'ctid' from a record type?

2017-03-10 Thread Tom Lane
Eric Ridge  writes:
> What I'm seeing is that the ctid returned from this function isn't always
> correct:

> # select ctid, foo(table) from table limit 10;
>  ctid  |foo
> ---+---
>  (0,1) | (19195,1)-- not correct!
>  (0,2) | (0,2)
>  (0,3) | (0,3)

I suspect the tuple at (0,1) has been the subject of a failed update.

Your problem here is that you're mistaking the t_ctid field of a tuple
header for the tuple's address.  It is not that; it's really just garbage
normally, and is only useful to link forward to the next version of the
row from an outdated tuple.  I think we do initialize it to the tuple's
own address during an INSERT, but either a completed or failed UPDATE
would change it.

I do not think there is any way to get the true address of a heap tuple
out of a composite Datum manufactured from the tuple.  Most of the other
system columns can't be gotten from a composite Datum either, because of
the field overlay in HeapTupleHeaderData's union t_choice.

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] Indirect assignment code for array slices is dead code?

2017-03-10 Thread Andres Freund
Hi,

In the context of my expression evaluation patch, I was trying to
increase test coverage of execQual.c.  I'm a bit confused about
$subject.  ExecEvalArrayRef() has the following codepath:

if (isAssignment)
{
Datum   sourceData;
Datum   save_datum;
boolsave_isNull;

/*
 * We might have a nested-assignment situation, in which the
 * refassgnexpr is itself a FieldStore or ArrayRef that needs to
 * obtain and modify the previous value of the array element or slice
 * being replaced.  If so, we have to extract that value from the
 * array and pass it down via the econtext's caseValue.  It's safe to
 * reuse the CASE mechanism because there cannot be a CASE between
 * here and where the value would be needed, and an array assignment
 * can't be within a CASE either.  (So saving and restoring the
 * caseValue is just paranoia, but let's do it anyway.)
 *
 * Since fetching the old element might be a nontrivial expense, do it
 * only if the argument appears to actually need it.
 */
save_datum = econtext->caseValue_datum;
save_isNull = econtext->caseValue_isNull;

if (isAssignmentIndirectionExpr(astate->refassgnexpr))
if (*isNull)
{
...
}
else if (lIndex == NULL)
{
...
}
else
{
econtext->caseValue_datum =
array_get_slice(array_source, i,
upper.indx, lower.indx,
upperProvided, lowerProvided,
astate->refattrlength,
astate->refelemlength,
astate->refelembyval,
astate->refelemalign);
econtext->caseValue_isNull = false;
}
}
...

That's support for multiple indirect assignments, assigning to array
slices.  But I can't figure out how to reach that bit of code.

The !slice code can be reached:
CREATE TABLE arr(d int[]);
CREATE TABLE arr2(arr arr)
INSERT INTO arr2(arr[1].d, arr[2].d) VALUES(ARRAY[1,2],ARRAY[3,4]) RETURNING *
┌───┐
│  arr  │
├───┤
│ {"(\"{1,2}\")","(\"{3,4}\")"} │
└───┘

But I don't see how to the slice code can be reached.  The indirection
code is only triggered when isAssignmentIndirectionExpr is true, which
requires that refassgnexpr is a FieldStore/ArrayRef containing a
CaseTest.

But that doesn't make sense for slices, because there can't be a
FieldStore directly below a slice (you'd get "subfield "d" is of type
integer[] but expression is of type integer" - makes sense), nor can
there be an ArrayRef inside a slice ArrayRef for assignemnts, because
there are no parens in the LHS, and consecutive []'s are collapsed into
a single ArrayRef.

Am I standing on my own foot here?

Greetings,

Andres Freund


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


[HACKERS] How to get the 'ctid' from a record type?

2017-03-10 Thread Eric Ridge
This is about Postgres 9.6...

I have a very simple 1-arg function, in C, that I want to return the ctid
of the record passed in.  Example:

CREATE OR REPLACE FUNCTION foo(record) RETURNS tid LANGUAGE c IMMUTABLE
STRICT AS 'my_extension';

Its implementation is simply:

Datum foo(PG_FUNCTION_ARGS) {
HeapTupleHeader td = PG_GETARG_HEAPTUPLEHEADER(0);

PG_RETURN_POINTER(>t_ctid);
}

What I'm seeing is that the ctid returned from this function isn't always
correct:

# select ctid, foo(table) from table limit 10;
 ctid  |foo
---+---
 (0,1) | (19195,1)-- not correct!
 (0,2) | (0,2)
 (0,3) | (0,3)
 (0,4) | (0,4)
 (0,5) | (0,5)
 (0,6) | (0,6)
 (0,7) | (0,7)
 (1,1) | (1,1)
 (1,2) | (1,2)
 (1,3) | (1,3)
(10 rows)

I've spent hours tracing through the PG sources trying to figure out why
and/or find a different way to do this, but I've got nothing.

Of the various examples in the PG sources that use
PG_GETARG_HEAPTUPLEHEADER, or otherwise deal with a HeapTupleHeader, I
can't find any that want to access a system column, so I'm starting to
think I'm just doing it wrong entirely.

Can anyone point me in the right direction?

Thanks for your time!

eric


Re: [HACKERS] WIP: Faster Expression Processing v4

2017-03-10 Thread Bruce Momjian
On Fri, Mar 10, 2017 at 07:15:59PM -0500, Peter Eisentraut wrote:
> On 3/10/17 14:40, Andres Freund wrote:
> > I'd really like to get it in.  The performance improvements on its own
> > are significant, and it provides the basis for significantly larger
> > improvements again (JIT) - those followup improvements are large patches
> > again though, so I'd rather not do all of that next cycle.
> > 
> > My next step (over the weekend) is to add tests to execQual.c to get it
> > a good chunk closer to 100% test coverage, and then do the same for the
> > new implementation (there's probably very little additional tests needed
> > after the conversion).  Given all tests pass before/after, and there's a
> > lot of them, I think we can have a reasonable confidence of a low bug
> > density.
> 
> That seems like a plan, but now would probably be a good time for some
> other hackers to take note of this proposal.

Well, the executor is long overdue for improvement, so I would love to
have this in 10.0.  I am not sure what additional polishing will happen
if we punt it for 11.0.  I think the only downside of having it in 10.0
is that it will not have lived in the source tree for as long a time
between commit and PG release, but if it is tested, that seems fine.

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

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +


-- 
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] Speed up Clog Access by increasing CLOG buffers

2017-03-10 Thread Amit Kapila
On Sat, Mar 11, 2017 at 2:10 AM, Robert Haas  wrote:
> On Fri, Mar 10, 2017 at 6:25 AM, Amit Kapila  wrote:
>> On Fri, Mar 10, 2017 at 11:51 AM, Tom Lane  wrote:
>>> Amit Kapila  writes:
 Just to let you know that I think I have figured out the reason of
 failure.  If we run the regressions with attached patch, it will make
 the regression tests fail consistently in same way.  The patch just
 makes all transaction status updates to go via group clog update
 mechanism.
>>>
>>> This does *not* give me a warm fuzzy feeling that this patch was
>>> ready to commit.  Or even that it was tested to the claimed degree.
>>>
>>
>> I think this is more of an implementation detail missed by me.  We
>> have done quite some performance/stress testing with a different
>> number of savepoints, but this could have been caught only by having
>> Rollback to Savepoint followed by a commit.  I agree that we could
>> have devised some simple way (like the one I shared above) to test the
>> wide range of tests with this new mechanism earlier.  This is a
>> learning from here and I will try to be more cautious about such
>> things in future.
>
> After some study, I don't feel confident that it's this simple.  The
> underlying issue here is that TransactionGroupUpdateXidStatus thinks
> it can assume that proc->clogGroupMemberXid, pgxact->nxids, and
> proc->subxids.xids match the values that were passed to
> TransactionIdSetPageStatus, but that's not checked anywhere.  For
> example, I thought about adding these assertions:
>
>Assert(nsubxids == MyPgXact->nxids);
>Assert(memcmp(subxids, MyProc->subxids.xids,
>   nsubxids * sizeof(TransactionId)) == 0);
>
> There's not even a comment in the patch anywhere that notes that we're
> assuming this, let alone anything that checks that it's actually true,
> which seems worrying.
>
> One thing that seems off is that we have this new field
> clogGroupMemberXid, which we use to determine the XID that is being
> committed, but for the subxids we think it's going to be true in every
> case.   Well, that seems a bit odd, right?  I mean, if the contents of
> the PGXACT are a valid way to figure out the subxids that we need to
> worry about, then why not also it to get the toplevel XID?
>
> Another point that's kind of bothering me is that this whole approach
> now seems to me to be an abstraction violation.  It relies on the set
> of subxids for which we're setting status in clog matching the set of
> subxids advertised in PGPROC.  But actually there's a fair amount of
> separation between those things.  What's getting passed down to clog
> is coming from xact.c's transaction state stack, which is completely
> separate from the procarray.  Now after going over the logic in some
> detail, it does look to me that you're correct that in the case of a
> toplevel commit they will always match, but in some sense that looks
> accidental.
>
> For example, look at this code from RecordTransactionAbort:
>
> /*
>  * If we're aborting a subtransaction, we can immediately remove failed
>  * XIDs from PGPROC's cache of running child XIDs.  We do that here for
>  * subxacts, because we already have the child XID array at hand.  For
>  * main xacts, the equivalent happens just after this function returns.
>  */
> if (isSubXact)
> XidCacheRemoveRunningXids(xid, nchildren, children, latestXid);
>
> That code paints the removal of the aborted subxids from our PGPROC as
> an optimization, not a requirement for correctness.  And without this
> patch, that's correct: the XIDs are advertised in PGPROC so that we
> construct correct snapshots, but they only need to be present there
> for so long as there is a possibility that those XIDs might in the
> future commit.  Once they've aborted, it's not *necessary* for them to
> appear in PGPROC any more, but it doesn't hurt anything if they do.
> However, with this patch, removing them from PGPROC becomes a hard
> requirement, because otherwise the set of XIDs that are running
> according to the transaction state stack and the set that are running
> according to the PGPROC might be different.  Yet, neither the original
> patch nor your proposed fix patch updated any of the comments here.
>

There was a comment in existing code (proc.h) which states that it
will contain non-aborted transactions.  I agree that having it
explicitly mentioned in patch would have been much better.

/*
 * Each backend advertises up to PGPROC_MAX_CACHED_SUBXIDS TransactionIds
 * for non-aborted subtransactions of its current top transaction.  These
 * have to be treated as running XIDs by other backends.




> One might wonder whether it's even wise to tie these things together
> too closely.  For example, you can imagine a future patch for
> autonomous transactions stashing their XIDs in the subxids array.
> That'd be fine 

Re: [HACKERS] make check-world output

2017-03-10 Thread Jeff Janes
On Fri, Mar 10, 2017 at 4:00 PM, Peter Eisentraut <
peter.eisentr...@2ndquadrant.com> wrote:

> On 3/10/17 15:05, Jeff Janes wrote:
> > There was some recent discussion about making "make check-world"
> > faster.  I'm all for that, but how about making it quieter?  On both
> > machines I've run it on (CentOS6.8 and Ubuntu 16.04.2), it dumps some
> > gibberish to stderr, example attached.  Which first made me wonder
> > whether the test passed or failed, and then made me give up on running
> > it altogether when I couldn't easily figure that out. Am I supposed to
> > be seeing this?  Am I supposed to understand it?
>
> Well, you are kind of showing it out of context.  Normally it will tell
> you something at the end,


"make check" doesn't say anything (to stderr) at the end, unless there are
errors.  I am expecting the same of "make check-world".


> and there will be an exit code.
>

True.  But I generally don't rely on that, unless the docs explicitly tell
me to.


> If we show no output, then other people will complain that they can't
> tell whether it's hanging.
>

Isn't that what stdout is for?

Cheers,

Jeff


Re: [HACKERS] WIP: Faster Expression Processing v4

2017-03-10 Thread Peter Eisentraut
On 3/10/17 14:40, Andres Freund wrote:
> I'd really like to get it in.  The performance improvements on its own
> are significant, and it provides the basis for significantly larger
> improvements again (JIT) - those followup improvements are large patches
> again though, so I'd rather not do all of that next cycle.
> 
> My next step (over the weekend) is to add tests to execQual.c to get it
> a good chunk closer to 100% test coverage, and then do the same for the
> new implementation (there's probably very little additional tests needed
> after the conversion).  Given all tests pass before/after, and there's a
> lot of them, I think we can have a reasonable confidence of a low bug
> density.

That seems like a plan, but now would probably be a good time for some
other hackers to take note of this proposal.

-- 
Peter Eisentraut  http://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] make check-world output

2017-03-10 Thread Jeff Janes
On Fri, Mar 10, 2017 at 12:24 PM, Alvaro Herrera 
wrote:

> Jeff Janes wrote:
>
> > Also, it runs in about 8 minutes, not the 20 minutes reported by others.
> > My system is virtualized, and not particularly fast.  I wonder if it is
> > failing early somewhere without running to completion? How would/should I
> > know?
>
> Maybe you don't have --enable-tap-tests?
>

Yes, adding that does solve the too-fast "problem".  But it doesn't solve
the "I don't know what this output to stderr means" problem.

Should --enable-tap-tests be mentioned in "32.1.3. Additional Test Suites"?
  Or at least cross-referenced from "32.4. TAP Tests"?

It was obvious to me that check-world wouldn't test plperl unless plperl
was configured.  But not so obvious that it wouldn't test anything using
the tap framework unless that was configured, until after you pointed it
out.

Thanks,

Jeff


Re: [HACKERS] WIP: Faster Expression Processing v4

2017-03-10 Thread Peter Eisentraut
On 3/10/17 14:24, Andres Freund wrote:
> What problem are you thinking of exactly, and what'd be the solution?
> I'd guess that people wouldn't like being unable to change columns in a
> table if some function returned the type.

Well, that's pretty much the problem.

For example:

create type t1 as (a int, b int);
create function f1() returns setof t1 language plpgsql
as $$ begin return query values (1, 2); end $$;
alter type t1 drop attribute b;
select * from f1();  -- fail

The dependency system should arguably guard against that.  Yes, it would
make some things more cumbersome, but, well, it already does that.

>> Not necessarily in this patch, but I would like to keep the options
>> open.
> 
> Yea, seems worth thinking about.  I don't think the patch closes down
> options?

nope

-- 
Peter Eisentraut  http://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] Need a builtin way to run all tests faster manner

2017-03-10 Thread Peter Eisentraut
On 3/10/17 19:00, Jim Nasby wrote:
> Maybe instead of having the commitfest app try and divine patches from 
> the list it should be able to send patches to the list from a specified 
> git repo/branch. Anyone that provides that info would have tests run 
> automagically, patches sent, etc. Anyone who doesn't can just keep using 
> the old process.

Those people who know what they're doing will presumably run all those
checks before they submit a patch.  It's those people who send in
patches that don't apply cleanly or fail the tests that would benefit
from this system.  But if they're that careless, then they also won't
take care to use this particular system correctly.

-- 
Peter Eisentraut  http://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] Need a builtin way to run all tests faster manner

2017-03-10 Thread Peter Eisentraut
On 3/10/17 14:43, Andres Freund wrote:
> I do get regular issues, although the happen to not end up in visible
> failures.  All the tests output their regression.diffs into the same
> place - which means there'll every now be a failure to remove the file,
> and if there were an actual failure it'd possibly end up being
> attributed to the wrong test.  So I think we need to relocate that file
> to be relative to the sql/ | specs/ | expected/ directories?

I'm confused here.  Every test suite runs in a separate directory, and
for the tests in the same suite pg_regress will take care of running the
tests in parallel and producing the diffs.

-- 
Peter Eisentraut  http://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] make check-world output

2017-03-10 Thread Peter Eisentraut
On 3/10/17 15:24, Alvaro Herrera wrote:
> Jeff Janes wrote:
> 
>> Also, it runs in about 8 minutes, not the 20 minutes reported by others.
>> My system is virtualized, and not particularly fast.  I wonder if it is
>> failing early somewhere without running to completion? How would/should I
>> know?
> 
> Maybe you don't have --enable-tap-tests?

Yeah, that'll cut down the time.  The recovery tests alone are 5 minutes.

-- 
Peter Eisentraut  http://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] make check-world output

2017-03-10 Thread Peter Eisentraut
On 3/10/17 15:05, Jeff Janes wrote:
> There was some recent discussion about making "make check-world"
> faster.  I'm all for that, but how about making it quieter?  On both
> machines I've run it on (CentOS6.8 and Ubuntu 16.04.2), it dumps some
> gibberish to stderr, example attached.  Which first made me wonder
> whether the test passed or failed, and then made me give up on running
> it altogether when I couldn't easily figure that out. Am I supposed to
> be seeing this?  Am I supposed to understand it?

Well, you are kind of showing it out of context.  Normally it will tell
you something at the end, and there will be an exit code.

If we show no output, then other people will complain that they can't
tell whether it's hanging.

-- 
Peter Eisentraut  http://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] Need a builtin way to run all tests faster manner

2017-03-10 Thread Jim Nasby

On 3/10/17 5:57 PM, Peter Eisentraut wrote:

On 3/10/17 14:53, Jim Nasby wrote:

The biggest win we'd get from something like Travis would be if the
commitfest monitored for new patch files coming in for monitored threads
and it created a new branch, applied the patches, and if they applied
without error commit the branch and push to let Travis do it's thing. We
wouldn't want that running in the main git repo, but it should be fine
in a fork that's dedicated to that purpose.


This has been discussed several times before, e.g.,

https://www.postgresql.org/message-id/54dd2413.8030...@gmx.net


Maybe instead of having the commitfest app try and divine patches from 
the list it should be able to send patches to the list from a specified 
git repo/branch. Anyone that provides that info would have tests run 
automagically, patches sent, etc. Anyone who doesn't can just keep using 
the old process.

--
Jim Nasby, Chief Data Architect, OpenSCG
http://OpenSCG.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] Need a builtin way to run all tests faster manner

2017-03-10 Thread Peter Eisentraut
On 3/10/17 14:53, Jim Nasby wrote:
> The biggest win we'd get from something like Travis would be if the 
> commitfest monitored for new patch files coming in for monitored threads 
> and it created a new branch, applied the patches, and if they applied 
> without error commit the branch and push to let Travis do it's thing. We 
> wouldn't want that running in the main git repo, but it should be fine 
> in a fork that's dedicated to that purpose.

This has been discussed several times before, e.g.,

https://www.postgresql.org/message-id/54dd2413.8030...@gmx.net

-- 
Peter Eisentraut  http://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] Upgrading postmaster's log messages about bind/listen errors

2017-03-10 Thread Tom Lane
I wrote:
> I think that what would actually be of some use nowadays is a LOG-level
> message emitted if the wraparound *isn't* activated immediately at start.
> But otherwise, we should follow the rule that silence is golden.

Concretely, how about the attached?  It preserves the original
"protections are now enabled" message at LOG level, but emits it only
when oldestOffsetKnown becomes true *after* startup.  Meanwhile, if
oldestOffsetKnown is still not true at the conclusion of TrimMultiXact,
then it emits a new LOG message about "protections are not active".
In this way we have LOG messages but they're only emitted in "interesting"
cases.

I dropped the IsUnderPostmaster test because I see no good reason not
to warn in standalone backends as well.

I think this might actually be a reasonable candidate to back-patch,
because a deficiency of the existing code is that it fails to warn
you when something's wrong.  But in any case I'd like to put it in HEAD.

regards, tom lane

diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 59d1252..3f66cdb 100644
*** a/src/backend/access/transam/multixact.c
--- b/src/backend/access/transam/multixact.c
*** static void ExtendMultiXactOffset(MultiX
*** 363,369 
  static void ExtendMultiXactMember(MultiXactOffset offset, int nmembers);
  static bool MultiXactOffsetWouldWrap(MultiXactOffset boundary,
  		 MultiXactOffset start, uint32 distance);
! static bool SetOffsetVacuumLimit(void);
  static bool find_multixact_start(MultiXactId multi, MultiXactOffset *result);
  static void WriteMZeroPageXlogRec(int pageno, uint8 info);
  static void WriteMTruncateXlogRec(Oid oldestMultiDB,
--- 363,369 
  static void ExtendMultiXactMember(MultiXactOffset offset, int nmembers);
  static bool MultiXactOffsetWouldWrap(MultiXactOffset boundary,
  		 MultiXactOffset start, uint32 distance);
! static bool SetOffsetVacuumLimit(bool is_startup);
  static bool find_multixact_start(MultiXactId multi, MultiXactOffset *result);
  static void WriteMZeroPageXlogRec(int pageno, uint8 info);
  static void WriteMTruncateXlogRec(Oid oldestMultiDB,
*** TrimMultiXact(void)
*** 2095,2101 
  	LWLockRelease(MultiXactGenLock);
  
  	/* Now compute how far away the next members wraparound is. */
! 	SetMultiXactIdLimit(oldestMXact, oldestMXactDB);
  }
  
  /*
--- 2095,2101 
  	LWLockRelease(MultiXactGenLock);
  
  	/* Now compute how far away the next members wraparound is. */
! 	SetMultiXactIdLimit(oldestMXact, oldestMXactDB, true);
  }
  
  /*
*** MultiXactSetNextMXact(MultiXactId nextMu
*** 2186,2194 
   * Determine the last safe MultiXactId to allocate given the currently oldest
   * datminmxid (ie, the oldest MultiXactId that might exist in any database
   * of our cluster), and the OID of the (or a) database with that value.
   */
  void
! SetMultiXactIdLimit(MultiXactId oldest_datminmxid, Oid oldest_datoid)
  {
  	MultiXactId multiVacLimit;
  	MultiXactId multiWarnLimit;
--- 2186,2198 
   * Determine the last safe MultiXactId to allocate given the currently oldest
   * datminmxid (ie, the oldest MultiXactId that might exist in any database
   * of our cluster), and the OID of the (or a) database with that value.
+  *
+  * is_startup is true when we are just starting the cluster, false when we
+  * are updating state in a running cluster.  This only affects log messages.
   */
  void
! SetMultiXactIdLimit(MultiXactId oldest_datminmxid, Oid oldest_datoid,
! 	bool is_startup)
  {
  	MultiXactId multiVacLimit;
  	MultiXactId multiWarnLimit;
*** SetMultiXactIdLimit(MultiXactId oldest_d
*** 2277,2283 
  	Assert(!InRecovery);
  
  	/* Set limits for offset vacuum. */
! 	needs_offset_vacuum = SetOffsetVacuumLimit();
  
  	/*
  	 * If past the autovacuum force point, immediately signal an autovac
--- 2281,2287 
  	Assert(!InRecovery);
  
  	/* Set limits for offset vacuum. */
! 	needs_offset_vacuum = SetOffsetVacuumLimit(is_startup);
  
  	/*
  	 * If past the autovacuum force point, immediately signal an autovac
*** MultiXactAdvanceOldest(MultiXactId oldes
*** 2370,2376 
  	Assert(InRecovery);
  
  	if (MultiXactIdPrecedes(MultiXactState->oldestMultiXactId, oldestMulti))
! 		SetMultiXactIdLimit(oldestMulti, oldestMultiDB);
  }
  
  /*
--- 2374,2380 
  	Assert(InRecovery);
  
  	if (MultiXactIdPrecedes(MultiXactState->oldestMultiXactId, oldestMulti))
! 		SetMultiXactIdLimit(oldestMulti, oldestMultiDB, false);
  }
  
  /*
*** GetOldestMultiXactId(void)
*** 2537,2543 
   * otherwise.
   */
  static bool
! SetOffsetVacuumLimit(void)
  {
  	MultiXactId oldestMultiXactId;
  	MultiXactId nextMXact;
--- 2541,2547 
   * otherwise.
   */
  static bool
! SetOffsetVacuumLimit(bool is_startup)
  {
  	MultiXactId oldestMultiXactId;
  	MultiXactId nextMXact;
*** SetOffsetVacuumLimit(void)
*** 2619,2627 
  		/* 

Re: [HACKERS] asynchronous execution

2017-03-10 Thread Corey Huinker
On Thu, Feb 23, 2017 at 6:59 AM, Kyotaro HORIGUCHI <
horiguchi.kyot...@lab.ntt.co.jp> wrote:

> 9e43e87


Patch fails on current master, but correctly applies to 9e43e87. Thanks for
including the commit id.

Regression tests pass.

As with my last attempt at reviewing this patch, I'm confused about what
kind of queries can take advantage of this patch. Is it only cases where a
local table has multiple inherited foreign table children? Will it work
with queries where two foreign tables are referenced and combined with a
UNION ALL?


Re: [HACKERS] Should we eliminate or reduce HUP from docs?

2017-03-10 Thread David G. Johnston
On Fri, Mar 10, 2017 at 3:51 PM, Alvaro Herrera 
wrote:

> David G. Johnston wrote:
> > On Fri, Mar 10, 2017 at 3:28 PM, Alvaro Herrera <
> alvhe...@2ndquadrant.com>
>
> > is incomplete.
>
> Sure.  We can just reword that along the lines of " ... and when a
> reload signal is received, see 19.1.3".  Don't you like that?
>
>
​WFM
​

>
> > The order or some of these items is interesting but given the general
> lack
> > of field complaints and questions it mustn't be confusion inducing.  Even
> > this thread isn't an actual complaint but rather concern about signals in
> > general.  Pulling the relevant paragraph out to its own section in 19.1
> was
> > my first reaction as well and has the merit of simplicity.
>
> Simplicity FTW.
>
>
​WFM​

David J.


Re: [HACKERS] Should we eliminate or reduce HUP from docs?

2017-03-10 Thread Alvaro Herrera
David G. Johnston wrote:
> On Fri, Mar 10, 2017 at 3:28 PM, Alvaro Herrera 
> wrote:
> 
> > I think we could split 19.1.2 in two parts, where the first one is the
> > current content minus the paragraph "The configuration file is reread".
> > We'd create "19.1.3 Configuration File Reloads" to contain that
> > paragraph, perhaps not with the exact current wording.
> 
> ​If only 19 and 20 need it I would say its a coin-toss.​

Well, I think it's better to back-reference chapter 19 from 20 than the
other way around.

> > Dunno.  Given that other configuration elements such as config file
> > placement are already in chapter 19, it seems strange to put reloading
> > behavior in 18.
> >
> ​It wouldn't be hateful to cross link to 19 from 20 - but assuming
> pg_reload_conf() impacts pg_hba.conf​ (I don't know off-hand)

(Yes it does.)

> the paragraph
> 
> """
> The pg_hba.conf file is read on start-up and when the main server process
> receives a SIGHUP signal. If you edit the file on an active system, you
> will need to signal the postmaster (using pg_ctl reload or kill -HUP) to
> make it re-read the file.
> """
> 
> is incomplete.

Sure.  We can just reword that along the lines of " ... and when a
reload signal is received, see 19.1.3".  Don't you like that?

> Is "kill" portable?

It isn't -- it doesn't work on Windows, RDS.

> The order or some of these items is interesting but given the general lack
> of field complaints and questions it mustn't be confusion inducing.  Even
> this thread isn't an actual complaint but rather concern about signals in
> general.  Pulling the relevant paragraph out to its own section in 19.1 was
> my first reaction as well and has the merit of simplicity.

Simplicity FTW.

-- 
Álvaro Herrerahttps://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] scram and \password

2017-03-10 Thread Michael Paquier
On Sat, Mar 11, 2017 at 2:53 AM, Jeff Janes  wrote:
> Should the \password tool in psql inspect password_encryption and act on it
> being 'scram'?

Not sure if it is wise to change the default fot this release.

> I didn't see this issue discussed, but the ability to search the archives
> for backslashes is rather limited.

I'll save you some time: it has not been discussed. Nor has
PQencryptPassword been mentioned. Changing to SCRAM is not that
complicated, just call scram_build_verifier() and you are good to go.

Instead of changing the default, I think that we should take this
occasion to rename PQencryptPassword to something like
PQhashPassword(), and extend it with a method argument to support both
md5 and scram. PQencryptPassword could also be marked as deprecated,
or let as-is for some time. For \password, we could have another
meta-command but that sounds grotty, or just extend \password with a
--method argument. Switching the default could happen in another
release.

A patch among those lines would be a simple, do people feel that this
should be part of PG 10?
-- 
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] Should we eliminate or reduce HUP from docs?

2017-03-10 Thread David G. Johnston
On Fri, Mar 10, 2017 at 3:28 PM, Alvaro Herrera 
wrote:

> David G. Johnston wrote:
> > On Fri, Mar 10, 2017 at 1:02 PM, Alvaro Herrera <
> alvhe...@2ndquadrant.com>
> > wrote:
>
> > > There are several ways to cause a config file reload (pg_ctl reload,
> > > pg_reload_conf, direct SIGHUP).  We could have a section in docs
> listing
> > > them all, and then all the other places that say a reload needs to
> occur
> > > simply refer the reader to that section.
> >
> > ​19.1.2 contains a fairly comprehensive coverage of the topic ​- but
> > postgres.conf is not the only thing that gets reloaded.  Specifically,
> > "Client Authentication" (chapter 20) is also affected.
>
> I think we could split 19.1.2 in two parts, where the first one is the
> current content minus the paragraph "The configuration file is reread".
> We'd create "19.1.3 Configuration File Reloads" to contain that
> paragraph, perhaps not with the exact current wording.
>

​If only 19 and 20 need it I would say its a coin-toss.​


> > One theory would be to consider "configuration reload" part of "18.
> Server
> > ... Operation" and document the mechanics there with forward references
> to
> > 19/Configuration and 20/Authentication.
>
> Dunno.  Given that other configuration elements such as config file
> placement are already in chapter 19, it seems strange to put reloading
> behavior in 18.
>
>
​It wouldn't be hateful to cross link to 19 from 20 - but assuming
pg_reload_conf() impacts pg_hba.conf​ (I don't know off-hand) the paragraph

"""
The pg_hba.conf file is read on start-up and when the main server process
receives a SIGHUP signal. If you edit the file on an active system, you
will need to signal the postmaster (using pg_ctl reload or kill -HUP) to
make it re-read the file.
"""

is incomplete.

Is "kill" portable?

The order or some of these items is interesting but given the general lack
of field complaints and questions it mustn't be confusion inducing.  Even
this thread isn't an actual complaint but rather concern about signals in
general.  Pulling the relevant paragraph out to its own section in 19.1 was
my first reaction as well and has the merit of simplicity.

David J.


Re: [HACKERS] Should buffer of initialization fork have a BM_PERMANENT flag

2017-03-10 Thread Michael Paquier
On Sat, Mar 11, 2017 at 12:03 AM, Artur Zakirov
 wrote:
> Because BM_PERMANENT is used for init forks of unlogged indexes now.

Yes, indeed.
-- 
Michael
diff --git a/contrib/bloom/blinsert.c b/contrib/bloom/blinsert.c
index 913f1f8a51..3557b106d8 100644
--- a/contrib/bloom/blinsert.c
+++ b/contrib/bloom/blinsert.c
@@ -158,31 +158,24 @@ blbuild(Relation heap, Relation index, IndexInfo *indexInfo)
 void
 blbuildempty(Relation index)
 {
-	Page		metapage;
+	Buffer		MetaBuffer;
 
-	/* Construct metapage. */
-	metapage = (Page) palloc(BLCKSZ);
-	BloomFillMetapage(index, metapage);
+	/* An empty bloom index has one meta page */
+	MetaBuffer =
+		ReadBufferExtended(index, INIT_FORKNUM, P_NEW, RBM_NORMAL, NULL);
+	LockBuffer(MetaBuffer, BUFFER_LOCK_EXCLUSIVE);
 
-	/*
-	 * Write the page and log it.  It might seem that an immediate sync
-	 * would be sufficient to guarantee that the file exists on disk, but
-	 * recovery itself might remove it while replaying, for example, an
-	 * XLOG_DBASE_CREATE or XLOG_TBLSPC_CREATE record.  Therefore, we
-	 * need this even when wal_level=minimal.
-	 */
-	PageSetChecksumInplace(metapage, BLOOM_METAPAGE_BLKNO);
-	smgrwrite(index->rd_smgr, INIT_FORKNUM, BLOOM_METAPAGE_BLKNO,
-			  (char *) metapage, true);
-	log_newpage(>rd_smgr->smgr_rnode.node, INIT_FORKNUM,
-BLOOM_METAPAGE_BLKNO, metapage, false);
+	/* Initialize the meta buffer */
+	BloomFillMetapage(index, BufferGetPage(MetaBuffer));
 
-	/*
-	 * An immediate sync is required even if we xlog'd the page, because the
-	 * write did not go through shared_buffers and therefore a concurrent
-	 * checkpoint may have moved the redo pointer past our xlog record.
-	 */
-	smgrimmedsync(index->rd_smgr, INIT_FORKNUM);
+	/* And log it */
+	START_CRIT_SECTION();
+	MarkBufferDirty(MetaBuffer);
+	log_newpage_buffer(MetaBuffer, false);
+	END_CRIT_SECTION();
+
+	/* Unlock and release the buffer */
+	UnlockReleaseBuffer(MetaBuffer);
 }
 
 /*
diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index 775f2ff1f8..e1801ea939 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -23,6 +23,7 @@
 #include "access/xlog.h"
 #include "catalog/index.h"
 #include "commands/vacuum.h"
+#include "miscadmin.h"
 #include "pgstat.h"
 #include "storage/condition_variable.h"
 #include "storage/indexfsm.h"
@@ -282,31 +283,22 @@ btbuildCallback(Relation index,
 void
 btbuildempty(Relation index)
 {
-	Page		metapage;
-
-	/* Construct metapage. */
-	metapage = (Page) palloc(BLCKSZ);
-	_bt_initmetapage(metapage, P_NONE, 0);
-
-	/*
-	 * Write the page and log it.  It might seem that an immediate sync
-	 * would be sufficient to guarantee that the file exists on disk, but
-	 * recovery itself might remove it while replaying, for example, an
-	 * XLOG_DBASE_CREATE or XLOG_TBLSPC_CREATE record.  Therefore, we
-	 * need this even when wal_level=minimal.
-	 */
-	PageSetChecksumInplace(metapage, BTREE_METAPAGE);
-	smgrwrite(index->rd_smgr, INIT_FORKNUM, BTREE_METAPAGE,
-			  (char *) metapage, true);
-	log_newpage(>rd_smgr->smgr_rnode.node, INIT_FORKNUM,
-BTREE_METAPAGE, metapage, false);
-
-	/*
-	 * An immediate sync is required even if we xlog'd the page, because the
-	 * write did not go through shared_buffers and therefore a concurrent
-	 * checkpoint may have moved the redo pointer past our xlog record.
-	 */
-	smgrimmedsync(index->rd_smgr, INIT_FORKNUM);
+	Buffer		metabuffer;
+
+	/* An empty btree index has one meta page */
+	metabuffer =
+		ReadBufferExtended(index, INIT_FORKNUM, P_NEW, RBM_NORMAL, NULL);
+	LockBuffer(metabuffer, BUFFER_LOCK_EXCLUSIVE);
+
+	/* Initialize and xlog meta buffer */
+	START_CRIT_SECTION();
+	_bt_initmetapage(BufferGetPage(metabuffer), P_NONE, 0);
+	MarkBufferDirty(metabuffer);
+	log_newpage_buffer(metabuffer, false);
+	END_CRIT_SECTION();
+
+	/* Unlock and release the buffer */
+	UnlockReleaseBuffer(metabuffer);
 }
 
 /*
diff --git a/src/backend/access/spgist/spginsert.c b/src/backend/access/spgist/spginsert.c
index 00a0ab4438..4252c2eb53 100644
--- a/src/backend/access/spgist/spginsert.c
+++ b/src/backend/access/spgist/spginsert.c
@@ -156,49 +156,50 @@ spgbuild(Relation heap, Relation index, IndexInfo *indexInfo)
 void
 spgbuildempty(Relation index)
 {
-	Page		page;
-
-	/* Construct metapage. */
-	page = (Page) palloc(BLCKSZ);
-	SpGistInitMetapage(page);
+	Buffer		MetaBuffer,
+RootBuffer,
+TupleBuffer;
 
 	/*
-	 * Write the page and log it unconditionally.  This is important
-	 * particularly for indexes created on tablespaces and databases
-	 * whose creation happened after the last redo pointer as recovery
-	 * removes any of their existing content when the corresponding
-	 * create records are replayed.
+	 * An empty SPGist index has three pages:
+	 * - one meta page.
+	 * - one root page.
+	 * - one null-tuple root page.
 	 */
-	PageSetChecksumInplace(page, SPGIST_METAPAGE_BLKNO);
-	smgrwrite(index->rd_smgr, INIT_FORKNUM, 

Re: [HACKERS] Should we eliminate or reduce HUP from docs?

2017-03-10 Thread Alvaro Herrera
David G. Johnston wrote:
> On Fri, Mar 10, 2017 at 1:02 PM, Alvaro Herrera 
> wrote:

> > There are several ways to cause a config file reload (pg_ctl reload,
> > pg_reload_conf, direct SIGHUP).  We could have a section in docs listing
> > them all, and then all the other places that say a reload needs to occur
> > simply refer the reader to that section.
> 
> ​19.1.2 contains a fairly comprehensive coverage of the topic ​- but
> postgres.conf is not the only thing that gets reloaded.  Specifically,
> "Client Authentication" (chapter 20) is also affected.

I think we could split 19.1.2 in two parts, where the first one is the
current content minus the paragraph "The configuration file is reread".
We'd create "19.1.3 Configuration File Reloads" to contain that
paragraph, perhaps not with the exact current wording.

> One theory would be to consider "configuration reload" part of "18. Server
> ... Operation" and document the mechanics there with forward references to
> 19/Configuration and 20/Authentication.

Dunno.  Given that other configuration elements such as config file
placement are already in chapter 19, it seems strange to put reloading
behavior in 18.

-- 
Álvaro Herrerahttps://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] Speed up Clog Access by increasing CLOG buffers

2017-03-10 Thread Alvaro Herrera
Robert Haas wrote:

> The smoking gun was in 009_twophase_slave.log:
> 
> TRAP: FailedAssertion("!(nsubxids == MyPgXact->nxids)", File:
> "clog.c", Line: 288)
> 
> ...and then the node shuts down, which is why this hangs forever.
> (Also... what's up with it hanging forever instead of timing out or
> failing or something?)

This bit my while messing with 2PC tests recently.  I think it'd be
worth doing something about this, such as causing the test to die if we
request a server to (re)start and it doesn't start or it immediately
crashes.  This doesn't solve the problem of a server crashing at a point
not immediately after start, though.

(It'd be very annoying to have to sprinkle the Perl test code with
"assert $server->islive", but perhaps we can add assertions of some kind
in PostgresNode itself).

-- 
Álvaro Herrerahttps://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] Need a builtin way to run all tests faster manner

2017-03-10 Thread Jim Nasby

On 3/10/17 2:18 PM, Magnus Hagander wrote:

But if you can put together something that picks up the individual
patches out of the mail threads in the CF app and keeps branch-tips in a
git repo up-to-date with those, including feeding the results back into
the app, then go for it :)


Seems like an ideal project for someone not on -hackers... do we have a 
list of "How you can help Postgres besides hacking database code" anywhere?

--
Jim Nasby, Chief Data Architect, OpenSCG
http://OpenSCG.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] Speed up Clog Access by increasing CLOG buffers

2017-03-10 Thread Robert Haas
On Fri, Mar 10, 2017 at 3:40 PM, Robert Haas  wrote:
> Finally, I had an unexplained hang during the TAP tests while testing
> out your fix patch.  I haven't been able to reproduce that so it
> might've just been an artifact of something stupid I did, or of some
> unrelated bug, but I think it's best to back up and reconsider a bit
> here.

I was able to reproduce this with the following patch:

diff --git a/src/backend/access/transam/clog.c
b/src/backend/access/transam/clog.c
index bff42dc..0546425 100644
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -268,9 +268,11 @@ set_status_by_pages(int nsubxids, TransactionId *subxids,
  * has a race condition (see TransactionGroupUpdateXidStatus) but the
  * worst thing that happens if we mess up is a small loss of efficiency;
  * the intent is to avoid having the leader access pages it wouldn't
- * otherwise need to touch.  Finally, we skip it for prepared transactions,
- * which don't have the semaphore we would need for this optimization,
- * and which are anyway probably not all that common.
+ * otherwise need to touch.  We also skip it if the transaction status is
+ * other than commit, because for rollback and rollback to savepoint, the
+ * list of subxids won't be same as subxids array in PGPROC.  Finally, we skip
+ * it for prepared transactions, which don't have the semaphore we would need
+ * for this optimization, and which are anyway probably not all that common.
  */
 static void
 TransactionIdSetPageStatus(TransactionId xid, int nsubxids,
@@ -280,15 +282,20 @@ TransactionIdSetPageStatus(TransactionId xid,
int nsubxids,
 {
 if (all_xact_same_page &&
 nsubxids < PGPROC_MAX_CACHED_SUBXIDS &&
+status == TRANSACTION_STATUS_COMMITTED &&
 !IsGXactActive())
 {
+Assert(nsubxids == MyPgXact->nxids);
+Assert(memcmp(subxids, MyProc->subxids.xids,
+   nsubxids * sizeof(TransactionId)) == 0);
+
 /*
  * If we can immediately acquire CLogControlLock, we update the status
  * of our own XID and release the lock.  If not, try use group XID
  * update.  If that doesn't work out, fall back to waiting for the
  * lock to perform an update for this transaction only.
  */
-if (LWLockConditionalAcquire(CLogControlLock, LW_EXCLUSIVE))
+if (false && LWLockConditionalAcquire(CLogControlLock, LW_EXCLUSIVE))
 {
 TransactionIdSetPageStatusInternal(xid, nsubxids,
subxids, status, lsn, pageno);
 LWLockRelease(CLogControlLock);

make check-world hung here:

t/009_twophase.pl ..
1..13
ok 1 - Commit prepared transaction after restart
ok 2 - Rollback prepared transaction after restart

[rhaas pgsql]$ ps uxww | grep postgres
rhaas 72255   0.0  0.0  2447996   1684 s000  S+3:40PM   0:00.00
/Users/rhaas/pgsql/tmp_install/Users/rhaas/install/dev/bin/psql -XAtq
-d port=64230 host=/var/folders/y8/r2ycj_jj2vd65v71rmyddpr4gn/T/ZVWy0JGbuw
dbname='postgres' -f - -v ON_ERROR_STOP=1
rhaas 72253   0.0  0.0  2478532   1548   ??  Ss3:40PM   0:00.00
postgres: bgworker: logical replication launcher
rhaas 72252   0.0  0.0  2483132740   ??  Ss3:40PM   0:00.05
postgres: stats collector process
rhaas 72251   0.0  0.0  2486724   1952   ??  Ss3:40PM   0:00.02
postgres: autovacuum launcher process
rhaas 72250   0.0  0.0  2477508880   ??  Ss3:40PM   0:00.03
postgres: wal writer process
rhaas 72249   0.0  0.0  2477508972   ??  Ss3:40PM   0:00.03
postgres: writer process
rhaas 72248   0.0  0.0  2477508   1252   ??  Ss3:40PM   0:00.00
postgres: checkpointer process
rhaas 72246   0.0  0.0  2481604   5076 s000  S+3:40PM   0:00.03
/Users/rhaas/pgsql/tmp_install/Users/rhaas/install/dev/bin/postgres -D
/Users/rhaas/pgsql/src/test/recovery/tmp_check/data_master_Ylq1/pgdata
rhaas 72337   0.0  0.0  2433796688 s002  S+4:14PM   0:00.00
grep postgres
rhaas 72256   0.0  0.0  2478920   2984   ??  Ss3:40PM   0:00.00
postgres: rhaas postgres [local] COMMIT PREPARED waiting for 0/301D0D0

Backtrace of PID 72256:

#0  0x7fff8ecc85c2 in poll ()
#1  0x0001078eb727 in WaitEventSetWaitBlock [inlined] () at
/Users/rhaas/pgsql/src/backend/storage/ipc/latch.c:1118
#2  0x0001078eb727 in WaitEventSetWait (set=0x7fab3c8366c8,
timeout=-1, occurred_events=0x7fff585e5410, nevents=1,
wait_event_info=)
at latch.c:949
#3  0x0001078eb409 in WaitLatchOrSocket (latch=, wakeEvents=, sock=-1, timeout=,
wait_event_info=134217741) at latch.c:349
#4  0x0001078cf077 in SyncRepWaitForLSN (lsn=, commit=) at syncrep.c:284
#5  0x0001076a2dab in FinishPreparedTransaction (gid=, isCommit=1 '\001') at
twophase.c:2110
#6  0x000107919420 in standard_ProcessUtility (pstmt=, queryString=,
context=PROCESS_UTILITY_TOPLEVEL, params=0x0, dest=0x7fab3c853cf8,
completionTag=)
at utility.c:452
#7  0x0001079186f3 in PortalRunUtility (portal=0x7fab3c874a40,

Re: [HACKERS] Should we eliminate or reduce HUP from docs?

2017-03-10 Thread David G. Johnston
On Fri, Mar 10, 2017 at 1:02 PM, Alvaro Herrera 
wrote:

> Joshua D. Drake wrote:
>
> > I am a bad speaker, I am writing a talk three weeks before the conference
> > (as opposed to on the plane).
>
> Hah.
>
> > I noticed in the docs we still reference the
> > passing of SIGHUP for reloading conf file but we now have
> pg_reload_conf();
> >
> > It seems the use of pg_reload_conf() would provide a better canonical
> > interface to our users. Especially those users who are not used to
> > interacting with the OS (Windows, Oracle etc...) for databases.
>
> There are several ways to cause a config file reload (pg_ctl reload,
> pg_reload_conf, direct SIGHUP).  We could have a section in docs listing
> them all, and then all the other places that say a reload needs to occur
> simply refer the reader to that section.
>

​19.1.2 contains a fairly comprehensive coverage of the topic ​- but
postgres.conf is not the only thing that gets reloaded.  Specifically,
"Client Authentication" (chapter 20) is also affected.

One theory would be to consider "configuration reload" part of "18. Server
... Operation" and document the mechanics there with forward references to
19/Configuration and 20/Authentication.  The existing content in those
chapters discussing reload would then send the reader back to 18 for "how
to reload" and just state "when to reload" in their particular situations.

Any other spots that warrant the same treatment?

If we are going to touch this area up it might be worth a fresh
consideration of index entries too.

David J.


Re: [HACKERS] [PATCH] Transaction traceability - txid_status(bigint)

2017-03-10 Thread Robert Haas
On Fri, Mar 10, 2017 at 2:00 AM, Craig Ringer  wrote:
> On 10 March 2017 at 02:55, Robert Haas  wrote:
>> Well, that's why I tried to advocate that their should be two separate
>> XID limits in shared memory: leave what's there now the way it is, and
>> then add a new limit for "don't try to look up XIDs before this point:
>> splat".  I still think that'd be less risky.
>
> I'm coming back to this "cold" after an extended break, so I hope I
> get the details right.

Yeah, sorry I've been away from this for a while.

> TL;DR: doing it that way duplicates a bunch of stuff and is ugly
> without offering significant benefit over just fixing the original.
>
> I started out with the approach you suggested, but it turns out to be
> less helpful than you'd think. Simply advancing a new lower limit
> field before beginning truncation is no help; there's then a race
> where the lower-limit field can be advanced after we test it but
> before we actually do the SLRU read from clog. To guard against this
> it's necessary for SLRU truncation to take an exclusive lock during
> which it advances the lower bound. That way a concurrent reader can
> take the lock in shared mode before reading the lower bound and hold
> it until it finishes the clog read, knowing that vacuum cannot then
> truncate the data out from under it because it'll block trying to
> advance the lower limit.

That's a good point which I hadn't fully considered.  On the other
hand, there really are two separate notions of the "oldest" XID.
There's the oldest XID that we can safely look up, and then there's
the oldest XID that we can't reuse.  These two are the same when no
truncation is in progress, but when a truncation is in progress then
they're different: the oldest XID that's safe to look up is the first
one after whatever we're truncating away, but the oldest XID that we
can't reuse is the newest one preceding the stuff that we're busy
truncating.  IOW, when truncation is happening, there's a portion of
the XID space whose clog files are being removed - and the XIDs that
are in that range aren't safe to look up any more, but are also not
available for reuse to prevent wraparound.  Right now, all of the
relevant fields in VariableCacheData are based on the ready-for-reuse
concept, and I don't think that switching some but not all of them to
be based on the safe-to-look-up concept necessarily qualifies as an
improvement.  It's different, but I'm not sure it's better.

What if we approached this problem from the other end?  Suppose we use
a heavyweight lock on, say, transaction ID 1 to represent the right to
truncate CLOG.  We grab this lock in exclusive mode before beginning
to truncate, and in shared mode while looking up XIDs.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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


Re: [HACKERS] Speed up Clog Access by increasing CLOG buffers

2017-03-10 Thread Robert Haas
On Fri, Mar 10, 2017 at 6:25 AM, Amit Kapila  wrote:
> On Fri, Mar 10, 2017 at 11:51 AM, Tom Lane  wrote:
>> Amit Kapila  writes:
>>> Just to let you know that I think I have figured out the reason of
>>> failure.  If we run the regressions with attached patch, it will make
>>> the regression tests fail consistently in same way.  The patch just
>>> makes all transaction status updates to go via group clog update
>>> mechanism.
>>
>> This does *not* give me a warm fuzzy feeling that this patch was
>> ready to commit.  Or even that it was tested to the claimed degree.
>>
>
> I think this is more of an implementation detail missed by me.  We
> have done quite some performance/stress testing with a different
> number of savepoints, but this could have been caught only by having
> Rollback to Savepoint followed by a commit.  I agree that we could
> have devised some simple way (like the one I shared above) to test the
> wide range of tests with this new mechanism earlier.  This is a
> learning from here and I will try to be more cautious about such
> things in future.

After some study, I don't feel confident that it's this simple.  The
underlying issue here is that TransactionGroupUpdateXidStatus thinks
it can assume that proc->clogGroupMemberXid, pgxact->nxids, and
proc->subxids.xids match the values that were passed to
TransactionIdSetPageStatus, but that's not checked anywhere.  For
example, I thought about adding these assertions:

   Assert(nsubxids == MyPgXact->nxids);
   Assert(memcmp(subxids, MyProc->subxids.xids,
  nsubxids * sizeof(TransactionId)) == 0);

There's not even a comment in the patch anywhere that notes that we're
assuming this, let alone anything that checks that it's actually true,
which seems worrying.

One thing that seems off is that we have this new field
clogGroupMemberXid, which we use to determine the XID that is being
committed, but for the subxids we think it's going to be true in every
case.   Well, that seems a bit odd, right?  I mean, if the contents of
the PGXACT are a valid way to figure out the subxids that we need to
worry about, then why not also it to get the toplevel XID?

Another point that's kind of bothering me is that this whole approach
now seems to me to be an abstraction violation.  It relies on the set
of subxids for which we're setting status in clog matching the set of
subxids advertised in PGPROC.  But actually there's a fair amount of
separation between those things.  What's getting passed down to clog
is coming from xact.c's transaction state stack, which is completely
separate from the procarray.  Now after going over the logic in some
detail, it does look to me that you're correct that in the case of a
toplevel commit they will always match, but in some sense that looks
accidental.

For example, look at this code from RecordTransactionAbort:

/*
 * If we're aborting a subtransaction, we can immediately remove failed
 * XIDs from PGPROC's cache of running child XIDs.  We do that here for
 * subxacts, because we already have the child XID array at hand.  For
 * main xacts, the equivalent happens just after this function returns.
 */
if (isSubXact)
XidCacheRemoveRunningXids(xid, nchildren, children, latestXid);

That code paints the removal of the aborted subxids from our PGPROC as
an optimization, not a requirement for correctness.  And without this
patch, that's correct: the XIDs are advertised in PGPROC so that we
construct correct snapshots, but they only need to be present there
for so long as there is a possibility that those XIDs might in the
future commit.  Once they've aborted, it's not *necessary* for them to
appear in PGPROC any more, but it doesn't hurt anything if they do.
However, with this patch, removing them from PGPROC becomes a hard
requirement, because otherwise the set of XIDs that are running
according to the transaction state stack and the set that are running
according to the PGPROC might be different.  Yet, neither the original
patch nor your proposed fix patch updated any of the comments here.

One might wonder whether it's even wise to tie these things together
too closely.  For example, you can imagine a future patch for
autonomous transactions stashing their XIDs in the subxids array.
That'd be fine for snapshot purposes, but it would break this.

Finally, I had an unexplained hang during the TAP tests while testing
out your fix patch.  I haven't been able to reproduce that so it
might've just been an artifact of something stupid I did, or of some
unrelated bug, but I think it's best to back up and reconsider a bit
here.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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


Re: [HACKERS] pg_dump segfaults with publication

2017-03-10 Thread Peter Eisentraut
On 3/6/17 03:06, Amit Langote wrote:
> pg_dump segfaults if there are more than one DO_PUBLICATION_REL objects to
> dump.

Fix committed with a test case.  Thanks.

-- 
Peter Eisentraut  http://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] Upgrading postmaster's log messages about bind/listen errors

2017-03-10 Thread Tom Lane
Alvaro Herrera  writes:
> Tom Lane wrote:
>> Uh, what's that got to do with it?  I'm not proposing to downgrade this
>> message in 9.3, or indeed any current release branch.  But it's hard
>> to believe that a 9.3 installation that had the problem would manage
>> to make it all the way to a v10 upgrade without the problem having been
>> fixed.  Even if there's one or two incompetent DBAs out there whom
>> that would apply to, I don't think the rest of the world needs to keep
>> seeing this message by default.

> Well, you could take a broken 9.3 installation and pg_upgrade it to
> pg10.  It wouldn't be any less broken.

Sure, but does the rest of the world need to keep looking at this message?
Anybody who's actually in that situation probably never looks at the
postmaster log at all.

I think that what would actually be of some use nowadays is a LOG-level
message emitted if the wraparound *isn't* activated immediately at start.
But otherwise, we should follow the rule that silence is golden.

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] Upgrading postmaster's log messages about bind/listen errors

2017-03-10 Thread Alvaro Herrera
Tom Lane wrote:
> Euler Taveira  writes:
> > 2017-03-10 1:43 GMT-03:00 Tom Lane :
> >> 2017-03-09 23:40:12.334 EST [19335] LOG:  MultiXact member wraparound
> >> protections are now enabled
> 
> > It should be DEBUG1 as soon as 9.3 is deprecated.
> 
> Uh, what's that got to do with it?  I'm not proposing to downgrade this
> message in 9.3, or indeed any current release branch.  But it's hard
> to believe that a 9.3 installation that had the problem would manage
> to make it all the way to a v10 upgrade without the problem having been
> fixed.  Even if there's one or two incompetent DBAs out there whom
> that would apply to, I don't think the rest of the world needs to keep
> seeing this message by default.

Well, you could take a broken 9.3 installation and pg_upgrade it to
pg10.  It wouldn't be any less broken.

There's still people running buggy 9.3 releases, as we see in these
lists every now and then.

-- 
Álvaro Herrerahttps://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] make check-world output

2017-03-10 Thread Alvaro Herrera
Jeff Janes wrote:

> Also, it runs in about 8 minutes, not the 20 minutes reported by others.
> My system is virtualized, and not particularly fast.  I wonder if it is
> failing early somewhere without running to completion? How would/should I
> know?

Maybe you don't have --enable-tap-tests?

-- 
Álvaro Herrerahttps://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] Upgrading postmaster's log messages about bind/listen errors

2017-03-10 Thread Tom Lane
Euler Taveira  writes:
> 2017-03-10 1:43 GMT-03:00 Tom Lane :
>> 2017-03-09 23:40:12.334 EST [19335] LOG:  MultiXact member wraparound
>> protections are now enabled

> It should be DEBUG1 as soon as 9.3 is deprecated.

Uh, what's that got to do with it?  I'm not proposing to downgrade this
message in 9.3, or indeed any current release branch.  But it's hard
to believe that a 9.3 installation that had the problem would manage
to make it all the way to a v10 upgrade without the problem having been
fixed.  Even if there's one or two incompetent DBAs out there whom
that would apply to, I don't think the rest of the world needs to keep
seeing this message by default.

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] Need a builtin way to run all tests faster manner

2017-03-10 Thread Magnus Hagander
On Fri, Mar 10, 2017 at 3:11 PM, Jim Nasby  wrote:

> On 3/10/17 2:05 PM, Magnus Hagander wrote:
>
>>
>> Travis specifically would not help us with this, due to the dependency
>> on gifhub, but something that knows how to run "patch ... && configure
>> && make && make check" in a container would.
>>
>
> Who's updating https://github.com/postgres/postgres/ right now?
> Presumably that script would be the basis for this...


Yes. It's a pure mirror script.

I'm pretty sure we *don't* want to push a branch for every single patch
that goes in a CF all into our master repository...

We could use a completely separate repo on github for it if we want, yes.
Then we just need to figure out how to get the patches there..


>
> I'm unsure what would be easiest -- have something drive a "throwaway
>> github repo" off the data in the CF app and try to pull things from
>> there, or to just spawn containers and run it directly without travis.
>>
>
> I'd be a bit nervous about creating our own container solution and opening
> that to automatically deploying patches. Travis (and other tools) already
> have that problem solved (or at least if they get hacked it's on them to
> clean up and not us :)
>
> Plus it'd be a heck of a lot more work on our side to set all that stuff
> up.


This is what I'm not so sure about. Setting up an empty container provided
we only ever need to test the one thing and only ever need the one platform
is not particularly complicated.

But if you can put together something that picks up the individual patches
out of the mail threads in the CF app and keeps branch-tips in a git repo
up-to-date with those, including feeding the results back into the app,
then go for it :)




> The bigger issue with those is the usual -- how do you handle patches
>> that have dependencies on each other,because they're always going to
>> show up as broken individually. I guess we could tell people doing those
>> to just push a git branch on github and register that one in the CF app
>> (which does have some very basic support for tracking that, but I doubt
>> anybody uses it today).
>>
>
> If people use git format-patch it should JustWork(tm). Specifying a
> specific repo is another option.
>

Right. But people don't use that all the time, and it's not currently a
requirement on patch submitters. and we've traditionally been of the
opinion that we don't want to put too much requirements on such things for
submitters.



> Even if we can't make it work for really complicated patches it might
> still be a win.


Definitely as long as we can keep some manual process for thos epatches.



> If the travis build failed, commitfest could notify the author.
>>
>> It could also rebase master into each branch on a daily basis so
>> authors would know very quickly if something got committed that
>> broke their patch.
>>
>>
>> It could at least verify that the patch still applies, yes.
>>
>
> If the rebase was pushed to github and travis was setup, travis would then
> test the changes as well.
>
>
Right.

//Magnus


Re: [HACKERS] Need a builtin way to run all tests faster manner

2017-03-10 Thread Jim Nasby

On 3/10/17 2:05 PM, Magnus Hagander wrote:


Travis specifically would not help us with this, due to the dependency
on gifhub, but something that knows how to run "patch ... && configure
&& make && make check" in a container would.


Who's updating https://github.com/postgres/postgres/ right now? 
Presumably that script would be the basis for this...



I'm unsure what would be easiest -- have something drive a "throwaway
github repo" off the data in the CF app and try to pull things from
there, or to just spawn containers and run it directly without travis.


I'd be a bit nervous about creating our own container solution and 
opening that to automatically deploying patches. Travis (and other 
tools) already have that problem solved (or at least if they get hacked 
it's on them to clean up and not us :)


Plus it'd be a heck of a lot more work on our side to set all that stuff up.


The bigger issue with those is the usual -- how do you handle patches
that have dependencies on each other,because they're always going to
show up as broken individually. I guess we could tell people doing those
to just push a git branch on github and register that one in the CF app
(which does have some very basic support for tracking that, but I doubt
anybody uses it today).


If people use git format-patch it should JustWork(tm). Specifying a 
specific repo is another option.


Even if we can't make it work for really complicated patches it might 
still be a win.



If the travis build failed, commitfest could notify the author.

It could also rebase master into each branch on a daily basis so
authors would know very quickly if something got committed that
broke their patch.


It could at least verify that the patch still applies, yes.


If the rebase was pushed to github and travis was setup, travis would 
then test the changes as well.

--
Jim Nasby, Chief Data Architect, OpenSCG
http://OpenSCG.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] Need a builtin way to run all tests faster manner

2017-03-10 Thread Magnus Hagander
On Fri, Mar 10, 2017 at 2:53 PM, Jim Nasby  wrote:

> On 3/10/17 1:09 PM, Peter Eisentraut wrote:
>
>> On 3/10/17 03:27, Jim Nasby wrote:
>>
>>> Perhaps https://travis-ci.org/ or something similar could be used for
>>> this. That avoids any issues about random code.
>>>
>>
>> That doesn't achieve any platform coverage, which is the main point here.
>>
>
> I don't think platform coverage is the first thing to worry about with
> patches, nor with ongoing development.
>

I think we are talking about solving two different problems...


>
> The biggest win we'd get from something like Travis would be if the
> commitfest monitored for new patch files coming in for monitored threads
> and it created a new branch, applied the patches, and if they applied
> without error commit the branch and push to let Travis do it's thing. We
> wouldn't want that running in the main git repo, but it should be fine in a
> fork that's dedicated to that purpose.
>

Travis specifically would not help us with this, due to the dependency on
gifhub, but something that knows how to run "patch ... && configure && make
&& make check" in a container would.

I'm unsure what would be easiest -- have something drive a "throwaway
github repo" off the data in the CF app and try to pull things from there,
or to just spawn containers and run it directly without travis.

The bigger issue with those is the usual -- how do you handle patches that
have dependencies on each other,because they're always going to show up as
broken individually. I guess we could tell people doing those to just push
a git branch on github and register that one in the CF app (which does have
some very basic support for tracking that, but I doubt anybody uses it
today).



> If the travis build failed, commitfest could notify the author.
>
> It could also rebase master into each branch on a daily basis so authors
> would know very quickly if something got committed that broke their patch.
>

It could at least verify that the patch still applies, yes.



> Obviously that doesn't remove the need for manual testing or the
> buildfarm, but it would at least let everyone know that the patch passed a
> smoke test


It's definitely something that would be useful, but as you say, it solves a
different problem.

//Magnus


[HACKERS] make check-world output

2017-03-10 Thread Jeff Janes
There was some recent discussion about making "make check-world" faster.
I'm all for that, but how about making it quieter?  On both machines I've
run it on (CentOS6.8 and Ubuntu 16.04.2), it dumps some gibberish to
stderr, example attached.  Which first made me wonder whether the test
passed or failed, and then made me give up on running it altogether when I
couldn't easily figure that out. Am I supposed to be seeing this?  Am I
supposed to understand it?

Also, it runs in about 8 minutes, not the 20 minutes reported by others.
My system is virtualized, and not particularly fast.  I wonder if it is
failing early somewhere without running to completion? How would/should I
know?

Cheers,

Jeff
+ standard_initdb 
/home/jjanes/pgsql/git/src/bin/pg_upgrade/tmp_check/install//usr/local/pgsql/bin/initdb
+ 
/home/jjanes/pgsql/git/src/bin/pg_upgrade/tmp_check/install//usr/local/pgsql/bin/initdb
 -N

WARNING: enabling "trust" authentication for local connections
You can change this by editing pg_hba.conf or using the option -A, or
--auth-local and --auth-host, the next time you run initdb.
+ [ -n  -a -r  ]
+ ../../test/regress/pg_regress --config-auth 
/home/jjanes/pgsql/git/src/bin/pg_upgrade/tmp_check/data.old
+ 
/home/jjanes/pgsql/git/src/bin/pg_upgrade/tmp_check/install//usr/local/pgsql/bin/pg_ctl
 start -l /home/jjanes/pgsql/git/src/bin/pg_upgrade/log/postmaster1.log -o -F 
-c listen_addresses= -k "/tmp/pg_upgrade_check-rP2T4V" -w
+ awk BEGIN { for (i= 1; i < 46; i++)
if (i != 7 && i != 10 && i != 13) printf "%c", i }
+ dbname1=    !"#$%&'()*+,-
+ dbname1=\"\ !"#$%&'()*+,-\\"\\\
+ awk BEGIN { for (i = 46; i <  91; i++) printf "%c", i }
+ dbname2=./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ
+ awk BEGIN { for (i = 91; i < 128; i++) printf "%c", i }
+ dbname3=[\]^_`abcdefghijklmnopqrstuvwxyz{|}~
+ createdb \"\    !"#$%&'()*+,-\\"\\\
+ createdb ./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ
+ createdb [\]^_`abcdefghijklmnopqrstuvwxyz{|}~
+ make -C /home/jjanes/pgsql/git installcheck
NOTICE:  database "regression" does not exist, skipping
+ pg_dumpall -f /home/jjanes/pgsql/git/src/bin/pg_upgrade/tmp_check/dump1.sql
+ [ /home/jjanes/pgsql/git != /home/jjanes/pgsql/git ]
+ 
/home/jjanes/pgsql/git/src/bin/pg_upgrade/tmp_check/install//usr/local/pgsql/bin/pg_ctl
 -m fast stop
+ [ -n  ]
+ [ -n  ]
+ [ -n  ]
+ [ -n  ]
+ PGDATA=/home/jjanes/pgsql/git/src/bin/pg_upgrade/tmp_check/data
+ standard_initdb initdb
+ initdb -N

WARNING: enabling "trust" authentication for local connections
You can change this by editing pg_hba.conf or using the option -A, or
--auth-local and --auth-host, the next time you run initdb.
+ [ -n  -a -r  ]
+ ../../test/regress/pg_regress --config-auth 
/home/jjanes/pgsql/git/src/bin/pg_upgrade/tmp_check/data
+ pg_upgrade -d /home/jjanes/pgsql/git/src/bin/pg_upgrade/tmp_check/data.old -D 
/home/jjanes/pgsql/git/src/bin/pg_upgrade/tmp_check/data -b 
/home/jjanes/pgsql/git/src/bin/pg_upgrade/tmp_check/install//usr/local/pgsql/bin
 -B 
/home/jjanes/pgsql/git/src/bin/pg_upgrade/tmp_check/install//usr/local/pgsql/bin
 -p 50848 -P 50848
+ pg_ctl start -l /home/jjanes/pgsql/git/src/bin/pg_upgrade/log/postmaster2.log 
-o -F -c listen_addresses= -k "/tmp/pg_upgrade_check-rP2T4V" -w
+ sh ./analyze_new_cluster.sh
+ pg_dumpall -f /home/jjanes/pgsql/git/src/bin/pg_upgrade/tmp_check/dump2.sql
+ pg_ctl -m fast stop
+ set +x

-- 
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] Should we eliminate or reduce HUP from docs?

2017-03-10 Thread Alvaro Herrera
Joshua D. Drake wrote:

> I am a bad speaker, I am writing a talk three weeks before the conference
> (as opposed to on the plane).

Hah.

> I noticed in the docs we still reference the
> passing of SIGHUP for reloading conf file but we now have pg_reload_conf();
> 
> It seems the use of pg_reload_conf() would provide a better canonical
> interface to our users. Especially those users who are not used to
> interacting with the OS (Windows, Oracle etc...) for databases.

There are several ways to cause a config file reload (pg_ctl reload,
pg_reload_conf, direct SIGHUP).  We could have a section in docs listing
them all, and then all the other places that say a reload needs to occur
simply refer the reader to that section.

-- 
Álvaro Herrerahttps://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] Should we eliminate or reduce HUP from docs?

2017-03-10 Thread Andres Freund
Hi,

On 2017-03-10 11:57:30 -0800, Joshua D. Drake wrote:
> I am a bad speaker, I am writing a talk three weeks before the conference
> (as opposed to on the plane). I noticed in the docs we still reference the
> passing of SIGHUP for reloading conf file but we now have pg_reload_conf();
> 
> It seems the use of pg_reload_conf() would provide a better canonical
> interface to our users. Especially those users who are not used to
> interacting with the OS (Windows, Oracle etc...) for databases.

-1 HUP is useful for external control. Doesn't require to have a valid
log-in into the database.

Regards,

Andres


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


[HACKERS] Should we eliminate or reduce HUP from docs?

2017-03-10 Thread Joshua D. Drake

Hello,

I am a bad speaker, I am writing a talk three weeks before the 
conference (as opposed to on the plane). I noticed in the docs we still 
reference the passing of SIGHUP for reloading conf file but we now have 
pg_reload_conf();


It seems the use of pg_reload_conf() would provide a better canonical 
interface to our users. Especially those users who are not used to 
interacting with the OS (Windows, Oracle etc...) for databases.


Sincerely,

JD
--
Command Prompt, Inc.  http://the.postgres.company/
+1-503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Everyone appreciates your honesty, until you are honest with them.
Unless otherwise stated, opinions are my own.


--
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] ANALYZE command progress checker

2017-03-10 Thread Jim Nasby

On 3/10/17 1:06 PM, Andres Freund wrote:

Hi,

On 2017-03-10 02:11:18 -0600, Jim Nasby wrote:

Perhaps instead of adding more clutter to \dvS we could just have a SRF for
now.


I don't see that as clutter, it's useful information, and keeping it
discoverable is good, not bad.


If we keep adding status reporting commands at some point it's going to 
get unwieldy. Though, if they were in their own schema...



At over 2800 rows currently, you're not going to notice one more
addition to \dfS.


I think it's hard to design a good SRF for this. Because the fields for
different types of progress are different / empty, you can't just
trivially return them as rows.  You'd have to do some EAV like
'command, field_name1, field_value1, ...' type of thing - not
particularly pretty / easy to use.



Oh, I wasn't suggesting a single SRF for everything. Hopefully users 
will eventually figure out a good formula to drive a "progress bar" for 
each type of monitor, which is what you really want anyway (at least 99% 
of the time). If we got there we could have a single view that gave the 
% complete for every command that was providing feedback. If someone 
wanted details they could hit the individual SRF.

--
Jim Nasby, Chief Data Architect, OpenSCG
http://OpenSCG.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] Need a builtin way to run all tests faster manner

2017-03-10 Thread Jim Nasby

On 3/10/17 1:09 PM, Peter Eisentraut wrote:

On 3/10/17 03:27, Jim Nasby wrote:

Perhaps https://travis-ci.org/ or something similar could be used for
this. That avoids any issues about random code.


That doesn't achieve any platform coverage, which is the main point here.


I don't think platform coverage is the first thing to worry about with 
patches, nor with ongoing development.


The biggest win we'd get from something like Travis would be if the 
commitfest monitored for new patch files coming in for monitored threads 
and it created a new branch, applied the patches, and if they applied 
without error commit the branch and push to let Travis do it's thing. We 
wouldn't want that running in the main git repo, but it should be fine 
in a fork that's dedicated to that purpose.


If the travis build failed, commitfest could notify the author.

It could also rebase master into each branch on a daily basis so authors 
would know very quickly if something got committed that broke their patch.


Obviously that doesn't remove the need for manual testing or the 
buildfarm, but it would at least let everyone know that the patch passed 
a smoke test.

--
Jim Nasby, Chief Data Architect, OpenSCG
http://OpenSCG.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] Query fails when SRFs are part of FROM clause (Commit id: 69f4b9c85f)

2017-03-10 Thread Andres Freund
On 2017-03-09 13:34:22 -0500, Robert Haas wrote:
> On Mon, Jan 30, 2017 at 6:54 PM, Tom Lane  wrote:
> > Andres Freund  writes:
> >> Wonder if we there's an argument to be made for implementing this
> >> roughly similarly to split_pathtarget_at_srf - instead of injecting a
> >> ProjectSet node we'd add a FunctionScan node below a Result node.
> >
> > Yeah, possibly.  That would have the advantage of avoiding an ExecProject
> > step when the SRFs aren't buried, which would certainly be the expected
> > case.
> >
> > If you don't want to make ExecInitExpr responsible, then the planner would
> > have to do something like split_pathtarget_at_srf anyway to decompose the
> > expressions, no matter which executor representation we use.
> 
> Did we do anything about this?  Are we going to?

Working on a patch.

- Andres


-- 
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] Need a builtin way to run all tests faster manner

2017-03-10 Thread Andres Freund
On 2017-03-10 01:59:53 -0500, Peter Eisentraut wrote:
> On 3/8/17 16:49, Andres Freund wrote:
> >> make check-world -j2 seems to run fine for me.
> > 
> > Hm, I at least used to get a lot of spurious failures with this. I
> > e.g. don't think the free port selection is race free.
> 
> I was also not sure about that, but as Michael has pointed out, that
> doesn't matter anymore, because it now uses a private socket directory.

Yea, I had forgotten about that bit.


> I have been pounding it a bit, and every so often the test_decoding
> tests fail in mysterious ways, but otherwise it seems to work fine.  I'm
> curious what you are seeing.

I do get regular issues, although the happen to not end up in visible
failures.  All the tests output their regression.diffs into the same
place - which means there'll every now be a failure to remove the file,
and if there were an actual failure it'd possibly end up being
attributed to the wrong test.  So I think we need to relocate that file
to be relative to the sql/ | specs/ | expected/ directories?

Curious about the test decoding thing - hadn't seen that here.

Greetings,

Andres Freund


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


Re: [HACKERS] WIP: Faster Expression Processing v4

2017-03-10 Thread Andres Freund
On 2017-03-10 09:00:14 -0500, Peter Eisentraut wrote:
> I have also looked at the 0002 and 0003 patches, and they seem OK, but
> they are obviously not of much use without 0004.  What is your ambition
> for getting 0004 reviewed and committed for PG10?

I'd really like to get it in.  The performance improvements on its own
are significant, and it provides the basis for significantly larger
improvements again (JIT) - those followup improvements are large patches
again though, so I'd rather not do all of that next cycle.

My next step (over the weekend) is to add tests to execQual.c to get it
a good chunk closer to 100% test coverage, and then do the same for the
new implementation (there's probably very little additional tests needed
after the conversion).  Given all tests pass before/after, and there's a
lot of them, I think we can have a reasonable confidence of a low bug
density.

Large parts of the changes are fairly mechanical, the interesting bits
probably are:

- there's a lot fewer "full blown" node types for expression evaluation
  anymore. In most cases there's now only a top-level ExprState node
  that's nodeTag() able.  The actual information to execute an
  instruction now is in ExprState->steps[i]
- because of the previous point, it now isn't legal anymore to call
  ExecInitExpr() on a list of expressions - ExecInitExprList() is a
  convenience wrapper around manually iterating over the list (gets rid
  of ugly casts, too)
- Because they behave differently on an actual expression evaluation
  basis, quals / check constraint, now have to be initialized with
  ExecInitQual/ExecInitCheck(). That way the shortcut behaviour and such
  can be retained, while also being faster.
- PlanState->targetlist is gone. Because expressions aren't initialized
  on their own anymore, but as part of ExecProject, there's no need for it.
- The seperation between ExecInitExprRec() and
  ExecEvalExpr/ExecInterpExpr() is different - we try to do as much as
  possible during initialization.
- I retained some ugly hackering around
  innermost_caseval|null/domainval|null - I had started down a path of
  removing the use of those in random parts of the system, but I ended
  up not going for it.

Greetings,

Andres Freund


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


Re: [HACKERS] Explicit subtransactions for PL/Tcl

2017-03-10 Thread Victor Wagner
On Thu, 9 Mar 2017 12:04:31 +0100
Pavel Stehule  wrote:


> > Now test demonstrate how errors uncaught on the Tcl level interact
> > with postgresql error system.
> >
> 
> you can catch the exception outside and write own message

OK, here is patch with tests which don't depend on function OIDs

They ignore stack trace ($::errorinfo global variable) completely,
and analyze just error message.


-- 
   Victor Wagner 
diff --git a/doc/src/sgml/pltcl.sgml b/doc/src/sgml/pltcl.sgml
index ad216dd..87bc4ad 100644
--- a/doc/src/sgml/pltcl.sgml
+++ b/doc/src/sgml/pltcl.sgml
@@ -901,6 +901,79 @@ if {[catch { spi_exec $sql_command }]} {
  is a global variable.)
 

+   
+   Explicit Subtransactions
+  
+   Recovering from errors caused by database access as described in
+can lead to an undesirable
+   situation where some operations succeed before one of them fails,
+   and after recovering from that error the data is left in an
+   inconsistent state.  PL/Tcl offers a solution to this problem in
+   the form of explicit subtransactions.
+  
+   
+Consider a function that implements a transfer between two
+accounts:
+
+CREATE FUNCTION transfer_funds() RETURNS void AS $$
+if [catch {
+spi_exec "UPDATE accounts SET balance = balance - 100 WHERE account_name = 'joe'"
+spi_exec "UPDATE accounts SET balance = balance + 100 WHERE account_name = 'mary'"
+} errormsg] {
+set result [format "error transferring funds: %s" $errormsg ]
+} else {
+set result "funds transferred correctly"
+}
+set plan [spi_prepare "INSERT INTO operations (result) VALUES ($1)"]
+spi_execp -count 1 $plan, [list $result)
+$$ LANGUAGE pltclu;
+
+If the second UPDATE statement results in an
+exception being raised, this function will report the error, but
+the result of the first UPDATE will
+nevertheless be committed.  In other words, the funds will be
+withdrawn from Joe's account, but will not be transferred to
+Mary's account.
+   
+   
+To avoid such issues, you can wrap your
+spi_exec calls in an explicit
+subtransaction.  The PL/Tcl provides a
+commmand subtransaction to manage explicit
+	subtransactions.
+ Using explicit subtransactions
+we can rewrite our function as:
+
+CREATE FUNCTION transfer_funds2() RETURNS void AS $$
+if [catch {
+	subtransaction {
+spi_exec "UPDATE accounts SET balance = balance - 100 WHERE account_name = 'joe'"
+spi_exec "UPDATE accounts SET balance = balance + 100 WHERE account_name = 'mary'"
+	}
+	} errormsg] {
+		set result [format "error transferring funds: %s" $errormsg]
+	} else {
+   set result "funds transferred correctly"
+	}
+set plan  [spi_prepare "INSERT INTO operations (result) VALUES ($1)"]
+spi_execp $plan, [list $result]
+$$ LANGUAGE pltclu;
+
+Note that the use of catch is still
+required.  Otherwise the exception would propagate to the top of
+the  stack and would cause the whole function to abort with
+a PostgreSQL error, so that the
+operations table would not have any row
+inserted into it.  The subtransaction command does not
+trap errors, it only assures that all database operations executed
+inside its scope will be atomically committed or rolled back.  A
+rollback of the subtransaction block occurs on any kind of
+exception exit, not only ones caused by errors originating from
+database access.  A regular Tcl exception raised inside an
+explicit subtransaction block would also cause the subtransaction
+to be rolled back.
+   
+  
 

 PL/Tcl Configuration
diff --git a/src/pl/tcl/Makefile b/src/pl/tcl/Makefile
index 1096c4f..b6b6b19 100644
--- a/src/pl/tcl/Makefile
+++ b/src/pl/tcl/Makefile
@@ -28,7 +28,7 @@ DATA = pltcl.control pltcl--1.0.sql pltcl--unpackaged--1.0.sql \
pltclu.control pltclu--1.0.sql pltclu--unpackaged--1.0.sql
 
 REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-extension=pltcl
-REGRESS = pltcl_setup pltcl_queries pltcl_start_proc pltcl_unicode
+REGRESS = pltcl_setup pltcl_queries pltcl_start_proc pltcl_unicode pltcl_subxact
 
 # Tcl on win32 ships with import libraries only for Microsoft Visual C++,
 # which are not compatible with mingw gcc. Therefore we need to build a
diff --git a/src/pl/tcl/expected/pltcl_subxact.out b/src/pl/tcl/expected/pltcl_subxact.out
new file mode 100644
index 000..3ea3ef3
--- /dev/null
+++ b/src/pl/tcl/expected/pltcl_subxact.out
@@ -0,0 +1,146 @@
+--
+-- Test explicit subtransactions
+--
+CREATE TABLE subtransaction_tbl (
+i integer
+);
+-- test subtransaction successfully commited
+CREATE FUNCTION subtransaction_ctx_success() RETURNS text
+AS $$
+	spi_exec "INSERT INTO subtransaction_tbl VALUES(1)"
+subtransaction {
+		spi_exec "INSERT INTO subtransaction_tbl VALUES(2)"
+	}
+$$ LANGUAGE pltcl;
+BEGIN;
+INSERT INTO subtransaction_tbl VALUES(0);
+SELECT subtransaction_ctx_success();

Re: [HACKERS] SQL/JSON in PostgreSQL

2017-03-10 Thread Josh Berkus
On 03/09/2017 10:12 AM, Sven R. Kunze wrote:
> On 08.03.2017 20:52, Magnus Hagander wrote:
>> On Wed, Mar 8, 2017 at 11:48 AM, Peter van Hardenberg > > wrote:
>>
>> Small point of order: YAML is not strictly a super-set of JSON.
>>
>> Editorializing slightly, I have not seen much interest in the
>> world for YAML support though I'd be interested in evidence to the
>> contrary.
>>
>>
>> The world of configuration management seems to for some reason run off
>> YAML, but that's the only places I've seen it recently (ansible,
>> puppet etc).
> 
> SaltStack uses YAML for their tools, too. I personally can empathize
> with them (as a user of configuration management) about this as writing
> JSON would be nightmare with all the quoting, commas, curly braces etc.
> But that's my own preference maybe.
> 
> (Btw. does "run off" mean like or avoid? At least my dictionaries tend
> to the latter.)

Yes, but automated tools can easily convert between JSON and
newline-delimited YAML and back.

-- 
Josh Berkus
Containers & Databases Oh My!


-- 
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] Upgrading postmaster's log messages about bind/listen errors

2017-03-10 Thread Peter Eisentraut
On 3/9/17 23:43, Tom Lane wrote:
> However, if we're measuring this on a scale of usefulness to the average
> DBA, I would argue that it's of more interest than any of these messages
> that currently appear by default:
> 
> 2017-03-09 23:40:12.334 EST [19335] LOG:  MultiXact member wraparound 
> protections are now enabled
> 2017-03-09 23:40:12.335 EST [19339] LOG:  autovacuum launcher started
> 2017-03-09 23:40:12.336 EST [19341] LOG:  logical replication launcher started
> 
> The first of those is surely past its sell-by date.  As for the other two,
> we should log *failure* to start, but not the normal case.

I'm OK with removing these.  No news is good news.

-- 
Peter Eisentraut  http://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] WIP: Faster Expression Processing v4

2017-03-10 Thread Andres Freund
On 2017-03-10 08:51:25 -0500, Peter Eisentraut wrote:
> On 3/7/17 19:14, Andres Freund wrote:
> >> Why shouldn't the function itself also depend on the components of its
> >> return type?
> > Because that'd make it impossible to change the return type components -
> > if the return type is baked into the view that's necessary, but for a
> > "freestanding function" it's not.  If you e.g. have a function that just
> > returns a table's rows, it'd certainly be annoying if that'd prevent you
> > from dropping columns.
> 
> I think functions breaking when the return type is fiddled with is
> actually a not-uncommon problem in practice.  It would be nice if that
> could be addressed.

What problem are you thinking of exactly, and what'd be the solution?
I'd guess that people wouldn't like being unable to change columns in a
table if some function returned the type.

One rather extreme way would be to have functions return a "different"
type, that's initially identical to the table/composite type.  If the
ocmposite type then changes, we'd transparently map between the actual
return type, and the one callers expect.   But that'd a fair bit of
effort, and presumably also quite confusing for users that might
e.g. expect a new column to show up.


> Not necessarily in this patch, but I would like to keep the options
> open.

Yea, seems worth thinking about.  I don't think the patch closes down
options?

Greetings,

Andres Freund


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


Re: [HACKERS] [PATCH] Enabling atomics on ARM64

2017-03-10 Thread Andres Freund
Hi,

On 2017-03-08 19:18:04 -0800, Roman Shaposhnik wrote:
> I'd like to offer a forward port from a change I'm contributing
> the Greenplum code base over here

I think the change makes sense.  Any chance we could convince you to
bring up an arm64 buildfarm animal (our test infrastructure) up, so we
can be sure arm64 stays working?  See https://buildfarm.postgresql.org/

Pushed, thanks for your contribution.

Greetings,

Andres Freund


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


Re: [HACKERS] Adding support for Default partition in partitioning

2017-03-10 Thread Peter Eisentraut
On 3/2/17 21:40, Robert Haas wrote:
> On the point mentioned above, I
> don't think adding a partition should move tuples, necessarily; seems
> like it would be good enough - maybe better - for it to fail if there
> are any that would need to be moved.

ISTM that the uses cases of various combinations of adding a default
partition, adding another partition after it, removing the default
partition, clearing out the default partition in order to add more
nondefault partitions, and so on, need to be more clearly spelled out
for each partitioning type.  We also need to consider that pg_dump and
pg_upgrade need to be able to reproduce all those states.  Seems to be a
bit of work still ...

-- 
Peter Eisentraut  http://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] SQL Standard Feature T211

2017-03-10 Thread Peter Eisentraut
On 3/9/17 18:39, Kevin Grittner wrote:
> With the addition of transition tables we have all four, although
> I'm not sure whether the CREATE TRIGGER statement conforms closely
> enough to claim the feature.  The two basic issues I can think of
> are:
>  - we don't allow the OLD and NEW transition variable names to be
>specified -- using hard-coded values instead
>  - we don't support the standard  formats,
>instead supporting only our EXECUTE PROCEDURE syntax
> 
> Do we leave T211, "Basic trigger capability" on our "unsupported"
> list until those two points are addressed?

Those seem pretty big ones.

-- 
Peter Eisentraut  http://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] Need a builtin way to run all tests faster manner

2017-03-10 Thread Peter Eisentraut
On 3/10/17 03:27, Jim Nasby wrote:
> Perhaps https://travis-ci.org/ or something similar could be used for 
> this. That avoids any issues about random code.

That doesn't achieve any platform coverage, which is the main point here.

-- 
Peter Eisentraut  http://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] Need a builtin way to run all tests faster manner

2017-03-10 Thread Peter Eisentraut
On 3/10/17 13:02, Magnus Hagander wrote:
> for their largest feature development. Could be a good idea to for
> example ave an example yml file somewhere on the wiki that people could
> put into their branches. 

https://github.com/petere/postgresql/blob/travis/.travis.yml

-- 
Peter Eisentraut  http://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] ANALYZE command progress checker

2017-03-10 Thread Andres Freund
Hi,

On 2017-03-10 02:11:18 -0600, Jim Nasby wrote:
> Perhaps instead of adding more clutter to \dvS we could just have a SRF for
> now.

I don't see that as clutter, it's useful information, and keeping it
discoverable is good, not bad.


> At over 2800 rows currently, you're not going to notice one more
> addition to \dfS.

I think it's hard to design a good SRF for this. Because the fields for
different types of progress are different / empty, you can't just
trivially return them as rows.  You'd have to do some EAV like
'command, field_name1, field_value1, ...' type of thing - not
particularly pretty / easy to use.

Greetings,

Andres Freund


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


Re: [HACKERS] logical replication access control patches

2017-03-10 Thread Peter Eisentraut
On 3/3/17 10:07, Stephen Frost wrote:
> Will users really understand that the PUBLISH right actually allows
> complete access to the entire relation, rather than just the ability for
> a user to PUBLISH what they are currently about to SELECT?  It certainly
> doesn't seem intuitive to me, which is why I am concerned that it's
> going to lead to confusion and bug/security reports down the road, or,
> worse, poorly configured systems. 

Well, this is a new system with its own properties, which is why I'm
proposing a new way to manage access.  If it were just the same as the
existing stuff, we wouldn't need this conversation.  I'm interested in
other ideas.

-- 
Peter Eisentraut  http://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] logical replication access control patches

2017-03-10 Thread Peter Eisentraut
On 2/27/17 22:10, Stephen Frost wrote:
> Peter,
> 
> * Peter Eisentraut (peter.eisentr...@2ndquadrant.com) wrote:
>> On 2/18/17 18:06, Stephen Frost wrote:
>>> I'm not convinced that it really makes sense to have PUBLICATION of a
>>> table be independent from the rights an owner of a table has.  We don't
>>> allow other ALTER commands on objects based on GRANT'able rights, in
>>> general, so I'm not really sure that it makes sense to do so here.
>>
>> The REFERENCES and TRIGGER privileges are very similar in principle.
> 
> TRIGGER is a great example of an, ultimately, poorly chosen privilege to
> GRANT away, with a good history of discussion about it:
> 
> https://www.postgresql.org/message-id/21827.1166115978%40sss.pgh.pa.us
> https://www.postgresql.org/message-id/7389.1405554356%40sss.pgh.pa.us

Those discussions about the trigger privileges are valid, but the actual
reason why this is a problem is because our trigger implementation is
broken by default.  In the SQL standard, triggers are executed as the
table owner, so the trigger procedure does not have full account access
to the role that is causing the trigger to fire.  This is an independent
problem that deserves consideration, but it does not invalidate the kind
of privilege that can be granted.

>> Then you couldn't set up a replication structure involving tables owned
>> by different users without resorting to blunt instruments like having
>> everything owned by the same user or using superusers.
> 
> That's not correct- you would simply need a user who is considered an
> owner for all of the objects which you want to replicate, that doesn't
> have to be a *direct* owner or a superuser.
> 
> The tables can have individual roles, with some admin role who is a
> member of those other roles, or another mechanism (as Simon has proposed
> not too long ago...) to have a given role be considered equivilant to an
> owner for all of the relations in a particular database.

I'm not really following what you are saying here.  It seems to me that
you are describing a new kind of facility that gives a role a given
capability with respect to a table.

If so, we already have that, namely privileges.  If not, please elaborate.

-- 
Peter Eisentraut  http://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] Bizarre choice of case for RELKIND_PARTITIONED_TABLE

2017-03-10 Thread Tom Lane
I wrote:
> Robert Haas  writes:
>> On Tue, Mar 7, 2017 at 12:55 PM, Tom Lane  wrote:
>>> Is there a good reason why RELKIND_PARTITIONED_TABLE is 'P' not 'p'?

>> I can't muster a lot of outrage about this one way or another.  One
>> possible advantage of 'P' is that there are fewer places where 'P' is
>> mentioned in the source code than 'p'.

> Hm, one would hope that the vast majority of code references are neither
> of those, but rather "RELKIND_PARTITIONED_TABLE".  information_schema.sql
> and system_views.sql will need to be gone over carefully, certainly, but
> we shouldn't be hard-coding this anywhere that there's a reasonable
> alternative.

Pushed.  I was a bit disappointed to find that make check-world passed
just fine without having updated either information_schema.sql or
system_views.sql.  Evidently our test coverage for these views leaves
something to be desired.

regards, tom lane


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


Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2017-03-10 Thread Alvaro Herrera
Robert Haas wrote:
> On Wed, Mar 8, 2017 at 2:30 PM, Alvaro Herrera  
> wrote:
> > Not really -- it's a bit slower actually in a synthetic case measuring
> > exactly the slowed-down case.  See
> > https://www.postgresql.org/message-id/cad__ougk12zqmwwjzim-yyud1y8jmmy6x9yectnif3rpp6h...@mail.gmail.com
> > I bet in normal cases it's unnoticeable.  If WARM flies, then it's going
> > to provide a larger improvement than is lost to this.
> 
> Hmm, that test case isn't all that synthetic.  It's just a single
> column bulk update, which isn't anything all that crazy,

The problem is that the update touches the second indexed column.  With
the original code we would have stopped checking at that point, but with
the patched code we continue to verify all the other indexed columns for
changes.

Maybe we need more than one bitmapset to be given -- multiple ones for
for "any of these" checks (such as HOT, KEY and Identity) which can be
stopped as soon as one is found, and one for "all of these" (for WARM,
indirect indexes) which needs to be checked to completion.

-- 
Álvaro Herrerahttps://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] Need a builtin way to run all tests faster manner

2017-03-10 Thread Magnus Hagander
On Fri, Mar 10, 2017 at 11:29 AM, Dagfinn Ilmari Mannsåker <
ilm...@ilmari.org> wrote:

> Magnus Hagander  writes:
>
> > AFAIK travis-ci would require us to use github as our hoster for all
> those
> > things, and embrace that workflow, they don't support anything else.
> >
> > There might be others that do, just not travis.
>
> It merely requires the repository to exist on GitHub, and postgresql.git
> is already mirrored to https://github.com/postgres/postgres.  If there
> was a .travis.yml in the repo, people who fork it could easily enable
> Travis-CI for their fork, even the official repo isn't hooked up.
>

That's true. It would require a bunch of additional branches to make it
useful in core though, but it śeems like it could be a useful thing for
people to enable in their own personal forks/branches if they use github
for their largest feature development. Could be a good idea to for example
ave an example yml file somewhere on the wiki that people could put into
their branches.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


Re: [HACKERS] SQL/JSON in PostgreSQL

2017-03-10 Thread Magnus Hagander
On Thu, Mar 9, 2017 at 1:12 PM, Sven R. Kunze  wrote:

> On 08.03.2017 20:52, Magnus Hagander wrote:
>
> On Wed, Mar 8, 2017 at 11:48 AM, Peter van Hardenberg  wrote:
>
>> Small point of order: YAML is not strictly a super-set of JSON.
>>
>> Editorializing slightly, I have not seen much interest in the world for
>> YAML support though I'd be interested in evidence to the contrary.
>>
>>
> The world of configuration management seems to for some reason run off
> YAML, but that's the only places I've seen it recently (ansible, puppet
> etc).
>
>
> SaltStack uses YAML for their tools, too. I personally can empathize with
> them (as a user of configuration management) about this as writing JSON
> would be nightmare with all the quoting, commas, curly braces etc. But
> that's my own preference maybe.
>
> (Btw. does "run off" mean like or avoid? At least my dictionaries tend to
> the latter.)
>

In this case, it means like. "run off" as in "the car runs off fuel" or
something like that. Probably a bad choice of words.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


[HACKERS] scram and \password

2017-03-10 Thread Jeff Janes
Should the \password tool in psql inspect password_encryption and act on it
being 'scram'?

I didn't see this issue discussed, but the ability to search the archives
for backslashes is rather limited.

Cheers,

Jeff


Re: [HACKERS] GSOC Introduction / Eliminate O(N^2) scaling from rw-conflict tracking in serializable transactions

2017-03-10 Thread George Papadrosou
Hi all and thank you for your quick replies.

> [two people interested in the same GSoC project]

Mr. Grittner thank you for sharing this ahead of time.


Liu(is this your first name?),

> I have been concentrating on it for a long time, reading papers, reading 
> source codes, and discussing details with Mr Grittner.  

I understand your efforts and I am willing to back down. This is not the only 
project that appeals to me :)


Mr. Frost, Mr. Munro,  thank you for your suggestions. I am now between the 
TOAST’ing slices and the predicate locking project. I am keen on the fact the 
“toasting” project is related to on-disk data structures so I will probably 
send you an email about that later today.

In general, I would like to undertake a project interesting enough and 
important for Postgres. Also, I could take into account if you favor one over 
another, so please let me know. I understand that these projects should be 
strictly defined to fit in the GSOC period, however the potential for future 
improvements or challenges is what drives and motivates me.

Thank you!
George




-- 
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] SQL/JSON in PostgreSQL

2017-03-10 Thread Sven R. Kunze

On 10.03.2017 05:07, Petr Jelinek wrote:

The original complain was about JSON_VALUE extracting date but I don't
understand why there is problem with that, the SQL/JSON defines that
behavior. The RETURNING clause there is more or less just shorthand for
casting with some advanced options.


Thanks for clarifying. I mistook it as if JSON_VALUE itself returns a 
date value.


Sven


--
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] GSOC Introduction / Eliminate O(N^2) scaling from rw-conflict tracking in serializable transactions

2017-03-10 Thread Stephen Frost
Kevin,

* Kevin Grittner (kgri...@gmail.com) wrote:
> > [two people interested in the same GSoC project]
> 
> It's an interesting problem to have.
> 
> If neither of you chooses to voluntarily back down, the obvious
> resolution is for the mentors to vote on the better proposal.  If we
> do that early enough during the student application period, there
> might still be time for the person whose proposal wasn't chosen to
> submit a proposal for an alternative project.  As I see it, that
> means:
> 
>  - I would tend to favor a proposal submitted on the first day
>(beginning March 20 16:00 UTC), if only one is.
> 
>  - I would push for very early voting by the PostgreSQL mentors.

Agreed.  Many thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Parallel Append implementation

2017-03-10 Thread Robert Haas
On Fri, Mar 10, 2017 at 12:17 AM, Amit Khandekar  wrote:
> I agree that the two-lists approach will consume less memory than
> bitmapset. Keeping two lists will effectively have an extra pointer
> field which will add up to the AppendPath size, but this size will not
> grow with the number of subpaths, whereas the Bitmapset will grow.

Sure.  You'll use about one BIT of memory per subpath.  I'm kind of
baffled as to why we're treating this as an issue worth serious
discussion; the amount of memory involved is clearly very small.  Even
for an appendrel with 1000 children, that's 125 bytes of memory.
Considering the amount of memory we're going to spend planning that
appendrel overall, that's not significant.

However, Ashutosh's response made me think of something: one thing is
that we probably do want to group all of the non-partial plans at the
beginning of the Append so that they get workers first, and put the
partial plans afterward.  That's because the partial plans can always
be accelerated by adding more workers as they become available, but
the non-partial plans are just going to take as long as they take - so
we want to start them as soon as possible.  In fact, what we might
want to do is actually sort the non-partial paths in order of
decreasing cost, putting the most expensive one first and the others
in decreasing order after that - and then similarly afterward with the
partial paths.  If we did that, we wouldn't need to store a bitmapset
OR two separate lists.  We could just store the index of the first
partial plan in the list.  Then you can test whether a path is partial
by checking whether this_index >= first_partial_index.

One problem with that is that, since the leader has about a 4ms head
start on the other workers, it would tend to pick the most expensive
path to run locally before any other worker had a chance to make a
selection, and that's probably not what we want.  To fix that, let's
have the leader start at the end of the list of plans and work
backwards towards the beginning, so that it prefers cheaper and
partial plans over decisions that would force it to undertake a large
amount of work itself.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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


Re: [HACKERS] Upgrading postmaster's log messages about bind/listen errors

2017-03-10 Thread Tom Lane
Stephen Frost  writes:
> * Tom Lane (t...@sss.pgh.pa.us) wrote:
>> ... So I think the logging setup I had in
>> my patch is pretty much the only sane way to do it, and we just have
>> to decide whether it's worth exposing at default log level or not.

> I definitely think we should include it at the default log level.  We
> certainly wouldn't be the first daemon process to do so (bind9 comes to
> mind, but I notice ntpd, nrpe, and strongswan do also, and probably some
> others).

I'm leaning in that direction as well now.  I think we could address
Robert's concern about startup chattiness by downgrading the other
mentioned messages to DEBUG1.  I will check, but I'm pretty sure that
there is already adequate logging for subprocess startup failure ---
and if there is not, that would be a bug in itself.

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] GSOC Introduction / Eliminate O(N^2) scaling from rw-conflict tracking in serializable transactions

2017-03-10 Thread Kevin Grittner
> [two people interested in the same GSoC project]

It's an interesting problem to have.

If neither of you chooses to voluntarily back down, the obvious
resolution is for the mentors to vote on the better proposal.  If we
do that early enough during the student application period, there
might still be time for the person whose proposal wasn't chosen to
submit a proposal for an alternative project.  As I see it, that
means:

 - I would tend to favor a proposal submitted on the first day
   (beginning March 20 16:00 UTC), if only one is.

 - I would push for very early voting by the PostgreSQL mentors.

--
Kevin Grittner


-- 
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] Need a builtin way to run all tests faster manner

2017-03-10 Thread Dagfinn Ilmari Mannsåker
Magnus Hagander  writes:

> AFAIK travis-ci would require us to use github as our hoster for all those
> things, and embrace that workflow, they don't support anything else.
>
> There might be others that do, just not travis.

It merely requires the repository to exist on GitHub, and postgresql.git
is already mirrored to https://github.com/postgres/postgres.  If there
was a .travis.yml in the repo, people who fork it could easily enable
Travis-CI for their fork, even the official repo isn't hooked up.

- ilmari

-- 
- Twitter seems more influential [than blogs] in the 'gets reported in
  the mainstream press' sense at least.   - Matt McLeod
- That'd be because the content of a tweet is easier to condense down
  to a mainstream media article.  - Calle Dybedahl


-- 
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] GSOC Introduction / Eliminate O(N^2) scaling from rw-conflict tracking in serializable transactions

2017-03-10 Thread Mengxing Liu
Hi George, 

I am Mengxing Liu. Happy to meet someone with the same idea : )

I have been concentrating on it for a long time, reading papers, reading source 
codes, and discussing details with Mr Grittner.  So I really understand your 
passion on it. But definitely I don't want all these effects to be in vain. So, 
maybe a little ruthless, would you mind to consider transferring to the other 
one?


> -原始邮件-
> 发件人: "Kevin Grittner" 
> 发送时间: 2017-03-10 22:57:03 (星期五)
> 收件人: "George Papadrosou" , "刘梦醒" 
> 
> 抄送: "pgsql-hackers@postgresql.org" 
> 主题: Re: [HACKERS] GSOC Introduction / Eliminate O(N^2) scaling from 
> rw-conflict tracking in serializable transactions
> 
> [including Mengxing Liu in response, for reasons that should become
> obvious below...]
> 
> Hi George,
> 
> On Thu, Mar 9, 2017 at 6:49 PM, George Papadrosou  
> wrote:
> 
> > my name is George Papadrosou, this is my first semester as
> > graduate student at Georgia Tech and would like to submit a
> > proposal to Google Summer of Code, for the project "Eliminate
> > O(N^2) scaling from rw-conflict tracking in serializable
> > transactions”.
> 
> I was recently contacted off-list by Mengxing Liu, who has been
> looking at the same project, and said he was planning to submit a
> GSoC proposal.  Rather than have one of you sit this out, do either
> of you feel comfortable taking a different project instead?  Since
> you've both been looking at the serializable code and supporting
> documents, perhaps one of you could change to the other suggested
> Serializable project?
> 
> > I am going to prepare a draft proposal for this project and share
> > it with you soon. The project’s description is pretty clear, do
> > you think it should be more strictly defined in the proposal?
> 
> At a minimum, the proposal should include list of milestones you
> expect to reach along the way, and a timeline indicating when you
> expect to reach them.  Some description of the benchmarks you intend
> to run would be also be very good.
> 
> > Until then, I would like to familiarize myself a bit with the
> > codebase and fix some bug/todo. I didn’t find many [E] marked
> > tasks in the todo list so the task I was thinking is "\s without
> > arguments (display history) fails with libedit, doesn't use pager
> > either - psql \s not working on OSX”. However, it works on my OSX
> > El Capitan laptop with Postgres 9.4.4. Would you suggest some
> > other starter task?
> 
> There is a CommitFest in progress; reviewing patches is a good way
> to become involved and familiar with the community processes.
> 
> https://wiki.postgresql.org/wiki/CommitFest
> 
> --
> Kevin Grittner


--
Mengxing Liu










-- 
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] Upgrading postmaster's log messages about bind/listen errors

2017-03-10 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote:
> Stephen Frost  writes:
> > database system is ready to accept connections on (1.2.3.4)
> 
> That would be a problem from a couple of directions.  First, it wouldn't
> be unusual for there to be half a dozen addresses to list, not just one.
> Even a default configuration would probably read like
> 
> database system is ready to accept connections on (127.0.0.1, ::1, 
> /tmp/.s.PGSQL.5432)

Yeah, that's probably a bit much to have all on one line.

> which doesn't seem very appetizing to me.  Second, it would be
> considerably messier to implement because the "ready to accept
> connections" message comes out physically distant from the
> StreamServerPort function, and we don't save the struct addrinfo list
> past the end of that function.  So I think the logging setup I had in
> my patch is pretty much the only sane way to do it, and we just have
> to decide whether it's worth exposing at default log level or not.

I definitely think we should include it at the default log level.  We
certainly wouldn't be the first daemon process to do so (bind9 comes to
mind, but I notice ntpd, nrpe, and strongswan do also, and probably some
others).

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Upgrading postmaster's log messages about bind/listen errors

2017-03-10 Thread Euler Taveira
2017-03-10 1:43 GMT-03:00 Tom Lane :

> Yeah, my thought was that if we've gotten along without this for 20 years,
> it's probably not of interest to most people most of the time.
>
> +1 for DEBUG1.


> 2017-03-09 23:40:12.334 EST [19335] LOG:  MultiXact member wraparound
> protections are now enabled
>
It should be DEBUG1 as soon as 9.3 is deprecated.


> 2017-03-09 23:40:12.335 EST [19339] LOG:  autovacuum launcher started
> 2017-03-09 23:40:12.336 EST [19341] LOG:  logical replication launcher
> started
>
> +1 for DEBUG1.


-- 
   Euler Taveira   Timbira -
http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento



Re: [HACKERS] [PATCH] few fts functions for jsonb

2017-03-10 Thread Dmitry Dolgov
> On 28 February 2017 at 19:21, Oleg Bartunov  wrote:
> 1. add json support

I've added json support for all functions.

>  Its_headline  should returns the original json with highlighting

Yes, I see now. I don't think it's worth it to add a special option for that
purpose, so I've just changed the implementation to return the original
json(b).
diff --git a/src/backend/tsearch/to_tsany.c b/src/backend/tsearch/to_tsany.c
index 6e5de8f..8f7bcfe 100644
--- a/src/backend/tsearch/to_tsany.c
+++ b/src/backend/tsearch/to_tsany.c
@@ -16,6 +16,7 @@
 #include "tsearch/ts_cache.h"
 #include "tsearch/ts_utils.h"
 #include "utils/builtins.h"
+#include "utils/jsonb.h"
 
 
 typedef struct MorphOpaque
@@ -24,6 +25,14 @@ typedef struct MorphOpaque
 	int			qoperator;		/* query operator */
 } MorphOpaque;
 
+typedef struct TSVectorBuildState
+{
+	ParsedText	*prs;
+	TSVector	result;
+	Oid			cfgId;
+} TSVectorBuildState;
+
+static void add_to_tsvector(void *state, char *elem_value, int elem_len);
 
 Datum
 get_current_ts_config(PG_FUNCTION_ARGS)
@@ -256,6 +265,109 @@ to_tsvector(PG_FUNCTION_ARGS)
 		PointerGetDatum(in)));
 }
 
+Datum
+jsonb_to_tsvector(PG_FUNCTION_ARGS)
+{
+	Jsonb*jb = PG_GETARG_JSONB(0);
+	TSVectorBuildState	state;
+	ParsedText			*prs = (ParsedText *) palloc(sizeof(ParsedText));
+
+	prs->words = NULL;
+	state.result = NULL;
+	state.cfgId = getTSCurrentConfig(true);
+	state.prs = prs;
+
+	iterate_jsonb_values(jb, , (JsonIterateAction) add_to_tsvector);
+
+	PG_FREE_IF_COPY(jb, 1);
+
+	if (state.result == NULL)
+	{
+		/* There weren't any string elements in jsonb,
+		 * so wee need to return an empty vector */
+
+		if (prs->words != NULL)
+			pfree(prs->words);
+
+		state.result = palloc(CALCDATASIZE(0, 0));
+		SET_VARSIZE(state.result, CALCDATASIZE(0, 0));
+		state.result->size = 0;
+	}
+
+	PG_RETURN_TSVECTOR(state.result);
+}
+
+Datum
+json_to_tsvector(PG_FUNCTION_ARGS)
+{
+	text*json = PG_GETARG_TEXT_P(0);
+	TSVectorBuildState	state;
+	ParsedText			*prs = (ParsedText *) palloc(sizeof(ParsedText));
+
+	prs->words = NULL;
+	state.result = NULL;
+	state.cfgId = getTSCurrentConfig(true);
+	state.prs = prs;
+
+	iterate_json_values(json, , (JsonIterateAction) add_to_tsvector);
+
+	PG_FREE_IF_COPY(json, 1);
+	if (state.result == NULL)
+	{
+		/* There weren't any string elements in json,
+		 * so wee need to return an empty vector */
+
+		if (prs->words != NULL)
+			pfree(prs->words);
+
+		state.result = palloc(CALCDATASIZE(0, 0));
+		SET_VARSIZE(state.result, CALCDATASIZE(0, 0));
+		state.result->size = 0;
+	}
+
+	PG_RETURN_TSVECTOR(state.result);
+}
+
+/*
+ * Extend current TSVector from _state with a new one,
+ * build over a json(b) element.
+ */
+static void
+add_to_tsvector(void *_state, char *elem_value, int elem_len)
+{
+	TSVectorBuildState *state = (TSVectorBuildState *) _state;
+	ParsedText	*prs = state->prs;
+	TSVector	item_vector;
+	int			i;
+
+	prs->lenwords = elem_len / 6;
+	if (prs->lenwords == 0)
+		prs->lenwords = 2;
+
+	prs->words = (ParsedWord *) palloc(sizeof(ParsedWord) * prs->lenwords);
+	prs->curwords = 0;
+	prs->pos = 0;
+
+	parsetext(state->cfgId, prs, elem_value, elem_len);
+
+	if (prs->curwords)
+	{
+		if (state->result != NULL)
+		{
+			for (i = 0; i < prs->curwords; i++)
+prs->words[i].pos.pos = prs->words[i].pos.pos + TS_JUMP;
+
+			item_vector = make_tsvector(prs);
+
+			state->result = (TSVector) DirectFunctionCall2(tsvector_concat,
+	TSVectorGetDatum(state->result),
+	PointerGetDatum(item_vector));
+		}
+		else
+			state->result = make_tsvector(prs);
+	}
+}
+
 /*
  * to_tsquery
  */
diff --git a/src/backend/tsearch/wparser.c b/src/backend/tsearch/wparser.c
index 8ca1c62..b648996 100644
--- a/src/backend/tsearch/wparser.c
+++ b/src/backend/tsearch/wparser.c
@@ -21,6 +21,7 @@
 #include "tsearch/ts_utils.h"
 #include "utils/builtins.h"
 #include "utils/varlena.h"
+#include "utils/jsonb.h"
 
 
 /**sql-level interface**/
@@ -31,6 +32,19 @@ typedef struct
 	LexDescr   *list;
 } TSTokenTypeStorage;
 
+/* state for ts_headline_json_* */
+typedef struct HeadlineJsonState
+{
+	HeadlineParsedText *prs;
+	TSConfigCacheEntry *cfg;
+	TSParserCacheEntry *prsobj;
+	TSQueryquery;
+	List*prsoptions;
+	booltransformed;
+} HeadlineJsonState;
+
+static text * headline_json_value(void *_state, char *elem_value, int elem_len);
+
 static void
 tt_setup_firstcall(FuncCallContext *funcctx, Oid prsid)
 {
@@ -362,3 +376,177 @@ ts_headline_opt(PG_FUNCTION_ARGS)
 		PG_GETARG_DATUM(1),
 		PG_GETARG_DATUM(2)));
 }
+
+Datum
+ts_headline_jsonb_byid_opt(PG_FUNCTION_ARGS)
+{
+	Jsonb			*out, *jb = PG_GETARG_JSONB(1);
+	TSQuery			query = PG_GETARG_TSQUERY(2);
+	text			*opt = (PG_NARGS() > 3 && PG_GETARG_POINTER(3)) ? PG_GETARG_TEXT_P(3) : NULL;
+
+	HeadlineParsedText prs;
+	HeadlineJsonState *state = palloc0(sizeof(HeadlineJsonState));
+
+	memset(, 0, sizeof(HeadlineParsedText));
+	prs.lenwords = 32;
+	prs.words = 

Re: [HACKERS] Need a builtin way to run all tests faster manner

2017-03-10 Thread Magnus Hagander
On Fri, Mar 10, 2017 at 3:27 AM, Jim Nasby  wrote:

> On 3/7/17 9:52 PM, Magnus Hagander wrote:
>
>> There have also on and off been discussions about building arbitrary
>> patches as they are sent to the mailinglists. Doing that without any
>> committer (or other trusted party) as a filter is a completely different
>> challenge of course, given that it basically amounts to downloading and
>> running random code off the internet.
>>
>
> Perhaps https://travis-ci.org/ or something similar could be used for
> this. That avoids any issues about random code.


AFAIK travis-ci would require us to use github as our hoster for all those
things, and embrace that workflow, they don't support anything else.

There might be others that do, just not travis.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


Re: [HACKERS] Upgrading postmaster's log messages about bind/listen errors

2017-03-10 Thread Tom Lane
Stephen Frost  writes:
> * Tels (nospam-pg-ab...@bloodgate.com) wrote:
>> I'd argue that from a security standpoint it is important to log at
>> startup what addresses the service binds to, just so it is visible,
>> explicit and logged.

> It's also terribly useful for realizing there's an issue.

Good points both.

> Perhaps we could compromise by simply including the bind information in
> the 'ready to accept connections' message, like so:

> database system is ready to accept connections on (1.2.3.4)

That would be a problem from a couple of directions.  First, it wouldn't
be unusual for there to be half a dozen addresses to list, not just one.
Even a default configuration would probably read like

database system is ready to accept connections on (127.0.0.1, ::1, 
/tmp/.s.PGSQL.5432)

which doesn't seem very appetizing to me.  Second, it would be
considerably messier to implement because the "ready to accept
connections" message comes out physically distant from the
StreamServerPort function, and we don't save the struct addrinfo list
past the end of that function.  So I think the logging setup I had in
my patch is pretty much the only sane way to do it, and we just have
to decide whether it's worth exposing at default log level or not.

regards, tom lane


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


Re: [HACKERS] rename pg_log directory?

2017-03-10 Thread Bruce Momjian
On Fri, Mar 10, 2017 at 11:23:54AM +0100, Andreas Karlsson wrote:
> On 03/09/2017 11:25 PM, Bruce Momjian wrote:
> >"data" and "base" where chosen because it is a "data-base", but with the
> >pg_ prefixes it would be a pg_data_pg_base.  ;-)
> 
> Haha, I had not spotted that one despite always naming my data directory
> "data" while developing. Fun little tidbit there.

Yeah, we can thank the creative Berkeley students for that one, I think.

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

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +


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


Re: [HACKERS] Re: proposal - psql: possibility to specify sort for describe commands, when size is printed

2017-03-10 Thread Pavel Stehule
2017-03-10 16:05 GMT+01:00 Peter Eisentraut <
peter.eisentr...@2ndquadrant.com>:

> On 3/10/17 09:57, Pavel Stehule wrote:
> > PREFERRED_SORT_COLUMNS
> > and PREFERRED_SORT_DIRECTION ?
>
> I think the name "preferred" implies that it will be ignored if it's not
> found or something like that, but I don't think that's what you are
> implementing.
>

ok if it will be used only for verbose describe commands , then the name
EXTENDED_DESCRIBE_SORT_COLUMNS, and EXTENDED_DESCRIBE_SORT_DIRECTION.

Pavel


>
> --
> Peter Eisentraut  http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>


Re: [HACKERS] Re: proposal - psql: possibility to specify sort for describe commands, when size is printed

2017-03-10 Thread Pavel Stehule
2017-03-10 16:00 GMT+01:00 Alexander Korotkov :

> On Fri, Mar 10, 2017 at 5:16 PM, Stephen Frost  wrote:
>
>> * Peter Eisentraut (peter.eisentr...@2ndquadrant.com) wrote:
>> > On 2/24/17 16:32, Pavel Stehule wrote:
>> > > set EXTENDED_DESCRIBE_SORT size_desc
>> > > \dt+
>> > > \l+
>> > > \di+
>> > >
>> > > Possible variants: schema_table, table_schema, size_desc, size_asc
>> >
>> > I can see this being useful, but I think it needs to be organized a
>> > little better.
>> >
>> > Sort key and sort direction should be separate settings.
>> >
>> > I'm not sure why we need to have separate settings to sort by schema
>> > name and table name.  But if we do, then we should support that for all
>> > object types.  I think maybe that's something we shouldn't get into
>> > right now.
>> >
>> > So I would have one setting for sort key = {name|size} and on for sort
>> > direction = {asc|desc}.
>>
>> Perhaps I'm trying to be overly cute here, but why not let the user
>> simply provide a bit of SQL to be put at the end of the query?
>>
>> That is, something like:
>>
>> \pset EXTENDED_DESCRIBE_ORDER_LIMIT 'ORDER BY 5 DESC LIMIT 10'
>>
>
> I think that's the question of usability.  After all, one can manually
> type corresponding SQL instead of \d* commands.  However, it's quite
> cumbersome to do this every time.
> I found quite useful to being able to switch between different sortings
> quickly.  For instance, after seeing tables sorted by name, user can
> require them sorted by size descending, then sorted by size ascending,
> etc...
> Therefore, I find user-defined SQL clause to be cumbersome.  Even psql
> variable itself seems to be cumbersome for me.
> I would propose to add sorting as second optional argument to \d*
> commands.  Any thoughts?
>

This proposal was here already - maybe two years ago. The psql command
parser doesn't allow any complex syntax - more - the more parameters in one
psql commands is hard to remember, hard to read.

With my proposal, and patch I would to cover following use case. It is real
case. Anytime when we used \dt+ in psql we needed sort by size desc. When
we should to see a size, then the top is interesting. This case is not
absolute, but very often, so I would to create some simple way, how to do
some parametrization (really simple).

Pavel



>
> --
> Alexander Korotkov
> Postgres Professional: http://www.postgrespro.com
> The Russian Postgres Company
>


Re: [HACKERS] Should buffer of initialization fork have a BM_PERMANENT flag

2017-03-10 Thread Artur Zakirov

On 10.03.2017 04:00, Michael Paquier wrote:

On Thu, Mar 9, 2017 at 10:25 PM, Artur Zakirov  wrote:

I think this is good fixes. I've checked them. And in my opinion they are
correct.

The code also is good.


Having something with conflicts is not nice, so attached is a rebased version.


Thank you!

I've rerun regression and TAP tests. They all passed.

Also maybe it will be good to fix comments.

In buf_internals.h:

#define BM_PERMANENT(1U << 31)/* permanent relation (not
   * unlogged) */


And in FlushBuffer():

/*
 * Force XLOG flush up to buffer's LSN.  This implements the basic WAL
 * rule that log updates must hit disk before any of the data-file changes
 * they describe do.
 *
 * However, this rule does not apply to unlogged relations, which will be
 * lost after a crash anyway.  Most unlogged relation pages do not bear


Because BM_PERMANENT is used for init forks of unlogged indexes now.

--
Artur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres 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] Re: proposal - psql: possibility to specify sort for describe commands, when size is printed

2017-03-10 Thread Peter Eisentraut
On 3/10/17 09:57, Pavel Stehule wrote:
> PREFERRED_SORT_COLUMNS
> and PREFERRED_SORT_DIRECTION ? 

I think the name "preferred" implies that it will be ignored if it's not
found or something like that, but I don't think that's what you are
implementing.

-- 
Peter Eisentraut  http://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] some dblink refactoring

2017-03-10 Thread Peter Eisentraut
On 3/8/17 00:10, Tsunakawa, Takayuki wrote:
> I changed the status to ready for committer.  The patch applied cleanly, 
> passed the regression test (make installcheck in contrib/dblink/), and the 
> code looks perfect.
> 
> How about applying the attached small patch for another refactoring?  This 
> merely changes makeStringInfo() to initStringInfo() at two sites just other 
> places in the same file.  makeStringInfo() on the function local variables 
> leaves memory for StringInfoData allocated unnecessarily (which may be 
> automatically reclaimed some time after.)

Committed both.  Thanks.

-- 
Peter Eisentraut  http://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] Re: proposal - psql: possibility to specify sort for describe commands, when size is printed

2017-03-10 Thread Pavel Stehule
2017-03-10 15:16 GMT+01:00 Stephen Frost :

> * Peter Eisentraut (peter.eisentr...@2ndquadrant.com) wrote:
> > On 2/24/17 16:32, Pavel Stehule wrote:
> > > set EXTENDED_DESCRIBE_SORT size_desc
> > > \dt+
> > > \l+
> > > \di+
> > >
> > > Possible variants: schema_table, table_schema, size_desc, size_asc
> >
> > I can see this being useful, but I think it needs to be organized a
> > little better.
> >
> > Sort key and sort direction should be separate settings.
> >
> > I'm not sure why we need to have separate settings to sort by schema
> > name and table name.  But if we do, then we should support that for all
> > object types.  I think maybe that's something we shouldn't get into
> > right now.
> >
> > So I would have one setting for sort key = {name|size} and on for sort
> > direction = {asc|desc}.
>
> Perhaps I'm trying to be overly cute here, but why not let the user
> simply provide a bit of SQL to be put at the end of the query?
>
> That is, something like:
>
> \pset EXTENDED_DESCRIBE_ORDER_LIMIT 'ORDER BY 5 DESC LIMIT 10'
>

For example - the size is displayed in pretty form - raw form is not
displayed - so simple ORDER BY clause is not possible.

But setting LIMIT is not bad idea - although it is probably much more
complex for implementation.

\pset DESCRIBE_LIMIT  100
\pset EXTENDED_DESCRIBE_LIMIT 100

can be implemented as next step

Regards

Pavel



>
> Thanks!
>
> Stephen
>


  1   2   >