Re: [HACKERS] Releasing in September

2016-01-20 Thread Peter Geoghegan
On Wed, Jan 20, 2016 at 8:32 PM, Michael Paquier
 wrote:
> Perhaps some people are more interested in implementing new
> features than working on bugs and would just continue hacking and
> arguing about new features, at least a stability period may attract
> more committer attention into actual bug fixes, in short: no new
> features can be committed until the previous versions has reached at
> least beta2, rc, whatever. This may accelerate the stability process.

Or it might just accelerate the amount of time it takes to *declare*
having reached that milestone. Besides, do you really think that
people need to be able to commit something before seriously working on
it?

Rules that are expressly coercive are a *bad* idea.

-- 
Peter Geoghegan


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


Re: [HACKERS] Releasing in September

2016-01-20 Thread Peter Geoghegan
On Wed, Jan 20, 2016 at 9:37 PM, Michael Paquier
 wrote:
> We have automatic buildfarm coverage on many platforms. Perhaps we
> could live without that with a buildfarm module though.

It's not clear that buildfarm coverage makes sense for sqlsmith. If it
does make sense, I see no reason why introducing it should be attached
to sqlsmith's inclusion in core.

-- 
Peter Geoghegan


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


Re: [HACKERS] postgres_fdw join pushdown (was Re: Custom/Foreign-Join-APIs)

2016-01-20 Thread Ashutosh Bapat
On Thu, Jan 21, 2016 at 2:07 AM, Robert Haas  wrote:

>
> Well, we kind of need to get it right, not just be guessing.
>
> It looks to me as though GetConnection() only uses the user ID as a
> cache lookup key.  What it's trying to do is ensure that if user A and
> user B need different user mapping options, we don't use the same
> connection for both.  So, actually, passing InvalidOid seems like it's
> not really a problem here.  It's arguably more correct than what we've
> been doing up until now, since it means we cache the connection under
> user OID whose options we used, rather than the user OID that asked
> about the options.
>
>
In that case, do we need GetUserMappingById changes in that patch and not
pull it out. If we are keeping those changes, I need some clarification
about your comment

Even if that were no issue, it doesn't seem to add anything.  The only
> caller of the new function is  postgresBeginForeignScan(), and that
> function already has a way of getting the user mapping.  The new way
> doesn't seem to be better or faster, so why bother changing it?
>

In pg_fdw_join_v2.patch, postgresBeginForeignScan() obtained user mapping
using GetUserMappingById() instead of the earlier way of fetching it by
userid and serverid. So even that change will remain, right?

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company


Re: [HACKERS] Combining Aggregates

2016-01-20 Thread David Rowley
On 21 January 2016 at 08:06, Robert Haas  wrote:

> On Wed, Jan 20, 2016 at 7:38 AM, David Rowley
>  wrote:
> > Agreed. So I've attached a version of the patch which does not have any
> of
> > the serialise/deserialise stuff in it.
>
> I re-reviewed this and have committed most of it with only minor
> kibitizing.  A few notes:
>

Great! Thank you for committing.

- I changed the EXPLAIN code, since it failed to display the aggregate
> node's mode of operation in non-text format.
>

Oops, thanks for fixing.


> - I removed some of the planner support, since I'm not sure that it's
> completely correct and I'm very sure that it contains more code
> duplication than I'm comfortable with (set_combineagg_references in
> particular).  Please feel free to resubmit this part, perhaps in
> combination with code that actually uses it for something.  But please
> also think about whether there's a way to reduce the duplication, and
> maybe consider adding some comments, too.
>

That part is a gray area. Wasn't that sure it belonged to this patch
either. The problem is, that without it an Aggref's return type is wrong
when the node is in combine mode, which might not be a problem now as we
don't have a planner yet that generates these nodes.


> - I'm not sure that you made the right call regarding reusing
> transfn_oid and transfn for combine functions, vs. adding separate
> fields.  It is sort of logical and has the advantage of keeping the
> data structure smaller, but it might also be thought confusing or
> inappropriate on other grounds.  But I think we can change it later if
> that comes to seem like the right thing to do, so I've left it the way
> you had it for now.


Sharing the variable, as horrid as that might seem does simplify some code.
For example if you look at find_compatible_pertrans(), then you see:

if (aggtransfn != pertrans->transfn_oid ||
aggtranstype != pertrans->aggtranstype)
continue;

This would need to be changed to something like:

if (!aggstate->combineStates &&
(aggtransfn != pertrans->transfn_oid ||
aggtranstype != pertrans->aggtranstype))
continue;
else if (aggstate->combineStates &&
(aggcombinefn != pertrans->combinefn_oid ||
aggtranstype != pertrans->aggtranstype))
continue;

And we'd then have to pass aggcombinefn to that function too.

What might be better would be to rename the variable to something name that
screams something a bit more generic.

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


Re: [HACKERS] Releasing in September

2016-01-20 Thread Peter Geoghegan
On Wed, Jan 20, 2016 at 8:23 PM, Michael Paquier
 wrote:
> Another tool that we had better IMO put some efforts in porting
> into core is sqlsmith, which would actually be a complete rewrite
> because the upstream code is under GPL license and depends on libpqxx.

Then Andreas Seltenreich better get a commit bit. I've seen him turn
around changes in sqlsmith to test new areas of Postgres in a couple
of days. That's a big reason why he has been so effective.

What benefit does porting sqlsmith for inclusion in core have? I can
only think of costs, including those that you mentioned.

-- 
Peter Geoghegan


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


Re: [HACKERS] Combining Aggregates

2016-01-20 Thread David Rowley
On 21 January 2016 at 04:59, Robert Haas  wrote:

> On Wed, Jan 20, 2016 at 7:53 AM, David Rowley
>  wrote:
> > On 21 January 2016 at 01:44, Robert Haas  wrote:
> >>
> >> On Wed, Jan 20, 2016 at 7:38 AM, David Rowley
> >>  wrote:
> >> >> To my mind, priority #1 ought to be putting this fine new
> >> >> functionality to some use.  Expanding it to every aggregate we've got
> >> >> seems like a distinctly second priority.  That's not to say that it's
> >> >> absolutely gotta go down that way, but those would be my priorities.
> >> >
> >> > Agreed. So I've attached a version of the patch which does not have
> any
> >> > of
> >> > the serialise/deserialise stuff in it.
> >> >
> >> > I've also attached a test patch which modifies the grouping planner to
> >> > add a
> >> > Partial Aggregate node, and a final aggregate node when it's possible.
> >> > Running the regression tests with this patch only shows up variances
> in
> >> > the
> >> > EXPLAIN outputs, which is of course expected.
> >>
> >> That seems great as a test, but what's the first patch that can put
> >> this to real and permanent use?
> >
> > There's no reason why parallel aggregates can't use the
> > combine_aggregate_state_d6d480b_2016-01-21.patch patch.
>
> I agree.  Are you going to work on that?  Are you expecting me to work
> on that?  Do you think we can use Haribabu's patch?  What other
> applications are in play in the near term, if any?


At the moment I think everything which will use this is queued up behind
the pathification of the grouping planner which Tom is working on. I think
naturally Parallel Aggregate makes sense to work on first, given all the
other parallel stuff in this release. I plan on working on that that by
either assisting Haribabu, or... whatever else it takes.

The other two usages which I have thought of are;

1) Aggregating before UNION ALL, which might be fairly simple after the
grouping planner changes, as it may just be a matter of considering another
"grouping path" which partially aggregates before the UNION ALL, and
performs the final grouping stage after UNION ALL. At this stage it's hard
to say how that will work as I'm not sure how far changes to the grouping
planner will go. Perhaps Tom can comment?

2) Group before join. e.g select p.description,sum(s.qty) from sale s inner
join s.product_id = p.product_id group by s.product_id group by
p.description;  I have a partial patch which implements this, although I
was a bit stuck on if I should invent the concept of "GroupingPaths", or
just inject alternative subquery relations which are already grouped by the
correct clause.  The problem with "GroupingPaths" was down to the row
estimates currently come from the RelOptInfo and is set
in set_baserel_size_estimates() which always assumes the ungrouped number
of rows, which is not what's needed if the grouping is already performed. I
was holding off to see how Tom does this in the grouping planner changes.

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


Re: [HACKERS] Combining Aggregates

2016-01-20 Thread David Rowley
On 21 January 2016 at 15:53, Haribabu Kommi 
wrote:

> On Thu, Jan 21, 2016 at 1:33 PM, Haribabu Kommi
>  wrote:
> >
> > Here I attached updated patch of parallel aggregate based on the latest
> > changes in master. Still it lack of cost comparison of normal aggregate
> to
> > parallel aggregate because of difficulty. This cost comparison is
> required
> > in parallel aggregate as this is having some regression when the number
> > of groups are less in the query plan.
> >
>
> Updated patch is attached after removing a warning in building group
> aggregate path.


Hi,

Thanks for updating the patch. I'd like to look at this with priority, but
can you post it on the Parallel Agg thread? that way anyone following there
can chime in over there rather than here.  I've still got a bit of work to
do (in the not too distant future) on the serial/deserial part, so would be
better to keep this thread for discussion on that.

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


Re: [HACKERS] [PATCH] Refactoring of LWLock tranches

2016-01-20 Thread Amit Kapila
On Thu, Jan 21, 2016 at 3:07 AM, Alvaro Herrera 
wrote:
>
> So far as I can tell, there are three patches in flight here:
>

Yes, thats right.

> * replication slot IO lwlocks
> * ability of extensions to request tranches dynamically
> * PGPROC
>
> The first one hasn't been reviewed at all, but the other two have seen a
> bit of discussion and evolution.
>

The last one "PGProc" has been reviewed and all the reported problems
have been fixed. It is "Ready For Committer".




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


Re: [HACKERS] Parallel Aggregate

2016-01-20 Thread Haribabu Kommi
On Thu, Dec 24, 2015 at 5:12 AM, Robert Haas  wrote:
> On Mon, Dec 21, 2015 at 6:38 PM, David Rowley
>  wrote:
>> On 22 December 2015 at 04:16, Paul Ramsey  wrote:
>>>
>>> Shouldn’t parallel aggregate come into play regardless of scan
>>> selectivity?
>>
>> I'd say that the costing should take into account the estimated number of
>> groups.
>>
>> The more tuples that make it into each group, the more attractive parallel
>> grouping should seem. In the extreme case if there's 1 tuple per group, then
>> it's not going to be of much use to use parallel agg, this would be similar
>> to a scan with 100% selectivity. So perhaps the costings for it can be
>> modeled around a the parallel scan costing, but using the estimated groups
>> instead of the estimated tuples.
>
> Generally, the way that parallel costing is supposed to work (with the
> parallel join patch, anyway) is that you've got the same nodes costed
> the same way you would otherwise, but the row counts are lower because
> you're only processing 1/Nth of the rows.  That's probably not exactly
> the whole story here, but it's something to think about.

Here I attached update parallel aggregate patch on top of recent commits
of combine aggregate and parallel join patch. It still lacks of cost comparison
code to compare both parallel and normal aggregates.


Regards,
Hari Babu
Fujitsu Australia


parallelagg_poc_v5.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] Releasing in September

2016-01-20 Thread Michael Paquier
On Thu, Jan 21, 2016 at 2:30 PM, Peter Geoghegan  wrote:
> On Wed, Jan 20, 2016 at 8:23 PM, Michael Paquier
>  wrote:
>> Another tool that we had better IMO put some efforts in porting
>> into core is sqlsmith, which would actually be a complete rewrite
>> because the upstream code is under GPL license and depends on libpqxx.
>
> Then Andreas Seltenreich better get a commit bit. I've seen him turn
> around changes in sqlsmith to test new areas of Postgres in a couple
> of days. That's a big reason why he has been so effective.
>
> What benefit does porting sqlsmith for inclusion in core have? I can
> only think of costs, including those that you mentioned.

We have automatic buildfarm coverage on many platforms. Perhaps we
could live without that with a buildfarm module though.
-- 
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] Batch update of indexes

2016-01-20 Thread konstantin knizhnik

On Jan 21, 2016, at 5:14 AM, Simon Riggs wrote:

> On 20 January 2016 at 14:55, Konstantin Knizhnik  
> wrote:
> Hi,
> 
> Hi, I glad to see that you interested in that too.
> I think this is a good feature and I think it will be very useful to have.
> I have already mentioned some related problems and possible improvements in 
> my presentation.
> http://www.slideshare.net/AnastasiaLubennikova/indexes-dont-mean-slow-inserts
> Last two slides concern to this thread. Briefly, I've suggested to think 
> about insertion buffer. Something very similar to it is already implemented 
> in BRIN. It does not index last data from heap, while the number of last 
> pages is less than pages_per_block.
> 
> Do you mean GIN-like usage of insertion buffer (here it is called "pending 
> list")?
> So that we have to combine search in the main tree and in the insert buffer?
> Actually this is what I want to avoided (because at least in case of GIN 
> pending list cause significant degrade of performance,
> while up-to-date state of full text index is rarely required).
> 
> Degrade in performance is because scan of pending list is O(N).
> 
> If we did the same thing for monotonic inserts into a btree, the performance 
> of ruling out any contents in the pending list would be O(1), so it is more 
> feasible than you say.

Sorry, didn't get it.
Certainly for B-Tree we can organize insert buffer (or pending list) as sorted 
array or also as a tree.
But in both case complexity of search in this buffer will be O(log(N)), not 
O(1).
O(1) is possible only if we will use hash table for pending list and are lucky 
with hash function.
But in this case it will be not possible to support range queries and proper 
order.

In any case, necessity to perform two searches instead of one and combining 
results will significantly complicate index implementation.
But definitely this solution is more convenient and transparent for users...


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



Re: [HACKERS] Batch update of indexes

2016-01-20 Thread Simon Riggs
On 20 January 2016 at 14:55, Konstantin Knizhnik 
wrote:

> Hi,
>
> Hi, I glad to see that you interested in that too.
>> I think this is a good feature and I think it will be very useful to have.
>> I have already mentioned some related problems and possible improvements
>> in my presentation.
>>
>> http://www.slideshare.net/AnastasiaLubennikova/indexes-dont-mean-slow-inserts
>> Last two slides concern to this thread. Briefly, I've suggested to think
>> about insertion buffer. Something very similar to it is already implemented
>> in BRIN. It does not index last data from heap, while the number of last
>> pages is less than pages_per_block.
>>
>
> Do you mean GIN-like usage of insertion buffer (here it is called "pending
> list")?
> So that we have to combine search in the main tree and in the insert
> buffer?
> Actually this is what I want to avoided (because at least in case of GIN
> pending list cause significant degrade of performance,
> while up-to-date state of full text index is rarely required).


Degrade in performance is because scan of pending list is O(N).

If we did the same thing for monotonic inserts into a btree, the
performance of ruling out any contents in the pending list would be O(1),
so it is more feasible than you say.

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: [HACKERS] COPY (... tab completion

2016-01-20 Thread Peter Eisentraut
On 1/19/16 6:00 AM, Andreas Karlsson wrote:
> On 01/19/2016 07:55 AM, Michael Paquier wrote:
>> +/* If we have COPY BINARY, compelete with list of tables */
>> s/compelete/complete
> 
> Fixed.
> 
>> +else if (TailMatches2("COPY|\\copy", "("))
>> +COMPLETE_WITH_LIST7("SELECT", "TABLE", "VALUES", "INSERT",
>> "UPDATE", "DELETE", "WITH");
>> This one should be Matches, no?
> 
> Yep, fixed.

Committed v4, thanks.



-- 
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] ALTER TABLE behind-the-scenes effects' CONTEXT

2016-01-20 Thread Simon Riggs
On 5 January 2016 at 06:45, Simon Riggs  wrote:

> On 4 January 2016 at 20:44, Alvaro Herrera 
> wrote:
>
>
>> Maybe
>> there are more ALTER TABLE subcommands that should be setting something
>> up?  In cases where multiple subcommands are being run, it might be
>> useful to see which one caused a certain error message.
>>
>
> I like the patch.
>
> We should have a message for each subcommand, since there are many that
> run for a long time and we support the optimization of allowing many
> subcommands together at once.
>
> There should also be a comment about making name a requirement for any
> subcommand.
>
>
>> I think some additional tests wouldn't hurt.
>>
>
> Each subcommand message should be generated at least once in tests.
>
>
>> I await feedback from Simon Riggs, who set himself up as reviewer a
>> couple of days ago.  Simon, do you also intend to be committer?  If so,
>> please mark yourself as such in the commitfest app.
>>
>
> Happy to be the committer on this.
>

Marko, I was/am waiting for an updated patch. Could you comment please?

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: [HACKERS] [PATCH] better systemd integration

2016-01-20 Thread Pavel Stehule
2016-01-21 3:33 GMT+01:00 Peter Eisentraut :

> On 1/18/16 10:58 AM, Alvaro Herrera wrote:
> > Peter Eisentraut wrote:
> >> I have written a couple of patches to improve the integration of the
> >> postgres daemon with systemd.
> >
> > Great.  Is anything happening with these patches, or?  They've been
> > inactive for quite a while now.
>
> Well, they are waiting for someone to review them.
>

I read some basic materials about systemd and these patche looks correct.
Next week I'll test it.

Regards

Pavel


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


Re: [HACKERS] Releasing in September

2016-01-20 Thread Michael Paquier
On Thu, Jan 21, 2016 at 1:32 PM, Michael Paquier
 wrote:
> On Thu, Jan 21, 2016 at 12:59 AM, Bruce Momjian  wrote:
>> On Wed, Jan 20, 2016 at 10:55:07AM -0500, Robert Haas wrote:
>>> On Wed, Jan 20, 2016 at 10:48 AM, Andres Freund  wrote:
>>> > I think this has very little to do with commitfest schedules, and much
>>> > more with the "early" forking of the new version branch. For both 9.4
>>> > and 9.5 we essentially spent a couple months twiddling our thumbs.
>>>
>>> It's certainly true that we twiddled our thumbs quite a bit about
>>> getting 9.5 ready to ship.  However, the old process where nobody
>>> could get anything committed for six months out of the year blew
>>> chunks, too.  Personally, I think that the solution is to cut off the
>>> last CommitFest a lot sooner, and then reopen the tree for the next
>>> release as soon as possible.  But this never works, because there are
>>> always patches we want to slip in late.
>>
>> The bottom line is we can't sustain five commitfests and stay on time
>> --- we need to go back to four, which I think is what we used to do.  We
>> twiddled our thumbs back in the September-release years too, but had
>> consistency because twiddling was built into the schedule.
>
> Here I agree. Five commit fests is proving to be a lot and CF managers
> are facing a severe burnout. Though I guess it sounded like a fine
> idea at the moment this was begun (disclaimer: I was not there). So we
> may want to do back to 4, and avoid too much overlapping between CFs
> and what would be a stability period, without any commit fests going
> on. It seems to me that the problem regarding the fact that fixes got
> committed late is that committer's, author's and reviewer's attention
> are getting distracted with commit fests and the new features proposed
> there. Perhaps some people are more interested in implementing new
> features than working on bugs and would just continue hacking and
> arguing about new features, at least a stability period may attract
> more committer attention into actual bug fixes, in short: no new
> features can be committed until the previous versions has reached at
> least beta2, rc, whatever. This may accelerate the stability process.

Another idea popping to my mind in order to fix the CF manager burnout
and actually motivate people into becoming CF managers: say that one a
CF manager is done with a commit fest, folks on -hackers discuss if
the CFM has done a good job or not. If yes, he gets a travel paid to
the conference of his choice. That's assuming that there are actual
funds for that. Most people here perhaps may not care as their company
are usually willing to pay for such trip, but in some cases I am sure
that there are people who can be funded by their company *if* they do
a presentation to this conference. Just a random idea.
-- 
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] Releasing in September

2016-01-20 Thread Tom Lane
Michael Paquier  writes:
> On Thu, Jan 21, 2016 at 2:30 PM, Peter Geoghegan  wrote:
>> What benefit does porting sqlsmith for inclusion in core have? I can
>> only think of costs, including those that you mentioned.

> We have automatic buildfarm coverage on many platforms. Perhaps we
> could live without that with a buildfarm module though.

I do not think we should necessarily try to include every testing tool
in the core distribution.  What is important is that they be readily
available: easy to find, easy to use, documented, portable.  "Same
license as the PG core code" is not on that list.

An immediately relevant example is that the buildfarm server and client
code aren't in the core distribution, and AFAIR no one has suggested
that they need to be.

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] better systemd integration

2016-01-20 Thread Peter Eisentraut
On 1/18/16 10:58 AM, Alvaro Herrera wrote:
> Peter Eisentraut wrote:
>> I have written a couple of patches to improve the integration of the
>> postgres daemon with systemd.
> 
> Great.  Is anything happening with these patches, or?  They've been
> inactive for quite a while now.

Well, they are waiting for someone to review them.



-- 
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] Combining Aggregates

2016-01-20 Thread Haribabu Kommi
On Thu, Jan 21, 2016 at 12:32 PM, David Rowley
 wrote:
> On 21 January 2016 at 04:59, Robert Haas  wrote:
>>
>> On Wed, Jan 20, 2016 at 7:53 AM, David Rowley
>>  wrote:
>> > On 21 January 2016 at 01:44, Robert Haas  wrote:
>> >>
>> >> On Wed, Jan 20, 2016 at 7:38 AM, David Rowley
>> >>  wrote:
>> >> >> To my mind, priority #1 ought to be putting this fine new
>> >> >> functionality to some use.  Expanding it to every aggregate we've
>> >> >> got
>> >> >> seems like a distinctly second priority.  That's not to say that
>> >> >> it's
>> >> >> absolutely gotta go down that way, but those would be my priorities.
>> >> >
>> >> > Agreed. So I've attached a version of the patch which does not have
>> >> > any
>> >> > of
>> >> > the serialise/deserialise stuff in it.
>> >> >
>> >> > I've also attached a test patch which modifies the grouping planner
>> >> > to
>> >> > add a
>> >> > Partial Aggregate node, and a final aggregate node when it's
>> >> > possible.
>> >> > Running the regression tests with this patch only shows up variances
>> >> > in
>> >> > the
>> >> > EXPLAIN outputs, which is of course expected.
>> >>
>> >> That seems great as a test, but what's the first patch that can put
>> >> this to real and permanent use?
>> >
>> > There's no reason why parallel aggregates can't use the
>> > combine_aggregate_state_d6d480b_2016-01-21.patch patch.
>>
>> I agree.  Are you going to work on that?  Are you expecting me to work
>> on that?  Do you think we can use Haribabu's patch?  What other
>> applications are in play in the near term, if any?
>
>
> At the moment I think everything which will use this is queued up behind the
> pathification of the grouping planner which Tom is working on. I think
> naturally Parallel Aggregate makes sense to work on first, given all the
> other parallel stuff in this release. I plan on working on that that by
> either assisting Haribabu, or... whatever else it takes.
>

Here I attached updated patch of parallel aggregate based on the latest
changes in master. Still it lack of cost comparison of normal aggregate to
parallel aggregate because of difficulty. This cost comparison is required
in parallel aggregate as this is having some regression when the number
of groups are less in the query plan.

Regards,
Hari Babu
Fujitsu Australia


parallelagg_poc_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] Spurious standby query cancellations

2016-01-20 Thread Simon Riggs
On 24 December 2015 at 20:15, Jeff Janes  wrote:

> On Wed, Dec 23, 2015 at 9:40 PM, Jeff Janes  wrote:
> > On Wed, Sep 23, 2015 at 11:33 PM, Jeff Janes 
> wrote:
> >>
> >> On further thought, neither do I.  The attached patch inverts
> >> ResolveRecoveryConflictWithLock to be called back from the lmgr code so
> that
> >> is it like ResolveRecoveryConflictWithBufferPin code.  It does not try
> to
> >> cancel the conflicting lock holders from the signal handler, rather it
> just
> >> loops an extra time and cancels the transactions on the next call.
> >>
> >> It looks like the deadlock detection is adequately handled within normal
> >> lmgr code within the back-ends of the other parties to the deadlock, so
> I
> >> didn't do a timeout for deadlock detection purposes.
> >
> > I was testing that this still applies to git HEAD.  And it doesn't due
> > to the re-factoring of lock.h into lockdef.h.  (And apparently it
> > never actually did, because that refactoring happened long before I
> > wrote this patch.  I guess I must have done this work against
> > 9_5_STABLE.)
> >
> > standby.h cannot include lock.h because standby.h is included
> > indirectly by pg_xlogdump.  But it has to get LOCKTAG which is only in
> > lock.h.
> >
> > Does this mean that standby.h also needs to get parts spun off into a
> > new standbydef.h that can be included from front-end code?
>
>
> That is how I've done it.
>
> The lock cancel patch applies over the header split patch.
>

This looks good to me, apart from some WhitespaceCrime.

Header split applied, will test and apply the main patch this week.

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: [HACKERS] Releasing in September

2016-01-20 Thread Michael Paquier
On Thu, Jan 21, 2016 at 12:59 AM, Bruce Momjian  wrote:
> On Wed, Jan 20, 2016 at 10:55:07AM -0500, Robert Haas wrote:
>> On Wed, Jan 20, 2016 at 10:48 AM, Andres Freund  wrote:
>> > I think this has very little to do with commitfest schedules, and much
>> > more with the "early" forking of the new version branch. For both 9.4
>> > and 9.5 we essentially spent a couple months twiddling our thumbs.
>>
>> It's certainly true that we twiddled our thumbs quite a bit about
>> getting 9.5 ready to ship.  However, the old process where nobody
>> could get anything committed for six months out of the year blew
>> chunks, too.  Personally, I think that the solution is to cut off the
>> last CommitFest a lot sooner, and then reopen the tree for the next
>> release as soon as possible.  But this never works, because there are
>> always patches we want to slip in late.
>
> The bottom line is we can't sustain five commitfests and stay on time
> --- we need to go back to four, which I think is what we used to do.  We
> twiddled our thumbs back in the September-release years too, but had
> consistency because twiddling was built into the schedule.

Here I agree. Five commit fests is proving to be a lot and CF managers
are facing a severe burnout. Though I guess it sounded like a fine
idea at the moment this was begun (disclaimer: I was not there). So we
may want to do back to 4, and avoid too much overlapping between CFs
and what would be a stability period, without any commit fests going
on. It seems to me that the problem regarding the fact that fixes got
committed late is that committer's, author's and reviewer's attention
are getting distracted with commit fests and the new features proposed
there. Perhaps some people are more interested in implementing new
features than working on bugs and would just continue hacking and
arguing about new features, at least a stability period may attract
more committer attention into actual bug fixes, in short: no new
features can be committed until the previous versions has reached at
least beta2, rc, whatever. This may accelerate the stability process.
-- 
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] WIP patch to improve amvalidate functions

2016-01-20 Thread Alvaro Herrera
Tom Lane wrote:

> I'm posting this now just in case anyone has some comments, or quibbles
> about the overall intent.  In particular, if anyone has an idea for a more
> thorough missing-objects check on BRIN opfamilies, I would like to hear
> it.  The fact that there are two kinds of opfamilies with rather different
> internal consistency requirements is a real pain there, and the check
> rules I have here are definitely the result of some trial and error.

Without reading your code: maybe each opfamily framework could itself
provide a validator function as a separate support procedure (currently
brin_minmax_validate and brin_inclusion_validate); so the generic BRIN
amvalidate verifies the basic elements of the opfamily, then hands off
to the opfamily-framework-specific validator function for additional
checking.

-- 
Álvaro Herrerahttp://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] Combining Aggregates

2016-01-20 Thread Haribabu Kommi
On Thu, Jan 21, 2016 at 1:33 PM, Haribabu Kommi
 wrote:
>
> Here I attached updated patch of parallel aggregate based on the latest
> changes in master. Still it lack of cost comparison of normal aggregate to
> parallel aggregate because of difficulty. This cost comparison is required
> in parallel aggregate as this is having some regression when the number
> of groups are less in the query plan.
>

Updated patch is attached after removing a warning in building group
aggregate path.

Regards,
Hari Babu
Fujitsu Australia


parallelagg_poc_v5.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] WIP patch to improve amvalidate functions

2016-01-20 Thread Tom Lane
Alvaro Herrera  writes:
> Tom Lane wrote:
>> I'm posting this now just in case anyone has some comments, or quibbles
>> about the overall intent.  In particular, if anyone has an idea for a more
>> thorough missing-objects check on BRIN opfamilies, I would like to hear
>> it.  The fact that there are two kinds of opfamilies with rather different
>> internal consistency requirements is a real pain there, and the check
>> rules I have here are definitely the result of some trial and error.

> Without reading your code: maybe each opfamily framework could itself
> provide a validator function as a separate support procedure (currently
> brin_minmax_validate and brin_inclusion_validate); so the generic BRIN
> amvalidate verifies the basic elements of the opfamily, then hands off
> to the opfamily-framework-specific validator function for additional
> checking.

Yeah, there were already some discussions in the index-AM-API-rewrite
thread about providing ways for individual opclasses to help in checking.
BRIN's not the only AM with an issue --- GIST, GIN, SPGIST all have the
property that only an individual opclass really knows which operator
strategy numbers are valid for it.  OTOH, I'm not certain how important
that is for those AMs, because their opclasses are fairly self-contained.
BRIN seems to have a two-level structure here which makes it more
important to offer cross-checking.

regards, tom lane


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


Re: [HACKERS] exposing pg_controldata and pg_config as functions

2016-01-20 Thread Michael Paquier
On Wed, Jan 20, 2016 at 2:32 AM, Joe Conway  wrote:
> The only things I know of still lacking is:
> 1) Documentation
> 2) Decision on REVOKE ... FROM PUBLIC

Yep, regarding 2) I am the only one actually making noise to protect
this information by default, against at least 2 committers :)

> I'm assuming by the lack of complainants that there is enough support
> for the feature (conceptually at least) that it is worthwhile for me to
> write the docs. Will do that over the next couple of days or so.

I would think so as well.

+typedef struct configdata
+{
+   char   *name;
+   char   *setting;
+} configdata;
For a better analogy to ControlFileData, this could be renamed ConfigData?

-OBJS_COMMON = exec.o pg_lzcompress.o pgfnames.o psprintf.o relpath.o \
-   rmtree.o string.o username.o wait_error.o
+# don't include subdirectory-path-dependent -I and -L switches
+STD_CPPFLAGS := $(filter-out -I$(top_srcdir)/src/include
-I$(top_builddir)/src/include,$(CPPFLAGS))
+STD_LDFLAGS := $(filter-out -L$(top_builddir)/src/port,$(LDFLAGS))
The point of the move to src/common is to remove the duplication in
src/bin/pg_config/Makefile.

