Re: [HACKERS] Reviewing freeze map code

[Masahiko Sawada]

On Sat, Jun 4, 2016 at 12:59 AM, Robert Haas  wrote:
> On Fri, Jun 3, 2016 at 11:21 AM, Masahiko Sawada  
> wrote:
>>> Can you submit that part as a separate patch?
>>
>> Attached.
>
> Thanks, committed.
>
 I'm address the review comment of 7087166 commit, and will post the patch.
>>>
>>> When?
>>
>> On Saturday.
>
> Great.  Will that address everything for this open item, then?
>

Attached patch for commit 7087166 on another mail.
I think that only the test tool for visibility map is remaining and
under the discussion.
Even if we have verification tool or function for visibility map, we
cannot repair the contents of visibility map if we turned out that
contents of visibility map is something wrong.
So I think we should have the way that re-generates the visibility map.
For this purpose, doing vacuum while ignoring visibility map by a new
option or new function is one idea.
But IMHO, it's not good idea to allow a function to do vacuum, and
expanding the VACUUM syntax might be somewhat overkill.

So other idea is to have GUC parameter, vacuum_even_frozen_page for example.
If this parameter is set true (false by default), we do vacuum whole
table forcibly and re-generate visibility map.
The advantage of this idea is that we don't necessary to expand VACUUM
syntax and relatively easily can remove this parameter if it's not
necessary anymore.

Thought?

Regards,

--
Masahiko Sawada


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

Re: [HACKERS] Reviewing freeze map code

[Masahiko Sawada]

On Sat, Jun 4, 2016 at 12:41 PM, Robert Haas  wrote:
> On Fri, Jun 3, 2016 at 10:25 PM, Masahiko Sawada  
> wrote:
 +   charnew_vmbuf[BLCKSZ];
 +   char   *new_cur = new_vmbuf;
 +   boolempty = true;
 +   boolold_lastpart;
 +
 +   /* Copy page header in advance */
 +   memcpy(new_vmbuf, , 
 SizeOfPageHeaderData);

 Shouldn't we zero out new_vmbuf? Afaics we're not necessarily zeroing it
 with old_lastpart && !empty, right?
>>>
>>> Oh, dear.  That seems like a possible data corruption bug.  Maybe we'd
>>> better fix that right away (although I don't actually have time before
>>> the wrap).
>
> Actually, on second thought, I'm not seeing the bug here.  It seems to
> me that the loop commented this way:
>
> /* Process old page bytes one by one, and turn it into new page. 
> */
>
> ...should always write to every byte in new_vmbuf, because we process
> exactly half the bytes in the old block at a time, and so that's going
> to generate exactly one full page of new bytes.  Am I missing
> something?

Yeah, you're right.
the rewriteVisibilityMap() always exactly writes whole new_vmbuf.

>
>> Since the force is always set true, I removed the force from argument
>> of copyFile() and rewriteVisibilityMap().
>> And destination file is always opened with O_RDWR, O_CREAT, O_TRUNC flags .
>
> I'm not happy with this.  I think we should always open with O_EXCL,
> because the new file is not expected to exist and if it does,
> something's probably broken.  I think we should default to the safe
> behavior (which is failing) rather than the unsafe behavior (which is
> clobbering data).

I specified O_EXCL instead of O_TRUNC.

Attached updated patch.

Regards,

--
Masahiko Sawada


fix_freeze_map_7087166_v2.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] [BUGS] BUG #14155: bloom index error with unlogged table

[Tom Lane]

Jeff Janes  writes:
> One thing from the commit-message:
> "On-disk, we can still store it in words, so as to not break on-disk
> compatibility with beta1."

> Hasn't that ship already sailed?

No.

> Or is the concern about intra-version pg_upgrade rather than direct
> on-disk compatibility?

This.  You can pg_upgrade across a catversion bump, but not across
changes in user table or index contents.

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] IPv6 link-local addresses and init data type

[Tom Lane]

Robert Haas  writes:
> Yeah, but what if somebody doesn't want to store scopes?

[ shrug... ]  We can invent a "strip_scope()" sort of function,
analogous to the weight-stripping function for tsvectors, or several
other examples.  That's a really lame excuse for not being *able*
to store scopes.

> ff::1%fourscoreandsevenyearsagoourforefathersbroughtforthonthiscontinentanewnationconceivedinlibertyanddedicatedtothepropositionthatlallmenarecreatedequal

I have not looked at the spec, but I wouldn't be surprised if there
were an upper limit on the length of valid scope names.

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] IPv6 link-local addresses and init data type

[Robert Haas]

On Fri, Jun 3, 2016 at 5:02 PM, Tom Lane  wrote:
> Markus Wanner  writes:
>> That leaves me wondering if it's really worth extending INET, though.
>> TEXT would be just fine to store a textual scope id. And it makes it
>> utterly clear that there's no magic involved.
>
> True, but it would force users to disassemble and reassemble the
> "address%scope" notation, which is kind of a pain.

Yeah, but what if somebody doesn't want to store scopes?
ff::1%fourscoreandsevenyearsagoourforefathersbroughtforthonthiscontinentanewnationconceivedinlibertyanddedicatedtothepropositionthatlallmenarecreatedequal

-- 
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] Reviewing freeze map code

[Robert Haas]

On Fri, Jun 3, 2016 at 10:25 PM, Masahiko Sawada  wrote:
>>> +   charnew_vmbuf[BLCKSZ];
>>> +   char   *new_cur = new_vmbuf;
>>> +   boolempty = true;
>>> +   boolold_lastpart;
>>> +
>>> +   /* Copy page header in advance */
>>> +   memcpy(new_vmbuf, , 
>>> SizeOfPageHeaderData);
>>>
>>> Shouldn't we zero out new_vmbuf? Afaics we're not necessarily zeroing it
>>> with old_lastpart && !empty, right?
>>
>> Oh, dear.  That seems like a possible data corruption bug.  Maybe we'd
>> better fix that right away (although I don't actually have time before
>> the wrap).

Actually, on second thought, I'm not seeing the bug here.  It seems to
me that the loop commented this way:

/* Process old page bytes one by one, and turn it into new page. */

...should always write to every byte in new_vmbuf, because we process
exactly half the bytes in the old block at a time, and so that's going
to generate exactly one full page of new bytes.  Am I missing
something?

> Since the force is always set true, I removed the force from argument
> of copyFile() and rewriteVisibilityMap().
> And destination file is always opened with O_RDWR, O_CREAT, O_TRUNC flags .

I'm not happy with this.  I think we should always open with O_EXCL,
because the new file is not expected to exist and if it does,
something's probably broken.  I think we should default to the safe
behavior (which is failing) rather than the unsafe behavior (which is
clobbering data).

(Status update for Noah: I expect Masahiko Sawada will respond
quickly, but if not I'll give some kind of update by Monday COB
anyhow.)

-- 
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] [BUGS] BUG #14155: bloom index error with unlogged table

[Jeff Janes]

On Thu, Jun 2, 2016 at 9:03 PM, Tom Lane  wrote:
> I wrote:
>> Jeff Janes  writes:
>>> My biggest gripe with it at the moment is that the signature size should be
>>> expressed in bits, and then internally rounded up to a multiple of 16,
>>> rather than having it be expressed in 'uint16'.
>>> If that were done it would be easier to fix the documentation to be more
>>> understandable.
>
>> +1 ... that sort of definition seems much more future-proof, too.
>> IMO it's not too late to change this.  (We probably don't want to change
>> the on-disk representation of the reloptions, but we could convert from
>> bits to words in bloptions().)
>
> There were no objections to this, but also no action.  Attached is a draft
> patch ... any complaints?

One thing from the commit-message:

"On-disk, we can still store it in words, so as to not break on-disk
compatibility with beta1."

Hasn't that ship already sailed?

from beta1 to HEAD:

The database cluster was initialized with CATALOG_VERSION_NO
201605051, but the server was compiled with CATALOG_VERSION_NO
201605191.

Or is the concern about intra-version pg_upgrade rather than direct
on-disk compatibility?

Cheers,

Jeff


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

Re: [HACKERS] [sqlsmith] Failed assertions on parallel worker shutdown

[Robert Haas]

On Thu, May 26, 2016 at 5:57 AM, Amit Kapila  wrote:
> On Tue, May 24, 2016 at 6:36 PM, Andreas Seltenreich 
> wrote:
>>
>>
>> Each of the sent plans was collected when a worker dumped core due to
>> the failed assertion.  More core dumps than plans were actually
>> observed, since with this failed assertion, multiple workers usually
>> trip on and dump core simultaneously.
>>
>> The following query corresponds to plan2:
>>
>> --8<---cut here---start->8---
>> select
>>   pg_catalog.pg_stat_get_bgwriter_requested_checkpoints() as c0,
>>   subq_0.c3 as c1, subq_0.c1 as c2, 31 as c3, 18 as c4,
>>   (select unique1 from public.bprime limit 1 offset 9) as c5,
>>   subq_0.c2 as c6
>> from
>> (select ref_0.tablename as c0, ref_0.inherited as c1,
>> ref_0.histogram_bounds as c2, 100 as c3
>>   from
>> pg_catalog.pg_stats as ref_0
>>   where 49 is not NULL limit 55) as subq_0
>> where true
>> limit 58;
>> --8<---cut here---end--->8---
>>
>
> I am able to reproduce the assertion (it occurs once in two to three times,
> but always at same place) you have reported upthread with the above query.
> It seems to me, issue here is that while workers are writing tuples in the
> tuple queue, the master backend has detached from the queues.  The reason
> why master backend has detached from tuple queues is because of Limit
> clause, basically after processing required tuples as specified by Limit
> clause, it calls shutdown of nodes in below part of code:

I can't reproduce this assertion failure on master.  I tried running
'make installcheck' and then running this query repeatedly in the
regression database with and without
parallel_setup_cost=parallel_tuple_cost=0, and got nowhere.  Does that
work for you, or do you have some other set of steps?

> I think the workers should stop processing tuples after the tuple queues got
> detached.  This will not only handle above situation gracefully, but will
> allow to speed up the queries where Limit clause is present on top of Gather
> node.  Patch for the same is attached with mail (this was part of original
> parallel seq scan patch, but was not applied and the reason as far as I
> remember was we thought such an optimization might not be required for
> initial version).

This is very likely a good idea, but...

> Another approach to fix this issue could be to reset mqh_partial_bytes and
> mqh_length_word_complete in shm_mq_sendv  in case of SHM_MQ_DETACHED.  These
> are currently reset only incase of success.

...I think we should do this too, because it's intended that calling
shm_mq_sendv again after it previously returned SHM_MQ_DETACHED should
again return SHM_MQ_DETACHED, not fail an assertion.  Can you see
whether the attached patch fixes this for you?

(Status update for Noah: I will provide another update regarding this
issue no later than Monday COB, US time.  I assume that Amit will have
responded by then, and it should hopefully be clear what the next step
is at that point.)

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


dont-fail-mq-assert-v1.patch
Description: binary/octet-stream

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

Re: [HACKERS] XTM & parallel search

[Amit Kapila]

On Fri, Jun 3, 2016 at 6:49 PM, Konstantin Knizhnik <
k.knizh...@postgrespro.ru> wrote:

>
>
> On 03.06.2016 16:05, Amit Kapila wrote:
>
> On Fri, Jun 3, 2016 at 1:34 AM, Konstantin Knizhnik <
> k.knizh...@postgrespro.ru> wrote:
>
>> We have to add three more functions to eXtensible Transaction Manager API
>> (XTM):
>>
>> /*
>>  * Calculate transaction state size. This method is invoked by
>> EstimateTransactionStateSpace to copy transaction
>>  * state to parallel workers
>>  */
>> size_t  (*GetTransactionStateSize)(void);
>>
>> /*
>>  * Serialize transaction state
>>  */
>> void(*SerializeTransactionState)(void* ctx);
>>
>> /*
>>  * Deserialize transaction state
>>  */
>> void(*DeserializeTransactionState)(void* ctx);
>>
>>
>
> In above proposal, are you suggesting to change the existing API's as
> well, because the parameters of function pointers don't match with exiting
> API's. I think it is better to consider this along with the overall XTM API.
>
>
> Sorry, but right now I have not replaced existed functions
>
> EstimateTransactionStateSpace/SerializeTransactionState/StartParallelWorkerTransaction
> with XTM indirect calls. If was my original intention, but these functions
> access static variable CurrentTransactionState defined in xact.c.
> So if  user-defined TM wants to override this functions, it will have to
> invoke original functions to save/restore CurrentTransactionState.
> It is not  convenient.
> This is why three XTM functions above are now called by existed xact
> funcations to save additional state, for example:
>
>
Sounds reasonable, although one might want to have a hook for
EndParallelWorkerTransaction as well.  I think here larger question is
whether there is a consensus on getting XTM API in core.


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

Re: [HACKERS] HashAggregate row estimate = 200

[David Rowley]

On 4 June 2016 at 14:10, Christophe Pettus  wrote:
> Something I've noticed frequently is that HashAggregate will, especially if 
> the children are Append with one of the nodes a non-seqscan, estimate 200 
> rows rather than a calculated vlaue.  Where is that value coming from?  The 
> statistics target, a hardwired constant, or something else?

Most likely from DEFAULT_NUM_DISTINCT in selfuncs.h. This is the
fallback, when nothing else can be used.

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


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

Re: [HACKERS] Reviewing freeze map code

[Masahiko Sawada]

On Sat, May 7, 2016 at 5:42 AM, Robert Haas  wrote:
> On Thu, May 5, 2016 at 2:20 PM, Andres Freund  wrote:
>> On 2016-05-02 14:48:18 -0700, Andres Freund wrote:
>>> 7087166 pg_upgrade: Convert old visibility map format to new format.
>>
>> +const char *
>> +rewriteVisibilityMap(const char *fromfile, const char *tofile, bool force)
>> ...
>>
>> +   while ((bytesRead = read(src_fd, buffer, BLCKSZ)) == BLCKSZ)
>> +   {
>> ..
>>
>> Uh, shouldn't we actually fail if we read incompletely? Rather than
>> silently ignoring the problem? Ok, this causes no corruption, but it
>> indicates that something went significantly wrong.
>
> Sure, that's reasonable.
>

Fixed.

>> +   charnew_vmbuf[BLCKSZ];
>> +   char   *new_cur = new_vmbuf;
>> +   boolempty = true;
>> +   boolold_lastpart;
>> +
>> +   /* Copy page header in advance */
>> +   memcpy(new_vmbuf, , SizeOfPageHeaderData);
>>
>> Shouldn't we zero out new_vmbuf? Afaics we're not necessarily zeroing it
>> with old_lastpart && !empty, right?
>
> Oh, dear.  That seems like a possible data corruption bug.  Maybe we'd
> better fix that right away (although I don't actually have time before
> the wrap).

Since the force is always set true, I removed the force from argument
of copyFile() and rewriteVisibilityMap().
And destination file is always opened with O_RDWR, O_CREAT, O_TRUNC flags .

>> +   if ((dst_fd = open(tofile, O_RDWR | O_CREAT | (force ? 0 : O_EXCL), 
>> S_IRUSR | S_IWUSR)) < 0)
>> +   {
>> +   close(src_fd);
>> +   return getErrorText();
>> +   }
>>
>> I know you guys copied this, but what's the force thing about?
>> Expecially as it's always set to true by the callers (i.e. what is the
>> parameter even about?)?  Wouldn't we at least have to specify O_TRUNC in
>> the force case?
>
> I just work here.
>
>> +   old_cur += BITS_PER_HEAPBLOCK_OLD;
>> +   new_cur += BITS_PER_HEAPBLOCK;
>>
>> I'm not sure I'm understanding the point of the BITS_PER_HEAPBLOCK_OLD
>> stuff - as long as it's hardcoded into rewriteVisibilityMap() we'll not
>> be able to have differing ones anyway, should we decide to add a third
>> bit?
>
> I think that's just a matter of style.

So this comments is not incorporated.

Attached patch, please review it.

Regards,

--
Masahiko Sawada


fix_freeze_map_7087166.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] Prepared statements and generic plans

[Bruce Momjian]

