Re: [HACKERS] Patch: fix lock contention for HASHHDR.mutex

[Amit Kapila]

On Thu, Dec 17, 2015 at 8:44 PM, Andres Freund  wrote:
>
> On 2015-12-17 09:47:57 -0500, Robert Haas wrote:
> > On Tue, Dec 15, 2015 at 7:25 AM, Andres Freund 
wrote:
> > > I'd consider using a LWLock instead of a spinlock here. I've seen this
> > > contended in a bunch of situations, and the queued behaviour, combined
> > > with directed wakeups on the OS level, ought to improve the worst case
> > > behaviour measurably.
> >
> > Amit had the idea a while back of trying to replace the HASHHDR mutex
> > with something based on atomic ops.  It seems hard to avoid the
> > attendant A-B-A problems but maybe there's a way.
>
> It'd really like to see it being replaced by a queuing lock
> (i.e. lwlock) before we go there. And then maybe partition the freelist,
> and make nentries an atomic.  Just doing those might already be good
> enough and should be a lot easier.
>

makes sense to me, but I think we should as well try the Group leader idea
used for ProcArrayLock optimisation as during those tests, I found that
it gives better results as compare to partitioning.


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

Re: [HACKERS] Patch: fix lock contention for HASHHDR.mutex

[Aleksander Alekseev]

> Oops, 3.5-4 _times_ more TPS, i.e. 2206 TPS vs 546 TPS.

In fact these numbers are for similar but a little bit different
benchmark (same schema without checks on child tables). Here are exact
numbers for a benchmark described above.



Before:

$ pgbench -j 64 -c 64 -f pgbench.sql -T 30 my_database
transaction type: Custom query
scaling factor: 1
query mode: simple
number of clients: 64
number of threads: 64
duration: 30 s
number of transactions actually processed: 20433
latency average: 93.966 ms
tps = 679.698439 (including connections establishing)
tps = 680.353897 (excluding connections establishing)

$ pgbench -j 64 -c 64 -f pgbench.sql -T 30 my_database
transaction type: Custom query
scaling factor: 1
query mode: simple
number of clients: 64
number of threads: 64
duration: 30 s
number of transactions actually processed: 19111
latency average: 100.466 ms
tps = 635.763523 (including connections establishing)
tps = 636.112682 (excluding connections establishing)

$ pgbench -j 64 -c 64 -f pgbench.sql -T 30 my_database
transaction type: Custom query
scaling factor: 1
query mode: simple
number of clients: 64
number of threads: 64
duration: 30 s
number of transactions actually processed: 19218
latency average: 99.906 ms
tps = 639.506848 (including connections establishing)
tps = 639.838757 (excluding connections establishing)


After:

$ pgbench -j 64 -c 64 -f pgbench.sql -T 30 my_database
transaction type: Custom query
scaling factor: 1
query mode: simple
number of clients: 64
number of threads: 64
duration: 30 s
number of transactions actually processed: 95900
latency average: 20.021 ms
tps = 3194.142762 (including connections establishing)
tps = 3196.091843 (excluding connections establishing)

$ pgbench -j 64 -c 64 -f pgbench.sql -T 30 my_database
transaction type: Custom query
scaling factor: 1
query mode: simple
number of clients: 64
number of threads: 64
duration: 30 s
number of transactions actually processed: 96837
latency average: 19.827 ms
tps = 3225.822355 (including connections establishing)
tps = 3227.762847 (excluding connections establishing)

$ pgbench -j 64 -c 64 -f pgbench.sql -T 30 my_database
transaction type: Custom query
scaling factor: 1
query mode: simple
number of clients: 64
number of threads: 64
duration: 30 s
number of transactions actually processed: 96143
latency average: 19.970 ms
tps = 3202.637126 (including connections establishing)
tps = 3204.070466 (excluding connections establishing)


Ratio:

$ python

min(3194.0, 3225.0, 3202.0) / max(679.0, 635.0, 639.0)
4.703976435935199


-- 
Sent 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: fix lock contention for HASHHDR.mutex

[Amit Kapila]

On Thu, Dec 17, 2015 at 9:33 PM, Aleksander Alekseev <
a.aleks...@postgrespro.ru> wrote:
>
> > It'd really like to see it being replaced by a queuing lock
> > (i.e. lwlock) before we go there. And then maybe partition the
> > freelist, and make nentries an atomic.
>
> I believe I just implemented something like this (see attachment). The
> idea is to partition PROCLOCK hash table manually into NUM_LOCK_
> PARTITIONS smaller and non-partitioned hash tables. Since these tables
> are non-partitioned spinlock is not used and there is no lock
> contention.
>

This idea can improve the situation with ProcLock hash table, but I
think IIUC what Andres is suggesting would reduce the contention
around dynahash freelist and can be helpful in many more situations
including BufMapping locks.


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

Re: [HACKERS] psql - -dry-run option

[Shulgin, Oleksandr]

On Thu, Dec 17, 2015 at 9:13 PM, Tom Lane  wrote:

>
> Whether we really need a feature like that isn't clear though; it's not
> like it's hard to test things that way now.  Stick in a BEGIN with no
> COMMIT, you're there.  The problem only comes in if you start expecting
> the behavior to be bulletproof.  Maybe I'm being too pessimistic about
> what people would believe a --dry-run switch to be good for ... but
> I doubt it.
>

I'm on the same line: BEGIN/ROLLBACK requires trivial effort and a
--dry-run option might give a false sense of security, but it cannot
possibly rollback side-effects of user functions which modify filesystem or
interact with the outside world in some other way.

--
Alex

Re: [HACKERS] parallel joins, and better parallel explain

[Dilip Kumar]

On Fri, Dec 18, 2015 at 7.59 AM Robert Haas  Wrote,

> Uh oh.  That's not supposed to happen.  A GatherPath is supposed to
> have parallel_safe = false, which should prevent the planner from
> using it to form new partial paths.  Is this with the latest version
> of the patch?  The plan output suggests that we're somehow reaching
> try_partial_hashjoin_path() with inner_path being a GatherPath, but I
> don't immediately see how that's possible, because
> create_gather_path() sets parallel_safe to false unconditionally, and
> hash_inner_and_outer() never sets cheapest_safe_inner to a path unless
> that path is parallel_safe.

Yes, you are right, that create_gather_path() sets parallel_safe to false
unconditionally but whenever we are building a non partial path, that time
we should carry forward the parallel_safe state to its parent, and it seems
like that part is missing here..


I have done quick hack in create_nestloop_path and error is gone with this
change..

create_nestloop_path
{
   pathnode->path.param_info =
get_joinrel_parampathinfo(root,
  joinrel,
  outer_path,
  inner_path,
  sjinfo,
  required_outer,
  &restrict_clauses);
pathnode->path.parallel_aware = false;
pathnode->path.parallel_safe = joinrel->consider_parallel;  //may be
joinrel is parallel safe but particular path is unsafe, so we need take
this from inner_path and outer_path

// if any of the child path is parallel unsafe the mark parent as
parallel unsafe..
*pathnode->path.parallel_safe = (inner_path->parallel_safe &
outer_path->parallel_safe); *

}

New plan is attached in the mail.

with this change now we can see execution time is also become half (may be
because warning messages are gone)


> Do you have a self-contained test case that reproduces this, or any
> insight as to how it's happening here?

This is TPC-H benchmark case:
we can setup like this..
1. git clone https://tkej...@bitbucket.org/tkejser/tpch-dbgen.git
2. complie using make
3. ./dbgen –v –s 5
4. ./qgen


Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

On Fri, Dec 18, 2015 at 7:59 AM, Robert Haas  wrote:

> On Thu, Dec 17, 2015 at 12:33 AM, Amit Kapila 
> wrote:
> > While looking at plans of Q5 and Q7, I have observed that Gather is
> > pushed below another Gather node for which we don't have appropriate
> > way of dealing.  I think that could be the reason why you are seeing
> > the errors.
>
> Uh oh.  That's not supposed to happen.  A GatherPath is supposed to
> have parallel_safe = false, which should prevent the planner from
> using it to form new partial paths.  Is this with the latest version
> of the patch?  The plan output suggests that we're somehow reaching
> try_partial_hashjoin_path() with inner_path being a GatherPath, but I
> don't immediately see how that's possible, because
> create_gather_path() sets parallel_safe to false unconditionally, and
> hash_inner_and_outer() never sets cheapest_safe_inner to a path unless
> that path is parallel_safe.
>
> Do you have a self-contained test case that reproduces this, or any
> insight as to how it's happening here?
>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>


q7_parallel_new.out
Description: Binary data

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

Re: [HACKERS] Patch: fix lock contention for HASHHDR.mutex

[Aleksander Alekseev]

> This idea can improve the situation with ProcLock hash table, but I
> think IIUC what Andres is suggesting would reduce the contention
> around dynahash freelist and can be helpful in many more situations
> including BufMapping locks.

I agree. But as I understand PostgreSQL community doesn't generally
аpprоvе big changes that affects whole system. Especially if original
problem was only in one particular place. Therefore for now I suggest
only a small change. Naturally if it will be accepted there is no
reason not to apply same changes for BufMapping or even dynahash itself
with corresponding PROCLOCK hash refactoring.

BTW could you (or anyone?) please help me find this thread regarding
BufMapping or perhaps provide a benchmark? I would like to reproduce
this issue but I can't find anything relevant in a mailing list. Also
it seems to be a good idea to compare alternative approaches that were
mentioned (atomics ops, group leader). Are there any discussions,
benchmarks or patches regarding this topic?

Frankly I have serious doubts regarding atomics ops since they will more
likely create the same contention that a spinlock does. But perhaps
there is a patch that works not the way I think it could work.


-- 
Sent 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: fix lock contention for HASHHDR.mutex

[Andres Freund]

On 2015-12-18 11:40:58 +0300, Aleksander Alekseev wrote:
> $ pgbench -j 64 -c 64 -f pgbench.sql -T 30 my_database

What's in pgbench.sql?

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: fix lock contention for HASHHDR.mutex

[Aleksander Alekseev]

> What's in pgbench.sql?

It's from first message of this thread:

http://www.postgresql.org/message-id/20151211170001.78ded9d7@fujitsu


-- 
Sent 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] Proposal for \rotate in psql

[Pavel Stehule]

2015-12-17 21:33 GMT+01:00 Pavel Stehule :

>
>
> 2015-12-14 23:09 GMT+01:00 Daniel Verite :
>
>> Pavel Stehule wrote:
>>
>> > postgres=# \crosstabview 4 +month label
>> >
>> > Maybe using optional int order column instead label is better - then
>> you can
>> > do sort on client side
>> >
>> > so the syntax can be "\crosstabview VCol [+/-]HCol [[+-]HOrderCol]
>>
>> In the meantime I've followed a different idea: allowing the
>> vertical header to be sorted too, still server-side.
>>
>> That's because to me, the first impulse for a user noticing that
>> it's not sorted vertically would be to write
>>  \crosstabview +customer month
>> rather than figure out the
>>  \crosstabview customer +month_number month_name
>> invocation.
>> But both ways aren't even mutually exclusive. We could support
>>  \crosstabview [+|-]colV[:labelV] [+|-]colH[:labelH]
>> it's more complicated to understand, but not  harder to implement.
>>
>> Also, a non-zero FETCH_COUNT is supported by this version of the patch,
>> if the first internal FETCH retrieves less than FETCH_COUNT rows.
>> Otherwise a specific error is emitted.
>>
>> Also there are minor changes in arguments and callers following
>> recent code changes for \o
>>
>> Trying to crosstab with 10k+ distinct values vertically, I've noticed
>> that the current code is too slow, spending too much time
>> sorting.  I'm currently replacing its simple arrays of distinct values
>> with AVL binary trees, which I expect to be much more efficient for
>> this.
>>
>
> I played with last version and it is looking well. I have only one notice,
> but it is subjective - so can be ignored if you don't like it.
>
> The symbol 'X' in two column mode should be centred - now it is aligned to
> left, what is not nice. For unicode line style I prefer some unicode symbol
> - your chr(10003) is nice.
>
>
I checked code and I have only one note. The name "sortColumns" is not
valid now, and it isn't well - maybe ServerSideSort or some similar can be
better. The error message "Unexpected value when sorting horizontal
headers" is obsolete too.

Regards

Pavel




> Regards
>
> Pavel
>
>
>
>>
>> Best regards,
>> --
>> Daniel Vérité
>> PostgreSQL-powered mailer: http://www.manitou-mail.org
>> Twitter: @DanielVerite
>>
>
>

Re: [HACKERS] Patch: fix lock contention for HASHHDR.mutex

[Amit Kapila]

On Fri, Dec 18, 2015 at 2:50 PM, Aleksander Alekseev <
a.aleks...@postgrespro.ru> wrote:
>
> > This idea can improve the situation with ProcLock hash table, but I
> > think IIUC what Andres is suggesting would reduce the contention
> > around dynahash freelist and can be helpful in many more situations
> > including BufMapping locks.
>
> I agree. But as I understand PostgreSQL community doesn't generally
> approve big changes that affects whole system. Especially if original
> problem was only in one particular place. Therefore for now I suggest
> only a small change. Naturally if it will be accepted there is no
> reason not to apply same changes for BufMapping or even dynahash itself
> with corresponding PROCLOCK hash refactoring.
>
> BTW could you (or anyone?) please help me find this thread regarding
> BufMapping or perhaps provide a benchmark?
>

You can find that in below thread:
http://www.postgresql.org/message-id/CAA4eK1+U+GQDc2sio4adRk+ux6obFYRPxkY=ch5bknabtoo...@mail.gmail.com

