Re: [HACKERS] Final Patch for GROUPING SETS

[Andrew Gierth]

> "Andres" == Andres Freund  writes:

 Andres> Andrew, are you going to be working on any of these?

As discussed on IRC, current status is:

 >>> * The increased complexity of grouping_planner. It'd imo be good if some
 >>>   of that could be refactored into a separate function. Specifically the
 >>>   else if (parse->hasAggs || (parse->groupingSets && parse->groupClause))
 >>>   block.

done and pushed at you

 >>> * The Hopcroft-Karp stuff not being separate

done and pushed

 Andres> * to split agg_retrieve_direct into a version for grouping sets
 Andres> and one without. I think that'll be a pretty clear win for
 Andres> clarity.

I don't see how this helps given that the grouping sets version will be
exactly as complex as the current code.

 Andres> * to spin out common code between agg_retrieve_direct (in both
 Andres> the functions its split into), agg_retrieve_hashed and
 Andres> agg_retrieve_chained. It should e.g. be fairly simple to spin
 Andres> out the tail end processing of a input group
 Andres> (finalize_aggregate loop, ExecQual) into a separate function.

This isn't _quite_ as simple as it sounds but I'll have a go.

 >> * The code in nodeAgg.c isn't pretty in places. Stuff like if
 >> (node->chain_depth > 0) estate->agg_chain_head = save_chain_head;...
 >> Feels like a good bit of cleanup would be possible there.

I'll look.

-- 
Andrew (irc:RhodiumToad)


-- 
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] EvalPlanQual behaves oddly for FDW queries involving system columns

[Etsuro Fujita]

On 2015/05/13 3:24, Tom Lane wrote:

Etsuro Fujita  writes:

On 2015/05/12 7:42, Tom Lane wrote:

I don't much like the division of labor between LockForeignRow and
FetchForeignRow.  In principle that would lead to not one but two
extra remote accesses per locked row in SELECT FOR UPDATE, at least
in the case that an EvalPlanQual recheck is required.  (I see that
in your prototype patch for postgres_fdw you attempt to avoid that
by saving a retrieved tuple in LockForeignRow and then returning it
in FetchForeignRow, but that seems extremely ugly and bug-prone,
since there is nothing in the API spec insisting that those calls match
up one-to-one.)  The fact that locking and fetching a tuple are separate
operations in the heapam API is a historical artifact that probably
doesn't make sense to duplicate in the FDW API.



I understand your concern about the postgres_fdw patch.  However, I
think we should divide this into the two routines as the core patch
does, because I think that would allow an FDW author to implement these
routines so as to improve the efficiency for scenarios that seldom need
fetching, by not retrieving and saving locked tuples in LockForeignRow.


I find it hard to envision a situation where an FDW could lock a row
without being able to fetch its contents more or less for free.  We have
IIRC discussed changing the way that works even for heapam, since the
current design requires multiple buffer lock/unlock cycles which aren't
free either.  In any case, I think that the temptation to do probably-
buggy stuff like what you did in your prototype would be too strong for
most people, and that we're much better off with a simpler one-step API.


OK


Another problem is that we fail to detect whether an EvalPlanQual recheck
is required during a SELECT FOR UPDATE on a remote table, which we
certainly ought to do if the objective is to make postgres_fdw semantics
match the local ones.



I think that is interesting in theory, but I'm not sure that is worth
much in practice.


Hm, well, AFAICS the entire point of this patch is to make it possible for
FDWs to duplicate the semantics used for local tables, so I'm not sure
why you'd want to ignore that aspect of it.


Sorry, my explanation was not correct.  For me, the objective was not 
necessarily to make it possible for FDWs to duplicate the semantics, but 
to make it possible for FDWs to improve the efficiency of SELECT FOR 
UPDATE on foreign tables (and UPDATE/DELETE involving foreign tables as 
source relations) by making it possible for FDWs to duplicate the 
semantics.  And I didn't think that the idea of detecting such a thing 
would be in itself directly useful for the improved efficiency.  Maybe 
I'm missing something, though.



Anyway, I redid the patch along those lines, improved the documentation,
and have committed it.


Thanks for improving and committing the patch.  I'm so happy with the 
committed version.



I did a very basic update of your postgres_fdw patch to test this with,
and attach that so that you don't have to repeat the effort.  I'm not sure
whether we want to try to convert that into something committable.  I'm
afraid that the extra round trips involved in doing row locking this way
will be so expensive that no one really wants it for postgres_fdw.  It's
more credible that FDWs operating against local storage would have use
for it.


I think so too.  And thanks for updating the patch.

Best regards,
Etsuro Fujita


--
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] EvalPlanQual behaves oddly for FDW queries involving system columns

[Etsuro Fujita]

On 2015/05/13 6:22, Tom Lane wrote:

I wrote:

I did a very basic update of your postgres_fdw patch to test this with,
and attach that so that you don't have to repeat the effort.  I'm not sure
whether we want to try to convert that into something committable.  I'm
afraid that the extra round trips involved in doing row locking this way
will be so expensive that no one really wants it for postgres_fdw.  It's
more credible that FDWs operating against local storage would have use
for it.


Of course, if we don't do that then we still have your original gripe
about ctid not being correct in EvalPlanQual results.  I poked at this
a bit and it seems like we could arrange to pass ctid through even in
the ROW_MARK_COPY case, if we define the t_ctid field of a composite
Datum as being the thing to use.  The attached WIP, mostly-comment-free
patch seems to fix the original test case.  It would likely result in
ctid coming out as (0,0) not (4294967295,0) for FDWs that don't take
any special steps to return a valid ctid, as a result of the fact that
heap_form_tuple just leaves that field zeroed.


+1

I did the same thing when creating the first version of the patch [1]. 
In addition, I made another change to ForeignNext so that the FDWs to 
get ctid coming out as the same value (0, 0) instead of (4294967295,0) 
before and after an EvalPlanQual recheck.  But IIRC, I think both of 
them were rejected by you ...



We could fix that by
adding an ItemPointerSetInvalid(&(td->t_ctid)) call to heap_form_tuple,
but I dunno if we want to add even a small number of cycles for this
purpose to such a core function.


I thought so too when creating the first version.

Best regards,
Etsuro Fujita

[1] http://www.postgresql.org/message-id/54b7691b.5080...@lab.ntt.co.jp


--
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] multivariate statistics / patch v6

[Kyotaro HORIGUCHI]

Hello, this might be somewhat out of place but strongly related
to this patch so I'll propose this here.

This is a proposal of new feature for this patch or asking for
your approval for my moving on this as a different (but very
close) project.

===

> Attached is v6 of the multivariate stats, with a number of
> improvements:
...
> 2) fix of pg_proc issues (reported by Jeff)
> 
> 3) rebase to current master

Unfortunately, the v6 patch suffers some system oid conflicts
with recently added ones. And what more unfortunate for me is
that the code for functional dependencies looks undone:)

I mention this because I recently had a issue from strong
correlation between two columns in dbt3 benchmark. Two columns in
some table are in strong correlation but not in functional
dependencies, there are too many values and the distribution of
them is very uniform so MCV is no use for the table (histogram
has nothing to do with equal conditions). As the result, planner
estimates the number of rows largely wrong as expected especially
for joins.

I, then, had a try calculating the ratio between the product of
distinctness of every column and the distinctness of the set of
the columns, call it multivariate coefficient here, and found
that it looks greately useful for the small storage space, less
calculation, and simple code.

The attached first is a script to generate problematic tables.
And the second is a patch to make use of the mv coef on current
master.  The patch is a very primitive POC so no syntactical
interfaces involved.

For the case of your first example,

> =# create table t (a int, b int, c int);
> =# insert into t (select a/1, a/1, a/1
>   from generate_series(0, 99) a);
> =# analyze t;
> =# explain analyze select * from t where a = 1 and b = 1 and c = 1;
>  Seq Scan on t  (cost=0.00..22906.00 rows=1 width=12)
> (actual time=3.878..250.628 rows=1 loops=1)

Make use of mv coefficient.

> =# insert into pg_mvcoefficient values ('t'::regclass, 1, 2, 3, 0);
> =# analyze t;
> =# explain analyze select * from t where a = 1 and b = 1 and c = 1;
>  Seq Scan on t  (cost=0.00..22906.00 rows=9221 width=12)
> (actual time=3.740..242.330 rows=1 loops=1)

Row number estimation was largely improved.

Well, my example,

> $ perl gentbl.pl 1 | psql postgres
> $ psql postgres
> =# explain analyze select * from t1 where a = 1 and b = 2501;
>  Seq Scan on t1  (cost=0.00..6216.00 rows=1 width=8)
>  (actual time=0.030..66.005 rows=8 loops=1)
> 
> =# explain analyze select * from t1 join t2 on (t1.a = t2.a and t1.b = t2.b);
>  Hash Join  (cost=1177.00..11393.76 rows=76 width=16)
> (actual time=29.811..322.271 rows=32 loops=1)

Too bad estimate for the join.

> =# insert into pg_mvcoefficient values ('t1'::regclass, 1, 2, 0, 0);
> =# analyze t1;
> =# explain analyze select * from t1 where a = 1 and b = 2501;
>  Seq Scan on t1  (cost=0.00..6216.00 rows=8 width=8)
>  (actual time=0.032..104.144 rows=8 loops=1)
> 
> =# explain analyze select * from t1 join t2 on (t1.a = t2.a and t1.b = t2.b);
>  Hash Join  (cost=1177.00..11393.76  rows=305652 width=16)
> (actual time=40.642..325.679 rows=32 loops=1)

It gives almost correct estimations.

I think the result above shows that the multivariate coefficient
is significant to imporove estimates when correlated colums are
involved.

Would you consider this in your patch? Otherwise I move on this
as a different project from yours if you don't mind. Except user
interface won't conflict with yours, I suppose. But finally they
should need some labor of consolidation.

regards,

> 1) fix of the contrib compile-time errors (reported by Jeff)
> 
> 2) fix of pg_proc issues (reported by Jeff)
> 
> 3) rebase to current master
> 
> 4) fix a bunch of issues in the previous patches, due to referencing
>some parts too early (e.g. histograms in the first patch, etc.)
> 
> 5) remove the explicit DELETEs from pg_mv_statistic (in the regression
>tests), this is now handled automatically by DROP TABLE etc.
> 
> 6) number of performance optimizations in selectivity estimations:
> 
>(a) minimize calls to get_oprrest, significantly reducing
>syscache calls
> 
>(b) significant reduction of palloc overhead in deserialization of
>MCV lists and histograms
> 
>(c) use more compact serialized representation of MCV lists and
>histograms, often removing ~50% of the size
> 
>(d) use histograms with limited deserialization, which also allows
>caching function calls
> 
>(e) modified histogram bucket partitioning, resulting in more even
>bucket distribution (i.e. producing buckets with more equal
>density and about equal size of each dimension)
> 
> 7) add functions for listing MCV list items and histogram buckets:
> 
> - pg_mv_mcvlist_items(oid)
> - pg_mv_histogram_buckets(oid, type)
> 
>   

Re: [HACKERS] Let PostgreSQL's On Schedule checkpoint write buffer smooth spread cycle by tuning IsCheckpointOnSchedule?

[Heikki Linnakangas]

(please keep the mailing list CC'd, and please don't top-post)

On 05/13/2015 05:00 AM, digoal zhou wrote:

I test it, but use exponent not very perfect in any environment.
why cann't use time only?


As you mentioned yourself earlier, if you only use time but you reach 
checkpoint_segments before checkpoint_timeout, you will not complete the 
checkpoint until you'd already need to begin the next checkpoint. You 
can't completely ignore checkpoint_segments.


Comparing the numbers you give below with 
compensate-fpw-effect-on-checkpoint-scheduling-1.patch, with the ones 
from your first post, it looks like the patch already made the situation 
much better. You still have a significant burst in the beginning of the 
checkpoint cycle, but it's a lot smaller than without the patch. Before 
the patch, the "count" topped at 9078, and below it topped at 2964. 
There is a strange "lull" after the burst, I'm not sure what's going on 
there, but overall it seems like a big improvement.


Did the patch alleviate the bump in latency that pgbench reports?

I put the "count" numbers from your original post and below into a 
spreadsheet, and created some fancy charts. See attached. It shows the 
same thing but with pretty pictures. Assuming we want the checkpoint to 
be spread as evenly as possible across the cycle, the ideal would be a 
straight line from 0 to about 15 in 270 seconds in the cumulative 
chart. You didn't give the full data, but you can extrapolate the lines 
to get a rough picture of how close the different versions are from that 
ideal.


In summary, the X^1.5 correction seems to work pretty well. It doesn't 
completely eliminate the problem, but it makes it a lot better.


I don't want to over-compensate for the full-page-write effect either, 
because there are also applications where that effect isn't so big. For 
example, an application that performs a lot of updates, but all the 
updates are on a small number of pages, so the full-page-write storm 
immediately after checkpoint doesn't last long. A worst case for this 
patch would be such an application - lots of updates on only a few pages 
- with a long checkpoint_timeoout but relatively small 
checkpoint_segments, so that checkpoints are always driven by 
checkpoint_segments. I'd like to see some benchmarking of that worst 
case before committing anything like this.



--end-
checkpoint start
buffer__sync__start num_buffers: 524288, dirty_buffers: 156931
r1_or_w2 2, pid: 29132, min: 44, max: 151, avg: 52, sum: 49387, count: 932
--end-
r1_or_w2 2, pid: 29132, min: 44, max: 95, avg: 49, sum: 41532, count: 837
--end-
r1_or_w2 2, pid: 29132, min: 44, max: 747, avg: 54, sum: 100419, count: 1849
--end-
r1_or_w2 2, pid: 29132, min: 44, max: 372, avg: 52, sum: 110701, count: 2090
--end-
r1_or_w2 2, pid: 29132, min: 44, max: 115, avg: 57, sum: 147510, count: 2575
--end-
r1_or_w2 2, pid: 29132, min: 44, max: 470, avg: 58, sum: 145217, count: 2476
--end-
r1_or_w2 2, pid: 29132, min: 44, max: 120, avg: 54, sum: 161401, count: 2964
--end-
r1_or_w2 2, pid: 29132, min: 44, max: 208, avg: 59, sum: 170280, count: 2847
--end-
r1_or_w2 2, pid: 29132, min: 44, max: 10089, avg: 62, sum: 136106, count:
2181
--end-
r1_or_w2 2, pid: 29132, min: 41, max: 487, avg: 56, sum: 88990, count: 1570
--end-
r1_or_w2 2, pid: 29132, min: 39, max: 102, avg: 55, sum: 59807, count: 1083
--end-
r1_or_w2 2, pid: 29132, min: 40, max: 557, avg: 56, sum: 117274, count: 2083
--end-
r1_or_w2 2, pid: 29132, min: 44, max: 537, avg: 58, sum: 169867, count: 2882
--end-
r1_or_w2 2, pid: 29132, min: 44, max: 147, avg: 60, sum: 92835, count: 1538
--end-
r1_or_w2 2, pid: 29132, min: 30, max: 93, avg: 55, sum: 14641, count: 264
--end-
r1_or_w2 2, pid: 29132, min: 48, max: 92, avg: 56, sum: 11834, count: 210
--end-
r1_or_w2 2, pid: 29132, min: 45, max: 91, avg: 56, sum: 9151, count: 162
--end-
r1_or_w2 2, pid: 29132, min: 46, max: 92, av

Re: [HACKERS] Default Roles

[Heikki Linnakangas]

On 05/13/2015 06:07 AM, Stephen Frost wrote:

This does change the XLOG functions to require pg_monitor, as discussed
on the other thread where it was pointed out by Heikki that the XLOG
location information could be used to extract sensitive information
based on what happens during compression.


That seems like an orthogonal issue, not something that should be 
bundled in this patch. IIRC we didn't reach a consensus on what to do 
about the compression-leaks-information issue. One idea was to make it 
configurable on a per-table basis, and if we do that, perhaps we don't 
need to restrict access to pg_current_xlog_location() and friends.


- Heikki



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

Re: [HACKERS] Sequence Access Method WIP

[Heikki Linnakangas]

On 05/13/2015 07:10 AM, Alvaro Herrera wrote:

Heikki, do you have time to go through this at this point?