On Fri, Jun  3, 2016 at 08:27:38AM -0400, David G. Johnston wrote:
> ​This goes back to Bruce's motivation but as long as it goes into the 
> internals
> section I have no problem adding material that someone felt was worth their

OK, updated version attached.  I added "potential" to the first
paragraph, and added "estimated cost" to the later part, fixed the
"cheaper than", and clarified that we add the plan time cost to the
non-generic plan, which is how it can be cheaper than the generic plan. 
I also moved the "Once a generic plan is chosen" line.

Yeah, that's a lot of changes, but they all improved the text.  Thanks.

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

+ As you are, so once was I. As I am, so you will be. +
+ Ancient Roman grave inscription +
diff --git a/doc/src/sgml/ref/prepare.sgml b/doc/src/sgml/ref/prepare.sgml
new file mode 100644
index dbce8f2..af6c15e
*** a/doc/src/sgml/ref/prepare.sgml
--- b/doc/src/sgml/ref/prepare.sgml
*** PREPARE n
*** 70,80 

  

!Prepared statements have the largest performance advantage when a
!single session is being used to execute a large number of similar
 statements. The performance difference will be particularly
!significant if the statements are complex to plan or rewrite, for
!example, if the query involves a join of many tables or requires
 the application of several rules. If the statement is relatively simple
 to plan and rewrite but relatively expensive to execute, the
 performance advantage of prepared statements will be less noticeable.
--- 70,80 

  

!Prepared statements potentially have the largest performance advantage
!when a single session is being used to execute a large number of similar
 statements. The performance difference will be particularly
!significant if the statements are complex to plan or rewrite, e.g. 
!if the query involves a join of many tables or requires
 the application of several rules. If the statement is relatively simple
 to plan and rewrite but relatively expensive to execute, the
 performance advantage of prepared statements will be less noticeable.
*** PREPARE n
*** 127,140 
Notes
  

!If a prepared statement is executed enough times, the server may eventually
!decide to save and re-use a generic plan rather than re-planning each time.
!This will occur immediately if the prepared statement has no parameters;
!otherwise it occurs only if the generic plan appears to be not much more
!expensive than a plan that depends on specific parameter values.
!Typically, a generic plan will be selected only if the query's performance
!is estimated to be fairly insensitive to the specific parameter values
!supplied.

  

--- 127,151 
Notes
  

!Prepared statements can optionally use generic plans rather than
!re-planning with each set of supplied EXECUTE values.
!This occurs immediately for prepared statements with no parameters;
!otherwise it occurs only after five or more executions produce estimated
!plan costs, with planning overhead added, that are, on average, more
!expensive than the generic plan cost.  Once a generic plan is chosen,
!it is used for the remaining lifetime of the prepared statement.
!   
! 
!   
!A generic plan assumes each value supplied to EXECUTE
!is one of the column's distinct values and that column values are
!uniformly distributed.  For example, if statistics records three
!distinct column values, a generic plan assumes a column equality
!comparison will match 33% of processed rows.  Column statistics
!also allows generic plans to accurately compute the selectivity of
!unique columns.  Comparisons on non-uniformly-distributed columns and
!specification of non-existent values affects the average plan cost,
!and hence if and when a generic plan is chosen.

  

*** PREPARE n
*** 142,148 
 for a prepared statement, use .
 If a generic plan is in use, it will contain parameter symbols
 $n, while a custom plan will have the
!current actual parameter values substituted into it.

  

--- 153,161 
 for a prepared statement, use .
 If a generic plan is in use, it will contain parameter symbols
 $n, while a custom plan will have the
!supplied parameter values substituted into it.
!The row estimates in the generic plan reflect the selectivity
!computed for the parameters.

  


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

[HACKERS] HashAggregate row estimate = 200

[Christophe Pettus]

Something I've noticed frequently is that HashAggregate will, especially if the 
children are Append with one of the nodes a non-seqscan, estimate 200 rows 
rather than a calculated vlaue.  Where is that value coming from?  The 
statistics target, a hardwired constant, or something else?
--
-- Christophe Pettus
   x...@thebuild.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] Perf Benchmarking and regression.

[Andres Freund]

On 2016-06-03 20:41:33 -0400, Noah Misch wrote:
> Disabling just backend_flush_after by default works for me, so let's do that.
> Though I would not elect, on behalf of PostgreSQL, the risk of enabling
> {bgwriter,checkpoint,wal_writer}_flush_after by default, a reasonable person
> may choose to do so.  I doubt the community could acquire the data necessary
> to ascertain which choice has more utility.

Note that wal_writer_flush_after was essentially already enabled before,
just a lot more *aggressively*.


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

Re: [HACKERS] Perf Benchmarking and regression.

[Noah Misch]

On Fri, Jun 03, 2016 at 03:17:06PM -0400, Robert Haas wrote:
> On Fri, Jun 3, 2016 at 2:20 PM, Andres Freund  wrote:

> >> > I'm inclined to give up and disable backend_flush_after (not the rest),
> >> > because it's new and by far the "riskiest". But I do think it's a
> >> > disservice for the majority of our users.
> >>
> >> I think that's the right course of action.  I wasn't arguing for
> >> disabling either of the other two.
> >
> > Noah was...
> 
> I know, but I'm not Noah.  :-)
> 
> We have no evidence of the other settings causing any problems yet, so
> I see no reason to second-guess the decision to leave them on by
> default at this stage.  Other people may disagree with that analysis,
> and that's fine, but my analysis is that the case for
> disable-by-default has been made for backend_flush_after but not the
> others.  I also agree that backend_flush_after is much more dangerous
> on theoretical grounds; the checkpointer is in a good position to sort
> the requests to achieve locality, but backends are not.

Disabling just backend_flush_after by default works for me, so let's do that.
Though I would not elect, on behalf of PostgreSQL, the risk of enabling
{bgwriter,checkpoint,wal_writer}_flush_after by default, a reasonable person
may choose to do so.  I doubt the community could acquire the data necessary
to ascertain which choice has more utility.


-- 
Sent 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 safety tagging of extension functions

[Andreas Karlsson]

Hi,

Here is the patch split into many small patches as you suggested. The 
current patches are based on top of the patch which fixes the signatures 
for gin and gist functions.


These patches only touch functions which never should be called 
directly, so they are fine to skip. I decided to attach them anyway in 
case you fell like applying them.


parallel-contrib-v2-bloom.patch.gz
parallel-contrib-v2-btree_gin.patch.gz
parallel-contrib-v2-btree_gist.patch.gz
parallel-contrib-v2-dict_int.patch.gz
parallel-contrib-v2-dict_xsyn.patch.gz
parallel-contrib-v2-file_fdw.patch.gz
parallel-contrib-v2-hstore_plperl.patch.gz
parallel-contrib-v2-hstore_plpython.patch.gz
parallel-contrib-v2-ltree_plpython.patch.gz
parallel-contrib-v2-postgres_fdw.patch.gz
parallel-contrib-v2-tsm_system_rows.patch.gz
parallel-contrib-v2-tsm_system_time.patch.gz

These two mostly concern triggers so i guess they could be skipped too.

parallel-contrib-v2-tcn.patch.gz
parallel-contrib-v2-spi.patch.gz

Andreas


parallel-contrib-v2-adminpack.patch.gz
Description: application/gzip


parallel-contrib-v2-bloom.patch.gz
Description: application/gzip


parallel-contrib-v2-btree_gin.patch.gz
Description: application/gzip


parallel-contrib-v2-btree_gist.patch.gz
Description: application/gzip


parallel-contrib-v2-chkpass.patch.gz
Description: application/gzip


parallel-contrib-v2-citext.patch.gz
Description: application/gzip


parallel-contrib-v2-cube.patch.gz
Description: application/gzip


parallel-contrib-v2-dblink.patch.gz
Description: application/gzip


parallel-contrib-v2-dict_int.patch.gz
Description: application/gzip


parallel-contrib-v2-dict_xsyn.patch.gz
Description: application/gzip


parallel-contrib-v2-earthdistance.patch.gz
Description: application/gzip


parallel-contrib-v2-file_fdw.patch.gz
Description: application/gzip


parallel-contrib-v2-fuzzystrmatch.patch.gz
Description: application/gzip


parallel-contrib-v2-hstore.patch.gz
Description: application/gzip


parallel-contrib-v2-hstore_plperl.patch.gz
Description: application/gzip


parallel-contrib-v2-hstore_plpython.patch.gz
Description: application/gzip


parallel-contrib-v2-intagg.patch.gz
Description: application/gzip


parallel-contrib-v2-intarray.patch.gz
Description: application/gzip


parallel-contrib-v2-isn.patch.gz
Description: application/gzip


parallel-contrib-v2-lo.patch.gz
Description: application/gzip


parallel-contrib-v2-ltree.patch.gz
Description: application/gzip


parallel-contrib-v2-ltree_plpython.patch.gz
Description: application/gzip


parallel-contrib-v2-pageinspect.patch.gz
Description: application/gzip


parallel-contrib-v2-pg_buffercache.patch.gz
Description: application/gzip


parallel-contrib-v2-pgcrypto.patch.gz
Description: application/gzip


parallel-contrib-v2-pg_freespacemap.patch.gz
Description: application/gzip


parallel-contrib-v2-pg_prewarm.patch.gz
Description: application/gzip


parallel-contrib-v2-pgrowlocks.patch.gz
Description: application/gzip


parallel-contrib-v2-pg_stat_statements.patch.gz
Description: application/gzip


parallel-contrib-v2-pgstattuple.patch.gz
Description: application/gzip


parallel-contrib-v2-pg_trgm.patch.gz
Description: application/gzip


parallel-contrib-v2-pg_visibility.patch.gz
Description: application/gzip


parallel-contrib-v2-postgres_fdw.patch.gz
Description: application/gzip


parallel-contrib-v2-seg.patch.gz
Description: application/gzip


parallel-contrib-v2-spi.patch.gz
Description: application/gzip


parallel-contrib-v2-sslinfo.patch.gz
Description: application/gzip


parallel-contrib-v2-tablefunc.patch.gz
Description: application/gzip


parallel-contrib-v2-tcn.patch.gz
Description: application/gzip


parallel-contrib-v2-tsearch2.patch.gz
Description: application/gzip


parallel-contrib-v2-tsm_system_rows.patch.gz
Description: application/gzip


parallel-contrib-v2-tsm_system_time.patch.gz
Description: application/gzip


parallel-contrib-v2-unaccent.patch.gz
Description: application/gzip


parallel-contrib-v2-uuid-ossp.patch.gz
Description: application/gzip


parallel-contrib-v2-xml2.patch.gz
Description: application/gzip

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

Re: [HACKERS] Number of parentheses in check constraints affected by operator_precedence_warning

[Tom Lane]

Jean-Pierre Pelletier  writes:
> We noticed on PostgreSQL 9.5.3 that the number of parentheses in check
> constraints expressions
> vary depending on the setting of operator_precedence_warning.

Hmm, yeah, another place that needs to look through AEXPR_PAREN nodes :-(.
Fixed, thanks for the report!

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] Number of parentheses in check constraints affected by operator_precedence_warning

[Jean-Pierre Pelletier]

Hi,

We noticed on PostgreSQL 9.5.3 that the number of parentheses in check
constraints expressions
vary depending on the setting of operator_precedence_warning.

-- The following reproduces this issue which we first saw by restoring
into 9.5.3 a 9.4.8 dump
on two servers which had different operator_precedence_warning.

SET operator_precedence_warning = off; -- on would yield a different
number of parentheses

DROP TABLE IF EXISTS t1;
CREATE TABLE t1 (col1 INT, col2 INT, col3 INT, col4 INT);

-- 1) Check Constraints

ALTER TABLE t1
   ADD CONSTRAINT cc1 CHECK ((col1 IS NULL) OR (((col2 IS NULL) AND (col3
IS NULL)) AND (col4 IS NULL))),
   ADD CONSTRAINT cc2 CHECK (((col1 IS NOT NULL) OR (col2 IS NOT NULL)) OR
(col3 IS NOT NULL));


SELECT conname, consrc FROM pg_constraint WHERE conrelid = 't1'::regclass;

-- With  9.5.3 operator_precedence_warning = on (or pg 9.4)
-- cc1 ((col1 IS NULL) OR (((col2 IS NULL) AND (col3 IS NULL)) AND (col4
IS NULL)))
-- cc2 (((col1 IS NOT NULL) OR (col2 IS NOT NULL)) OR (col3 IS NOT NULL))

-- With 9.5.3 operator_precedence_warning = off
-- cc1 ((col1 IS NULL) OR ((col2 IS NULL) AND (col3 IS NULL) AND (col4 IS
NULL)))
-- cc2 ((col1 IS NOT NULL) OR (col2 IS NOT NULL) OR (col3 IS NOT NULL))


-- 2) WHEN condition on triggers also have this issue, for example:

DROP FUNCTION IF EXISTS f1();

CREATE FUNCTION f1() RETURNS trigger
LANGUAGE plpgsql
AS $$
BEGIN
END;
$$;

CREATE TRIGGER tr1 BEFORE UPDATE ON t1 FOR EACH ROW WHEN (new.col1 IS
DISTINCT FROM old.col1) OR (new.col2 IS DISTINCT FROM old.col2)) OR
((new.col3 IS NOT NULL) AND (new.col3 IS DISTINCT FROM old.col3))) OR
((new.col3 IS NULL) AND (new.col3 IS DISTINCT FROM old.col3 EXECUTE
PROCEDURE f1();

We got these results on Windows x64.

Thanks,
Jean-Pierre Pelletier


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

Re: [HACKERS] [GENERAL] Permission Denied Error on pg_xlog/RECOVERYXLOG file

[David Steele]

On 6/3/16 4:13 PM, Robert Haas wrote:
> On Fri, Jun 3, 2016 at 2:12 PM, Andres Freund  wrote:
>> On 2016-06-03 14:00:00 -0400, Robert Haas wrote:
>>> On Fri, May 27, 2016 at 8:44 PM, Andres Freund  wrote:
 I'm not convinced of that.  Hiding unexpected issues for longer, just to
 continue kind-of-operating, can make the impact of problems a lot worse,
 and it makes it very hard to actually learn about the issues.
>>>
>>> So if we made this a WARNING rather than an ERROR, it wouldn't hiding
>>> the issue, but it would be less likely to break things that worked
>>> before.  No?
>>
>> Except that we're then accepting the (proven!) potential for data
>> loss. We're talking about a single report of an restore_command setting
>> odd permissions. Which can easily be fixed.
> 
> Well, I think that having restore_command start failing after a minor
> release update can cause data loss, too.  Or even an outage.

I'm mostly with Andres on this but you do make a good point, Robert.

Andres, what if on EPERM you set write privs on the file and retry?
Maybe only back patch that change and for 9.6 expect restore_command
scripts to set sane permissions.

-- 
-David
da...@pgmasters.net


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

Re: [HACKERS] PostmasterPid not marked with PGDLLIMPORT

[Michael Paquier]

On Sat, Jun 4, 2016 at 2:56 AM, Robert Haas  wrote:
> On Wed, Jun 1, 2016 at 7:39 PM, Michael Paquier
>  wrote:
>>> In short, I'd vote for putting this change in HEAD, but I see no need to
>>> back-patch.
>>
>> OK, fine for me.
>
> Done.

Thanks you.
-- 
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] Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold

[Kevin Grittner]

On Fri, May 27, 2016 at 10:35 AM, Kevin Grittner  wrote:
> On Tue, May 24, 2016 at 4:10 PM, Robert Haas  wrote:

>> [ANALYZE of index with expression may fail to update statistics
>> if ANALYZE runs longer than old_snapshot_threshold]

>> If we can get away with it, it would be a rather large win to only set
>> a snapshot when the table has an expression index.  For purposes of
>> "snapshot too old", though, it will be important that a function in an
>> index which tries to read data from some other table which has been
>> pruned cancels itself when necessary.
>
> I will make this my top priority after resolving the question of whether
> there is an issue with CREATE INDEX.  I expect to have a resolution,
> probably involving some patch, by 3 June.

Due to 9.5 bug-fixing and the index issue taking a bit longer than
I expected, this is now looking like a 7 June resolution.

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


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

Re: [HACKERS] Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold

[Kevin Grittner]

On Fri, May 27, 2016 at 10:18 AM, Kevin Grittner  wrote:
> On Fri, May 27, 2016 at 9:58 AM, Kevin Grittner  wrote:
>> On Tue, May 24, 2016 at 4:56 PM, Andres Freund  wrote:
>
>>> If an old session with >= repeatable read accesses a clustered
>>> table (after the cluster committed), they'll now not see any
>>> errors, because all the LSNs look new.
>>
>> Again, it is new LSNs that trigger errors; if the page has not been
>> written recently the LSN is old and there is no error.  I think you
>> may be seeing problems based on getting the basics of this
>> backwards.
>
> I am reviewing the suggestion of a possible bug now, and will make
> it my top priority until resolved.  By the end of 1 June I will
> either have committed a fix or posted an explanation of why the
> concern is mistaken, with test results to demonstrate correct
> behavior.

This got set back by needing to fix a bug in the 9.5 release.  I am
back on this and have figured out that everyone who commented on
this specific issue was wrong about a very important fact -- the
LSNs in index pages after CREATE INDEX (with or without
CONCURRENTLY) and for REINDEX are always == InvalidXLogRecPtr (0).

That means that a snapshot from before an index build does not
always generate errors when it should on the use of the new index.
(Any early pruning/vacuuuming from before the index build is
missed; activity subsequent to the index build is recognized.)
Consequently, causing the index to be ignored in planning when
using the old index is not a nice optimization, but necessary for
correctness.  We already have logic to do this for other cases
(like HOT updates), so it is a matter of tying in to that existing
code correctly.  This won't be all that novel.

I now expect to push a fix along those lines by Tuesday, 6 June.

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


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

Re: [HACKERS] [sqlsmith] Failed assertion in parallel worker (ExecInitSubPlan)

[Robert Haas]

On Wed, Jun 1, 2016 at 9:35 PM, Noah Misch  wrote:
> [Action required within 72 hours.  This is a generic notification.]
>
> The above-described topic is currently a PostgreSQL 9.6 open item.  Robert,
> 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
> 9.6 open item, please let us know.  Otherwise, please observe the policy on
> open item ownership[1] and send a status update within 72 hours 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 9.6rc1.  Consequently, I will appreciate your
> efforts toward speedy resolution.  Thanks.

Status update:

I'm working on it.  I think there are several issues here, of which
I've committed a fix for just one.  I hope to have them all fixed by
next Friday.  If not, I'll send another update then.

-- 
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] IPv6 link-local addresses and init data type