This even contains the original idea and initial patch for replacing
spinlocks with atomic ops.  I have mentioned about the A-B-A problem
few mails down in that thread and given a link to paper suggesting how
that can be solved.  It needs more work, but doable.

> I would like to reproduce
> this issue but I can't find anything relevant in a mailing list. Also
> it seems to be a good idea to compare alternative approaches that were
> mentioned (atomics ops, group leader). Are there any discussions,
> benchmarks or patches regarding this topic?
>

You can find the discussion and patch related to Group leader approach
in the thread:
http://www.postgresql.org/message-id/caa4ek1jbx4fzphignt0jsaz30a85bpjv+ewhk+wg_o-t6xu...@mail.gmail.com
This patch is already committed.

> Frankly I have serious doubts regarding atomics ops since they will more
> likely create the same contention that a spinlock does. But perhaps
> there is a patch that works not the way I think it could work.
>

I think it is difficult to say without implementing it.  If we want
to evaluate
multiple approaches then what we can do here is I can help with writing a
patch using LWLocks and you can once evaluate the atomic ops approach
and we already have your current patch, then we can see what works out
best.


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

[HACKERS] Costing foreign joins in postgres_fdw

[Ashutosh Bapat]

Hi All,
Costs for foreign queries are either obtained from the foreign server using
EXPLAIN (if use_remote_estimate is ON) otherwise they are cooked up locally
based on the statistics available. For joins as well, we have to do the
same. If use_remote_estimates [1] is ON, we can get the costs from the
foreign server. Rest of the mail discusses approaches for estimating the
costs when use_remote_estimates is OFF.

1. Unlike base relations where the table data "has to be" fetched from the
foreign server, a join doesn't "have to be" fetched from the foreign
server. So, even if use_remote_estimate is OFF for a base relation, we do
try to estimate something locally. But for a join that's not compulsory, so
we can choose not to estimate anything and not push down the join if
use_remote_estimate is OFF. Whether we do that depends upon how well we can
estimate the join cost when use_remote_estimate is OFF.

2. Locally estimating the cost of join that will be performed on the
foreign server is difficult because we do not know which join strategy the
foreign server is going to use, which in turn depends upon the availability
of indexes, work memory, statistics about joining expressions etc. One way
to do this is to use the cost of cheapest local join path built upon
foreign outer and inner paths, to estimate the cost of executing the join
at the foreign server The startup and run time costs for sending, parsing
and planning query at the foreign server as well as the cost to fetch the
tuples need to be adjusted, so that it doesn't get counted twice. We may
assume that the cost for the foreign join will be some factor of the
adjusted cost, like we have done for estimating cost of sort pushdown. The
reason we choose cheapest path with foreign inner and outer paths is
because that's likely to be a closer to the real estimate than the path
which does not have foreign inner and outer paths. In the absence of such
path, we should probably not push the join down since no local path has
found pushing inner and outer to be cheaper and it's likely (certainly not
a rule) that pushing the join in question down is not going to be cheaper
than the local paths.

1st option is easy but it sounds too restrictive. 2nd option liberal but is
complex.

Any other ideas as to how we can estimate cost of foreign join when
use_remote_estimate is OFF?

[1]
http://www.postgresql.org/message-id/CAFjFpRepSC2e3mZ1uYSopJD6R19fOZ0dNNf9Z=gnyksb6wg...@mail.gmail.com
-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

Re: [HACKERS] Costing foreign joins in postgres_fdw

[Albe Laurenz]

Ashutosh Bapat wrote:
> Costs for foreign queries are either obtained from the foreign server using 
> EXPLAIN (if
> use_remote_estimate is ON) otherwise they are cooked up locally based on the 
> statistics available. For
> joins as well, we have to do the same. If use_remote_estimates [1] is ON, we 
> can get the costs from
> the foreign server. Rest of the mail discusses approaches for estimating the 
> costs when
> use_remote_estimates is OFF.
> 
> 
> 1. Unlike base relations where the table data "has to be" fetched from the 
> foreign server, a join
> doesn't "have to be" fetched from the foreign server. So, even if 
> use_remote_estimate is OFF for a
> base relation, we do try to estimate something locally. But for a join that's 
> not compulsory, so we
> can choose not to estimate anything and not push down the join if 
> use_remote_estimate is OFF. Whether
> we do that depends upon how well we can estimate the join cost when 
> use_remote_estimate is OFF.
> 
> 2. Locally estimating the cost of join that will be performed on the foreign 
> server is difficult
> because we do not know which join strategy the foreign server is going to 
> use, which in turn depends
> upon the availability of indexes, work memory, statistics about joining 
> expressions etc. One way to do
> this is to use the cost of cheapest local join path built upon foreign outer 
> and inner paths, to
> estimate the cost of executing the join at the foreign server The startup and 
> run time costs for
> sending, parsing and planning query at the foreign server as well as the cost 
> to fetch the tuples need
> to be adjusted, so that it doesn't get counted twice. We may assume that the 
> cost for the foreign join
> will be some factor of the adjusted cost, like we have done for estimating 
> cost of sort pushdown. The
> reason we choose cheapest path with foreign inner and outer paths is because 
> that's likely to be a
> closer to the real estimate than the path which does not have foreign inner 
> and outer paths. In the
> absence of such path, we should probably not push the join down since no 
> local path has found pushing
> inner and outer to be cheaper and it's likely (certainly not a rule) that 
> pushing the join in question
> down is not going to be cheaper than the local paths.
> 
> 
> 1st option is easy but it sounds too restrictive. 2nd option liberal but is 
> complex.

My gut feeling is that for a join where all join predicates can be pushed down, 
it
will usually be a win to push the join to the foreign server.

So in your first scenario, I'd opt for always pushing down the join
if possible if use_remote_estimate is OFF.

Your second scenario is essentially to estimate that a pushed down join will
always be executed as a nested loop join, which will in most cases produce
an unfairly negative estimate.

What about using local statistics to come up with an estimated row count for
the join and use that as the basis for an estimate?  My idea here is that it
is always be a win to push down a join unless the result set is so large that
transferring it becomes the bottleneck.
Maybe, to come up with something remotely realistic, a formula like

sum of locally estimated costs of sequential scan for the base table
plus count of estimated result rows (times a factor)

Yours,
Laurenz Albe



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

Re: [HACKERS] statistics for array types

[Alexander Korotkov]

On Wed, Sep 16, 2015 at 8:01 PM, Alexander Korotkov <
a.korot...@postgrespro.ru> wrote:

> On Mon, Aug 24, 2015 at 8:26 PM, Jeff Janes  wrote:
>
>> On Thu, Aug 20, 2015 at 6:00 PM, Tomas Vondra <
>> tomas.von...@2ndquadrant.com> wrote:
>>
>>> Hi,
>>>
>>> On 08/11/2015 04:38 PM, Jeff Janes wrote:
>>>
 When reviewing some recent patches, I decided the statistics gathered
  for arrays had some pre-existing shortcomings.

 The main one is that when the arrays contain rare elements there is
 no histogram to fall back upon when the MCE array is empty, the way
 there is for scalar stats.  So it has to punt completely and resort
 to saying that it is 0.5% selectivity without recourse to any data at
 all.

 The rationale for applying the threshold before things are eligible
 for inclusion in the MCE array seems to be that this puts some
 theoretical bound on the amount of error we are likely to have in
 that element.  But I think it is better to exceed that theoretical
 bound than it is to have no data at all.

 The attached patch forces there to be at least one element in MCE,
 keeping the one element with the highest predicted frequency if the
 MCE would otherwise be empty.  Then any other element queried for is
 assumed to be no more common than this most common element.

>>>
>>> We only really need the frequency, right? So do we really need to keep
>>> the actual MCV element? I.e. most_common_elem_freqs does not have the
>>> same number of values as most_common_elems anyway:
>>>
>>>   A list of the frequencies of the most common element values, i.e., the
>>>   fraction of rows containing at least one instance of the given value.
>>>   Two or three additional values follow the per-element frequencies;
>>>   these are the minimum and maximum of the preceding per-element
>>>   frequencies, and optionally the frequency of null elements.
>>>   (Null when most_common_elems is.)
>>>
>>> So we might modify it so that it's always defined - either it tracks the
>>> same values as today (when most_common_elems is defined), or the
>>> frequency of the most common element (when most_common_elems is NULL).
>>>
>>
>> I had also considered that.  It requires more changes to make it happen,
>> and it seems to create a more complex contract on what those columns mean,
>> but without giving a corresponding benefit.
>>
>>
>>>
>>> This way we can keep the current theoretical error-bound on the MCE
>>> frequencies, and if that's not possible we can have at least the new
>>> value without confusing existing code.
>>
>>
>> But if the frequency of the most common element was grossly wrongly, then
>> whatever value we stick in there is still going to be grossly wrong.
>> Removing the value associated with it isn't going to stop it from being
>> wrong.  When we do query with the (incorrectly thought) first most common
>> element, either it will find and use the wrong value from slot 1, or it
>> will find nothing and fall back on the same wrong value from slot 3.
>>
>
> Hmm, I think we should store cutoff_freq / nonnull_cnt as minfreq when we
> collect no MCEs. Moreover, I think we should store it even when num_mcelem
> >= track_len and we haven't cut MCEs we find. In this case we can get more
> precise estimation for rare element using the knowledge that all MCEs which
> exceeds the threshold are present (assuming their frequecies could be much
> higher than threshold).
>
> When there are no MCEs then we should use assumption that there are no
> elements more frequent than cutoff_freq / nonnull_cnt. Using lower values
> wouldn't be statistically correct.
>

​The patch implementing my idea above​ is attached. In your example it
gives following result.

# explain (analyze) select * from foobar where foo @> '{567}';
  QUERY PLAN
---
 Seq Scan on foobar  (cost=0.00..2387.00 rows=30 width=61) (actual
time=28.691..28.691 rows=0 loops=1)
   Filter: (foo @> '{567}'::integer[])
   Rows Removed by Filter: 10
 Planning time: 0.044 ms
 Execution time: 28.707 ms
(5 rows)

In this particular example it gives less accurate estimate. However, I
believe it would be more safe estimate in general.

I've faced difficulties with storing empty mcelements
array. update_attstats turns empty array into null, and get_attstatsslot
throws error when trying to read null array. I've changes get_attstatsslot
so that it returns empty array when it meets null. I'm not sure in this
solution.

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


array_typanalyze_0_mce.patch
Description: Binary data

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

Re: [HACKERS] Patch: fix lock contention for HASHHDR.mutex

[Teodor Sigaev]

Oh, that's an interesting idea.  I guess the problem is that if the
freelist is unshared, then users might get an error that the lock
table is full when some other partition still has elements remaining.


Could we split one freelist in hash to NUM_LOCK_PARTITIONS freelists?
Each partition will have its own freelist and if freelist is empty then 
partition should search an entry in freelists of other partitions. To prevent 
concurrent access it's needed to add one LWLock to hash, each partition should 
lock LWlock in share mode to work with its own freelist and exclusive to work 
with other freelists.


Actually, I'd like to improve all partitioned hashes instead of improve only one 
case.


--
Teodor Sigaev   E-mail: teo...@sigaev.ru
   WWW: http://www.sigaev.ru/


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

Re: [HACKERS] Function and view to retrieve WAL receiver status

[Michael Paquier]

On Fri, Dec 18, 2015 at 8:39 AM, Robert Haas  wrote:
> On Mon, Dec 14, 2015 at 7:23 PM, Michael Paquier
>  wrote:
>> On Tue, Dec 15, 2015 at 5:27 AM, Gurjeet Singh wrote:
>>> The function, maybe. But emitting an all-nulls row from a view seems
>>> counter-intuitive, at least when looking at it in context of relational
>>> database.
>>
>> OK, noted. Any other opinions?
>
> I wouldn't bother with the view.  If we're going to do it, I'd say
> just provide the function and let people SELECT * from it if they want
> to.

OK, I took some time to write a patch for that as attached, added in
the next CF here:
https://commitfest.postgresql.org/8/447/
I am fine switching to an SRF depending on other opinions of people
here, it just seems like an overkill knowing the uniqueness of the WAL
sender in a server.

I have finished with a function and a system view, this came up more
in line with the existing things like pg_stat_archiver, and this makes
as well the documentation clearer, at least that was my feeling when
hacking that.
Regards,
-- 
Michael


0001-Add-system-view-and-function-to-report-WAL-receiver-.patch
Description: binary/octet-stream

-- 
Sent 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: fix lock contention for HASHHDR.mutex

[Aleksander Alekseev]

> Could we split one freelist in hash to NUM_LOCK_PARTITIONS freelists?
> Each partition will have its own freelist and if freelist is empty
> then partition should search an entry in freelists of other
> partitions. To prevent concurrent access it's needed to add one
> LWLock to hash, each partition should lock LWlock in share mode to
> work with its own freelist and exclusive to work with other freelists.
> 
> Actually, I'd like to improve all partitioned hashes instead of
> improve only one case.

It seems to be a most promising direction of research for now. I will
send a patch and benchmark results 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] A question regarding LWLock in ProcSleep

[Amit Kapila]

On Thu, Dec 17, 2015 at 1:51 PM, Kenan Yao  wrote:

> Hi there,
>
> In function ProcSleep, after the process has been waken up, either with
> lock granted or deadlock detected, it would re-acquire the lock table's
> partition LWLock.
>
> The code episode is here:
>
> /*
>  * Re-acquire the lock table's partition lock.  We have to do this to hold
>  * off cancel/die interrupts before we can mess with lockAwaited (else we
>  * might have a missed or duplicated locallock update).
>  */
> LWLockAcquire(partitionLock, LW_EXCLUSIVE);
>
> /*
>  * We no longer want LockErrorCleanup to do anything.
>  */
> lockAwaited = NULL;
>
> /*
>  * If we got the lock, be sure to remember it in the locallock table.
>  */
> if (MyProc->waitStatus == STATUS_OK)
> GrantAwaitedLock();
>
> /*
>  * We don't have to do anything else, because the awaker did all the
>  * necessary update of the lock table and MyProc.
>  */
> return MyProc->waitStatus;
>
> ​
> Questions are:
>
> (1) The comment says that "we might have a missed or duplicated locallock
> update", in what cases would we hit this if without holding the LWLock?
>
> (2) The comment says "we have to do this to hold off cancel/die
> interrupts", then:
>
>- why using LWLockAcquire instead of HOLD_INTERRUPTS directly?
>- From the handler of SIGINT and SIGTERM, seems nothing serious would
>be processed here, since no CHECK_FOR_INTERRUPS is called before releasing
>this LWLock. Why we should hold off cancel/die interrupts here?
>
> (3) Before releasing this LWLock, the only share memory access is
> MyProc->waitStatus; since the process has been granted the lock or removed
> from the lock's waiting list because of deadlock, is it possible some other
> processes would access this field? if not, then why we need LWLock here?
> what does this lock protect?
>
>
I think the other thing which needs protection of LWLock is
access to proclock which is done in the caller
(LockAcquireExtended).




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

Re: [HACKERS] Remove array_nulls?

[Robert Treat]

On Thu, Dec 17, 2015 at 4:31 PM, Robert Haas  wrote:
> On Wed, Dec 16, 2015 at 10:48 PM, Jim Nasby  wrote:
>> IIUC, that means supporting backwards compat. GUCs for 10 years, which seems
>> a bit excessive. Granted, that's about the worse-case scenario for what I
>> proposed (ie, we'd still be supporting 8.0 stuff right now).
>
> Not to me.  GUCs like array_nulls don't really cost much - there is no
> reason to be in a hurry about removing them that I can see.
>

Perhaps not with rock solid consistency, but we've certainly used the
argument of the "not a major major version release" to shoot down
introducing incompatible features / improvements (protocol changes
come to mind), which further lends credence to Jim's point about
people expecting backwards incompatible breakage to be in a major
major version changes.

Given the overhead from a development standpoint is low, whats the
better user experience: delay removal for as long as possible (~10
years) to narrow the likely of people being affected, or make such
changes as visible as possible (~6+ years) so that people have clear
expectations / lines of demarcation?

Robert Treat
play: xzilla.net
work: omniti.com


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

Re: [HACKERS] Patch: fix lock contention for HASHHDR.mutex

[Aleksander Alekseev]

> This idea can improve the situation with ProcLock hash table, but I
> think IIUC what Andres is suggesting would reduce the contention
> around dynahash freelist and can be helpful in many more situations
> including BufMapping locks.

I agree. But as I understand PostgreSQL community doesn't generally
approve big changes that affects whole system. Especially if original
problem was only in one particular place. Therefore for now I suggest
only a small change. Naturally if it will be accepted there is no
reason not to apply same changes for BufMapping or even dynahash itself
with corresponding PROCLOCK hash refactoring.

BTW could you (or anyone?) please help me find this thread regarding
BufMapping or perhaps provide a benchmark? I would like to reproduce
this issue but I can't find anything relevant in a mailing list. Also
it seems to be a good idea to compare alternative approaches that were
mentioned (atomics ops, group leader). Are there any discussions,
benchmarks or patches regarding this topic?

Frankly I have serious doubts regarding atomics ops since they will more
likely create the same contention that a spinlock does. But perhaps
there is a patch that works not the way I think it could work.


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

Re: [HACKERS] parallel joins, and better parallel explain

[Robert Haas]

On Fri, Dec 18, 2015 at 3:54 AM, Dilip Kumar  wrote:
> On Fri, Dec 18, 2015 at 7.59 AM Robert Haas  Wrote,
>> Uh oh.  That's not supposed to happen.  A GatherPath is supposed to
>> have parallel_safe = false, which should prevent the planner from
>> using it to form new partial paths.  Is this with the latest version
>> of the patch?  The plan output suggests that we're somehow reaching
>> try_partial_hashjoin_path() with inner_path being a GatherPath, but I
>> don't immediately see how that's possible, because
>> create_gather_path() sets parallel_safe to false unconditionally, and
>> hash_inner_and_outer() never sets cheapest_safe_inner to a path unless
>> that path is parallel_safe.
>
> Yes, you are right, that create_gather_path() sets parallel_safe to false
> unconditionally but whenever we are building a non partial path, that time
> we should carry forward the parallel_safe state to its parent, and it seems
> like that part is missing here..

Ah, right.  Woops.  I can't exactly replicate your results, but I've
attempted to fix this in a systematic way in the new version attached
here (parallel-join-v3.patch).

>> Do you have a self-contained test case that reproduces this, or any
>> insight as to how it's happening here?
>
> This is TPC-H benchmark case:
> we can setup like this..
> 1. git clone https://tkej...@bitbucket.org/tkejser/tpch-dbgen.git
> 2. complie using make
> 3. ./dbgen –v –s 5
> 4. ./qgen

Thanks.  After a bit of fiddling I was able to get this to work.  I'm
attaching two other patches that seem to help this case quite
considerably.  The first (parallel-reader-order-v1) cause Gather to
read from the same worker repeatedly until it can't get another tuple
from that worker without blocking, and only then move on to the next
worker.  With 4 workers, this seems to be drastically more efficient
than what's currently in master - I saw the time for Q5 drop from over
17 seconds to about 6 (this was an assert-enabled build running with
EXPLAIN ANALYZE, though, so take those numbers with a grain of salt).
The second (gather-disuse-physical-tlist.patch) causes Gather to force
underlying scan nodes to project, which is a good idea here for
reasons very similar to why it's a good idea for the existing node
types that use disuse_physical_tlist: forcing extra data through the
Gather node is bad.  That shaved another half second off this query.

The exact query I was using for testing was:

explain (analyze, verbose) select n_name, sum(l_extendedprice * (1 -
l_discount)) as revenue from customer, orders, lineitem, supplier,
nation, region where c_custkey = o_custkey and l_orderkey = o_orderkey
and l_suppkey = s_suppkey and c_nationkey = s_nationkey and
s_nationkey = n_nationkey and n_regionkey = r_regionkey and r_name =
'EUROPE' and o_orderdate >= date '1995-01-01' and o_orderdate < date
'1995-01-01' + interval '1' year group by n_name order by revenue
desc;

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
From 088a96363231b441fe6aab744b04a522d01fbc17 Mon Sep 17 00:00:00 2001
From: Robert Haas 
Date: Thu, 19 Nov 2015 20:28:34 -0500
Subject: [PATCH 1/3] Gather pushdown for child tables, hash joins, nested
 loops.

Cost model fix for parallel seq scan.

Fixed this so it propagates the parallel_safe flag up the plan tree.
---
 src/backend/executor/execParallel.c |  66 +++---
 src/backend/nodes/outfuncs.c|   4 +-
 src/backend/optimizer/README|  55 -
 src/backend/optimizer/path/allpaths.c   | 164 +++
 src/backend/optimizer/path/costsize.c   |  32 +--
 src/backend/optimizer/path/joinpath.c   | 253 +-
 src/backend/optimizer/path/joinrels.c   |   3 +-
 src/backend/optimizer/plan/createplan.c |   2 +-
 src/backend/optimizer/plan/planmain.c   |   3 +-
 src/backend/optimizer/util/pathnode.c   | 361 +---
 src/backend/optimizer/util/relnode.c|   2 +
 src/include/nodes/relation.h|   4 +-
 src/include/optimizer/cost.h|   2 +-
 src/include/optimizer/pathnode.h|  12 +-
 src/include/optimizer/paths.h   |   2 +
 15 files changed, 845 insertions(+), 120 deletions(-)

diff --git a/src/backend/executor/execParallel.c b/src/backend/executor/execParallel.c
index 30e6b3d..5bc8eef 100644
--- a/src/backend/executor/execParallel.c
+++ b/src/backend/executor/execParallel.c
@@ -167,25 +167,25 @@ ExecParallelEstimate(PlanState *planstate, ExecParallelEstimateContext *e)
 	e->nnodes++;
 
 	/* Call estimators for parallel-aware nodes. */
-	switch (nodeTag(planstate))
+	if (planstate->plan->parallel_aware)
 	{
-		case T_SeqScanState:
-			ExecSeqScanEstimate((SeqScanState *) planstate,
-e->pcxt);
-			break;
-		default:
-			break;
+		switch (nodeTag(planstate))
+		{
+			case T_SeqScanState:
+ExecSeqScanEstimate((SeqScanState *) planstate,
+	e->pcxt);
+break;
+			default:
+break;
+		}
 	}
 
 	return planstate_tree_walker(planstate, 

Re: [HACKERS] Remove array_nulls?

[Robert Haas]

On Fri, Dec 18, 2015 at 9:52 AM, Robert Treat  wrote:
> On Thu, Dec 17, 2015 at 4:31 PM, Robert Haas  wrote:
>> On Wed, Dec 16, 2015 at 10:48 PM, Jim Nasby  wrote:
>>> IIUC, that means supporting backwards compat. GUCs for 10 years, which seems
>>> a bit excessive. Granted, that's about the worse-case scenario for what I
>>> proposed (ie, we'd still be supporting 8.0 stuff right now).
>>
>> Not to me.  GUCs like array_nulls don't really cost much - there is no
>> reason to be in a hurry about removing them that I can see.
>>
>
> Perhaps not with rock solid consistency, but we've certainly used the
> argument of the "not a major major version release" to shoot down
> introducing incompatible features / improvements (protocol changes
> come to mind), which further lends credence to Jim's point about
> people expecting backwards incompatible breakage to be in a major
> major version changes.

My memory is that Tom usually argues pretty vigorously against the
idea that there's anything special about a first-digit bump in terms
of incompatibilities.  But your memory may have a longer reach than
mine.

> Given the overhead from a development standpoint is low, whats the
> better user experience: delay removal for as long as possible (~10
> years) to narrow the likely of people being affected, or make such
> changes as visible as possible (~6+ years) so that people have clear
> expectations / lines of demarcation?

IMHO, it's almost hopeless to expect users to prepare for incompatible
changes we want to make.  When we try to force it, as we did with
standard_conforming_strings or the 8.3-vintage casting changes, we
cause a lot of user pain and that's about it.  People don't say "ah,
these changes are coming, I need to adjust my app to be safe in this
new world"; instead, they say "crap, I can't upgrade, PostgreSQL
hackers suck".  We spend our days and nights worrying about this
stuff, but real users don't.  They just got knocked over when the
change hits.  Or the people who understand that the problem is coming
are in a different group than the people who have to fix it, so it
doesn't get fixed.  Or whatever.

My experience is that it is very common for users to upgrade across a
whole series of releases at the same time.  People don't upgrade from
8.3 to 8.4 and then to 9.0, or even from 8.3 to 9.0 to 9.2.  I mean,
some do.  But people doing things like 8.2 -> 9.3 is not that
uncommon, at least in my experience with EnterpriseDB customers.
That's why we support releases for five full years, right?  So that
people don't necessarily have to upgrade more than about that often.
Ideally they'd upgrade about every 4 years, so that they get off the
oldest supported release before it actually drops out of support, but
for various reasons it often takes longer than that, and their current
version is out of support before they get around to upgrading.  We can
wag our tongues and cluck at those people, but when we're quick to
pull the trigger on removing backward compatibility hacks, we actually
make the problem worse, not better.  Now people stay on the old
versions even longer, because they can't upgrade until they fix their
app.

-- 
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: fix lock contention for HASHHDR.mutex

[Robert Haas]

On Fri, Dec 18, 2015 at 8:46 AM, Teodor Sigaev  wrote:
>> Oh, that's an interesting idea.  I guess the problem is that if the
>> freelist is unshared, then users might get an error that the lock
>> table is full when some other partition still has elements remaining.
>
> Could we split one freelist in hash to NUM_LOCK_PARTITIONS freelists?
> Each partition will have its own freelist and if freelist is empty then
> partition should search an entry in freelists of other partitions. To
> prevent concurrent access it's needed to add one LWLock to hash, each
> partition should lock LWlock in share mode to work with its own freelist and
> exclusive to work with other freelists.
>
> Actually, I'd like to improve all partitioned hashes instead of improve only
> one case.

Yeah.  I'm not sure that should be an LWLock rather than a spinlock,
but we can benchmark it both ways.

-- 
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] Remove array_nulls?

[Alvaro Herrera]

Robert Haas wrote:
> On Fri, Dec 18, 2015 at 9:52 AM, Robert Treat  wrote:

> > Perhaps not with rock solid consistency, but we've certainly used the
> > argument of the "not a major major version release" to shoot down
> > introducing incompatible features / improvements (protocol changes
> > come to mind), which further lends credence to Jim's point about
> > people expecting backwards incompatible breakage to be in a major
> > major version changes.
> 
> My memory is that Tom usually argues pretty vigorously against the
> idea that there's anything special about a first-digit bump in terms
> of incompatibilities.  But your memory may have a longer reach than
> mine.

We haven't made a lot of first-digit changes, and if I recall correctly
it has been mostly a matter of PR, based on sets of particular features,
rather than objective technical criteria (such as changes in backwards
compatibility or such).  For instance we changed from 7 to 8 mostly
because of adding the Windows port and PITR, and from 8 to 9 because of
replication -- you could think of those as major steering changes, with
large influence in what came afterwards.

I don't know what would be a good reason to change from 9 to 10, but
certainly we shouldn't do it just to remove a couple of GUCs -- much
less do it for no reason at all (which would be what "but 9.6 is too
close to 9.10 already" would boil down to.)  I sure hope we're gonna
find some good reason to do it before 9.10 actually comes around.

> > Given the overhead from a development standpoint is low, whats the
> > better user experience: delay removal for as long as possible (~10
> > years) to narrow the likely of people being affected, or make such
> > changes as visible as possible (~6+ years) so that people have clear
> > expectations / lines of demarcation?
> 
> IMHO, it's almost hopeless to expect users to prepare for incompatible
> changes we want to make.  When we try to force it, as we did with
> standard_conforming_strings or the 8.3-vintage casting changes, we
> cause a lot of user pain and that's about it.  People don't say "ah,
> these changes are coming, I need to adjust my app to be safe in this
> new world"; instead, they say "crap, I can't upgrade, PostgreSQL
> hackers suck".  We spend our days and nights worrying about this
> stuff, but real users don't.  They just got knocked over when the
> change hits.

Agreed on this.  (As anecdote, I remember people relying on
add_missing_from, and never fixing the apps, until they absolutely had
no choice because the GUC was removed, even though I warned years in
advance.)

On the other hand, while I agree with you that we should strive to
maintain backwards compatible options for a long time, and that in this
particular case I see no reason not to wait a few more releases since it
doesn't hurt anything, I don't think we should make this an iron-clad
rule: I imagine there might be cases where there will be good reasons to
break it sooner than those 10 years if maintenance becomes a serious
problem otherwise.  We will need to discuss each case individually as it
comes up.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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

Re: [HACKERS] Remove array_nulls?

[Tom Lane]

Robert Haas  writes:
> My experience is that it is very common for users to upgrade across a
> whole series of releases at the same time.  People don't upgrade from
> 8.3 to 8.4 and then to 9.0, or even from 8.3 to 9.0 to 9.2.  I mean,
> some do.  But people doing things like 8.2 -> 9.3 is not that
> uncommon, at least in my experience with EnterpriseDB customers.
> That's why we support releases for five full years, right?  So that
> people don't necessarily have to upgrade more than about that often.

Yeah.  For a recent example, see yesterday's thread with someone inquiring
about known bugs in 8.1 ... and they did not sound like they had any
intention of getting off that soon.  But when they do, they're going to
have eight or ten years' worth of incompatibilities to deal with.

I did a quick troll through the commit log looking for previous cases
where we have removed backwards-compatibility GUCs.  I could find only
two:


commit ab61df9e527dcedbd3bbefbcb8b634b0b72f2ad5
Author: Tom Lane 
Date:   Wed Oct 21 20:38:58 2009 +

Remove regex_flavor GUC, so that regular expressions are always "advanced"
style by default.  Per discussion, there seems to be hardly anything that
really relies on being able to change the regex flavor, so the ability to
select it via embedded options ought to be enough for any stragglers.
Also, if we didn't remove the GUC, we'd really be morally obligated to
mark the regex functions non-immutable, which'd possibly create performance
issues.

commit 289e2905c82fc37f8b82b088bb823742aad4bb68
Author: Tom Lane 
Date:   Wed Oct 21 20:22:38 2009 +

Remove add_missing_from GUC and associated parser support for "implicit 
RTEs".
Per recent discussion, add_missing_from has been deprecated for long enough 
to
consider removing, and it's getting in the way of planned parser 
refactoring.
The system now always behaves as though add_missing_from were OFF.


So in both those cases, there was an active reason to remove the GUC,
not just "it seems like this is too old for anyone to want it anymore".

Also worthy of remark is that add_missing_from and regex_flavor were both
added in 2003, so their useful lifespan was a lot shorter than what's
being suggested in this thread.

Not entirely sure what to make of this.  It occurs to me that the "it
breaks immutability" argument might apply to array_nulls, though I've
not done any legwork to confirm or disprove that.  If it doesn't apply,
though, I'm leaning to the position that there's no reason to remove
array_nulls.

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] Remove array_nulls?

[Robert Haas]

On Fri, Dec 18, 2015 at 11:08 AM, Alvaro Herrera
 wrote:
> I don't know what would be a good reason to change from 9 to 10, but
> certainly we shouldn't do it just to remove a couple of GUCs -- much
> less do it for no reason at all (which would be what "but 9.6 is too
> close to 9.10 already" would boil down to.)  I sure hope we're gonna
> find some good reason to do it before 9.10 actually comes around.

I don't want to toot my own horn too much here, but I think parallel
query might be an appropriate milestone.  If we don't get any more
than what we have now done for this release, well, no, probably not.
But what if we get the parallel join stuff I've posted and the work
David Rowley and Haribabu Kommi are doing on parallel aggregate done?
At that point I think it starts to get pretty interesting.  Sure,
there will be plenty of work left to do, but that's often true: 9.0
introduced streaming replication and Hot Standby, but they got a lot
more usable in 9.1.

> On the other hand, while I agree with you that we should strive to
> maintain backwards compatible options for a long time, and that in this
> particular case I see no reason not to wait a few more releases since it
> doesn't hurt anything, I don't think we should make this an iron-clad
> rule: I imagine there might be cases where there will be good reasons to
> break it sooner than those 10 years if maintenance becomes a serious
> problem otherwise.  We will need to discuss each case individually as it
> comes up.

Right.  If a particular backward-compatibility flag is preventing
important improvements, that's a good argument for phasing it out
sooner.  But that's certainly not the case with array_nulls so, uh,
who cares?

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


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

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

[Robert Haas]

On Fri, Dec 18, 2015 at 1:16 AM, Amit Kapila  wrote:
> 1. At scale factor 300, there is gain of 11% at 128-client count and
> 27% at 256 client count with Patch-1. At 4 clients, the performance with
> Patch is 0.6% less (which might be a run-to-run variation or there could
> be a small regression, but I think it is too less to be bothered about)
>
> 2. At scale factor 1000, there is no visible difference and there is some
> at lower client count there is a <1% regression which could be due to
> I/O bound nature of test.
>
> 3. On these runs, Patch-2 is mostly always worse than Patch-1, but
> the difference between them is not significant.

Hmm, that's interesting.  So the slots don't help.  I was concerned
that with only a single slot, you might have things moving quickly
until you hit the point where you switch over to the next clog
segment, and then you get a bad stall.  It sounds like that either
doesn't happen in practice, or more likely it does happen but the
extra slot doesn't eliminate the stall because there's I/O at that
point.  Either way, it sounds like we can forget the slots idea for
now.

>> Some random comments:
>>
>> - TransactionGroupUpdateXidStatus could do just as well without
>> add_proc_to_group.  You could just say if (group_no >= NUM_GROUPS)
>> break; instead.  Also, I think you could combine the two if statements
>> inside the loop.  if (nextidx != INVALID_PGPROCNO &&
>> ProcGlobal->allProcs[nextidx].clogPage == proc->clogPage) break; or
>> something like that.
>>
>> - memberXid and memberXidstatus are terrible names.  Member of what?
>
> How about changing them to clogGroupMemberXid and
> clogGroupMemberXidStatus?

What we've currently got for group XID clearing for the ProcArray is
clearXid, nextClearXidElem, and backendLatestXid.  We should try to
make these things consistent.  Maybe rename those to
procArrayGroupMember, procArrayGroupNext, procArrayGroupXid and then
start all of these identifiers with clogGroup as you propose.

>> That's going to be clear as mud to the next person looking at the
>> definitiono f PGPROC.
>
> I understand that you don't like the naming convention, but using
> such harsh language could sometimes hurt others.

Sorry.  If I am slightly frustrated here I think it is because this
same point has been raised about three times now, by me and also by
Andres, just with respect to this particular technique, and also on
other patches.  But you are right - that is no excuse for being rude.

-- 
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] Costing foreign joins in postgres_fdw

[Robert Haas]

On Fri, Dec 18, 2015 at 8:09 AM, Albe Laurenz  wrote:
> My gut feeling is that for a join where all join predicates can be pushed 
> down, it
> will usually be a win to push the join to the foreign server.
>
> So in your first scenario, I'd opt for always pushing down the join
> if possible if use_remote_estimate is OFF.
>
> Your second scenario is essentially to estimate that a pushed down join will
> always be executed as a nested loop join, which will in most cases produce
> an unfairly negative estimate.

+1 to all that.  Whatever we do here for costing in detail, it should
be set up so that the pushed-down join wins unless there's some pretty
tangible reason to think, in a particular case, that it will lose.

> What about using local statistics to come up with an estimated row count for
> the join and use that as the basis for an estimate?  My idea here is that it
> is always be a win to push down a join unless the result set is so large that
> transferring it becomes the bottleneck.

This also sounds about right.

> Maybe, to come up with something remotely realistic, a formula like
>
> sum of locally estimated costs of sequential scan for the base table
> plus count of estimated result rows (times a factor)

Was this meant to say "the base tables", plural?

I think whatever we do here should try to extend the logic in
postgres_fdw's estimate_path_cost_size() to foreign tables in some
reasonably natural way, but I'm not sure exactly what that should look
like.  Maybe do what that function currently does for single-table
scans, and then add all the values up, or something like that.  I'm a
little worried, though, that the planner might then view a query that
will be executed remotely as a nested loop with inner index-scan as
not worth pushing down, because in that case the join actually will
not touch every row from both tables, as a hash or merge join would.

-- 
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] [COMMITTERS] pgsql: Handle policies during DROP OWNED BY

[Stephen Frost]

* Robert Haas (robertmh...@gmail.com) wrote:
> This appears to address one of the open items at
> https://wiki.postgresql.org/wiki/PostgreSQL_9.5_Open_Items -- if so,
> please update that page.

Done (and re-done with the wiki restore).

No open items remain against 9.5.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] [COMMITTERS] pgsql: Handle policies during DROP OWNED BY