I'm afraid I won't :-(. I did intend to, but looking at the calendar, I 
won't have the time to review this thoroughly enough to commit. Sorry.


I haven't looked at the CREATE/DROP ACCESS METHOD FOR SEQUENCE syntax 
patch at all yet.


We discussed using a single amdata column vs. any number of am-specific 
columns. We settled on amdata, but I'm still not 100% convinced that's 
the best approach. Just as a data point, this removes the log_cnt field 
and moves it into amdata in a non-human-readable format. So for someone 
who only uses the local seqam, this just makes things slightly worse. 
For more complicated seqam's, it would be even more important to display 
the state in a human-readable format. Perhaps it's OK that each seqam 
provides its own functions or similar to do that, but I'd like to 
revisit that decision.


I still don't like the serial_sequenceam GUC. Not sure what to do 
instead. Needs some thought.


- Heikki



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

Re: [HACKERS] Sequence Access Method WIP

[Simon Riggs]

On 13 May 2015 at 11:56, Heikki Linnakangas  wrote:

> On 05/13/2015 07:10 AM, Alvaro Herrera wrote:
>
>> Heikki, do you have time to go through this at this point?
>>
>
> I'm afraid I won't :-(. I did intend to, but looking at the calendar, I
> won't have the time to review this thoroughly enough to commit. Sorry.
>
> I haven't looked at the CREATE/DROP ACCESS METHOD FOR SEQUENCE syntax
> patch at all yet.
>
> We discussed using a single amdata column vs. any number of am-specific
> columns. We settled on amdata, but I'm still not 100% convinced that's the
> best approach. Just as a data point, this removes the log_cnt field and
> moves it into amdata in a non-human-readable format. So for someone who
> only uses the local seqam, this just makes things slightly worse. For more
> complicated seqam's, it would be even more important to display the state
> in a human-readable format. Perhaps it's OK that each seqam provides its
> own functions or similar to do that, but I'd like to revisit that decision.
>
> I still don't like the serial_sequenceam GUC. Not sure what to do instead.
> Needs some thought.


This has had around 2 years of thought at this point. I don't agree it
needs more thought.

There is one clear use case for this and it is of benefit to many
distributed architectures.

I don't see what calamity will occur if we commit this. If you don't want a
sequence AM, don't ever use this.

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

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

Re: [HACKERS] Sequence Access Method WIP

[Petr Jelinek]

On 13/05/15 12:56, Heikki Linnakangas wrote:

On 05/13/2015 07:10 AM, Alvaro Herrera wrote:

Heikki, do you have time to go through this at this point?


I'm afraid I won't :-(. I did intend to, but looking at the calendar, I
won't have the time to review this thoroughly enough to commit. Sorry.

We discussed using a single amdata column vs. any number of am-specific
columns. We settled on amdata, but I'm still not 100% convinced that's
the best approach. Just as a data point, this removes the log_cnt field
and moves it into amdata in a non-human-readable format. So for someone
who only uses the local seqam, this just makes things slightly worse.
For more complicated seqam's, it would be even more important to display
the state in a human-readable format. Perhaps it's OK that each seqam
provides its own functions or similar to do that, but I'd like to
revisit that decision.



I do think it's ok for seqam to provide functions that can give you the 
data in human readable form.


I think main argument against custom human readable columns is that it 
will kill any possibility to have common storage for sequences.


There is also additional complexity in the API required for that.

The performance implications are probably small as one could still 
define opaque bytea column and store the data same way a now.



I still don't like the serial_sequenceam GUC. Not sure what to do
instead. Needs some thought.



I think it would be ok if this issue was solved by follow-up patch at 
later time.


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


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

Re: [HACKERS] Default Roles

[Stephen Frost]

* Heikki Linnakangas (hlinn...@iki.fi) wrote:
> On 05/13/2015 06:07 AM, Stephen Frost wrote:
> >This does change the XLOG functions to require pg_monitor, as discussed
> >on the other thread where it was pointed out by Heikki that the XLOG
> >location information could be used to extract sensitive information
> >based on what happens during compression.
> 
> That seems like an orthogonal issue, not something that should be
> bundled in this patch. IIRC we didn't reach a consensus on what to
> do about the compression-leaks-information issue. One idea was to
> make it configurable on a per-table basis, and if we do that,
> perhaps we don't need to restrict access to
> pg_current_xlog_location() and friends.

Alright, I'll pull it out.  I see it's already been added to the
open-items list, so we shouldn't forget about it.

For my 2c, I'd much rather have the information restricted to a
privileged role instead of having to disable the feature.  Further, all
tables need to be considered as having privileged information, not just
systems ones like pg_authid, as the user might not have rights on the
other columns or rows in the table.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] RFC: Non-user-resettable SET SESSION AUTHORISATION

[Stephen Frost]

* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> Craig Ringer wrote:
> > For some time I've wanted a way to "SET SESSION AUTHORISATION" or "SET
> > ROLE" in a way that cannot simply be RESET, so that a connection may be
> > handed to a less-trusted service or application to do some work with.
> 
> Some years back, I checked the SQL standard for insight on how they
> handle this stuff (courtesy of Jim Nasby IIRC).  It took me a while to
> figure out that the way they do it is not to have a RESET command in the
> first place!  In their model, you enter a secure execution context (for
> example, an SQL function) by calling SET SESSION AUTHORIZATION; and once
> there, the only way to revert to the original session authorization is
> to exit the execution context -- and once that happens, the "attacker"
> no longer has control.  Since they have reduced privileges, they can't
> call SET SESSION AUTHORIZATION themselves to elevate their access.  In
> this model, you're automatically protected.
> 
> I mentioned this in some developer meeting; got blank stares back, IIRC.
> I mentioned it to Stephen in hallway track, and as I recall he was in
> agreement with what I was proposing.  Biggest problem is, I can't recall
> in detail what it was.

The issue here ends up being that you don't get the pooling advantage
because the connection pooler ends up having to drop the connection
after using it.

I'm not against a 'SET-and-never-return' concept, but I don't think it'd
help what Craig's after.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Disabling trust/ident authentication configure option

[Volker Aßmann]

On Mon, May 11, 2015 at 10:00 PM, Robert Haas  wrote:

> On Thu, May 7, 2015 at 4:57 PM, Stephen Frost  wrote:
> > * Robert Haas (robertmh...@gmail.com) wrote:
> >> On Thu, May 7, 2015 at 11:02 AM, Stephen Frost 
> wrote:
> >> > I realize it's not going to be popular, but I'd love to have 'trust'
> >> > only allowed if a command-line option is passed to the postmaster or
> >> > something along those lines.  It's really got no business being an
> >> > option for a network service like PG.
> >>
> >> I disagree wholeheartedly.  There is such a thing as a trusted network.
> >
> > Likely a good topic of conversation to be had in Ottawa. :)  I agree
> > that there are trusted networks, but the ones that I work with still
> > expect network services to require authentication and authorization.
> > Perhaps they're not really "trusted" then, from your perspective.  On
> > the other hand, I suppose if you use pg_hba to limit which accounts can
> > be logged into with 'trust' then you might be able to have, say, a
> > "read-only" user/database that anyone could see.  That's a pretty narrow
> > case though and I'd rather we figure out how to address it directly and
> > more specifically (no-password login roles?) than the broad
> > disable-all-authentication "trust" method.
>
> Let's suppose that you have an application server and a DB server
> running on the same node.  That turns out to be too much load, so you
> move the application server to a separate machine and connect the two
> machines with a crossover cable, or a VLAN that has nothing else on
> it.  To me, it's quite sane to want connections on that network to
> proceed without authentication or authorization.  If you've got to
> open up the database more than that then, yes, you need authentication
> and authorization.
>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Even in this case it still means that any breach in any of the network
services running on your application server would immediately own your
database, or at least everything your application can access. This applies
even to totally unrelated services running with restricted permissions.
Using password or certificate based authentication at least gives you the
additional security of local filesystem access controls and is not much
harder to setup. M2M authentication is always a difficult topic as the
"authentication tokens" have to be secured but I would agree that a more
specific / secure method than "disable-all-authentication" would be
preferable.

Best regards,

Volker

Re: [HACKERS] Sequence Access Method WIP

[Heikki Linnakangas]

On 05/13/2015 02:12 PM, Simon Riggs wrote:

This has had around 2 years of thought at this point. I don't agree it
needs more thought.


Noted.


There is one clear use case for this and it is of benefit to many
distributed architectures.


Right. What's your point?


I don't see what calamity will occur if we commit this. If you don't want a
sequence AM, don't ever use this.


I'd like the API to be good for its purpose. Also, I did mention that 
the the current patch makes the situation slightly worse for people who 
never use this: it makes the log_cnt field non human-readable. That's a 
really minor thing, but it shows that it *does* matter how this is 
implemented, even if you only ever use the local seq AM.


- Heikki



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

Re: [HACKERS] Disabling trust/ident authentication configure option

[Robert Haas]

On Wed, May 13, 2015 at 8:01 AM, Volker Aßmann  wrote:
> Even in this case it still means that any breach in any of the network
> services running on your application server would immediately own your
> database, or at least everything your application can access. This applies
> even to totally unrelated services running with restricted permissions.
> Using password or certificate based authentication at least gives you the
> additional security of local filesystem access controls and is not much
> harder to setup. M2M authentication is always a difficult topic as the
> "authentication tokens" have to be secured but I would agree that a more
> specific / secure method than "disable-all-authentication" would be
> preferable.

Sure, opinions on the best way to do any given thing are going to
vary, and nobody's trying to prevent you from configuring your
instances of PostgreSQL however you like.  The email to which I was
responding was suggesting limiting MY ability to set up MY instances
of PostgreSQL the way I like.  And I'm opposed to that.

All of this is fairly far afield from the original topic of this
thread, which was whether a configure option disabling trust + ident
authentication would be a good idea.  I said no.  Then we had a bunch
of counter-proposals:

Alvaro: Support a configure switch whose value is a comma-separated
list of authentication methods to disable.
Peter: Generalized hardening facility.
Andrew: Like what Alvaro said, but require at least one of trust +
peer to remain enabled so people can't hose themselves.
Andrew, v2: Rip out RFC1413 ident authentication completely.
Stephen: Require a command-line option to use trust auth.

There's clearly no consensus on any of these proposals, and most of
them don't address your original requirement anyway, though Alvaro's
would.   I guess the point is that nothing is going to get changed
here on one person's say-so if other people don't agree, so if you
want to get something done, you're going to need to pick something
that can achieve consensus and then implement that.  Also, anything
you want to get done is certainly going to be in 9.6 at the earliest,
because the time for 9.5 proposals has already come and gone.

-- 
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] Default Roles

[Stephen Frost]

All,

* Heikki Linnakangas (hlinn...@iki.fi) wrote:
> On 05/13/2015 06:07 AM, Stephen Frost wrote:
> >This does change the XLOG functions to require pg_monitor, as discussed
> >on the other thread where it was pointed out by Heikki that the XLOG
> >location information could be used to extract sensitive information
> >based on what happens during compression.
> 
> That seems like an orthogonal issue, not something that should be
> bundled in this patch. IIRC we didn't reach a consensus on what to
> do about the compression-leaks-information issue. One idea was to
> make it configurable on a per-table basis, and if we do that,
> perhaps we don't need to restrict access to
> pg_current_xlog_location() and friends.

Updated patch attached which removes the changes to the XLOG location
functions and adds checks for AlterRole and RenameRole to prevent
altering or renaming the default roles.  Also adds '\duS'/'\dgS'
support to psql, to show default roles only when asked.

Thanks!

Stephen
From 88615d712892220cfe4c338317842e368f1fb62e Mon Sep 17 00:00:00 2001
From: Stephen Frost 
Date: Thu, 7 May 2015 23:35:03 -0400
Subject: [PATCH] Create default roles for administrative functions

To reduce the number of users on a system who are superusers,
create a set of roles by default during initdb which are allowed to run
certain functions that allow non-superusers to perform specific
administrative tasks and have access to privileged information.

The prefix "pg_" is reserved for default system roles, similar to
schemas and tablespaces.  pg_upgrade is modified to check for any roles
which start with "pg_" and complain if they exist.  pg_dumpall is
modified to not dump out roles starting with "pg_" on 9.5-and-above
systems.  CreateRole is modified to refuse creation of roles which start
with "pg_", similar to CreateSchema.  psql only shows default roles when
called with \duS or \dgS, similar to other system objects.

Roles created are: pg_backup, pg_monitor, pg_replay, pg_replication,
pg_rotate_logfile, pg_signal_backend, pg_file_settings and pg_admin.
---
 contrib/test_decoding/expected/permissions.out |  8 +--
 doc/src/sgml/catalogs.sgml |  9 +++
 doc/src/sgml/user-manag.sgml   | 91 ++
 src/backend/access/transam/xlogfuncs.c | 30 +
 src/backend/catalog/catalog.c  |  5 +-
 src/backend/catalog/system_views.sql   |  6 +-
 src/backend/commands/user.c| 30 +++--
 src/backend/replication/logical/logicalfuncs.c | 17 ++---
 src/backend/replication/slotfuncs.c| 29 
 src/backend/replication/walsender.c|  8 ++-
 src/backend/utils/adt/misc.c   | 12 ++--
 src/backend/utils/adt/pgstatfuncs.c| 25 ---
 src/backend/utils/misc/guc.c   |  7 ++
 src/bin/pg_dump/pg_dumpall.c   |  2 +
 src/bin/pg_upgrade/check.c | 40 ++-
 src/bin/psql/command.c |  4 +-
 src/bin/psql/describe.c|  5 +-
 src/bin/psql/describe.h|  2 +-
 src/bin/psql/help.c|  4 +-
 src/include/catalog/pg_authid.h| 19 +-
 20 files changed, 280 insertions(+), 73 deletions(-)

diff --git a/contrib/test_decoding/expected/permissions.out b/contrib/test_decoding/expected/permissions.out
index 212fd1d..79a7f86 100644
--- a/contrib/test_decoding/expected/permissions.out
+++ b/contrib/test_decoding/expected/permissions.out
@@ -54,13 +54,13 @@ RESET ROLE;
 -- plain user *can't* can control replication
 SET ROLE lr_normal;
 SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot', 'test_decoding');
-ERROR:  must be superuser or replication role to use replication slots
+ERROR:  must be superuser or member of pg_replication to use replication slots
 INSERT INTO lr_test VALUES('lr_superuser_init');
 ERROR:  permission denied for relation lr_test
 SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1');
-ERROR:  must be superuser or replication role to use replication slots
+ERROR:  must be superuser or member of pg_replication to use replication slots
 SELECT pg_drop_replication_slot('regression_slot');
-ERROR:  must be superuser or replication role to use replication slots
+ERROR:  must be superuser or member of pg_replication to use replication slots
 RESET ROLE;
 -- replication users can drop superuser created slots
 SET ROLE lr_superuser;
@@ -90,7 +90,7 @@ SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot', 'test_d
 RESET ROLE;
 SET ROLE lr_normal;
 SELECT pg_drop_replication_slot('regression_slot');
-ERROR:  must be superuser or replication role to use replication slots
+ERROR:  must be superuser or member of pg_replication to use replication slots
 RESET ROLE;
 -- all users can see existing slots
 SET 

Re: [HACKERS] Streaming replication and WAL archive interactions

[Robert Haas]

On Mon, May 11, 2015 at 12:00 PM, Heikki Linnakangas  wrote:
> And here is a new version of the patch. I kept the approach of using pgstat,
> but it now only polls pgstat every 10 seconds, and doesn't block to wait for
> updated stats.

It's not entirely a new problem, but this error message has gotten pretty crazy:

+   (errmsg("WAL archival
(archive_mode=on/always/shared) requires wal_level \"archive\",
\"hot_standby\", or \"logical\"")));

Maybe: WAL archival cannot be enabled when wal_level is "minimal"

I think the documentation should be explicit about what happens if the
primary archives a file and dies before the standby gets notified that
the archiving happened.  The standby, running in shared mode, is then
promoted.  My first guess would be that the standby will end up with
files that thinks it needs to archive but, being unable to do so
because they're already there, they'll live forever in pg_xlog.  I
hope that's not the case.

-- 
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] One question about security label command

[Robert Haas]

On Sun, May 10, 2015 at 3:15 AM, Kohei KaiGai  wrote:
> 2015-05-01 9:52 GMT+09:00 Kohei KaiGai :
>> 2015-05-01 7:40 GMT+09:00 Alvaro Herrera :
>>> Kouhei Kaigai wrote:
 > * Tom Lane (t...@sss.pgh.pa.us) wrote:
 > > The idea of making the regression test entirely independent of the
 > > system's policy would presumably solve this problem, so I'd kind of
 > > like to see progress on that front.
 >
 > Apologies, I guess it wasn't clear, but that's what I was intending to
 > advocate.
 >
 OK, I'll try to design a new regression test policy that is independent
 from the system's policy assumption, like unconfined domain.

 Please give me time for this work.
>>>
>>> Any progress here?
>>>
>> Not done.
>> The last version I rebuild had a trouble on user/role transition from
>> unconfined_u/unconfined_r to the self defined user/role...
>> So, I'm trying to keep the user/role field (that is not redefined for
>> several years) but to define self domain/types (that have been
>> redefined multiple times) for the regression test at this moment.
>>
> The second approach above works.
> I defined a own privileged domain (sepgsql_regtest_superuser_t)
> instead of system's unconfined_t domain.
> The reason why regression test gets failed was, definition of
> unconfined_t in the system default policy was changed to bypass
> multi-category rules; which our regression test depends on.
> So, the new sepgsql_regtest_superuser_t domain performs almost
> like as unconfined_t, but restricted by multi-category policy as
> traditional unconfined_t did.
> It is self defined domain, so will not affected by system policy
> change.
> Even though the sepgsql-regtest.te still uses unconfined_u and
> unconfined_r pair for selinux-user and role, it requires users to
> define additional selinux-user by hand if we try to define own one.
> In addition, its definition has not been changed for several years.
> So, I thought it has less risk to rely on unconfined_u/unconfined_r
> field unlike unconfined_t domain.

Can you add this to the next CommitFest?

-- 
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] One question about security label command

[Kohei KaiGai]

2015-05-13 21:45 GMT+09:00 Robert Haas :
> On Sun, May 10, 2015 at 3:15 AM, Kohei KaiGai  wrote:
>> 2015-05-01 9:52 GMT+09:00 Kohei KaiGai :
>>> 2015-05-01 7:40 GMT+09:00 Alvaro Herrera :
 Kouhei Kaigai wrote:
> > * Tom Lane (t...@sss.pgh.pa.us) wrote:
> > > The idea of making the regression test entirely independent of the
> > > system's policy would presumably solve this problem, so I'd kind of
> > > like to see progress on that front.
> >
> > Apologies, I guess it wasn't clear, but that's what I was intending to
> > advocate.
> >
> OK, I'll try to design a new regression test policy that is independent
> from the system's policy assumption, like unconfined domain.
>
> Please give me time for this work.

 Any progress here?