[Tom Lane]

Markus Wanner  writes:
> That leaves me wondering if it's really worth extending INET, though.
> TEXT would be just fine to store a textual scope id. And it makes it
> utterly clear that there's no magic involved.

True, but it would force users to disassemble and reassemble the
"address%scope" notation, which is kind of a pain.

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] Changed SRF in targetlist handling

[Tom Lane]

"David G. Johnston"  writes:
> On Friday, June 3, 2016, Tom Lane  > wrote:
>> Merlin Moncure  writes:
>>> another interesting case today is:
>>> create sequence s;
>>> select generate_series(1,nextval('s')), generate_series(1,nextval('s'));

> If taking the 2.5 approach this one would fail as opposed to being
> rewritten.

Well, it'd be rewritten and then would fail at runtime because of the SRF
calls not producing the same number of rows.  But even option #3 would not
be strictly bug-compatible because it would (I imagine) evaluate the
arguments of each SRF only once.  The reason this case doesn't terminate
in the current implementation is that it re-evaluates the SRF arguments
each time we start a SRF over.  That's just weird ...

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] [sqlsmith] Failed assertion in parallel worker (ExecInitSubPlan)

[Robert Haas]

On Thu, May 12, 2016 at 2:07 PM, Tom Lane  wrote:
>> Err, wow.  That makes my head hurt.  Can you explain why this case
>> only arises for appendrel children, and not for plain rels?
>
> Well, plain rels only output Vars ;-)

Hmm.  Dilip's example in
https://www.postgresql.org/message-id/CAFiTN-vzg5BkK6kAh3OMhvgRu-uJvkjz47ybtopMAfGJp=z...@mail.gmail.com
seems to show that it's possible to end up with a targetlist
containing non-Vars even for a baserel.  In that example,
pull_up_subqueries() injects t2 into the parent query with a target
list containing two items, one of which is a PlaceHolderVar
referencing the subplan for t3.

Unfortunately, that makes it a bit hard to avoid wasting cycles here.
Amit's patch in
https://www.postgresql.org/message-id/CAA4eK1L-Uo=s4=0jvvva51pj06u5wddvsqg673yuxj_ja+x...@mail.gmail.com
is a bit brute force: it just checks the target list for every
relation for parallel-restricted constructs.  In most cases, no
subqueries having been pulled up, it will find none, but it'll still
have to walk the whole target list to figure that out.  If we had some
way of knowing that nothing was pulled up into the current rel, we
could avoid that.  This seems to apply equally to baserels and
appendrels; both will commonly have a target list containing only
Vars, but both can end up with non-Vars in the target list after
subqueries are pulled up.

-- 
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] Perf Benchmarking and regression.

[Andres Freund]

On 2016-06-03 15:17:06 -0400, Robert Haas wrote:
> On Fri, Jun 3, 2016 at 2:20 PM, Andres Freund  wrote:
> >> I've always heard that guideline as "roughly 1/4, but not more than
> >> about 8GB" - and the number of people with more than 32GB of RAM is
> >> going to just keep going up.
> >
> > I think that upper limit is wrong.  But even disregarding that:
>
> Many people think the upper limit should be even lower, based on good,
> practical experience.  Like I've seen plenty of people recommend
> 2-2.5GB.

Which largely imo is because of the writeback issue. And the locking
around buffer replacement, if you're doing it highly concurrently (which
is now mostly solved).


> > To hit the issue in that case you have to access more data than
> > shared_buffers (8GB), and very frequently re-dirty already dirtied
> > data. So you're basically (on a very rough approximation) going to have
> > to write more than 8GB within 30s (256MB/s).  Unless your hardware can
> > handle that many mostly random writes, you are likely to hit the worst
> > case behaviour of pending writeback piling up and stalls.
>
> I'm not entire sure that this is true, because my experience is that
> the background writing behavior under Linux is not very aggressive.  I
> agree you need a working set >8GB, but I think if you have that you
> might not actually need to write data this quickly, because if Linux
> decides to only do background writing (as opposed to blocking
> processes) it may not actually keep up.

But that's *bad*. Then a checkpoint comes around and latency and
throughput is shot to hell while the writeback from the fsyncs is
preventing any concurrent write activity. And if it's not keeping up
before, it's now really bad.


> And in fact I
> think what the testing shows so far is that when they can't achieve
> locality, backend flush control sucks.

FWIW, I don't think that's generally enough true. For pgbench
bigger-than-20%-of-avail-memory there's pretty much no locality, and
backend flushing helps considerably,

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] [GENERAL] Permission Denied Error on pg_xlog/RECOVERYXLOG file

[Robert Haas]

On Fri, Jun 3, 2016 at 2:12 PM, Andres Freund  wrote:
> On 2016-06-03 14:00:00 -0400, Robert Haas wrote:
>> On Fri, May 27, 2016 at 8:44 PM, Andres Freund  wrote:
>> > I'm not convinced of that.  Hiding unexpected issues for longer, just to
>> > continue kind-of-operating, can make the impact of problems a lot worse,
>> > and it makes it very hard to actually learn about the issues.
>>
>> So if we made this a WARNING rather than an ERROR, it wouldn't hiding
>> the issue, but it would be less likely to break things that worked
>> before.  No?
>
> Except that we're then accepting the (proven!) potential for data
> loss. We're talking about a single report of an restore_command setting
> odd permissions. Which can easily be fixed.

Well, I think that having restore_command start failing after a minor
release update can cause data loss, too.  Or even an outage.

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


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

[HACKERS] Changed SRF in targetlist handling

[David G. Johnston]

On Friday, June 3, 2016, Tom Lane > wrote:

> Merlin Moncure  writes:
> > On Wed, May 25, 2016 at 3:55 PM, Tom Lane  wrote:
> >> Andres Freund  writes:
> >>> If we go with rewriting this into LATERAL, I'd vote for 2.5 (trailed by
> >>> option 1), that'd keep most of the functionality, and would break
> >>> visibly rather than invisibly in the cases where not.
> >> 2.5 would be okay with me.
>
> > Curious if this approach will also rewrite:
> > select generate_series(1,generate_series(1,3)) s;
> > ...into
> > select s from generate_series(1,3) x cross join lateral
> generate_series(1,x) s;
>
> Yeah, that would be the idea.
>
>
Ok...  It's only a single srf as far as the outer query is concerned so
while it is odd the behavior is well defined and can be transformed while
giving the same result.


> > another interesting case today is:
> > create sequence s;
> > select generate_series(1,nextval('s')), generate_series(1,nextval('s'));
>
> > this statement never terminates.  a lateral rewrite of this query
> > would always terminate with much better defined and well understood
> > behaviors -- this is good.
>
> Interesting example demonstrating that 100% bug compatibility is not
> possible.  But as you say, most people would probably prefer the other
> behavior anyhow.
>
>
If taking the 2.5 approach this one would fail as opposed to being
rewritten.

This could be an exception to the policy in #3 and would be ok in #2.  It
would fail in #1.

Given the apparent general consensus for 2.5 and the lack of working field
versions of this form the error seems like a no brainer.

David J.

Re: [HACKERS] Changed SRF in targetlist handling

[Tom Lane]

Merlin Moncure  writes:
> On Wed, May 25, 2016 at 3:55 PM, Tom Lane  wrote:
>> Andres Freund  writes:
>>> If we go with rewriting this into LATERAL, I'd vote for 2.5 (trailed by
>>> option 1), that'd keep most of the functionality, and would break
>>> visibly rather than invisibly in the cases where not.
>> 2.5 would be okay with me.

> Curious if this approach will also rewrite:
> select generate_series(1,generate_series(1,3)) s;
> ...into
> select s from generate_series(1,3) x cross join lateral generate_series(1,x) 
> s;

Yeah, that would be the idea.

> another interesting case today is:
> create sequence s;
> select generate_series(1,nextval('s')), generate_series(1,nextval('s'));

> this statement never terminates.  a lateral rewrite of this query
> would always terminate with much better defined and well understood
> behaviors -- this is good.

Interesting example demonstrating that 100% bug compatibility is not
possible.  But as you say, most people would probably prefer the other
behavior anyhow.

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] Negators for operators

[Tom Lane]

Robert Haas  writes:
> On Fri, Jun 3, 2016 at 3:02 PM, David Fetter  wrote:
>> While constructing a somewhat hairy query with HAVING in it, I noticed
>> that a lot of operators don't have negators, which would have been
>> convenient for the class of queries I was constructing.  Further
>> investigation showed that while 380 of the built-in operators had
>> negators, 395 do not.

Really?

postgres=# select count(*) from pg_operator where oprresult = 'bool'::regtype 
and oprnegate = 0;
 count 
---
   135
(1 row)

The entire concept of negator is meaningless for operators that don't
return bool.

>> For some fraction I'll investigate if warranted, a negator makes no
>> sense.  For the rest, I'd like to propose adding negator operators
>> prefixed with '!', just as we have for the negators of regex-like
>> things.

> How about using NOT?

Indeed.

A quick look through the catalogs suggests that some fraction of these
cases may just be oversights, ie there is a suitable negator operator
but we just forgot to provide the link.  I'd be in favor of fixing those
by adding the links.  But in general, this proposal is going to mean
creating, and having to document, a lot of operators named like !|&>
... I seriously doubt that anyone's going to find that useful.

postgres=# select distinct oprname, obj_description(oid,'pg_operator') from 
pg_operator where oprresult = 'bool'::regtype and oprnegate = 0 order by 1;

 oprname | obj_description 
-+-
 &&  | overlaps
 &&  | overlaps (is subnet or supernet)
 &<  | overlaps or is left of
 &<| | overlaps or is below
 &>  | overlaps or is right of
 -|- | is adjacent to
 <   | less than
 <<  | contains
 <<  | is left of
 <<  | is subnet
 <<= | is subnet or equal
 <<| | is below
 <=  | less than or equal
  | is contained by
 <@  | is contained by
 <@  | lseg on line
 <@  | point inside box
 <@  | point on line
 <@  | point within closed path, or point on open path
 <^  | is below
 <^  | is below (allows touching)
 =   | equal
 =   | equal by area
 >   | greater than
 >=  | greater than or equal
 >>  | is right of
 >>  | is supernet
 >>= | is supernet or equal
 >^  | is above
 >^  | is above (allows touching)
 ?   | key exists
 ?#  | deprecated, use && instead
 ?#  | intersect
 ?&  | all keys exist
 ?-  | horizontal
 ?-  | horizontally aligned
 ?-| | perpendicular
 ?|  | any key exists
 ?|  | vertical
 ?|  | vertically aligned
 ?|| | parallel
 @   | deprecated, use <@ instead
 @>  | contains
 @@  | text search match
 @@@ | deprecated, use @@ instead
 |&> | overlaps or is above
 |>> | is above
 ~   | deprecated, use @> instead
 ~=  | same as
(49 rows)

Most of those look pretty "meh" to me.

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] Negators for operators

[Robert Haas]

On Fri, Jun 3, 2016 at 3:02 PM, David Fetter  wrote:
> While constructing a somewhat hairy query with HAVING in it, I noticed
> that a lot of operators don't have negators, which would have been
> convenient for the class of queries I was constructing.  Further
> investigation showed that while 380 of the built-in operators had
> negators, 395 do not.
>
> For some fraction I'll investigate if warranted, a negator makes no
> sense.  For the rest, I'd like to propose adding negator operators
> prefixed with '!', just as we have for the negators of regex-like
> things.
>
> What say?

How about using NOT?

-- 
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] Perf Benchmarking and regression.

[Robert Haas]

On Fri, Jun 3, 2016 at 2:20 PM, Andres Freund  wrote:
>> I've always heard that guideline as "roughly 1/4, but not more than
>> about 8GB" - and the number of people with more than 32GB of RAM is
>> going to just keep going up.
>
> I think that upper limit is wrong.  But even disregarding that:

Many people think the upper limit should be even lower, based on good,
practical experience.  Like I've seen plenty of people recommend
2-2.5GB.

> To hit the issue in that case you have to access more data than
> shared_buffers (8GB), and very frequently re-dirty already dirtied
> data. So you're basically (on a very rough approximation) going to have
> to write more than 8GB within 30s (256MB/s).  Unless your hardware can
> handle that many mostly random writes, you are likely to hit the worst
> case behaviour of pending writeback piling up and stalls.

I'm not entire sure that this is true, because my experience is that
the background writing behavior under Linux is not very aggressive.  I
agree you need a working set >8GB, but I think if you have that you
might not actually need to write data this quickly, because if Linux
decides to only do background writing (as opposed to blocking
processes) it may not actually keep up.

Also, 256MB/s is not actually all that crazy write rate.  I mean, it's
a lot, but even if each random UPDATE touched only 1 8kB block, that
would be about 32k TPS.  When you add in index updates and TOAST
traffic, the actual number of block writes per TPS could be
considerably higher, so we might be talking about something <10k TPS.
That's well within the range of what people try to do with PostgreSQL,
at least IME.

>> > I'm inclined to give up and disable backend_flush_after (not the rest),
>> > because it's new and by far the "riskiest". But I do think it's a
>> > disservice for the majority of our users.
>>
>> I think that's the right course of action.  I wasn't arguing for
>> disabling either of the other two.
>
> Noah was...

I know, but I'm not Noah.  :-)

We have no evidence of the other settings causing any problems yet, so
I see no reason to second-guess the decision to leave them on by
default at this stage.  Other people may disagree with that analysis,
and that's fine, but my analysis is that the case for
disable-by-default has been made for backend_flush_after but not the
others.  I also agree that backend_flush_after is much more dangerous
on theoretical grounds; the checkpointer is in a good position to sort
the requests to achieve locality, but backends are not.  And in fact I
think what the testing shows so far is that when they can't achieve
locality, backend flush control sucks.  When it can, it's neutral or
positive.  But I really see no reason to believe that that's likely to
be true on general workloads.