[Robert Haas]

On Fri, Dec 18, 2015 at 11:40 AM, Stephen Frost  wrote:
> * Robert Haas (robertmh...@gmail.com) wrote:
>> This appears to address one of the open items at
>> https://wiki.postgresql.org/wiki/PostgreSQL_9.5_Open_Items -- if so,
>> please update that page.
>
> Done (and re-done with the wiki restore).
>
> No open items remain against 9.5.

Woohoo.  And also, phew.

-- 
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] Refactoring speculative insertion with unique indexes a little

[Robert Haas]

On Thu, Dec 17, 2015 at 2:55 AM, Peter Geoghegan  wrote:
> On Wed, Dec 16, 2015 at 11:44 PM, Peter Geoghegan  wrote:
>>> In any case, at this point 9.5 is really aimed to be stabilized, so
>>> targeting only master is a far saner approach IMO for this patch.
>>> Pushing that in 9.5 a couple of months back may have given enough
>>> reason to do so... But well life is life.
>>
>> No, this really isn't an optimization at all.
>
> I should add: I think that the chances of this patch destabilizing the
> code are very slim, once it receives the proper review. Certainly, I
> foresee no possible downside to not inserting the doomed IndexTuple,
> since it's guaranteed to have its heap tuple super-deleted immediately
> afterwards.
>
> That's the only real behavioral change proposed here. So, I would
> prefer it if we got this in before the first stable release of 9.5.

No, it's far too late to be pushing this into 9.5.  We are at RC1 now
and hoping to cut a final release right after Christmas.  I think it's
quite wrong to argue that these changes have no risk of destabilizing
9.5.  Nobody is exempt from having bugs in their code - not me, not
you, not Tom Lane.  But quite apart from that, there seems to be no
compelling benefit to having these changes in 9.5.  You say that the
branches will diverge needlessly, but the whole point of having
branches is that we do need things to diverge.  The question isn't
"why shouldn't these go into 9.5?" but "do these fix something that is
clearly broken in 9.5 and must be fixed to avoid hurting users?".
Andres has said clearly that he doesn't think so, and Heikki didn't
seem convinced that we wanted the changes at all.  I've read over the
thread and I think that even if all the good things you say about this
patch are 100% true, it doesn't amount to a good reason to back-patch.
Code that does something possibly non-sensical or sub-optimal isn't a
reason to back-patch in the absence of a clear, user-visible
consequence.

I think it's a shame that we haven't gotten this patch dealt with just
because when somebody submits a patch in June, it's not very nice for
it to still be pending in December, but since this stuff is even
further outside my area of expertise than the sorting stuff, and since
me and my split personalities only have so many hours in the day, I'm
going to have to leave it to somebody else to pick up anyhow.  But
that's a separate issue from whether this should be back-patched.

-- 
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] Remove array_nulls?

[Andres Freund]

On 2015-12-16 19:01:40 -0500, Robert Haas wrote:
> Yeah, there's something to be said for that, although to be honest in
> most cases I'd prefer to wait longer.   I wonder about perhaps
> planning to drop things after two lifecycles.

I don't really give a damn in this specific case. Seems to cost pretty
much nothing to continue having the GUC.

But I think in the more general case, which Tom seems to have brought up
as a point of policy, I think this is far to conservative. Yes, we owe
our users to not break their applications gratuitously. But we also owe
it to ourselves to keep development timeframes realistic, and not pay
overly much heed to people using seriously bad development and
maintenance practices.

It doesn't even benefit users really much delaying things that
long. Usually the migration costs, of fixing code previously kept
working by a GUC, increase over time, not decrease.


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

Re: [HACKERS] Typo in the comment above heap_prepare_freeze_tuple()

[Robert Haas]

On Fri, Dec 18, 2015 at 1:25 AM, Amit Langote
 wrote:
> I think the following may be a typo:
>
>   * Caller is responsible for ensuring that no other backend can access the
>   * storage underlying this tuple, either by holding an exclusive lock on the
> - * buffer containing it (which is what lazy VACUUM does), or by having it by
> + * buffer containing it (which is what lazy VACUUM does), or by having it be
>   * in private storage (which is what CLUSTER and friends do).
>
> If so, attached is the patch.