--- /dev/null
+++ b/src/common/config_info.c
[...]
+#include "postgres.h"
+
+#include "miscadmin.h"
+#include "common/config_info.h"
All the files in src/common should begin their include declarations with that:
#ifndef FRONTEND
#include "postgres.h"
#else
#include "postgres_fe.h"
#endif

+configdata *
+get_configdata(char *my_exec_path, size_t *configdata_len)
+{
It may be good to mention that the result is palloc'd and that caller
may need to free it if necessary. It does not matter in the two code
paths of this patch, but it may matter for other users calling that.

MSVC compilation is working correctly here.
-- 
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] Expanded Objects and Order By

2016-01-20 Thread Tom Lane
I wrote:
> Paul Ramsey  writes:
>> Thank the Maker, it is reproduceable: returning an expanded header in
>> the _in function is not appreciated in a very narrow number of cases.

> Thanks for finding a test case!  I'll check into it shortly.

So the short answer is that the planner assumes, not unreasonably,
that copying a parse node with copyObject() should produce a node
that's still equal() to the original.  (The proximate cause of the
"could not find pathkey item to sort" error is failure to match
an ORDER BY expression to an equivalence-class item that was made
from it earlier.)

In the case here, coerce_type() produces a Const node whose constvalue
is pointing at the expanded array returned by array_in().  Nothing
further happens to that node until the planner tries to copy it ---
and copyObject will produce a flattened array value, cf datumCopy().
Even if copying made a new expanded object, that object would not be
equal() to the original node according to datumIsEqual's very
simplistic comparisons.

Now that I've seen this, I'm suspicious that we probably have a lot
of other related bugs --- for example, short-header and non-short-header
varlena values would also not be seen as equal().

What I'm thinking is that we had best apply pg_detoast_datum() to any
varlena value being put into a Const node.  That will do nothing in the
normal case where the value already is a simple standard-header varlena
value, but will fix any cases where we might be trying to put a toasted,
expanded, etc value into a Const.  Generally it'd be a bad idea to have a
toast reference in a Const anyhow; for example, there is code at the end
of evaluate_expr() that does the equivalent of this when making a Const
from the result of constant-simplifying an expression.  That got put in
there as the result of hard experience; but now it seems we did not go
deep enough.

An alternative idea would be to make datumIsEqual smarter; but given the
other ways in which it's used I doubt we want it to be trying to do
detoasting.

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] Releasing in September

2016-01-20 Thread Michael Paquier
On Thu, Jan 21, 2016 at 8:51 AM, Peter Geoghegan  wrote:
> On Wed, Jan 20, 2016 at 2:56 PM, Tom Lane  wrote:
>> As a concrete example, I recall that Heikki or someone had a tool for
>> checking WAL replay by comparing master and slave disk contents.  We
>> should make an effort to get that into a state where anyone can use it.
>
> I think that that was initially Jeff Janes. Jeff also provided great
> testing infrastructure for UPSERT, something that I was very grateful
> for.

You mean the full-page write consistency tool? Heikki initiated that,
a common interface to be able to mask pages and make them consistent
for comparison on the master and the standby was also part of the
deal. Another tool that we had better IMO put some efforts in porting
into core is sqlsmith, which would actually be a complete rewrite
because the upstream code is under GPL license and depends on libpqxx.
-- 
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] checkpointer continuous flushing

2016-01-20 Thread Amit Kapila
On Wed, Jan 20, 2016 at 9:07 PM, Andres Freund  wrote:
>
> On 2016-01-20 12:16:24 -0300, Alvaro Herrera wrote:
> > Andres Freund wrote:
> >
> > > The relevant thread is at
> > >
http://archives.postgresql.org/message-id/CA%2BTgmoaCr3kDPafK5ygYDA9mF9zhObGp_13q0XwkEWsScw6h%3Dw%40mail.gmail.com
> > > what I didn't remember is that I voiced concern back then about
exactly this:
> > >
http://archives.postgresql.org/message-id/201112011518.29964.andres%40anarazel.de
> > > ;)
> >
> > Interesting.  If we consider for a minute that part of the cause for the
> > slowdown is slowness in pg_clog, maybe we should reconsider the initial
> > decision to flush as quickly as possible (i.e. adopt a strategy where
> > walwriter sleeps a bit between two flushes) in light of the group-update
> > feature for CLOG being proposed by Amit Kapila in another thread -- it
> > seems that these things might go hand-in-hand.
>
> I don't think it's strongly related - the contention here is on read
> access to the clog, not on write access.

Aren't reads on clog contended with parallel writes to clog?


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


Re: [HACKERS] Batch update of indexes

2016-01-20 Thread Simon Riggs
On 21 January 2016 at 06:41, konstantin knizhnik 
wrote:


> Certainly for B-Tree we can organize insert buffer (or pending list) as
> sorted array or also as a tree.
> But in both case complexity of search in this buffer will be O(log(N)),
> not O(1).
>

You are right; search within this buffer is O(log(N)). But we can test
whether we need to search in the buffer at all with O(1).

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: [HACKERS] Re: pglogical_output - a general purpose logical decoding output plugin

2016-01-20 Thread Craig Ringer
On 21 January 2016 at 06:23, Tomasz Rybak  wrote:


>
> I reviewed more files:


Thanks.

Can you try to put more whitespace between items? It can be hard to follow
at the moment.


> pglogical_proto_native.c
>
> +   pq_sendbyte(out, 'N');  /* column name block
> follows */
> +   attname = NameStr(att->attname);
> +   len = strlen(attname) + 1;
> +   pq_sendint(out, len, 2);
> +   pq_sendbytes(out, attname, len); /* data */
> Identifier names are limited to 63 - so why we're sending 2 bytes here?
>

Good question. It should be one byte. I'll need to amend the protocol for
that, but I don't think that's a problem this early on.


> +   pq_sendbyte(out, 'B');  /* BEGIN */
> +
> +   /* send the flags field its self */
> +   pq_sendbyte(out, flags);
> Comment: "flags field its self"? Shouldn't be "... itself"?
> Similarly in write_origin; write_insert just says:
>

itself is an abbreviation of its self.


> +   /* send the flags field */
> +   pq_sendbyte(out, flags);
>
> Speaking about flags - in most cases they are 0; only for attributes
> we might have:

+   flags |= IS_REPLICA_IDENTITY;
> I assume that flags field is put into protocol for future needs?
>

Correct. The protocol specifies (in most places; need to double check all
sites) that the lower 4 bits are reserved and must be treated as an ERROR
if set. The high 4 bits must be ignored if set and not recognised. That
gives us some room to wiggle without bumping the protocol version
incompatibly, and lets us use capabilities (via client-supplied parameters)
to add extra information on the wire.

+   /* FIXME support whole tuple (O tuple type) */
> +   if (oldtuple != NULL)
> +   {
> +   pq_sendbyte(out, 'K');  /* old key follows */
> +   pglogical_write_tuple(out, data, rel, oldtuple);
> +   }
> I don't fully understand. We are sending whole old tuple here,
> so this FIXME should be more about supporting sending just keys.
> But then comment "old key follows" is not true. Or am I missing
> something here?
>

In wal_level=logical the tuple that's written is an abbreviated tuple
containing data only for the REPLICA IDENTITY fields.

Ideally we'd also be able to support sending the _whole_ old tuple, but
this would require the ability to log the whole old tuple in WAL when
logging a DELETE or UPDATE into WAL. This isn't so much a FIXME as a
logical decoding limitation and wishlist item; I'll amend to that effect.




>
> +   pq_sendbyte(out, 'S');  /* message type field */
> +   pq_sendbyte(out, 1);/* startup message version */
> For now protocol is 1, but for code readability it might be better
> to change this line to:
> +   pq_sendbyte(out, PGLOGICAL_PROTO_VERSION_NUM);  /* startup message
> version */
>

The startup message format isn't the same as the protocol version.
Hopefully we'll never have to change it. The reason it's specified is so
that if we ever do bump it a decoding plugin can recognise an old client
and fall back. Maybe it's BC overkill but I'd kick myself for not doing it
if we ever decided to (say) add support for structured json startup options
from the client. Working on BDR has taught me that there's no such thing as
too much consideration for cross-version compat and negotiation in
replication.

I'm happy to create a new define for that an comment to this effect.


> Just for the sake of avoiding code repetition:
> +   for (i = 0; i < desc->natts; i++)
> +   {
> +   if (desc->attrs[i]->attisdropped)
> +   continue;
> +   nliveatts++;
> +   }
> +   pq_sendint(out, nliveatts, 2);



The exact same code is in write_tuple and write_attrs. I don't know what's
> policy for refactoring, but this might be extracted into separate function.
>

Seems trivial enough not to care, but probably can.


>
> +   else if (att->attlen == -1)
> +   {
> +   char *data =
> DatumGetPointer(values[i]);
> +
> +   /* send indirect datums inline */
> +   if
> (VARATT_IS_EXTERNAL_INDIRECT(values[i]))
> +   {
> +   struct varatt_indirect
> redirect;
> +
>  VARATT_EXTERNAL_GET_POINTER(redirect, data);
> +   data = (char *)
> redirect.pointer;
> +   }



I really don't like this. We have function parameter "data" and now
> are creating new variable with the same name.


I agree. Good catch.

I don't much like the use of 'data' as the param name for the plugin
private data and am quite inclined to change that instead, to
plugin_private or something.

pglogical_proto_json.c
>
> +   

Re: [HACKERS] Batch update of indexes

2016-01-20 Thread Konstantin Knizhnik

Hi,


Hi, I glad to see that you interested in that too.
I think this is a good feature and I think it will be very useful to have.
I have already mentioned some related problems and possible 
improvements in my presentation.

http://www.slideshare.net/AnastasiaLubennikova/indexes-dont-mean-slow-inserts
Last two slides concern to this thread. Briefly, I've suggested to 
think about insertion buffer. Something very similar to it is already 
implemented in BRIN. It does not index last data from heap, while the 
number of last pages is less than pages_per_block.


Do you mean GIN-like usage of insertion buffer (here it is called 
"pending list")?

So that we have to combine search in the main tree and in the insert buffer?
Actually this is what I want to avoided (because at least in case of GIN 
pending list cause significant degrade of performance,

while up-to-date state of full text index is rarely required).


The next point, I've thought about is a bulk update. Problem is that 
update like "UPDATE mytable set a = a+1;" causes N searches from the 
root of B-tree. I looks very strange to me, and I'd like to fix it 
somehow. The obvious solution is to update all tuples on the page at a 
time, and keep the number of last updated page. But, maybe it's a bit 
off-thread here.


Bulk update is the second question (but very important).
First I just want to be able to append index concurrently, not blocking 
insert.




One interesting approach of solving this problem is discussed in this 
article:


https://mark.zealey.org/2016/01/08/how-we-tweaked-postgres-upsert-performance-to-be-2-3-faster-than-mongodb

Them are using materialized views to build indexes in background.
Interesting idea, but copying content of the whole table just to be 
able to build index concurrently seems to be overkill.


This approach seems like a tricky crutch to me. And I agree that it 
requires a lot of extra work.


It will be very interesting to know how people are using materialized views.
Delayed building of indexes seems to be one of the popular use cases, 
although requiring large overhead, first of all storage overhead.






Please notice that such alter table statement, changing condition for 
partial index, is not supported now.


Don't you think, that this feature could be used in a very wrong way? 
Do not take it as criticism, just a bit of thoughts.




Everything which can be misused, will be misused:)
But I do not worry much about it...
If it can address real challenges, then it will be good thing in any case.

Ideally we should be able to alter everything. Naive implementation of 
such alter clause can just to build new index with temporary name, then 
drop old index and rename new index.





There was the discussion of the patch for partial indexes.
http://postgresql.nabble.com/PATCH-index-only-scans-with-partial-indexes-td5857568.html
 Since I haven't watched it closely, It seems to be open still. I 
think it'll be interesting to you.




So small patch...
Why it was not accepted?
I do no see any problems with it...



--
Anastasia Lubennikova
Postgres Professional:http://www.postgrespro.com
The Russian Postgres Company


--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The 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] WIP: Failover Slots

2016-01-20 Thread Craig Ringer
On 20 January 2016 at 21:02, Craig Ringer  wrote:


> TL;DR: this doesn't work yet, working on it.
>

Nothing is logged on slot creation because ReplicationSlot->data.failover
is never true. Once that's fixed by - for now - making all slots failover
slots, there's a crash in XLogInsert because of the use of reserved bits in
the XLogInsert info argument. Fix pushed.

I also noticed that slot drops seem are being logged whether or not the
slot is a failover slot. Pushed a fix for that.

The WAL writing is now working. I've made improvements to the rmgr xlogdump
support to make it clearer what's written.

Slots are still not visible on the replica so there's work to do tracing
redo, promotion, slot handling after starting from a basebackup, etc. The
patch is still very much WIP.

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


Re: [HACKERS] jsonb array-style subscription

2016-01-20 Thread Dmitry Dolgov
On 20 January 2016 at 02:14, Alvaro Herrera 
wrote:

> Dmitry Dolgov wrote:
>
> > I've cleaned up the code, created a separate JsonbRef node (and there
> are a
> > lot of small changes because of that), abandoned an idea of "deep
> nesting"
> > of assignments (because it doesn't relate to jsonb subscription, is more
> > about the
> > "jsonb_set" function, and anyway it's not a good idea). It looks fine for
> > me, and I need a little guidance - is it ok to propose this feature for
> > commitfest 2016-03 for a review?
>
> Has this patch been proposed in some commitfest previously?


Unfortunately, it wasn't. I just sent an intermediate version of this patch
to hackers several months ago to discuss it.

you can't add patches that are too invasive to the last one -- so your last
> chance for 9.6 was 2016-01.


Yes, that's what I was worried about (although I didn't know exactly about
this rule before).


Re: [HACKERS] PostgreSQL 9.5.0 regress tests fails with Python 3.5.0

2016-01-20 Thread Tom Lane
Pavel Stehule  writes:
> make check-world fails,
> It is fixed in HEAD.

BTW, I have now spun up a new buildfarm member "longfin", which is
testing against Python 3.5.1 (except in the 9.1 branch, where that
doesn't work).

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] Releasing in September

2016-01-20 Thread Robert Haas
On Wed, Jan 20, 2016 at 12:03 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> But I'm not very sure that we're talking about the same set of people
>> here.  If we're going to go to a system where nobody's allowed to
>> commit anything for the next release until the current release is
>> finalized, then we'd better have some procedure for making sure that
>> happens relatively quickly.  And the procedure can't be that the
>> people who are hot to get started on the next release have to bat
>> cleanup for the people who don't have time to fix the bugs they
>> introduced previously.  Because *that* would be manifestly unfair.
>
> You're assuming that the people who are hot to get started on the next
> release are different from the people who don't have time to fix the bugs
> they introduced previously.  IME it's mostly the same people.

The last common commit between master and REL9_5_STABLE is dated June
30th.  Here is the open items list as of that date:

https://wiki.postgresql.org/index.php?title=PostgreSQL_9.5_Open_Items=25373