-- 
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] Change in order of criteria - reg

[David G. Johnston]

On Wed, Jun 1, 2016 at 12:07 AM, sri harsha  wrote:

>
> Hi,
>
> In PostgreSQL , does the order in which the criteria is given matter
> ?? For example
>
> Query 1 : Select * from TABLE where a > 5 and b < 10;
>
> Query 2 : Select * from TABLE where b <10 and a > 5;
>
> Are query 1 and query 2 the same in PostgreSQL or different ?? If its
> different , WHY ??
>
>
​Why are you asking?  Do you have any context in which you want to measure
"sameness"?

I
​ was thinking that pg_stat_statements might treat them differently but the
comparison there is object based, not string based, so these should end up
with the same hash.

​If everything is working correctly there will be no observable and
persistent difference between executing those exact two queries as far as
PostgreSQL is concerned. You might find a difference in the parse tree
representation where a > 5 is on the left leaf of a branch in one query but
on the right leaf in the other...

David J.

[HACKERS] Negators for operators

[David Fetter]

Folks,

While constructing a somewhat hairy query with HAVING in it, I noticed
that a lot of operators don't have negators, which would have been
convenient for the class of queries I was constructing.  Further
investigation showed that while 380 of the built-in operators had
negators, 395 do not.

For some fraction I'll investigate if warranted, a negator makes no
sense.  For the rest, I'd like to propose adding negator operators
prefixed with '!', just as we have for the negators of regex-like
things.

What say?

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


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

Re: [HACKERS] IPv6 link-local addresses and init data type

[Markus Wanner]

On 06/03/2016 06:55 PM, Tom Lane wrote:
> More importantly,
> on what basis do you conclude that the inet type will only be asked to
> store link-local addresses that are currently valid on the local machine?
> It is not very hard to think of applications where that wouldn't be the
> case.

That's a good point.

> I think a better plan is just to store the zone string verbatim.  It is
> not our job to check its validity or try to determine which spellings
> are equivalent.

Agreed.

That leaves me wondering if it's really worth extending INET, though.
TEXT would be just fine to store a textual scope id. And it makes it
utterly clear that there's no magic involved.

Kind Regards

Markus Wanner



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

Re: [HACKERS] PostmasterPid not marked with PGDLLIMPORT

[David G. Johnston]

On Fri, Jun 3, 2016 at 1:55 PM, Robert Haas  wrote:

> On Wed, Jun 1, 2016 at 5:59 PM, David G. Johnston
>  wrote:
> > Maybe I don't understand PGDLLEXPORT...
>
> We're talking about PGDLLIMPORT.
>

​Typo, was thinking "we export this for others to consume"...
​


>
> > The PostgreSQL function/feature in question is already in place and can
> be
> > accessed by someone using Linux or other unix-like variant.  But it
> cannot
> > be access by our Window's users because we failed to add a PGDLLEXPORT
> > somewhere.  If it is our goal to treat Windows and Linux/Unix equally
> then
> > that discrepancy is on its face a bug.  The fact we don't catch these
> until
> > some third-party points it out doesn't make it any less a bug.
>
> If we had a policy of putting PGDLLIMPORT on everything, I'd agree
> with you, but we clearly don't.  Something's only a bug if we intended
> A but accidentally got B.  If we intended and got A and somebody
> doesn't like that, that's not a bug; that's a difference of opinion.
>
>
​I find it a stretch that there is intent involved here.​  Your comments
below further that belief.

I personally feel that we should sprinkle PGDLLIMPORT markings onto a
> lot more things, but Tom Lane has opposed that at every turn.  I hope
> we'll change our policy about that someday, but that's a different
> question from whether such changes should be back-patched.
>
>
Different but related.

​I'm sure the opinion I've expressed has supporters but their isn't enough
bashing of us by the community at large that this is likely to get the
decision makers to switch their feelings.

David J.

Re: [HACKERS] [sqlsmith] Failed assertion in parallel worker (ExecInitSubPlan)

[Robert Haas]

On Sat, May 7, 2016 at 9:07 AM, Amit Kapila  wrote:
> From the above output it is clear that parallel restricted function is
> pushed down below gather node.  I found that though we have have care fully
> avoided to push pathtarget below GatherPath in apply_projection_to_path() if
> pathtarget contains any parallel unsafe or parallel restricted clause, but
> we are separately also trying to apply pathtarget to partialpath list which
> doesn't seem to be the correct way even if it is required.  I think this has
> been added during parallel aggregate patch and it seems to me this is not
> required after the changes related to GatherPath in
> apply_projection_to_path().

Good catch.  Committed.

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


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

Re: [HACKERS] Changed SRF in targetlist handling

[Merlin Moncure]

On Wed, May 25, 2016 at 3:55 PM, Tom Lane  wrote:
> Andres Freund  writes:
>> On 2016-05-25 15:20:03 -0400, Tom Lane wrote:
>>> We could certainly make a variant behavior in nodeFunctionscan.c that
>>> emulates that, if we feel that being exactly bug-compatible on the point
>>> is actually what we want.  I'm dubious about that though, not least
>>> because I don't think *anyone* actually believes that that behavior isn't
>>> broken.  Did you read my upthread message suggesting assorted compromise
>>> choices?
>
>> You mean 
>> https://www.postgresql.org/message-id/21076.1464034...@sss.pgh.pa.us ?
>> If so, yes.
>
>> If we go with rewriting this into LATERAL, I'd vote for 2.5 (trailed by
>> option 1), that'd keep most of the functionality, and would break
>> visibly rather than invisibly in the cases where not.
>
> 2.5 would be okay with me.
>
>> I guess you're not planning to work on this?
>
> Well, not right now, as it's clearly too late for 9.6.  I might hack on
> it later if nobody beats me to it.

Curious if this approach will also rewrite:
select generate_series(1,generate_series(1,3)) s;

...into
select s from generate_series(1,3) x cross join lateral generate_series(1,x) s;

another interesting case today is:
create sequence s;
select generate_series(1,nextval('s')), generate_series(1,nextval('s'));

this statement never terminates.  a lateral rewrite of this query
would always terminate with much better defined and well understood
behaviors -- this is good.

merlin


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

Re: [HACKERS] Perf Benchmarking and regression.

[Andres Freund]

On 2016-06-03 13:47:58 -0400, Robert Haas wrote:
> On Fri, Jun 3, 2016 at 1:43 PM, Andres Freund  wrote:
> >> I really don't get it.  There's nothing in any set of guidelines for
> >> setting shared_buffers that I've ever seen which would cause people to
> >> avoid this scenario.
> >
> > The "roughly 1/4" of memory guideline already mostly avoids it? It's
> > hard to constantly re-dirty a written-back page within 30s, before the
> > 10% (background)/20% (foreground) limits apply; if your shared buffers
> > are larger than the 10%/20% limits (which only apply to *available* not
> > total memory btw).
> 
> I've always heard that guideline as "roughly 1/4, but not more than
> about 8GB" - and the number of people with more than 32GB of RAM is
> going to just keep going up.

I think that upper limit is wrong.  But even disregarding that:

To hit the issue in that case you have to access more data than
shared_buffers (8GB), and very frequently re-dirty already dirtied
data. So you're basically (on a very rough approximation) going to have
to write more than 8GB within 30s (256MB/s).  Unless your hardware can
handle that many mostly random writes, you are likely to hit the worst
case behaviour of pending writeback piling up and stalls.


> > I'm inclined to give up and disable backend_flush_after (not the rest),
> > because it's new and by far the "riskiest". But I do think it's a
> > disservice for the majority of our users.
> 
> I think that's the right course of action.  I wasn't arguing for
> disabling either of the other two.

Noah was...

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] [GENERAL] Permission Denied Error on pg_xlog/RECOVERYXLOG file

[Andres Freund]

On 2016-06-03 14:00:00 -0400, Robert Haas wrote:
> On Fri, May 27, 2016 at 8:44 PM, Andres Freund  wrote:
> > I'm not convinced of that.  Hiding unexpected issues for longer, just to
> > continue kind-of-operating, can make the impact of problems a lot worse,
> > and it makes it very hard to actually learn about the issues.
> 
> So if we made this a WARNING rather than an ERROR, it wouldn't hiding
> the issue, but it would be less likely to break things that worked
> before.  No?

Except that we're then accepting the (proven!) potential for data
loss. We're talking about a single report of an restore_command setting
odd permissions. Which can easily be fixed.  And the permission setting
already has downsides, e.g. for the switch between archive and streaming
recovery (which would fail).

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] Change in order of criteria - reg

[Robert Haas]

On Wed, Jun 1, 2016 at 5:22 AM, Amit Langote
 wrote:
> On 2016/06/01 13:07, sri harsha wrote:
>> Hi,
>>
>> In PostgreSQL , does the order in which the criteria is given matter ??
>> For example
>>
>> Query 1 : Select * from TABLE where a > 5 and b < 10;
>>
>> Query 2 : Select * from TABLE where b <10 and a > 5;
>>
>> Are query 1 and query 2 the same in PostgreSQL or different ?? If its
>> different , WHY ??
>
> tl;dr they are the same.  As in they obviously produce the same result and
> result in invoking the same plan.
>
> Internally, optimizer will order application of those quals in resulting
> plan based on per-tuple cost of individual quals.  So a cheaper, more
> selective qual might result in short-circuiting of relatively expensive
> quals for a large number of rows in the table saving some cost in
> run-time.  Also, if index scan is chosen and quals pushed down, the
> underlying index method might know to order quals smartly.
>
> However, the cost-markings of operators/functions involved in quals better
> match reality.  By default, most operators/functions in a database are
> marked with cost of 1 unit.  Stable sorting used in ordering of quals
> would mean the order of applying quals in resulting plan matches the
> original order (ie, the order in which they appear in the query).  So, if
> the first specified qual really happens to be an expensive qual but marked
> as having the same cost as other less expensive quals, one would have to
> pay the price of evaluating it for all the rows.  Whereas, correctly
> marking the costs could have avoided that (as explained above).  Note that
> I am not suggesting that ordering quals in query by their perceived cost
> is the solution.  Keep optimizer informed by setting costs appropriately
> and it will do the right thing more often than not. :)

I think that if the costs are actually identical, the system will keep
the quals in the same order they were written - so then the order does
matter, a little bit.

-- 
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] [GENERAL] Permission Denied Error on pg_xlog/RECOVERYXLOG file

[Robert Haas]

On Fri, May 27, 2016 at 8:44 PM, Andres Freund  wrote:
> I'm not convinced of that.  Hiding unexpected issues for longer, just to
> continue kind-of-operating, can make the impact of problems a lot worse,
> and it makes it very hard to actually learn about the issues.

So if we made this a WARNING rather than an ERROR, it wouldn't hiding
the issue, but it would be less likely to break things that worked
before.  No?

-- 
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] PostmasterPid not marked with PGDLLIMPORT

[Robert Haas]

On Wed, Jun 1, 2016 at 7:39 PM, Michael Paquier
 wrote:
>> In short, I'd vote for putting this change in HEAD, but I see no need to
>> back-patch.
>
> OK, fine for me.

Done.

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


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

Re: [HACKERS] PostmasterPid not marked with PGDLLIMPORT

[Robert Haas]

On Wed, Jun 1, 2016 at 5:59 PM, David G. Johnston
 wrote:
> Maybe I don't understand PGDLLEXPORT...

We're talking about PGDLLIMPORT.

> The PostgreSQL function/feature in question is already in place and can be
> accessed by someone using Linux or other unix-like variant.  But it cannot
> be access by our Window's users because we failed to add a PGDLLEXPORT
> somewhere.  If it is our goal to treat Windows and Linux/Unix equally then
> that discrepancy is on its face a bug.  The fact we don't catch these until
> some third-party points it out doesn't make it any less a bug.

If we had a policy of putting PGDLLIMPORT on everything, I'd agree
with you, but we clearly don't.  Something's only a bug if we intended
A but accidentally got B.  If we intended and got A and somebody
doesn't like that, that's not a bug; that's a difference of opinion.

I personally feel that we should sprinkle PGDLLIMPORT markings onto a
lot more things, but Tom Lane has opposed that at every turn.  I hope
we'll change our policy about that someday, but that's a different
question from whether such changes should be back-patched.

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


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

Re: [HACKERS] Perf Benchmarking and regression.

[Robert Haas]

On Fri, Jun 3, 2016 at 1:43 PM, Andres Freund  wrote:
>> I really don't get it.  There's nothing in any set of guidelines for
>> setting shared_buffers that I've ever seen which would cause people to
>> avoid this scenario.
>
> The "roughly 1/4" of memory guideline already mostly avoids it? It's
> hard to constantly re-dirty a written-back page within 30s, before the
> 10% (background)/20% (foreground) limits apply; if your shared buffers
> are larger than the 10%/20% limits (which only apply to *available* not
> total memory btw).

I've always heard that guideline as "roughly 1/4, but not more than
about 8GB" - and the number of people with more than 32GB of RAM is
going to just keep going up.

>> You're the first person I've ever heard describe this as a
>> misconfiguration.
>
> Huh? People tried addressing this problem for *years* with bigger /
> smaller shared buffers, but couldn't easily.

I'm saying that setting 8GB of shared_buffers on a system with
lotsamem is not widely regarded as misconfiguration.

> I'm inclined to give up and disable backend_flush_after (not the rest),
> because it's new and by far the "riskiest". But I do think it's a
> disservice for the majority of our users.

I think that's the right course of action.  I wasn't arguing for
disabling either of the other two.

-- 
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] Perf Benchmarking and regression.

[Andres Freund]

On 2016-06-03 13:42:09 -0400, Tom Lane wrote:
> Robert Haas  writes:
> > On Fri, Jun 3, 2016 at 12:39 PM, Andres Freund  wrote:
> >> Note that other operating systems like windows and freebsd *alreaddy*
> >> write back much more aggressively (independent of this change). I seem
> >> to recall you yourself being quite passionately arguing that the linux
> >> behaviour around this is broken.
> 
> > Sure, but being unhappy about the Linux behavior doesn't mean that I
> > want our TPS on Linux to go down.  Whether I like the behavior or not,
> > we have to live with it.
> 
> Yeah.  Bug or not, it's reality for lots of our users.

That means we need to address it. Which is what the feature does. So
yes, some linux specific tuning might need to be tweaked in the more
extreme cases. But that's better than relying on linux' extreme
writeback behaviour, which changes every few releases to boot. From the
tuning side this makes shared buffer sizing more common between unixoid
OSs.

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] array of domain types

[Jim Nasby]

On 6/3/16 11:50 AM, Konstantin Knizhnik wrote:

Originally I was mostly interested in domains as kind of typedefs:
convenient way to assign type to some particular kind of columns,
for example object reference used in ORM.
There are two main goals of using domain here:
1. Be able to easily change representation of object identifier, for
example from integer to bigint.
2. Detect all columns containing references (distinguish them from
columns containing just normal integers).
I do not see any other mechanism in PostgreSQL which can address this
problem (for example user defined type can not help here).

I wonder if it is possible to support arrays of domain which do not have
constraints?
Or such partial support is worser than prohibiting arrays of domains at all?


I don't know that domains without constraints gets you terribly much. At 
that point you could just create a brand new type using all the existing 
infrastructure (though admittedly that's a LOT more work than CREATE 
DOMAIN).


I definitely think that domains should work the way you're envisioning. 
To me, they should be the exact same thing as any other type, except 
that they have constraints attached and a different named. You should be 
able to use them everywhere and in every way that you currently use a 
type. Ideally you'd even be able to create casts against them.


I'm not suggesting you try and fix all those things at once, but I don't 
think we should add only partial support for arrays of domains. If you 
can have a domain array, it should work exactly how you'd expect, 
including all of the constraint checking.


Before focusing further on the code, I think you should focus on adding 
appropriate regression tests to make sure things work correctly. I'm not 
sure what's currently tested, but what comes to mind is making certain 
that constraints work with a domain array when used both by themselves 
and as part of a composite type:


- as an argument to a function
- inside a sql function
- as a plpgsql variable
- inside a plpgsql function
- as a table column