Committed.

-- 
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] Remove array_nulls?

[Robert Haas]

On Fri, Dec 18, 2015 at 12:02 PM, Andres Freund  wrote:
> On 2015-12-16 19:01:40 -0500, Robert Haas wrote:
>> Yeah, there's something to be said for that, although to be honest in
>> most cases I'd prefer to wait longer.   I wonder about perhaps
>> planning to drop things after two lifecycles.
>
> I don't really give a damn in this specific case. Seems to cost pretty
> much nothing to continue having the GUC.
>
> But I think in the more general case, which Tom seems to have brought up
> as a point of policy, I think this is far to conservative. Yes, we owe
> our users to not break their applications gratuitously. But we also owe
> it to ourselves to keep development timeframes realistic, and not pay
> overly much heed to people using seriously bad development and
> maintenance practices.

Well, Tom, Alvaro, and I all pretty much said that removing things
when it's blocking further development makes sense, but that there's
no hurry to remove anything else.  That sounds like what you are
saying, too.  So what's the actual disagreement here?

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


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

Re: [HACKERS] Freeze avoidance of very large table.

[Robert Haas]

On Thu, Dec 17, 2015 at 1:17 AM, Michael Paquier
 wrote:
> I am not really getting the meaning of this sentence. Shouldn't this
> be reworded something like:
> "Freezing occurs on the whole table once all pages of this relation require 
> it."

That statement isn't remotely true, and I don't think this patch
changes that.  Freezing occurs on the whole table once relfrozenxid is
old enough that we think there might be at least one page in the table
that requires 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] Remove array_nulls?

[Andres Freund]

On 2015-12-18 12:06:43 -0500, Robert Haas wrote:
> Well, Tom, Alvaro, and I all pretty much said that removing things
> when it's blocking further development makes sense, but that there's
> no hurry to remove anything else.  That sounds like what you are
> saying, too.  So what's the actual disagreement here?

I'm saying that 10 year deprecation periods don't make sense. Either we
decide to remove the compat switch because we dislike it for $reasons,
in which case it should be removed sooner. Or we decide to keep the
switch indefinitely.


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

Re: [HACKERS] Remove array_nulls?

[Tom Lane]

I wrote:
> Not entirely sure what to make of this.  It occurs to me that the "it
> breaks immutability" argument might apply to array_nulls, though I've
> not done any legwork to confirm or disprove that.  If it doesn't apply,
> though, I'm leaning to the position that there's no reason to remove
> array_nulls.

OK, I went and looked.  Array_nulls is consulted only in array_in(),
which is marked stable (and would need to be so marked even without
this consideration, since the array element type's input function
might only be stable).  So it's not breaking any rules.

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] Freeze avoidance of very large table.

[Robert Haas]

On Thu, Dec 17, 2015 at 2:26 AM, Andres Freund  wrote:
> On 2015-12-17 16:22:24 +0900, Michael Paquier wrote:
>> On Thu, Dec 17, 2015 at 4:10 PM, Andres Freund  wrote:
>> > On 2015-12-17 15:56:35 +0900, Michael Paquier wrote:
>> >> On Thu, Dec 17, 2015 at 3:44 PM, Simon Riggs  
>> >> wrote:
>> >> > For me, rewriting the visibility map is a new data loss bug waiting to
>> >> > happen. I am worried that the group is not taking seriously the 
>> >> > potential
>> >> > for catastrophe here.
>> >>
>> >> FWIW, I'm following this line and merging the vm file into a single
>> >> unit looks like a ticking bomb.
>> >
>> > And what are those risks?
>>
>> Incorrect vm file rewrite after a pg_upgrade run.
>
> If we can't manage to rewrite a file, replacing a binary b1 with a b10,
> then we shouldn't be working on a database. And if we screw up, recovery
> i is an rm *_vm away. I can't imagine that this is going to be the
> actually complicated part of this feature.

Yeah.  If that part of this feature isn't right, the chances that the
rest of the patch are robust enough to commit seem extremely low.
That is, as Andres says, not the hard part.

-- 
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] Remove array_nulls?

[Robert Haas]

On Fri, Dec 18, 2015 at 12:10 PM, Andres Freund  wrote:
> On 2015-12-18 12:06:43 -0500, Robert Haas wrote:
>> Well, Tom, Alvaro, and I all pretty much said that removing things
>> when it's blocking further development makes sense, but that there's
>> no hurry to remove anything else.  That sounds like what you are
>> saying, too.  So what's the actual disagreement here?
>
> I'm saying that 10 year deprecation periods don't make sense. Either we
> decide to remove the compat switch because we dislike it for $reasons,
> in which case it should be removed sooner. Or we decide to keep the
> switch indefinitely.

Forever is an awfully long time.  I think that it's OK to remove
backward-compatibility features at some point even if they're not
really harming anything.  I think the time before we do that should be
long, but I don't think it needs to be forever.

-- 
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] Remove array_nulls?

[Joshua D. Drake]

On 12/18/2015 09:12 AM, Robert Haas wrote:

On Fri, Dec 18, 2015 at 12:10 PM, Andres Freund  wrote:

On 2015-12-18 12:06:43 -0500, Robert Haas wrote:

Well, Tom, Alvaro, and I all pretty much said that removing things
when it's blocking further development makes sense, but that there's
no hurry to remove anything else.  That sounds like what you are
saying, too.  So what's the actual disagreement here?


I'm saying that 10 year deprecation periods don't make sense. Either we
decide to remove the compat switch because we dislike it for $reasons,
in which case it should be removed sooner. Or we decide to keep the
switch indefinitely.


Forever is an awfully long time.  I think that it's OK to remove
backward-compatibility features at some point even if they're not
really harming anything.  I think the time before we do that should be
long, but I don't think it needs to be forever.


Why not just keep it at the same rate as our support policy? The feature 
gets 5 years, then it is removed.


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] Remove array_nulls?

[Robert Haas]

On Fri, Dec 18, 2015 at 12:19 PM, Joshua D. Drake  
wrote:
> On 12/18/2015 09:12 AM, Robert Haas wrote:
>>
>> On Fri, Dec 18, 2015 at 12:10 PM, Andres Freund 
>> wrote:
>>>
>>> On 2015-12-18 12:06:43 -0500, Robert Haas wrote:

 Well, Tom, Alvaro, and I all pretty much said that removing things
 when it's blocking further development makes sense, but that there's
 no hurry to remove anything else.  That sounds like what you are
 saying, too.  So what's the actual disagreement here?
>>>
>>>
>>> I'm saying that 10 year deprecation periods don't make sense. Either we
>>> decide to remove the compat switch because we dislike it for $reasons,
>>> in which case it should be removed sooner. Or we decide to keep the
>>> switch indefinitely.
>>
>>
>> Forever is an awfully long time.  I think that it's OK to remove
>> backward-compatibility features at some point even if they're not
>> really harming anything.  I think the time before we do that should be
>> long, but I don't think it needs to be forever.
>
>
> Why not just keep it at the same rate as our support policy? The feature
> gets 5 years, then it is removed.

I did discuss that exact question in several previous postings to this thread...

-- 
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] Remove array_nulls?

[Tom Lane]

Robert Haas  writes:
> On Fri, Dec 18, 2015 at 12:10 PM, Andres Freund  wrote:
>> I'm saying that 10 year deprecation periods don't make sense. Either we
>> decide to remove the compat switch because we dislike it for $reasons,
>> in which case it should be removed sooner. Or we decide to keep the
>> switch indefinitely.

> Forever is an awfully long time.  I think that it's OK to remove
> backward-compatibility features at some point even if they're not
> really harming anything.  I think the time before we do that should be
> long, but I don't think it needs to be forever.

Maybe I shouldn't put words in Andres' mouth, but I don't think that by
"indefinitely" he meant "forever".  I read that more as "until some
positive reason to remove it arrives".  I could imagine that at some point
we decide to do a wholesale cleanup of backwards-compatibility GUCs, and
then we'd zap this one along with others.

By itself, though, array_nulls seems about as harmless as such things get.
The sum total of the code simplification we'd get from removing it is
that the first segment of this if-test would go away:

if (Array_nulls && !hasquoting &&
pg_strcasecmp(itemstart, "NULL") == 0)

So there's no plausible argument that it's causing development problems.

I am mindful of Josh's frequent complaint that we have too many GUCs,
which is a legitimate concern; but removing just one won't do much
for that.

regards, tom lane


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

Re: [HACKERS] [PATCH] Copy-pasteo in logical decoding

[Robert Haas]

On Thu, Dec 17, 2015 at 11:46 PM, Craig Ringer  wrote:
> Trivial fix for a copy-and-paste error in a logical decoding error callback.

Committed and back-patched to 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] Remove array_nulls?

[David G. Johnston]

On Fri, Dec 18, 2015 at 10:25 AM, Tom Lane  wrote:

> Robert Haas  writes:
> > On Fri, Dec 18, 2015 at 12:10 PM, Andres Freund 
> wrote:
> >> I'm saying that 10 year deprecation periods don't make sense. Either we
> >> decide to remove the compat switch because we dislike it for $reasons,
> >> in which case it should be removed sooner. Or we decide to keep the
> >> switch indefinitely.
>
> > Forever is an awfully long time.  I think that it's OK to remove
> > backward-compatibility features at some point even if they're not
> > really harming anything.  I think the time before we do that should be
> > long, but I don't think it needs to be forever.
>
> Maybe I shouldn't put words in Andres' mouth, but I don't think that by
> "indefinitely" he meant "forever".  I read that more as "until some
> positive reason to remove it arrives".  I could imagine that at some point
> we decide to do a wholesale cleanup of backwards-compatibility GUCs, and
> then we'd zap this one along with others.
>

​Hand-waving from me but I see a "positive reason" being that someone wants
to write and commit a patch that does not play nicely with the old
behavior.  That patch can then do away with giving the user an option as
long as the GUC itself was introduced in a now unsupported release.

I do not have a feel for how much grief having these GUCs in the code
causes but if the concern is for the end-user then simply removing them
from (or tucking them into a dark corner of) the documentation seems like
it would be the most useful means of "removing" while still be friendly to
users that just haven't wanted to update their application code to the new
way of doing things.

David J.

Re: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

[Robert Haas]

On Thu, Dec 17, 2015 at 11:43 PM, Peter Geoghegan  wrote:
> On Wed, Dec 16, 2015 at 12:04 PM, Peter Geoghegan  wrote:
>>> What kind of state is that?  Can't we define this in terms of what it
>>> is rather than how it gets that way?
>>
>> It's zeroed.
>>
>> I guess we can update everything, including existing comments, to reflect 
>> that.

Thanks, this looks much easier to understand from here.

> Attached revision updates both the main commit (the optimization), and
> the backpatch commit (updated the contract).

-   /* abbreviation is possible here only for by-reference types */
+   /*
+* Abbreviation is possible here only for by-reference types.
Note that a
+* pass-by-value representation for abbreviated values is forbidden, but
+* that's a distinct, generic restriction imposed by the SortSupport
+* contract.

I think that you have not written what you meant to write here.  I
think what you mean is "Note that a pass-by-REFERENCE representation
for abbreviated values is forbidden...".

+   /*
+* If we produced only one initial run (quite likely if the total data
+* volume is between 1X and 2X workMem), we can just use that
tape as the
+* finished output, rather than doing a useless merge.  (This obvious
+* optimization is not in Knuth's algorithm.)
+*/
+   if (state->currentRun == 1)
+   {
+   state->result_tape = state->tp_tapenum[state->destTape];
+   /* must freeze and rewind the finished output tape */
+   LogicalTapeFreeze(state->tapeset, state->result_tape);
+   state->status = TSS_SORTEDONTAPE;
+   return;
+   }

I don't understand the point of moving this code.  If there's some
reason to do this after rewinding the tapes rather than beforehand, I
think we should articulate that reason in the comment block.

The last hunk in your 0001 patch properly belongs in 0002.

-- 
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] Bug in TupleQueueReaderNext() ?

[Robert Haas]

On Wed, Dec 16, 2015 at 1:09 AM, Rushabh Lathia
 wrote:
> TupleQueueReaderNext() always pass true for the nowait into
> shm_mq_receive() call. I think here it need to pass the nowait
> which is passed by the caller of TupleQueueReaderNext.
>
> This is usefull if the caller want TupleQueueReaderNext() to wait
> until it gets the tuple from the particular queue.

Boy, that's an embarassing mistake.  *blushes*

Thanks for the report and fix.  Committed.

-- 
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] Schedule plans for upcoming Postgres releases

[Tom Lane]

Just so everyone's on the same page: the release team is currently
assuming that we'll release 9.5.0 the first week of January (ie
wrap on Monday 4 Jan for public announcement Thursday 7 Jan).
And we're thinking that we'll do a round of back-branch updates,
including 9.5.1, the second week of February (wrap on Feb 8).

Discovery of dire bugs might cause these dates to move, but that's
the plan at the moment.

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] Remove array_nulls?

[Tom Lane]

"David G. Johnston"  writes:
> On Fri, Dec 18, 2015 at 10:25 AM, Tom Lane  wrote:
>> Maybe I shouldn't put words in Andres' mouth, but I don't think that by
>> "indefinitely" he meant "forever".  I read that more as "until some
>> positive reason to remove it arrives".  I could imagine that at some point
>> we decide to do a wholesale cleanup of backwards-compatibility GUCs, and
>> then we'd zap this one along with others.

>​Hand-waving from me but I see a "positive reason" being that someone wants
> to write and commit a patch that does not play nicely with the old
> behavior.

Sure, that's also possible.  But no such patch is on the table now.

regards, tom lane


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