The only thing on that list that I had anything to do with is "Foreign
join pushdown vs EvalPlanQual".  And that only would have mattered to
people who want to do out-of-core FDW join pushdown against 9.5 and
aren't comfortable restricting the feature to queries without
rowmarks, which is probably nobody, and "PGXS "check" target forcing
an install ?", which was a very minor issue that I didn't create but
did help fix.  Similarly, I see nothing on that list that could be
attributed to, say, you.

I hate to name names, but it seems to me that where things came off
the rails for 9.5 is that (1) the row-level security feature was
absolutely riddled with bugs and that those bugs were not fixed in
anything like timely fashion; (2) the commit timestamp stuff was
pretty badly broken, too; and (3) Andres's multixact reworks landed
quite late and IMHO were not safe enough to back-patch, yet they
needed to be in the release.  In my view, the last major fix for (1)
was in 7d8db3e8f37aec9d252353904e77381a18a2fa9f on September 30th, for
(2) was in commit 69e7235c93e2965cc0e17186bd044e4c54997c19 on December
11th, and for (3) was in aa29c1ccd9f785f9365809f5133e5491acc7ae53 on
September 26th.  I think the row-security stuff is particularly bad
because that feature was originally committed on September 19, *2014*.
It's not good when you commit a security feature and are still
redefining the way that the privilege system interacts with that
feature a year later.

Also note the dates.  We put out beta1 in the beginning of October,
right after those major fixes for (1) and (3) in late September.  And
we put out rc1 just after (2) got patched up.  There is of course a
chicken and egg problem here, because it's unclear to what degree the
commits drove the wrap date and to what degree the wrap date drove the
commits, but if we can't get fixes for those kinds of issues into the
tree on time, we are not going to release on time.  Now maybe if we
refuse to branch until we get these kinds of issues fixed, we'll fix
them sooner (or revert the patches that caused the problems in the
first place, which is another way to go).  That would be a positive
development.  But if we end up waiting just as long but with nothing
substantial getting committed in the meantime, that will be awful.

-- 
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] Releasing in September

2016-01-20 Thread Robert Haas
On Wed, Jan 20, 2016 at 1:29 PM, Andres Freund  wrote:
> On 2016-01-20 13:13:29 -0500, Robert Haas wrote:
>> (3) Andres's multixact reworks landed quite late and IMHO were not
>> safe enough to back-patch, yet they needed to be in the release.  In
>> my view, the last major fix for
>
> At least in that case I was, for a long while, basically hoping for more
> input, probably unwisely so. I think the 9.4 jsonb compressability thing
> essentially was in a somewhat similar state for months; the proposed
> patch was mostly there, but we didn't press ahead.  I think deadlines
> and multiple people being blocked help tremendously with such issues.
> They're often too complex for an individual and/or delayed due to lack
> of concensus.

True.

>> But if we end up waiting just as long but with nothing substantial
>> getting committed in the meantime, that will be awful.
>
> That's true. Which I think means that we'll need to be agressive in
> threatening reverts in such cases. That wouldn't have worked for 3), but
> in the other two cases it'd have been doable.

Yes.  One problem is that it's very hard to get a patch reverted.  Tom
opined that too often patches roll along toward commit and that it's
too hard to say "no, we want that".  But that's nothing compared to
how hard it is to get a patch reverted.  It's true that, if Tom calls
for the revert, it more often than not happens, but nobody else can
really pull that off without a major war.

And, you know, I did my time fighting major wars to try to compress
the release schedule, and honestly, it wasn't that much fun.  The
process we have now is imperfect in many ways, but I no longer have
abuse heaped on me for wanting to boot a patch out of a CommitFest.
That may be bad for the project, but it's spared me a lot of grief
personally.  I think that many other people are in the same situation.
Everybody would like somebody else to be the schedule enforcer ...
unless they have something that they still want to get in, in which
case they would like nobody to do it.

-- 
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] Proposal for UPDATE: do not insert new tuple on heap if update does not change data

2016-01-20 Thread Tom Lane
Gasper Zejn  writes:
> I was wondering if PostgreSQL adds new tuple if data is not changed
> when using UPDATE. It turns out it does add them and I think it might
> be beneficial not to add a new tuple in this case, since it causes a
> great deal of maintenance: updating indexes, vacuuming table and
> index, also heap fragmentation.

This has been discussed in the past, and the conclusion was that expending
cycles on every UPDATE to check for this case would be a net loss.  How
many real applications do no-op updates often enough that it's worth
optimizing for?

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] Releasing in September

2016-01-20 Thread Joshua D. Drake

On 01/20/2016 07:40 AM, Bruce Momjian wrote:


Our current 9.5/9.6 timing looks more like the 8.X series of release
dates.  Everyone might be fine with that, but we had better be prepared
for November-February major release dates going forward.


Bruce,

Thank you for bringing this up it is a good time to discuss it. I think 
we have a number of things we need to consider when it comes to releases:


1. When we release
2. What we release
3. When not to release

#1 September is actually a great time to release. However, I think that 
we are going to run into trouble keeping that time frame. The reality is 
PostgreSQL of today is a lot more complex than the PostgreSQL of even 3 
years ago.


That means that unless we are willing to have stunted releases, we are 
going to miss release dates. We need to be able to say, "Ompf, no whiz 
bang features this release, mostly polish" or "These whiz bang features 
need more testing, we are pushing our release". If we aren't willing to 
do that then we need to reconsider having a fixed release date and 
perhaps start a release time frame (9.6 will be released Q4 2016).


#2 This is a longer topic. I have been stewing in my head about releases 
for years. I have even brought up the idea of an Ubuntu style release 
cycle on list once or twice. The more I think about it, the more I think 
this can help our community. We essentially would have two types of 
releases:


STS:

* Supported for 1 release cycle plus 6 months (18-24 months)
* Inline upgrades supported

LTS:

* Supported for standard 5 years
* Is allowed to break binary format from STS but not previous LTS. This 
allows two LTS versions per 5 year support cycle


There is more to say on this but this is already a long reply.

#3 The fact that 9.5 was delayed is a good thing, not a bad one. I 
believe we need to be much more willing to push a button that says, "It 
isn't done." Which goes back to the idea in #1 of having a release 
"window" versus date. We aren't a company. We don't need to adhere to an 
exact release schedule.


Sincerely,

Joshua D. Drake






--
Command Prompt, Inc.  http://the.postgres.company/
 +1-503-667-4564
PostgreSQL Centered full stack support, consulting and development.


--
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] Releasing in September

2016-01-20 Thread Tom Lane
Magnus Hagander  writes:
> That's pretty much what I suggested :)

> Except that we need to do it for the last one as well. With the only
> exception that the last one might be a bit longer. But the fact is that the
> recent of CFs *and* releases, we've taken the approach of closing the CF
> when everything in it is done or explicitly reviewed-and-bumped, and tried
> really really hard not to bump things because nobody had time to look at
> them. That's what I'm suggesting we change, and actually just cut them.
> Yes, one of the reasons for the CFs was to allow people a fair chance to
> get reviewed.But given that there isn't actually a deadline in practice
> doesn't help with that.

Yeah.  It's certainly unfair if someone's patch doesn't get reviewed,
but there are only 24 hours in a day, and we have a limited pool of
reviewer and committer manpower.  I think we just have to say that
sometimes life is unfair.

I think it might also be a good idea if we could somehow distinguish
"nobody had time for that yet" from "nobody is interested", with an eye
to flat-out rejecting patches that no one cares enough about to review.
Maybe we could reduce the workload by inventing some kind of up-front
filter whereby a patch isn't even a candidate to get reviewed unless, say,
at least two people besides the author say "this sounds like it's worth
pursuing".

One of the other things I do not like about the current CF process is that
it's created a default assumption that most submitted patches should get
accepted eventually.  It is *very* hard to reject a patch altogether in
the CF process: you more or less have to show cause why it should not get
accepted.  That default is backwards.  Maybe this isn't the process' fault
exactly, but it sure seems like that's how we approach patches now.

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] Releasing in September

2016-01-20 Thread Joshua D. Drake

On 01/20/2016 09:31 AM, Joel Jacobson wrote:

On Wed, Jan 20, 2016 at 5:29 PM, Andres Freund  wrote:

I think one thing we should work on, is being absolutely religious about
requiring, say, 2 reviews for every nontrivial contribution.  We
currently seem to have a significantly increased submission rate, and at
the same time the number of reviews per patch has gone down
substantially.  I think the "honor" system has failed in that regard.


Good point, totally agree.

So the project should try to think of new ideas on how to incentivise reviewing?

I have three ideas so far:


4. Submit a patch, review a patch.

Don't review patches? Don't submit patches.

JD

--
Command Prompt, Inc.  http://the.postgres.company/
 +1-503-667-4564
PostgreSQL Centered full stack support, consulting and development.


--
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] Releasing in September

2016-01-20 Thread Alvaro Herrera
Joshua D. Drake wrote:

> 4. Submit a patch, review a patch.
> 
> Don't review patches? Don't submit patches.

Here's one area where the commitfest app could help the CFM.  I would
like to have a report that shows, for each person, the list of patches
where they are author, and the list of patches where they are reviewer.
Then I can check his reviewer score by skimming a couple of threads and
say "you don't have enough author credits, go away".

-- 
Álvaro Herrerahttp://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] Releasing in September

2016-01-20 Thread Andres Freund
On 2016-01-20 19:00:05 +0100, Magnus Hagander wrote:
> And you want the actual patches, and not just a count?

I think the actual patches makes sense, because reviewing one 200KB
patch obviously is something different than reviewing 3 onelinesrs.


-- 
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] Releasing in September

2016-01-20 Thread Alvaro Herrera
Magnus Hagander wrote:
> On Jan 20, 2016 18:57, "Alvaro Herrera"  wrote:

> > Here's one area where the commitfest app could help the CFM.  I would
> > like to have a report that shows, for each person, the list of patches
> > where they are author, and the list of patches where they are reviewer.
> > Then I can check his reviewer score by skimming a couple of threads and
> > say "you don't have enough author credits, go away".
> 
> I assume you mean a summary review? You can already use the filter feature
> to get this information if you want to look at an individual contributor,
> but I'm guessing you're looking for a global overview?

Yes, a summary.  I tried using the filters but it's too cumbersome.

> And you want the actual patches, and not just a count?

Yes, the actual patches, so that I can go and see the threads.
Otherwise it's easy to mark oneself as reviewer and not actually review
anything ...

-- 
Álvaro Herrerahttp://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] Releasing in September

2016-01-20 Thread Andres Freund
On 2016-01-20 13:03:05 -0500, Tom Lane wrote:
> I am not sure what exactly ought to be different about them [small
> patches], but probably something should.  I think for small patches,
> we are using the CF app mostly to be sure things don't fall through
> the cracks, but maybe we don't need the whole process otherwise.

Aren't we already treating them differently on an ad-hoc basis? Several
committers fast-track such patches through the process. We could mark
them differently in the CF app, but I'm not sure what we'd want to
achieve by doing that. Such small patches are frequently enough
buggy. Committers can most of the time can easily polish them up, but
that still takes time; so I think tests/reviews of those are still
worthwhile.

So the only real difference I see an explicit difference would make is
making it easier to spot such patches when looking for an easy thing to
commit, and to make life for the commitfest manager easier. Not sure if
such a labeling would save more time than doing the labeling itself
would cost.

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] Releasing in September

2016-01-20 Thread Magnus Hagander
On Jan 20, 2016 5:03 PM, "Andres Freund"  wrote:
>
> On 2016-01-20 10:55:07 -0500, Robert Haas wrote:
> > It's certainly true that we twiddled our thumbs quite a bit about
> > getting 9.5 ready to ship.  However, the old process where nobody
> > could get anything committed for six months out of the year blew
> > chunks, too.  Personally, I think that the solution is to cut off the
> > last CommitFest a lot sooner, and then reopen the tree for the next
> > release as soon as possible.  But this never works, because there are
> > always patches we want to slip in late.
>
> Said twiddling seems to largely happened from July to December. In which
> the other branch was open, and no 9.5 commitfest was happening. If we
> move the commitfests to earlier, but still have a half year of nothing
> happening, we're still in a bad situation.
>
> FWIW, looking at the last few commitfests, aside heroic and
> unsustainable efforts by individual CF managers, I haven't noticed any
> effect of when fests started/stopped. Aside from a short time increase
> in unfinished patches being posted the day before the next CFs starts.

Yeah, we seem to be firmly stuck at two month long commitfests started
every two months. The plan was for them to be one month..

Maybe we should try just very drastically cutting them at one month and
bumping everything left. No questions asked, no extra time for anybody.
Regardless of if it's the first or the last commitfest.

Just to see what happens. Because what we are doing now clearly doesn't
work..

/Magnus


Re: [HACKERS] Releasing in September

2016-01-20 Thread Tom Lane
Magnus Hagander  writes:
> On Jan 20, 2016 5:03 PM, "Andres Freund"  wrote:
>> FWIW, looking at the last few commitfests, aside heroic and
>> unsustainable efforts by individual CF managers, I haven't noticed any
>> effect of when fests started/stopped. Aside from a short time increase
>> in unfinished patches being posted the day before the next CFs starts.

> Yeah, we seem to be firmly stuck at two month long commitfests started
> every two months. The plan was for them to be one month..

> Maybe we should try just very drastically cutting them at one month and
> bumping everything left. No questions asked, no extra time for anybody.
> Regardless of if it's the first or the last commitfest.

> Just to see what happens. Because what we are doing now clearly doesn't
> work..

I do not think commitfest length is the problem (though surely it's not
working as intended).  What happened with 9.5 is we forked the 9.6
development branch on June 30th, with the expectation of releasing in
September, and then couldn't release in September because nobody had done
any significant amount of stabilization work.  Instead we had the 2015-07
commitfest.  And the 2015-09 commitfest.  And the 2015-11 commitfest.

We will not get back to on-schedule releases unless we can keep -hackers
working on release testing/stabilization when it's time to do that,
rather than being distracted by shiny new stuff going into the next
release.

I'd suggest that the easiest process fix is to not fork a new devel branch
until we reach, say, RC status.

And that does mean losing at least one commitfest per release cycle ---
but it would be the currently-first one, not the currently-last one.

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] Releasing in September

2016-01-20 Thread Robert Haas
On Wed, Jan 20, 2016 at 11:19 AM, Tom Lane  wrote:
> I do not think commitfest length is the problem (though surely it's not
> working as intended).  What happened with 9.5 is we forked the 9.6
> development branch on June 30th, with the expectation of releasing in
> September, and then couldn't release in September because nobody had done
> any significant amount of stabilization work.  Instead we had the 2015-07
> commitfest.  And the 2015-09 commitfest.  And the 2015-11 commitfest.

But I'm not very sure that we're talking about the same set of people
here.  If we're going to go to a system where nobody's allowed to
commit anything for the next release until the current release is
finalized, then we'd better have some procedure for making sure that
happens relatively quickly.  And the procedure can't be that the
people who are hot to get started on the next release have to bat
cleanup for the people who don't have time to fix the bugs they
introduced previously.  Because *that* would be manifestly unfair.

-- 
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] Releasing in September

2016-01-20 Thread Andres Freund
> I admit, I may have grabbed your comment out of an unrelated portion
> of the thread.

Ceterum censeo Carthaginem esse delendam.


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


Re: [HACKERS] Proposal for UPDATE: do not insert new tuple on heap if update does not change data

2016-01-20 Thread Kevin Grittner
On Wed, Jan 20, 2016 at 3:55 AM, Gasper Zejn  wrote:

> I was wondering if PostgreSQL adds new tuple if data is not changed
> when using UPDATE. It turns out it does add them and I think it might
> be beneficial not to add a new tuple in this case, since it causes a
> great deal of maintenance: updating indexes, vacuuming table and
> index, also heap fragmentation.

If you have one or more tables on which you routinely updated rows
to the values they already have, you might want to attach an update
trigger using the suppress_redundant_updates_trigger() function.

http://www.postgresql.org/docs/current/interactive/functions-trigger.html

A better solution, where possible, is to use the WHERE clause to
avoid the update attempt where the new values are not distinct from
the old ones.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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


Re: [HACKERS] Releasing in September

2016-01-20 Thread Alvaro Herrera
Magnus Hagander wrote:

> Except that we need to do it for the last one as well. With the only
> exception that the last one might be a bit longer. But the fact is that the
> recent of CFs *and* releases, we've taken the approach of closing the CF
> when everything in it is done or explicitly reviewed-and-bumped, and tried
> really really hard not to bump things because nobody had time to look at
> them. That's what I'm suggesting we change, and actually just cut them.
> Yes, one of the reasons for the CFs was to allow people a fair chance to
> get reviewed.But given that there isn't actually a deadline in practice
> doesn't help with that.

We have a rule in the commitfest process, which I mentioned in some
other thread yesterday, which we have never enforced as far as I
remember -- which is that the last commitfest only accepts patches that
have already been submitted to previous commitfests.  If we were to
enforce that one, I think we put a limit to the size of the last
commitfest, reducing the amount of work necessary to close it.

I definitely think we should be stricter about closing each commitfest
on the last day of the month.  To improve on the fairness front we could
also be stricter about giving priority to reviewing patches that have
been punted from previous commitfest.

-- 
Álvaro Herrerahttp://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] Releasing in September

2016-01-20 Thread Joshua D. Drake

On 01/20/2016 09:48 AM, David E. Wheeler wrote:

On Jan 20, 2016, at 9:42 AM, Joshua D. Drake  wrote:


4. Submit a patch, review a patch.

Don't review patches? Don't submit patches.


There will always be patches desirable-enough that they will be reviewed 
whether or not the submitter reviewed other patches.



Correct.


And there will often be patches that generate so little interest that they’ll 
never be reviewed no matter how many other patches the submitter reviews.



Correct.


That said, it’s not a bad heuristic, and I suspect that someone who reviews 
patches is more likely to get their patch reviewed. But obviously there are no 
guarantees.


Well a good portion of humanity is built on karma. I do for you, you do 
for me. It isn't explicit, that is just bad taste but there is an 
implicit desire that gets built up to help people when you are also 
being helped.


Sincerely,

JD



--
Command Prompt, Inc.  http://the.postgres.company/
 +1-503-667-4564
PostgreSQL Centered full stack support, consulting and development.


--
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] Releasing in September

2016-01-20 Thread Joel Jacobson
On Wed, Jan 20, 2016 at 5:29 PM, Andres Freund  wrote:
> I think one thing we should work on, is being absolutely religious about
> requiring, say, 2 reviews for every nontrivial contribution.  We
> currently seem to have a significantly increased submission rate, and at
> the same time the number of reviews per patch has gone down
> substantially.  I think the "honor" system has failed in that regard.

Good point, totally agree.

So the project should try to think of new ideas on how to incentivise reviewing?

I have three ideas so far:

1. The way I see it, the honor system is based on being mentioned
in the commit message and the release notes.

Authors are always mentioned in the release notes,
but I see reviewers are mostly only mentioned in the commit messages.

Maybe more skilled developers would think it's cool to do reviewing
if they were "paid" by also being mentioned in the release notes?

At least some skilled people would probably be motivated by it,
in addition to the good feeling of doing something just because it's
fun or important.

2. Currently "Needs review" can mean a patch is in any of the phases
(Submission -> Usability -> Feature test -> Performance review ->
Coding review -> Architecture review -> Review review).
If you as a reviewer don't even know if the patch will be accepted and
committed by a committer,
there is a risk review-time will be wasted until the patch has been
accepted by the community.

I suggest we inject a new intermediate optional step "Needs Code
Review" to mean the feature has been discussed on pghackers
and there is a consensus among committers that they at least agree the
feature is something desired,
and that someone has at least reviewed the patch applies and the
feature works like expected.
And maybe renaming the "Needs review" step to "Needs Usability Review"
(or some other word to capture the Submission -> Usability -> Feature
test -> Performance review, phase before the Code review phase).

Then you as a reviewer would be confident the feature will get
accepted as long as the code is correct,
and the time you spent on code-reviewing will not be wasted.

3. For those who are partly motivated by money,
maybe this idea would work too to some extent:

Since Trustly's Bug Country program [1] is aimed at motivating people
to find bugs in HEAD hasn't produced a single report so far,
maybe we should extend it to also cover reviews of commits
marked as "Ready For Committer" that shed light on a problem
that could have caused data corruption,
preventing a bad commit from being committed.

If we suddenly get hundreds of new reviewers who find hundreds of bugs
in uncommitted code, then maybe we have to change the terms again,
but I doubt neither of those will happen.

If people think this is a good idea, I can discuss internally if we can change
the conditions of the bug country program accordingly.

[1] https://trustly.com/se/postgresql-bug-bounty/


-- 
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] Releasing in September

2016-01-20 Thread Andres Freund
On 2016-01-20 09:48:24 -0800, David E. Wheeler wrote:
> There will always be patches desirable-enough that they will be reviewed 
> whether or not the submitter reviewed other patches.

I think that's actually getting less and less true. By now the really
desirable-enough features imply so much work by reviewer and committers,
over a prolonged time, that they're imo unlikely to be picked up just
because they're desirable, even when the author doesn't play by the
rules.  I don't think an exceptional case or two is particularly
important.


-- 
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] custom function for converting human readable sizes to bytes

2016-01-20 Thread Pavel Stehule
2016-01-19 4:45 GMT+01:00 Vitaly Burovoy :

> On 1/4/16, Pavel Stehule  wrote:
> > 2016-01-04 18:13 GMT+01:00 Shulgin, Oleksandr <
> oleksandr.shul...@zalando.de> :
> >> On Mon, Jan 4, 2016 at 6:03 PM, Pavel Stehule 
> wrote:
> >> > 2016-01-04 17:48 GMT+01:00 Shulgin, Oleksandr <
> oleksandr.shul...@zalando.de>:
> >> >> On Mon, Jan 4, 2016 at 4:51 PM, Robert Haas 
> wrote:
> >> >>
> >> >> I'm also inclined on dropping that explicit check for empty string
> >> >> below and let numeric_in() error out on that.  Does this look OK, or
> can
> >> >> it confuse someone:
> >> >> postgres=# select pg_size_bytes('');
> >> >> ERROR:  invalid input syntax for type numeric: ""
> >> >
> >> > both fixed
> >>
> >> Hm...
> >>
> >> > + switch (*strptr)
> >> > + {
> >> > + /* ignore plus symbol */
> >> > + case '+':
> >> > + case '-':
> >> > + *bufptr++ = *strptr++;
> >> > + break;
> >> > + }
> >>
> >> Well, to that amount you don't need any special checks, I'm just not
> sure
> >> if reported error message is not misleading if we let numeric_in()
> handle
> >> all the errors.  At least it can cope with the leading spaces, +/- and
> >> empty input quite well.
> >>
> >
> > I don't would to catch a exception from numeric_in - so I try to solve
> some
> > simple situations, where I can raise possible better error message.
>
> There are several cases where your behavior gives strange errors (see
> below).
>
> Next batch of notes:
>
> src/include/catalog/pg_proc.h:
> ---
> + DATA(insert OID = 3317 ( pg_size_bytes...
> now oid 3317 is used (by pg_stat_get_wal_receiver), 3318 is free
>

fixed


>
> ---
> + DESCR("convert a human readable text with size units to big int bytes");
> May be the best way is to copy the first sentence from the doc?
> ("convert a size in human-readable format with size units into bytes")
>

fixed


> 
> src/backend/utils/adt/dbsize.c:
> + text *arg = PG_GETARG_TEXT_PP(0);
> + char *str = text_to_cstring(arg);
> ...
> +   /* working buffer cannot be longer than original string */
> +   buffer = (char *) palloc(VARSIZE_ANY_EXHDR(arg) + 1);
> Is there any reason to get TEXT for only converting it to cstring and
> call VARSIZE_ANY_EXHDR instead of strlen?
>

performance - but these strings should be short, so I can use strlen

fixed


>
> ---
> +   text   *arg = PG_GETARG_TEXT_PP(0);
> +   char   *str = text_to_cstring(arg);
> +   char*strptr = str;
> +   char   *buffer;
> There are wrong offsets of variable names after their types (among all
> body of the "pg_size_bytes" function).
> See variable declarations in nearby functions (
>   >> "make the new code look like the existing code around it"
>   http://www.postgresql.org/docs/devel/static/source-format.html
> )
>
>
fixed


> ---
> +errmsg("\"%s\" is not number",
> str)));
> s/is not number/is not a number/
> (the second version can be found in a couple places besides translations)
>

fixed

but this message can be little bit no intuitive - better text is "is not a
valid number"


>
> ---
> +   if (*strptr != '\0')
> ...
> +   while (*strptr && !isspace(*strptr))
> Sometimes it explicitly compares to '\0', sometimes implicitly.
> Common use is explicit comparison and it is preferred due to different
> compilers (their conversions to boolean).
>

fixed

>
> ---
> +   /* Skip leading spaces */
> ...
> +   /* ignore plus symbol */
> ...
> +   /* copy digits to working buffer */
> ...
> +   /* allow whitespace between integer and unit */
> I'm also inclined on dropping that explicit skipping spaces, checking
> for +/- symbols, but copying all digits, spaces, dots and '+-' symbols
> and let numeric_in() error out on that.
>

This is difficult - you have to divide string to two parts and first world
is number, second world is unit.

For example "+912+ kB" is correct number +912 and broken unit "+ kB".


> It allows to get correct error messages for something like:
> postgres=# select pg_size_bytes('.+912');
> ERROR:  invalid input syntax for type numeric: ".+912"
> postgres=# select pg_size_bytes('+912+ kB');
> ERROR:  invalid input syntax for type numeric: "+912+ "
> postgres=# select pg_size_bytes('++123 kB');
> ERROR:  invalid input syntax for type numeric: "++123 "
>
> instead of current:
> postgres=# select pg_size_bytes('.+912');
> ERROR:  invalid input syntax for type numeric: "."
> postgres=# select pg_size_bytes('+912+ kB');
> ERROR:  invalid unit: "+ kB"
> postgres=# select pg_size_bytes('++123 kB');
> ERROR:  invalid input syntax for type numeric: "+"
>
>
I redesigned this check. Now it is

popostgres=# select pg_size_bytes('.+912');
ERROR:  22023: ".+912" is not a valid number
stgres=# select pg_size_bytes('++123 kB');
ERROR:  22023: "++123 kB" is not a valid number



> ---
> +  

Re: [HACKERS] Releasing in September

2016-01-20 Thread Simon Riggs
On 20 January 2016 at 15:40, Bruce Momjian  wrote:

> Many people where happy with our consistent releasing major releases in
> September, e.g. 9.0 to 9.3:
>

What is the point in having a special mailing list to discuss the release
schedule plus a face-to-face dev meeting to discuss this if we are going to
discuss it here?

ISTM the wrong starting point to discuss plans in an unplanned way and
assume that everyone has time to take part today, right now.

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: [HACKERS] Releasing in September

2016-01-20 Thread Andres Freund
On 2016-01-20 10:40:14 -0500, Bruce Momjian wrote:
> We have gotten off of that cycle in the last two major releases, and
> this isn't going to improve as long as we have commitfests starting
> after January.

I think this has very little to do with commitfest schedules, and much
more with the "early" forking of the new version branch. For both 9.4
and 9.5 we essentially spent a couple months twiddling our thumbs.


-- 
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] Releasing in September

2016-01-20 Thread Robert Haas
On Wed, Jan 20, 2016 at 10:48 AM, Andres Freund  wrote:
> On 2016-01-20 10:40:14 -0500, Bruce Momjian wrote:
>> We have gotten off of that cycle in the last two major releases, and
>> this isn't going to improve as long as we have commitfests starting
>> after January.
>
> I think this has very little to do with commitfest schedules, and much
> more with the "early" forking of the new version branch. For both 9.4
> and 9.5 we essentially spent a couple months twiddling our thumbs.

It's certainly true that we twiddled our thumbs quite a bit about
getting 9.5 ready to ship.  However, the old process where nobody
could get anything committed for six months out of the year blew
chunks, too.  Personally, I think that the solution is to cut off the
last CommitFest a lot sooner, and then reopen the tree for the next
release as soon as possible.  But this never works, because there are
always patches we want to slip in late.

-- 
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] Releasing in September

2016-01-20 Thread Joshua D. Drake

On 01/20/2016 09:17 AM, Andres Freund wrote:

On 2016-01-20 09:15:01 -0800, Joshua D. Drake wrote:

On 01/20/2016 09:03 AM, Andres Freund wrote:

If people don't fix the issues in time, there needs to be
direct pushback, leading to much less stuff getting in next time round.



We have been slowly moving to a more dictator based release anyway. It used
to be that we released "when it's done", then we moved to commitfest, and
now we are having yet another discussion on the same topic of how to manage
all of this.

It seems clear that we need to be able to say:

* "Your feature is awesome. Sorry, you are out of time"


That's not really related to the discussion here tho? The debated
problem is things that have already been committed that have have bugs,
preventing beta/rc releases from being made. In some cases in the last
releases such bugs were in features integrated very early in the cycle.


Sorry if I was causing noise. I was under the impression that the 
discussion was more than just things that already have bugs, but also 
how the commitfests were working (scheduling etc..). I admit, I may have 
grabbed your comment out of an unrelated portion of the thread.


JD


--
Command Prompt, Inc.  http://the.postgres.company/
 +1-503-667-4564
PostgreSQL Centered full stack support, consulting and development.


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


Re: [HACKERS] Proposal for UPDATE: do not insert new tuple on heap if update does not change data

2016-01-20 Thread Konstantin Knizhnik

Hi,

To eliminate creation of new tuple version in this case it is necessary 
to check that update actually doesn't change the record.
It is not a cheapest test and it seems to be not so good idea to perform 
it always.
But if you fill that in your case there are many "identical" updates, 
you can always explicitly rewrite query by adding extra check:


UPDATE foo SET val = 'second' where pk = 2 and val <> 'second';




On 20.01.2016 12:55, Gasper Zejn wrote:

Hi,

I was wondering if PostgreSQL adds new tuple if data is not changed
when using UPDATE. It turns out it does add them and I think it might
be beneficial not to add a new tuple in this case, since it causes a
great deal of maintenance: updating indexes, vacuuming table and
index, also heap fragmentation.

How to check:

CREATE TABLE foo (pk serial primary key, val text);
-- Starting point: two rows.
INSERT INTO foo VALUES (1, 'first');
INSERT INTO foo VALUES (2, 'second');
CHECKPOINT;

-- Updating row with same value.
UPDATE foo SET val = 'second' where pk = 2;
CHECKPOINT;