So that's 5 x 2 (once for domain[], once for create type blah(x 
domain[])) test cases. There might be some other cases that are missing 
(what cast testing needs to happen?)

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)   mobile: 512-569-9461


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

Re: [HACKERS] Perf Benchmarking and regression.

[Andres Freund]

On 2016-06-03 13:33:31 -0400, Robert Haas wrote:
> On Fri, Jun 3, 2016 at 12:39 PM, Andres Freund  wrote:
> > On 2016-06-03 12:31:58 -0400, Robert Haas wrote:
> >> Now, what varies IME is how much total RAM there is in the system and
> >> how frequently they write that data, as opposed to reading it.  If
> >> they are on a tightly RAM-constrained system, then this situation
> >> won't arise because they won't be under the dirty background limit.
> >> And if they aren't writing that much data then they'll be fine too.
> >> But even putting all of that together I really don't see why you're
> >> trying to suggest that this is some bizarre set of circumstances that
> >> should only rarely happen in the real world.
> >
> > I'm saying that if that happens constantly, you're better off adjusting
> > shared_buffers, because you're likely already suffering from latency
> > spikes and other issues. Optimizing for massive random write throughput
> > in a system that's not configured appropriately, at the cost of well
> > configured systems to suffer, doesn't seem like a good tradeoff to me.
> 
> I really don't get it.  There's nothing in any set of guidelines for
> setting shared_buffers that I've ever seen which would cause people to
> avoid this scenario.

The "roughly 1/4" of memory guideline already mostly avoids it? It's
hard to constantly re-dirty a written-back page within 30s, before the
10% (background)/20% (foreground) limits apply; if your shared buffers
are larger than the 10%/20% limits (which only apply to *available* not
total memory btw).


> You're the first person I've ever heard describe this as a
> misconfiguration.

Huh? People tried addressing this problem for *years* with bigger /
smaller shared buffers, but couldn't easily.

I'm inclined to give up and disable backend_flush_after (not the rest),
because it's new and by far the "riskiest". But I do think it's a
disservice for the majority of our users.

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] Perf Benchmarking and regression.

[Tom Lane]

Robert Haas  writes:
> On Fri, Jun 3, 2016 at 12:39 PM, Andres Freund  wrote:
>> Note that other operating systems like windows and freebsd *alreaddy*
>> write back much more aggressively (independent of this change). I seem
>> to recall you yourself being quite passionately arguing that the linux
>> behaviour around this is broken.

> Sure, but being unhappy about the Linux behavior doesn't mean that I
> want our TPS on Linux to go down.  Whether I like the behavior or not,
> we have to live with it.

Yeah.  Bug or not, it's reality for lots of our users.

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] Tracking wait event for latches

[Robert Haas]

On Thu, Jun 2, 2016 at 1:34 AM, Michael Paquier
 wrote:
> This patch is shaped this way intentionally based on the feedback I
> received at PGCon (Robert and others). We could provide a routine that
> extensions call in _PG_init to register a new latch event name in
> shared memory, but I didn't see much use in doing so, take for example
> the case of background worker, it is possible to register a custom
> string for pg_stat_activity via pgstat_report_activity. One could take
> advantage of having custom latch wait names in shared memory if an
> extension has wait points with latches though... But I am still not
> sure if that's worth the complexity.

I can't see how you could ever guarantee that it wouldn't just fail.
We allocate a certain amount of "slop" in the main shared memory
segment, but it's not infinite and can certainly be exhausted.  It
seems like it would suck if you tried to load your extension and it
failed because there was no room left for more wait-point names.
Maybe it would suck less than not having wait-point names, but I'm not
really sure.  I think we'd do better to get something that handles the
core stuff well and then consider extensions later or not at all.  It
only matters if you're running multiple extensions that all use LWLock
tranches and you need to distinguish between waits on their various
LWLocks.  But since LWLock contention is something we hope will be
infrequent I'm just not sure that case is common enough to justify
building a lot of mechanism.

-- 
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] Perf Benchmarking and regression.

[Robert Haas]

On Fri, Jun 3, 2016 at 12:39 PM, Andres Freund  wrote:
> On 2016-06-03 12:31:58 -0400, Robert Haas wrote:
>> Now, what varies IME is how much total RAM there is in the system and
>> how frequently they write that data, as opposed to reading it.  If
>> they are on a tightly RAM-constrained system, then this situation
>> won't arise because they won't be under the dirty background limit.
>> And if they aren't writing that much data then they'll be fine too.
>> But even putting all of that together I really don't see why you're
>> trying to suggest that this is some bizarre set of circumstances that
>> should only rarely happen in the real world.
>
> I'm saying that if that happens constantly, you're better off adjusting
> shared_buffers, because you're likely already suffering from latency
> spikes and other issues. Optimizing for massive random write throughput
> in a system that's not configured appropriately, at the cost of well
> configured systems to suffer, doesn't seem like a good tradeoff to me.

I really don't get it.  There's nothing in any set of guidelines for
setting shared_buffers that I've ever seen which would cause people to
avoid this scenario.  You're the first person I've ever heard describe
this as a misconfiguration.

> Note that other operating systems like windows and freebsd *alreaddy*
> write back much more aggressively (independent of this change). I seem
> to recall you yourself being quite passionately arguing that the linux
> behaviour around this is broken.

Sure, but being unhappy about the Linux behavior doesn't mean that I
want our TPS on Linux to go down.  Whether I like the behavior or not,
we have to live with it.

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


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

Re: [HACKERS] [DOC][PATCH]bloom index options limits

[Tom Lane]

Nikolay Shaplov  writes:
> Hi! I've changed limits of the length option in the "Introduction" section. 
> They should be 1 ≤ colN ≤ 2048 instead of 1 < colN < 2048 (I've talk with 
> the 
> author about it)

I rewrote that text this morning, so this patch won't apply anymore...

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] IPv6 link-local addresses and init data type

[Tom Lane]

Markus Wanner  writes:
> Considering that Postgres is not unlikely to write INET types to
> permanent storage, its values should better survive a reboot. And while
> I have some doubts about persistence of interface names, those clearly
> have a higher chance of surviving a reboot compared to interface
> indices. Therefore, I'd advocate resolving interface indices (if given)
> to interface names using if_indextoname(3) and let INET types store only
> names.

What will you do on machines without if_indextoname()?  More importantly,
on what basis do you conclude that the inet type will only be asked to
store link-local addresses that are currently valid on the local machine?
It is not very hard to think of applications where that wouldn't be the
case.

I think a better plan is just to store the zone string verbatim.  It is
not our job to check its validity or try to determine which spellings
are equivalent.

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] array of domain types

[Konstantin Knizhnik]

On 03.06.2016 02:02, Rod Taylor wrote:



On Thu, Jun 2, 2016 at 10:42 AM, Konstantin Knizhnik 
> wrote:


On 02.06.2016 17:22, Tom Lane wrote:

konstantin knizhnik > writes:

Attached please find patch for DefineDomain function.

You didn't attach the patch,


Sorry, but I did attached the patch - I see the attachment in my
mail received from the group.
Multidimensional arrays work fine:

knizhnik=# SELECT '{{14},{20}}'::teenager[][];
ERROR:  value for domain teenager violates check constraint
"teenager_check"
LINE 1: SELECT '{{14},{20}}'::teenager[][];
   ^
knizhnik=# SELECT '{{14},{19}}'::teenager[][];
  teenager
-
 {{14},{19}}
(1 row)

knizhnik=# SELECT ('{{14},{19}}'::teenager[][])[1][1];
 teenager
--
   14
(1 row)


Domain of array of domain also works:


I applied the domain.patch from above on HEAD, and all I get is cache 
lookup failures. The type_sanity regression test fails too.


postgres=# CREATE DOMAIN teenager AS int CHECK (VALUE BETWEEN 13 AND 20);
CREATE DOMAIN
postgres=# CREATE DOMAIN teenager_groups AS teenager[];
CREATE DOMAIN
postgres=# CREATE TABLE x (col teenager_groups);
ERROR:  cache lookup failed for type 0


Anyway, if that worked for me I would have done this which I expect 
will succeed when it shouldn't.


INSERT INTO x VALUES (ARRAY[13,14,20]);
ALTER DOMAIN teenager DROP CONSTRAINT teenager_check;
ALTER DOMAIN teenager ADD CHECK (VALUE BETWEEN 13 AND 19);



Sorry, the problem is more difficult than I originally expected:(
Attached patch passes all regression tests and correctly handle 
conversion of arrays.
But constraints are not checked for table columns. I failed to locate 
place where this check should be inserted...


Originally I was mostly interested in domains as kind of typedefs: 
convenient way to assign type to some particular kind of columns,

for example object reference used in ORM.
There are two main goals of using domain here:
1. Be able to easily change representation of object identifier, for 
example from integer to bigint.
2. Detect all columns containing references (distinguish them from 
columns containing just normal integers).
I do not see any other mechanism in PostgreSQL which can address this 
problem (for example user defined type can not help here).


I wonder if it is possible to support arrays of domain which do not have 
constraints?

Or such partial support is worser than prohibiting arrays of domains at all?

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

diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index 71d4df9..e227fa8 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -752,11 +752,13 @@ DefineDomain(CreateDomainStmt *stmt)
 	ListCell   *listptr;
 	Oid			basetypeoid;
 	Oid			old_type_oid;
+	Oid array_oid;
 	Oid			domaincoll;
 	Form_pg_type baseType;
 	int32		basetypeMod;
 	Oid			baseColl;
 	ObjectAddress address;
+	char   *array_type;
 
 	/* Convert list of names to a name and namespace */
 	domainNamespace = QualifiedNameGetCreationNamespace(stmt->domainname,
@@ -1023,6 +1025,14 @@ DefineDomain(CreateDomainStmt *stmt)
 		}
 	}
 
+
+	/*
+	 * OK, we're done checking, time to make the type.  We must assign the
+	 * array type OID ahead of calling TypeCreate, since the base type and
+	 * array type each refer to the other.
+	 */
+	array_oid = AssignTypeArrayOid();
+
 	/*
 	 * Have TypeCreate do all the real work.
 	 */
@@ -1047,7 +1057,7 @@ DefineDomain(CreateDomainStmt *stmt)
    analyzeProcedure,	/* analyze procedure */
    InvalidOid,	/* no array element type */
    false,		/* this isn't an array */
-   InvalidOid,	/* no arrays for domains (yet) */
+   array_oid,	/* array type we are about to create */
    basetypeoid, /* base type ID */
    defaultValue,	/* default type value (text) */
    defaultValueBin,		/* default type value (binary) */