Re: [HACKERS] A typo in syncrep.c

[Robert Haas]

On Wed, Dec 16, 2015 at 3:33 AM, Kyotaro HORIGUCHI
 wrote:
> Hello, I think I found a typo in a comment of syncrep.c.
>
>>   * acknowledge the commit nor raise ERROR or FATAL.  The latter would
>> - * lead the client to believe that that the transaction aborted, which
>>   * is not true: it's already committed locally. The former is no good
>
> The 'that' looks duplicate.

Agreed.

> And it might be better to put a
> be-verb before the 'aborted'.
>
>> + * lead the client to believe that the transaction is aborted, which

No, that's correct the way it is.  What you're proposing wouldn't
exactly be wrong, but it's a little less clear and direct.

Committed the part of your patch that removes the extra "that".

-- 
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: ResourceOwner optimization for tables with many partitions

[Robert Haas]

On Mon, Dec 14, 2015 at 6:47 AM, Aleksander Alekseev
 wrote:
> Here is my fix for item 4.

I don't know, I'm still not very comfortable with this.  And Tom
didn't like dictating that hash_any() must be no-fail, though I'm not
sure why.

Let's wait to see what others think.  I kind of hope there's a way of
getting the benefits we want here without so much code churn.

-- 
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] Using quicksort for every external sort run

[Robert Haas]

On Sat, Dec 12, 2015 at 5:28 PM, Peter Geoghegan  wrote:
> On Sat, Dec 12, 2015 at 12:10 AM, Jeff Janes  wrote:
>> I have a question about the terminology used in this patch.  What is a
>> tuple proper?  What is it in contradistinction to?  I would think that
>> a tuple which is located in its own palloc'ed space is the "proper"
>> one, leaving a tuple allocated in the bulk memory pool to be
>> called...something else.  I don't know what the
>> non-judgmental-sounding antonym of postpositive "proper" is.
>
> "Tuple proper" is a term that appears 5 times in tuplesort.c today. As
> it says at the top of that file:
>
> /*
>  * The objects we actually sort are SortTuple structs.  These contain
>  * a pointer to the tuple proper (might be a MinimalTuple or IndexTuple),
>  * which is a separate palloc chunk --- we assume it is just one chunk and
>  * can be freed by a simple pfree().  SortTuples also contain the tuple's
>  * first key column in Datum/nullflag format, and an index integer.

I see only three.  In each case, "the tuple proper" could be replaced
by "the tuple itself" or "the actual tuple" without changing the
meaning, at least according to my understanding of the meaning.  If
that's causing confusion, perhaps we should just change the existing
wording.

Anyway, I agree with Jeff that this terminology shouldn't creep into
function and structure member names.

I don't really like the term "memory pool" either.  We're growing a
bunch of little special-purpose allocators all over the code base
because of palloc's somewhat dubious performance and memory usage
characteristics, but if any of those are referred to as memory pools
it has thus far escaped my notice.

-- 
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] Additional LWLOCK_STATS statistics

[Robert Haas]

On Wed, Dec 16, 2015 at 5:02 AM, Jesper Pedersen
 wrote:
> On 09/16/2015 12:44 PM, Jesper Pedersen wrote:
>>
>> So, I think there is some value in keeping this information separate.
>>
>
> Just a rebased patch after the excellent LWLockTranche work.
>
> And a new sample report with -c/-j 200 -M prepared.

Is this just for informational purposes, or is this something you are
looking to have committed?  I originally thought the former, but now
I'm wondering if I misinterpreted your intent.  I have a hard time
getting excited about committing something that would, unless I'm
missing something, pretty drastically increase the overhead of running
with LWLOCK_STATS...

-- 
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] extend pgbench expressions with functions

[Robert Haas]

On Wed, Dec 16, 2015 at 12:54 AM, Michael Paquier
 wrote:
> On Wed, Dec 16, 2015 at 6:10 AM, Robert Haas  wrote:
>> On Mon, Dec 14, 2015 at 7:25 AM, Michael Paquier  
>> wrote:
>>> I have looked for now at the first patch and finished with the
>>> attached while looking at it. Perhaps a committer could look already
>>> at that?
>>
>> It looks fine to me except that I think we should spell out "param" as
>> "parameter" throughout, instead of abbreviating.
>
> Fine for me. I have updated the first patch as attached (still looking
> at the second).

Committed.  I was on the fence about whether to back-patch this
considering how close we are to release, but ended up deciding to go
for 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] Parallel Aggregate

[Robert Haas]

On Wed, Dec 16, 2015 at 5:59 AM, David Rowley
 wrote:
> One thing I noticed is that you're only enabling Parallel aggregation when
> there's already a Gather node in the plan. Perhaps this is fine for a proof
> of concept, but I'm wondering how we can move forward from this to something
> that can be committed.

As far as that goes, I think the infrastructure introduced by the
parallel join patch will be quite helpful here.  That introduces the
concept of a "partial path" - that is, a path that needs a Gather node
in order to be completed.  And that's exactly what you need here:
after join planning, if there's a partial path available for the final
rel, then you can consider
FinalizeAggregate->Gather->PartialAggregate->[the best partial path].
Of course, whether a partial path is available or not, you can
consider Aggregate->[the best regular old path].

-- 
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: ResourceOwner optimization for tables with many partitions

[Tom Lane]

Robert Haas  writes:
> I don't know, I'm still not very comfortable with this.  And Tom
> didn't like dictating that hash_any() must be no-fail, though I'm not
> sure why.

What I definitely didn't like was assuming at a distance that it would
be no-fail.  If we're to depend on that, the patch had better attach
a comment saying so to the header comments of the function(s) it's
assuming that about.  Otherwise, somebody could hack up hashfunc.c
in a way that breaks the assumption, without any clue that some code
in a very-far-away module is critically reliant on it.

> Let's wait to see what others think.

A few observations:

* This bit is too cute by half, if not three-quarters:

+   uint32  itemsizelg:2;   /* sizeof one item log 2 */
+   uint32  capacity:30;/* capacity of array */

Is there a good reason to assume that the only things we'll ever store
in these arrays are of size no more than 8 bytes?  Are we so desperate
to save space that we cannot spare two separate words for itemsize and
capacity?  (ISTM it's a good bet that the extra code for accessing these
bitfields occupies more space than would be saved, considering how few
ResourceOwners typically exist at one time.)  Let's just make it a couple
of ints and be done.  Actually, maybe nitems and capacity should be
size_t, just in case.

* An alternative design would be to forget itemsizelg altogether and insist
that everything stored in the resource arrays be a Datum, which could then
be coerced to/from some form of integer or some form of pointer as
appropriate.  That would waste some space in the int case, but it would
considerably simplify both the ResourceArray code and the APIs to it,
which might be worth the price of assuming we'll never store anything
bigger than 8 bytes.  It also would make this look more like some
existing APIs such as the on_exit callbacks.

* A lot of the code churn comes from the insistence on defining callbacks,
which I'm dubious that we need.  We could instead have a function that is
"get any convenient one of the array elements" and revise the loops in
ResourceOwnerReleaseInternal to be like

while ((item = getconvenientitem(resourcearray)))
{
drop item in exactly the same way as before
}

I find that preferable to the proposed ResourceArrayRemoveAll

+   while (resarr->nitems > 0)
+   {
+   releasecb(resarr->itemsarr, isCommit);
+   }

which certainly looks like it's an infinite loop; it's assuming (again
with no documentation) that the callback function will cause the array
to get smaller somehow.  With the existing coding, it's much more clear
why we think the loops will terminate.

* The reason that ResourceOwnerReleaseInternal was not horribly
inefficient was that its notion of "any convenient one" of the items
to be deleted next was in fact the one that the corresponding Forget
function would examine first, thus avoiding an O(N^2) cost to
re-identify the item to be dropped.  I think we should make an effort
to be more explicit about that connection in any rewrite.  In particular,
it looks to me like when a hash array is in use, things will get slower
not faster because we'll be adding a hash lookup step to each forget
operation.  Maybe we should consider adjusting the APIs so that that
can be avoided.  Or possibly we could have internal state in the
ResourceArrays that says "we expect this item to be dropped in a moment,
check that before going to the trouble of a hash lookup".

* Actually, I'm not convinced that the proposed reimplementation of
ResourceArrayRemove isn't horribly slow much of the time.  It sure
looks like it could degrade to a linear search very easily.

* I still say that the assumption embodied as RESOURCE_ARRAY_ZERO_ELEMENT
(ie that no valid entry is all-zero-bits) is pretty unacceptable.  It
might work for pointers, but I don't like it for resources represented
by integer indexes.

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 Typo in regress/sql/privileges.sql

[Robert Haas]

On Wed, Dec 16, 2015 at 11:51 PM, Tatsuro Yamada
 wrote:
> I found typos in privileges.sql and privileges.out
> Please find attached a patch.

Thanks, good catch.  But even aside from this particular issue, isn't
that comment in need of a little more love?  An inference means a
deduction, or something you can figure out from something else.  ON
CONFLICT (four) is not an inference.  Why don't we say:

-- Check that the columns in the ON CONFLICT clause require select privileges

-- 
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] Using quicksort for every external sort run

[Peter Geoghegan]

On Fri, Dec 18, 2015 at 10:12 AM, Robert Haas  wrote:
> Anyway, I agree with Jeff that this terminology shouldn't creep into
> function and structure member names.

Okay.

> I don't really like the term "memory pool" either.  We're growing a
> bunch of little special-purpose allocators all over the code base
> because of palloc's somewhat dubious performance and memory usage
> characteristics, but if any of those are referred to as memory pools
> it has thus far escaped my notice.

It's a widely accepted term: https://en.wikipedia.org/wiki/Memory_pool

But, sure, I'm not attached to it.

-- 
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] A Typo in regress/sql/privileges.sql

[Andres Freund]

On 2015-12-18 13:50:34 -0500, Robert Haas wrote:
> On Wed, Dec 16, 2015 at 11:51 PM, Tatsuro Yamada
>  wrote:
> > I found typos in privileges.sql and privileges.out
> > Please find attached a patch.
> 
> Thanks, good catch.  But even aside from this particular issue, isn't
> that comment in need of a little more love?  An inference means a
> deduction, or something you can figure out from something else.  ON
> CONFLICT (four) is not an inference.

It's the index(es) that are inferred, from the ON(columns) and the ON
CONFLICT's WHERE clause. If we want to get rid of that terminology we'd
need to start elsewhere, and it'd be a bigger patch.

Andres


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

Re: [HACKERS] Refactoring speculative insertion with unique indexes a little

[Peter Geoghegan]

On Fri, Dec 18, 2015 at 8:58 AM, Robert Haas  wrote:
> No, it's far too late to be pushing this into 9.5.  We are at RC1 now
> and hoping to cut a final release right after Christmas.  I think it's
> quite wrong to argue that these changes have no risk of destabilizing
> 9.5.  Nobody is exempt from having bugs in their code - not me, not
> you, not Tom Lane.  But quite apart from that, there seems to be no
> compelling benefit to having these changes in 9.5.  You say that the
> branches will diverge needlessly, but the whole point of having
> branches is that we do need things to diverge.  The question isn't
> "why shouldn't these go into 9.5?" but "do these fix something that is
> clearly broken in 9.5 and must be fixed to avoid hurting users?".
> Andres has said clearly that he doesn't think so, and Heikki didn't
> seem convinced that we wanted the changes at all.

It isn't true that Heikki was not basically in favor of this. This
should have been committed as part of the original patch, really.

I hope to avoid needless confusion about the documented (by the
official documentation) AM interface. Yes, that is

> I think it's a shame that we haven't gotten this patch dealt with just
> because when somebody submits a patch in June, it's not very nice for
> it to still be pending in December, but since this stuff is even
> further outside my area of expertise than the sorting stuff, and since
> me and my split personalities only have so many hours in the day, I'm
> going to have to leave it to somebody else to pick up anyhow.  But
> that's a separate issue from whether this should be back-patched.

Note that I've already proposed a compromise, even though I don't
think my original position was at all unreasonable. There'd be zero
real changes (only the addition of the new constant name,
documentation updates, comment updates, etc) under that compromise (as
against one change.).

-- 
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] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

[Peter Geoghegan]

On Fri, Dec 18, 2015 at 9:35 AM, Robert Haas  wrote:
>> Attached revision updates both the main commit (the optimization), and
>> the backpatch commit (updated the contract).
>
> -   /* abbreviation is possible here only for by-reference types */
> +   /*
> +* Abbreviation is possible here only for by-reference types.
> Note that a
> +* pass-by-value representation for abbreviated values is forbidden, 
> but
> +* that's a distinct, generic restriction imposed by the SortSupport
> +* contract.
>
> I think that you have not written what you meant to write here.  I
> think what you mean is "Note that a pass-by-REFERENCE representation
> for abbreviated values is forbidden...".

You're right. Sorry about that.

> +   /*
> +* If we produced only one initial run (quite likely if the total data
> +* volume is between 1X and 2X workMem), we can just use that
> tape as the
> +* finished output, rather than doing a useless merge.  (This obvious
> +* optimization is not in Knuth's algorithm.)
> +*/
> +   if (state->currentRun == 1)
> +   {
> +   state->result_tape = state->tp_tapenum[state->destTape];
> +   /* must freeze and rewind the finished output tape */
> +   LogicalTapeFreeze(state->tapeset, state->result_tape);
> +   state->status = TSS_SORTEDONTAPE;
> +   return;
> +   }
>
> I don't understand the point of moving this code.  If there's some
> reason to do this after rewinding the tapes rather than beforehand, I
> think we should articulate that reason in the comment block.