-- "Upsert" is the same.
INSERT INTO foo VALUES (2, 'second') ON CONFLICT (pk) DO UPDATE SET
val = 'second';
CHECKPOINT;

If after any checkpoint you look at page data, you can see multiple
versions of same row with "second".

Unfortunately, I don't believe I can come up with a patch on my own,
but will happily offer any further help with testing and ideas.


Attached is a script with minimal test case.

Kind regards,
Gasper Zejn




--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company



Re: [HACKERS] Releasing in September

2016-01-20 Thread Bruce Momjian
On Wed, Jan 20, 2016 at 07:53:25AM -0800, Joshua Drake wrote:
> #2 This is a longer topic. I have been stewing in my head about
> releases for years. I have even brought up the idea of an Ubuntu
> style release cycle on list once or twice. The more I think about
> it, the more I think this can help our community. We essentially
> would have two types of releases:
> 
> STS:
> 
> * Supported for 1 release cycle plus 6 months (18-24 months)
> * Inline upgrades supported
> 
> LTS:
> 
> * Supported for standard 5 years
> * Is allowed to break binary format from STS but not previous LTS.
> This allows two LTS versions per 5 year support cycle

I just don't buy the Ubuntu release model for our database.  Ubuntu is
trying to balance hot features vs stability, while we are really focused
on stability, similar to Debian.

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

+ As you are, so once was I. As I am, so you will be. +
+ 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] Releasing in September

2016-01-20 Thread Andres Freund
On 2016-01-20 09:15:01 -0800, Joshua D. Drake wrote:
> On 01/20/2016 09:03 AM, Andres Freund wrote:
> >If people don't fix the issues in time, there needs to be
> >direct pushback, leading to much less stuff getting in next time round.
> >
> 
> We have been slowly moving to a more dictator based release anyway. It used
> to be that we released "when it's done", then we moved to commitfest, and
> now we are having yet another discussion on the same topic of how to manage
> all of this.
> 
> It seems clear that we need to be able to say:
> 
> * "Your feature is awesome. Sorry, you are out of time"

That's not really related to the discussion here tho? The debated
problem is things that have already been committed that have have bugs,
preventing beta/rc releases from being made. In some cases in the last
releases such bugs were in features integrated very early in the cycle.


-- 
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] checkpointer continuous flushing

2016-01-20 Thread Alvaro Herrera
Andres Freund wrote:

> The relevant thread is at
> http://archives.postgresql.org/message-id/CA%2BTgmoaCr3kDPafK5ygYDA9mF9zhObGp_13q0XwkEWsScw6h%3Dw%40mail.gmail.com
> what I didn't remember is that I voiced concern back then about exactly this:
> http://archives.postgresql.org/message-id/201112011518.29964.andres%40anarazel.de
> ;)

Interesting.  If we consider for a minute that part of the cause for the
slowdown is slowness in pg_clog, maybe we should reconsider the initial
decision to flush as quickly as possible (i.e. adopt a strategy where
walwriter sleeps a bit between two flushes) in light of the group-update
feature for CLOG being proposed by Amit Kapila in another thread -- it
seems that these things might go hand-in-hand.

-- 
Álvaro Herrerahttp://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


[HACKERS] Releasing in September

2016-01-20 Thread Bruce Momjian
Many people where happy with our consistent releasing major releases in
September, e.g. 9.0 to 9.3:

9.5   2016-01-07
9.4   2014-12-18
9.3   2013-09-09 <--
9.2   2012-09-10 <--
9.1   2011-09-12 <--
9.0   2010-09-20 <--
8.4   2009-07-01
8.3   2008-02-04
8.2   2006-12-05
8.1   2005-11-08
8.0   2005-01-19

We have gotten off of that cycle in the last two major releases, and
this isn't going to improve as long as we have commitfests starting
after January.  In the September-release years, we used to start
preparing for beta in mid-February or early March.  However, for 9.5 we
had our last commitfest _start_ in mid-February, and it didn't end until
mid-May.  For 9.6 we have a commitfest starting in March 1.  This will
never allow a September release.

Our current 9.5/9.6 timing looks more like the 8.X series of release
dates.  Everyone might be fine with that, but we had better be prepared
for November-February major release dates going forward.

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

+ As you are, so once was I. As I am, so you will be. +
+ 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] Releasing in September

2016-01-20 Thread Bruce Momjian
On Wed, Jan 20, 2016 at 11:53:32AM -0500, Robert Haas wrote:
> On Wed, Jan 20, 2016 at 11:19 AM, Tom Lane  wrote:
> > I do not think commitfest length is the problem (though surely it's not
> > working as intended).  What happened with 9.5 is we forked the 9.6
> > development branch on June 30th, with the expectation of releasing in
> > September, and then couldn't release in September because nobody had done
> > any significant amount of stabilization work.  Instead we had the 2015-07
> > commitfest.  And the 2015-09 commitfest.  And the 2015-11 commitfest.
> 
> But I'm not very sure that we're talking about the same set of people
> here.  If we're going to go to a system where nobody's allowed to
> commit anything for the next release until the current release is
> finalized, then we'd better have some procedure for making sure that
> happens relatively quickly.  And the procedure can't be that the
> people who are hot to get started on the next release have to bat
> cleanup for the people who don't have time to fix the bugs they
> introduced previously.  Because *that* would be manifestly unfair.

Unfair or not, that is probably the effect.  I can also see people
publishing git trees to avoid this roadblock.

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

+ As you are, so once was I. As I am, so you will be. +
+ 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] Releasing in September

2016-01-20 Thread Andres Freund
On 2016-01-20 11:53:32 -0500, Robert Haas wrote:
> But I'm not very sure that we're talking about the same set of people
> here.  If we're going to go to a system where nobody's allowed to
> commit anything for the next release until the current release is
> finalized, then we'd better have some procedure for making sure that
> happens relatively quickly.  And the procedure can't be that the
> people who are hot to get started on the next release have to bat
> cleanup for the people who don't have time to fix the bugs they
> introduced previously.  Because *that* would be manifestly unfair.

Yes, that's a problem. But I don't see how we can avoid dealing with it
directly. Avoiding being stalled by unfixed bugs of others by having a
separate branch to continue working, inevitably leads to delayed
releases.  If people don't fix the issues in time, there needs to be
direct pushback, leading to much less stuff getting in next time round.


-- 
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] Releasing in September

2016-01-20 Thread Tom Lane
Robert Haas  writes:
> But I'm not very sure that we're talking about the same set of people
> here.  If we're going to go to a system where nobody's allowed to
> commit anything for the next release until the current release is
> finalized, then we'd better have some procedure for making sure that
> happens relatively quickly.  And the procedure can't be that the
> people who are hot to get started on the next release have to bat
> cleanup for the people who don't have time to fix the bugs they
> introduced previously.  Because *that* would be manifestly unfair.

You're assuming that the people who are hot to get started on the next
release are different from the people who don't have time to fix the bugs
they introduced previously.  IME it's mostly the same people.

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] Releasing in September

2016-01-20 Thread Tom Lane
Magnus Hagander  writes:
> On Wed, Jan 20, 2016 at 5:19 PM, Tom Lane  wrote:
>> I do not think commitfest length is the problem (though surely it's not
>> working as intended).  What happened with 9.5 is we forked the 9.6

> I agree that it's not the same problem. I do believe that it is *a* problem
> though, and a fairly significant one too. Because there's *never* any
> downtime from CF mode, regardless of where in the cycle we are.

True, we've been failing badly on the intention that there would be time
off from CF mode, and I'd like to see a fix for that.  I do not think it's
directly related to the can't-get-a-release-out problem.

I'm not really sure why we've allowed CFs to drift on, though.  Can't we
just arbitrarily decree them closed on the last day of the month?  And
push unfinished work to the next one?  Admittedly, this probably doesn't
work for the last CF of a release cycle, but that one's always been a
special case.

regards, tom lane


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


Re: [HACKERS] Releasing in September

2016-01-20 Thread Andres Freund
On 2016-01-20 12:18:28 -0500, Tom Lane wrote:
> I'm not really sure why we've allowed CFs to drift on, though.  Can't we
> just arbitrarily decree them closed on the last day of the month?  And
> push unfinished work to the next one?  Admittedly, this probably doesn't
> work for the last CF of a release cycle, but that one's always been a
> special case.

I think the problem there is the assumption that each CF entry deserves
some amount of feedback. It's not a big problem to close entries of
patches that have gotten a fair amount, but it's pretty common to have a
set of entries that have barely gotten any review. Usually either
because they're perceived as too boring (fair enough), because they're
too complex for the majority of non-busy people (uhhh), or because
they're seen as claimed.

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] Releasing in September

2016-01-20 Thread Magnus Hagander
On Wed, Jan 20, 2016 at 6:18 PM, Tom Lane  wrote:

> Magnus Hagander  writes:
> > On Wed, Jan 20, 2016 at 5:19 PM, Tom Lane  wrote:
> >> I do not think commitfest length is the problem (though surely it's not
> >> working as intended).  What happened with 9.5 is we forked the 9.6
>
> > I agree that it's not the same problem. I do believe that it is *a*
> problem
> > though, and a fairly significant one too. Because there's *never* any
> > downtime from CF mode, regardless of where in the cycle we are.
>
> True, we've been failing badly on the intention that there would be time
> off from CF mode, and I'd like to see a fix for that.  I do not think it's
> directly related to the can't-get-a-release-out problem.
>

In a way you could say they are two symptoms of the same underlying
problem, being that we've partially lost control over our development and
release schedule.




> I'm not really sure why we've allowed CFs to drift on, though.  Can't we
> just arbitrarily decree them closed on the last day of the month?  And
> push unfinished work to the next one?  Admittedly, this probably doesn't
> work for the last CF of a release cycle, but that one's always been a
> special case.
>

That's pretty much what I suggested :)

Except that we need to do it for the last one as well. With the only
exception that the last one might be a bit longer. But the fact is that the
recent of CFs *and* releases, we've taken the approach of closing the CF
when everything in it is done or explicitly reviewed-and-bumped, and tried
really really hard not to bump things because nobody had time to look at
them. That's what I'm suggesting we change, and actually just cut them.
Yes, one of the reasons for the CFs was to allow people a fair chance to
get reviewed.But given that there isn't actually a deadline in practice
doesn't help with that.

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


Re: [HACKERS] checkpointer continuous flushing

2016-01-20 Thread Andres Freund
On 2016-01-20 12:16:24 -0300, Alvaro Herrera wrote:
> Andres Freund wrote:
> 
> > The relevant thread is at
> > http://archives.postgresql.org/message-id/CA%2BTgmoaCr3kDPafK5ygYDA9mF9zhObGp_13q0XwkEWsScw6h%3Dw%40mail.gmail.com
> > what I didn't remember is that I voiced concern back then about exactly 
> > this:
> > http://archives.postgresql.org/message-id/201112011518.29964.andres%40anarazel.de
> > ;)
> 
> Interesting.  If we consider for a minute that part of the cause for the
> slowdown is slowness in pg_clog, maybe we should reconsider the initial
> decision to flush as quickly as possible (i.e. adopt a strategy where
> walwriter sleeps a bit between two flushes) in light of the group-update
> feature for CLOG being proposed by Amit Kapila in another thread -- it
> seems that these things might go hand-in-hand.

I don't think it's strongly related - the contention here is on read
access to the clog, not on write access. While Amit's patch will reduce
the impact of that a bit, I don't see it making a fundamental
difference.

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] Releasing in September

2016-01-20 Thread Joshua D. Drake

On 01/20/2016 09:03 AM, Andres Freund wrote:

If people don't fix the issues in time, there needs to be
direct pushback, leading to much less stuff getting in next time round.



We have been slowly moving to a more dictator based release anyway. It 
used to be that we released "when it's done", then we moved to 
commitfest, and now we are having yet another discussion on the same 
topic of how to manage all of this.


It seems clear that we need to be able to say:

* "Your feature is awesome. Sorry, you are out of time"

Life is hard sometimes, we don't always get what we want.

Sincerely,

JD



--
Command Prompt, Inc.  http://the.postgres.company/
 +1-503-667-4564
PostgreSQL Centered full stack support, consulting and development.


--
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] Set search_path + server-prepared statements = cached plan must not change result type

2016-01-20 Thread Vladimir Sitnikov
>  I believe, and the conclusion was that
>if you think you need this, you're doing it wrong

So what is the recommended approach to use server-prepared statements
at the client side (I mean at JDBC driver side)?

Currently "prepare, switch search_path, execute" leads to "cached plan
must not change result type" error.
Can one expect the issue to be fixed in subsequent 8.4, 8.5, ..., 9.5 versions?

Vladimir


-- 
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] Combining Aggregates

2016-01-20 Thread Robert Haas
On Wed, Jan 20, 2016 at 7:53 AM, David Rowley
 wrote:
> On 21 January 2016 at 01:44, Robert Haas  wrote:
>>
>> On Wed, Jan 20, 2016 at 7:38 AM, David Rowley
>>  wrote:
>> >> To my mind, priority #1 ought to be putting this fine new
>> >> functionality to some use.  Expanding it to every aggregate we've got
>> >> seems like a distinctly second priority.  That's not to say that it's
>> >> absolutely gotta go down that way, but those would be my priorities.
>> >
>> > Agreed. So I've attached a version of the patch which does not have any
>> > of
>> > the serialise/deserialise stuff in it.
>> >
>> > I've also attached a test patch which modifies the grouping planner to
>> > add a
>> > Partial Aggregate node, and a final aggregate node when it's possible.
>> > Running the regression tests with this patch only shows up variances in
>> > the
>> > EXPLAIN outputs, which is of course expected.
>>
>> That seems great as a test, but what's the first patch that can put
>> this to real and permanent use?
>
> There's no reason why parallel aggregates can't use the
> combine_aggregate_state_d6d480b_2016-01-21.patch patch.

I agree.  Are you going to work on that?  Are you expecting me to work
on that?  Do you think we can use Haribabu's patch?  What other
applications are in play in the near term, if any?

-- 
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] Releasing in September

2016-01-20 Thread Bruce Momjian
On Wed, Jan 20, 2016 at 10:55:07AM -0500, Robert Haas wrote:
> On Wed, Jan 20, 2016 at 10:48 AM, Andres Freund  wrote:
> > I think this has very little to do with commitfest schedules, and much
> > more with the "early" forking of the new version branch. For both 9.4
> > and 9.5 we essentially spent a couple months twiddling our thumbs.
> 
> It's certainly true that we twiddled our thumbs quite a bit about
> getting 9.5 ready to ship.  However, the old process where nobody
> could get anything committed for six months out of the year blew
> chunks, too.  Personally, I think that the solution is to cut off the
> last CommitFest a lot sooner, and then reopen the tree for the next
> release as soon as possible.  But this never works, because there are
> always patches we want to slip in late.

The bottom line is we can't sustain five commitfests and stay on time
--- we need to go back to four, which I think is what we used to do.  We
twiddled our thumbs back in the September-release years too, but had
consistency because twiddling was built into the schedule.

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

+ As you are, so once was I. As I am, so you will be. +
+ 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] custom function for converting human readable sizes to bytes

2016-01-20 Thread Pavel Stehule
Hi

2016-01-19 0:56 GMT+01:00 Vitaly Burovoy :