>>> Not done.
>>> The last version I rebuild had a trouble on user/role transition from
>>> unconfined_u/unconfined_r to the self defined user/role...
>>> So, I'm trying to keep the user/role field (that is not redefined for
>>> several years) but to define self domain/types (that have been
>>> redefined multiple times) for the regression test at this moment.
>>>
>> The second approach above works.
>> I defined a own privileged domain (sepgsql_regtest_superuser_t)
>> instead of system's unconfined_t domain.
>> The reason why regression test gets failed was, definition of
>> unconfined_t in the system default policy was changed to bypass
>> multi-category rules; which our regression test depends on.
>> So, the new sepgsql_regtest_superuser_t domain performs almost
>> like as unconfined_t, but restricted by multi-category policy as
>> traditional unconfined_t did.
>> It is self defined domain, so will not affected by system policy
>> change.
>> Even though the sepgsql-regtest.te still uses unconfined_u and
>> unconfined_r pair for selinux-user and role, it requires users to
>> define additional selinux-user by hand if we try to define own one.
>> In addition, its definition has not been changed for several years.
>> So, I thought it has less risk to rely on unconfined_u/unconfined_r
>> field unlike unconfined_t domain.
>
> Can you add this to the next CommitFest?
>
OK, done

https://commitfest.postgresql.org/5/249/

-- 
KaiGai Kohei 


-- 
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] Streaming replication and WAL archive interactions

[Heikki Linnakangas]

On 05/13/2015 03:36 PM, Robert Haas wrote:

On Mon, May 11, 2015 at 12:00 PM, Heikki Linnakangas  wrote:

And here is a new version of the patch. I kept the approach of using pgstat,
but it now only polls pgstat every 10 seconds, and doesn't block to wait for
updated stats.


It's not entirely a new problem, but this error message has gotten pretty crazy:

+   (errmsg("WAL archival
(archive_mode=on/always/shared) requires wal_level \"archive\",
\"hot_standby\", or \"logical\"")));

Maybe: WAL archival cannot be enabled when wal_level is "minimal"

I think the documentation should be explicit about what happens if the
primary archives a file and dies before the standby gets notified that
the archiving happened.


Yes, good point.


 The standby, running in shared mode, is then
promoted.  My first guess would be that the standby will end up with
files that thinks it needs to archive but, being unable to do so
because they're already there, they'll live forever in pg_xlog.  I
hope that's not the case.


Hmm. That is exactly what happens. The standby will attempt to archive 
them, which will fail, so the archiver will get stuck retrying.


That's not actually a new problem though. Even with a single server 
doing archiving, it's possible that you crash just after archive_command 
has archived a file, but before it has created the .done file. After 
restart, the server will try to archive the file again, which will fail. 
But yeah, with this patch, that's much more likely to happen after a 
promotion.


Our manual says that archive_command should refuse to overwrite an 
existing file. But to work-around the double-archival problem, where the 
same file is archived twice, it would be even better if it would simply 
return success if the file exists, *and has identical contents*. I don't 
know how to code that logic in a simple one-liner though.


- Heikki


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

Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)

[Robert Haas]

On Mon, May 11, 2015 at 9:07 PM, David Steele  wrote:
> On 5/1/15 5:58 AM, Sawada Masahiko wrote:
>> For now, since pg_audit patch has a pg_audit_ddl_command_end()
>> function which uses part of un-committed "deparsing utility commands"
>> patch API,
>> pg_audit patch is completely depend on that patch.
>> If API name, interface are changed, it would affect for pg_audit patch.
>> I'm not sure about progress of "deparsing utility command" patch but
>> you have any idea if that patch is not committed into 9.5 until May
>> 15?
>
> The attached v12 patch removes the code that became redundant with
> Alvaro committing the event trigger/deparse work.  I've updated the
> regression tests to reflect the changes, which were fairly minor and add
> additional information to the output.  There are no longer any #ifdef'd
> code blocks.

This is not a full review, but just a few thoughts...

What happens if the server crashes?  Presumably, audit records emitted
just before the crash can be lost, possibly even if the transaction
went on to commit.  That's no worse than what is already the case for
regular logging, of course, but it's maybe a bit more relevant here
because of the intended use of this information.

+if (audit_on_relation(relOid, auditOid, auditPerms))
+{
+auditEventStack->auditEvent.granted = true;
+}

Braces around single-statement blocks are not PostgreSQL style.

I wonder if driving the auditing system off of the required
permissions is really going to work well.  That means that decisions
we make about permissions enforcement will have knock-on effects on
auditing.  For example, the fact that we check permission on a view
rather than on the underlying tables will (I think) flow through to
how the auditing happens.

+stackItem->auditEvent.commandTag == T_DoStmt &&

Use IsA(..., DoStmt) for this kind of test.  There are many instances
of this pattern in the patch; they should al be fixed.

Using auditLogNotice to switch the log level between LOG and NOTICE is
weird.  Switching from LOG to NOTICE is an increase in the logging
level relative to client_min_messages, but a decrease relative to
log_min_messages.   With default settings, logging at LOG will go only
to the log (not the client) and logging at NOTICE will go only to the
client (not the log).

The documentation and comments in this patch are, by my estimation,
somewhat below our usual standard.  For example:

+   DefineCustomBoolVariable("pg_audit.log_notice",
+"Raise a
notice when logging",

Granted, there is a fuller explanation in the documentation, but that
doesn't make that explanation particularly good.  (One might also
wonder why the log level isn't fully configurable instead of offering
only two choices.)

Here's another example:

+   DefineCustomBoolVariable("pg_audit.log_parameter",
+"Enable
statement parameter logging",

It would be far better to say "lnclude the values of statement
parameters in auditing messages" or something like that.  It is quite
clear that this GUC doesn't enable some sort of general statement
parameter logging facility; it changes the contents of auditing
messages that would have been emitted either way.

I don't want to focus too much on this particular example.  The
comments and documentation really deserve a bit more attention
generally than they have gotten thus far.  I am not saying they are
bad.  I am just saying that, IMHO, they are not as good as we
typically try to make them.

-- 
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] Streaming replication and WAL archive interactions

[Robert Haas]

On Wed, May 13, 2015 at 8:53 AM, Heikki Linnakangas  wrote:
> Our manual says that archive_command should refuse to overwrite an existing
> file. But to work-around the double-archival problem, where the same file is
> archived twice, it would be even better if it would simply return success if
> the file exists, *and has identical contents*. I don't know how to code that
> logic in a simple one-liner though.

This is why we really, really need that pg_copy command that was
proposed a while back.

-- 
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 does contain_leaked_vars believe MinMaxExpr is safe?

[Robert Haas]

On Tue, May 12, 2015 at 7:22 PM, Tom Lane  wrote:
> MinMaxExpr is an implicit invocation of a btree comparison function.
> Are we supposing that all of those are necessarily leakproof?

I suspect it's an oversight, because the comment gives no hint that
any such intention was present.  It's been more than three years since
I committed that code (under a different function name) so my memory
is a little fuzzy, but I believe it just didn't occur to me that
MinMaxExpr could include a function call.

I suspect it's safe in practice, but in theory it's probably a bug.

-- 
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] Sequence Access Method WIP

[Petr Jelinek]

On 13/05/15 14:15, Heikki Linnakangas wrote:

I don't see what calamity will occur if we commit this. If you don't
want a
sequence AM, don't ever use this.


I'd like the API to be good for its purpose. Also, I did mention that
the the current patch makes the situation slightly worse for people who
never use this: it makes the log_cnt field non human-readable. That's a
really minor thing, but it shows that it *does* matter how this is
implemented, even if you only ever use the local seq AM.



It definitely does matter.

I don't think we'll find perfect compromise here though, you can either 
do it one way or the other. Trust me it does not make me happy either, I 
like perfect solutions too, but when there is lack of perfect solution I 
prefer the simpler one.


Both of the solutions have drawbacks
 - amdata has opaque blob which does not store data in user visible way 
but that can be worked around by providing function that shows it in 
human readable way (and the dump function for many sequence types 
actually does that).
 - multiple columns basically kill any future ability to unify the 
storage for sequences and also adds complexity, especially around alter 
table (since it means drop/add column and stuff)


But I already wrote both versions anyway so from that perspective it 
does not matter much which part we merge.


(As a side-node I would have preferred to have this discussion earlier 
than 2 days before feature freeze because the current implementation is 
something that we agreed on several months ago so there was plenty of 
time to revisit that decision.)


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


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

Re: [HACKERS] Silent Data Manipulation - Time(stamp) w/o Time Zone Literals w/ TZ

[Robert Haas]

On Tue, May 12, 2015 at 1:57 PM, David G. Johnston
 wrote:
> "In a literal that has been determined to be timestamp without time zone,
> PostgreSQL will silently ignore any time zone indication. That is, the
> resulting value is derived from the date/time fields in the input value, and
> is not adjusted for time zone."
>
> http://www.postgresql.org/docs/9.3/static/datatype-datetime.html
>
> 8.5.1.3 Time Stamps
>
> Not exactly a widely impactful decision but is there harm in emitting a
>
> "NOTICE: timezone specification ignored"

Well, it could emit a catastrophic amount of log chatter.  Try doing a
bulk load into a table with a column of that type.

In general, I'm not a big fan of accepting things and ignoring them.
We've prided ourselves on having tighter data type checking than
pretty much any other database product out there, but somehow we still
have edge cases like this floating around.

-- 
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] Default Roles (was: Additional role attributes)

[Robert Haas]

On Tue, May 12, 2015 at 11:07 PM, Stephen Frost  wrote:
> Thoughts?  Comments?  Suggestions?

Yes: let's punt this to 9.6.  The decisions you're making here are way
too significant to be making a couple of days before feature freeze,
and this patch has changed massively since it was first submitted.
There isn't time now for people who want to have an opinion on this to
form an educated one.

-- 
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] Default Roles (was: Additional role attributes)

[Bruce Momjian]

On Wed, May 13, 2015 at 10:16:39AM -0400, Robert Haas wrote:
> On Tue, May 12, 2015 at 11:07 PM, Stephen Frost  wrote:
> > Thoughts?  Comments?  Suggestions?
> 
> Yes: let's punt this to 9.6.  The decisions you're making here are way
> too significant to be making a couple of days before feature freeze,
> and this patch has changed massively since it was first submitted.
> There isn't time now for people who want to have an opinion on this to
> form an educated one.

Yeah, pretty much any patch that needs significant redesign at this
point should be kept for 9.6.

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

  + Everyone has their own god. +


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

[HACKERS] a few thoughts on the schedule

[Robert Haas]

Hi,

It's pretty clear that the impending feature freeze proposed by core
has spurred a lot of activity.  This is both good and bad.  On the
good side, we're getting a bunch of stuff done.  On the bad side,
there is inevitably going to be a temptation to rush things in that
are really not quite fully-baked just yet.  If we fail to resist that
temptation, we WILL pay the consequences.  Those consequences may
include overt bugs (as per the recent discussion on what went wrong
with multi-xacts) or bad design decisions from which we will not
easily be able to disentangle ourselves.

I am already concerned about some of the commits that have gone in
very recently, particularly these:

- Map basebackup tablespaces using a tablespace_map file - Discussion
on pgsql-committers suggests that this may have some scary problems.
Maybe I just don't understand the situation.
- Allow on-the-fly capture of DDL event details - This looks really
half-baked to me.  At the least, the documentation expresses an
optimism about what can be done with a pg_ddl_command that isn't
realized by the code.
- Code review for foreign/custom join pushdown patch - Whacks around
my earlier commit without agreement or adequate discussion, apparently
on the theory that Tom always knows best.
- Add support for INSERT ... ON CONFLICT DO NOTHING/UPDATE - Huge
feature introducing novel infrastructure.

We also need to start thinking about what happens after feature
freeze.  The CommitFest application currently lists a 2015-06
CommitFest which, according to previous practice, would be expected to
start on the 15th of the month.  Given where we are now, that seems
entirely unrealistic.  I am doubtful that we will be ready to ship a
beta by mid-June, let alone begin a new development cycle.

-- 
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] subxcnt defined as signed integer in SnapshotData and SerializeSnapshotData

[Robert Haas]

On Sat, May 9, 2015 at 8:48 AM, Michael Paquier
 wrote:
> On Fri, May 8, 2015 at 10:27 PM, Simon Riggs wrote:
>> On 8 May 2015 at 13:02, Michael Paquier wrote:
>>> I think that we should redefine subxcnt as uint32 for consistency with
>>> xcnt, and remove the two assertions that 924bcf4 has introduced. I
>>> could get a patch quickly done FWIW.
>>
>> (uint32) +1
>
> Attached is the patch. This has finished by being far simpler than
> what I thought first.

I'm just going to remove the useless assertion for now.  What you're
proposing here may (or may not) be worth doing, but it carries a
non-zero risk of breaking something somewhere, if anyone is relying on
the signed-ness of that type.  Removing the assertion is definitely
safe.

-- 
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] a few thoughts on the schedule

[Tom Lane]

Robert Haas  writes:
> It's pretty clear that the impending feature freeze proposed by core
> has spurred a lot of activity.  This is both good and bad.  On the
> good side, we're getting a bunch of stuff done.  On the bad side,
> there is inevitably going to be a temptation to rush things in that
> are really not quite fully-baked just yet.  If we fail to resist that
> temptation, we WILL pay the consequences.

Yeah.  At this point I think it's clear that only a minority of the
remaining commitfest items can get into 9.5; we're going to have to
punt many of them to next time.  I'll post some thoughts about specific
patches separately, but let's keep this thread for discussion of the
overall situation.

> I am already concerned about some of the commits that have gone in
> very recently, particularly these:

There is going to need to be a mop-up period, and we ought to be willing
to revert anything we feel wasn't really ready.  I don't feel that those
decisions need to be made in a hurry though.  I'm envisioning taking about
a month to look more closely at committed patches and see what needs to be
cleaned up or undone altogether.

> - Code review for foreign/custom join pushdown patch - Whacks around
> my earlier commit without agreement or adequate discussion, apparently
> on the theory that Tom always knows best.

As far as that goes, obviously I've got strong opinions on what the FDW
APIs ought to look like, and I'm happy to discuss this issue further ---
but I think that's a topic for the mop-up period, not for this week.

What I think we need to be doing this week is triage.  Commit what's
ready, punt what's not.  I'll post a separate list about that.

regards, tom lane


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

Re: [HACKERS] PATCH: pgbench allow '=' in \set

[Robert Haas]

On Thu, May 7, 2015 at 2:18 PM, Fabien COELHO  wrote:
> Since an expression syntax has been added to pgbench, I find that the
> readability of expressions is not great. An extra '=' improves the situation
> for me:
>
>\set id = 1 + abs((:id * 1021) % (10 * :scale))
>
> seems slightly better than:
>
>\set id 1 + abs((:id * 1021) % (10 * :scale))
>
> But that is debatable!

I would like to debate that.  :-)

Generally, I don't like optional noise words.  A good example of how
this can bite you is the whole problem with LOCK TABLE foo.  You can
also write that as LOCK foo.  What if you want to lock something
that's not a table?  If the word "table" were required, then you could
support LOCK VIEW foo or LOCK MATERIALIZED VIEW foo or even LOCK
FUNCTION foo().  But because the word TABLE is optional, that syntax
creates parser ambiguities.  We have not been able to agree on a
reasonable solution to this problem, and it has blocked implementation
of this important and useful feature for years.  While I can't foresee
what trouble specifically your proposal might cause, I have become
wary of such things.

I also don't like to restate what has already been said.  \set at the
beginning of the line tells you that you will be setting a variable.
Adding = or := only restates the same thing.  I agree it superficially
looks a little nicer, but I'm not sure it's really going to add
clarity, because it's basically just redundant.

-- 
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] a few thoughts on the schedule

[Robert Haas]

On Wed, May 13, 2015 at 11:09 AM, Tom Lane  wrote:
>> - Code review for foreign/custom join pushdown patch - Whacks around
>> my earlier commit without agreement or adequate discussion, apparently
>> on the theory that Tom always knows best.
>
> As far as that goes, obviously I've got strong opinions on what the FDW
> APIs ought to look like, and I'm happy to discuss this issue further ---
> but I think that's a topic for the mop-up period, not for this week.
>
> What I think we need to be doing this week is triage.  Commit what's
> ready, punt what's not.  I'll post a separate list about that.

That sounds like a fine plan.  Thanks for responding.

-- 
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] a few thoughts on the schedule

[Bruce Momjian]

On Wed, May 13, 2015 at 10:32:03AM -0400, Robert Haas wrote:
> We also need to start thinking about what happens after feature
> freeze.  The CommitFest application currently lists a 2015-06
> CommitFest which, according to previous practice, would be expected to
> start on the 15th of the month.  Given where we are now, that seems
> entirely unrealistic.  I am doubtful that we will be ready to ship a
> beta by mid-June, let alone begin a new development cycle.

This is a very good analysis.  I have been holding back my trivial new
patches for 9.6 in hopes of setting a good example.  However, all my
stuff is new, while many of these other patches have waited their turn
for review, and we are going to be unfair to submitters if we don't give
them the attention they deserve --- that is always the tension we have
at this time.

We have three days left so I think we need committers to devote serious
time, if possible, to helping us resolve as much as we can.  If we start
thinking about this on Friday, it is too late.

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

  + Everyone has their own god. +


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

Re: [HACKERS] Proposal : REINDEX xxx VERBOSE

[Robert Haas]

On Thu, May 7, 2015 at 6:55 PM, Sawada Masahiko  wrote:
> Sorry, I forgot attach files.

Review comments:

- Customarily we use int, rather than uint8, for flags variables.  I
think we should do that here also.

- reindex_index() emits a log message either way, but at DEBUG2 level
without VERBOSE and at the INFO level with it.  I think we should skip
it altogether without VERBOSE.  i.e. if (options & REINDEXOPT_VERBOSE)
ereport(...)

- The errmsg() in that function should not end with a period.

- The 000 patch makes a pointless whitespace change to tab-complete.c.

- Instead of an enumerated type (ReindexOption) just use #define to
define the flag value.

Apart from those fairly minor issues, I think this looks pretty solid.

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

[HACKERS] Triaging the remaining open commitfest items

[Tom Lane]

Looking at what remains open in the current commitfest:

* fsync $PGDATA recursively at startup

Andres is the reviewer of record on this one.  He should either commit it
if he feels it's ready, or bounce it to next CF if not.

* EvalPlanQual behaves oddly for FDW queries involving system columns

I've been working this one and will deal with any mop-up needed, but
I think probably what ought to happen now is just to commit the small
ctid-transmission hack I posted yesterday and call it good.

* BRIN inclusion operator class

This is Alvaro's turf, obviously.

* PageRepairFragmentation optimization

Heikki's patch, so his to commit or not, though since it's marked
Waiting on Author I'm guessing it's not ready?

* Abbreviated key support for Datum sorts

I've been assuming Robert would either commit this or not, since he's
been the committer for the predecessor patches.

* KNN-GiST with recheck

Heikki's been dealing with this thread so far, and is surely the best
qualified to decide if it's ready or not.

* GIN fillfactor

I'd like to put this one on Heikki's plate as well, since he's touched
the GIN code more than anyone else lately.

* Manipulating complex types as non-contiguous structures in-memory

This one's mine of course.  I've been hoping to get more independent
performance testing than it's gotten, but time grows short.  I'm inclined
to just go ahead and push it in.

* Optimization for updating foreign tables in Postgres FDW

I concur with Stephen's assessment that this doesn't look well designed.
I think we should just mark it RWF for now.

* iteration over record in plpgsql

I fear this one slipped through the cracks --- it's marked Waiting on
Author in the CF app, but it looks like Pavel did submit an updated
version after that marking was made.  However, it's not a terribly
significant feature and there was doubt as to whether we want it at
all anyway.  Suggest we just punt it to next CF at this point.

* Sequence Access Method

Heikki's marked as reviewer, so it's his call as to whether this is ready,
but the impression I have is that there's not really consensus as to
whether the API is good.  If not, it's something I think we should push
to 9.6.

* archive_mode=shared/always

Heikki's patch, so his call.

* Sending WAL receiver feedback regularly even if the receiver is under heavy 
load

This seems to not be ready, but it also seems to be a bug fix, so when
it is ready I think we could commit it.

* Auditing extension

I do not get the impression that there is consensus on this.  Push to 9.6?

* ctidscan as an example of custom-scan

This basically hasn't gotten any attention, which may mean nobody cares
enough to justify putting it in the tree.  We need to either push it to
next CF or reject altogether.

* parallel mode/contexts

Robert's patch, his to deal with (likewise for "assessing parallel-safety").

* RLS: row-level security, more review

Stephen's baby.

* Cube extension kNN support

This is still marked as "needs review", I'm afraid we have to push to 9.6.

* compress method for spgist

* Join pushdown support for foreign tables

There doesn't seem to be any current patch linked to this CF item.
If there is a patch to get postgres_fdw to make use of the recently
committed join-path support, I assume it's in need of a rebase anyway.

* Grouping Sets

I had originally promised to be committer for this one, and still want
to go look at it, but Robert's nearby message about not committing stuff
in haste definitely seems to apply.

* TABLESAMPLE clause

I assume we'd better push this to 9.6 at this point.

* REINDEX xxx VERBOSE

Seems like we just need somebody to make a decision on syntax.

* Additional role attributes

Is this ready to commit?  Stephen's call.

* catalog view to pg_hba.conf file

Greg Stark is marked as committer of record on this.

* Add pg_settings.pending_restart column

Peter's patch, his call.


So there you have it.  If everyone would go make decisions on the patches
that they are the most obvious committer for, we could get those taken
care of one way or the other pretty quickly.  As for the ones I proposed
pushing to 9.6, any committer who feels differently can pick those up,
else I'll go change their status in the CF app tomorrow or so.

regards, tom lane


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

Re: [HACKERS] a few thoughts on the schedule

[Bruce Momjian]

On Wed, May 13, 2015 at 11:19:29AM -0400, Bruce Momjian wrote:
> On Wed, May 13, 2015 at 10:32:03AM -0400, Robert Haas wrote:
> > We also need to start thinking about what happens after feature
> > freeze.  The CommitFest application currently lists a 2015-06
> > CommitFest which, according to previous practice, would be expected to
> > start on the 15th of the month.  Given where we are now, that seems
> > entirely unrealistic.  I am doubtful that we will be ready to ship a
> > beta by mid-June, let alone begin a new development cycle.
> 
> This is a very good analysis.  I have been holding back my trivial new
> patches for 9.6 in hopes of setting a good example.  However, all my
> stuff is new, while many of these other patches have waited their turn
> for review, and we are going to be unfair to submitters if we don't give
> them the attention they deserve --- that is always the tension we have
> at this time.
> 
> We have three days left so I think we need committers to devote serious
> time, if possible, to helping us resolve as much as we can.  If we start
> thinking about this on Friday, it is too late.

Let me put a finer point on this --- whatever gets pushed to 9.6
unreasonably will be a feature we don't have in 9.5 and will discourage
future development.  I know we can't do magic, but now is the time to
try.

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

  + Everyone has their own god. +


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

Re: [HACKERS] Triaging the remaining open commitfest items

[Tom Lane]

I wrote:
> * Cube extension kNN support

> This is still marked as "needs review", I'm afraid we have to push to 9.6.

> * compress method for spgist

Sigh.  There was a "Ditto" under this one, but somehow it disappeared
in editing :-(

regards, tom lane


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

Re: [HACKERS] a few thoughts on the schedule

[Stephen Frost]

* Bruce Momjian (br...@momjian.us) wrote:
> On Wed, May 13, 2015 at 11:19:29AM -0400, Bruce Momjian wrote:
> > On Wed, May 13, 2015 at 10:32:03AM -0400, Robert Haas wrote:
> > > We also need to start thinking about what happens after feature
> > > freeze.  The CommitFest application currently lists a 2015-06
> > > CommitFest which, according to previous practice, would be expected to
> > > start on the 15th of the month.  Given where we are now, that seems
> > > entirely unrealistic.  I am doubtful that we will be ready to ship a
> > > beta by mid-June, let alone begin a new development cycle.
> > 
> > This is a very good analysis.  I have been holding back my trivial new
> > patches for 9.6 in hopes of setting a good example.  However, all my
> > stuff is new, while many of these other patches have waited their turn
> > for review, and we are going to be unfair to submitters if we don't give
> > them the attention they deserve --- that is always the tension we have
> > at this time.
> > 
> > We have three days left so I think we need committers to devote serious
> > time, if possible, to helping us resolve as much as we can.  If we start
> > thinking about this on Friday, it is too late.
> 
> Let me put a finer point on this --- whatever gets pushed to 9.6
> unreasonably will be a feature we don't have in 9.5 and will discourage
> future development.  I know we can't do magic, but now is the time to
> try.

I certainly agree with this and have been putting a fair bit of effort
into it over the past week. :/

More help would absolutely be appreciated.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] a few thoughts on the schedule

[Robert Haas]

On Wed, May 13, 2015 at 11:40 AM, Bruce Momjian  wrote:
>> We have three days left so I think we need committers to devote serious
>> time, if possible, to helping us resolve as much as we can.  If we start
>> thinking about this on Friday, it is too late.
>
> Let me put a finer point on this --- whatever gets pushed to 9.6
> unreasonably will be a feature we don't have in 9.5 and will discourage
> future development.  I know we can't do magic, but now is the time to
> try.

And on the flip side, whatever is pushed to 9.6 will not create a
stability issue in 9.5.

-- 
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] Default Roles (was: Additional role attributes)

[Stephen Frost]

* Robert Haas (robertmh...@gmail.com) wrote:
> Yes: let's punt this to 9.6.  The decisions you're making here are way
> too significant to be making a couple of days before feature freeze,
> and this patch has changed massively since it was first submitted.
> There isn't time now for people who want to have an opinion on this to
> form an educated one.

Perhaps I'm missing something, but the patch has been simplified down to
the point where the only question seems to be "should we have default
roles or not?", which I had asked about two weeks ago and again last
week on a new thread.  I feel like we're waiting for the silent majority
to chime in.

Put another way, I'm afraid that posting this next week, next month, or
next year is going to garner just as many responses as it's seen in the
past 2 weeks, while I continue to field questions on -bugs, -admin, and
IRC about "how do I set up Nagios with a non-superuser account?" and
similar issues.  It's not a novel idea, certainly;  Magnus suggested it
back in December on the thread, Tom made a similar comment that it might
make sense to have them later on and it's come up quite a few times
previously as it's something other RDBMS's have and we don't.  Clearly,
others have read the proposal, at least (You and Alvaro on the other
thread, Heikki on this one).

It's my fault that I didn't follow up on their suggestions earlier and
instead spent a bunch of time fighting with pg_dump, but it doesn't seem
like there is a lot of disagreement about the idea.  I'd offer to
simplify it down, but it seems like the obvious change in that direction
would be to reserve pg_ as a role prefix and not actually create any
default roles, but that doesn't gain us anything and makes a potential
headache for users without any feature to go with it.

Bruce's point is a better one, except that all of the changes have been
about reducing changes to core, down to an almost trivial level.  I wish
it had been a smoother ride to get here from the original proposal six
months ago, but I've certainly got a better understanding of the level
of effort involved and changes required for the other approaches and
this certainly seems like the best and simplest.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] a few thoughts on the schedule

[Tom Lane]

Bruce Momjian  writes:
> Let me put a finer point on this --- whatever gets pushed to 9.6
> unreasonably will be a feature we don't have in 9.5 and will discourage
> future development.  I know we can't do magic, but now is the time to
> try.

The other side of that coin is that the stuff that ends up getting pushed
will, in many cases, be stuff that nobody cared a whole lot about.

One thing that continues to bother me about the commitfest process is that
it's created a default expectation that things get committed eventually.
But many new ideas are just plain bad, and others are things that nobody
but the author cares about.  We need to remember that every new feature
we add creates an ongoing maintenance burden, and might foreclose better
ideas later.  I'd like to see a higher threshold for accepting feature
patches than we seem to have applied of late.

regards, tom lane


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

Re: [HACKERS] a few thoughts on the schedule

[Bruce Momjian]

On Wed, May 13, 2015 at 11:52:40AM -0400, Tom Lane wrote:
> Bruce Momjian  writes:
> > Let me put a finer point on this --- whatever gets pushed to 9.6
> > unreasonably will be a feature we don't have in 9.5 and will discourage
> > future development.  I know we can't do magic, but now is the time to
> > try.
> 
> The other side of that coin is that the stuff that ends up getting pushed
> will, in many cases, be stuff that nobody cared a whole lot about.
> 
> One thing that continues to bother me about the commitfest process is that
> it's created a default expectation that things get committed eventually.
> But many new ideas are just plain bad, and others are things that nobody
> but the author cares about.  We need to remember that every new feature
> we add creates an ongoing maintenance burden, and might foreclose better
> ideas later.  I'd like to see a higher threshold for accepting feature
> patches than we seem to have applied of late.

Agreed.  If the idea is good someone else will pick it up.

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

  + Everyone has their own god. +


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

Re: [HACKERS] Triaging the remaining open commitfest items

[Robert Haas]

On Wed, May 13, 2015 at 11:38 AM, Tom Lane  wrote:
> * fsync $PGDATA recursively at startup
>
> Andres is the reviewer of record on this one.  He should either commit it
> if he feels it's ready, or bounce it to next CF if not.

I committed the first part of this as
2ce439f3379aed857517c8ce207485655000fc8e.  I think that we do not have
design consensus on the rest.  I think we should mark this committed,
and if the folks who proposed the further work here still want to
press their case, that should wait for 9.6.

> * Abbreviated key support for Datum sorts
>
> I've been assuming Robert would either commit this or not, since he's
> been the committer for the predecessor patches.

I'll deal with this.

> * Sequence Access Method
>
> Heikki's marked as reviewer, so it's his call as to whether this is ready,
> but the impression I have is that there's not really consensus as to
> whether the API is good.  If not, it's something I think we should push
> to 9.6.

I share your concern on this one.

> * ctidscan as an example of custom-scan
>
> This basically hasn't gotten any attention, which may mean nobody cares
> enough to justify putting it in the tree.  We need to either push it to
> next CF or reject altogether.

Agreed.  I was fine with never committing this.  I don't think we have
a requirement that every hook or bit of functionality we expose at the
C level must have an example in core.  But other people (you?  Simon?)
seemed to want a demonstration in the core repository.  If that's
still a priority, I am willing to work on it more for 9.6, but there
is not time now.

> * parallel mode/contexts
>
> Robert's patch, his to deal with (likewise for "assessing parallel-safety").

Most of the parallel mode stuff is committed.  What's left will have
to wait for 9.6.

Assessing parallel-safety will also need to wait for 9.6.

> * Join pushdown support for foreign tables
>
> There doesn't seem to be any current patch linked to this CF item.
> If there is a patch to get postgres_fdw to make use of the recently
> committed join-path support, I assume it's in need of a rebase anyway.

I think there is a rebased patch around.  I think it's just not linked
into the CF thread.  I don't think it's committable as is.

> * Grouping Sets
>
> I had originally promised to be committer for this one, and still want
> to go look at it, but Robert's nearby message about not committing stuff
> in haste definitely seems to apply.

I really feel we didn't give this a fair shake.  I'm not saying we
should make up for that by committing it in haste, but not giving
things a fair shake is bad for the project regardless of anything
else.

> * TABLESAMPLE clause
>
> I assume we'd better push this to 9.6 at this point.

+1

> * REINDEX xxx VERBOSE
>
> Seems like we just need somebody to make a decision on syntax.

I just posted a review of this raising minor points only.  If it is
timely revised, I will commit it.

> * Additional role attributes
>
> Is this ready to commit?  Stephen's call.

-1 for committing this, per discussion earlier today on a thread
that's probably not linked into the CF app.

> * catalog view to pg_hba.conf file
>
> Greg Stark is marked as committer of record on this.

I am doubtful whether this is ready.

-- 
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] Triaging the remaining open commitfest items

[Andres Freund]

On 2015-05-13 11:38:27 -0400, Tom Lane wrote:
> Looking at what remains open in the current commitfest:
> 
> * fsync $PGDATA recursively at startup
> 
> Andres is the reviewer of record on this one.  He should either commit it
> if he feels it's ready, or bounce it to next CF if not.

The more important part of this has been committed by Robert. The other
part, while also important in my opinion, has by now slipped 9.5.

I've moved it.

> * Manipulating complex types as non-contiguous structures in-memory
> 
> This one's mine of course.  I've been hoping to get more independent
> performance testing than it's gotten, but time grows short.  I'm inclined
> to just go ahead and push it in.

I'm a bit hesitant about performance regressions around it. And I'd
obviously rather not see the macros but the inline version ;). But I
think overall we're in a better position with it, than without. If it
turns out to have bad edge cases performancewise, we can still "turn it
off" in plpgsql without much problems. If we, preferrably, can't find a
better solution for the performance problem.

> * Grouping Sets
> 
> I had originally promised to be committer for this one, and still want
> to go look at it, but Robert's nearby message about not committing stuff
> in haste definitely seems to apply.

This has been in a limbo for a very long time. I'm right now working
with Andrew getting it into a a committable shape. I'm not 100% sure
we'll succeed for 9.5, but it deserves a chance. If it doesn't make it
into 9.6, I plan to get into 9.6 soon after the branch opens.

> * Additional role attributes
> 
> Is this ready to commit?  Stephen's call.

Not yet ready in my opinion.

Greetings,

Andres Freund


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

Re: [HACKERS] subxcnt defined as signed integer in SnapshotData and SerializeSnapshotData

[Michael Paquier]

On Thu, May 14, 2015 at 12:02 AM, Robert Haas  wrote:
> On Sat, May 9, 2015 at 8:48 AM, Michael Paquier
>  wrote:
>> On Fri, May 8, 2015 at 10:27 PM, Simon Riggs wrote:
>>> On 8 May 2015 at 13:02, Michael Paquier wrote:
 I think that we should redefine subxcnt as uint32 for consistency with
 xcnt, and remove the two assertions that 924bcf4 has introduced. I
 could get a patch quickly done FWIW.
>>>
>>> (uint32) +1
>>
>> Attached is the patch. This has finished by being far simpler than
>> what I thought first.
>
> I'm just going to remove the useless assertion for now.  What you're
> proposing here may (or may not) be worth doing, but it carries a
> non-zero risk of breaking something somewhere, if anyone is relying on
> the signed-ness of that type.  Removing the assertion is definitely
> safe.

Fine for me. That's indeed possible for an extension.
-- 
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] a few thoughts on the schedule

[Andres Freund]

On 2015-05-13 11:49:45 -0400, Robert Haas wrote:
> On Wed, May 13, 2015 at 11:40 AM, Bruce Momjian  wrote:
> > Let me put a finer point on this --- whatever gets pushed to 9.6
> > unreasonably will be a feature we don't have in 9.5 and will discourage
> > future development.  I know we can't do magic, but now is the time to
> > try.
> 
> And on the flip side, whatever is pushed to 9.6 will not create a
> stability issue in 9.5.

I'm not sure it's that simple. For one I think patches don't age that
well. Often enough patches don't really get significantly more review
when they're delayed, and at the same time the understanding of the
innards reduces again.  But more importantly not integrating things that
are either ready or nearly ready and that have been waiting for a long
while, might be better for stability short term. But I think in the
mid/long term it creates a lack of reviewers, contributors and
eventually committers.

Obviously that's *not* meant as a call to just commit everything
pending.

Greetings,

Andres Freund


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

Re: [HACKERS] a few thoughts on the schedule

[Joshua D. Drake]

On 05/13/2015 08:09 AM, Tom Lane wrote:


What I think we need to be doing this week is triage.  Commit what's
ready, punt what's not.  I'll post a separate list about that.

regards, tom lane


+1

JD


--
Command Prompt, Inc. - http://www.commandprompt.com/  503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Announcing "I'm offended" is basically telling the world you can't
control your own emotions, so everyone else should do it for you.