@@ -1060,6 +1070,48 @@ DefineDomain(CreateDomainStmt *stmt)
    domaincoll); /* type's collation */
 
 	/*
+	 * Create the array type that goes with it.
+	 */
+	array_type = makeArrayTypeName(domainName, domainNamespace);
+
+/* alignment must be 'i' or 'd' for arrays */
+	alignment = (alignment == 'd') ? 'd' : 'i';
+
+	TypeCreate(array_oid,/* force assignment of this type OID */
+			   array_type,/* type name */
+			   domainNamespace,/* namespace */
+			   InvalidOid,/* relation oid (n/a here) */
+			   0,/* relation kind (ditto) */
+			   GetUserId(),/* owner's ID */
+			   -1,/* internal size (always varlena) */
+			   TYPTYPE_BASE,/* type-type (base type) */
+			   TYPCATEGORY_ARRAY,/* type-category (array) */
+			   false,/* array types are never preferred 

Re: [HACKERS] Perf Benchmarking and regression.

[Andres Freund]

On 2016-06-03 12:31:58 -0400, Robert Haas wrote:
> Now, what varies IME is how much total RAM there is in the system and
> how frequently they write that data, as opposed to reading it.  If
> they are on a tightly RAM-constrained system, then this situation
> won't arise because they won't be under the dirty background limit.
> And if they aren't writing that much data then they'll be fine too.
> But even putting all of that together I really don't see why you're
> trying to suggest that this is some bizarre set of circumstances that
> should only rarely happen in the real world.

I'm saying that if that happens constantly, you're better off adjusting
shared_buffers, because you're likely already suffering from latency
spikes and other issues. Optimizing for massive random write throughput
in a system that's not configured appropriately, at the cost of well
configured systems to suffer, doesn't seem like a good tradeoff to me.

Note that other operating systems like windows and freebsd *alreaddy*
write back much more aggressively (independent of this change). I seem
to recall you yourself being quite passionately arguing that the linux
behaviour around this is broken.

Greetings,

Andres Freund


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

[HACKERS] [DOC][PATCH]bloom index options limits

[Nikolay Shaplov]

Hi! I've changed limits of the length option in the "Introduction" section. 
They should be 1 ≤ colN ≤ 2048 instead of 1 < colN < 2048 (I've talk with the 
author about it)

I've also added minimal maximum and default values for description of options 
in the "Parameters" section, because I think user should be able to see full 
description of the options in the section where they are described, not in the 
introduction.

-- 
Nikolay Shaplov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Companydiff --git a/doc/src/sgml/bloom.sgml b/doc/src/sgml/bloom.sgml
index 49cb066..36ddc39 100644
--- a/doc/src/sgml/bloom.sgml
+++ b/doc/src/sgml/bloom.sgml
@@ -23,7 +23,7 @@
Since a signature is a lossy representation of all indexed attributes,
search results must be rechecked using heap information.
The user can specify signature length (in uint16, default is 5) and the
-   number of bits, which can be set per attribute (1 < colN < 2048).
+   number of bits, which can be set per attribute (1  colN  2048).
   
 
   
@@ -51,7 +51,8 @@
 length
 
  
-  Length of signature in uint16 type values
+  Number of uint16 units in signature, e. g. default value length = 5
+  defines 80-bit signature. Allowed values for length are from 1 to 256.
  
 

@@ -61,7 +62,8 @@
 col1  col16
 
  
-  Number of bits for corresponding column
+  Number of bits for corresponding column. Allowed values are from 1
+  to 2048, but not greater than length*16. Default value is 2.
  
 


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

Re: [HACKERS] Problem with dumping bloom extension

[Stephen Frost]

Michael,

* Michael Paquier (michael.paqu...@gmail.com) wrote:
> On Fri, Jun 3, 2016 at 8:57 PM, Thom Brown  wrote:
> > If a database with the bloom extension installed is dumped and restored,
> > there's an error with the access method creation:
> >
> > createdb bloomtest
> > psql -c 'CREATE EXTENSION bloom;' bloomtest
> > pg_dump -d bloomtest > ~/tmp/bloom.sql
> > createdb bloomtest2
> > psql -d bloomtest2 -f ~/tmp/bloom.sql
> >
> > The output of the last command produces:
> >
> > "psql:/home/thom/tmp/bloom.sql:48: ERROR:  access method "bloom" already
> > exists"
> >
> > So pg_dump shouldn't be dumping this access method as it's part of the
> > extension.
> 
> Stephen, something is smelling wrong in checkExtensionMembership()
> since 5d58999, an access method does not have ACLs and I would have
> expected that when this routine is invoked in
> selectDumpableAccessMethod() the object is not selected as dumpable.

Yeah, I saw this also and was going to look into it.

I suspect the issue may actually be that dumpAccessMethod() wasn't ever
updated to have the appropriate conditionals for each of the components
of the object.

Specifically, there should be if statements along the lines of:

if (aminfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
ArchiveEntry ...

if (aminfo->dobj.dump & DUMP_COMPONENT_COMMENT)
dumpComment()

towards the end of dumpAccessMethod().

I'm not 100% sure that addresses this, but it definitely needs to be
fixed also.  I'll take care of it in the next few days.

I'll also look more directly into what's going on here this weekend when
I've got a bit more time to do so.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Perf Benchmarking and regression.

[Robert Haas]

On Fri, Jun 3, 2016 at 2:09 AM, Andres Freund  wrote:
> On 2016-06-03 01:57:33 -0400, Noah Misch wrote:
>> > Which means that transactional workloads that are bigger than the OS
>> > memory, or which have a non-uniform distribution leading to some
>> > locality, are likely to be faster. In practice those are *hugely* more
>> > likely than the uniform distribution that pgbench has.
>>
>> That is formally true; non-benchmark workloads rarely issue uniform writes.
>> However, enough non-benchmark workloads have too little locality to benefit
>> from caches.  Those will struggle against *_flush_after like uniform writes
>> do, so discounting uniform writes wouldn't simplify this project.
>
> But such workloads rarely will hit the point of constantly re-dirtying
> already dirty pages in kernel memory within 30s.

I don't know why not.  It's not exactly uncommon to update the same
data frequently, nor is it uncommon for the hot data set to be larger
than shared_buffers and smaller than the OS cache, even significantly
smaller.  Any workload of that type is going to have this problem
regardless of whether the access pattern is uniform.  If you have a
highly non-uniform access pattern then you just have this problem on
the small subset of the data that is hot.  I think that asserting that
there's something wrong with this test is just wrong.  Many people
have done many tests very similar to this one on Linux systems over
many years to assess PostgreSQL performance.  It's a totally
legitimate test configuration.

Indeed, I'd argue that this is actually a pretty common real-world
scenario.  Most people's hot data fits in memory, because if it
doesn't, their performance sucks so badly that they either redesign
something or buy more memory until it does.  Also, most people have
more hot data than shared_buffers.  There are some who don't because
their data set is very small, and that's nice when it happens; and
there are others who don't because they carefully crank shared_buffers
up high enough that everything fits, but most don't, either because it
causes other problems, or because they just don't think to tinkering
with it, or because they set it up that way initially but then the
data grows over time.  There are a LOT of people running with 8GB or
less of shared_buffers and a working set that is in the tens of GB.

Now, what varies IME is how much total RAM there is in the system and
how frequently they write that data, as opposed to reading it.  If
they are on a tightly RAM-constrained system, then this situation
won't arise because they won't be under the dirty background limit.
And if they aren't writing that much data then they'll be fine too.
But even putting all of that together I really don't see why you're
trying to suggest that this is some bizarre set of circumstances that
should only rarely happen in the real world.  I think it clearly does
happen, and I doubt it's particularly uncommon.  If your testing
didn't discover this scenario, I feel rather strongly that that's an
oversight in your testing rather than a problem with the scenario.

-- 
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] Perf Benchmarking and regression.

[Andres Freund]

On 2016-06-03 09:24:28 -0700, Andres Freund wrote:
> This unstable performance issue, with the minute-long stalls, is the
> worst and most frequent production problem people hit with postgres in
> my experience, besides issues with autovacuum.  Ignoring that is just
> hurting our users.

Oh, and a good proportion of the "autovacuum causes my overall systems
to slow down unacceptably" issues come from exactly this.


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

Re: [HACKERS] Perf Benchmarking and regression.

[Andres Freund]

On 2016-06-03 10:48:18 -0400, Noah Misch wrote:
> On Thu, Jun 02, 2016 at 11:09:22PM -0700, Andres Freund wrote:
> > > Today's defaults for *_flush_after greatly smooth and accelerate 
> > > performance
> > > for one class of plausible workloads while greatly slowing a different 
> > > class
> > > of plausible workloads.
> 
> The usual PostgreSQL handling of a deeply workload-dependent performance
> feature is to disable it by default.

Meh. That's not actually all that often the case.  This unstable
performance issue, with the minute-long stalls, is the worst and most
frequent production problem people hit with postgres in my experience,
besides issues with autovacuum.  Ignoring that is just hurting our
users.


> > I don't think checkpoint_flush_after is in that class, due to the
> > fsync()s we already emit at the end of checkpoints.
> 
> That's a promising hypothesis.  Some future project could impose a nonzero
> default checkpoint_flush_after, having demonstrated that it imposes negligible
> harm in the plausible cases it does not help.

Have you actually looked at the thread with all the numbers? This isn't
an issue that has been decided willy-nilly. It's been discussed *over
months*.

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] IPv6 link-local addresses and init data type

[Markus Wanner]

On 06/03/2016 12:14 AM, Tom Lane wrote:
> Markus Wanner  writes:
>> I'm even wondering if 'fe80::1%1'::inet = 'fe80::1%2'::inet shouldn't
>> simply yield true. After all, it's the same (non-global) address.
> 
> Surely not?  If the zone_ids didn't mean anything, why would the concept
> even exist?  ISTM that what you've got there is two different addresses
> neither of which is reachable from outside the given machine --- but
> within that machine, they are different.

You are right, both of these addresses actually are considered different
from 'fe80::1', i.e. I cannot ping6 fe80::1, but need to explicitly
specify an interface - either via an additional argument or with the
percent notation.

That's interesting news to me, as this means that only global IPv6
addresses can be stored in 128 bits. For non-global ones that's not
sufficient and 'fe80::1' isn't even considered a valid address by
itself. Checking the headers and manpages, the sockaddr_in6 sports an
uint32_t sin6_scope_id (as per RFC 2553).

Then, there also is the embedded form which uses the 2nd 16-bit word of
the IPv6 address to store the scope id, see for example "8.1.1.3 Scope
Index" in [0]. However, that document also clearly states: "When you
specify scoped address to the command line, NEVER write the embedded
form (such as ff02:1::1 or fe80:2::fedc)".

Given that interfaces are addressed by index internally, I'm pretty sure
the two representations are equivalent (i.e. if eth3 currently happens
to be the 7th interface , then 'fe80::1%eth3' resolves to 'fe80::1%7').

Considering that Postgres is not unlikely to write INET types to
permanent storage, its values should better survive a reboot. And while
I have some doubts about persistence of interface names, those clearly
have a higher chance of surviving a reboot compared to interface
indices. Therefore, I'd advocate resolving interface indices (if given)
to interface names using if_indextoname(3) and let INET types store only
names.

Assuming such an implementation, the following would hold (assuming an
example system as mentioned above):

  'fe80::1%1'::inet != 'fe80::1%2'::inet

but:

  'fe80::1%7'::inet = 'fe80::1%eth3'::inet

I also tend to deny scope ids for global addresses (like ping6), but
allow non-global ('fe80::1') addresses to be stored without an interface
name, as pre-existing applications may rely on that to work (unlike ping6).

Does that sound like a more reasonable plan?

Kind Regards

Markus Wanner


[0]: FreeBSD dev handbook, Section 8.1.1.3: Scope Index:
https://www.freebsd.org/doc/en/books/developers-handbook/ipv6.html



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

Re: [HACKERS] Reviewing freeze map code

[Robert Haas]

On Fri, Jun 3, 2016 at 11:21 AM, Masahiko Sawada  wrote:
>> Can you submit that part as a separate patch?
>
> Attached.

Thanks, committed.

>>> I'm address the review comment of 7087166 commit, and will post the patch.
>>
>> When?
>
> On Saturday.

Great.  Will that address everything for this open item, then?

-- 
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] Perf Benchmarking and regression.

[Fabien COELHO]

Hello Noah,


The usual PostgreSQL handling of a deeply workload-dependent performance
feature is to disable it by default.  That's what I'm inclined to do here, for
every GUC the feature added.  Sophisticated users will nonetheless fully
exploit this valuable mechanism in 9.6.



I don't think checkpoint_flush_after is in that class, due to the
fsync()s we already emit at the end of checkpoints.


I agree with Andres that checkpoint_flush_after shoud not be treated as 
other _flush_after settings.



That's a promising hypothesis.


This is not an hypothesis but a proven fact. There has been hundreds of 
hours of pgbenchs runs to test and demonstrate the positive impact in 
various reasonable configurations.


Some future project could impose a nonzero default 
checkpoint_flush_after, having demonstrated that it imposes negligible 
harm in the plausible cases it does not help.


I think that the significant and general benefit of checkpoint_flush_after 
has been largely demonstrated and reported on the hacker thread at various 
point of the development of the feature, and that it is safe, and even 
highly advisable to keep it on by default.


The key point is that it is flushing sorted buffers so that it mostly 
results in sequential writes. It avoids in a lot of case where the final 
sync at the end of the checkpoint generates too many ios which results in 
putting postgresql off line till the fsync is completed, from seconds to 
minutes at a time.


The other *_flush_after do not benefit for any buffer reordering, so their 
positive impact is maybe more questionnable, so I would be okay if these 
are disabled by default.


--
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] Reviewing freeze map code

[Masahiko Sawada]

On Sat, Jun 4, 2016 at 12:08 AM, Robert Haas  wrote:
> On Fri, Jun 3, 2016 at 10:49 AM, Masahiko Sawada  
> wrote:
>> That patch also incorporates the following review comment.
>> We can push at least this fix.
>
> Can you submit that part as a separate patch?

Attached.

>> I'm address the review comment of 7087166 commit, and will post the patch.
>
> When?
>

On Saturday.

Regards,

--
Masahiko Sawada


fix_freeze_map_fd31cd2.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] Reviewing freeze map code

[Robert Haas]

On Fri, Jun 3, 2016 at 10:49 AM, Masahiko Sawada  wrote:
> That patch also incorporates the following review comment.
> We can push at least this fix.

Can you submit that part as a separate patch?

> I'm address the review comment of 7087166 commit, and will post the patch.

When?

-- 
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] Reviewing freeze map code

[Masahiko Sawada]

On Fri, Jun 3, 2016 at 11:03 PM, Robert Haas  wrote:
> On Thu, Jun 2, 2016 at 11:24 AM, Masahiko Sawada  
> wrote:
>> Attached patch optimises skipping pages logic so that blkno can jump to
>> next_unskippable_block directly while counting the number of all_visible
>> and all_frozen pages. So we can avoid double checking visibility map.
>
> I think this is 9.7 material.  This patch has already won the
> "scariest patch" tournament.  Changing the logic more than necessary
> at this late date seems like it just increases the scariness.  I think
> this is an opportunity for further optimization, not a defect.
>

I agree with you.
I'll submit this as a improvement for 9.7.
That patch also incorporates the following review comment.
We can push at least this fix.
>> /*
>>  * Compute whether we actually scanned the whole relation. If we 
>> did, we
>>  * can adjust relfrozenxid and relminmxid.
>>  *
>>  * NB: We need to check this before truncating the relation, because 
>> that
>>  * will change ->rel_pages.
>>  */
>>
>> Comment is out-of-date now.

I'm address the review comment of 7087166 commit, and will post the patch.
And testing feature for freeze map is under the discussion.

Regards,

--
Masahiko Sawada


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

Re: [HACKERS] Perf Benchmarking and regression.

[Noah Misch]

On Thu, Jun 02, 2016 at 11:09:22PM -0700, Andres Freund wrote:
> On 2016-06-03 01:57:33 -0400, Noah Misch wrote:
> > > Which means that transactional workloads that are bigger than the OS
> > > memory, or which have a non-uniform distribution leading to some
> > > locality, are likely to be faster. In practice those are *hugely* more
> > > likely than the uniform distribution that pgbench has.
> > 
> > That is formally true; non-benchmark workloads rarely issue uniform writes.
> > However, enough non-benchmark workloads have too little locality to benefit
> > from caches.  Those will struggle against *_flush_after like uniform writes
> > do, so discounting uniform writes wouldn't simplify this project.
> 
> But such workloads rarely will hit the point of constantly re-dirtying
> already dirty pages in kernel memory within 30s.

Rarely, yes.  Not rarely enough to discount.

> > Today's defaults for *_flush_after greatly smooth and accelerate performance
> > for one class of plausible workloads while greatly slowing a different class
> > of plausible workloads.

The usual PostgreSQL handling of a deeply workload-dependent performance
feature is to disable it by default.  That's what I'm inclined to do here, for
every GUC the feature added.  Sophisticated users will nonetheless fully
exploit this valuable mechanism in 9.6.

> I don't think checkpoint_flush_after is in that class, due to the
> fsync()s we already emit at the end of checkpoints.

That's a promising hypothesis.  Some future project could impose a nonzero
default checkpoint_flush_after, having demonstrated that it imposes negligible
harm in the plausible cases it does not help.


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

Re: [HACKERS] chkpass_in should not be volatile

[David G. Johnston]

On Fri, Jun 3, 2016 at 10:41 AM, Thom Brown  wrote:

> On 3 June 2016 at 15:26, David G. Johnston 
> wrote:
>
>> On Fri, Jun 3, 2016 at 9:56 AM, Tom Lane  wrote:
>>
>>> Thom Brown  writes:
>>> > ...or at least according to the warning message:
>>> > postgres=# CREATE EXTENSION chkpass ;
>>> > WARNING:  type input function chkpass_in should not be volatile
>>>
>>> See thread here:
>>>
>>>
>>> https://www.postgresql.org/message-id/flat/CACfv%2BpL2oX08SSZSoaHpyC%3DUbfTFmPt4UmVEKJTH7y%3D2QMRCBw%40mail.gmail.com
>>>
>>> Given the lack of complaints so far, maybe we could think about
>>> redefining
>>> the behavior of chkpass_in.  I'm not very sure to what, though.
>>>
>>
>> Thom, how did you end up encountering this?
>>
>
> I built the extension and tried to create it.  Not really anything other
> than that.
>
>
​I guess, "what was the motivation for creating the extension" would have
been a better question.  Just a test suite for completeness or something
application-level?

David J.

Re: [HACKERS] chkpass_in should not be volatile

[Thom Brown]

On 3 June 2016 at 15:26, David G. Johnston 
wrote:

> On Fri, Jun 3, 2016 at 9:56 AM, Tom Lane  wrote:
>
>> Thom Brown  writes:
>> > ...or at least according to the warning message:
>> > postgres=# CREATE EXTENSION chkpass ;
>> > WARNING:  type input function chkpass_in should not be volatile
>>
>> See thread here:
>>
>>
>> https://www.postgresql.org/message-id/flat/CACfv%2BpL2oX08SSZSoaHpyC%3DUbfTFmPt4UmVEKJTH7y%3D2QMRCBw%40mail.gmail.com
>>
>> Given the lack of complaints so far, maybe we could think about redefining
>> the behavior of chkpass_in.  I'm not very sure to what, though.
>>
>
> Thom, how did you end up encountering this?
>

I built the extension and tried to create it.  Not really anything other
than that.

Thom

Re: [HACKERS] [BUGS] BUG #14155: bloom index error with unlogged table

[Tom Lane]

Michael Paquier  writes:
> Actually, the docs could be more polished.

I think the docs could stand to be rewritten from scratch ;-).  But
upthread there was an offer to work on them if we made the code behavior
saner.  I've done the latter part, I don't want to do the former.

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] Re: pg9.6 segfault using simple query (related to use fk for join estimates)

[Noah Misch]

On Wed, Jun 01, 2016 at 09:29:54PM -0400, Noah Misch wrote:
> On Sun, May 29, 2016 at 01:36:01AM -0400, Noah Misch wrote:
> > On Fri, May 06, 2016 at 03:06:01PM -0400, Robert Haas wrote:
> > > On Thu, May 5, 2016 at 10:48 AM, David Rowley
> > >  wrote:
> > > > On 5 May 2016 at 16:04, David Rowley  
> > > > wrote:
> > > >> I've started making some improvements to this, but need to talk to
> > > >> Tomas. It's currently in the middle of his night, but will try to
> > > >> catch him in his morning to discuss this with him.
> > > >
> > > > Ok, so I spoke to Tomas about this briefly, and he's asked me to send
> > > > in this patch. He didn't get time to look over it due to some other
> > > > commitments he has today.
> > > >
> > > > I do personally feel that if the attached is not good enough, or not
> > > > very close to good enough then probably the best course of action is
> > > > to revert the whole thing.
> > > 
> > > Tom, what do you think about this patch?  Is it good enough, or should
> > > we revert the whole thing?
> > 
> > [This is a generic notification.]
> > 
> > The above-described topic is currently a PostgreSQL 9.6 open item.  Simon,
> > 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
> > 9.6 open item, please let us know.  Otherwise, please observe the policy on
> > open item ownership[1] and send a status update within 72 hours 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 9.6rc1.  Consequently, I will appreciate your
> > efforts toward speedy resolution.  Thanks.
> > 
> > [1] 
> > http://www.postgresql.org/message-id/20160527025039.ga447...@tornado.leadboat.com
> 
> This PostgreSQL 9.6 open item is past due for your status update.  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:
> http://www.postgresql.org/message-id/20160527025039.ga447...@tornado.leadboat.com