> On 1/4/16, Robert Haas  wrote:
> > On Mon, Jan 4, 2016 at 10:17 AM, Pavel Stehule 
> > wrote:
> >> [ new patch ]
> >
> > + case '-':
> > + ereport(ERROR,
> > + (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> > +  errmsg("size cannot be negative")));
> >
> > Why not?  I bet if you copy any - sign to the buffer, this will Just
> Work.
>
> Hmm. The function's name is pg_size_bytes. How number of bytes can be
> negative? How any length can be negative? If anyone insert '-' sign to
> an argument, it is copy-paste error. I don't see any case where there
> is possible negatives as input value.
>
> I prefer error message instead of getting all relations (by using
> comparison from the initial letter) just because of copy-paste mistake
> or incomplete checking of input values at app-level.
>

the last version of this patch support negative numbers.

Regards

Pavel


>
> > ...
> >
> > --
> > Robert Haas
> > EnterpriseDB: http://www.enterprisedb.com
> > The Enterprise PostgreSQL Company
>
> --
> Best regards,
> Vitaly Burovoy
>


Re: [HACKERS] Releasing in September

2016-01-20 Thread Joshua D. Drake

On 01/20/2016 08:51 AM, Bruce Momjian wrote:

On Wed, Jan 20, 2016 at 07:53:25AM -0800, Joshua Drake wrote:

#2 This is a longer topic. I have been stewing in my head about
releases for years. I have even brought up the idea of an Ubuntu
style release cycle on list once or twice. The more I think about
it, the more I think this can help our community. We essentially
would have two types of releases:

STS:

* Supported for 1 release cycle plus 6 months (18-24 months)
* Inline upgrades supported

LTS:

* Supported for standard 5 years
* Is allowed to break binary format from STS but not previous LTS.
This allows two LTS versions per 5 year support cycle


I just don't buy the Ubuntu release model for our database.  Ubuntu is
trying to balance hot features vs stability, while we are really focused
on stability, similar to Debian.


I understand but I think we are missing out on an opportunity here. 
Notice that the shorter release cycle for STS will actually make some 
things easier. Including:


 * Increased test base (just like Fedora/Ubuntu)
 * Increased early adopter testing (that is what early adopting is 
really about for us anyway)

 * Decreased concerns about upgrades and ability to extend upgrade status.

I am not in any way suggesting that this is a slam dunk but I do think 
something along these lines can really bring forth a lot of 
possibilities. Here is another example:


pg_audit was pulled from core, did it have to be? I can't say. I didn't 
read the code. I was a proactive member stating we could pull it though 
because it was an extension.


However, if it was an STS release it was going into we could be a little 
more relaxed and say, "O.k. let's get this into the wild for some 
general user testing".


There are a lot of people (think about the multixact problem) that will 
run any software because it is new. Let's put the proper disclaimers on 
there and let them run it.


Sincerely,

Joshua D. Drake






--
Command Prompt, Inc.  http://the.postgres.company/
 +1-503-667-4564
PostgreSQL Centered full stack support, consulting and development.


--
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] Releasing in September

2016-01-20 Thread Magnus Hagander
On Wed, Jan 20, 2016 at 5:19 PM, Tom Lane  wrote:

> Magnus Hagander  writes:
> > On Jan 20, 2016 5:03 PM, "Andres Freund"  wrote:
> >> FWIW, looking at the last few commitfests, aside heroic and
> >> unsustainable efforts by individual CF managers, I haven't noticed any
> >> effect of when fests started/stopped. Aside from a short time increase
> >> in unfinished patches being posted the day before the next CFs starts.
>
> > Yeah, we seem to be firmly stuck at two month long commitfests started
> > every two months. The plan was for them to be one month..
>
> > Maybe we should try just very drastically cutting them at one month and
> > bumping everything left. No questions asked, no extra time for anybody.
> > Regardless of if it's the first or the last commitfest.
>
> > Just to see what happens. Because what we are doing now clearly doesn't
> > work..
>
> I do not think commitfest length is the problem (though surely it's not
> working as intended).  What happened with 9.5 is we forked the 9.6
>


I agree that it's not the same problem. I do believe that it is *a* problem
though, and a fairly significant one too. Because there's *never* any
downtime from CF mode, regardless of where in the cycle we are.

While not the same, we need to fix both.

We will not get back to on-schedule releases unless we can keep -hackers
> working on release testing/stabilization when it's time to do that,
> rather than being distracted by shiny new stuff going into the next
> release.
>

Agreed.

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


Re: [HACKERS] Releasing in September

2016-01-20 Thread Andres Freund
On 2016-01-20 10:55:07 -0500, Robert Haas wrote:
> It's certainly true that we twiddled our thumbs quite a bit about
> getting 9.5 ready to ship.  However, the old process where nobody
> could get anything committed for six months out of the year blew
> chunks, too.  Personally, I think that the solution is to cut off the
> last CommitFest a lot sooner, and then reopen the tree for the next
> release as soon as possible.  But this never works, because there are
> always patches we want to slip in late.

Said twiddling seems to largely happened from July to December. In which
the other branch was open, and no 9.5 commitfest was happening. If we
move the commitfests to earlier, but still have a half year of nothing
happening, we're still in a bad situation.

FWIW, looking at the last few commitfests, aside heroic and
unsustainable efforts by individual CF managers, I haven't noticed any
effect of when fests started/stopped. Aside from a short time increase
in unfinished patches being posted the day before the next CFs starts.

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] Releasing in September

2016-01-20 Thread Andres Freund
On 2016-01-20 11:19:28 -0500, Tom Lane wrote:
> We will not get back to on-schedule releases unless we can keep -hackers
> working on release testing/stabilization when it's time to do that,
> rather than being distracted by shiny new stuff going into the next
> release.

Agreed. I'll note that the linux kernel used to have a similar
problem. Now there's a two week integration and a 10 week stabilization
period, with occasionally a week added/subtracted. If a feature patch is
submitted for integration (not just parallel review) into the main
branch outside the merge window you get yelled at.  Now, we're at least
two magnitudes smaller, so not everything there applies to us. But I
think looking over that fence, to see how they scaled from smaller where
we are now, to way way bigger, isn't a bad idea.


I think one thing we should work on, is being absolutely religious about
requiring, say, 2 reviews for every nontrivial contribution.  We
currently seem to have a significantly increased submission rate, and at
the same time the number of reviews per patch has gone down
substantially.  I think the "honor" system has failed in that regard.


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] Releasing in September

2016-01-20 Thread Simon Riggs
On 20 January 2016 at 15:55, Robert Haas  wrote:

> On Wed, Jan 20, 2016 at 10:48 AM, Andres Freund 
> wrote:
> > On 2016-01-20 10:40:14 -0500, Bruce Momjian wrote:
> >> We have gotten off of that cycle in the last two major releases, and
> >> this isn't going to improve as long as we have commitfests starting
> >> after January.
> >
> > I think this has very little to do with commitfest schedules, and much
> > more with the "early" forking of the new version branch. For both 9.4
> > and 9.5 we essentially spent a couple months twiddling our thumbs.
>
> It's certainly true that we twiddled our thumbs quite a bit about
> getting 9.5 ready to ship.  However, the old process where nobody
> could get anything committed for six months out of the year blew
> chunks, too.  Personally, I think that the solution is to cut off the
> last CommitFest a lot sooner, and then reopen the tree for the next
> release as soon as possible.


+1


> But this never works, because there are
> always patches we want to slip in late.
>

I haven't seen that in the last few years, apart from maybe JSONB. Our
views on the state of patches differ, mainly because we've not all reviewed
every patch, so its hard to say whether a commit has been brewing for ages
and is right, or not.

Some bad stuff happened in 9.3 and we were scared to release early. Let's
get back on track and release on time (Sept).

The main problem is the length of the integration phase, which is mostly
where nothing happens. We need to manage that process just as we do with
CFs.

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: [HACKERS] multivariate statistics v8

2016-01-20 Thread Robert Haas
On Wed, Dec 23, 2015 at 2:07 PM, Tomas Vondra
 wrote:
>The remaining question is how unique the statistics name should be.
>My initial plan was to make it unique within a table, but that of
>course does not work well with the DROP STATISTICS (it'd have to
>specify the table name also), and it'd also now work with statistics
>on multiple tables (which is one of the reasons for abandoning ALTER
>TABLE stuff).
>
>So I think it should be unique across tables. Statistics are hardly
>a global object, so it should be unique within a schema. I thought
>that simply using the schema of the table would work, but that of
>course breaks with multiple tables in different schemas. So the only
>solution seems to be explicit schema for statistics.

That solution seems good to me.

(with apologies for not having looked at the rest of this much at all)

-- 
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] Releasing in September

2016-01-20 Thread Simon Riggs
On 20 January 2016 at 19:45, Robert Haas  wrote:

> On Wed, Jan 20, 2016 at 2:26 PM, Tom Lane  wrote:
> > and...@anarazel.de (Andres Freund) writes:
> >> On 2016-01-20 18:53:54 +, Simon Riggs wrote:
> >>> What is the point in having a special mailing list to discuss the
> release
> >>> schedule plus a face-to-face dev meeting to discuss this if we are
> going to
> >>> discuss it here?
> >
> >> That list exists to discuss concrete releases, and is separate so we
> >> e.g. can mention there's security issues and such. This topic in
> >> contrast quite validly merits input from more then the usual suspects
> >> going to a developer meeting.
> >
> > In particular, we have several important people who won't be in Brussels
> > next week, so it's appropriate to hear their thoughts beforehand ...
>
> As somebody who definitely won't be there, +1 from me.  :-)


OK, sorry, I forgot for a moment that you aren't going to be there.

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: Odd behavior in foreign table modification (Was: Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW)

2016-01-20 Thread Robert Haas
On Wed, Jan 20, 2016 at 4:50 AM, Etsuro Fujita
 wrote:
> My concern about that is that would make the code in deparseTargetList()
> complicated.
>
> Essentially, I think your propossal needs a two-pass algorithm for
> deparseTargetList; (1) create an integer List of the columns being retrieved
> from the given attrs_used (getRetrievedAttrs()), and (2) print those columns
> (printRetrievedAttrs()).  How about sharing those two functions between
> deparseTargetList and deparseReturningList?:

I don't see why we'd need that.  I adjusted the code in postgres_fdw
along the lines I had in mind and am attaching the result.  It doesn't
look complicated to me, and it passes the regression test you wrote.

By the way, I'm not too sure I understand the need for the core
changes that are part of this patch, and I think that point merits
some discussion.  Whenever you change core like this, you're changing
the contract between the FDW and core; it's not just postgres_fdw that
needs updating, but every FDW.  So we'd better be pretty sure we need
these changes and they are adequately justified before we think about
putting them into the tree.  Are these core changes really needed
here, or can we fix this whole issue in postgres_fdw and leave the
core code alone?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index e59af2c..12a1031 100644
--- a/contrib/postgres_fdw/deparse.c
+++ b/contrib/postgres_fdw/deparse.c
@@ -110,6 +110,7 @@ static void deparseTargetList(StringInfo buf,
   PlannerInfo *root,
   Index rtindex,
   Relation rel,
+  bool is_returning,
   Bitmapset *attrs_used,
   List **retrieved_attrs);
 static void deparseReturningList(StringInfo buf, PlannerInfo *root,
@@ -724,7 +725,7 @@ deparseSelectSql(StringInfo buf,
 	 * Construct SELECT list
 	 */
 	appendStringInfoString(buf, "SELECT ");
-	deparseTargetList(buf, root, baserel->relid, rel, attrs_used,
+	deparseTargetList(buf, root, baserel->relid, rel, false, attrs_used,
 	  retrieved_attrs);
 
 	/*
@@ -738,7 +739,8 @@ deparseSelectSql(StringInfo buf,
 
 /*
  * Emit a target list that retrieves the columns specified in attrs_used.
- * This is used for both SELECT and RETURNING targetlists.
+ * This is used for both SELECT and RETURNING targetlists; the is_returning
+ * parameter is true only for a RETURNING targetlist.
  *
  * The tlist text is appended to buf, and we also create an integer List
  * of the columns being retrieved, which is returned to *retrieved_attrs.
@@ -748,6 +750,7 @@ deparseTargetList(StringInfo buf,
   PlannerInfo *root,
   Index rtindex,
   Relation rel,
+  bool is_returning,
   Bitmapset *attrs_used,
   List **retrieved_attrs)
 {
@@ -777,6 +780,8 @@ deparseTargetList(StringInfo buf,
 		{
 			if (!first)
 appendStringInfoString(buf, ", ");
+			else if (is_returning)
+appendStringInfoString(buf, " RETURNING ");
 			first = false;
 
 			deparseColumnRef(buf, rtindex, i, root);
@@ -794,6 +799,8 @@ deparseTargetList(StringInfo buf,
 	{
 		if (!first)
 			appendStringInfoString(buf, ", ");
+		else if (is_returning)
+			appendStringInfoString(buf, " RETURNING ");
 		first = false;
 
 		appendStringInfoString(buf, "ctid");
@@ -803,7 +810,7 @@ deparseTargetList(StringInfo buf,
 	}
 
 	/* Don't generate bad syntax if no undropped columns */
-	if (first)
+	if (first && !is_returning)
 		appendStringInfoString(buf, "NULL");
 }
 
@@ -1022,11 +1029,8 @@ deparseReturningList(StringInfo buf, PlannerInfo *root,
 	}
 
 	if (attrs_used != NULL)
-	{
-		appendStringInfoString(buf, " RETURNING ");
-		deparseTargetList(buf, root, rtindex, rel, attrs_used,
+		deparseTargetList(buf, root, rtindex, rel, true, attrs_used,
 		  retrieved_attrs);
-	}
 	else
 		*retrieved_attrs = NIL;
 }
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index b471c67..b0d12ac 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -2408,6 +2408,59 @@ SELECT c1,c2,c3,c4 FROM ft2 ORDER BY c1;
  1104 | 204 | ddd| 
 (819 rows)
 
+EXPLAIN (verbose, costs off)
+INSERT INTO ft2 (c1,c2,c3) VALUES (,999,'foo') RETURNING tableoid::regclass;
+   QUERY PLAN
+-
+ Insert on public.ft2
+   Output: (tableoid)::regclass
+   Remote SQL: INSERT INTO "S 1"."T 1"("C 1", c2, c3, c4, c5, c6, c7, c8) VALUES ($1, $2, $3, $4, $5, $6, $7, $8)
+   ->  Result
+ Output: , 999, 

Re: [HACKERS] Combining Aggregates

2016-01-20 Thread Robert Haas
On Wed, Jan 20, 2016 at 7:38 AM, David Rowley
 wrote:
> Agreed. So I've attached a version of the patch which does not have any of
> the serialise/deserialise stuff in it.

I re-reviewed this and have committed most of it with only minor
kibitizing.  A few notes:

- I changed the EXPLAIN code, since it failed to display the aggregate
node's mode of operation in non-text format.

- I removed some of the planner support, since I'm not sure that it's
completely correct and I'm very sure that it contains more code
duplication than I'm comfortable with (set_combineagg_references in
particular).  Please feel free to resubmit this part, perhaps in
combination with code that actually uses it for something.  But please
also think about whether there's a way to reduce the duplication, and
maybe consider adding some comments, too.

- I'm not sure that you made the right call regarding reusing
transfn_oid and transfn for combine functions, vs. adding separate
fields.  It is sort of logical and has the advantage of keeping the
data structure smaller, but it might also be thought confusing or
inappropriate on other grounds.  But I think we can change it later if
that comes to seem like the right thing to do, so I've left it the way
you had it for now.

-- 
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] Releasing in September