--
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] Default Roles (was: Additional role attributes)

[Robert Haas]

On Wed, May 13, 2015 at 11:50 AM, Stephen Frost  wrote:
> * Robert Haas (robertmh...@gmail.com) wrote:
>> Yes: let's punt this to 9.6.  The decisions you're making here are way
>> too significant to be making a couple of days before feature freeze,
>> and this patch has changed massively since it was first submitted.
>> There isn't time now for people who want to have an opinion on this to
>> form an educated one.
>
> Perhaps I'm missing something, but the patch has been simplified down to
> the point where the only question seems to be "should we have default
> roles or not?", which I had asked about two weeks ago and again last
> week on a new thread.  I feel like we're waiting for the silent majority
> to chime in.

The thing is, right now, there are many, many patches in flight and
everybody is really, really busy with them.  What we should be trying
to push in right now are the patches that we know we want, and there
are at most a few minor implementation details to sort out.  We should
not be trying to push in any patches where we are not confident in the
design.  I really don't see how you can be confident that this design
will have the backing of the community at this point.  It's changed in
major ways, multiple times.  The latest version, again majorly
revised, was posted TWO DAYS before feature freeze.  Two days is not
enough time to get meaningful feedback on significant design decisions
under the best of circumstances, and even less so when those two days
are the last remaining days before feature freeze.

Now, if six people who are all well-known PostgreSQL contributors show
up and they all say "I looked at the latest version of this carefully
and I'm confident you've got it right", then go ahead: push it.  But
don't make the mistake of thinking that because you're confident that
you've now got it right everybody else will like it too.  Even since
you posted the last version, Heikki expressed a concern that resulted
in (surprise!) more revisions.  There comes a point where a patch that
is still heavily in flux is just too late for the release cycle, and
we're well past that point at this stage of the game.

-- 
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] Triaging the remaining open commitfest items

[Stephen Frost]

Tom, all,

* Tom Lane (t...@sss.pgh.pa.us) wrote:
> * Optimization for updating foreign tables in Postgres FDW
> 
> I concur with Stephen's assessment that this doesn't look well designed.
> I think we should just mark it RWF for now.

I had meant to do that already, sorry about that, done now.

> * iteration over record in plpgsql
> 
> I fear this one slipped through the cracks --- it's marked Waiting on
> Author in the CF app, but it looks like Pavel did submit an updated
> version after that marking was made.  However, it's not a terribly
> significant feature and there was doubt as to whether we want it at
> all anyway.  Suggest we just punt it to next CF at this point.

I had been interested but also thought it was waiting for a new
version.  Doesn't look like I'll have time now.

> * Auditing extension
> 
> I do not get the impression that there is consensus on this.  Push to 9.6?

I've not seen much disagreement about what it provides recently, those
were dealt with a while ago.  I agree with Robert that it needs more
work on documentation and comments and I've sent my thoughts about what
to improve in those areas over to David.  I've reviewed it in various
forms and am hoping to commit it, unless there are objections.

> * RLS: row-level security, more review
> 
> Stephen's baby.

I don't have anything pending on this at the moment.  Post
feature-freeze I'm planning to spend time on bug hunting, documentation
improvements, etc, for this, UPSERT, and other things.  If anyone is
aware of any outstanding issues here, please let me know.

> * Additional role attributes
> 
> Is this ready to commit?  Stephen's call.

I'm pretty happy with the latest version.  Would be great for others to
weigh in on if they have any concerns about reserving the 'pg_' prefix
for system roles (or if they're fine with it, that'd be useful to know
too..).  I'll also be improving the documentation for it.

> * catalog view to pg_hba.conf file
> 
> Greg Stark is marked as committer of record on this.

I was hoping to look at that also, as I do think it'd be valuable to
have.  The current patch needs rework though.  I agree with Peter that
using "keyword_*" arrays is not a good approach and that it'd really be
better to have this in conjunction with a function that users could use
to see what row is returned.  I might have time to work on it tomorrow,
if other things fall into place, but it's not going to be without
changes and perhaps that means it has to punt to 9.6.

> So there you have it.  If everyone would go make decisions on the patches
> that they are the most obvious committer for, we could get those taken
> care of one way or the other pretty quickly.  As for the ones I proposed
> pushing to 9.6, any committer who feels differently can pick those up,
> else I'll go change their status in the CF app tomorrow or so.

Done, for my part.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Triaging the remaining open commitfest items

[Tom Lane]

Andres Freund  writes:
> On 2015-05-13 11:38:27 -0400, Tom Lane wrote:
>> * Manipulating complex types as non-contiguous structures in-memory
>> 
>> This one's mine of course.  I've been hoping to get more independent
>> performance testing than it's gotten, but time grows short.  I'm inclined
>> to just go ahead and push it in.

> I'm a bit hesitant about performance regressions around it. And I'd
> obviously rather not see the macros but the inline version ;). But I
> think overall we're in a better position with it, than without. If it
> turns out to have bad edge cases performancewise, we can still "turn it
> off" in plpgsql without much problems. If we, preferrably, can't find a
> better solution for the performance problem.

Right, I should have said "absorb Andres' input and then commit".  What
I wanted to know was whether there would be objections to committing
this at all.

regards, tom lane


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

Re: [HACKERS] a few thoughts on the schedule

[Andres Freund]

On 2015-05-13 11:52:40 -0400, Tom Lane wrote:
> One thing that continues to bother me about the commitfest process is that
> it's created a default expectation that things get committed eventually.
> But many new ideas are just plain bad, and others are things that nobody
> but the author cares about.  We need to remember that every new feature
> we add creates an ongoing maintenance burden, and might foreclose better
> ideas later.  I'd like to see a higher threshold for accepting feature
> patches than we seem to have applied of late.

Agreed that this is a problem. I think we need to work on giving that
feedback rather sooner than later. It's one thing to be given a -1 a
week or two after a patch gets proposed, another being given it 10
revisions and half a year later.

How about we really try to triage the patches a) before the CF starts,
b) half into the CF?

I guess we'd have to somebody making a summary of each patch, and their
own opinion. Then that list can be discussed.  I don't really like that,
because it involves a fair amount of work and has a good bit of
potential for personal preferences to creep in. But I don't have a
better idea.

If necessary I'll do that for the first CF.

Greetings,

Andres Freund


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

Re: [HACKERS] Triaging the remaining open commitfest items

[Stephen Frost]

* Robert Haas (robertmh...@gmail.com) wrote:
> On Wed, May 13, 2015 at 11:38 AM, Tom Lane  wrote:
> > * Additional role attributes
> >
> > Is this ready to commit?  Stephen's call.
> 
> -1 for committing this, per discussion earlier today on a thread
> that's probably not linked into the CF app.

Linked it into the CF, sorry for not doing so earlier.

I'm not going to push it over objections, but I'm certainly hopeful that
there will be further discussion on that thread.  For my part, at least,
it's a really nice capability to have that we've been missing for a long
time- I can remember back to the first time I was setting up Nagios and
wondering how in the world I could monitor the system without giving the
Nagios user full superuser rights.  I know there are instances out in
the wild that I've set up which are still hacked up to deal with this
lack of capability.

Further, if we don't have something like this, then I'm worried about
people who use the XLOG functions today for monitoring, as we'd end up
having to lock those down to superuser-only, since there isn't anything
between 'public' and 'superuser'.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] subxcnt defined as signed integer in SnapshotData and SerializeSnapshotData

[Michael Paquier]

On Thu, May 14, 2015 at 1:03 AM, Michael Paquier wrote:
> On Thu, May 14, 2015 at 12:02 AM, Robert Haas wrote:
>> I'm just going to remove the useless assertion for now.  What you're
>> proposing here may (or may not) be worth doing, but it carries a
>> non-zero risk of breaking something somewhere, if anyone is relying on
>> the signed-ness of that type.  Removing the assertion is definitely
>> safe.
>
> Fine for me. That's indeed possible for an extension.

Btw, I think that your commit message should have given some credit to
Coverity for finding the problem. Not a big deal 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] Triaging the remaining open commitfest items

[Andrew Dunstan]

On 05/13/2015 11:38 AM, Tom Lane wrote:

* Grouping Sets

I had originally promised to be committer for this one, and still want
to go look at it, but Robert's nearby message about not committing stuff
in haste definitely seems to apply.




That makes me sad. I wish you would still try.

Sadly, my plate is full with other stuff, the last few days absorbed 
more than my available time.


cheers

andrew


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

Re: [HACKERS] Default Roles (was: Additional role attributes)

[Tom Lane]

Robert Haas  writes:
> Now, if six people who are all well-known PostgreSQL contributors show
> up and they all say "I looked at the latest version of this carefully
> and I'm confident you've got it right", then go ahead: push it.  But
> don't make the mistake of thinking that because you're confident that
> you've now got it right everybody else will like it too.  Even since
> you posted the last version, Heikki expressed a concern that resulted
> in (surprise!) more revisions.  There comes a point where a patch that
> is still heavily in flux is just too late for the release cycle, and
> we're well past that point at this stage of the game.

FWIW, I agree that we're past the point where we should be committing
features whose external definition hasn't been stable for awhile.  Fixing
bugs post-feature-freeze is one thing, but if there's a significant chance
that you'll be having to adjust the feature definition, then it's probably
too late for 9.5.  And this particular item sure looks like it's in that
category.

There's always another release cycle.

regards, tom lane


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

Re: [HACKERS] a few thoughts on the schedule

[Tom Lane]

Andres Freund  writes:
> On 2015-05-13 11:52:40 -0400, Tom Lane wrote:
>> One thing that continues to bother me about the commitfest process is that
>> it's created a default expectation that things get committed eventually.

> Agreed that this is a problem. I think we need to work on giving that
> feedback rather sooner than later. It's one thing to be given a -1 a
> week or two after a patch gets proposed, another being given it 10
> revisions and half a year later.

> How about we really try to triage the patches a) before the CF starts,
> b) half into the CF?

We keep talking about doing something like that (I remember it's come up
several times in the annual developer meetings), and then not actually
doing it.  But I agree it seems like a good idea.

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] Triaging the remaining open commitfest items

[Petr Jelinek]

On 13/05/15 17:38, Tom Lane wrote:


* Sequence Access Method

Heikki's marked as reviewer, so it's his call as to whether this is ready,
but the impression I have is that there's not really consensus as to
whether the API is good.  If not, it's something I think we should push
to 9.6.



Heikki said he won't have time for this one before freeze so I guess it 
can be pushed to 9.6.


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


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

Re: [HACKERS] a few thoughts on the schedule

[Joshua D. Drake]

On 05/13/2015 09:27 AM, Tom Lane wrote:

Andres Freund  writes:

On 2015-05-13 11:52:40 -0400, Tom Lane wrote:

One thing that continues to bother me about the commitfest process is that
it's created a default expectation that things get committed eventually.



Agreed that this is a problem. I think we need to work on giving that
feedback rather sooner than later. It's one thing to be given a -1 a
week or two after a patch gets proposed, another being given it 10
revisions and half a year later.



How about we really try to triage the patches a) before the CF starts,
b) half into the CF?


We keep talking about doing something like that (I remember it's come up
several times in the annual developer meetings), and then not actually
doing it.  But I agree it seems like a good idea.


What if:

* Commitfest starts at branch.

* Accept new patches for 9.6 until X date

* At X date, tree is closed and patch review begins

* Patch review continues until all patches are committed, kicked or Y 
date is met.


* At Y date, we go to Alpha

* At Z date, we go to Beta

Well crap, I ran out of sequence letters but you get the idea. In short, 
no more ethereal "We kind of do this on this date sort of". Stick to it, 
it may suck sometimes but it is really the only reasonable way to do it 
anymore.


JD





regards, tom lane





--
Command Prompt, Inc. - http://www.commandprompt.com/  503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Announcing "I'm offended" is basically telling the world you can't
control your own emotions, so everyone else should do it for you.


--
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] subxcnt defined as signed integer in SnapshotData and SerializeSnapshotData

[Robert Haas]

On Wed, May 13, 2015 at 12:17 PM, Michael Paquier
 wrote:
> On Thu, May 14, 2015 at 1:03 AM, Michael Paquier wrote:
>> On Thu, May 14, 2015 at 12:02 AM, Robert Haas wrote:
>>> I'm just going to remove the useless assertion for now.  What you're
>>> proposing here may (or may not) be worth doing, but it carries a
>>> non-zero risk of breaking something somewhere, if anyone is relying on
>>> the signed-ness of that type.  Removing the assertion is definitely
>>> safe.
>>
>> Fine for me. That's indeed possible for an extension.
>
> Btw, I think that your commit message should have given some credit to
> Coverity for finding the problem. Not a big deal though.

The first report I received was from Andres via IM, actually: it
showed up as a compiler warning for him.  I just didn't get around to
doing anything about it before this one showed up.  I could have
explained all that in the commit message, but for a one-line change,
it didn't quite seem worth having 3 or 4 lines to attribute credit.

-- 
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] Triaging the remaining open commitfest items

[Andres Freund]

On 2015-05-13 11:38:27 -0400, Tom Lane wrote:
> * parallel mode/contexts
> 
> Robert's patch, his to deal with (likewise for "assessing parallel-safety").

Just as a note, a large part of this has been committed.


-- 
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] EvalPlanQual behaves oddly for FDW queries involving system columns

[Tom Lane]

Etsuro Fujita  writes:
> On 2015/05/13 6:22, Tom Lane wrote:
>> Of course, if we don't do that then we still have your original gripe
>> about ctid not being correct in EvalPlanQual results.  I poked at this
>> a bit and it seems like we could arrange to pass ctid through even in
>> the ROW_MARK_COPY case, if we define the t_ctid field of a composite
>> Datum as being the thing to use.

> I did the same thing when creating the first version of the patch [1]. 
> In addition, I made another change to ForeignNext so that the FDWs to 
> get ctid coming out as the same value (0, 0) instead of (4294967295,0) 
> before and after an EvalPlanQual recheck.  But IIRC, I think both of 
> them were rejected by you ...

Ah, right.  AFAIR, people objected to the other things you'd done in
that patch, and I'd still say that the change in nodeForeignscan.c
is unnecessary.  But the idea of using t_ctid to solve the problem
for the ROW_MARK_COPY code path seems reasonable enough.

>> We could fix that by
>> adding an ItemPointerSetInvalid(&(td->t_ctid)) call to heap_form_tuple,
>> but I dunno if we want to add even a small number of cycles for this
>> purpose to such a core function.

> I thought so too when creating the first version.

On the other hand, that would only be three additional instructions
on typical machines, which is surely in the noise compared to the rest of
heap_form_tuple, and you could argue that it's a bug fix: leaving the
t_ctid field with an apparently valid value is not very appropriate.
So after thinking about it I think we ought to do that.

regards, tom lane


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

Re: [HACKERS] Triaging the remaining open commitfest items

[Tom Lane]

Andres Freund  writes:
> On 2015-05-13 11:38:27 -0400, Tom Lane wrote:
>> * parallel mode/contexts
>> 
>> Robert's patch, his to deal with (likewise for "assessing parallel-safety").

> Just as a note, a large part of this has been committed.

Right, and Robert commented that he isn't planning to do more here
for 9.5, so probably these CF items should be closed as committed?

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] Triaging the remaining open commitfest items

[Robert Haas]

On Wed, May 13, 2015 at 1:06 PM, Tom Lane  wrote:
> Andres Freund  writes:
>> On 2015-05-13 11:38:27 -0400, Tom Lane wrote:
>>> * parallel mode/contexts
>>>
>>> Robert's patch, his to deal with (likewise for "assessing parallel-safety").
>
>> Just as a note, a large part of this has been committed.
>
> Right, and Robert commented that he isn't planning to do more here
> for 9.5, so probably these CF items should be closed as committed?

Already done.

-- 
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] Streaming replication and WAL archive interactions

[Heikki Linnakangas]

On 05/13/2015 04:29 PM, Robert Haas wrote:

On Wed, May 13, 2015 at 8:53 AM, Heikki Linnakangas  wrote:

Our manual says that archive_command should refuse to overwrite an existing
file. But to work-around the double-archival problem, where the same file is
archived twice, it would be even better if it would simply return success if
the file exists, *and has identical contents*. I don't know how to code that
logic in a simple one-liner though.


This is why we really, really need that pg_copy command that was
proposed a while back.


Yeah..

I took a step back and looked at the big picture again:

If we just implement the "always" mode, and you have a pg_copy command 
or similar that handles duplicates correctly, you don't necessarily need 
the "shared" mode at all. You can just set archive_command='always', and 
have the master and standby archive to the same location. As long as the 
archive_command works correctly and is race-free, that should work.


I cut back the patch to implement just the "always" mode. The "shared" 
mode might still make sense as a future patch, as I think it's easier to 
understand and has less strict requirements for the archive_command, but 
let's take one step at a time.


So attached is a patch that just adds the "always" mode. This is pretty 
close to what Fujii submitted long ago.


- Heikki

From 71332900247a8c68a61fcf60782cb35cf662b756 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas 
Date: Thu, 16 Apr 2015 14:40:24 +0300
Subject: [PATCH 1/1] Add archive_mode='always' option.

In 'always' mode, the standby's WAL archive is taken to be separate from the
primary's, and the standby independently archives all files it receives from
the primary.

Fujii Masao and me.
---
 doc/src/sgml/config.sgml  | 13 +++--
 doc/src/sgml/high-availability.sgml   | 39 +++
 src/backend/access/transam/xlog.c | 22 +--
 src/backend/access/transam/xlogarchive.c  |  5 +++-
 src/backend/postmaster/postmaster.c   | 37 ++---
 src/backend/replication/walreceiver.c | 10 +--
 src/backend/utils/misc/guc.c  | 21 ---
 src/backend/utils/misc/postgresql.conf.sample |  2 +-
 src/include/access/xlog.h | 13 +++--
 9 files changed, 133 insertions(+), 29 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 0d8624a..5549b7d 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -2521,7 +2521,7 @@ include_dir 'conf.d'
 
 
  