I thought that was made clear by the 0001 commit message. Think of
what happens when we don't disable abbreviated in the final
TSS_SORTEDONTAPE phase should the "if (state->currentRun == 1)" path
have been taken (but *not* the path that also ends in
TSS_SORTEDONTAPE, when caller requires randomAccess but we spill to
tape, or any other case). What happens is: The code in 0002 gets
confused, and attempts to pass back a pointer value as an "abbreviated
key". That's a bug.

> The last hunk in your 0001 patch properly belongs in 0002.

You could certainly argue that the last hunk of 0001 belongs in 0002.
I only moved it to 0001 when I realized that we might as well keep the
branches in sync, since the ordering is insignificant from a 9.5
perspective (although it might still be tidier), and there is a need
to backpatch anyway. I'm not insisting on doing it that way.

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

[Artur Zakirov]

On 18.12.2015 22:43, Artur Zakirov wrote:

Hello.

PostgreSQL has a contrib module named pg_trgm. It is used to the fuzzy 
text search. It provides some functions and operators for determining 
the similarity of the given texts using trigram matching.



Sorry, I have forgotten to mark previous message with [PROPOSAL].
I registered the patch in commitfest:
https://commitfest.postgresql.org/8/448/

--
Artur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company



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

Re: [HACKERS] Using quicksort for every external sort run

[Peter Geoghegan]

On Fri, Dec 18, 2015 at 10:12 AM, Robert Haas  wrote:
> I don't really like the term "memory pool" either.  We're growing a
> bunch of little special-purpose allocators all over the code base
> because of palloc's somewhat dubious performance and memory usage
> characteristics, but if any of those are referred to as memory pools
> it has thus far escaped my notice.

BTW, I'm not necessarily determined to make the new special-purpose
allocator work exactly as proposed. It seemed useful to prioritize
simplicity, and currently so there is one big "huge palloc()" with
which we blow our memory budget, and that's it. However, I could
probably be more clever about "freeing ranges" initially preserved for
a now-exhausted tape. That kind of thing.

With the on-the-fly merge memory patch, I'm improving locality of
access (for each "tuple proper"/"tuple itself"). If I also happen to
improve the situation around palloc() fragmentation at the same time,
then so much the better, but that's clearly secondary.

-- 
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] [patch] Proposal for \rotate in psql

[Daniel Verite]

Pavel Stehule wrote:

> The symbol 'X' in two column mode should be centred - now it is aligned to
> left, what is not nice

Currently print.c does not support centered alignment, only left and right.
Should we add it, it would have to work for all output formats
(except obviously for "unaligned"):
- aligned
- wrapped
- html
- latex
- latex-longtable
- troff-ms
- asciidoc

Because of this, I believe that adding support for a 'c' alignment
might be a significant patch by itself, and that it should be considered
separately.

I agree that if it existed, the crosstabview command should use it
as you mention, but I'm not volunteering to implement it myself, at
least not in the short term.

Best regards,
-- 
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite


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

Re: [HACKERS] Making tab-complete.c easier to maintain

[Tom Lane]

Michael Paquier  writes:
> OK, I am marking that as ready for committer. Let's see what happens next.

I'll pick this up, as penance for not having done much in this commitfest.
I think it's important to get it pushed quickly so that Thomas doesn't
have to keep tracking unrelated changes in tab-complete.c.

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] Making tab-complete.c easier to maintain

[Tom Lane]

Thomas Munro  writes:
> [ tab-complete-macrology-v11.patch.gz ]

A couple of stylistic reactions after looking through the patch for the
first time in a long time:

1. It seems inconsistent that all the new macros are named in CamelCase
style, whereas there is still plenty of usage of the existing macros like
COMPLETE_WITH_LIST.  It looks pretty jarring IMO.  I think we should
either rename the new macros back to all-upper-case style, or rename the
existing macros in CamelCase style.

I slightly favor the latter option; we're already pretty much breaking any
hope of tab-complete fixes applying backwards over this patch, so changing
the code even more doesn't seem like a problem.  Either way, it's a quick
search-and-replace.  Thoughts?

2. Why does MatchAnyExcept use "'" as the inversion flag, rather than
say "!" or "~" ?  Seems pretty random.

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] Getting sorted data from foreign server for merge join

[Robert Haas]

On Thu, Dec 17, 2015 at 3:32 AM, Ashutosh Bapat
 wrote:
> On Wed, Dec 9, 2015 at 12:14 AM, Robert Haas  wrote:
>> On Wed, Dec 2, 2015 at 6:45 AM, Rushabh Lathia 
>> wrote:
>> > Thanks Ashutosh.
>> >
>> > Re-reviewed and Re-verified the patch, pg_sort_all_pd_v5.patch
>> > looks good to me.
>>
>> This patch needs a rebase.
>
> Done.

Thanks.

>> It's not going to work to say this is a patch proposed for commit when
>> it's still got a TODO comment in it that obviously needs to be
>> changed.   And the formatting of that long comment is pretty weird,
>> too, and not consistent with other functions in that same file (e.g.
>> get_remote_estimate, ec_member_matches_foreign, create_cursor).
>>
>
> The TODO was present in v4 but not in v5 and is not present in v6 attached
> here.. Formatted comment according estimate_path_cost_size(),
> convert_prep_stmt_params().

Hrm, I must have been looking at the wrong version somehow.  Sorry about that.

>> Aside from that, I think before we commit this, somebody should do
>> some testing that demonstrates that this is actually a good idea.  Not
>> as part of the test case set for this patch, but just in general.
>> Merge joins are typically going to be relevant for large tables, but
>> the examples in the regression tests are necessarily tiny.  I'd like
>> to see some sample data and some sample queries that get appreciably
>> faster with this code.  If we can't find any, we don't need the code.
>>
>
> I tested the patch on my laptop with two types of queries, a join between
> two foreign tables on different foreign servers (pointing to the same self
> server) and a join between one foreign and one local table. The foreign
> tables and servers are created using sort_pd_setup.sql attached. Foreign
> tables pointed to table with index useful for join clause. Both the joining
> tables had 10M rows. The execution time of query was measured for 100 runs
> and average and standard deviation were calculated (using function
> query_execution_stats() in script sort_pd.sql) and are presented below.

OK, cool.

I went over this patch in some detail today and did a lot of cosmetic
cleanup.  The results are attached.  I'm fairly happy with this
version, but let me know what you think.  Of course, feedback from
others is more than welcome also.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 866a09b..f10752d 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -343,6 +343,76 @@ SELECT 'fixed', NULL FROM ft1 t1 WHERE c1 = 1;
  fixed| 
 (1 row)
 
+-- Test forcing the remote server to produce sorted data for a merge join.
+SET enable_hashjoin TO false;
+SET enable_nestloop TO false;
+-- inner join; expressions in the clauses appear in the equivalence class list
+EXPLAIN (VERBOSE, COSTS false)
+	SELECT t1.c1, t2."C 1" FROM ft2 t1 JOIN "S 1"."T 1" t2 ON (t1.c1 = t2."C 1") OFFSET 100 LIMIT 10;
+ QUERY PLAN 
+
+ Limit
+   Output: t1.c1, t2."C 1"
+   ->  Merge Join
+ Output: t1.c1, t2."C 1"
+ Merge Cond: (t1.c1 = t2."C 1")
+ ->  Foreign Scan on public.ft2 t1
+   Output: t1.c1
+   Remote SQL: SELECT "C 1" FROM "S 1"."T 1" ORDER BY "C 1" ASC
+ ->  Index Only Scan using t1_pkey on "S 1"."T 1" t2
+   Output: t2."C 1"
+(10 rows)
+
+SELECT t1.c1, t2."C 1" FROM ft2 t1 JOIN "S 1"."T 1" t2 ON (t1.c1 = t2."C 1") OFFSET 100 LIMIT 10;
+ c1  | C 1 
+-+-
+ 101 | 101
+ 102 | 102
+ 103 | 103
+ 104 | 104
+ 105 | 105
+ 106 | 106
+ 107 | 107
+ 108 | 108
+ 109 | 109
+ 110 | 110
+(10 rows)
+
+-- outer join; expressions in the clauses do not appear in equivalence class
+-- list but no output change as compared to the previous query
+EXPLAIN (VERBOSE, COSTS false)
+	SELECT t1.c1, t2."C 1" FROM ft2 t1 LEFT JOIN "S 1"."T 1" t2 ON (t1.c1 = t2."C 1") OFFSET 100 LIMIT 10;
+ QUERY PLAN 
+
+ Limit
+   Output: t1.c1, t2."C 1"
+   ->  Merge Left Join
+ Output: t1.c1, t2."C 1"
+ Merge Cond: (t1.c1 = t2."C 1")
+ ->  Foreign Scan on public.ft2 t1
+   Output: t1.c1
+   Remote SQL: SELECT "C 1" FROM "S 1"."T 1" ORDER BY "C 1" ASC
+ ->  Index Only Scan using t1_pkey on "S 1"."T 1" t2
+   Output: t2."C 1"
+(10 rows)
+
+SELECT t1.c1, t2."C 1" FROM ft2 t1 LEFT JOIN "S 1"."T 1" t2 ON (t1.c1 = t2."C 1") OFFSET 100 LIMIT 10;
+ c1  | C 1 
+-+-
+ 101 | 101
+ 102 | 102
+ 103 | 103
+ 104 | 104
+ 105 | 105
+ 106 | 106
+ 107 | 107
+ 108 | 108
+ 109 | 109
+ 110 | 110
+(10 rows)
+
+RESET enable_hashj

Re: [HACKERS] Using quicksort for every external sort run

[Robert Haas]

On Fri, Dec 18, 2015 at 2:57 PM, Peter Geoghegan  wrote:
> On Fri, Dec 18, 2015 at 10:12 AM, Robert Haas  wrote:
>> I don't really like the term "memory pool" either.  We're growing a
>> bunch of little special-purpose allocators all over the code base
>> because of palloc's somewhat dubious performance and memory usage
>> characteristics, but if any of those are referred to as memory pools
>> it has thus far escaped my notice.
>
> BTW, I'm not necessarily determined to make the new special-purpose
> allocator work exactly as proposed. It seemed useful to prioritize
> simplicity, and currently so there is one big "huge palloc()" with
> which we blow our memory budget, and that's it. However, I could
> probably be more clever about "freeing ranges" initially preserved for
> a now-exhausted tape. That kind of thing.

What about the case where we think that there will be a lot of data
and have a lot of work_mem available, but then the user sends us 4
rows because of some mis-estimation?

> With the on-the-fly merge memory patch, I'm improving locality of
> access (for each "tuple proper"/"tuple itself"). If I also happen to
> improve the situation around palloc() fragmentation at the same time,
> then so much the better, but that's clearly secondary.

I don't really understand this comment.

-- 
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 Typo in regress/sql/privileges.sql

[Robert Haas]

On Fri, Dec 18, 2015 at 1:57 PM, Andres Freund  wrote:
> On 2015-12-18 13:50:34 -0500, Robert Haas wrote:
>> On Wed, Dec 16, 2015 at 11:51 PM, Tatsuro Yamada
>>  wrote:
>> > I found typos in privileges.sql and privileges.out
>> > Please find attached a patch.
>>
>> Thanks, good catch.  But even aside from this particular issue, isn't
>> that comment in need of a little more love?  An inference means a
>> deduction, or something you can figure out from something else.  ON
>> CONFLICT (four) is not an inference.
>
> It's the index(es) that are inferred, from the ON(columns) and the ON
> CONFLICT's WHERE clause. If we want to get rid of that terminology we'd
> need to start elsewhere, and it'd be a bigger patch.

It might be an inference specification, but there is no way that it is
an inference.  If we use that terminology in other places, it's wrong
there, too.

Mind you, I don't think "inference specification" is very good
terminology, but what's there right now is just wrong.

-- 
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] Refactoring speculative insertion with unique indexes a little

[Robert Haas]

On Fri, Dec 18, 2015 at 2:04 PM, Peter Geoghegan  wrote:
> It isn't true that Heikki was not basically in favor of this. This
> should have been committed as part of the original patch, really.

Maybe he wasn't against the whole thing, but he's posted two messages
to this thread and they can't be read as unequivocally in favor of
these changes.  He clearly didn't like at least some of it.

> I hope to avoid needless confusion about the documented (by the
> official documentation) AM interface. Yes, that is

Something maybe got cut off here?

>> I think it's a shame that we haven't gotten this patch dealt with just
>> because when somebody submits a patch in June, it's not very nice for
>> it to still be pending in December, but since this stuff is even
>> further outside my area of expertise than the sorting stuff, and since
>> me and my split personalities only have so many hours in the day, I'm
>> going to have to leave it to somebody else to pick up anyhow.  But
>> that's a separate issue from whether this should be back-patched.
>
> Note that I've already proposed a compromise, even though I don't
> think my original position was at all unreasonable. There'd be zero
> real changes (only the addition of the new constant name,
> documentation updates, comment updates, etc) under that compromise (as
> against one change.).

I only see one patch version on the thread.

-- 
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] Using quicksort for every external sort run

[Peter Geoghegan]

On Fri, Dec 18, 2015 at 12:50 PM, Robert Haas  wrote:
>> BTW, I'm not necessarily determined to make the new special-purpose
>> allocator work exactly as proposed. It seemed useful to prioritize
>> simplicity, and currently so there is one big "huge palloc()" with
>> which we blow our memory budget, and that's it. However, I could
>> probably be more clever about "freeing ranges" initially preserved for
>> a now-exhausted tape. That kind of thing.
>
> What about the case where we think that there will be a lot of data
> and have a lot of work_mem available, but then the user sends us 4
> rows because of some mis-estimation?

The memory patch only changes the final on-the-fly merge phase. There
is no estimate involved there.