2016-01-20 Thread Tom Lane
and...@anarazel.de (Andres Freund) writes:
> On 2016-01-20 18:53:54 +, Simon Riggs wrote:
>> What is the point in having a special mailing list to discuss the release
>> schedule plus a face-to-face dev meeting to discuss this if we are going to
>> discuss it here?

> That list exists to discuss concrete releases, and is separate so we
> e.g. can mention there's security issues and such. This topic in
> contrast quite validly merits input from more then the usual suspects
> going to a developer meeting.

In particular, we have several important people who won't be in Brussels
next week, so it's appropriate to hear their thoughts beforehand ...

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] Releasing in September

2016-01-20 Thread Peter Geoghegan
On Wed, Jan 20, 2016 at 11:13 AM, Andres Freund  wrote:
> That list exists to discuss concrete releases, and is separate so we
> e.g. can mention there's security issues and such. This topic in
> contrast quite validly merits input from more then the usual suspects
> going to a developer meeting.

I am not on the release list.


-- 
Peter Geoghegan


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


Re: [HACKERS] Releasing in September

2016-01-20 Thread Andres Freund
On 2016-01-20 18:53:54 +, Simon Riggs wrote:
> What is the point in having a special mailing list to discuss the release
> schedule plus a face-to-face dev meeting to discuss this if we are going to
> discuss it here?

That list exists to discuss concrete releases, and is separate so we
e.g. can mention there's security issues and such. This topic in
contrast quite validly merits input from more then the usual suspects
going to a developer meeting.

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] Releasing in September

2016-01-20 Thread Simon Riggs
On 20 January 2016 at 16:29, Andres Freund  wrote:


> I think one thing we should work on, is being absolutely religious about
> requiring, say, 2 reviews for every nontrivial contribution.  We
> currently seem to have a significantly increased submission rate, and at
> the same time the number of reviews per patch has gone down
> substantially.  I think the "honor" system has failed in that regard.
>

It failed because its never mentioned or even attempted to be enforced.

Some way to balance submissions against reviews is now essential.

If we believe in "peer review", we need peers that review.

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: [HACKERS] Releasing in September

2016-01-20 Thread Simon Riggs
On 20 January 2016 at 15:40, Bruce Momjian  wrote:

> Many people were happy with our consistent releasing major releases in
> September, e.g. 9.0 to 9.3:
>
> 9.5   2016-01-07
> 9.4   2014-12-18
> 9.3   2013-09-09 <--
> 9.2   2012-09-10 <--
> 9.1   2011-09-12 <--
> 9.0   2010-09-20 <--
> 8.4   2009-07-01
> 8.3   2008-02-04
> 8.2   2006-12-05
> 8.1   2005-11-08
> 8.0   2005-01-19
>
> We have gotten off of that cycle in the last two major releases


Yes we have and I agree that it would be useful to return to it.


> For 9.6 we have a commitfest starting in March 1.  This will
> never allow a September release.
>

If that is the case, then its too late to change.

March - last CF
April - integration
May - release Beta
Sept - release Prod

It seems perfectly OK for me to have Beta start at beginning May and for us
to release in September.

If the process takes more than 5 months (Apr - Sept) then there's something
wrong with that process, rather than there being a problem with the
schedule.


> Our current 9.5/9.6 timing looks more like the 8.X series of release
> dates.  Everyone might be fine with that, but we had better be prepared
> for November-February major release dates going forward.
>

I don't mind what month we pick, as long as we stick to the schedule.

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: [HACKERS] Releasing in September

2016-01-20 Thread Gavin Flower

On 21/01/16 06:40, Tom Lane wrote:

Magnus Hagander  writes:

That's pretty much what I suggested :)
Except that we need to do it for the last one as well. With the only
exception that the last one might be a bit longer. But the fact is that the
recent of CFs *and* releases, we've taken the approach of closing the CF
when everything in it is done or explicitly reviewed-and-bumped, and tried
really really hard not to bump things because nobody had time to look at
them. That's what I'm suggesting we change, and actually just cut them.
Yes, one of the reasons for the CFs was to allow people a fair chance to
get reviewed.But given that there isn't actually a deadline in practice
doesn't help with that.

Yeah.  It's certainly unfair if someone's patch doesn't get reviewed,
but there are only 24 hours in a day, and we have a limited pool of
reviewer and committer manpower.  I think we just have to say that
sometimes life is unfair.

I think it might also be a good idea if we could somehow distinguish
"nobody had time for that yet" from "nobody is interested", with an eye
to flat-out rejecting patches that no one cares enough about to review.
Maybe we could reduce the workload by inventing some kind of up-front
filter whereby a patch isn't even a candidate to get reviewed unless, say,
at least two people besides the author say "this sounds like it's worth
pursuing".

One of the other things I do not like about the current CF process is that
it's created a default assumption that most submitted patches should get
accepted eventually.  It is *very* hard to reject a patch altogether in
the CF process: you more or less have to show cause why it should not get
accepted.  That default is backwards.  Maybe this isn't the process' fault
exactly, but it sure seems like that's how we approach patches now.

regards, tom lane


Possibly the emphasis should be on what patches should be ACCEPTED, 
rather than on what patches should be REJECTED?


Then there is less stigma in a patch missing out, and people don't have 
justify rejecting patches.



Cheers,
Gavin



--
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] Releasing in September

2016-01-20 Thread Simon Riggs
On 20 January 2016 at 17:03, Tom Lane  wrote:

> Robert Haas  writes:
> > But I'm not very sure that we're talking about the same set of people
> > here.  If we're going to go to a system where nobody's allowed to
> > commit anything for the next release until the current release is
> > finalized, then we'd better have some procedure for making sure that
> > happens relatively quickly.  And the procedure can't be that the
> > people who are hot to get started on the next release have to bat
> > cleanup for the people who don't have time to fix the bugs they
> > introduced previously.  Because *that* would be manifestly unfair.
>
> You're assuming that the people who are hot to get started on the next
> release are different from the people who don't have time to fix the bugs
> they introduced previously.  IME it's mostly the same people.
>

That sounds like all people who wanted to start the next release had bugs
that needed fixing. That was definitely not the case for 9.5.

I don't think we can allow the release to be slowed down by people that
need to perform actions and yet aren't available.

Ultimately, we should decide to simply turn off that feature and release
anyway.

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: [HACKERS] Releasing in September

2016-01-20 Thread Robert Haas
On Wed, Jan 20, 2016 at 2:26 PM, Tom Lane  wrote:
> and...@anarazel.de (Andres Freund) writes:
>> On 2016-01-20 18:53:54 +, Simon Riggs wrote:
>>> What is the point in having a special mailing list to discuss the release
>>> schedule plus a face-to-face dev meeting to discuss this if we are going to
>>> discuss it here?
>
>> That list exists to discuss concrete releases, and is separate so we
>> e.g. can mention there's security issues and such. This topic in
>> contrast quite validly merits input from more then the usual suspects
>> going to a developer meeting.
>
> In particular, we have several important people who won't be in Brussels
> next week, so it's appropriate to hear their thoughts beforehand ...

As somebody who definitely won't be there, +1 from me.  :-)

-- 
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] Fuzzy substring searching with the pg_trgm extension

2016-01-20 Thread Alvaro Herrera
Artur Zakirov wrote:

> >I don't quite understand why aren't we using a custom GUC variable here.
> >These already have SHOW and SET support ...
> >
> 
> Added GUC variables:
> - pg_trgm.limit
> - pg_trgm.substring_limit
> I added this variables to the documentation.
> show_limit() and set_limit() functions work correctly and they are marked as
> deprecated.

Thanks.  I'm willing to commit quickly a small patch that only changes
the existing function to GUCs, then have a look at a separate patch that
adds the new substring operator.  Would you split the patch that way?

-- 
Álvaro Herrerahttp://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] postgres_fdw join pushdown (was Re: Custom/Foreign-Join-APIs)

2016-01-20 Thread Robert Haas
On Mon, Jan 18, 2016 at 6:47 AM, Ashutosh Bapat
 wrote:
> 2. pg_fdw_join_v2.patch: postgres_fdw changes for supporting join pushdown

The very first hunk of this patch contains annoying whitespace
changes.  Even if the result is what pgindent is going to do anyway,
such changes should be stripped out of patches for ease of review.  In
this case, though, I'm pretty sure it isn't what pgindent is going to
do, so it's just useless churn.  Please remove all such changes from
the patch.

find_var_pos() looks like it should just be inlined into its only caller.

Node *node = (Node *) var;
TargetListEntry *tle = tlist_member(node, context->outerlist);
if (tle)
{
side = OUTER_ALIAS;
pos = tle->resno;
}
else
{
side = INNER_ALIAS;
tle = tlist_member(node, context->innertlist);
pos = tle->resno;
}

Why are we calling the return value "pos" instead of "resno" as we
typically do elsewhere?

get_jointype_name() would probably be better written as a switch.  On
the flip side, deparseColumnRef() would have a lot less code churn if
it *weren't* written as a switch.

What this patch does to the naming and calling conventions in
deparse.c does not good.  Previously, we had deparseTargetList().
Now, we sometimes use that and sometimes deparseAttrsUsed() for almost
the same thing.  Previously, we had deparseColumnRef(); now we have
both that and deparseJoinVar() doing very similar things.  But in each
case, the function names are not parallel and the calling conventions
are totally different.  Like here:

+   if (context->foreignrel->reloptkind == RELOPT_JOINREL)
+   deparseJoinVar(node, context);
+   else
+   deparseColumnRef(buf, node->varno,
node->varattno, context->root);

We pass the buf separately to deparseColumnRef(), but for some reason
not to deparseJoinVar().  I wonder if these functions need to be two
separate things or if the work done by deparseJoinVar() should
actually be part of deparseColumnRef().  But even if it needs to be
separate, I wonder why we can't arrange things so that they get the
same arguments, more or less.

Generally, I think this patch is on the right track, but I think
there's a good bit of work to be done to make it clearer and more
understandable.

-- 
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] Releasing in September

2016-01-20 Thread Joshua D. Drake

On 01/20/2016 10:53 AM, Simon Riggs wrote:

On 20 January 2016 at 15:40, Bruce Momjian > wrote:

Many people where happy with our consistent releasing major releases in
September, e.g. 9.0 to 9.3:


What is the point in having a special mailing list to discuss the
release schedule plus a face-to-face dev meeting to discuss this if we
are going to discuss it here?


Because, like core although they (may) have the final word, taking solid 
feedback from people not on that committee or having that discussion 
publicly lends itself to:


 * Community
 * Transparency
 * The Open Source way



ISTM the wrong starting point to discuss plans in an unplanned way and
assume that everyone has time to take part today, right now.


If it is important to them, they will make time. Everybody has a choice.

Sincerely,

JD

--
Command Prompt, Inc.  http://the.postgres.company/
 +1-503-667-4564
PostgreSQL Centered full stack support, consulting and development.


--
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] Releasing in September

2016-01-20 Thread Magnus Hagander
On Wed, Jan 20, 2016 at 6:57 PM, Alvaro Herrera 
wrote:

> Joshua D. Drake wrote:
>
> > 4. Submit a patch, review a patch.
> >
> > Don't review patches? Don't submit patches.
>
> Here's one area where the commitfest app could help the CFM.  I would
> like to have a report that shows, for each person, the list of patches
> where they are author, and the list of patches where they are reviewer.
> Then I can check his reviewer score by skimming a couple of threads and
> say "you don't have enough author credits, go away".
>
>
Report added. You will find a new "Reports" button at the bottom of the CF
(as a CF admin) which has a link to it.

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


Re: [HACKERS] postgres_fdw join pushdown (was Re: Custom/Foreign-Join-APIs)

2016-01-20 Thread Robert Haas
On Wed, Jan 20, 2016 at 8:53 AM, Ashutosh Bapat
 wrote:
> I missed the example plan cache revalidation patch in the previous mail.
> Sorry. Here it is.

I see.  Yeah, I don't see a reason offhand why that wouldn't work.
But you need to update src/backend/nodes for the new field in each
place where PlannedStmt or PlannerGlobal is mentioned.

-- 
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] Why format() adds double quote?

2016-01-20 Thread Robert Haas
On Wed, Jan 20, 2016 at 4:20 AM, Pavel Stehule  wrote:
>> If we would go this way, question is if we should back patch this or
>> not since the patch apparently changes the existing
>> behaviors. Comments?  I would think we should not.
>
> I am sure, so we should not backport this change. This can breaks customer
> regress tests - and the current behave isn't 100% correct, but it is safe.

Quite.  This is not a bug fix.  It's a behavior change, perhaps for the better.

-- 
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] postgres_fdw join pushdown (was Re: Custom/Foreign-Join-APIs)

2016-01-20 Thread Robert Haas
On Wed, Jan 20, 2016 at 8:50 AM, Ashutosh Bapat
 wrote:
>> Third, I removed GetUserMappingById().  As mentioned in the email to
>> which I replied earlier, that doesn't actually produce the same result
>> as the way we're doing it now, and might leave the user ID invalid.
>
> The comment you quoted was my comment :). I never got a reply from
> Hanada-san on that comment. A bit of investigation revealed this: A pushed
> down foreign join which involves N foreign tables, might have different
> effective userid for each of them.
> userid = OidIsValid(rte->checkAsUser) ? rte->checkAsUser : GetUserId()
> In such a case, AFAIU, the join will be pushed down only if none of those
> have user mapping and there is public user mapping. Is that right?

Yes, I think that is right.

> In such a
> case, which userid should be stored in UserMapping structure?It might look
> like setting GetUserId() as userid in UserMapping is wise, but not really.
> All the foreign tables might have different effective userids, each
> different from GetUserId() and what GetUserId() would return might have a
> user mapping different from the effective userids. What userid should
> UserMapping structure have in that case? I thought, that's why Hanada-san
> used invalid userid there, so left as it is.

Well, we kind of need to get it right, not just be guessing.

It looks to me as though GetConnection() only uses the user ID as a
cache lookup key.  What it's trying to do is ensure that if user A and
user B need different user mapping options, we don't use the same
connection for both.  So, actually, passing InvalidOid seems like it's
not really a problem here.  It's arguably more correct than what we've
been doing up until now, since it means we cache the connection under
user OID whose options we used, rather than the user OID that asked
about the options.

-- 
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] Releasing in September

2016-01-20 Thread Simon Riggs
On 20 January 2016 at 20:29, Joshua D. Drake  wrote:

> On 01/20/2016 10:53 AM, Simon Riggs wrote:
>
>> On 20 January 2016 at 15:40, Bruce Momjian > > wrote:
>>
>> Many people where happy with our consistent releasing major releases
>> in
>> September, e.g. 9.0 to 9.3:
>>
>>
>> What is the point in having a special mailing list to discuss the
>> release schedule plus a face-to-face dev meeting to discuss this if we
>> are going to discuss it here?
>>
>
> Because, like core although they (may) have the final word, taking solid
> feedback from people not on that committee or having that discussion
> publicly lends itself to:
>
>  * Community
>  * Transparency
>  * The Open Source way


I'm in agreement with your points.

It's clear I'd placed too much emphasis on the meeting and the time
reserved for it, not realising some key people would be excluded and the
effects that might have. I withdraw my earlier comments and apologize.

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


  1   2   >