IMMEDIATE ATTENTION REQUIRED.  This PostgreSQL 9.6 open item is long past due
for your status update.  Please reacquaint yourself with the policy on open
item ownership[1] and then reply immediately.  If I do not hear from you by
2016-06-04 15:00 UTC, I will transfer this item to release management team
ownership without further notice.

[1] 
http://www.postgresql.org/message-id/20160527025039.ga447...@tornado.leadboat.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] chkpass_in should not be volatile

[David G. Johnston]

On Fri, Jun 3, 2016 at 9:56 AM, Tom Lane  wrote:

> Thom Brown  writes:
> > ...or at least according to the warning message:
> > postgres=# CREATE EXTENSION chkpass ;
> > WARNING:  type input function chkpass_in should not be volatile
>
> See thread here:
>
>
> https://www.postgresql.org/message-id/flat/CACfv%2BpL2oX08SSZSoaHpyC%3DUbfTFmPt4UmVEKJTH7y%3D2QMRCBw%40mail.gmail.com
>
> Given the lack of complaints so far, maybe we could think about redefining
> the behavior of chkpass_in.  I'm not very sure to what, though.
>

Thom, how did you end up encountering this?

​While it seems to have resulted in the right effect (here) maybe we could
have written: "WARNING: If you are reading this please email
pgsql-b...@postgresql.org" and mention checkpass_in volatility in the
subject.​" instead

David J.

Re: [HACKERS] Rename max_parallel_degree?

[Josh berkus]

On 06/02/2016 09:33 PM, Peter Eisentraut wrote:
> On 6/3/16 12:21 AM, Petr Jelinek wrote:
>> On 01/06/16 17:55, David G. Johnston wrote:
>>> On Wed, Jun 1, 2016 at 11:45 AM, Petr Jelinek >> >wrote:
>>>
>>> That GUC also controls worker processes that are started by
>>> extensions, not just ones that parallel query starts. This is btw
>>> one thing I don't like at all about how the current limits work, the
>>> parallel query will fight for workers with extensions because they
>>> share the same limit.
>>>
>>>
>>> ​Given that this models reality the GUC is doing its job.  Now, maybe we
>>> need additional knobs to give the end-user the ability to influence how
>>> those fights will turn out.
>>
>> Agreed, my point is that I think we do need additional knob.
> 
> We need one knob to control how many process slots to create at server
> start, and then a bunch of sliders to control how to allocate those
> between regular connections, superuser connections, replication,
> autovacuum, parallel workers, background workers (by tag/label/group),
> and so on.

Now that's crazy talk.  I mean, next thing you'll be saying that we need
the ability to monitor this, or even change it at runtime.  Where does
the madness end?  ;-)

Seriously, you have a point here; it's maybe time to stop tackling
process management per server piecemeal.  Question is, who wants to work
on this?

-- 
--
Josh Berkus
Red Hat OSAS
(any opinions are my own)


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

Re: [HACKERS] Reviewing freeze map code

[Robert Haas]

On Wed, Jun 1, 2016 at 3:50 AM, Masahiko Sawada  wrote:
> Attached patch fixes only above comments, other are being addressed now.

Committed.

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


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

Re: [HACKERS] Reviewing freeze map code

[Robert Haas]

On Thu, Jun 2, 2016 at 11:24 AM, Masahiko Sawada  wrote:
> Attached patch optimises skipping pages logic so that blkno can jump to
> next_unskippable_block directly while counting the number of all_visible
> and all_frozen pages. So we can avoid double checking visibility map.

I think this is 9.7 material.  This patch has already won the
"scariest patch" tournament.  Changing the logic more than necessary
at this late date seems like it just increases the scariness.  I think
this is an opportunity for further optimization, not a defect.

-- 
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] chkpass_in should not be volatile

[Tom Lane]

Thom Brown  writes:
> ...or at least according to the warning message:
> postgres=# CREATE EXTENSION chkpass ;
> WARNING:  type input function chkpass_in should not be volatile

See thread here:

https://www.postgresql.org/message-id/flat/CACfv%2BpL2oX08SSZSoaHpyC%3DUbfTFmPt4UmVEKJTH7y%3D2QMRCBw%40mail.gmail.com

Given the lack of complaints so far, maybe we could think about redefining
the behavior of chkpass_in.  I'm not very sure to what, though.

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] Problem with dumping bloom extension

[Michael Paquier]

On Fri, Jun 3, 2016 at 8:57 PM, Thom Brown  wrote:
> If a database with the bloom extension installed is dumped and restored,
> there's an error with the access method creation:
>
> createdb bloomtest
> psql -c 'CREATE EXTENSION bloom;' bloomtest
> pg_dump -d bloomtest > ~/tmp/bloom.sql
> createdb bloomtest2
> psql -d bloomtest2 -f ~/tmp/bloom.sql
>
> The output of the last command produces:
>
> "psql:/home/thom/tmp/bloom.sql:48: ERROR:  access method "bloom" already
> exists"
>
> So pg_dump shouldn't be dumping this access method as it's part of the
> extension.

Stephen, something is smelling wrong in checkExtensionMembership()
since 5d58999, an access method does not have ACLs and I would have
expected that when this routine is invoked in
selectDumpableAccessMethod() the object is not selected as dumpable.
-- 
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] Parallel pg_dump's error reporting doesn't work worth squat

[Tom Lane]

Kyotaro HORIGUCHI  writes:
> For sure, any of the "dangers" of TerminateThread don't matter
> for this case.

I think that this one:

>> If the target thread is allocating memory from the heap, the heap
>> lock will not be released.

is potentially a hazard, which is why I made sure to use write_stderr
later on in the console interrupt handler.  Your original suggestion
to use write_msg would end up going through fprintf, which might well
use malloc internally.  (It's possible that Windows' version of write()
could too, I suppose, but that's probably as low-level as we are
going to get.)

regards, tom lane


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

Re: [HACKERS] Rename max_parallel_degree?

[Tom Lane]

Robert Haas  writes:
> I think we should just go with max_parallel_workers for a limit on
> total parallel workers within max_work_processes, and
> max_parallel_workers_per_gather for a per-Gather limit.  It's slightly
> annoying that we may end up renaming the latter GUC, but not as
> annoying as spending another three weeks debating this and missing
> beta2.

+1.  I'm not as convinced as you are that we'll replace the GUC later,
but in any case this is an accurate description of the current
semantics.  And I'm really *not* in favor of fudging the name with
the intent of changing the GUC's semantics later --- that would fail
all sorts of compatibility expectations.

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] XTM & parallel search

[Konstantin Knizhnik]

On 03.06.2016 16:05, Amit Kapila wrote:
On Fri, Jun 3, 2016 at 1:34 AM, Konstantin Knizhnik 
> wrote:


We have to add three more functions to eXtensible Transaction
Manager API (XTM):

/*
 * Calculate transaction state size. This method is invoked by
EstimateTransactionStateSpace to copy transaction
 * state to parallel workers
 */
size_t  (*GetTransactionStateSize)(void);

/*
 * Serialize transaction state
 */
void(*SerializeTransactionState)(void* ctx);

/*
 * Deserialize transaction state
 */
void(*DeserializeTransactionState)(void* ctx);



In above proposal, are you suggesting to change the existing API's as 
well, because the parameters of function pointers don't match with 
exiting API's. I think it is better to consider this along with the 
overall XTM API.


Sorry, but right now I have not replaced existed functions
EstimateTransactionStateSpace/SerializeTransactionState/StartParallelWorkerTransaction
with XTM indirect calls. If was my original intention, but these 
functions access static variable CurrentTransactionState defined in xact.c.
So if  user-defined TM wants to override this functions, it will have to 
invoke original functions to save/restore CurrentTransactionState.

It is not  convenient.
This is why three XTM functions above are now called by existed xact 
funcations to save additional state, for example:


Size
EstimateTransactionStateSpace(void)
{
TransactionState s;
Sizenxids = 6;/* iso level, deferrable, top & 
current XID,

 * command counter, XID count */

for (s = CurrentTransactionState; s != NULL; s = s->parent)
{
if (TransactionIdIsValid(s->transactionId))
nxids = add_size(nxids, 1);
nxids = add_size(nxids, s->nChildXids);
}

nxids = add_size(nxids, nParallelCurrentXids);
nxids = mul_size(nxids, sizeof(TransactionId));
return add_size(nxids, TM->GetTransactionStateSize());
}

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

Re: [HACKERS] XTM & parallel search

[Amit Kapila]

On Fri, Jun 3, 2016 at 1:34 AM, Konstantin Knizhnik <
k.knizh...@postgrespro.ru> wrote:

> We have to add three more functions to eXtensible Transaction Manager API
> (XTM):
>
> /*
>  * Calculate transaction state size. This method is invoked by
> EstimateTransactionStateSpace to copy transaction
>  * state to parallel workers
>  */
> size_t  (*GetTransactionStateSize)(void);
>
> /*
>  * Serialize transaction state
>  */
> void(*SerializeTransactionState)(void* ctx);
>
> /*
>  * Deserialize transaction state
>  */
> void(*DeserializeTransactionState)(void* ctx);
>
>

In above proposal, are you suggesting to change the existing API's as well,
because the parameters of function pointers don't match with exiting API's.
I think it is better to consider this along with the overall XTM API.

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

Re: [HACKERS] Rename max_parallel_degree?

[Robert Haas]

On Fri, Jun 3, 2016 at 8:30 AM, David G. Johnston
 wrote:
> How big is the hazard of future-naming this and documenting the present
> limitation?  Is the casual user reading explains even going to be aware of
> that particular implementation detail?

Well, see, the nice thing about max_parallel_degree is that it is very
slightly ambiguous, just enough to paper over this.  But I'm going to
oppose naming the GUC inaccurately based on a hoped-for future
development project.

Another way to paper over the difference that would be to call this
max_parallel_workers_per_operation.  Then we can document that an
operation is a Gather node, and in the future we could document that
it's now a statement.  However, if we pick a name like this, then
we're sort of implying that this GUC will control DDL, too.

I think we should just go with max_parallel_workers for a limit on
total parallel workers within max_work_processes, and
max_parallel_workers_per_gather for a per-Gather limit.  It's slightly
annoying that we may end up renaming the latter GUC, but not as
annoying as spending another three weeks debating this and missing
beta2.

-- 
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] Rename max_parallel_degree?

[David G. Johnston]

On Fri, Jun 3, 2016 at 8:20 AM, Robert Haas  wrote:

> On Thu, Jun 2, 2016 at 4:35 PM, Josh berkus  wrote:
> > I was assuming that we would have *both* per-operation and per-statement
> > limits.  I can see reasons for having both, I can see why power users
> > would want both, but it's going to be overwhelming to casual users.
>
> I don't think so.  I think the fact that this is per-gather-node
> rather than per-statement right now is basically a defect.  Once we
> have a per-statement limit, I see no value in having the
> per-gather-node setting.  So, yes, at that point, I would push to
> rename the GUC.
>
>
​How big is the hazard of future-naming this and documenting the present
limitation?  Is the casual user reading explains even going to be aware of
that particular implementation detail?

David J.

Re: [HACKERS] Prepared statements and generic plans

[David G. Johnston]

On Fri, Jun 3, 2016 at 3:17 AM, Albe Laurenz 
wrote:

>
> > I'd maybe go with something like this:
> >
> > All executions of a prepared statement having zero parameters will use
> the same plan so the planning
> > time taken during the first execution will be spread across all
> subsequent executions.  For statements
> > having parameters the first five executions will result in
> value-specific plans as previously
> > described.  However, on the sixth execution a generic plan will also be
> computed and if the average
> > planning + execution cost of all previous value-specific plans is about
> equal to the execution cost of
> > the generic plan the generic plan will be chosen for that and all
> subsequent executions.
>
> I think that is much better, but I suggest this wording:
>
> "All executions of a prepared statement having zero parameters use the
> same plan, so they
>  will use the generic plan immediately.  For statements having parameters
> the first five executions
>  will result in value-specific plans as previously described.
>  However, on the sixth execution a generic plan will also be computed, and
> if the average cost estimate
>  of all previous value-specific plans is about equal to the cost estimate
> of the generic plan,
>  the generic plan will be chosen for that and all subsequent executions."
>
> This emphasizes that it is only estimates we are dealing with, otherwise
> it would be hard
> to understand why estimation errors can lead to generic plans being chosen
> that are much worse.
>
>
​You've dropped what I think is the essential point of comparing (planning
+ execution) cost of the per-value setup to just the execution costs of the
generic plan.  You also make it sound like only the sixth plan is performs
the comparison and if the value-specific plan is still preferred the
generic plan will never again be considered.  Change "on" to "beginning
with" and that goes away.  That, and adding back in the costs types, will
make me happy.



> > 
> >
> > If we are getting generic plans significantly cheaper than the
> value-specific plans I suspect there is
> > a problem...so any comparison that indicates "less-than" is prone to
> cause confusion.  The original is
> > worded well on this point: "...generic plan appears to be not much more
> expensive..." but lacks detail
> > elsewhere.
>
> I don't quite get that.  Do you mean the same thing that I wrote above?
>

​Yeah.  I was pointing out the Bruce's was trying to compare using a
less-than which I think is prone to confusion if not outright wrong.
​


>
> > This part:
> >
> > !A generic plan assumes each value supplied to
> EXECUTE
> > !is one of the column's distinct values and that column values are
> > !uniformly distributed.  For example, if statistics records three
> > !distinct column values, a generic plan assumes a column equality
> > !comparison will match 33% of processed rows.  Column statistics
> > !also allows generic plans to accurately compute the selectivity of
> > !unique columns.  Comparisons on non-uniformly-distributed columns
> and
> > !specification of non-existent values affects the average plan cost,
> > !and hence if and when a generic plan is chosen.  [elided the last
> sentence, placed in the first
> > paragraph]
> >
> > I'm not sure of the specific goal here but this level detail seems a bit
> out-of-place in the SQL
> > Command documentation.  So, do we want this user-facing and if so do we
> want it here?
>
> [...]
>
> > This leaves Bruce's second alteration: which probably should follow the
> rest over to chapter 66.  The
> > point of the existing sentence is to give the casual user the means to
> detect the current type of plan
> > and I think that is all that is needed here.
>
> I agree that this is too much detail.
> I would vote for omitting it altogether.
>
> Anybody who needs that level of detail is better served with the source
> anyway.
>

​This goes back to Bruce's motivation but as long as it goes into the
internals section I have no problem adding material that someone felt was
worth their time to write.

​David J.

Re: [HACKERS] Rename max_parallel_degree?

[Robert Haas]

On Thu, Jun 2, 2016 at 4:35 PM, Josh berkus  wrote:
> I was assuming that we would have *both* per-operation and per-statement
> limits.  I can see reasons for having both, I can see why power users
> would want both, but it's going to be overwhelming to casual users.

I don't think so.  I think the fact that this is per-gather-node
rather than per-statement right now is basically a defect.  Once we
have a per-statement limit, I see no value in having the
per-gather-node setting.  So, yes, at that point, I would push to
rename the GUC.

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


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

[HACKERS] Problem with dumping bloom extension

[Thom Brown]

Hi,

If a database with the bloom extension installed is dumped and restored,
there's an error with the access method creation:

createdb bloomtest
psql -c 'CREATE EXTENSION bloom;' bloomtest
pg_dump -d bloomtest > ~/tmp/bloom.sql
createdb bloomtest2
psql -d bloomtest2 -f ~/tmp/bloom.sql

The output of the last command produces:

"psql:/home/thom/tmp/bloom.sql:48: ERROR:  access method "bloom" already
exists"

So pg_dump shouldn't be dumping this access method as it's part of the
extension.

Regards

Thom

Re: [HACKERS] PATCH: Batch/pipelining support for libpq

[Dmitry Igrishin]

2016-05-24 5:01 GMT+03:00 Craig Ringer :
>
> On 24 May 2016 at 00:00, Michael Paquier  wrote:
>>
>> On Mon, May 23, 2016 at 8:50 AM, Andres Freund  wrote:
>
>
>>
>> > yay^2.
>>
>> I'll follow this mood. Yeha.
>
>
>
> BTW, I've publushed the HTML-ified SGML docs to
> http://2ndquadrant.github.io/postgres/libpq-batch-mode.html as a preview.
Typo detected: "Returns 1 if the batch curently being received" -- "curently".