-  archive_mode (boolean)
+  archive_mode (enum)
   
archive_mode configuration parameter
   
@@ -2530,7 +2530,16 @@ include_dir 'conf.d'

 When archive_mode is enabled, completed WAL segments
 are sent to archive storage by setting
-.
+. In addition to off,
+to disable, there are two modes: on, and
+always. During normal operation, there is no
+difference between the two modes, but when set to always
+the WAL archiver is enabled also during archive recovery or standby
+mode. In always mode, all files restored from the archive
+or streamed with streaming replication will be archived (again). See
+ for details.
+ 
+   
 archive_mode and archive_command are
 separate variables so that archive_command can be
 changed without leaving archiving mode.
diff --git a/doc/src/sgml/high-availability.sgml b/doc/src/sgml/high-availability.sgml
index a17f555..e93b711 100644
--- a/doc/src/sgml/high-availability.sgml
+++ b/doc/src/sgml/high-availability.sgml
@@ -1220,6 +1220,45 @@ primary_slot_name = 'node_a_slot'
 

   
+
+  
+   Continuous archiving in standby
+
+   
+ continuous archiving
+ in standby
+   
+
+   
+ When continuous WAL archiving is used in a standby, there are two
+ different scenarios: the WAL archive can be shared between the primary
+ and the standby, or the standby can have its own WAL archive. When
+ the standby has its own WAL archive, set archive_mode
+ to always, and the standby will call the archive
+ command for every WAL segment it receives, whether it's by restoring
+ from the archive or by streaming replication. The shared archive can
+ be handled similarly, but the archive_command should test if the file
+ being archived exists already, and if the existing file has identical
+ contents. This requires more care in the archive_command, as it must
+ be careful to not overwrite an existing file with different contents,
+ but return success if the exactly same file is archived twice. And
+ all that must be done free of race conditions, if two servers attempt
+ to archive the same file at the same time.
+   
+
+   
+ If archive_mode is set to on, the
+ archiver is not enabled during recovery or standby mode. If the standby
+

Re: [HACKERS] Proposal : REINDEX xxx VERBOSE

[Sawada Masahiko]

On Thu, May 14, 2015 at 12:28 AM, Robert Haas  wrote:
> On Thu, May 7, 2015 at 6:55 PM, Sawada Masahiko  wrote:
>> Sorry, I forgot attach files.
>
> Review comments:
>
> - Customarily we use int, rather than uint8, for flags variables.  I
> think we should do that here also.
>
> - reindex_index() emits a log message either way, but at DEBUG2 level
> without VERBOSE and at the INFO level with it.  I think we should skip
> it altogether without VERBOSE.  i.e. if (options & REINDEXOPT_VERBOSE)
> ereport(...)
>
> - The errmsg() in that function should not end with a period.
>
> - The 000 patch makes a pointless whitespace change to tab-complete.c.
>
> - Instead of an enumerated type (ReindexOption) just use #define to
> define the flag value.
>
> Apart from those fairly minor issues, I think this looks pretty solid.
>

Thank you for reviwing..
All fixed.

Regards,

---
Sawada Masahiko


000_reindex_verbose_v14.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] Proposal : REINDEX xxx VERBOSE

[Alvaro Herrera]

Uh, are we really using INFO to log this?  I thought that was a
discouraged level to use anymore.  Why not NOTICE?

Also, when multiple tables are reindexed, do we emit lines for each
index, or only for each table?  If we're going to emit a line for each
index in the single-table mode, it seems more sensible to do the same
for the multi-table forms also.

-- 
Á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] Re: Abbreviated keys for Datum tuplesort (was: Re: B-Tree support function number 3 (strxfrm() optimization))

[Robert Haas]

On Fri, Jan 23, 2015 at 4:13 PM, Andrew Gierth
 wrote:
> [pruning the Cc: list and starting a new thread]
>
> Here's the cleaned-up version of the patch to allow abbreviated keys
> when sorting a single Datum. This also removes comments that suggest
> that the caller of tuplesort_begin_datum should ever have to care
> about the abbreviated key optimization.
>
> I'll add this to the CF.

Committed.  Thanks for the patch and your patience.

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

[HACKERS] Re: Abbreviated keys for Datum tuplesort (was: Re: B-Tree support function number 3 (strxfrm() optimization))

[Peter Geoghegan]

On Wed, May 13, 2015 at 11:40 AM, Robert Haas  wrote:
> Committed.  Thanks for the patch and your patience.

This comment was not updated:

/*
 * The sortKeys variable is used by every case other than the datum and
 * hash index cases; it is set by tuplesort_begin_xxx.  tupDesc is only
 * used by the MinimalTuple and CLUSTER routines, though.
 */
TupleDesc tupDesc;
SortSupport sortKeys; /* array of length nKeys */



-- 
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] Proposal : REINDEX xxx VERBOSE

[Sawada Masahiko]

On Thu, May 14, 2015 at 3:10 AM, Alvaro Herrera
 wrote:
> Uh, are we really using INFO to log this?  I thought that was a
> discouraged level to use anymore.  Why not NOTICE?
>

I think it should be INFO level because it is a information of REINDEX
command,such as progress of itself, CPU usage and so on. it would be
overkill if we output the logs as NOTICE level, and verbose outputs of
other maintenance command are emitted as INFO level.

> Also, when multiple tables are reindexed, do we emit lines for each
> index, or only for each table?  If we're going to emit a line for each
> index in the single-table mode, it seems more sensible to do the same
> for the multi-table forms also.
>

I agree that we emit lines for each table when we do reindex multiple tables.
The latest patch is attached.


Regards,

---
Sawada Masahiko


000_reindex_verbose_v15.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] KNN-GiST with recheck

[Heikki Linnakangas]

On 04/17/2015 12:05 PM, Alexander Korotkov wrote:

On Wed, Feb 25, 2015 at 12:15 PM, Alexander Korotkov 
wrote:


Hi!

On Tue, Feb 24, 2015 at 5:39 PM, Tomas Vondra <
tomas.von...@2ndquadrant.com> wrote:


On 17.2.2015 14:21, Alexander Korotkov wrote:

On Sun, Feb 15, 2015 at 2:08 PM, Alexander Korotkov
mailto:aekorot...@gmail.com>> wrote:

Revised patch with reordering in GiST is attached
(knn-gist-recheck-in-gist.patch) as well as testing script (test.py).


I meant to do a bit of testing on this (assuming it's still needed), but
the patches need rebasing - Heikki fixed a few issues, so they don't
apply cleanly.



Both patches are revised.



Both patches are rebased against current master.


This looks pretty much ready. I'm going to spend some time on this on 
Friday, and if all looks good, commit. (Thursday's a public holiday here).


One quick comment:

It would be good to avoid the extra comparisons of the distances, when 
the index doesn't return any lossy items. As the patch stands, it adds 
one extra copyDistances() call and a cmp_distances() call for each tuple 
(in a knn-search), even if there are no lossy tuples.


- Heikki



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

Re: [HACKERS] KNN-GiST with recheck

[Alexander Korotkov]

On Wed, May 13, 2015 at 10:16 PM, Heikki Linnakangas 
wrote:

> On 04/17/2015 12:05 PM, Alexander Korotkov wrote:
>
>> On Wed, Feb 25, 2015 at 12:15 PM, Alexander Korotkov <
>> aekorot...@gmail.com>
>> wrote:
>>
>>  Hi!
>>>
>>> On Tue, Feb 24, 2015 at 5:39 PM, Tomas Vondra <
>>> tomas.von...@2ndquadrant.com> wrote:
>>>
>>>  On 17.2.2015 14:21, Alexander Korotkov wrote:

> On Sun, Feb 15, 2015 at 2:08 PM, Alexander Korotkov
> mailto:aekorot...@gmail.com>> wrote:
>
> Revised patch with reordering in GiST is attached
> (knn-gist-recheck-in-gist.patch) as well as testing script (test.py).
>

 I meant to do a bit of testing on this (assuming it's still needed), but
 the patches need rebasing - Heikki fixed a few issues, so they don't
 apply cleanly.


>>> Both patches are revised.
>>>
>>>
>> Both patches are rebased against current master.
>>
>
> This looks pretty much ready. I'm going to spend some time on this on
> Friday, and if all looks good, commit. (Thursday's a public holiday here).
>

Very good, thanks!


> One quick comment:
>
> It would be good to avoid the extra comparisons of the distances, when the
> index doesn't return any lossy items. As the patch stands, it adds one
> extra copyDistances() call and a cmp_distances() call for each tuple (in a
> knn-search), even if there are no lossy tuples.


I will fix it until Friday.

--
With best regards,
Alexander Korotkov.

[HACKERS] Re: Abbreviated keys for Datum tuplesort (was: Re: B-Tree support function number 3 (strxfrm() optimization))

[Robert Haas]

On Wed, May 13, 2015 at 2:44 PM, Peter Geoghegan  wrote:
> On Wed, May 13, 2015 at 11:40 AM, Robert Haas  wrote:
>> Committed.  Thanks for the patch and your patience.
>
> This comment was not updated:
>
> /*
>  * The sortKeys variable is used by every case other than the datum and
>  * hash index cases; it is set by tuplesort_begin_xxx.  tupDesc is only
>  * used by the MinimalTuple and CLUSTER routines, though.
>  */
> TupleDesc tupDesc;
> SortSupport sortKeys; /* array of length nKeys */

Oops.  Fixed.  Thanks.

-- 
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 : REINDEX xxx VERBOSE

[Robert Haas]

On Wed, May 13, 2015 at 2:10 PM, Alvaro Herrera
 wrote:
> Uh, are we really using INFO to log this?  I thought that was a
> discouraged level to use anymore.  Why not NOTICE?

Well, as Masahiko-san points out, VACUUM uses INFO.  I can't see any
good reason to make this different.

> Also, when multiple tables are reindexed, do we emit lines for each
> index, or only for each table?  If we're going to emit a line for each
> index in the single-table mode, it seems more sensible to do the same
> for the multi-table forms also.

Hmm, yeah, I agree with that.  I thought the patch worked that way,
but I see now that it doesn't.  I think that should be changed.

-- 
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] PATCH: pgbench allow '=' in \set

[Fabien COELHO]

I also don't like to restate what has already been said.  \set at the
beginning of the line tells you that you will be setting a variable.
Adding = or := only restates the same thing.  I agree it superficially
looks a little nicer, but I'm not sure it's really going to add
clarity, because it's basically just redundant.


Ok. I've marked this one as REJECTED.


Otherwise, I was considering this kind of things:

  n := expr

If we have functions, that could include:

  n := random(1, 100)

with more work (handling of double constants):

  n := exprandom(1, 100, 3.5)

and maybe:

  n := SELECT ...

or even:

  n, m, p, q := SELECT ...

Also, having ";" as a end of commands could also help by allowing 
multiline commands, but that would break compatibility. Maybe allowing 
continuations (\\\n) would be an acceptable compromise.


--
Fabien.


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

Re: [HACKERS] Proposal : REINDEX xxx VERBOSE

[Alvaro Herrera]

Robert Haas wrote:
> On Wed, May 13, 2015 at 2:10 PM, Alvaro Herrera
>  wrote:
> > Uh, are we really using INFO to log this?  I thought that was a
> > discouraged level to use anymore.  Why not NOTICE?
> 
> Well, as Masahiko-san points out, VACUUM uses INFO.  I can't see any
> good reason to make this different.

I was misremembering the INFO situation.  Here's one item in the
archives I found in a very quick search, which says that INFO is the
right thing to use:
http://www.postgresql.org/message-id/24874.1231183...@sss.pgh.pa.us

Cheers

-- 
Á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] Re: [COMMITTERS] pgsql: Map basebackup tablespaces using a tablespace_map file

[Andrew Dunstan]

[redirecting to -hackers]

On 05/12/2015 01:30 PM, Amit Kapila wrote:
On Tue, May 12, 2015 at 9:02 PM, Andrew Dunstan > wrote:



On 05/12/2015 10:33 AM, Heikki Linnakangas wrote:

On 05/12/2015 04:42 PM, Andrew Dunstan wrote:

+
+   /*
+* Remove the existing symlink if any and
Create the symlink
+* under PGDATA.  We need to use rmtree
instead of rmdir as
+* the link location might contain
directories or files
+* corresponding to the actual path. Some
tar utilities do
+* things that way while extracting symlinks.
+*/
+   if (lstat(linkloc, &st) == 0 &&
S_ISDIR(st.st_mode))
+   {
+   if (!rmtree(linkloc,true))
+   ereport(ERROR,
+  (errcode_for_file_access(),
+errmsg("could not remove
directory \"%s\": %m",
+   linkloc)));
+   }
+   else
+   {
+   if (unlink(linkloc) < 0 && errno !=
ENOENT)
+   ereport(ERROR,
+  (errcode_for_file_access(),
+errmsg("could not remove
symbolic link \"%s\": %m",
+   linkloc)));
+   }
+


That's scary. What tar utilitiy replaces the symlink with a
non-empty directory?


IIRC, it was star utility by using --copysymlinks options.
It will actually copy the symlink data at appropriate location,
but will not maintain symlink after extraction.
I don't have a link handly for it, but can again search for
it and send you if you want to have a look.

What if you call pg_start_backup() and take the backup with a
utility that follows symlinks? I wouldn't recommend using such
a tool, but with this patch, it will zap all the tablespaces.
Before this, you at least got a working database and could
read out all the data or fix the symlinks afterwards.



Yeah, but I don't think user will do such a thing without
being aware of the same and if he is aware, he will either
restore the symlinks before starting the server or would
atleast keep a copy of such a backup until he is able to
restore the database completely.

Do you think adding a note in docs makes sense?




How about if we simply abort if we find a non-symlink where we want the 
symlink to be, and only remove something that is actually a symlink (or 
a junction point, which is more or less the same thing)? Then it would 
be up to the user to recover the situation, by moving or removing the 
offending file/directory, and trying again.


cheers

andrew


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

[HACKERS] Fix token exceeding NAMELEN

[Aaron W. Swenson]

Trying to build HEAD and ran into this issue building the docs:

openjade:logicaldecoding.sgml:575:62:Q: length of name token must
not exceed NAMELEN (44)
openjade:replication-origins.sgml:87:67:Q: length of name token must
not exceed NAMELEN (44)

I've tried playing with the flags we (you) pass to openjade, but
couldn't make it override NAMELEN.

So, I've attached a patch that'll fix it.
From 231b317e2eda1f63ff3f5485105c4e8ac1f36146 Mon Sep 17 00:00:00 2001
From: "Aaron W. Swenson" 
Date: Wed, 13 May 2015 15:42:46 -0400
Subject: [PATCH] Fix token exceeding NAMELEN

---
 doc/src/sgml/logicaldecoding.sgml | 2 +-
 doc/src/sgml/replication-origins.sgml | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/src/sgml/logicaldecoding.sgml b/doc/src/sgml/logicaldecoding.sgml
index f817af3..00f6eee 100644
--- a/doc/src/sgml/logicaldecoding.sgml
+++ b/doc/src/sgml/logicaldecoding.sgml
@@ -572,7 +572,7 @@ typedef void (*LogicalDecodeChangeCB) (
  
 
 
- 
+ 
  Origin Filter Callback
 
  
diff --git a/doc/src/sgml/replication-origins.sgml b/doc/src/sgml/replication-origins.sgml
index 5a4b4cb..40fcc6d 100644
--- a/doc/src/sgml/replication-origins.sgml
+++ b/doc/src/sgml/replication-origins.sgml
@@ -84,7 +84,7 @@
   generated by the session is tagged with the replication origin of the
   generating session.  This allows to treat them differently in the output
   plugin, e.g. ignoring all but locally originating rows.  Additionally
-  the 
+  the 
   filter_by_origin_cb callback can be used
   to filter the logical decoding change stream based on the
   source. While less flexible, filtering via that callback is
-- 
2.3.6



signature.asc
Description: Digital signature

Re: [HACKERS] Final Patch for GROUPING SETS

[Andres Freund]

On 2015-05-12 05:36:19 +0200, Andres Freund wrote:
> > Another controversial item was the introduction of GroupedVar. The need
> > for this can be avoided by explicitly setting to NULL the relevant
> > columns of the representative group tuple when evaluating result rows,
> > but (a) I don't think that's an especially clean approach (though I'm
> > not pushing back very hard on it) and (b) the logic needed in its
> > absence is different between the current chaining implementation and a
> > possible union implementation, so I decided against making any changes
> > on wasted-effort grounds.
> 
> Seems like fairly minor point to me.  I very tentatively lean towards
> setting the columns in the group tuple to NULL.

I'm pretty sure by now that I dislike the introduction of GroupedVar,
and not just tentatively.  While I can see why you found its
introduction to be nicer than fiddling with the result tuple, for me the
disadvantages seem to outweigh the advantage.  For one it's rather wierd
to have Var nodes be changed into GroupedVar in setrefs.c.  The number
of places that need to be touched even when it's a 'planned stmt only'
type of node is still pretty large.

Andrew: I'll work on changing this in a couple hours unless you're
speaking up about doing it yourself.

Greetings,

Andres Freund


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

Re: [HACKERS] PATCH: adaptive ndistinct estimator v4

[Jeff Janes]

On Thu, Apr 30, 2015 at 6:20 PM, Tomas Vondra 
wrote:

> Hi,
>
> On 04/30/15 22:57, Robert Haas wrote:
>
>> On Tue, Mar 31, 2015 at 3:02 PM, Tomas Vondra
>>  wrote:
>>
>>> attached is v4 of the patch implementing adaptive ndistinct estimator.
>>>
>>
>> So, I took a look at this today. It's interesting work, but it looks
>> more like a research project than something we can commit to 9.5. As
>> far as I can see, this still computes the estimate the way we do
>> today, but then also computes the estimate using this new method.
>> The estimate computed the new way isn't stored anywhere, so this
>> doesn't really change behavior, except for printing out a WARNING
>> comparing the values produced by the two estimators.
>>
>
> I agree that this is not ready for 9.5 - it was meant as an experiment
> (hence printing the estimate in a WARNING, to make it easier to compare
> the value to the current estimator). Without that it'd be much more
> complicated to compare the old/new estimates, but you're right this is
> not suitable for commit.
>

With the warning it is very hard to correlate the discrepancy you do see
with which column is causing it, as the warnings don't include table or
column names (Assuming of course that you run it on a substantial
database--if you just run it on a few toy cases then the warning works
well).

If we want to have an explicitly experimental patch which we want people
with interesting real-world databases to report back on, what kind of patch
would it have to be to encourage that to happen?  Or are we never going to
get such feedback no matter how friendly we make it?  Another problem is
that you really need to have the gold standard to compare them to, and
getting that is expensive (which is why we resort to sampling in the first
place).  I don't think there is much to be done on that front other than
bite the bullet and just do it--perhaps only for the tables which have
discrepancies.

Some of the regressions I've seen are at least partly a bug:

+   /* find the 'm' value minimizing the difference */
+   for (m = 1; m <= total_rows; m += step)
+   {
+   double q = k / (sample_rows * m);

sample_rows and m are both integers, and their product overflows
vigorously.  A simple cast to double before the multiplication fixes the
first example I produced.  The estimate goes from 137,177 to 1,108,076.
The reality is 1,062,223.

Perhaps m should be just be declared a double, as it is frequently used in
double arithmetic.



>  Leaving that aside, at some point, you'll say, "OK, there may be some
>> regressions left but overall I believe this is going to be a win in
>> most cases". It's going to be really hard for anyone, no matter how
>> smart, to figure out through code review whether that is true. So
>> committing this figures to be extremely frightening. It's just not
>> going to be reasonably possible to know what percentage of users are
>> going to be more happy after this change and what percentage are
>> going to be less happy.
>>
>
> For every pair of estimators you can find cases where one of them is
> better than the other one. It's pretty much impossible to find an estimator
> that beats all other estimators on all possible inputs.
>
> There's no way to make this an improvement for everyone - it will produce
> worse estimates in some cases, and we have to admit that. If we think this
> is unacceptable, we're effectively stuck with the current estimator forever.
>
>  Therefore, I think that:
>>
>> 1. This should be committed near the beginning of a release cycle,
>> not near the end. That way, if there are problem cases, we'll have a
>> year or so of developer test to shake them out.
>>
>
It can't hurt, but how effective will it be?  Will developers know or care
whether ndistinct happened to get better or worse while they are working on
other things?  I would think that problems will be found by focused
testing, or during beta, and probably not by accidental discovery during
the development cycle.  It can't hurt, but I don't know how much it will
help.



>  2. There should be a compatibility GUC to restore the old behavior.
>> The new behavior should be the default, because if we're not
>> confident that the new behavior will be better for most people, we
>> have no business installing it in the first place (plus few people
>> will try it). But just in case it turns out to suck for some people,
>> we should provide an escape hatch, at least for a few releases.
>>
>
> I think a "compatibility GUC" is a damn poor solution, IMNSHO.
>

> For example, GUCs are database-wide, but I do expect the estimator to
> perform worse only on a few data sets / columns. So making this a
> column-level settings would be more appropriate, I think.
>
> But it might work during the development cycle, as it would make comparing
> the estimators possible (just run the tests with the GUC set differently).
> Assuming we'll re-evaluate it at the end, and remove the GUC if possible.


I agree with the "ex

[HACKERS] i feel like compelled !

[essam Gndelee essam]

hi
i don't want to make this post long.  i just started to use PostgreSQL and
it is absolutely awesome . just want to say thank you very much

Re: [HACKERS] i feel like compelled !

[Gavin Flower]

On 14/05/15 09:35, essam Gndelee essam wrote:

hi
i don't want to make this post long.  i just started to use PostgreSQL 
and it is absolutely awesome . just want to say thank you very much

You're allowed to say more!!!  :-)

What particular features do you like best, and why?

What O/S, hardware, use cases (what are you using pg for), volumes of 
data, performance...


What other Databases have you used?

How much experience do you have?


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] Fix token exceeding NAMELEN

[Tom Lane]

"Aaron W. Swenson"  writes:
> Trying to build HEAD and ran into this issue building the docs:
> openjade:logicaldecoding.sgml:575:62:Q: length of name token must
> not exceed NAMELEN (44)
> openjade:replication-origins.sgml:87:67:Q: length of name token must
> not exceed NAMELEN (44)

Hmm ... that's odd.  I don't see any such failure here, and the buildfarm
members that build the docs aren't complaining either.  What version of
openjade are you using exactly?

> So, I've attached a patch that'll fix it.

I have no particular objection to the patch as stated, but I'm just
wondering if this is the tip of a tool compatibility iceberg we were
not previously aware of.

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] Fix token exceeding NAMELEN

[Christopher Browne]

On 13 May 2015 at 17:55, Tom Lane  wrote:

> "Aaron W. Swenson"  writes:
> > Trying to build HEAD and ran into this issue building the docs:
> > openjade:logicaldecoding.sgml:575:62:Q: length of name token must
> > not exceed NAMELEN (44)
> > openjade:replication-origins.sgml:87:67:Q: length of name token must
> > not exceed NAMELEN (44)
>
> Hmm ... that's odd.  I don't see any such failure here, and the buildfarm
> members that build the docs aren't complaining either.  What version of
> openjade are you using exactly?
>
> > So, I've attached a patch that'll fix it.
>
> I have no particular objection to the patch as stated, but I'm just
> wondering if this is the tip of a tool compatibility iceberg we were
> not previously aware of.
>

I recall us hitting this with Slony documentation.  The NAMELEN limit
lay in the SGML/DocBook configuration that was configured at the
distribution level, so that it differed (crucially) betwen Debian and
Red Hat.

Red Hat used to have a lower name length limit, and while overriding
it was technically possible, it required modifying configuration that
the distribution thought was owned by one of the SGML packages,
and hence the modification seemed pretty inadvisable.

I thought that this restriction was alleviated years ago, so I'm a bit
surprised to see this come up in 2015.  (Or perhaps Gentoo hasn't
yet opened up some limits???  :-) )
-- 
When confronted by a difficult problem, solve it by reducing it to the
question, "How would the Lone Ranger handle this?"

Re: [HACKERS] PATCH: pgbench allow '=' in \set

[Robert Haas]

On Wed, May 13, 2015 at 3:54 PM, Fabien COELHO  wrote:
> Ok. I've marked this one as REJECTED.
>
> Otherwise, I was considering this kind of things:
>
>   n := expr
>   n, m, p, q := SELECT ...
>
> Also, having ";" as a end of commands could also help by allowing multiline
> commands, but that would break compatibility. Maybe allowing continuations
> (\\\n) would be an acceptable compromise.

It's been my assumption that we wanted to keep the existing pgbench
syntax mostly intact, and extend it.  We could, of course, invent a
completely new syntax, but it might be hard to get everybody to agree
on what the new thing should look like.

I loathe violently the convention of using a backslash at the end of a
line, because it's too easy to write backslash-space-newline or
backslash-tab-newline when you meant to write backslash-newline.  But
maybe we should do it anyway.  We certainly need some solution to that
problem, because the status quo is monumentally annoying, and that
might be the least bad solution available.

Another option, breaking backward compatibility, would be to decide
that backslash commands have to be terminated by a semicolon token.
Then we wouldn't need a continuation character, because we'd just
continue across lines until we hit the terminator.  Of course, that
doesn't solve the problem for people who want to include multi-line
SQL queries.  If we wanted to make that work, the best option might be
to duplicate the backend lexer into pgbench just as we already do with
psql.  Then it could identify properly-terminated SQL queries
automatically.  That, too, would be a backward compatibility break,
since the terminating semicolon would become required there as well.

I somewhat lean toward this second option, because I think it will be
a lot more convenient in the long run.  We'll probably get some
complains about breaking people's pgbench scripts, but I'd personally
be prepared to accept that as the price of progress.

-- 
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] i feel like compelled !

