Re: [HACKERS] Walsender timeouts and large transactions

[Kyotaro HORIGUCHI]

Hello Sokolov.

At Fri, 29 Sep 2017 15:19:23 +0300, Sokolov Yura  
wrote in 
> I don't want to make test to lasts so long and generate so many data.
> That is why I used such small timeouts for tests.

I understand your point, but still *I* think such a short timeout
is out of expectation by design. (But it can be set.)

Does anyone have opinions on this?

> Test is failing if there is "short quit" after
> `!pq_is_send_pending()`,
> so I doubt your patch will pass the test.

It is because I think that the test "should" fail since the
timeout is out of expected range. I (and perhaps also Petr) is
thinking that the problem is just that a large transaction causes
a timeout with an ordinary timeout. My test case is based on the
assumption.

Your test is for a timeout during replication-startup with
extremely short timeout. This may be a different problem to
discuss, but perhaps better to be solved together.

I'd like to have opnions from others on this point.

> And you've change calculated sleep time with sane waiting on all
> insteresting events (using WaitLatchOrSocket) to semi-busy loop.
> It at least could affect throughput.

Uggh! I misunderstood there. It wais for writing socket so the
sleep is wrong and WaitLatchOrSocket is right.

After all, I put +1 for Petr's latest patch. Sorry for my
carelessness.

> And why did you remove `SetLatch(MyLatch)` in the end of function?
> Probably this change is correct, but not obvious.

Latch is needless there if it waited a fixed duration, but if it
waits writefd events there, also latch should be waited.


> > Any thoughts?
> 
> It certainly could be my test and my patch is wrong. But my point
> is that test should be written first. Especially for such difficult
> case. Without test it is impossible to say does our patches fix
> something. And it is impossible to say if patch does something
> wrong. And impossible to say if patch fixes this problem but
> introduce new problem.
> 
> Please, write test for your remarks. If you think, my patch breaks
> something, write test for the case my patch did broke. If you think
> my test is wrong, write your test that is more correct.
> 
> Without tests it will be just bird's hubbub.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



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

Re: [HACKERS] Logging idle checkpoints

[Andres Freund]

On 2017-10-02 07:43:31 +0900, Michael Paquier wrote:
> On Mon, Oct 2, 2017 at 7:41 AM, Andres Freund  wrote:
> > On 2017-10-02 07:39:18 +0900, Michael Paquier wrote:
> >> On Mon, Oct 2, 2017 at 7:27 AM, Andres Freund  wrote:
> >> > On 2017-10-02 00:19:33 +0200, Vik Fearing wrote:
> >> > I'd be ok with applying this now, or in 10.1 - but I do think we should
> >> > fix this before 11.  If nobody protests I'll push later today, so we can
> >> > get some bf cycles for the very remote case that this causes problems.
> >>
> >> This point has been discussed during review and removed from the patch
> >> (adding Stephen in the loop here):
> >> https://www.postgresql.org/message-id/CAOuzzgq8pHneMHy6JiNiG6Xm5V=cm+k2wcd2w-sctpjdg7x...@mail.gmail.com
> >
> > I find that reasoning unconvincing. log_checkpoints is enabled after
> > all. And we're not talking about 10 log messages a second. There's
> > plenty systems that analyze the logs that'd possibly be affected by
> > this.
> 
> No real objections from here, actually.

Vik, because there was some, even though mild, objections, I'd rather
not push this right now. Stephen deserves a chance to reply.  So this'll
have to wait for 10.1, sorry :(

- 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] [PATCH] Off-by-one error in logical slot resource retention

[Craig Ringer]

On 2 October 2017 at 05:27, Daniel Gustafsson  wrote:

> > On 15 Sep 2017, at 13:26, Daniel Gustafsson  wrote:
> >
> >> On 01 Sep 2017, at 14:28, Aleksander Alekseev <
> a.aleks...@postgrespro.ru> wrote:
> >>
> >> The following review has been posted through the commitfest application:
> >> make installcheck-world:  not tested
> >> Implements feature:   not tested
> >> Spec compliant:   not tested
> >> Documentation:not tested
> >>
> >> Hi Craig,
> >>
> >> I'm afraid patches 0002 and 0003 don't apply anymore. Could you please
> resolve the conflicts?
> >>
> >> The new status of this patch is: Waiting on Author
> >
> > Have you had a chance to look at rebasing this patch so we can get it
> further
> > in the process?
>
> Since this patch has been in Waiting for Author state for the duration of
> the
> commitfest without moving, I’m marking it Returned with Feedback.  If
> there is
> still interest in pursuing this patch, please re-submit it to the next
> commitfest with the comments addressed.
>

Thanks. I'll revisit it next CF.

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

Re: [HACKERS] pg_prepared_xact_status

[Craig Ringer]

On 30 September 2017 at 14:10, konstantin knizhnik <
k.knizh...@postgrespro.ru> wrote:


> So I do not see any troubles caused by adding this functions. And it can
> really be helpful for DBA in some cases.
>

If it's self-contained and exposes existing functionality, then I'm not
opposed, I just don't really see the point. I'm not seeing where it'd come
in useful.

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

Re: [HACKERS] pg_prepared_xact_status

[Craig Ringer]

On 2 October 2017 at 08:09, Robert Haas  wrote:

> On Sat, Sep 30, 2017 at 2:10 AM, konstantin knizhnik
>  wrote:
> > txid_status() also not always be able to return status of transaction
> (if wraparound happen).
> > But it is still considered as one of the key features of 10 (transaction
> traceability...).
>
> Not by me.  It's a feature, though, for sure.


Same here. It's nice, and obviously I wanted it since I submitted it, but
it's not a key feature by any stretch.

Even if Pg also reported the xid to the client when assigned it'd still be
a nice utility, not a huge headline feature.


> It's also a LOT more
> stable than what you're proposing.  Even on a busy system, it takes a
> while to go through 200 million transactions; you probably can't
> realistically do that in under an hour, and you'll probably raise the
> 200 million transaction limit if you're anywhere close to that rate.
> In practice, you'll almost always be able to look up transactions for
> several days, and often weeks or months.


Not necessarily, since it doesn't hold down or delay clog truncation, so if
the system is really eagerly VACUUMed it might discard things sooner.

At the moment though we're quite lazy about clog truncation, mainly because
we're very lazy about vacuuming template1 and template0 (which we should
autofreeze, really) so they tend to hold down global datfrozenxid. If we
get better about that, then we might need some way to ask Pg to keep extra
clog. But for now it works well enough.

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

Re: [HACKERS] Bug with pg_basebackup and 'shared' tablespace

[Kyotaro HORIGUCHI]

Thanks for the objection with clear reasoning.

For clarity, I first proposed to prohibit servers of different
versions from sharing same tablespace directory.

https://www.postgresql.org/message-id/20170406.160844.120459562.horiguchi.kyot...@lab.ntt.co.jp

And I had -1 that it is just a reverting to the previous behavior
(it was exactly the patch intended, though) and persuaded to take
the way in this thread there, so I'm here.

At Fri, 29 Sep 2017 13:43:22 -0400, Robert Haas  wrote 
in 
> On Fri, Sep 29, 2017 at 2:06 AM, Michael Paquier
>  wrote:
> > My tendency about this patch is still that it should be rejected. This
> > is presenting additional handling for no real gain.
> 
> I vehemently disagree.  If the server lets you create a tablespace,
> then everything that happens after that ought to work.
> 
> On another thread, there is the issue that if you create a tablespace
> inside $PGDATA, things break.  We should either unbreak those things

pg_basebackup copies the tablespace twice, or some maintenaince
commands give a wrong result, or careless cleanup script can blow
away a part of the data.

> or not allow creating the tablespace in the first place.  On this
> thread, there is the issue that if you create two tablespaces for
> different PG versions in the same directory, things break.  We should

Server never accesses out of /CARVER/ directory in the
 and servers with different versoins can share the
 directory (I think this is a bug). pg_upgrade will
complain if it finds the destination CATVER directory created
even though no data will be broken.

Just a clarification, not "fixing" the problem, users may get
punished by pg_basebackup later. If "fixing" in this way,
pg_basebacup will work in the case but in turn pg_upgrade may
punish them later. May I assume that we agree on this point?

> either unbreak those things or not allow creating the tablespace in
> the first place.
> 
> It is completely awful behavior to let users do things and then punish
> them later for having done them.  Users are not obliged to read the
> minds of the developers and guess what things the developers consider
> "reasonable".  They should be able to count on the principle that if
> they do something that we consider wrong, they'll get an error when
> they try to do it -- not have it superficially appear to work and then
> explode later.
> 
> To put that another way, there should be ONE rule about what is or is
> not allowable in a particular situation, and all commands, utilities,
> etc. that deal with that situation should handle it in a uniform
> fashion.  Each .c file shouldn't get to make up its own notion of what
> is or is not supported.

Anyway currently server and pg_basebackup disagrees on the
point. If the "just reverting" patch above is not rejected again,
I'll resume working on it. Or other way to go? This is not an
issue that ought to take a long time.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



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

Re: [HACKERS] path toward faster partition pruning

[Amit Langote]

On 2017/09/30 1:28, Robert Haas wrote:
> On Thu, Sep 28, 2017 at 5:16 AM, David Rowley
>  wrote:
>> I'd imagine, for
>> each partition key, you'd want to store a Datum with the minimum and
>> maximum possible value based on the quals processed. If either the
>> minimum or maximum is still set to NULL, then it's unbounded at that
>> end. If you encounter partkey = Const, then minimum and maximum can be
>> set to the value of that Const. From there you can likely ignore any
>> other quals for that partition key, as if the query did contain
>> another qual with partkey = SomeOtherConst, then that would have
>> become a gating qual during the constant folding process. This way if
>> the user had written WHERE partkey >= 1 AND partkey <= 1 the
>> evaluation would end up the same as if they'd written WHERE partkey =
>> 1 as the minimum and maximum would be the same value in both cases,
>> and when those two values are the same then it would mean just one
>> theoretical binary search on a partition range to find the correct
>> partition instead of two.
> 
> I have not looked at the code submitted here in detail yet but I do
> think we should try to avoid wasting cycles in the
> presumably-quite-common case where equality is being tested.  The
> whole idea of thinking of this as minimum/maximum seems like it might
> be off precisely for that reason.

I agree.  Equality checks are going to be common enough to warrant them to
be handled specially, instead of implementing equality-pruning on top of
min/max framework.

Thanks,
Amit



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

Re: [HACKERS] [WIP] Zipfian distribution in pgbench

[Daniel Gustafsson]

> On 06 Sep 2017, at 08:42, Fabien COELHO  wrote:
> 
> Hello Alik,
> 
> Applies, compiles, works for me.
> 
> Some minor comments and suggestions.
> 
> Two typos:
> - "usinng" -> "using"
> - "a rejection method used" -> "a rejection method is used"
> 
> I'm not sure of "least_recently_used_i", this naming style is not used in 
> pgbench. "least_recently_used" would be ok.
> 
> "..nb_cells.. != ZIPF_CACHE_SIZE", ISTM that "<" is more logical,
> even if the result is the same?
> 
> I would put the parameter value check in getZipfianRand, so that if someone 
> reuse the function elsewhere the check is also performed. That would also 
> simplify a bit the already very large expression evaluation function.
> 
> When/if the pgbench tap test patch get through, coverage tests should
> be added.
> 
> Maybe the cache overflow could be counted, to allow for a possible warning 
> message in the final report?

Since this patch has been Waiting for author and no update on this patch has
been posted during the commitfest, it is Returned with feedback.  When you have
a new version of the patch, please re-submit to a future commitfest.

cheers ./daniel

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

Re: [HACKERS] Small patch for pg_basebackup argument parsing

[Daniel Gustafsson]

> On 18 Sep 2017, at 23:18, Pierre Ducroquet  wrote:
> 
> On Monday, September 18, 2017 5:13:38 PM CEST Tom Lane wrote:
>> Ryan Murphy  writes:
>>> Looked thru the diffs and it looks fine to me.
>>> Changing a lot of a integer/long arguments that were being read directly
>>> via atoi / atol to be read as strings first, to give better error
>>> handling.
>>> 
>>> This looks good to go to me.  Reviewing this as "Ready for Committer".
>>> Thanks for the patch, Pierre!
>> 
>> I took a quick look through this and had a few thoughts:
> 
> I agree with your suggestions, I will try to produce a new patch before the 
> end of the week.

This patch has been marked as Returned with Feedback as no new version has been
submitted.  Please re-submit the new version of this patch to the next commit-
fest when it’s ready.

cheers ./daniel

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

Re: [HACKERS] Optional message to user when terminating/cancelling backend

[Daniel Gustafsson]

> On 28 Sep 2017, at 14:55, Yugo Nagata  wrote:
> 
> On Sun, 3 Sep 2017 22:47:10 +0200
> Daniel Gustafsson  wrote:
> 
> I have reviewed your latest patch. 
> 
> I can apply this to the master branch and build this successfully,
> and the behavior is as expected. 
> 
> However, here are some comments and suggestions.

Thanks for the review!  As I haven’t had time to address the comments for a new
versin of this patch, I’m closing this as Returned with Feedback and will
re-submit for the next commitfest.

cheers ./daniel

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

Re: [HACKERS] CLUSTER command progress monitor

[Daniel Gustafsson]