-- 
// Dmitry.


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

[HACKERS] chkpass_in should not be volatile

[Thom Brown]

...or at least according to the warning message:

postgres=# CREATE EXTENSION chkpass ;
WARNING:  type input function chkpass_in should not be volatile

Thom

[HACKERS] hstore: add hstore_length function

[Korbin Hoffman]

Hi there-

I've attached a small patch exposing HS_COUNT to the user as
"hstore_length(hstore)". Documentation, upgrade sql files, and a few
tests are also included.

Almost every hstore function calls HS_COUNT, but I couldn't determine
if there was a reason this exposure didn't already exist.

Without this function, I've been converting an hstore into an array
and then counting it - a more expensive operation (~30-40% slower than
SELECTing the hstore itself in a few of my tests).

I will add this thread and patch to the next Commitfest.

Thanks,
Korbin Hoffman


hstore_length-v1.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] Rename synchronous_standby_names?

[Vik Fearing]

On 01/06/16 02:49, Michael Paquier wrote:
> On Wed, Jun 1, 2016 at 3:56 AM, David G. Johnston
>  wrote:
>> On Tue, May 31, 2016 at 2:19 PM, Peter Eisentraut
>>  wrote:
>>>
>>> On 5/31/16 1:47 PM, Jaime Casanova wrote:

 Are we going to change synchronous_standby_names? Certainly the GUC is
 not *only* a list of names anymore.

 synchronous_standby_config?
 synchronous_standbys (adjust to correct english if necesary)?
>>>
>>> If the existing values are still going to be accepted, then I would leave
>>> it as is.
>>
>> +1
> 
> +1. We've made quite a lot of deal to take an approach for the N-sync
> that is 100% backward-compatible, it would be good to not break that
> effort.

We could always accept it like we do for archive/hot_standby->replica.

I like synchronous_standby_config, so I vote for changing it.
-- 
Vik Fearing  +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support


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

[Tatsuo Ishii]

>> I didn't noticed it. Could you give me the message id or URL?
>>
>>
> https://commitfest.postgresql.org/10/634/
> 
> https://www.postgresql.org/message-id/flat/CAMsr+YFUjJytRyV4J-16bEoiZyH=4nj+sQ7JP9ajwz=b4dm...@mail.gmail.com#CAMsr+YFUjJytRyV4J-16bEoiZyH=4nj+sQ7JP9ajwz=b4dm...@mail.gmail.com

Thanks.

>> Another issue is inconsistency with log duration, which shows the the
>> elapsed time for each execute message. I think statement timeout
>> should be consistent with statement duration. Otherwise users will be
>> confused.
>>
> 
> While I agree that's confusing, I think that's actualyl a problem with
> log_duration.
> 
> log_duration is really more of an internal trace parameter that should be
> named debug_log_duration or something IMO. It also fails to print the
> message type, which makes it even more confusing since it for a typical
> extended protocl query it usually logs 3 durations with no indication of
> which is what.

It's definitely a poor design.

> Users should be using log_min_duration_statement. You know, the confusingly
> named one. Or is it log_duration_min_statement or
> log_statement_min_duration or ...?
> 
> Yeah, log_duration is confusing to the point I think it needs a comment
> like "To record query run-time you probably want
> log_min_duration_statement, not log_duration".

I'm confused. Regarding the timing whether duration is emitted at sync
or each message, log_duration and log_min_duration_statement behave
exactly same, no? If so, log_min_duration_statement is not consistent
with statement_timeout, anyway.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp


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

Re: [HACKERS] Parallel pg_dump's error reporting doesn't work worth squat

[Kyotaro HORIGUCHI]

At Thu, 2 Jun 2016 12:17:11 -0400, Alvaro Herrera  
wrote in <20160602161711.GA239156@alvherre.pgsql>
> Tom Lane wrote:
> > Kyotaro HORIGUCHI  writes:
> > > Apart from the invalid snapshot problem, I looked the patch
> > > previously mentioned mainly for Windows.
> > 
> > Thanks for looking!
> > 
> > > Even though the threads started by beginthread cannot be
> > > terminated cleanly from outside, but the whole process will soon
> > > terminate anyway, so we could use TreminateThread. This seems
> > > working. (Attached patch)
> > 
> > Seems reasonable to me; I was unhappy about the lack of any direct
> > equivalent to the child SIGTERMs that the Unix code does.

For sure, any of the "dangers" of TerminateThread don't matter
for this case.

https://msdn.microsoft.com/en-us/em-us/library/windows/desktop/ms686717(v=vs.85).aspx

> If the target thread owns a critical section, the critical
> section will not be released.
> 
> If the target thread is allocating memory from the heap, the heap
> lock will not be released.
> 
> If the target thread is executing certain kernel32 calls when it
> is terminated, the kernel32 state for the thread's process could
> be inconsistent.
> 
> If the target thread is manipulating the global state of a shared
> DLL, the state of the DLL could be destroyed, affecting other
> users of the DLL.



> Given this testing, it's clear that the timeout on select() is useless;
> we could get rid of it in vacuumdb.c too.  I'll post a patch later.

Agreed. Command pipes may be in blocking mode for the case.

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] Let file_fdw access COPY FROM PROGRAM

[Corey Huinker]

On Fri, Jun 3, 2016 at 1:06 AM, Craig Ringer  wrote:

> On 3 June 2016 at 04:48, Corey Huinker  wrote:
>
>> A while back, there was a push to make COPY gzip-aware. That didn't
>> happen, but COPY FROM PROGRAM did, and it scratches the same itch.
>>
>
>
>> - writing unwanted columns to a temp/work table via COPY, and then
>> immediately re-reading them
>>
>
> Without wanting to take away from the value of letting file FDW access
> FROM PROGRAM, I think this really merits a solution that applies to COPY as
> well. Variants on "how do I COPY just some columns from a CSV" is a real
> FAQ, and it doesn't seem like it'd be excessively hard to support. We'd
> just need some way to pass a list of column-ordinals or header-names.
>
> Not asking you to do that work, just pointing out that this particular
> issue applies to COPY its self as well.
>

I agree, they are two different but slightly overlapping issues. file_fdw
needs a way to handle compressed/filtered files, and COPY needs the ability
to skip columns. But right now COPY is all about getting the shape of the
input from the shape of the destination table.

I had the bright idea of creating a custom datatype, call it "skip" /
"filler" / "nada" / "devnull" or something like that, that would map any
and all values to NULL, thus allowing COPY to naively "store" the unwanted
values into nothingness.

That idea falls apart when it hits InputFunctionCall() in fmgr.c, which
prevents any text string from coercing into NULL. I didn't think I was up
to the task of making InputFunctionCall respect a special case type (void?)
or somehow promoting the pseudo type void into a legit column type (unknown
side-effects there). I did create a type that coerced all input into the
empty string, but the disk usage of that was still pretty significant.

So maybe if there was a subspecies of COPY that returned SETOF RECORD:

SELECT important_column1, important_column2
FROM copy_srf( program := 'zcat filename', format := 'csv', header := true
) AS t(bogus_col1 text, important_column1 integer, important_column2 date,
bogus_col2 text, ... );


That would allow COPY to keep it's current efficiency and simplicity, while
(hopefully) avoiding the unwanted data from ever hitting disk. It would
also be vaguely reminiscent of SQL*Loader.

Re: [HACKERS] Prepared statements and generic plans

[Albe Laurenz]

David G. Johnston wrote:
> On Thu, Jun 2, 2016 at 9:56 PM, Bruce Momjian  wrote:
>> In Postgres 9.2 we improved the logic of when generic plans are used by
>> EXECUTE.  We weren't sure how well it would work, and the docs included
>> a vague description on when generic plans are chosen.
>> 
>> I have gotten a few questions lately about how prepared statements are
>> handled with multiple executions so I have updated the PREPARE manual
>> page with the attached patch to more clearly explain generic plans and
>> when they are chosen.
>> 
>> I would like to apply this to the 9.6 docs.

I think that this is a very good idea.

I had to dig through the sources before to answer such questions.

> ​While putting the proposed patch in context I came across this.
> 
> ​"""
> ​
> Prepared statements have the largest performance advantage when a single 
> session is being used to
> execute a large number of similar statements. The performance difference will 
> be particularly
> significant if the statements are complex to plan or rewrite, for example, if 
> the query involves a
> join of many tables or requires the application of several rules. If the 
> statement is relatively
> simple to plan and rewrite but relatively expensive to execute, the 
> performance advantage of prepared
> statements will be less noticeable.
> 
> ​"""
> 
> Until and unless the generic plan is used the  "...if statements are complex 
> to plan..." doesn't make
> sense; and no where in the description itself do we introduce the generic 
> plan concept.  This is
> inconsistent but I'm not addressing it below though its worth considering 
> before a patch to this area
> is committed.

I think that the paragraph is still true; it talks about "a large number of 
similar statements".
As long as "large" is sufficiently greater than five, that is.

> As to the patch...
> 
> Isn't this backwards?  [change]
> 
> """
> otherwise it  occurs only after five or more 
> executions produce
> [execution /strike plans] plus planning costs that are, on average, [roughly 
> equal to /strike cheaper]
> than the generic plan.
> """

I agree.
I also fouund this sentence hard to read.

> I'd maybe go with something like this:
> 
> All executions of a prepared statement having zero parameters will use the 
> same plan so the planning
> time taken during the first execution will be spread across all subsequent 
> executions.  For statements
> having parameters the first five executions will result in value-specific 
> plans as previously
> described.  However, on the sixth execution a generic plan will also be 
> computed and if the average
> planning + execution cost of all previous value-specific plans is about equal 
> to the execution cost of
> the generic plan the generic plan will be chosen for that and all subsequent 
> executions.

I think that is much better, but I suggest this wording:

"All executions of a prepared statement having zero parameters use the same 
plan, so they
 will use the generic plan immediately.  For statements having parameters the 
first five executions
 will result in value-specific plans as previously described.
 However, on the sixth execution a generic plan will also be computed, and if 
the average cost estimate
 of all previous value-specific plans is about equal to the cost estimate of 
the generic plan,
 the generic plan will be chosen for that and all subsequent executions."

This emphasizes that it is only estimates we are dealing with, otherwise it 
would be hard
to understand why estimation errors can lead to generic plans being chosen that 
are much worse.

> 
> 
> If we are getting generic plans significantly cheaper than the value-specific 
> plans I suspect there is
> a problem...so any comparison that indicates "less-than" is prone to cause 
> confusion.  The original is
> worded well on this point: "...generic plan appears to be not much more 
> expensive..." but lacks detail
> elsewhere.

I don't quite get that.  Do you mean the same thing that I wrote above?

> This part:
> 
> !A generic plan assumes each value supplied to EXECUTE
> !is one of the column's distinct values and that column values are
> !uniformly distributed.  For example, if statistics records three
> !distinct column values, a generic plan assumes a column equality
> !comparison will match 33% of processed rows.  Column statistics
> !also allows generic plans to accurately compute the selectivity of
> !unique columns.  Comparisons on non-uniformly-distributed columns and
> !specification of non-existent values affects the average plan cost,
> !and hence if and when a generic plan is chosen.  [elided the last 
> sentence, placed in the first
> paragraph]
> 
> I'm not sure of the specific goal here but this level detail seems a bit 
> out-of-place in the SQL
> Command documentation.  So, do we want this user-facing and if so do we want 
> it here?

[...]

> This leaves Bruce's second alteration: 

Re: [HACKERS] regexp_match() returning text

[Jim Nasby]

On 5/30/16 1:01 PM, Andrew Gierth wrote:

Returning an array containing the values of all capture groups might be
more useful (substring returns the value of the first capture group if
any, otherwise the matched string).


+1.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)   mobile: 512-569-9461


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

Re: [HACKERS] Rename max_parallel_degree?

[Albe Laurenz]

Robert Haas wrote:
> On Wed, Jun 1, 2016 at 5:29 PM, Tom Lane  wrote:
> > Robert Haas  writes:
> >> I've largely given up hope of coming up with an alternative that can
> >> attract more than one vote and that is also at least mildly accurate,
> >> but one idea is max_parallel_workers_per_gather_node.  That will be
> >> totally clear.
> >
> > Given the reference to Gather nodes, couldn't you drop the word
> > "parallel"?  "node" might not be necessary either.
> 
> Well, I think we could drop node, if you like.  I think parallel
> wouldn't be good to drop, though, because it sounds like we want a
> global limit on parallel workers also, and that can't be just
> max_workers.  So I think we should keep parallel in there for all of
> them, and have max_parallel_workers and
> max_parallel_workers_per_gather(_node).  The reloption and the Path
> struct field can be parallel_workers rather than parallel_degree.

I believe that it will be impossible to find a name that makes
the meaning clear to everybody.  Those who do not read the documentation
will always find a way to misunderstand it.

These suggestions have the added disadvantage that it is hard
to remember them.  I see myself going, "I have to change the maximum
for parallel workers, what was the name again?", and having to resort
to the manual (or SHOW ALL) each time.

I suggest to follow the precedent of "work_mem", stick with
something simple like "max_parallel_workers" and accept the risk
of not being totally self-explanatory.

Yours,
Laurenz Albe

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

Re: [HACKERS] [BUGS] BUG #14155: bloom index error with unlogged table

[Michael Paquier]

On Fri, Jun 3, 2016 at 3:11 PM, Michael Paquier
 wrote:
> On Fri, Jun 3, 2016 at 1:03 PM, Tom Lane  wrote:
>> I wrote:
>>> Jeff Janes  writes:
 My biggest gripe with it at the moment is that the signature size should be
 expressed in bits, and then internally rounded up to a multiple of 16,
 rather than having it be expressed in 'uint16'.
 If that were done it would be easier to fix the documentation to be more
 understandable.
>>
>>> +1 ... that sort of definition seems much more future-proof, too.
>>> IMO it's not too late to change this.  (We probably don't want to change
>>> the on-disk representation of the reloptions, but we could convert from
>>> bits to words in bloptions().)
>>
>> There were no objections to this, but also no action.  Attached is a draft
>> patch ... any complaints?
>
> None. This looks rather sane to me.

Actually, the docs could be more polished. "Limitation" should be
changed to "Limitations", and "Opclass interface" to "Operator Class
Interface". The current wording "Seqscan is slow" is not clear, this
should mention a sequential scan instead. And it is not that slow,
just slower than the heap index scan of bloom..
-- 
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] [BUGS] BUG #14155: bloom index error with unlogged table

[Michael Paquier]

On Fri, Jun 3, 2016 at 1:03 PM, Tom Lane  wrote:
> I wrote:
>> Jeff Janes  writes:
>>> My biggest gripe with it at the moment is that the signature size should be
>>> expressed in bits, and then internally rounded up to a multiple of 16,
>>> rather than having it be expressed in 'uint16'.
>>> If that were done it would be easier to fix the documentation to be more
>>> understandable.
>
>> +1 ... that sort of definition seems much more future-proof, too.
>> IMO it's not too late to change this.  (We probably don't want to change
>> the on-disk representation of the reloptions, but we could convert from
>> bits to words in bloptions().)
>
> There were no objections to this, but also no action.  Attached is a draft
> patch ... any complaints?

None. This looks rather sane to me.
-- 
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] Perf Benchmarking and regression.

[Andres Freund]

On 2016-06-03 01:57:33 -0400, Noah Misch wrote:
> > Which means that transactional workloads that are bigger than the OS
> > memory, or which have a non-uniform distribution leading to some
> > locality, are likely to be faster. In practice those are *hugely* more
> > likely than the uniform distribution that pgbench has.
> 
> That is formally true; non-benchmark workloads rarely issue uniform writes.
> However, enough non-benchmark workloads have too little locality to benefit
> from caches.  Those will struggle against *_flush_after like uniform writes
> do, so discounting uniform writes wouldn't simplify this project.

But such workloads rarely will hit the point of constantly re-dirtying
already dirty pages in kernel memory within 30s.


> Today's defaults for *_flush_after greatly smooth and accelerate performance
> for one class of plausible workloads while greatly slowing a different class
> of plausible workloads.

I don't think checkpoint_flush_after is in that class, due to the
fsync()s we already emit at the end of checkpoints.

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