[Gianni]

Oh well... then, THANKS GUYS!!!

I'm not the original poster, btw.

I felt a bit 'abandoned' a while back, since I started using 
Interbase/Firebird since, like, ~2000.  But since Firebird never really took 
off, I felt I had to look for better solutions.  I worked with Oracle for a 
bit, and then MySQL.  But I found Oracle to be expensive (obviously) and too 
intrusive into my OS (Linux, many flavours, but mostly RedHat-based).

What I really liked about Firebird, and then Postgres made me feel right at 
home, was standards-compliance with SQL and great feature set.  I find myself 
most-often-than-not guessing how something ought to work in Postgres, based on 
past experiences, and finding that it works exactly (mostly) like it 'should'.  
Plus, I found many new things that I loved and changed the way I think about 
stuff, like using Python for SP, JSON fields and RegEx in WHERE.  And a special 
mention to the Async NOTIFY stuff which finally works like it 'should' in a DB 
(Firebird had something like that, but with no payload).

Also, how postgres is easy to deploy really helps.  For example, I use it with 
a Qt App, which is compiled in MinGW.  So I recompiled libpq with the same 
compiler, thus avoiding extra DLLs.  

Thanks!



On Thursday 14 May 2015 09:48:35 Gavin Flower wrote:
> On 14/05/15 09:35, essam Gndelee essam wrote:
> > hi
> > i don't want to make this post long.  i just started to use PostgreSQL
> > and it is absolutely awesome . just want to say thank you very much
> 
> You're allowed to say more!!!  :-)
> 
> What particular features do you like best, and why?
> 
> What O/S, hardware, use cases (what are you using pg for), volumes of
> data, performance...
> 
> What other Databases have you used?
> 
> How much experience do you have?
> 
> 
> Cheers,
> Gavin

Re: [HACKERS] BRIN range operator class

[Alvaro Herrera]

Emre Hasegeli wrote:
> > I pushed patches 04 and 07, as well as adopting some of the changes to
> > the regression test in 06.  I'm afraid I caused a bit of merge pain for
> > you -- sorry about that.
> 
> No problem.  I rebased the remaining ones.

Thanks!

After some back-and-forth between Emre and me, here's an updated patch.
My changes are cosmetic; for a detailed rundown, see
https://github.com/alvherre/postgres/commits/brin-inclusion

Note that datatype point was removed: it turns out that unless we get
box_contain_pt changed to use FPlt() et al, indexes created with this
opclass would be corrupt.  And we cannot simply change box_contain_pt,
because that would break existing GiST and SP-GiST indexes that use it
today and pg_upgrade to 9.5!  So that needs to be considered separately.
Also, removing point support means remove the CAST support procedure,
because there is no use for it in the supported types.  Also, patch 05
in the previous submissions goes away completely because there's no need
for those (box,point) operators anymore.

There's nothing Earth-shattering here that hasn't been seen in previous
submissions by Emre.

One item of note is that this patch is blindly removing the assert-only
blocks as previously discussed, without any replacement.  Need to think
more on how to put something back ...

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/doc/src/sgml/brin.sgml b/doc/src/sgml/brin.sgml
index 92dac7c..b26bcf3 100644
--- a/doc/src/sgml/brin.sgml
+++ b/doc/src/sgml/brin.sgml
@@ -72,7 +72,9 @@
  
   The minmax
   operator classes store the minimum and the maximum values appearing
-  in the indexed column within the range.
+  in the indexed column within the range.  The inclusion
+  operator classes store a value which includes the values in the indexed
+  column within the range.
  
 
  
@@ -252,6 +254,18 @@
  
 
 
+ inet_inclusion_ops
+ inet
+ 
+  &&
+  >>
+  >>=
+  <<
+  <<=
+  =
+ 
+
+
  bpchar_minmax_ops
  character
  
@@ -373,6 +387,25 @@
  
 
 
+ range_inclusion_ops
+ any range type
+ 
+  &&
+  &>
+  &<
+  >>
+  <<
+  <@
+  =
+  @>
+  <
+  <=
+  =
+  >=
+  >
+ 
+
+
  pg_lsn_minmax_ops
  pg_lsn
  
@@ -383,6 +416,43 @@
   >
  
 
+
+ box_inclusion_ops
+ box
+ 
+  &&
+  &>
+  &<
+  >>
+  <<
+  <@
+  ~=
+  @>
+  &>|
+  |&<
+  >>|
+  |<<
+ 
+
+
+ point_box_inclusion_ops
+ point
+ 
+  &&
+  &>
+  &<
+  >>
+  <<
+  <@
+  ~=
+  &>|
+  |&<
+  >>|
+  |<<
+  >^
+  |<^
+ 
+

   
  
diff --git a/src/backend/access/brin/Makefile b/src/backend/access/brin/Makefile
index ac44fcd..fb36882 100644
--- a/src/backend/access/brin/Makefile
+++ b/src/backend/access/brin/Makefile
@@ -13,6 +13,6 @@ top_builddir = ../../../..
 include $(top_builddir)/src/Makefile.global
 
 OBJS = brin.o brin_pageops.o brin_revmap.o brin_tuple.o brin_xlog.o \
-	   brin_minmax.o
+		brin_minmax.o brin_inclusion.o
 
 include $(top_srcdir)/src/backend/common.mk
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 2b5fb8d..1995125 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -105,11 +105,6 @@ brininsert(PG_FUNCTION_ARGS)
 		BrinMemTuple *dtup;
 		BlockNumber heapBlk;
 		int			keyno;
-#ifdef USE_ASSERT_CHECKING
-		BrinTuple  *tmptup;
-		BrinMemTuple *tmpdtup;
-		Size 		tmpsiz;
-#endif
 
 		CHECK_FOR_INTERRUPTS();
 
@@ -137,45 +132,6 @@ brininsert(PG_FUNCTION_ARGS)
 
 		dtup = brin_deform_tuple(bdesc, brtup);
 