> On 12 Sep 2017, at 14:57, Tatsuro Yamada  wrote:
> 
> On 2017/09/12 21:20, Tatsuro Yamada wrote:
>> On 2017/09/11 23:38, Robert Haas wrote:
>>> On Sun, Sep 10, 2017 at 10:36 PM, Tatsuro Yamada
>>>  wrote:
 Thanks for the comment.
 
 As you know, CLUSTER command uses SEQ SCAN or INDEX SCAN as a scan method 
 by
 cost estimation. In the case of SEQ SCAN, these two phases not overlap.
 However, in INDEX SCAN, it overlaps. Therefore I created the phase of "scan
 heap and write new heap" when INDEX SCAN was selected.
 
 I agree that progress reporting for sort is difficult. So it only reports
 the phase ("sorting tuples") in the current design of progress monitor of
 cluster.
 It doesn't report counter of sort.
>>> 
>>> Doesn't that make it almost useless?  I would guess that scanning the
>>> heap and writing the new heap would ordinarily account for most of the
>>> runtime, or at least enough that you're going to want something more
>>> than just knowing that's the phase you're in.
>> 
>> Hmmm, Should I add a counter in tuplesort.c? (tuplesort_performsort())
>> I know that external merge sort takes a time than quick sort.
>> I'll try investigating how to get a counter from external merge sort 
>> processing.
>> Is this the right way?
>> 
>> 
> The patch is getting the value reported as heap_tuples_total from
> OldHeap->rd_rel->reltuples.  I think this is pointless: the user can
> see that value anyway if they wish.  The point of the progress
> counters is to expose things the user couldn't otherwise see.  It's
> also not necessarily accurate: it's only an estimate in the best case,
> and may be way off if the relation has recently be extended by a large
> amount.  I think it's pretty important that we try hard to only report
> values that are known to be accurate, because users hate (and mock)
> inaccurate progress reports.
 
 Do you mean to use the number of rows by using below calculation instead
 OldHeap->rd_rel->reltuples?
 
  estimate rows = physical table size / average row length
>>> 
>>> No, I mean don't report it at all.  The caller can do that calculation
>>> if they wish, without any help from the progress reporting machinery.
>> 
>> I see. I'll remove that column on next patch.
> 
> 
> I will summarize the current design and future corrections before sending
> the next patch.
> 
> 
> === Current design ===
> 
> CLUSTER command may use Index Scan or Seq Scan when scanning the heap.
> Depending on which one is chosen, the command will proceed in the
> following sequence of phases:
> 
>  * Scan method: Seq Scan
>1. scanning heap(*1)
>2. sorting tuples   (*2)
>3. writing new heap (*1)
>5. swapping relation files  (*2)
>6. rebuilding index (*2)
>7. performing final cleanup (*2)
> 
>  * Scan method: Index Scan
>4. scan heap and write new heap (*1)
>5. swapping relation files  (*2)
>6. rebuilding index (*2)
>7. performing final cleanup (*2)
> 
> VACUUM FULL command will proceed in the following sequence of phases:
> 
>1. scanning heap(*1)
>5. swapping relation files  (*2)
>6. rebuilding index (*2)
>7. performing final cleanup (*2)
> 
> (*1): increasing the value in heap_tuples_scanned column
> (*2): only shows the phase in the phase column
> 
> The view provides the information of CLUSTER command progress details as 
> follows
> # \d pg_stat_progress_cluster
>  View "pg_catalog.pg_stat_progress_cluster"
>  Column   |  Type   | Collation | Nullable | Default
> ---+-+---+--+-
> pid   | integer |   |  |
> datid | oid |   |  |
> datname   | name|   |  |
> relid | oid |   |  |
> command   | text|   |  |
> phase | text|   |  |
> scan_method   | text|   |  |
> scan_index_relid  | bigint  |   |  |
> heap_tuples_total | bigint  |   |  |
> heap_tuples_scanned   | bigint  |   |  |
> heap_tuples_vacuumed  | bigint  |   |  |
> heap_tuples_recently_dead | bigint  |   |  |
> 
> 
> === It will be changed on next patch ===
> 
> - Rename to pg_stat_progress_reolg from pg_stat_progress_cluster
> - Remove heap_tuples_total column from the view
> - Add a progress counter in the phase of "sorting tuples" (difficult?!)
> 
> 
> === My test case as a bonus ===
> 
> I share my test case of progress monitor.
> If someone wants to watch the current progress 

Re: [HACKERS] Fwd: Have a problem with citext

[Robert Haas]

On Fri, Sep 29, 2017 at 6:16 PM, David E. Wheeler  wrote:
> Are permissions correct in the citext extension?

Not to be picky, but couldn't you investigate that a bit before posting here?

Off-hand, I see no GRANT or REVOKE statements in any of the citext SQL
files, so I'm going to guess that this is an artifact of some general
PostgreSQL behavior rather than anything specific to citext.

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


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

Re: [HACKERS] pg_prepared_xact_status

[Michael Paquier]

On Mon, Oct 2, 2017 at 9:09 AM, Robert Haas  wrote:
> On Sat, Sep 30, 2017 at 2:10 AM, konstantin knizhnik
>  wrote:
>> txid_status() also not always be able to return status of transaction (if 
>> wraparound happen).
>> But it is still considered as one of the key features of 10 (transaction 
>> traceability...).
>
> Not by me.  It's a feature, though, for sure.  It's also a LOT more
> stable than what you're proposing.  Even on a busy system, it takes a
> while to go through 200 million transactions; you probably can't
> realistically do that in under an hour, and you'll probably raise the
> 200 million transaction limit if you're anywhere close to that rate.
> In practice, you'll almost always be able to look up transactions for
> several days, and often weeks or months.  With what you're proposing
> here, the information could disappear nearly instantly.  I can't see
> how that works out to a usable feature.

Also txid_status is way more performant as it requires only a clog
lookup. Going through potentially hundreds of WAL segments to get one
result status could take way longer than that. Even if network can
become easily the bottleneck when doing cross-node transaction
resolution, this would put too much load into the backends.
-- 
Michael


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

Re: [HACKERS] pg_prepared_xact_status

[Robert Haas]

On Sat, Sep 30, 2017 at 2:10 AM, konstantin knizhnik
 wrote:
> txid_status() also not always be able to return status of transaction (if 
> wraparound happen).
> But it is still considered as one of the key features of 10 (transaction 
> traceability...).

Not by me.  It's a feature, though, for sure.  It's also a LOT more
stable than what you're proposing.  Even on a busy system, it takes a
while to go through 200 million transactions; you probably can't
realistically do that in under an hour, and you'll probably raise the
200 million transaction limit if you're anywhere close to that rate.
In practice, you'll almost always be able to look up transactions for
several days, and often weeks or months.  With what you're proposing
here, the information could disappear nearly instantly.  I can't see
how that works out to a usable feature.

-- 
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] Push down more UPDATEs/DELETEs in postgres_fdw

[Daniel Gustafsson]

> On 08 Apr 2017, at 16:14, David Steele  wrote:
> 
> On 3/22/17 6:20 AM, Etsuro Fujita wrote:
>> On 2017/02/22 19:57, Rushabh Lathia wrote:
>>> Marked this as Ready for Committer.
>> 
>> I noticed that this item in the CF app was incorrectly marked as
>> Committed.  This patch isn't committed, so I returned it to the previous
>> status.  I also rebased the patch.  Attached is a new version of the patch.
> 
> This submission has been moved to CF 2017-07.

This patch has been marked Ready for Committer in the current commitfest
without being committed or rejected.  Moving to the next commitfest, but since
it has bitrotted I’m moving it to Waiting for author.

cheers ./daniel

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

Re: [HACKERS] postgres_fdw: support parameterized foreign joins

[Daniel Gustafsson]

> On 11 Apr 2017, at 10:55, Etsuro Fujita  wrote:
> 
> On 2017/04/07 22:54, Arthur Zakirov wrote:
>> Marked the patch as "Ready for Commiter". But the patch should be
>> commited only after the patch [1].
> 
> Thanks for reviewing!  I'll continue to work on this for PG11.

This patch has been marked Ready for Committer in the current commitfest
without being committed or rejected.  Moving to the next commitfest, but since
it has bitrotted I’m moving it to Waiting for author.

cheers ./daniel

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

Re: [HACKERS] issue: record or row variable cannot be part of multiple-item INTO list

[Daniel Gustafsson]