I continue to use whatever "slots" (memtuples) are available for the
final on-the-fly merge. However, I allocate all remaining memory that
I have budget for at once. My remarks about the efficient use of that
memory was only really about each tape's use of their part of that
over time.

Again, to emphasize, this is only for the final on-the-fly merge phase.

>> With the on-the-fly merge memory patch, I'm improving locality of
>> access (for each "tuple proper"/"tuple itself"). If I also happen to
>> improve the situation around palloc() fragmentation at the same time,
>> then so much the better, but that's clearly secondary.
>
> I don't really understand this comment.

I just mean that I wrote the memory patch with memory locality in
mind, not palloc() fragmentation or other overhead.

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

[HACKERS] [sqlsmith] Failing assertions in spgtextproc.c

[Andreas Seltenreich]

I do see two assertions in spgtextproc.c fail on occasion when testing
with sqlsmith:

TRAP: FailedAssertion([...], File: "spgtextproc.c", Line: 424)
TRAP: FailedAssertion([...], File: "spgtextproc.c", Line: 564)

I can't reproduce it reliably but looking at the coredumps, the failing
part of the expression is always

in->level == 0 && DatumGetPointer(in->reconstructedValue) == NULL

In all of the dumps I looked at, in->reconstructedValue contains a
zero-length text instead of the asserted NULL, and the tuples fed to
leaf_consistent()/inner_consistent() look like the one below.

,
| (gdb) p *in
| $1 = {scankeys = 0x60a3ee0, nkeys = 1, reconstructedValue = 101373680, level 
= 0, 
|   returnData = 1 '\001', allTheSame = 1 '\001', hasPrefix = 0 '\000', 
prefixDatum = 0, nNodes = 8, 
|   nodeLabels = 0x37b6768}
| (gdb) x ((text *)in->reconstructedValue)->vl_len_
| 0x60ad6f0:0x0010
| (gdb) p *(text *)in->scankeys[0]->sk_argument
| $2 = {vl_len_ = "0\000\000", vl_dat = 0x855950c "sqlsmith~", '\177' , "\020 "}
| (gdb) p in->nodeLabels[0]
| $3 = 65535
`

Maybe these assertions are just too strict?  I don't see the code
misbehaving when relaxing them to

reconstrValue != NULL && VARSIZE_ANY_EXHDR(reconstrValue) == in->level
  || in->level == 0

regards,
Andreas


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

Re: [HACKERS] Refactoring speculative insertion with unique indexes a little

[Peter Geoghegan]

On Fri, Dec 18, 2015 at 12:55 PM, Robert Haas  wrote:
> On Fri, Dec 18, 2015 at 2:04 PM, Peter Geoghegan  wrote:
>> It isn't true that Heikki was not basically in favor of this. This
>> should have been committed as part of the original patch, really.
>
> Maybe he wasn't against the whole thing, but he's posted two messages
> to this thread and they can't be read as unequivocally in favor of
> these changes.  He clearly didn't like at least some of it.

The issues were very trivial.

> I only see one patch version on the thread.

I'm not going to post a revision until I thrash out the tiny issues
with Heikki. He kind of trailed off. So maybe that kills it
immediately, which is a shame.

-- 
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] extend pgbench expressions with functions

[Fabien COELHO]

Hello Michael,

It was definitely useful to debug the double/int type stuff within 
expressions when writing a non trivial pgbench script. It is probably less 
interesting if there are only integers.


After looking again at the code, I remembered why double are useful: there 
are needed for random exponential & gaussian because the last parameter is 
a double.


I do not care about the sqrt, but double must be allowed to keep that, and 
the randoms are definitely useful for a pgbench script. Now the patch may 
just keep double constants, but it would look awkward, and the doc must 
explain why 1.3 and 1+2 are okay, but not 1.3 + 2.4.


So I'm less keen at removing double expressions, because it removes a key 
feature. If it is a blocker I'll go for just the constant, but this looks 
to me like a stupid 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] pg_tables bug?

[Gaetano Mendola]

>From documentation about "CREATE DATABASE name WITH TABLESAPCE =
tablespace_name":

tablespace_name
The name of the tablespace that will be associated with the new database,
or DEFAULT to
use the template database's tablespace. This tablespace will be the default
tablespace used
for objects created in this database. See CREATE TABLESPACE for more
information.

I'm sure that my tables are created in the name space but those are not
reported either in
pg_tables, either in pg_dump or by \d.

Look as this:

kalman@kalman-VirtualBox:~$ mkdir tablespace_XXX
kalman@kalman-VirtualBox:~$ sudo chown postgres.postgres tablespace_XXX
kalman@kalman-VirtualBox:~$ psql template1
psql (9.4.5)
Type "help" for help.

template1=# create tablespace XXX LOCATION '/home/kalman/tablespace_XXX';
CREATE TABLESPACE
template1=# create database db_test with tablespace = XXX;
CREATE DATABASE
template1=# \q

kalman@kalman-VirtualBox:~$ psql db_test
psql (9.4.5)
Type "help" for help.

db_test=# create table t_test ( a integer, b numeric);
CREATE TABLE
db_test=# \d+ t_test
Table "public.t_test"
 Column |  Type   | Modifiers | Storage | Stats target | Description
+-+---+-+--+-
 a  | integer |   | plain   |  |
 b  | numeric |   | main|  |

db_test=# select * from pg_tables where tablename = 't_test';
 schemaname | tablename | tableowner | tablespace | hasindexes | hasrules |
hastriggers
+---++++--+-
 public | t_test| kalman || f  | f|
f
(1 row)

db_test=# select oid from pg_database where datname = 'db_test';
  oid
---
 80335

db_test=# select relfilenode from pg_class where relname = 't_test';
 relfilenode
-
   80336
(1 row)

Unfortunately contrary to what postgres is showing me the table test is in
/home/kalman/tablespace_:

root@kalman-VirtualBox:~# file
/home/kalman/tablespace_XXX/PG_9.4_201409291/80335/80336
/home/kalman/tablespace_XXX/PG_9.4_201409291/80335/80336: empty

as you can see the CREATE DATABASE documentation is honored but the system
is failing to give me the right tablespace location for that table.


Regards





On Thu, 17 Dec 2015 at 15:36 Tom Lane  wrote:

> Gaetano Mendola  writes:
> > I'm playing around with tablespace (postgresq 9.4) and I found out what I
> > believe is a bug in pg_tables.
> > Basically if you create a database in a table space X and then you
> create a
> > table on the database the table is created correctly on the tablespace X
> (
> > I did a check on the filesystem) however if you do a select on pg_tables
> > the column tablespace for that table is empty and even worst if you dump
> > the DB there is no reporting about the the database or table being on
> that
> > tablespace.
> > Even \d doesn't report that the table is in the tablespace X.
>
> An empty entry in that column means that the table is in the default
> tablespace for the database.  Which it sounds like is what you have
> here.  I think it's operating as designed, though you might quibble
> with the decision that showing default tablespaces explicitly would
> have been clutter.
>
> regards, tom lane
>

Re: [HACKERS] pg_tables bug?

[Gaetano Mendola]

On Thu, 17 Dec 2015 at 15:36 Tom Lane  wrote:

> Gaetano Mendola  writes:
> > I'm playing around with tablespace (postgresq 9.4) and I found out what I
> > believe is a bug in pg_tables.
> > Basically if you create a database in a table space X and then you
> create a
> > table on the database the table is created correctly on the tablespace X
> (
> > I did a check on the filesystem) however if you do a select on pg_tables
> > the column tablespace for that table is empty and even worst if you dump
> > the DB there is no reporting about the the database or table being on
> that
> > tablespace.
> > Even \d doesn't report that the table is in the tablespace X.
>
> An empty entry in that column means that the table is in the default
> tablespace for the database.  Which it sounds like is what you have
> here.  I think it's operating as designed, though you might quibble
> with the decision that showing default tablespaces explicitly would
> have been clutter.
>

Now it's clear thank you.

Re: [HACKERS] [sqlsmith] Failing assertions in spgtextproc.c

[Peter Geoghegan]

On Fri, Dec 18, 2015 at 1:23 PM, Andreas Seltenreich  wrote:
> I do see two assertions in spgtextproc.c fail on occasion when testing
> with sqlsmith:
>
> TRAP: FailedAssertion([...], File: "spgtextproc.c", Line: 424)
> TRAP: FailedAssertion([...], File: "spgtextproc.c", Line: 564)
>
> I can't reproduce it reliably but looking at the coredumps, the failing
> part of the expression is always
>
> in->level == 0 && DatumGetPointer(in->reconstructedValue) == NULL
>
> In all of the dumps I looked at, in->reconstructedValue contains a
> zero-length text instead of the asserted NULL, and the tuples fed to
> leaf_consistent()/inner_consistent() look like the one below.

Can you do this?:

(gdb) p debug_query_string

It's a global variable, often useful in these situations.

-- 
Peter Geoghegan


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

Re: [HACKERS] pg_tables bug?

[Andrew Dunstan]

On 12/18/2015 05:18 PM, Gaetano Mendola wrote:
From documentation about "CREATE DATABASE name WITH TABLESAPCE = 
tablespace_name":


tablespace_name
The name of the tablespace that will be associated with the new 
database, or DEFAULT to
use the template database's tablespace. This tablespace will be the 
default tablespace used
for objects created in this database. See CREATE TABLESPACE for more 
information.


I'm sure that my tables are created in the name space but those are 
not reported either in

pg_tables, either in pg_dump or by \d.


1. Please don't top-post on the PostgreSQL lists. See 



2. The system is working as designed and as documented - see the 
comments in the docs on pg_tables. If nothing is shown for the table's 
tablespace then it will be in the default tablespace for the database. 
That's what you're seeing. You appear to be assuming incorrectly that it 
means that the table will be in the system's default tablespace.



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] Remove array_nulls?

[Jim Nasby]

On 12/18/15 11:44 AM, Tom Lane wrote:

"David G. Johnston"  writes:

>On Fri, Dec 18, 2015 at 10:25 AM, Tom Lane  wrote:

>>Maybe I shouldn't put words in Andres' mouth, but I don't think that by
>>"indefinitely" he meant "forever".  I read that more as "until some
>>positive reason to remove it arrives".  I could imagine that at some point
>>we decide to do a wholesale cleanup of backwards-compatibility GUCs, and
>>then we'd zap this one along with others.

>​Hand-waving from me but I see a "positive reason" being that someone wants
>to write and commit a patch that does not play nicely with the old
>behavior.

Sure, that's also possible.  But no such patch is on the table now.


Someone (Tom?) mentioned upthread that if we wanted to do cleanup it 
should be more than just one GUC, and I agree with that, and I'm willing 
to investigate when the current compat GUCs went in and create a patch 
to remove the really old ones. My inclination would be to just do this 
as part of 10.0. (And I agree with Robert's comments about parallel 
being the most likely driver for bumping to 10.0).

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com


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

Re: [HACKERS] [patch] Proposal for \rotate in psql

[Pavel Stehule]

2015-12-18 21:21 GMT+01:00 Daniel Verite :

> Pavel Stehule wrote:
>
> > The symbol 'X' in two column mode should be centred - now it is aligned
> to
> > left, what is not nice
>
> Currently print.c does not support centered alignment, only left and right.
> Should we add it, it would have to work for all output formats
> (except obviously for "unaligned"):
> - aligned
> - wrapped
> - html
> - latex
> - latex-longtable
> - troff-ms
> - asciidoc
>
> Because of this, I believe that adding support for a 'c' alignment
> might be a significant patch by itself, and that it should be considered
> separately.
>

ok


>
> I agree that if it existed, the crosstabview command should use it
> as you mention, but I'm not volunteering to implement it myself, at
> least not in the short term.
>

I'll look how much work it is

Regards

Pavel


>
> Best regards,
> --
> Daniel Vérité
> PostgreSQL-powered mailer: http://www.manitou-mail.org
> Twitter: @DanielVerite
>

Re: [HACKERS] [sqlsmith] Failing assertions in spgtextproc.c

[Andreas Seltenreich]

Peter Geoghegan writes:

> Can you do this?:
>
> (gdb) p debug_query_string

output below.  Since sqlsmith ist no longer restricted to read-only
statements, the chances for reproduction are low :-/.

select
  pg_catalog.pg_stat_get_buf_written_backend() as c0,
  subq_1.c0 as c1,
  subq_1.c0 as c2,
  subq_1.c0 as c3
from
  (select
(select ordinal_position from information_schema.parameters limit 1 
offset 12)
 as c0,
ref_2.t as c1
  from
public.radix_text_tbl as ref_2
  inner join pg_catalog.pg_stat_activity as ref_3
  on (ref_2.t = ref_3.application_name )
  where ref_2.t @@ cast(coalesce(ref_2.t, ref_3.client_hostname) as text)
  limit 111) as subq_1,
  lateral (select
subq_1.c0 as c0,
subq_2.c2 as c1,
56 as c2,
cast(coalesce(cast(coalesce((select pop from public.real_city limit 1 
offset 34)
, subq_1.c0) as integer), subq_2.c0) as integer) as c3,
74 as c4,
(select unique1 from public.onek2 limit 1 offset 17)
 as c5
  from
(select
  (select ordinal_position from information_schema.parameters limit 
1 offset 27)
 as c0,
  sample_2.umoptions as c1,
  sample_2.umserver as c2
from
  pg_catalog.pg_user_mapping as sample_2 tablesample system (6.2)
where 49 is NULL) as subq_2
  where cast(coalesce(subq_1.c0, subq_2.c0) as integer) is not NULL
  limit 105) as subq_3
where ((select x from public.tt0 limit 1 offset 12)
 <= subq_3.c0)
  and (subq_3.c4 <= 30);


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