-#ifdef USE_ASSERT_CHECKING
-		{
-			/*
-			 * When assertions are enabled, we use this as an opportunity to
-			 * test the "union" method, which would otherwise be used very
-			 * rarely: first create a placeholder tuple, and addValue the
-			 * value we just got into it.  Then union the existing index tuple
-			 * with the updated placeholder tuple.  The tuple resulting from
-			 * that union should be identical to the one resulting from the
-			 * regular operation (straight addValue) below.
-			 *
-			 * Here we create the tuple to compare with; the actual comparison
-			 * is below.
-			 */
-			tmptup = brin_form_placeholder_tuple(bdesc, heapBlk, &tmpsiz);
-			tmpdtup = brin_deform_tuple(bdesc, tmptup);
-			for (keyno = 0; keyno < bdesc->bd_tupdesc->natts; keyno++)
-			{
-BrinValues *bval;
-FmgrInfo   *addValue;
-
-bval = &tmpdtup->bt_columns[keyno];
-addValue = index_getprocinfo(idxRel, keyno + 1,
-			 BRIN_PROCNUM_ADDVALUE);
-FunctionCall4Coll(addValue,
-  idxRel->rd_indcollation[keyno],
-  PointerGetDatum(bdesc),
-  PointerGetDatum(bval),
-  values[keyno],
-  n

[HACKERS] pgAdmin4 Bug fix or my Fault ?

[Seçkin Alan]

Hi,
I am using Debian Jessie and install pgAdmin4 Required modules.
after I clone pgAdmin4 from
http://git.postgresql.org/gitweb/?p=pgadmin4.git;a=summary ,

First of, ıf I want run setup.py, I must fix bug .
after I want run pgadmin4.py, I must fix gravatar import line.

In this case,
There are really bugs?
or
is it my fault ?
 Thank you.

diff --git a/web/pgadmin/browser/views.py b/web/pgadmin/browser/views.py
index 07637be..f336f31 100644
--- a/web/pgadmin/browser/views.py
+++ b/web/pgadmin/browser/views.py
@@ -11,7 +11,7 @@
 MODULE_NAME = 'browser'

 from flask import Blueprint, Response, current_app, render_template, url_for
-from flaskext.gravatar import Gravatar
+from flask_gravatar import Gravatar
 from flask.ext.security import login_required
 from flask.ext.login import current_user
 from inspect import getmoduleinfo, getmembers
@@ -223,4 +223,4 @@ def get_nodes():
 return resp


-
\ No newline at end of file
+
diff --git a/web/setup.py b/web/setup.py
index c7398f5..da6652e 100644
--- a/web/setup.py
+++ b/web/setup.py
@@ -59,7 +59,9 @@ def do_setup():
 db.create_all()
 user_datastore.create_role(name='Administrators',
description='pgAdmin Administrators Role')
 user_datastore.create_user(email=email, password=password)
+db.session.commit()
 user_datastore.add_role_to_user(email, 'Administrators')
+

 # Get the user's ID and create the default server group
 user = User.query.filter_by(email=email).first()
@@ -138,4 +140,5 @@ if os.path.isfile(config.SQLITE_PATH):
 do_upgrade()
 else:
 print "The configuration database %s does not exist.\nEntering
initial setup mode...\n" % config.SQLITE_PATH
-do_setup()
\ No newline at end of file
+do_setup()



-- 
Seçkin ALAN
http://sckn.org


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

Re: [HACKERS] pgAdmin4 Bug fix or my Fault ?

[Bruce Momjian]

I think you want PGAdmin support:

http://www.pgadmin.org/support/list.php

Also, why isn't the non-subscribe email address listed on that webpage?

---

On Thu, May 14, 2015 at 12:11:15AM +0300, Seçkin Alan wrote:
> Hi,
> I am using Debian Jessie and install pgAdmin4 Required modules.
> after I clone pgAdmin4 from
> http://git.postgresql.org/gitweb/?p=pgadmin4.git;a=summary ,
> 
> First of, ıf I want run setup.py, I must fix bug .
> after I want run pgadmin4.py, I must fix gravatar import line.

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

  + Everyone has their own god. +


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

Re: [HACKERS] Proposal : REINDEX xxx VERBOSE

[Sawada Masahiko]

On Thu, May 14, 2015 at 4:53 AM, Robert Haas  wrote:
> On Wed, May 13, 2015 at 2:10 PM, Alvaro Herrera
>  wrote:
>> Uh, are we really using INFO to log this?  I thought that was a
>> discouraged level to use anymore.  Why not NOTICE?
>
> Well, as Masahiko-san points out, VACUUM uses INFO.  I can't see any
> good reason to make this different.
>
>> Also, when multiple tables are reindexed, do we emit lines for each
>> index, or only for each table?  If we're going to emit a line for each
>> index in the single-table mode, it seems more sensible to do the same
>> for the multi-table forms also.
>
> Hmm, yeah, I agree with that.  I thought the patch worked that way,
> but I see now that it doesn't.  I think that should be changed.
>

The v15 patch emits a line for each table when reindexing multiple
tables, and emits a line for each index when reindexing single table.
But v14 patch emits a line for each index, regardless of reindex target.
Should I change back to v14 patch?

Regards,

---
Sawada Masahiko


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

Re: [HACKERS] Proposal : REINDEX xxx VERBOSE

[Fabrízio de Royes Mello]

On Wed, May 13, 2015 at 2:49 PM, Sawada Masahiko 
wrote:
>
> On Thu, May 14, 2015 at 12:28 AM, Robert Haas 
wrote:
> > On Thu, May 7, 2015 at 6:55 PM, Sawada Masahiko 
wrote:
> >> Sorry, I forgot attach files.
> >
> > Review comments:
> >
> > - Customarily we use int, rather than uint8, for flags variables.  I
> > think we should do that here also.
> >
> > - reindex_index() emits a log message either way, but at DEBUG2 level
> > without VERBOSE and at the INFO level with it.  I think we should skip
> > it altogether without VERBOSE.  i.e. if (options & REINDEXOPT_VERBOSE)
> > ereport(...)
> >
> > - The errmsg() in that function should not end with a period.
> >
> > - The 000 patch makes a pointless whitespace change to tab-complete.c.
> >
> > - Instead of an enumerated type (ReindexOption) just use #define to
> > define the flag value.
> >
> > Apart from those fairly minor issues, I think this looks pretty solid.
> >
>
> Thank you for reviwing..
> All fixed.
>

IMHO we don't need "pg_rusage_init(&ru0)" if the verbose options is not
setted. Maybe change:

+
+pg_rusage_init(&ru0);

to

+
+if (options & REINDEXOPT_VERBOSE)
+pg_rusage_init(&ru0);


Regards,

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

Re: [HACKERS] Manipulating complex types as non-contiguous structures in-memory

[Tom Lane]

Andres Freund  writes:
> On 2015-05-10 12:09:41 -0400, Tom Lane wrote:
>>> * I find the ARRAY_ITER_VARS/ARRAY_ITER_NEXT macros rather ugly. I don't
>>> buy the argument that turning them into functions will be slower. I'd
>>> bet the contrary on common platforms.

>> Perhaps; do you want to do some testing and see?

> I've added new iterator functions using a on-stack state variable and
> array_iter_setup/next functions pretty analogous to the macros. And then
> converted arrayfuncs.c to use them.

I confirm that this doesn't seem to be any slower (at least not on a
compiler with inline functions).  And it's certainly less ugly, so I've
adopted it.

> Similarly using inline funcs for AARR_NDIMS/HASNULL does not appear to
> hamper performance and gets rid of the multiple evaluation risk.

I'm less excited about that part though.  The original ARR_FOO macros
mostly have multiple-evaluation risks as well, and that's been totally
academic so far.  By the time you get done dealing with the
STATIC_IF_INLINE dance, it's quite messy to have these be inline
functions, and I am not seeing a useful return from adding the mess.

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] Manipulating complex types as non-contiguous structures in-memory

[Andres Freund]

On 2015-05-13 20:28:52 -0400, Tom Lane wrote:
> Andres Freund  writes:
> > Similarly using inline funcs for AARR_NDIMS/HASNULL does not appear to
> > hamper performance and gets rid of the multiple evaluation risk.
> 
> I'm less excited about that part though.  The original ARR_FOO macros
> mostly have multiple-evaluation risks as well, and that's been totally
> academic so far.

Fair point.


-- 
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] Triaging the remaining open commitfest items

[Kouhei Kaigai]

> > * ctidscan as an example of custom-scan
> >
> > This basically hasn't gotten any attention, which may mean nobody cares
> > enough to justify putting it in the tree.  We need to either push it to
> > next CF or reject altogether.
> 
> Agreed.  I was fine with never committing this.  I don't think we have
> a requirement that every hook or bit of functionality we expose at the
> C level must have an example in core.  But other people (you?  Simon?)
> seemed to want a demonstration in the core repository.  If that's
> still a priority, I am willing to work on it more for 9.6, but there
> is not time now.
>
If no other people required it again, I don't think this module should
be kept in core and also I'm not favor to push ctidscan to v9.6 development
cycle. It intends to demonstrate custom-scan interface, however, it is
not certain an example always needs to be in-core.
If we split the patch partially, definition below makes sense to implement
out-of-core example module

+#define TIDNotEqualOperator402
 DATA(insert OID = 2799 (  "<" PGNSP PGUID b f f27  27  16 2800
 DESCR("less than");
 #define TIDLessOperator2799
 DATA(insert OID = 2800 (  ">" PGNSP PGUID b f f27  27  16 2799
 DESCR("greater than");
+#define TIDGreaterOperator 2800
 DATA(insert OID = 2801 (  "<="PGNSP PGUID b f f27  27  16 2802
 DESCR("less than or equal");
+#define TIDLessEqualOperator   2801
 DATA(insert OID = 2802 (  ">="PGNSP PGUID b f f27  27  16 2801
 DESCR("greater than or equal");
+#define TIDGreaterEqualOperator2802

> > * Join pushdown support for foreign tables
> >
> > There doesn't seem to be any current patch linked to this CF item.
> > If there is a patch to get postgres_fdw to make use of the recently
> > committed join-path support, I assume it's in need of a rebase anyway.
> 
> I think there is a rebased patch around.  I think it's just not linked
> into the CF thread.  I don't think it's committable as is.
>
I believe he is working to follow up the latest foreign/custom-join
interface on which postgres_fdw expected. One point we like to clarify
is how create_plan_recurse() shall be dealt with.
As Hanada-san posted yesterday, postgres_fdw internally uses
create_plan_recurse() to fetch SQL statement associated with inner /
outer sub-plan, so it will take additional adjustment work even
though he already begin to work.

| IMO it isn't necessary to apply the change onto
| ForeignPath/ForeignScan.  FDW would have a way to keep-and-use sub
| paths as private information.  In fact, I wrote postgres_fdw to use
| create_plan_recurse to generate SQL statements of inner/outer
| relations to be joined, but as of now SQL construction for join
| push-down is accomplished by calling private function deparseSelectSql
| recursively at the top of a join tree.

Thanks,
--
NEC Business Creation Division / PG-Strom Project
KaiGai Kohei 

-- 
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] Manipulating complex types as non-contiguous structures in-memory

[Tom Lane]

Andres Freund  writes:
> On 2015-05-10 12:09:41 -0400, Tom Lane wrote:
>> Andres Freund  writes:
>>> * The VARTAG_IS_EXPANDED(tag) trick in VARTAG_SIZE is unlikely to
>>> beneficial, before the compiler could implement the whole thing as a
>>> computed goto or lookup table, afterwards not.

>> Well, if you're worried about the speed of VARTAG_SIZE() then the right
>> thing to do would be to revert your change that made enum vartag_external
>> distinct from the size of the struct, so that we could go back to just
>> using the second byte of a varattrib_1b_e datum as its size.  As I said
>> at the time, inserting pad bytes to force each different type of toast
>> pointer to be a different size would probably be a better tradeoff than
>> what commit 3682025015 did.

> I doubt that'd be a net positive. Anyway, all I'm saying is that I can't
> see the VARTAG_IS_EXPANDED trick being beneficial in comparison to
> checking both explicit values.

I did some microbenchmarking on this, and AFAICT doing it your way makes
it slower.

I still think that going back to defining the second byte as the size
would be better.  Fortunately, since this is only a matter of in-memory
representations, we aren't committed to any particular answer.

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] Manipulating complex types as non-contiguous structures in-memory

[Andres Freund]

On 2015-05-13 20:48:51 -0400, Tom Lane wrote:
> I still think that going back to defining the second byte as the size
> would be better.  Fortunately, since this is only a matter of in-memory
> representations, we aren't committed to any particular answer.

Requiring sizes to be different still strikes me as a disaster. Or is
that not what you're proposing?


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

Re: [HACKERS] Proposal : REINDEX xxx VERBOSE

[Robert Haas]

On Wed, May 13, 2015 at 8:25 PM, Sawada Masahiko  wrote:
> The v15 patch emits a line for each table when reindexing multiple
> tables, and emits a line for each index when reindexing single table.
> But v14 patch emits a line for each index, regardless of reindex target.
> Should I change back to v14 patch?

Uh, maybe.  What made you change 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] Manipulating complex types as non-contiguous structures in-memory

[Tom Lane]

Andres Freund  writes:
> On 2015-05-13 20:48:51 -0400, Tom Lane wrote:
>> I still think that going back to defining the second byte as the size
>> would be better.  Fortunately, since this is only a matter of in-memory
>> representations, we aren't committed to any particular answer.

> Requiring sizes to be different still strikes me as a disaster. Or is
> that not what you're proposing?

It is, but why would it be a disaster?  We could add StaticAsserts
verifying that the sizes actually are different.  I doubt that the pad
space itself could amount to any issue performance-wise, since it would
only ever exist in transient in-memory tuples, and even that only seldom.

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] Manipulating complex types as non-contiguous structures in-memory

[Andres Freund]

On 2015-05-13 21:01:43 -0400, Tom Lane wrote:
> Andres Freund  writes:
> > On 2015-05-13 20:48:51 -0400, Tom Lane wrote:
> >> I still think that going back to defining the second byte as the size
> >> would be better.  Fortunately, since this is only a matter of in-memory
> >> representations, we aren't committed to any particular answer.
> 
> > Requiring sizes to be different still strikes me as a disaster. Or is
> > that not what you're proposing?
> 
> It is, but why would it be a disaster?  We could add StaticAsserts
> verifying that the sizes actually are different.  I doubt that the pad
> space itself could amount to any issue performance-wise, since it would
> only ever exist in transient in-memory tuples, and even that only seldom.

The sizes would be platform dependant. It's also just incredibly ugly to
have to add pad bytes to structures so we can disambiguate them.

Anyway, I think we can live with your & or my proposed additional branch
for now. I can't see either variant being a relevant performance
bottleneck anytime soon.


-- 
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] Manipulating complex types as non-contiguous structures in-memory

[Tom Lane]

Andres Freund  writes:
> On 2015-05-13 21:01:43 -0400, Tom Lane wrote:
>> It is, but why would it be a disaster?  We could add StaticAsserts
>> verifying that the sizes actually are different.  I doubt that the pad
>> space itself could amount to any issue performance-wise, since it would
>> only ever exist in transient in-memory tuples, and even that only seldom.

> The sizes would be platform dependant.

So what?  There are lots of platform-dependent constants in PG.

> It's also just incredibly ugly to
> have to add pad bytes to structures so we can disambiguate them.

Well, I agree it's not too pretty, but you were the one who brought up
the issue of the speed of VARTAG_SIZE().  We definitely gave up some
performance there already, and my patch will make it worse.

> Anyway, I think we can live with your & or my proposed additional branch
> for now. I can't see either variant being a relevant performance
> bottleneck anytime soon.

Actually, after having microbenchmarked the difference between those
two proposals, I'm not too sure that VARTAG_SIZE() is down in the noise.
But it doesn't matter for the moment --- any one of these alternatives
would be a very localized code change, and none of them would create
an on-disk compatibility break.  We can let it go until someone wants
to put together a more definitive benchmark for testing.

regards, tom lane


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

[HACKERS] upper planner path-ification

[Robert Haas]

Hi,

I've been pulling over Tom's occasional remarks about redoing
grouping_planner - and maybe further layers of the planner - to work
with Paths instead of Plans.  I've had difficulty locating all of the
relevant threads.  Here's everything I've found so far, which I'm
pretty sure is not everything:

http://www.postgresql.org/message-id/17400.1311716...@sss.pgh.pa.us
http://www.postgresql.org/message-id/2614.1375730...@sss.pgh.pa.us
http://www.postgresql.org/message-id/22721.1385048...@sss.pgh.pa.us
http://www.postgresql.org/message-id/banlktindjjfhnozesg2j2u4gokqlu69...@mail.gmail.com
http://www.postgresql.org/message-id/8479.1418420...@sss.pgh.pa.us

I think there are two separate problems here.  First, there's the
problem that grouping_planner() is complicated.  It's doing cost
comparisons, but all in ad-hoc fashion rather than using a consistent
mechanic the way add_path() does.  Generally, we can plan an aggregate
using either (1) a hashed aggregate, (2) a sorted aggregate, or (3)
for min or max, an index scan that just grabs the highest or lowest
value in lieu of a full table scan.  Instead of generating a plan for
each of these possibilities, we'd like to generate paths for each one,
and then pick one to turn into a plan.  AIUI, the hope is that this
would simplify the cost calculations, and also make it easier to
inject other paths, such as a path where an FDW performs the
aggregation step on the remote server.

Second, there's the problem that we might like to order aggregates
with respect to joins.  If we have something like SELECT DISTINCT ON
(foo.x) foo.x, foo.y, bar.y FROM foo, bar WHERE foo.x = bar.x, then
(a) if foo.x has many duplicates, it will be better to DISTINCT-ify
foo and then join to bar afterwards but (b) if foo.x = bar.x is highly
selective, it will be better to join to bar first and then
DISTINCT-ify the result.  Currently, aggregation is always last;
that's not great.  Hitoshi Harada's proposed strategy of essentially
figuring out where the aggregation steps can go and then re-planning
for each one is also not great, because each join problem will be a
subtree of the one we use for the aggregate-last strategy, and thus
we're wasting effort by planning the same subtrees multiple times.
Instead, we might imagine letting grouping planner fish out the best
paths for the joinrels that represent possible aggregation points,
generate aggregation paths for each of those, and then work out what
additional rels need to be joined afterwards.  That sounds hard, but
not as hard as doing something sensible with what we have today.

I'm inclined to think that it would be useful to solve the first
problem even if we didn't solve the second one right away (but that
might be wrong).  As a preparatory step, I'm thinking it would be
sensible to split grouping_planner() into an outer function that would
handle the addition of Limit and LockRows nodes and maybe planning of
set operations, and an inner function that would handle GROUP BY,
DISTINCT, and possibly window function planning.

Thoughts?

-- 
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] upper planner path-ification

[Tom Lane]

Robert Haas  writes:
> I've been pulling over Tom's occasional remarks about redoing
> grouping_planner - and maybe further layers of the planner - to work
> with Paths instead of Plans. ...
> I think there are two separate problems here.  First, there's the
> problem that grouping_planner() is complicated.
> Second, there's the problem that we might like to order aggregates
> with respect to joins.

Both of those are problems all right, but there is more context here.

* As some of the messages you cited mention, we would like to have Path
representations for things like aggregation, because that's the only
way we'll get to a sane API that lets FDWs propose remote aggregation.

* We have also had requests for the planner to be smarter about
UNION/INTERSECT/EXCEPT queries.  Again, that requires cost comparisons,
which would be better done if we had Path representations for the various
ways we'd want to consider.  Also, a big part of the issue there is
wanting to be able to consider sorted versus unsorted plans for the leaf
queries of the set-op (IOW, optionally pushing the sort requirements of
the set-op down into the leaves).  Right now, such comparisons are
impossible because prepunion.c uses subquery_planner to handle the leaf
queries, and what it gets back from that is one finished plan, not
alternative Paths.

* Likewise, subqueries-in-FROM are handled by recursing to
subquery_planner, which gives us back just one frozen Plan for the
subquery.  Among other things this seems to make it too expensive to
consider generating parameterized paths for the subquery.  I'd like
to keep subquery plans in Path form until much later as well.

So these considerations motivate wishing that the result of
subquery_planner could be a list of alternative Paths rather than a Plan,
which means that every operation it knows how to tack onto the scan/join
plan has to be representable by a Path of some sort.

I don't know how granular that needs to be, though.  For instance, one
could certainly imagine that it might be sufficient initially to have a
single "WindowPath" that represents "do all the window functions", and
then at create_plan time we'd generate multiple WindowAgg plan nodes in
the same ad-hoc way as now.  Breaking that down in the Path representation
would only become interesting if it would affect higher-level decisions,
and I'm not immediately seeing how it might do that.

> I'm inclined to think that it would be useful to solve the first
> problem even if we didn't solve the second one right away (but that
> might be wrong).  As a preparatory step, I'm thinking it would be
> sensible to split grouping_planner() into an outer function that would
> handle the addition of Limit and LockRows nodes and maybe planning of
> set operations, and an inner function that would handle GROUP BY,
> DISTINCT, and possibly window function planning.

For the reasons I mentioned, I'd like to get to a point where
subquery_planner's output is Paths not Plans as soon as possible.  But the
idea of coarse representation of steps that we aren't trying to be smart
about might be useful to save some labor in the short run.

The zero-order version of that might be a single Path node type that
represents "do whatever grouping_planner would do", which we'd start to
break down into multiple node types once we had the other APIs fixed.

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