> On 20 Sep 2017, at 01:05, David G. Johnston  
> wrote:
> 
> On Tue, Sep 19, 2017 at 11:29 AM, Tom Lane  > wrote:
> ​T​hat​ ​doesn't work today, and this patch doesn't fix it, but it does create
> enough confusion that we never would be able to fix it.
> 
> I'd be much happier if there were some notational difference
> between I-want-the-composite-variable-to-absorb-a-composite-column
> and I-want-the-composite-variable-to-absorb-N-scalar-columns.
> For backwards compatibility with what happens now, the latter would
> have to be the default.
> 
> ​So, using "()" syntax​
> 
> s,t: scalar text
> c,d: (text, text)
> 
> treat all numbers below as text; and the ((1,2),) as ("(1,2)",)
> 
> A. SELECT 1 INTO s; -- today 1, this patch is the same
> 
> B. SELECT 1, 2 INTO s; -- today 1, this patch is the same
> 
> C. SELECT 1, 2 INTO (s); -- ERROR syntax - scalars cannot be tagged with (), 
> this patch N/A
>  
> D. SELECT 1, 2 INTO s, t; -- today 1, 2, this patch is the same
> 
> E. SELECT 1, 2 INTO c; -- today (1,2), this patch is the same
> 
> F. SELECT 1, 2 INTO (c); --ERROR "1" cannot be converted to (text, text), 
> this patch N/A
> 
> 1. SELECT (1,2) INTO c; -- today ((1,2),); this patch is the same
> 
> 2. SELECT (1,2) INTO (c); -- (1,2) -- this patch N/A
> 
> 3. SELECT (1,2),(3,4) INTO c,d; -- ERROR syntax -- this patch gives [I 
> think...it can be made to give] (1,2),(3,4)
> 
> 4. SELECT (1,2),(3,4) INTO c,(d); -- ERROR syntax -- this patch N/A
> 
> 5. SELECT (1,2),(3,4) INTO (c),d; -- ERROR syntax -- this patch N/A
> 
> 6. SELECT (1,2),(3,4) INTO (c),(d); -- (1,2),(3,4) -- this patch N/A
> 
> !. SELECT 1, (2,3), 4 INTO s, (c), t -- 1, (2,3), 4 -- this patch N/A
> @. SELECT 1, 2, 3, 4 INTO s, (c), t -- ERROR "2" cannot be made into (text, 
> text) -- this patch N/A
> 
> IOW, this patch, if "c" is the only target (#1) and is composite pretend the 
> user wrote "INTO c.1, c.2" and assign each output column as a scalar in 
> one-by-one fashion.  If "c" is not the only target column (#3) assign a 
> single output column to it.  This maintains compatibility and clean syntax at 
> the cost of inconsistency.
> 
> The alternate, backward compatible, option introduces mandatory () in the 
> syntax for all composite columns in a multi-variable target (# 3-5 errors, #6 
> valid) while it changes the behavior if present on a single variable target 
> (#1 vs #2).
> 
> So, allowing #3 to work makes implementing #2 even more unappealing.  Making 
> only #2 and #6 work seems like a reasonable alternative position.
> 
> The last option is to fix #1 to return (1,2) - cleanly reporting an error if 
> possible, must like we just did with SRFs, and apply the patch thus gaining 
> both consistency and a clean syntax at the expense of limited backward 
> incompatibility.
> 
> Arrays not considered; single-column composites might end up looking like 
> scalars when presented to a (c) target.
> 
> Hope this helps someone besides me understand the problem space.

Based on the discussion in this thread I’ve moved this patch to the next CF
with the new status Waiting for author, as it seems to need a revised version.

cheers ./daniel



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

Re: [HACKERS] parallelize queries containing initplans

[Daniel Gustafsson]

> On 15 Sep 2017, at 04:45, Amit Kapila  wrote:
> 
> On Thu, Aug 31, 2017 at 11:23 AM, Amit Kapila  wrote:
>> On Mon, Aug 21, 2017 at 2:40 PM, Amit Kapila  wrote:
>>> On Mon, Aug 21, 2017 at 1:44 PM, Haribabu Kommi
>>>  wrote:
 
 
 Thanks for adding more details. It is easy to understand.
 
 I marked the patch as ready for committer in the commitfest.
>> 
>> Rebased the patch.  The output of test case added by the patch is also
>> slightly changed because of the recent commit
>> 7df2c1f8daeb361133ac8bdeaf59ceb0484e315a.  I have verified that the
>> new test result is as expected.
> 
> The latest patch again needs to be rebased.  Find rebased patch
> attached with this email.

Moved to next commitfest, but changed to Waiting for author since it no longer
applies cleanly.

cheers ./daniel

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

Re: [HACKERS] Vacuum: allow usage of more than 1GB of work mem

[Daniel Gustafsson]

> On 18 Aug 2017, at 13:39, Claudio Freire  wrote:
> 
> On Fri, Apr 7, 2017 at 10:51 PM, Claudio Freire  
> wrote:
>> Indeed they do, and that's what motivated this patch. But I'd need
>> TB-sized tables to set up something like that. I don't have the
>> hardware or time available to do that (vacuum on bloated TB-sized
>> tables can take days in my experience). Scale 4000 is as big as I can
>> get without running out of space for the tests in my test hardware.
>> 
>> If anybody else has the ability, I'd be thankful if they did test it
>> under those conditions, but I cannot. I think Anastasia's test is
>> closer to such a test, that's probably why it shows a bigger
>> improvement in total elapsed time.
>> 
>> Our production database could possibly be used, but it can take about
>> a week to clone it, upgrade it (it's 9.5 currently), and run the
>> relevant vacuum.
> 
> It looks like I won't be able to do that test with a production
> snapshot anytime soon.
> 
> Getting approval for the budget required to do that looks like it's
> going to take far longer than I thought.
> 
> Regardless of that, I think the patch can move forward. I'm still
> planning to do the test at some point, but this patch shouldn't block
> on it.

This patch has been marked Ready for committer after review, but wasn’t
committed in the current commitfest so it will be moved to the next.  Since it
no longer applies cleanly, it’s being reset to Waiting for author though.

cheers ./daniel

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

Re: [HACKERS] 64-bit queryId?

[Robert Haas]

On Sun, Oct 1, 2017 at 3:48 PM, Greg Stark  wrote:
> Well these kinds of monitoring systems tend to be used by operations
> people who are a lot more practical and a lot less worried about
> theoretical concerns like that.

+1, well said.

> In context the point was merely that the default
> pg_stat_statements.max of 5000 isn't sufficient to argue that 32-bit
> values are enough. It wouldn't be hard for there to be 64k different
> queries over time and across all the databases in a fleet and at that
> point it becomes likely there'll be a 32-bit collision.

Yeah.

I think Alexander Korotkov's points are quite good, too.

-- 
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] Crash on promotion when recovery.conf is renamed

[Daniel Gustafsson]

> On 02 Sep 2017, at 14:17, Michael Paquier  wrote:
> 
> On Fri, Sep 1, 2017 at 7:17 PM, Thomas Munro
>  wrote:
>> On Sat, Aug 26, 2017 at 7:32 AM, Robert Haas  wrote:
>>> On Sat, Apr 8, 2017 at 10:05 AM, David Steele  wrote:
 On 3/28/17 1:21 AM, Tsunakawa, Takayuki wrote:
> Thanks, marked as ready for committer, as the code is good and Alexander 
> reported the test success.
 
 This bug has been moved to CF 2017-07.
>>> 
>>> This bug fix has been pending in "Ready for Committer" state for about
>>> 4.5 months.  Three committers (Magnus, Heikki, Tom) have contributed
>>> to the thread to date.  Maybe one of them would like to commit this?
>> 
>> In the meantime its bits have begun to rot.  Michael, could you please 
>> rebase?
> 
> Thanks for the reminder, Thomas. The 2PC code triggered during
> recovery has been largely reworked in PG10, explaining the conflicts.
> As this has been some time since I touched this patch, I had again a
> look at its logic and could not find any problems around it. So
> attached are a rebased versions for both 0001 and 0002, I have updated
> the messages as well to be more in-line with what is in HEAD for
> corrupted entries.

I’ve moved this to the next CF, but since this no longer applies cleanly I’ve
reset it to Waiting for author.

cheers ./daniel

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

Re: [HACKERS] eval_const_expresisions and ScalarArrayOpExpr

[Tom Lane]

I wrote:
> This patch no longer applies cleanly on HEAD, so here's a rebased version
> (no substantive changes).  As before, I think the most useful review task
> would be to quantify whether it makes planning noticeably slower.

Rebased again (over the arrays-of-domains patch).

regards, tom lane

diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 7961362..c3fab75 100644
*** a/src/backend/optimizer/util/clauses.c
--- b/src/backend/optimizer/util/clauses.c
*** static List *find_nonnullable_vars_walke
*** 115,120 
--- 115,123 
  static bool is_strict_saop(ScalarArrayOpExpr *expr, bool falseOK);
  static Node *eval_const_expressions_mutator(Node *node,
  			   eval_const_expressions_context *context);
+ static bool contain_non_const_walker(Node *node, void *context);
+ static bool ece_function_is_safe(Oid funcid,
+ 	 eval_const_expressions_context *context);
  static List *simplify_or_arguments(List *args,
  	  eval_const_expressions_context *context,
  	  bool *haveNull, bool *forceTrue);
*** estimate_expression_value(PlannerInfo *r
*** 2472,2477 
--- 2475,2511 
  	return eval_const_expressions_mutator(node, );
  }
  
+ /*
+  * The generic case in eval_const_expressions_mutator is to recurse using
+  * expression_tree_mutator, which will copy the given node unchanged but
+  * const-simplify its arguments (if any) as far as possible.  If the node
+  * itself does immutable processing, and each of its arguments were reduced
+  * to a Const, we can then reduce it to a Const using evaluate_expr.  (Some
+  * node types need more complicated logic; for example, a CASE expression
+  * might be reducible to a constant even if not all its subtrees are.)
+  */
+ #define ece_generic_processing(node) \
+ 	expression_tree_mutator((Node *) (node), eval_const_expressions_mutator, \
+ 			(void *) context)
+ 
+ /*
+  * Check whether all arguments of the given node were reduced to Consts.
+  * By going directly to expression_tree_walker, contain_non_const_walker
+  * is not applied to the node itself, only to its children.
+  */
+ #define ece_all_arguments_const(node) \
+ 	(!expression_tree_walker((Node *) (node), contain_non_const_walker, NULL))
+ 
+ /* Generic macro for applying evaluate_expr */
+ #define ece_evaluate_expr(node) \
+ 	((Node *) evaluate_expr((Expr *) (node), \
+ 			exprType((Node *) (node)), \
+ 			exprTypmod((Node *) (node)), \
+ 			exprCollation((Node *) (node
+ 
+ /*
+  * Recursive guts of eval_const_expressions/estimate_expression_value
+  */
  static Node *
  eval_const_expressions_mutator(Node *node,
  			   eval_const_expressions_context *context)
*** eval_const_expressions_mutator(Node *nod
*** 2787,2792 
--- 2821,2845 
  newexpr->location = expr->location;
  return (Node *) newexpr;
  			}
+ 		case T_ScalarArrayOpExpr:
+ 			{
+ ScalarArrayOpExpr *saop;
+ 
+ /* Copy the node and const-simplify its arguments */
+ saop = (ScalarArrayOpExpr *) ece_generic_processing(node);
+ 
+ /* Make sure we know underlying function */
+ set_sa_opfuncid(saop);
+ 
+ /*
+  * If all arguments are Consts, and it's a safe function, we
+  * can fold to a constant
+  */
+ if (ece_all_arguments_const(saop) &&
+ 	ece_function_is_safe(saop->opfuncid, context))
+ 	return ece_evaluate_expr(saop);
+ return (Node *) saop;
+ 			}
  		case T_BoolExpr:
  			{
  BoolExpr   *expr = (BoolExpr *) node;
*** eval_const_expressions_mutator(Node *nod
*** 3011,3057 
  			}
  		case T_ArrayCoerceExpr:
  			{
! ArrayCoerceExpr *expr = (ArrayCoerceExpr *) node;
! Expr	   *arg;
! Expr	   *elemexpr;
! ArrayCoerceExpr *newexpr;
! 
! /*
!  * Reduce constants in the ArrayCoerceExpr's argument and
!  * per-element expressions, then build a new ArrayCoerceExpr.
!  */
! arg = (Expr *) eval_const_expressions_mutator((Node *) expr->arg,
! 			  context);
! elemexpr = (Expr *) eval_const_expressions_mutator((Node *) expr->elemexpr,
!    context);
  
! newexpr = makeNode(ArrayCoerceExpr);
! newexpr->arg = arg;
! newexpr->elemexpr = elemexpr;
! newexpr->resulttype = expr->resulttype;
! newexpr->resulttypmod = expr->resulttypmod;
! newexpr->resultcollid = expr->resultcollid;
! newexpr->coerceformat = expr->coerceformat;
! newexpr->location = expr->location;
  
  /*
!  * If constant argument and per-element expression is
   * immutable, we can simplify the whole thing to a constant.
   * Exception: although contain_mutable_functions considers
   * CoerceToDomain immutable for historical reasons, let's not
   * do so here; this ensures coercion to an array-over-domain
   * does not apply the domain's constraints until runtime.
   */
! if (arg && IsA(arg, Const) &&
! 	elemexpr && 

Re: [HACKERS] Logging idle checkpoints

[Michael Paquier]

On Mon, Oct 2, 2017 at 7:41 AM, Andres Freund  wrote:
> On 2017-10-02 07:39:18 +0900, Michael Paquier wrote:
>> On Mon, Oct 2, 2017 at 7:27 AM, Andres Freund  wrote:
>> > On 2017-10-02 00:19:33 +0200, Vik Fearing wrote:
>> > I'd be ok with applying this now, or in 10.1 - but I do think we should
>> > fix this before 11.  If nobody protests I'll push later today, so we can
>> > get some bf cycles for the very remote case that this causes problems.
>>
>> This point has been discussed during review and removed from the patch
>> (adding Stephen in the loop here):
>> https://www.postgresql.org/message-id/CAOuzzgq8pHneMHy6JiNiG6Xm5V=cm+k2wcd2w-sctpjdg7x...@mail.gmail.com
>
> I find that reasoning unconvincing. log_checkpoints is enabled after
> all. And we're not talking about 10 log messages a second. There's
> plenty systems that analyze the logs that'd possibly be affected by
> this.

No real objections from here, actually.

>> Actually, shouldn't we make BgWriterStats a bit smarter? We could add
>> a counter for skipped checkpoints in v11 (too late for v10).
>
> Wouldn't hurt, but seems orthogonal.

Sure.
-- 
Michael


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

Re: [HACKERS] Logging idle checkpoints

[Andres Freund]

On 2017-10-02 07:39:18 +0900, Michael Paquier wrote:
> On Mon, Oct 2, 2017 at 7:27 AM, Andres Freund  wrote:
> > On 2017-10-02 00:19:33 +0200, Vik Fearing wrote:
> > I'd be ok with applying this now, or in 10.1 - but I do think we should
> > fix this before 11.  If nobody protests I'll push later today, so we can
> > get some bf cycles for the very remote case that this causes problems.
> 
> This point has been discussed during review and removed from the patch
> (adding Stephen in the loop here):
> https://www.postgresql.org/message-id/CAOuzzgq8pHneMHy6JiNiG6Xm5V=cm+k2wcd2w-sctpjdg7x...@mail.gmail.com

I find that reasoning unconvincing. log_checkpoints is enabled after
all. And we're not talking about 10 log messages a second. There's
plenty systems that analyze the logs that'd possibly be affected by
this.


> Actually, shouldn't we make BgWriterStats a bit smarter? We could add
> a counter for skipped checkpoints in v11 (too late for v10).

Wouldn't hurt, but seems orthogonal.

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] Logging idle checkpoints

[Michael Paquier]

On Mon, Oct 2, 2017 at 7:27 AM, Andres Freund  wrote:
> On 2017-10-02 00:19:33 +0200, Vik Fearing wrote:
> I'd be ok with applying this now, or in 10.1 - but I do think we should
> fix this before 11.  If nobody protests I'll push later today, so we can
> get some bf cycles for the very remote case that this causes problems.

This point has been discussed during review and removed from the patch
(adding Stephen in the loop here):
https://www.postgresql.org/message-id/CAOuzzgq8pHneMHy6JiNiG6Xm5V=cm+k2wcd2w-sctpjdg7x...@mail.gmail.com
Actually, shouldn't we make BgWriterStats a bit smarter? We could add
a counter for skipped checkpoints in v11 (too late for v10).
-- 
Michael


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

Re: [HACKERS] Logging idle checkpoints

[Andres Freund]

Hi,

On 2017-10-02 00:19:33 +0200, Vik Fearing wrote:
> I recently had a sad because I noticed that checkpoint counts were
> increasing in pg_stat_bgwriter, but weren't accounted for in my logs
> with log_checkpoints enabled.
> 
> After some searching, I found that it was the idle checkpoints that
> weren't being logged.  I think this is a missed trick in 6ef2eba3f57.
> 
> Attached is a one-liner fix.  I realize how imminent we are from
> releasing v10 but I hope there is still time for such a minor issue as this.


> diff --git a/src/backend/access/transam/xlog.c 
> b/src/backend/access/transam/xlog.c
> index dd028a12a4..75f6bd4cc1 100644
> --- a/src/backend/access/transam/xlog.c
> +++ b/src/backend/access/transam/xlog.c
> @@ -8724,7 +8724,7 @@ CreateCheckPoint(int flags)
>   WALInsertLockRelease();
>   LWLockRelease(CheckpointLock);
>   END_CRIT_SECTION();
> - ereport(DEBUG1,
> + ereport(log_checkpoints ? LOG : DEBUG1,
>   (errmsg("checkpoint skipped because 
> system is idle")));
>   return;
>   }

I'd be ok with applying this now, or in 10.1 - but I do think we should
fix this before 11.  If nobody protests I'll push later today, so we can
get some bf cycles for the very remote case that this causes problems.

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] Statement-level rollback

[Daniel Gustafsson]

> On 15 Sep 2017, at 16:19, Daniel Gustafsson  wrote:
> 
>> On 01 Sep 2017, at 13:44, Simon Riggs  wrote:
>> 
>> On 14 August 2017 at 23:58, Peter Eisentraut
>>  wrote:
>>> On 2/28/17 02:39, Tsunakawa, Takayuki wrote:
 The code for stored functions is not written yet, but I'd like your 
 feedback for the specification and design based on the current patch.  
 I'll add this patch to CommitFest 2017-3.
>>> 
>>> This patch needs to be rebased for the upcoming commit fest.
>> 
>> I'm willing to review this if the patch is going to be actively worked on.
> 
> This sounds like a too good offer to pass up on, can we expect a rebased patch
> for the commitfest?

Since this patch was Waiting for author during the entire commitfest without
updates, I’m marking it Returned with Feedback.  When a new version is ready it
can be re-submitted to the then open commitfest.

cheers ./daniel

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

[HACKERS] Logging idle checkpoints

[Vik Fearing]

I recently had a sad because I noticed that checkpoint counts were
increasing in pg_stat_bgwriter, but weren't accounted for in my logs
with log_checkpoints enabled.

After some searching, I found that it was the idle checkpoints that
weren't being logged.  I think this is a missed trick in 6ef2eba3f57.

Attached is a one-liner fix.  I realize how imminent we are from
releasing v10 but I hope there is still time for such a minor issue as this.
-- 
Vik Fearing  +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index dd028a12a4..75f6bd4cc1 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8724,7 +8724,7 @@ CreateCheckPoint(int flags)
 			WALInsertLockRelease();
 			LWLockRelease(CheckpointLock);
 			END_CRIT_SECTION();
-			ereport(DEBUG1,
+			ereport(log_checkpoints ? LOG : DEBUG1,
 	(errmsg("checkpoint skipped because system is idle")));
 			return;
 		}

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

Re: [HACKERS] ANALYZE command progress checker

[Daniel Gustafsson]

> On 05 Apr 2017, at 03:17, Masahiko Sawada  wrote:
> 
> On Wed, Apr 5, 2017 at 1:49 AM, Robert Haas  wrote:
>> On Tue, Apr 4, 2017 at 4:57 AM, Amit Langote
>>  wrote:
>>> Hmm, you're right.  It could be counted with a separate variable
>>> initialized to 0 and incremented every time we decide to add a row to the
>>> final set of sampled rows, although different implementations of
>>> AcquireSampleRowsFunc have different ways of deciding if a given row will
>>> be part of the final set of sampled rows.
>>> 
>>> On the other hand, if we decide to count progress in terms of blocks as
>>> you suggested afraid, I'm afraid that FDWs won't be able to report the
>>> progress.
>> 
>> I think it may be time to push this patch out to v11.  It was
>> submitted one day before the start of the last CommitFest, the design
>> wasn't really right, and it's not clear even now that we know what the
>> right design is.  And we're pretty much out of time.
> 
> +1
> We're encountering the design issue and it takes more time to find out
> right design including FDWs support.

I’m marking this patch Returned with Feedback since it has been Waiting for
author during the commitfest without updates.

cheers ./daniel

-- 
Sent 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_ctl kill support for KILL signal was Re: [COMMITTERS] pgsql: Add test for postmaster crash restarts.

[Andres Freund]

On 2017-10-01 18:01:27 -0400, Tom Lane wrote:
> Andres Freund  writes:
> > On 2017-10-01 17:47:52 -0400, Andrew Dunstan wrote:
> >> So we can leave it out of there. OTOH I'm not a huge fan of security by
> >> obscurity. I guess this wouldn't be too bad a case.
> 
> > I'd personally include it, given that we already allow and document
> > ABRT. There's no meaningful difference between the two.
> 
> True.

I'll push it that way then. Adapting a help text later is fairly
harmless.

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] pg_ctl kill support for KILL signal was Re: [COMMITTERS] pgsql: Add test for postmaster crash restarts.

[Tom Lane]

Andres Freund  writes:
> On 2017-10-01 17:47:52 -0400, Andrew Dunstan wrote:
>> So we can leave it out of there. OTOH I'm not a huge fan of security by
>> obscurity. I guess this wouldn't be too bad a case.

> I'd personally include it, given that we already allow and document
> ABRT. There's no meaningful difference between the two.

True.

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] pgbench: faster version of tpcb-like transaction

[Daniel Gustafsson]

> On 27 Aug 2017, at 08:37, Fabien COELHO  wrote:
> 
> 
> About the patch:
> 
> I'm generally in favor of providing more options to pgbench, especially if it 
> can give optimization ideas to the performance conscious user.
> 
> I think that the name should be "tpcb-like-plfunc": the script does not 
> implement tpcb per spec, and such a function could be written in another 
> language with some performance benefit, or not.
> 
> Maybe that mean to relax the prefix condition to "take the first matching 
> name" when prefix are used.
> 
> If you are reimplementing the transaction anyway, you could consider using 
> UPDATE RETURNING instead of SELECT to get the balance. On the other hand the 
> doc says that the "steps" are put in a PL function, so maybe it should 
> reflect the original script.
> 
> I'm surprised by:
> 
>  "select * from pgbench_transaction(:aid, :bid, :tid, :delta);\n"
> 
> Why not simply:
> 
>"select pgbench_transaction(:aid, :bid, :tid, :delta);\n"
> 
> I would suggest to use a more precise function name, in case other functions 
> are thought of. Maybe "pgbench_tpcb_like_plfunc".
> 
> I would suggest to indent better the PL/function and put keywords and types 
> in capital, and add explicitely the properties of the function (eg STRICT, 
> VOLATILE?).
> 
> There is a spurious space at the end of the executeStatement call line.
> 
> The patch potentially interacts with other patches in the long and slow 
> queue...
> 
> As usual with pgbench there are no regression tests.

This patch has been Waiting for author during the commitfest without updates,
moving to Returned with feedback.

cheers ./daniel



-- 
Sent 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_ctl kill support for KILL signal was Re: [COMMITTERS] pgsql: Add test for postmaster crash restarts.

[Andres Freund]

On 2017-10-01 17:47:52 -0400, Andrew Dunstan wrote:
> 
> 
> On 10/01/2017 04:48 PM, Andres Freund wrote:
> > On 2017-10-01 16:42:44 -0400, Tom Lane wrote:
> >> Andrew Dunstan  writes:
> >>> On 09/30/2017 10:32 PM, Andres Freund wrote:
>  Heh.  I'm inclined to take it out. We could add a --use-the-force-luke
>  type parameter, but it doesn't seem worth it.
> >>> I agree, but I think we need this discussed on -hackers. Does anyone
> >>> have an objection to allowing "pg_ctl kill KILL somepid"? As Andres
> >>> points out, in most places you can just call kill from the command line
> >>> anyway, so disallowing it is not really a security feature. Having it
> >>> would let us have portable crash restart tests.
> >> +1 for portable tests, but it still seems like something we don't want
> >> to encourage users to use.  What do you think of leaving it out of the
> >> documentation?
> > As far as I can tell we've not documented the set of acceptable signals
> > anywhere but the source. I think we can just keep it that way?
> 
> 
> As documented it's in the help text:
> 
> printf(_("\nAllowed signal names for kill:\n"));
> printf("  ABRT HUP INT QUIT TERM USR1 USR2\n");

Oh, hm. I'd looked above.


> So we can leave it out of there. OTOH I'm not a huge fan of security by
> obscurity. I guess this wouldn't be too bad a case.

I'd personally include it, given that we already allow and document
ABRT. There's no meaningful difference between the two.

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: psql show index with type info

[Daniel Gustafsson]

> On 13 Sep 2017, at 01:24, Daniel Gustafsson  wrote:
> 
>> On 18 Apr 2017, at 05:13, Amos Bird  wrote:
>> 
>> Ah, thanks for the suggestions. I'll revise this patch soon :)
> 
> Have you had a chance to revise the patch to address the review comments such
> that the patch can move forward during this Commitfest?

Since the patch was Waiting for author without being updated during the
commitfest, it has been Returned with Feedback.

cheers ./daniel

-- 
Sent 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_ctl kill support for KILL signal was Re: [COMMITTERS] pgsql: Add test for postmaster crash restarts.

[Andrew Dunstan]

On 10/01/2017 04:48 PM, Andres Freund wrote:
> On 2017-10-01 16:42:44 -0400, Tom Lane wrote:
>> Andrew Dunstan  writes:
>>> On 09/30/2017 10:32 PM, Andres Freund wrote:
 Heh.  I'm inclined to take it out. We could add a --use-the-force-luke
 type parameter, but it doesn't seem worth it.
>>> I agree, but I think we need this discussed on -hackers. Does anyone
>>> have an objection to allowing "pg_ctl kill KILL somepid"? As Andres
>>> points out, in most places you can just call kill from the command line
>>> anyway, so disallowing it is not really a security feature. Having it
>>> would let us have portable crash restart tests.
>> +1 for portable tests, but it still seems like something we don't want
>> to encourage users to use.  What do you think of leaving it out of the
>> documentation?
> As far as I can tell we've not documented the set of acceptable signals
> anywhere but the source. I think we can just keep it that way?


As documented it's in the help text:

printf(_("\nAllowed signal names for kill:\n"));
printf("  ABRT HUP INT QUIT TERM USR1 USR2\n");


So we can leave it out of there. OTOH I'm not a huge fan of security by
obscurity. I guess this wouldn't be too bad a case.

cheers

andrew

-- 

Andrew Dunstanhttps://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



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

Re: [HACKERS] [PATCH] Off-by-one error in logical slot resource retention

[Daniel Gustafsson]

> On 15 Sep 2017, at 13:26, Daniel Gustafsson  wrote:
> 
>> On 01 Sep 2017, at 14:28, Aleksander Alekseev  
>> wrote:
>> 
>> The following review has been posted through the commitfest application:
>> make installcheck-world:  not tested
>> Implements feature:   not tested
>> Spec compliant:   not tested
>> Documentation:not tested
>> 
>> Hi Craig,
>> 
>> I'm afraid patches 0002 and 0003 don't apply anymore. Could you please 
>> resolve the conflicts?
>> 
>> The new status of this patch is: Waiting on Author
> 
> Have you had a chance to look at rebasing this patch so we can get it further
> in the process?

Since this patch has been in Waiting for Author state for the duration of the
commitfest without moving, I’m marking it Returned with Feedback.  If there is
still interest in pursuing this patch, please re-submit it to the next
commitfest with the comments addressed.

cheers ./daniel

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

Re: [HACKERS] 200 = 199 + 1?

[Tom Lane]

I wrote:
> I experimented a bit with the attached patch, which modifies
> eqjoinsel_semi in two ways.  First, if the number-of-distinct-values
> estimate for the inner rel is just a default rather than having any
> real basis, it replaces it with inner_rel->rows, effectively assuming
> that the inside of the IN or EXISTS is unique.

That might or might not be a good idea ...

> Second, it drops the
> fallback to selectivity 0.5 altogether, just applying the nd1 vs nd2
> heuristic all the time.

... but this probably isn't.  A review of the history shows me that
this change amounts to reverting 3f5d2fe30, which doesn't seem like
a good plan because that was fixing user-reported misbehavior.
The problem is still that the nd2/nd1 calculation can produce garbage
if nd2 or nd1 is made-up.  It's possible that we can get away with using
nd2 = inner_rel->rows as a suitable substitute for a default nd2, but
I'm much less happy to assume something like that for nd1.

Now, there's still not much to defend the 0.5 selectivity in particular;
according to the commit log for 3f5d2fe30, I used that because it
reproduced the behavior of previous versions that didn't understand what
a semijoin was at all.  So we could perhaps substitute some other rule
there, but I don't know what.

I also note that the precise behavior of HEAD in this area only dates
back to ca5f88502, which hasn't even shipped in a release yet, so it
surely doesn't have a lot of field experience justifying it.

Other useful-though-not-terribly-recent discussions in this area include

https://www.postgresql.org/message-id/flat/201201112110.40403.andres%40anarazel.de

https://www.postgresql.org/message-id/13290.1335976...@sss.pgh.pa.us

Given that eqjoinsel_semi has been more or less a constant source of
issues, maybe we need to think about a wholesale redesign rather than
just incremental tweaking.  But I have few ideas about what would be
better.

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] shm_mq_set_sender() crash

[Noah Misch]

On Thu, Sep 15, 2016 at 06:21:30PM -0400, Robert Haas wrote:
> On Thu, Sep 15, 2016 at 5:22 PM, Tom Lane  wrote:
> > Robert Haas  writes:
> >> Of course, it's also possible that the ParallelWorkerNumber code is
> >> entirely correct and something overwrote the null bytes that were
> >> supposed to be found at that location.  It would be very useful to see
> >> (a) the value of ParallelWorkerNumber and (b) the contents of
> >> vmq->mq_sender, and in particular whether that's actually a valid
> >> pointer to a PGPROC in the ProcArray.  But unless we can reproduce
> >> this I don't see how to manage that.
> >
> > Is it worth replacing that Assert with a test-and-elog that would
> > print those values?
> >
> > Given that we've seen only one such instance in the buildfarm, this
> > might've been just a cosmic ray bit-flip.  So one part of me says
> > not to worry too much until we see it again.  OTOH, if it is real
> > but rare, missing an opportunity to diagnose would be bad.
> 
> I wonder if we could persuade somebody to run pgbench on a Windows
> machine with a similar environment, at least some concurrency, and
> force_parallel_mode=on.  Assuming this is a generic
> initialize-the-parallel-stuff bug and not something specific to a
> particular query, that might well trigger it a lot quicker than
> waiting for it to recur in the BF.

For the sake of this thread in the archives, the cause was almost surely a
Cygwin signal handling bug:

  https://postgr.es/m/20170803034740.ga2641...@rfd.leadboat.com
  https://marc.info/?t=15018329641
  https://marc.info/?t=150291861700011


-- 
Sent 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_ctl kill support for KILL signal was Re: [COMMITTERS] pgsql: Add test for postmaster crash restarts.

[Andres Freund]

On 2017-10-01 16:42:44 -0400, Tom Lane wrote:
> Andrew Dunstan  writes:
> > On 09/30/2017 10:32 PM, Andres Freund wrote:
> >> Heh.  I'm inclined to take it out. We could add a --use-the-force-luke
> >> type parameter, but it doesn't seem worth it.
> 
> > I agree, but I think we need this discussed on -hackers. Does anyone
> > have an objection to allowing "pg_ctl kill KILL somepid"? As Andres
> > points out, in most places you can just call kill from the command line
> > anyway, so disallowing it is not really a security feature. Having it
> > would let us have portable crash restart tests.
> 
> +1 for portable tests, but it still seems like something we don't want
> to encourage users to use.  What do you think of leaving it out of the
> documentation?

As far as I can tell we've not documented the set of acceptable signals
anywhere but the source. I think we can just keep it that way?

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] pg_ctl kill support for KILL signal was Re: [COMMITTERS] pgsql: Add test for postmaster crash restarts.

[Tom Lane]

Andrew Dunstan  writes:
> On 09/30/2017 10:32 PM, Andres Freund wrote:
>> Heh.  I'm inclined to take it out. We could add a --use-the-force-luke
>> type parameter, but it doesn't seem worth it.

> I agree, but I think we need this discussed on -hackers. Does anyone
> have an objection to allowing "pg_ctl kill KILL somepid"? As Andres
> points out, in most places you can just call kill from the command line
> anyway, so disallowing it is not really a security feature. Having it
> would let us have portable crash restart tests.

+1 for portable tests, but it still seems like something we don't want
to encourage users to use.  What do you think of leaving it out of the
documentation?

regards, tom lane


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

[HACKERS] pg_ctl kill support for KILL signal was Re: [COMMITTERS] pgsql: Add test for postmaster crash restarts.

[Andrew Dunstan]

On 09/30/2017 10:32 PM, Andres Freund wrote:
> Hi,
>
> On 2017-09-30 22:28:39 -0400, Andrew Dunstan wrote:
 But even after fixing that, there unfortunately is:

 static void
 set_sig(char *signame)
 {
 …
 #if 0
/* probably should NOT provide SIGKILL */
else if (strcmp(signame, "KILL") == 0)
sig = SIGKILL;
 #endif

 I'm unclear on what that provision is achieving? If you can kill with
 pg_ctl you can do other nasty stuff too (like just use kill instead of
 pg_ctl)?
>>
>> I put it in when we rewrote pg_ctl in C many years ago, possibly out of
>> a superabundance of caution. I agree it's worth revisiting. I think the
>> idea was that there's a difference between an ordinary footgun and an
>> officially sanctioned footgun :-)
> Heh.  I'm inclined to take it out. We could add a --use-the-force-luke
> type parameter, but it doesn't seem worth it.
>
>
>


I agree, but I think we need this discussed on -hackers. Does anyone
have an objection to allowing "pg_ctl kill KILL somepid"? As Andres
points out, in most places you can just call kill from the command line
anyway, so disallowing it is not really a security feature. Having it
would let us have portable crash restart tests.

cheers

andrew


-- 

Andrew Dunstanhttps://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] 64-bit queryId?

[Greg Stark]

On 1 October 2017 at 16:40, Tom Lane  wrote:
> Greg Stark  writes:
>> Indeed. It's simple enough to export stats to prometheus with queryid
>> as the key. Then even if the query ages out of the database stats you
>> have graphs and derivative metrics going further back.
>
> I'm not really ready to buy into that as a supported use-case, because
> it immediately leads to the conclusion that we need to promise stability
> of query IDs across PG versions, which seems entirely infeasible to me
> (at least with anything like the current method of deriving them).

Well these kinds of monitoring systems tend to be used by operations
people who are a lot more practical and a lot less worried about
theoretical concerns like that.

What they're going to want to do is things like "alert on any query
using more than 0.1 cpu core" or "alert on any query being the top i/o
consumer that isn't amongst the top 10 over the previous day" or
"alert on any query using more than 4x the cpu or i/o on one database
than it does on average across all our databases".  That last one is a
case where it would be nice if the queryid values were stable across
versions but it's hardly surprising that they aren't.

In context the point was merely that the default
pg_stat_statements.max of 5000 isn't sufficient to argue that 32-bit
values are enough. It wouldn't be hard for there to be 64k different
queries over time and across all the databases in a fleet and at that
point it becomes likely there'll be a 32-bit collision.

-- 
greg


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

Re: [HACKERS] pg_basebackup --progress output for batch execution

[Martin Marques]

Updated patch with documentation of the new option.


-- 
Martín Marquéshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
>From ede201ed96d41d799dc3c83dfab1cdcc03e5ced4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mart=C3=ADn=20Marqu=C3=A9s?=
 
Date: Sun, 1 Oct 2017 16:39:41 -0300
Subject: [PATCH] Adding an option to pg_basebackup to output messages as if it
 were running in batch-mode, as opossed to running in a tty.

This is usefull when using --progress and redirecting the output to
a file for later inspection with tail.

New option --batch-mode with the short option -b added.
---
 doc/src/sgml/ref/pg_basebackup.sgml   | 16 
 src/bin/pg_basebackup/pg_basebackup.c | 19 +--
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml
index f790c56..db5160f 100644
--- a/doc/src/sgml/ref/pg_basebackup.sgml
+++ b/doc/src/sgml/ref/pg_basebackup.sgml
@@ -461,6 +461,22 @@ PostgreSQL documentation
  
 
  
+  -b
+  --batch-mode
+  
+   
+Runs pg_basebackup in batch mode. This is useful if
+the output is to be pipped so the other end of the pipe reads each line.
+   
+   
+Using this option with --progress will result in
+printing each progress output with a newline at the end, instead of a
+carrige return.
+   
+  
+ 
+
+ 
   -S slotname
   --slot=slotname
   
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index dac7299..cf97ea3 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -82,6 +82,7 @@ static char format = 'p';		/* p(lain)/t(ar) */
 static char *label = "pg_basebackup base backup";
 static bool noclean = false;
 static bool showprogress = false;
+static bool batchmode = false;
 static int	verbose = 0;
 static int	compresslevel = 0;
 static IncludeWal includewal = STREAM_WAL;
@@ -355,6 +356,7 @@ usage(void)
 	printf(_("  -P, --progress show progress information\n"));
 	printf(_("  -S, --slot=SLOTNAMEreplication slot to use\n"));
 	printf(_("  --no-slot  prevent creation of temporary replication slot\n"));
+	printf(_("  -b, --batch-mode   run in batch mode\n"));
 	printf(_("  -v, --verbose  output verbose messages\n"));
 	printf(_("  -V, --version  output version information, then exit\n"));
 	printf(_("  -?, --help show this help, then exit\n"));
@@ -806,7 +808,10 @@ progress_report(int tablespacenum, const char *filename, bool force)
 totaldone_str, totalsize_str, percent,
 tablespacenum, tablespacecount);
 
-	fprintf(stderr, "\r");
+	if (batchmode)
+		fprintf(stderr, "\n");
+	else
+		fprintf(stderr, "\r");
 }
 
 static int32
@@ -1786,7 +1791,13 @@ BaseBackup(void)
 progname);
 
 	if (showprogress && !verbose)
+	{
 		fprintf(stderr, "waiting for checkpoint\r");
+		if (batchmode)
+			fprintf(stderr, "\n");
+		else
+			fprintf(stderr, "\r");
+	}
 
 	basebkp =
 		psprintf("BASE_BACKUP LABEL '%s' %s %s %s %s %s %s",
@@ -2118,6 +2129,7 @@ main(int argc, char **argv)
 		{"status-interval", required_argument, NULL, 's'},
 		{"verbose", no_argument, NULL, 'v'},
 		{"progress", no_argument, NULL, 'P'},
+		{"batch-mode", no_argument, NULL, 'b'},
 		{"waldir", required_argument, NULL, 1},
 		{"no-slot", no_argument, NULL, 2},
 		{NULL, 0, NULL, 0}
@@ -2146,7 +2158,7 @@ main(int argc, char **argv)
 
 	atexit(cleanup_directories_atexit);
 
-	while ((c = getopt_long(argc, argv, "CD:F:r:RS:T:X:l:nNzZ:d:c:h:p:U:s:wWvP",
+	while ((c = getopt_long(argc, argv, "CD:F:r:RS:T:X:l:nNzZ:d:c:h:p:U:s:wWvPb",
 			long_options, _index)) != -1)
 	{
 		switch (c)
@@ -2288,6 +2300,9 @@ main(int argc, char **argv)
 			case 'P':
 showprogress = true;
 break;
+			case 'b':
+batchmode = true;
+break;
 			default:
 
 /*
-- 
2.9.5


-- 
Sent 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: CREATE COLLATION does not sanitize ICU's BCP 47 language tags. Should it?

[Noah Misch]

On Sun, Oct 01, 2017 at 09:56:11AM -0400, Peter Eisentraut wrote:
> On 9/30/17 03:01, Noah Misch wrote:
> > This PostgreSQL 10 open item is past due for your status update.  On the 
> > worst
> > week to be violating open item policies.  Kindly send a status update within
> > 24 hours, and include a date for your subsequent status update.  Refer to 
> > the
> > policy on open item ownership:
> > https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com
> 
> I had previously replied that I think nothing should be changed.  What
> process should be applied in that case?

If you determine community decision-making is not against that conclusion,
edit the list to move the item from "Open Issues" to "Non-bugs".


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

Re: [HACKERS] CREATE COLLATION does not sanitize ICU's BCP 47 language tags. Should it?

[Peter Geoghegan]

On Sun, Oct 1, 2017 at 6:27 AM, Peter Eisentraut
 wrote:
> I'm still pondering committing my documentation patch I had posted, and
> I've been reviewing your patches to see if there is anything else
> documentation-wise that could be added.

-1 from me -- I don't think we should tell users about the deprecated
locale format at all.

I hope that ICU continue to support the deprecated locale string
format within ucol_open() into the future, and that users will only
ever use BCP 47 within CREATE COLLATION (as the 'locale' string).
Perhaps it won't matter that much in the end, and will turn out to be
more of a wart than something that causes pain for users. It's hard to
predict.

> As I had mentioned before, I disagree with the substance of the
> functionality changes you are proposing, and I think it would be way too
> late to make such significant changes.  The general community opinion
> also seems to be in favor of letting things be.

It does seem too late. It's disappointing that we did not do better
here. This problem was entirely avoidable.

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

[Pavel Stehule]

2017-09-30 1:06 GMT+02:00 Nikita Glukhov :

> On 29.09.2017 20:07, Pavel Stehule wrote:
>
> 2017-09-29 12:15 GMT+02:00 Pavel Stehule :
>
>>
>> 2017-09-29 12:09 GMT+02:00 Nikita Glukhov :
>>
>>>
>>>
>>> I have some free time now. Is it last version?
>>>
>>> Regards
>>>
>>> Pavel
>>>
>>> Yes, this is still the latest version. Now I am working only on
>>> unfinished WIP
>>> patch no. 9, but I think it should be reviewed the last.
>>>
>>>
>>
>> ok
>>
>> Thank you
>>
>
> I have few queries and notes
>
> 1. Why first patch holds Gin related functionality? Can be it separated?
>
> Yes, it can be easily separated. Attached archive with separated GIN patch
> no.2.
>
> 2. Why Json path functions starts by "_" ? These functions are not removed
> by other patches.
>
> Originally, these functions were created only for testing purposes and
> should
> be treated as "internal". But with introduction of jsonpath operators
> jsonpath
> tests can be completely rewritten using this operators.
>

yes - it should be removed.

Probably separation to jsonpath and sqljson is not happy (or sqljson part
should not contains JSON_QUERY and related functions).

Why this code is in patch?

















*+/Example functions for
JsonPath***/++static Datum+returnDATUM(void *arg,
bool *isNull)+{+<->*isNull =
false;+<->return<>PointerGetDatum(arg);+}++static Datum+returnNULL(void
*arg, bool *isNull)+{+<->*isNull = true;+<->return Int32GetDatum(0);+}+*
Regards

Pavel


> 3. What is base for jsonpath-extensions? ANSI/SQL?
>
> Our jsonpath extensions are not based on any standards, so they are quite
> dangerous because they can conflict with the standard in the future.
>
> This patch is pretty big - so I propose to push JSONPath and SQL/JSON
> related patches first, and then in next iteration to push JSON_TABLE patch.
> Is it acceptable strategy?
>
> I think it's acceptable. And this was the main reason for the separation
> of patches.
>
> I am sure so JSON_TABLE is pretty important function, but it is pretty
> complex too (significantly more complex than XMLTABLE), so it can be
> practiacal to move this function to separate project. I hope so all patches
> will be merged in release 11 time.
>
>
> --
> Nikita Glukhov
> Postgres Professional: http://www.postgrespro.com
> The Russian Postgres Company
>

Re: [HACKERS] [PATCH]make pg_rewind to not copy useless WAL files

[chenhj]

On  2017-10-01 04:09:19,"Alexander Korotkov"  wrote:

On Sat, Sep 30, 2017 at 8:18 PM, chenhj  wrote:

On 2017-09-30 02:17:54，"Alexander Korotkov"  wrote:


Great.  Now code of this patch looks good for me.
However, we forgot about documentation.


  
   The result is equivalent to replacing the target data directory with the
   source one. Only changed blocks from relation files are copied;
   all other files are copied in full, including configuration files. The
   advantage of pg_rewind over taking a new base backup, or
   tools like rsync, is that pg_rewind does
   not require reading through unchanged blocks in the cluster. This makes
   it a lot faster when the database is large and only a small
   fraction of blocks differ between the clusters.
  


At least, this paragraph need to be adjusted, because it states whose files are 
copied.  And probably latter paragraphs whose state about WAL files.






Your are rigth.
I wrote a draft as following, but i'm afraid whether the english statement is 
accurate.


I'm not native english speaker too :(


Only the WAL files between the point of divergence and the current WAL insert 
location of the source server are copied, *for* other WAL files are useless for 
the target server. 


I'm not sure about this usage of word *for*.  For me, it probably should be 
just removed.  Rest of changes looks good for me.  Please, integrate them into 
the patch.




I had removed the *for* , Pleae check the new patch again.


---
Best Regards,
Chen Huajun

pg_rewind_wal_copy_reduce_v8.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] show precise repos version for dev builds?

[Fabien COELHO]

Hello,


Fabien COELHO  writes:

I wanted to know which version it was, and "11devel" is kind of imprecise.
...
ISTM that extending the version name with the commit id and or date in
some version output, eg "11devel [2632bcc 2017-09-30 ...]", would do it.


configure --with-extra-version=whateveryouwant


Thanks for the pointer!

So now I have to convince the apt.postgresql.org people to build the devel 
version with this trick.


--
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] 64-bit queryId?

[Tom Lane]

Greg Stark  writes:
> Indeed. It's simple enough to export stats to prometheus with queryid
> as the key. Then even if the query ages out of the database stats you
> have graphs and derivative metrics going further back.

I'm not really ready to buy into that as a supported use-case, because
it immediately leads to the conclusion that we need to promise stability
of query IDs across PG versions, which seems entirely infeasible to me
(at least with anything like the current method of deriving them).

regards, tom lane


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

Re: [HACKERS] show precise repos version for dev builds?

[Tom Lane]

Fabien COELHO  writes:
> I wanted to know which version it was, and "11devel" is kind of imprecise.
> ...
> ISTM that extending the version name with the commit id and or date in 
> some version output, eg "11devel [2632bcc 2017-09-30 ...]", would do it.

configure --with-extra-version=whateveryouwant

regards, tom lane


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

[HACKERS] Warnings in objectaddress.c

[Дмитрий Воронин]

Hello, hackers!

I'm building PostgreSQL 10 rc1 sources on Debian wheezy (gcc 4.7). I have those 
warnings:

objectaddress.c: In function ‘get_object_address’:
objectaddress.c:1646:10: warning: ‘typeoids[1]’ may be used uninitialized in 
this function [-Wmaybe-uninitialized]
objectaddress.c:1578:8: note: ‘typeoids[1]’ was declared here
objectaddress.c:1623:7: warning: ‘typenames[1]’ may be used uninitialized in 
this function [-Wmaybe-uninitialized]
objectaddress.c:1577:14: note: ‘typenames[1]’ was declared here
objectaddress.c:1646:10: warning: ‘typeoids[0]’ may be used uninitialized in 
this function [-Wmaybe-uninitialized]
objectaddress.c:1578:8: note: ‘typeoids[0]’ was declared here
objectaddress.c:1623:7: warning: ‘typenames[0]’ may be used uninitialized in 
this function [-Wmaybe-uninitialized]
objectaddress.c:1577:14: note: ‘typenames[0]’ was declared here

Those variables typeoids and typenames are arrays and are not initialized 
during definition.

Hope this helps. Thank you!

-- 
Best regargs, Dmitry Voronin


-- 
Sent 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: CREATE COLLATION does not sanitize ICU's BCP 47 language tags. Should it?

[Peter Eisentraut]

On 9/30/17 03:01, Noah Misch wrote:
> On Mon, Sep 25, 2017 at 08:26:21AM +, Noah Misch wrote:
>> On Tue, Sep 19, 2017 at 07:01:47PM -0700, Peter Geoghegan wrote:
>>> On Tue, Sep 19, 2017 at 5:52 PM, Peter Eisentraut
>>>  wrote:
 On 9/18/17 18:46, Peter Geoghegan wrote:
> As I pointed out a couple of times already [1], we don't currently
> sanitize ICU's BCP 47 language tags within CREATE COLLATION.

 There is no requirement that the locale strings for ICU need to be BCP
 47.  ICU locale names like 'de@collation=phonebook' are also acceptable.
>>>
>>> Right. But, we only document that BCP 47 is supported by Postgres.
>>> Maybe we can use get_icu_language_tag()/uloc_toLanguageTag() to ensure
>>> that we end up with BCP 47, even when the user happens to specify the
>>> legacy syntax. Should we be "canonicalizing" legacy ICU locale strings
>>> as BCP 47, too?
>>>
 The reason they are not validated is that, as you know, ICU accepts any
 locale string as valid.  You appear to have found a way to do some
 validation, but I would like to see that code.
>>>
>>> As I mentioned, I'm simply calling
>>> get_icu_language_tag()/uloc_toLanguageTag() to do that sanitization.
>>> The code to get the extra sanitization is completely trivial.
>>>
>>> I didn't post the patch that generates the errors in my opening e-mail
>>> because I'm not confident it's correct just yet. And, I think that I
>>> see a bigger problem: we pass a string that is almost certainly a BCP
>>> 47 string to ucol_open() from within pg_newlocale_from_collation(). We
>>> do so despite the fact that ucol_open() apparently doesn't accept BCP
>>> 47 syntax locale strings until ICU 54 [1]. Seems entirely possible
>>> that this accounts for the problems you saw on ICU 4.2, back when we
>>> were still creating keyword variants (I guess that the keyword
>>> variants seem very "BCP 47-ish" to me).
>>>
>>> I really think we need to add some kind of debug mode that makes ICU
>>> optionally spit out a locale display name at key points. We need this
>>> to gain confidence that the behavior that ICU provides actually
>>> matches what is expected across ICU different versions for different
>>> locale formats. We talked about this as a user-facing feature before,
>>> which can wait till v11; I just want this to debug problems like this
>>> one.
>>>
>>> [1] 
>>> https://ssl.icu-project.org/apiref/icu4c/ucol_8h.html#a4721e4c0a519bb0139a874e191223590
>>
>> [Action required within three days.  This is a generic notification.]
>>
>> The above-described topic is currently a PostgreSQL 10 open item.  Peter,
>> since you committed the patch believed to have created it, you own this open
>> item.  If some other commit is more relevant or if this does not belong as a
>> v10 open item, please let us know.  Otherwise, please observe the policy on
>> open item ownership[1] and send a status update within three calendar days of
>> this message.  Include a date for your subsequent status update.  Testers may
>> discover new open items at any time, and I want to plan to get them all fixed
>> well in advance of shipping v10.  Consequently, I will appreciate your 
>> efforts
>> toward speedy resolution.  Thanks.
>>
>> [1] 
>> https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com
> 
> This PostgreSQL 10 open item is past due for your status update.  On the worst
> week to be violating open item policies.  Kindly send a status update within
> 24 hours, and include a date for your subsequent status update.  Refer to the
> policy on open item ownership:
> https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com

I had previously replied that I think nothing should be changed.  What
process should be applied in that case?


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


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

Re: [HACKERS] Parallel Append implementation

[Amit Kapila]

On Sat, Sep 30, 2017 at 9:25 PM, Robert Haas  wrote:
> On Sat, Sep 30, 2017 at 12:20 AM, Amit Kapila  wrote:
>> Okay, but the point is whether it will make any difference
>> practically.  Let us try to see with an example, consider there are
>> two children (just taking two for simplicity, we can extend it to
>> many) and first having 1000 pages to scan and second having 900 pages
>> to scan, then it might not make much difference which child plan
>> leader chooses.  Now, it might matter if the first child relation has
>> 1000 pages to scan and second has just 1 page to scan, but not sure
>> how much difference will it be in practice considering that is almost
>> the maximum possible theoretical difference between two non-partial
>> paths (if we have pages greater than 1024 pages
>> (min_parallel_table_scan_size) then it will have a partial path).
>
> But that's comparing two non-partial paths for the same relation --
> the point here is to compare across relations.

Isn't it for both?  I mean it is about comparing the non-partial paths
for child relations of the same relation and also when there are
different relations involved as in Union All kind of query.  In any
case, the point I was trying to say is that generally non-partial
relations will have relatively smaller scan size, so probably should
take lesser time to complete.

>  Also keep in mind
> scenarios like this:
>
> SELECT ... FROM relation UNION ALL SELECT ... FROM generate_series(...);
>

I think for the FunctionScan case, non-partial paths can be quite costly.

>>> It's a lot fuzzier what is best when there are only partial plans.
>>>
>>
>> The point that bothers me a bit is whether it is a clear win if we
>> allow the leader to choose a different strategy to pick the paths or
>> is this just our theoretical assumption.  Basically, I think the patch
>> will become simpler if pick some simple strategy to choose paths.
>
> Well, that's true, but is it really that much complexity?
>
> And I actually don't see how this is very debatable.  If the only
> paths that are reasonably cheap are GIN index scans, then the only
> strategy is to dole them out across the processes you've got.  Giving
> the leader the cheapest one seems to be to be clearly smarter than any
> other strategy.
>

Sure, I think it is quite good if we can achieve that but it seems to
me that we will not be able to achieve that in all scenario's with the
patch and rather I think in some situations it can result in leader
ended up picking the costly plan (in case when there are all partial
plans or mix of partial and non-partial plans).  Now, we are ignoring
such cases based on the assumption that other workers might help to
complete master backend.  I think it is quite possible that the worker
backends picks up some plans which emit rows greater than tuple queue
size and they instead wait on the master backend which itself is busy
in completing its plan.  So master backend will end up taking too much
time.  If we want to go with a strategy of master (leader) backend and
workers taking a different strategy to pick paths to work on, then it
might be better if we should try to ensure that master backend always
starts from the place which has cheapest plans irrespective of whether
the path is partial or non-partial.

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


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

Re: [HACKERS] Re: CREATE COLLATION does not sanitize ICU's BCP 47 language tags. Should it?

[Peter Eisentraut]

On 9/30/17 15:28, Tom Lane wrote:
> This suggests to me that arguing about canonicalization is moot so
> far as avoiding reindexing goes: if you change ICU library versions,
> you're screwed and will have to jump through all the reindexing hoops,
> no matter what we do or don't do.

One reason for that is that the collation version also encodes things
like if the internal method for computing sort keys changes.

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


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

Re: [HACKERS] Re: CREATE COLLATION does not sanitize ICU's BCP 47 language tags. Should it?

[Peter Eisentraut]

On 9/30/17 15:28, Tom Lane wrote:
> Now, it may still be worthwhile to argue about whether canonicalization
> will help the other component of the problem, which is whether you can
> dump and reload CREATE COLLATION commands into a new ICU version and
> expect to get more-or-less-the-same behavior as before.

Arguably, you don't always want that.  Maybe you selected a locale name
like 'en-NEWCOUNTRY', and in old ICU versions you want that to fall back
to default 'en' behavior and in newer you get the updated custom behavior.

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


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

Re: [HACKERS] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple

[Alvaro Herrera]

Michael Paquier wrote:
> On Sun, Oct 1, 2017 at 10:25 AM, Peter Geoghegan  wrote:
> > On Thu, Sep 28, 2017 at 1:20 PM, Alvaro Herrera  
> > wrote:
> >> Maybe what this means is that we need to do both Dan's initially
> >> proposed patch (or something related to it) apart from the fixes already
> >> pushed.  IOW we need to put back some of the "tupkeep" business ...
> >
> > I took the time to specifically check if that would fix the problem.
> > Unfortunately, it did not. We see exactly the same problem, or at
> > least amcheck/REINDEX produces exactly the same error. I checked both
> > Dan's original update_freeze.patch, and your revision that retained
> > some of the "tupkeep" stuff,
> > 0002-Don-t-freeze-recently-dead-HOT-tuples, which you posted on
> > September 6th.
> 
> I did not take the time to dig into that more than two hours, but my
> first feeling is that some race condition is going on with the heap
> pruning.

I'll look into this on Monday.  I found out yesterday that the
problematic case is when HTSV returns HEAPTUPLE_DEAD and the HOT tests
return true.  I haven't yet figured if it is one of those specifically,
but I suspect it is the IsHotUpdated case.

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


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

Re: [HACKERS] CREATE COLLATION does not sanitize ICU's BCP 47 language tags. Should it?

[Peter Eisentraut]

On 9/29/17 19:38, Peter Geoghegan wrote:
> On Mon, Sep 25, 2017 at 11:40 AM, Peter Eisentraut
>  wrote:
>> Reading over this code again, it is admittedly not quite clear why this
>> "canonicalization" is in there right now.  I think it had something to
>> do with how we built the keyword variants at one point.  It might not
>> make sense.  I'd be glad to take that out and use the result straight
>> from uloc_getAvailable() for collcollate.  That is, after all, the
>> "canonical" version that ICU chooses to report to us.
> 
> Is anything going to happen here ahead of shipping v10? This remains
> an open item.

I'm still pondering committing my documentation patch I had posted, and
I've been reviewing your patches to see if there is anything else
documentation-wise that could be added.

As I had mentioned before, I disagree with the substance of the
functionality changes you are proposing, and I think it would be way too
late to make such significant changes.  The general community opinion
also seems to be in favor of letting things be.

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


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

Re: [HACKERS] list of credits for release notes

[Peter Eisentraut]

On 9/27/17 14:55, Dave Page wrote:
>> Attached is the proposed documentation commit as well as the raw list.
> At the very least my name is missing (I contributed the monitoring roles 
> patch and pg_ls_log/waldir. I have no idea if others are.
> 
> 
>> Thoughts on the heading?  I have considered "Credits",
>> "Acknowledgements", "Thanks", but the first seemed better than the other
>> ones
> I prefer Acknowledgments.

Committed with those changes.

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


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

Re: [HACKERS] 64-bit queryId?

[Greg Stark]

On 30 September 2017 at 21:03, Alexander Korotkov
 wrote:

> I heard from customers that they periodically dump contents of
> pg_stat_statements and then build statistics over long period of time.  If
> even they leave default pg_stat_statements.max unchanged, probability of
> collision would be significantly higher.

Indeed. It's simple enough to export stats to prometheus with queryid
as the key. Then even if the query ages out of the database stats you
have graphs and derivative metrics going further back.

I have to admit this was my first reaction to the idea of using sha1
hashes for git commits as well. But eventually I came around. If the
chances of a hash collision are smaller than a cosmic ray flipping a
bit or a digital electronics falling into a meta-stable state then I
had to admit there's not much value in being theoretically more
correct.

In practice if the query has aged out of pg_stat_statements and you're
exporting pg_stat_statements metrics to longer-term storage there's
really nothing more "correct" than just using a long enough hash and
assuming there are no collisions anyways. If 64-bits is still not
sufficient we could just go to 160-bit sha1 or 256-bit sha256.

Actually there's a reason I'm wondering if we shouldn't use a
cryptographic hash or even an HMAC. Currently if you're non-superuser
we, quite sensibly, hide the query text. But we also hide the queryid.
The latter is really inconvenient since it really makes the stats
utterly useless. I'm not sure what the rationale was but the only
thing I can think of is a fear that it's possible to reverse engineer
the query using brute force. An HMAC, or for most purposes even a
simple cryptographic hash with a secret salt would make that
impossible.

(I have other advances in pg_stat_statements I would love to see. It
would be so much more helpful if pg_stat_statements also kept a few
examples of query parameters such as the most recent set, the set that
caused the longest execution, maybe the set with the largest of each
metric.)

-- 
greg


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

Re: [HACKERS] why subplan is 10x faster then function?

[Pavel Stehule]

2017-09-30 23:23 GMT+02:00 Pavel Stehule :

> Hi
>
> I have some strange slow queries based on usage "view" functions
>
> one function looks like this:
>
> CREATE OR REPLACE FUNCTION ides_funcs.najdatsplt_cislo_exekuce(mid_najdatsplt
> bigint)
>  RETURNS character varying
>  LANGUAGE sql
>  STABLE
> AS $function$
> select CISLOEXEKUCE
>   from najzalobpr MT, najvzallok A1,
> NAJZALOBST A2, NAJZALOBCE A3 where
> MT.ID_NAJVZALLOK= A1.ID_NAJVZALLOK AND
> A1.ID_NAJZALOBST=A2.ID_NAJZALOBST AND
> A2.ID_NAJZALOBCE= A3.ID_NAJZALOBCE AND
> MT.ID_NAJDATSPLT = mID_NAJDATSPLT  LIMIT 1;
> $function$ cost 20
> ;
>
> I know so using this kind of functions is not good idea - it is customer
> old code generated from Oracle. I had idea about possible planner issues.
> But this is a executor issue.
>
> when this function is evaluated as function, then execution needs about 46
> sec
>
> ->  Nested Loop Left Join  (cost=0.71..780360.31 rows=589657
> width=2700) (actual time=47796.588..47796.588 rows=0 loops=1)
>   ->  Nested Loop  (cost=0.29..492947.20 rows=589657 width=2559)
> (actual time=47796.587..47796.587 rows=0 loops=1)
> ->  Seq Scan on najdatsplt mt  (cost=0.00..124359.24
> rows=1106096 width=1013) (actual time=47796.587..47796.587 rows=0 loops=1)
>   Filter: (najdatsplt_cislo_exekuce(id_najdatsplt) IS
> NOT NULL)
>   Rows Removed by Filter: 654
>
> When I use correlated subquery, then
>
>  ->  Nested Loop  (cost=0.29..19876820.11 rows=589657 width=2559) (actual
> time=3404.154..3404.154 rows=0 loops=1)
>   ->  Seq Scan on najdatsplt mt  (cost=0.00..19508232.15 rows=1106096
> width=1013) (actual time=3404.153..3404.153 rows=0 loops=1)
>   Filter: ((SubPlan 11) IS NOT NULL)
>   Rows Removed by Filter: 654
>   SubPlan 11
> ->  Limit  (cost=1.10..17.49 rows=1 width=144) (actual
> time=0.002..0.002 rows=0 loops=654)
>   ->  Nested Loop  (cost=1.10..17.49 rows=1 width=144) (actual
> time=0.002..0.002 rows=0 loops=654)
> ->  Nested Loop  (cost=0.83..17.02 rows=1 width=8)
> (actual time=0.002..0.002 rows=0 loops=654)
>   ->  Nested Loop  (cost=0.56..16.61 rows=1
> width=8) (actual time=0.002..0.002 rows=0 loops=654)
>
> The execution plan is +/- same - the bottleneck is in function execution
>
> Tested with same result on 9.6, 10.
>
> Is known overhead of function execution?
>
>
looks like this nested query are  expensive - some expensive operatiions
are pushed to exec_init_node. When the query are executed from function,
then exec_init_note is called too often




> Regards
>
> Pavel
>

Re: [HACKERS] why subplan is 10x faster then function?

[Pavel Stehule]

2017-10-01 12:45 GMT+02:00 Sokolov Yura :

> 1 октября 2017 г. 12:42:14 GMT+03:00, Pavel Stehule <
> pavel.steh...@gmail.com> пишет:
> >2017-09-30 23:23 GMT+02:00 Pavel Stehule :
> >
> >> Hi
> >>
> >> I have some strange slow queries based on usage "view" functions
> >>
> >> one function looks like this:
> >>
> >> CREATE OR REPLACE FUNCTION
> >ides_funcs.najdatsplt_cislo_exekuce(mid_najdatsplt
> >> bigint)
> >>  RETURNS character varying
> >>  LANGUAGE sql
> >>  STABLE
> >> AS $function$
> >> select CISLOEXEKUCE
> >>   from najzalobpr MT, najvzallok A1,
> >> NAJZALOBST A2, NAJZALOBCE A3 where
> >> MT.ID_NAJVZALLOK= A1.ID_NAJVZALLOK AND
> >> A1.ID_NAJZALOBST=A2.ID_NAJZALOBST AND
> >> A2.ID_NAJZALOBCE= A3.ID_NAJZALOBCE AND
> >> MT.ID_NAJDATSPLT = mID_NAJDATSPLT  LIMIT 1;
> >> $function$ cost 20
> >> ;
> >>
> >> I know so using this kind of functions is not good idea - it is
> >customer
> >> old code generated from Oracle. I had idea about possible planner
> >issues.
> >> But this is a executor issue.
> >>
> >> when this function is evaluated as function, then execution needs
> >about 46
> >> sec
> >>
> >> ->  Nested Loop Left Join  (cost=0.71..780360.31 rows=589657
> >> width=2700) (actual time=47796.588..47796.588 rows=0 loops=1)
> >>   ->  Nested Loop  (cost=0.29..492947.20 rows=589657
> >width=2559)
> >> (actual time=47796.587..47796.587 rows=0 loops=1)
> >> ->  Seq Scan on najdatsplt mt  (cost=0.00..124359.24
> >> rows=1106096 width=1013) (actual time=47796.587..47796.587 rows=0
> >loops=1)
> >>   Filter:
> >(najdatsplt_cislo_exekuce(id_najdatsplt) IS
> >> NOT NULL)
> >>   Rows Removed by Filter: 654
> >>
> >> When I use correlated subquery, then
> >>
> >>  ->  Nested Loop  (cost=0.29..19876820.11 rows=589657 width=2559)
> >(actual
> >> time=3404.154..3404.154 rows=0 loops=1)
> >>   ->  Seq Scan on najdatsplt mt  (cost=0.00..19508232.15 rows=1106096
> >> width=1013) (actual time=3404.153..3404.153 rows=0 loops=1)
> >>   Filter: ((SubPlan 11) IS NOT NULL)
> >>   Rows Removed by Filter: 654
> >>   SubPlan 11
> >> ->  Limit  (cost=1.10..17.49 rows=1 width=144) (actual
> >> time=0.002..0.002 rows=0 loops=654)
> >>   ->  Nested Loop  (cost=1.10..17.49 rows=1 width=144)
> >(actual
> >> time=0.002..0.002 rows=0 loops=654)
> >> ->  Nested Loop  (cost=0.83..17.02 rows=1
> >width=8)
> >> (actual time=0.002..0.002 rows=0 loops=654)
> >>   ->  Nested Loop  (cost=0.56..16.61 rows=1
> >> width=8) (actual time=0.002..0.002 rows=0 loops=654)
> >>
> >> The execution plan is +/- same - the bottleneck is in function
> >execution
> >>
> >> Tested with same result on 9.6, 10.
> >>
> >> Is known overhead of function execution?
> >>
> >>
> >profile of slow execution looks like
> >
> >+   24,71%24,40% 48235  postmaster  [.] SearchCatCache
> >+   14,25% 0,00% 0  postmaster  [unknown]   [.]
> >
> >+9,76% 9,65% 19071  postmaster  [.]
> >TupleDescInitEntry
> >+3,91% 3,86%  7625  postmaster  [.]
> >ExecAssignScanProjectionInfoWithVarno
> >+3,56% 3,52%  6955  postmaster  [.] AllocSetAlloc
> >+2,66% 2,63%  5193  postmaster  [.]
> >FunctionCall2Coll
> >+2,65% 2,62%  5183  postmaster  [.]
> >ResourceArrayRemove
> >+2,42% 2,39%  4719  postmaster  [.]
> >ExecTypeFromTLInternal
> >+2,21% 2,19%  4321  postmaster  [.]
> >DirectFunctionCall1Coll
> >+2,02% 2,00%  3961  postmaster  [.]
> >heap_getsysattr
> >+1,85% 1,82%  3604  postmaster  [.]
> >exprTypmod
> >+1,81% 1,79%  3540  postmaster  [.]
> >ResourceArrayAdd
> >+1,68% 1,66%  3282  postmaster  [.]
> >hash_uint32
> >+1,65% 1,63%  3214  postmaster  [.]
> >hash_search_with_hash_value
> >+1,64% 1,62%  3208  postmaster  [.]
> >CatalogCacheComputeHashValue
> >+1,28% 1,26%  2498  postmaster  [.]
> >MemoryContextAllocZeroAligned
> >+1,25% 1,24%  2446  postmaster  [.] palloc0
> >
> >Any ides why SearchCatCache is called too often?
> >
> >
> >
> >> Regards
> >>
> >> Pavel
> >>
>
> Looks like you've already collected profile with call-graph. So you can
> tell us where it were called from.
>

There was zero info. I'll try to install this database on my notebook, and
I'll see

Pavel


>
> With regards,
> --
> Sokolov Yura aka funny_falcon
>

Re: [HACKERS] why subplan is 10x faster then function?

[Sokolov Yura]

1 октября 2017 г. 12:42:14 GMT+03:00, Pavel Stehule  
пишет:
>2017-09-30 23:23 GMT+02:00 Pavel Stehule :
>
>> Hi
>>
>> I have some strange slow queries based on usage "view" functions
>>
>> one function looks like this:
>>
>> CREATE OR REPLACE FUNCTION
>ides_funcs.najdatsplt_cislo_exekuce(mid_najdatsplt
>> bigint)
>>  RETURNS character varying
>>  LANGUAGE sql
>>  STABLE
>> AS $function$
>> select CISLOEXEKUCE
>>   from najzalobpr MT, najvzallok A1,
>> NAJZALOBST A2, NAJZALOBCE A3 where
>> MT.ID_NAJVZALLOK= A1.ID_NAJVZALLOK AND
>> A1.ID_NAJZALOBST=A2.ID_NAJZALOBST AND
>> A2.ID_NAJZALOBCE= A3.ID_NAJZALOBCE AND
>> MT.ID_NAJDATSPLT = mID_NAJDATSPLT  LIMIT 1;
>> $function$ cost 20
>> ;
>>
>> I know so using this kind of functions is not good idea - it is
>customer
>> old code generated from Oracle. I had idea about possible planner
>issues.
>> But this is a executor issue.
>>
>> when this function is evaluated as function, then execution needs
>about 46
>> sec
>>
>> ->  Nested Loop Left Join  (cost=0.71..780360.31 rows=589657
>> width=2700) (actual time=47796.588..47796.588 rows=0 loops=1)
>>   ->  Nested Loop  (cost=0.29..492947.20 rows=589657
>width=2559)
>> (actual time=47796.587..47796.587 rows=0 loops=1)
>> ->  Seq Scan on najdatsplt mt  (cost=0.00..124359.24
>> rows=1106096 width=1013) (actual time=47796.587..47796.587 rows=0
>loops=1)
>>   Filter:
>(najdatsplt_cislo_exekuce(id_najdatsplt) IS
>> NOT NULL)
>>   Rows Removed by Filter: 654
>>
>> When I use correlated subquery, then
>>
>>  ->  Nested Loop  (cost=0.29..19876820.11 rows=589657 width=2559)
>(actual
>> time=3404.154..3404.154 rows=0 loops=1)
>>   ->  Seq Scan on najdatsplt mt  (cost=0.00..19508232.15 rows=1106096
>> width=1013) (actual time=3404.153..3404.153 rows=0 loops=1)
>>   Filter: ((SubPlan 11) IS NOT NULL)
>>   Rows Removed by Filter: 654
>>   SubPlan 11
>> ->  Limit  (cost=1.10..17.49 rows=1 width=144) (actual
>> time=0.002..0.002 rows=0 loops=654)
>>   ->  Nested Loop  (cost=1.10..17.49 rows=1 width=144)
>(actual
>> time=0.002..0.002 rows=0 loops=654)
>> ->  Nested Loop  (cost=0.83..17.02 rows=1
>width=8)
>> (actual time=0.002..0.002 rows=0 loops=654)
>>   ->  Nested Loop  (cost=0.56..16.61 rows=1
>> width=8) (actual time=0.002..0.002 rows=0 loops=654)
>>
>> The execution plan is +/- same - the bottleneck is in function
>execution
>>
>> Tested with same result on 9.6, 10.
>>
>> Is known overhead of function execution?
>>
>>
>profile of slow execution looks like
>
>+   24,71%24,40% 48235  postmaster  [.] SearchCatCache
>+   14,25% 0,00% 0  postmaster  [unknown]   [.]
>
>+9,76% 9,65% 19071  postmaster  [.]
>TupleDescInitEntry
>+3,91% 3,86%  7625  postmaster  [.]
>ExecAssignScanProjectionInfoWithVarno
>+3,56% 3,52%  6955  postmaster  [.] AllocSetAlloc
>+2,66% 2,63%  5193  postmaster  [.]
>FunctionCall2Coll
>+2,65% 2,62%  5183  postmaster  [.]
>ResourceArrayRemove
>+2,42% 2,39%  4719  postmaster  [.]
>ExecTypeFromTLInternal
>+2,21% 2,19%  4321  postmaster  [.]
>DirectFunctionCall1Coll
>+2,02% 2,00%  3961  postmaster  [.]
>heap_getsysattr
>+1,85% 1,82%  3604  postmaster  [.]
>exprTypmod
>+1,81% 1,79%  3540  postmaster  [.]
>ResourceArrayAdd
>+1,68% 1,66%  3282  postmaster  [.]
>hash_uint32
>+1,65% 1,63%  3214  postmaster  [.]
>hash_search_with_hash_value
>+1,64% 1,62%  3208  postmaster  [.]
>CatalogCacheComputeHashValue
>+1,28% 1,26%  2498  postmaster  [.]
>MemoryContextAllocZeroAligned
>+1,25% 1,24%  2446  postmaster  [.] palloc0
>
>Any ides why SearchCatCache is called too often?
>
>
>
>> Regards
>>
>> Pavel
>>

Looks like you've already collected profile with call-graph. So you can tell us 
where it were called from.


With regards,
--
Sokolov Yura aka funny_falcon


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

Re: [HACKERS] why subplan is 10x faster then function?

[Pavel Stehule]

2017-09-30 23:23 GMT+02:00 Pavel Stehule :

> Hi
>
> I have some strange slow queries based on usage "view" functions
>
> one function looks like this:
>
> CREATE OR REPLACE FUNCTION ides_funcs.najdatsplt_cislo_exekuce(mid_najdatsplt
> bigint)
>  RETURNS character varying
>  LANGUAGE sql
>  STABLE
> AS $function$
> select CISLOEXEKUCE
>   from najzalobpr MT, najvzallok A1,
> NAJZALOBST A2, NAJZALOBCE A3 where
> MT.ID_NAJVZALLOK= A1.ID_NAJVZALLOK AND
> A1.ID_NAJZALOBST=A2.ID_NAJZALOBST AND
> A2.ID_NAJZALOBCE= A3.ID_NAJZALOBCE AND
> MT.ID_NAJDATSPLT = mID_NAJDATSPLT  LIMIT 1;
> $function$ cost 20
> ;
>
> I know so using this kind of functions is not good idea - it is customer
> old code generated from Oracle. I had idea about possible planner issues.
> But this is a executor issue.
>
> when this function is evaluated as function, then execution needs about 46
> sec
>
> ->  Nested Loop Left Join  (cost=0.71..780360.31 rows=589657
> width=2700) (actual time=47796.588..47796.588 rows=0 loops=1)
>   ->  Nested Loop  (cost=0.29..492947.20 rows=589657 width=2559)
> (actual time=47796.587..47796.587 rows=0 loops=1)
> ->  Seq Scan on najdatsplt mt  (cost=0.00..124359.24
> rows=1106096 width=1013) (actual time=47796.587..47796.587 rows=0 loops=1)
>   Filter: (najdatsplt_cislo_exekuce(id_najdatsplt) IS
> NOT NULL)
>   Rows Removed by Filter: 654
>
> When I use correlated subquery, then
>
>  ->  Nested Loop  (cost=0.29..19876820.11 rows=589657 width=2559) (actual
> time=3404.154..3404.154 rows=0 loops=1)
>   ->  Seq Scan on najdatsplt mt  (cost=0.00..19508232.15 rows=1106096
> width=1013) (actual time=3404.153..3404.153 rows=0 loops=1)
>   Filter: ((SubPlan 11) IS NOT NULL)
>   Rows Removed by Filter: 654
>   SubPlan 11
> ->  Limit  (cost=1.10..17.49 rows=1 width=144) (actual
> time=0.002..0.002 rows=0 loops=654)
>   ->  Nested Loop  (cost=1.10..17.49 rows=1 width=144) (actual
> time=0.002..0.002 rows=0 loops=654)
> ->  Nested Loop  (cost=0.83..17.02 rows=1 width=8)
> (actual time=0.002..0.002 rows=0 loops=654)
>   ->  Nested Loop  (cost=0.56..16.61 rows=1
> width=8) (actual time=0.002..0.002 rows=0 loops=654)
>
> The execution plan is +/- same - the bottleneck is in function execution
>
> Tested with same result on 9.6, 10.
>
> Is known overhead of function execution?
>
>
profile of slow execution looks like

+   24,71%24,40% 48235  postmaster  [.] SearchCatCache
+   14,25% 0,00% 0  postmaster  [unknown]   [.]

+9,76% 9,65% 19071  postmaster  [.]
TupleDescInitEntry
+3,91% 3,86%  7625  postmaster  [.]
ExecAssignScanProjectionInfoWithVarno
+3,56% 3,52%  6955  postmaster  [.] AllocSetAlloc
+2,66% 2,63%  5193  postmaster  [.]
FunctionCall2Coll
+2,65% 2,62%  5183  postmaster  [.]
ResourceArrayRemove
+2,42% 2,39%  4719  postmaster  [.]
ExecTypeFromTLInternal
+2,21% 2,19%  4321  postmaster  [.]
DirectFunctionCall1Coll
+2,02% 2,00%  3961  postmaster  [.]
heap_getsysattr
+1,85% 1,82%  3604  postmaster  [.]
exprTypmod
+1,81% 1,79%  3540  postmaster  [.]
ResourceArrayAdd
+1,68% 1,66%  3282  postmaster  [.]
hash_uint32
+1,65% 1,63%  3214  postmaster  [.]
hash_search_with_hash_value
+1,64% 1,62%  3208  postmaster  [.]
CatalogCacheComputeHashValue
+1,28% 1,26%  2498  postmaster  [.]
MemoryContextAllocZeroAligned
+1,25% 1,24%  2446  postmaster  [.] palloc0

Any ides why SearchCatCache is called too often?



> Regards
>
> Pavel
>

Re: [HACKERS] GSoC 2017: weekly progress reports (week 6)

[Shubham Barai]

On 1 October 2017 at 01:47, Alexander Korotkov 
wrote:

> On Sat, Sep 30, 2017 at 6:12 PM, Shubham Barai 
> wrote:
>
>> I have made changes according to your suggestions. Please have a look at
>> the updated patch.
>> I am also considering your suggestions for my other patches also. But, I
>> will need some time to
>> make changes as I am currently busy doing my master's.
>>
>
> I don't understand why sometimes you call PredicateLockPage() only when
> fast update is off.  For example:
>
> @@ -94,6 +101,9 @@ scanPostingTree(Relation index, GinScanEntry scanEntry,
>>   break; /* no more pages */
>>
>>   buffer = ginStepRight(buffer, index, GIN_SHARE);
>> +
>> + if (!GinGetUseFastUpdate(index))
>> + PredicateLockPage(index, BufferGetBlockNumber(buffer), snapshot);
>>   }
>>
>>   UnlockReleaseBuffer(buffer);
>
>
> But sometimes you call PredicateLockPage() unconditionally.
>
> @@ -131,6 +141,8 @@ collectMatchBitmap(GinBtreeData *btree, GinBtreeStack
>> *stack,
>>   attnum = scanEntry->attnum;
>>   attr = btree->ginstate->origTupdesc->attrs[attnum - 1];
>>
>> + PredicateLockPage(btree->index, stack->buffer, snapshot);
>> +
>>   for (;;)
>>   {
>>   Page page;
>
>
> As I understand, all page-level predicate locking should happen only when
> fast update is off.
>
> Also, despite general idea is described in README-SSI, in-code comments
> would be useful.
>
>
Hi Alexander,

Yes, page-level predicate locking should happen only when fast update is
off.
Actually, I forgot to put conditions in updated patch. Does everything else
look ok ?




Kind Regards,
Shubham

>

Re: [HACKERS] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple

[Michael Paquier]

On Sun, Oct 1, 2017 at 10:25 AM, Peter Geoghegan  wrote:
> On Thu, Sep 28, 2017 at 1:20 PM, Alvaro Herrera  
> wrote:
>> Maybe what this means is that we need to do both Dan's initially
>> proposed patch (or something related to it) apart from the fixes already
>> pushed.  IOW we need to put back some of the "tupkeep" business ...
>
> I took the time to specifically check if that would fix the problem.
> Unfortunately, it did not. We see exactly the same problem, or at
> least amcheck/REINDEX produces exactly the same error. I checked both
> Dan's original update_freeze.patch, and your revision that retained
> some of the "tupkeep" stuff,
> 0002-Don-t-freeze-recently-dead-HOT-tuples, which you posted on
> September 6th.

I did not take the time to dig into that more than two hours, but my
first feeling is that some race condition is going on with the heap
pruning.
-- 
Michael


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

[HACKERS] show precise repos version for dev builds?

[Fabien COELHO]

Hello,

I use postgres "11devel" packages kindly distributed on 
"apt.postgresql.org" and recompiled every few hours.


I wanted to know which version it was, and "11devel" is kind of imprecise.

Ok, there is a hint in the deb package name:

11~~devel~20170930.2214-1~87.git2632bcc.pgdg16.04+1

But this information does not seem to be available from the executables 
themselves:


  sh> psql --version
  psql (PostgreSQL) 11devel

  sh> pgbench --version
  pgbench (PostgreSQL) 9.6.5


Some projects provide a repository or build date information:

  sh> clang --version
  clang version 6.0.0 (trunk 314585)

  sh> gcc --version
  gcc (GCC) 8.0.0 20170930 (experimental)

With some svn project I use "svnversion" which shows a summary of the 
status, eg "5432M" which tells that the sources are locally modified

from version 5432.

I would find it useful to have something like that in pg as well, and I 
have not found it available.


ISTM that extending the version name with the commit id and or date in 
some version output, eg "11devel [2632bcc 2017-09-30 ...]", would do it.


Thoughts?

--
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] pgbench - minor fix for meta command only scripts

[Heikki Linnakangas]

On 09/29/2017 08:43 PM, Fabien COELHO wrote:

reality.  So I don't know if this needs backpatching or not.  But it
should be fixed for v10, as there it becomes a demonstrably live issue.


Yes.


Patch looks good to me, so committed to master and v10. Thanks!

- Heikki


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