Re: [HACKERS] patch proposal

[Michael Paquier]

On Tue, Aug 16, 2016 at 11:06 PM, Stephen Frost  wrote:
> I could see supporting an additional "pause" option that means "pause at
> the end of WAL if you don't reach the recovery target point".  I'd also
> be happy with a warning being emitted in the log if the recovery target
> point isn't reached before reaching the end of WAL, but I don't think it
> makes sense to change the existing behavior.

Indeed, let's not change the existing behavior. A warning showing up
by default would be useful in itself, even if there are people that I
think set up overly high recovery targets to be sure to replay WAL as
much as possible. As recovery_target_action has meaning when a
recovery target has been reached, I would guess that we would want a
new option that has the same mapping value as recovery_target_action,
except that it activates when the target recovery is *not* reached.
Hence it would be possible to shutdown, pause or promote at will when
recovery completes, and be able to take a separate action is the
recovery target is indeed reached. The default of this parameter would
be "promote", which is what happens now.
--
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] Most efficient way for libPQ .. PGresult serialization

[Tatsuo Ishii]

> On 18 August 2016 at 10:05, Joshua Bay  wrote:
> 
>> Hi,
>>
>> I was trying to implement a middleware that lies between client and
>> postgres.
>>
>> So, this middleware is supposed to run query with libpq, do its job on
>> them, and then serialize the result of query, and send it to the client !
>> (client deserializes to PGresult)
>>
>> I could simply iterate over rows and columns but than that would be slow.
>> I also found that query results consist of 3 parts (PGresult, tuples, data
>> blocks).
>>
>> Could I please get some pointers ? :)
>>
>>
> Take a look at the code for PgBouncer and PgPool-II. Both implement
> PostgreSQL protocol proxies you could use as starting points.

This one is based on Pgpool-II.

https://github.com/treasure-data/prestogres

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] patch proposal

[Venkata B Nagothi]

On Wed, Aug 17, 2016 at 11:27 PM, Stephen Frost  wrote:

> Venkata,
>
> * Venkata B Nagothi (nag1...@gmail.com) wrote:
> > Agreed. Additional option like "pause" would. As long as there is an
> option
> > to ensure following happens if the recovery target is not reached -
> >
> >  a) PG pauses the recovery at the end of the WAL
> >  b) Generates a warning in the log file saying that recovery target point
> > is not reached (there is a patch being worked upon on by Thom on this)
> >  c) Does not open-up the database exiting from the recovery process by
> > giving room to resume the replay of WALs
>
> One thing to consider is just how different this is from simply bringing
> PG up as a warm standby instead, with the warning added to indicate if
> the recovery point wasn't reached.
>

I am referring to a specific scenario (performing point-in time recovery)
where-in a DBA attempts to bring up a standalone PG instance by restoring
the backup and performing recovery to a particular recover target (XID,
time or a named restore point) in the past by replaying all the available
WAL archives, which is not quite possible through a warm-standby setup.

Warm standby is more of a high-availability solution and i do not think so,
it addresses PITR kind of situation.

I will share more details defining PG behaviour across various recovery
scenarios (as asked by David Steele) when using various recovery*
parameters. Will also explain what difference the proposed patch could make.

Regards,

Venkata B N
Fujitsu Australia

Re: [HACKERS] Anyone want to update our Windows timezone map?

[Michael Paquier]

On Tue, Aug 16, 2016 at 6:22 PM, Magnus Hagander  wrote:
> I think what we want is basically the UNION ALL of all the different active
> versions. If we just use the definitions from Win10, we will map incorrectly
> on Win7. As long as they are not *conflicting*, we should just add them all.

I see, so that's why the script does not do any suggestions to remove any data.

> The UTC->GMT switch is mostly just annoying as it only hits the comments,
> right? I think we just accept that change and do a once-over patch that
> changes all those things.

OK, so after re-running that on my Win10 station and a newly updated
Win7 station I am finishing with the attached that combines all the
changes.
-- 
Michael


win32-tz-v2.patch
Description: application/download

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

Re: [HACKERS] Most efficient way for libPQ .. PGresult serialization

[Craig Ringer]

On 18 August 2016 at 10:05, Joshua Bay  wrote:

> Hi,
>
> I was trying to implement a middleware that lies between client and
> postgres.
>
> So, this middleware is supposed to run query with libpq, do its job on
> them, and then serialize the result of query, and send it to the client !
> (client deserializes to PGresult)
>
> I could simply iterate over rows and columns but than that would be slow.
> I also found that query results consist of 3 parts (PGresult, tuples, data
> blocks).
>
> Could I please get some pointers ? :)
>
>
Take a look at the code for PgBouncer and PgPool-II. Both implement
PostgreSQL protocol proxies you could use as starting points.

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

Re: [HACKERS] How to do failover in pglogical replication?

[Craig Ringer]

On 17 August 2016 at 18:21, roshan_myrepublic  wrote:

> Hi,
>
> I am currently exploring pglogical replication for my db servers. I would
> like to know how can I automatically failover from Provider Node to
> Subscriber Node, if the Provider node goes down for some reasons. How can I
> redirect all the traffic to SubscriberNode automatically ? In the normal
> replication, we use recovery_file and triggers to get this job done. Do we
> have any similar alternative for pglogical replications as well?


 There is not, as yet, any integegration into tooling like repmgr. You'll
want some fairly simple scripts to manage failover, likely:

* Update pgbouncer / haproxy / whatever to redirect connections
* Drop the subscription on the replica
* STONITH to make sure the master is really down
* Clone and start a new master

There's some work on automating this through repmgr, but at this time
pglogical isn't really focused on failover deployments as its main use
case. The limitations in PostgreSQL's logical decoding and replication when
it comes to handling of big xacts, sequences, etc mean it's still better
suited to data movement/integration etc than HA.

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

[HACKERS] synchronous_commit = remote_flush

[Thomas Munro]

Hi hackers,

To do something about the confusion I keep seeing about what exactly
"on" means, I've often wished we had "remote_flush".  But it's not
obvious how the backwards compatibility could work, ie how to keep the
people happy who use "local" vs "on" to control syncrep, and also the
people who use "off" vs "on" to control asynchronous commit on
single-node systems.  Is there any sensible way to do that, or is it
not broken and I should pipe down, or is it just far too entrenched
and never going to change?

-- 
Thomas Munro
http://www.enterprisedb.com


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

Re: [HACKERS] Increasing timeout of poll_query_until for TAP tests

[Michael Paquier]

On Thu, Aug 4, 2016 at 6:56 AM, Michael Paquier
 wrote:
> On Thu, Aug 4, 2016 at 2:34 AM, Alvaro Herrera  
> wrote:
>> Michael Paquier wrote:
>>> On Wed, Aug 3, 2016 at 7:21 AM, Alvaro Herrera  
>>> wrote:
>>
>>> > Why not capture both items in a single select, such as in the attached
>>> > patch?
>>>
>>> Let me test this
>>> [... A while after ...]
>>> This looks to work properly. 12 runs in a row have passed.
>>
>> Okay, applied that way.
>>
>> BTW, one-line long queries look awful in that perl code.  I don't
>> propose to change anything now, but I propose that long queries are
>> split using here-docs in new code,
>>
>> $node->safe_psql(<> SELECT foo
>>   FROM bar
>> EQ
>
> Yep, that would be a good idea. I didn't know this grammar existed. Or
> use qq() directly.

hamster has not failed even once for two weeks now, so I think that
we're good. Let's see if the problem with pg_basebackup pops up once
again.
-- 
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] Patch: initdb: "'" for QUOTE_PATH (non-windows)

[Andres Freund]

On August 17, 2016 8:15:56 PM PDT, Michael Paquier  
wrote:

>+{ /* pg_ctl command w path, properly quoted */
>+PQExpBuffer pg_ctl_path = createPQExpBuffer();
>+printfPQExpBuffer(pg_ctl_path, "%s%spg_ctl",
>+bin_dir,
>+(strlen(bin_dir) > 0) ? DIR_SEP : ""
>+);
>+appendShellString(start_db_cmd, pg_ctl_path->data);
>+destroyPQExpBuffer(pg_ctl_path);
>+}
>
>This is not really project-style to have an independent block. Usually
>those are controlled by for, while or if. 

Besides the comment positioning I'd not say that that is against the usual 
style, there's a number of such blocks already.  Don't think it's necessarily 
needed here though...
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.


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

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

[Michael Paquier]

On Thu, Aug 18, 2016 at 11:35 AM, Ryan Murphy  wrote:

Be careful of top-posting, this is not this ML style:
http://www.idallen.com/topposting.html

>> I think that's actually a good thing to forbid.
>
> I think I agree Andres, there are already comments in the appendShellString
> function to this effect - they say that CR/LF chars in a file name are
> mostly used for malicious hacking attempts anyways - I know I've hardly ever
> needed a newline in a file name.
>
> Did you see anything else in my code that you have recommendations about?  I
> made sure to free the PQExpBufferStr vars that I allocated.

+{ /* pg_ctl command w path, properly quoted */
+PQExpBuffer pg_ctl_path = createPQExpBuffer();
+printfPQExpBuffer(pg_ctl_path, "%s%spg_ctl",
+bin_dir,
+(strlen(bin_dir) > 0) ? DIR_SEP : ""
+);
+appendShellString(start_db_cmd, pg_ctl_path->data);
+destroyPQExpBuffer(pg_ctl_path);
+}

This is not really project-style to have an independent block. Usually
those are controlled by for, while or if. And you could use the same
PQExpBuffer for everything.
-- 
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] Add -c to rsync commands on SR tutorial wiki page

[Stephen Frost]

* Jim Nasby (jim.na...@bluetreble.com) wrote:
> https://wiki.postgresql.org/wiki/Binary_Replication_Tutorial does
> not specify -c for any of the rsync commands. That's maybe safe for
> WAL, but I don't think it's safe for any of the other uses, right?
> I'd like someone to confirm before I just change the page... my
> intention is to just stick -c in all the commands.

-c is only relevant when you are doing an incremental copy, but on a
quick look, all those rsync commands appear to be doing full copies?

You would want -c if you were taking a backup and then doing an update
of it using rsync. or something along those lines, as you can't really
trust rsync's time/size based comparison as it only has a 1 second level
granularity.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Add -c to rsync commands on SR tutorial wiki page

[Peter Eisentraut]

On 8/17/16 5:34 PM, Jim Nasby wrote:
> https://wiki.postgresql.org/wiki/Binary_Replication_Tutorial does not 
> specify -c for any of the rsync commands. That's maybe safe for WAL, but 
> I don't think it's safe for any of the other uses, right? I'd like 
> someone to confirm before I just change the page... my intention is to 
> just stick -c in all the commands.

Why do you think that?

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


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

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

[Ryan Murphy]

> I think that's actually a good thing to forbid.

I think I agree Andres, there are already comments in the appendShellString
function to this effect - they say that CR/LF chars in a file name are
mostly used for malicious hacking attempts anyways - I know I've hardly
ever needed a newline in a file name.

Did you see anything else in my code that you have recommendations about?
I made sure to free the PQExpBufferStr vars that I allocated.

Best,
Ryan

On Wed, Aug 17, 2016 at 7:41 PM, Andres Freund  wrote:

> On 2016-08-18 09:14:44 +0900, Michael Paquier wrote:
> > On Thu, Aug 18, 2016 at 12:21 AM, Ryan Murphy 
> wrote:
> > > I have created a better patch (attached) that correctly escapes the
> shell
> > > arguments using PQExpBufferStr and the appendShellString function, as
> per
> > > Michael and Andres' suggestions.
> > >
> > > Further suggestions welcome of course.
> >
> > As far as I know, it is perfectly possible to have LF/CR in a path
> > name (that's bad practice btw...), and your patch would make initdb
> > fail in such cases. Do we want to authorize that?
>
> I think that's actually a good thing to forbid.
>

Re: [HACKERS] Way to access LSN (for each transaction) by directly talking to postgres?

[Joshua Bay]

Sorry I forgot to reply. Thanks! using decoding plugins works great

On Wed, Aug 3, 2016 at 8:37 PM, Michael Paquier 
wrote:

> On Thu, Aug 4, 2016 at 3:02 AM, Joshua Bay  wrote:
> > Could you please tell me how I can get LSN of each transaction at decoder
> > plugin?
>
> Craig already gave you hints, but here are more. You will need to hack
> your own plugin. You could just use the one in contrib/test_decoding,
> remove most of its code, and use the commit callback to issue the LSN
> you are interested in. Note as well that when using
> pg_logical_slot_peek_changes or pg_logical_slot_get_changes, you can
> get a LSN location. Using test_decoding as a base, that's not a
> complicated effort.
> --
> Michael
>

[HACKERS] Most efficient way for libPQ .. PGresult serialization

[Joshua Bay]

Hi,

I was trying to implement a middleware that lies between client and
postgres.

So, this middleware is supposed to run query with libpq, do its job on
them, and then serialize the result of query, and send it to the client !
(client deserializes to PGresult)

I could simply iterate over rows and columns but than that would be slow.
I also found that query results consist of 3 parts (PGresult, tuples, data
blocks).

Could I please get some pointers ? :)

Thanks,
Joshua

Re: [HACKERS] WIP: Barriers

[Thomas Munro]

On Tue, Aug 16, 2016 at 1:55 AM, Robert Haas  wrote:
> On Sat, Aug 13, 2016 at 7:18 PM, Thomas Munro
>  wrote:
>> My use case for this is coordinating the phases of parallel hash
>> joins, but I strongly suspect there are other cases.  Parallel sort
>> springs to mind, which is why I wanted to post this separately and
>> earlier than my larger patch series, to get feedback from people
>> working on other parallel features.
>
> I was thinking about this over the weekend and I started to wonder
> whether this is really going to work well for hash joins.  For
> example, suppose that 6GB of work_mem is available and the projected
> size of the hash table is 8GB.  Clearly, we're going to need 2
> batches, but, if our estimates are accurate and the tuples split
> evenly between batches, each batch will be only 4GB!  That means that
> we can build up to 2GB of the hash table for the next batch before
> we've finished with the hash table for the previous batch.  It seems
> like a really good idea to try to take advantage of that as much as
> possible.

Right.  Thanks for that example.  A naive approach with barriers
between building and probing does indeed introduce a kind of pipeline
stall where workers twiddle their thumbs, despite there being unused
work_mem and useful things that could be done concurrently to fill it.

> The simple version of this is that when a worker gets done with its
> own probe phase for batch X, it can immediately start building the
> hash table for phase X+1, stopping if it fills up the unused portion
> of work_mem before the old hash table goes away.  Of course, there are
> some tricky issues with reading tapes that were originally created by
> other backends, but if I understand correctly, Peter Geoghegan has
> already done some work on that problem, and it seems like something we
> can eventually solve, even if not in the first version.
>
> The more complicated version of this is that we might want to delegate
> one or more workers to start building as much of the next-batch hash
> table as will fit instead of assisting with the current probe phase.
> Once work_mem is full, they join the probe phase and continue until
> it's done.  Again, tape management is an issue.  But you can see that
> if you can make this work, in this example, you can reduce the
> enforced pause between batches by about 50%; half the work is already
> done by the time the old hash table goes away.  I bet that has a
> chance of being fairly significant, especially for hash joins that
> have tons of batches.  I once saw a 64-batch hash join outperform a
> nested loop with inner index scan!
>
> Anyway, my point here is that I'm not sure whether the barrier
> mechanic is going to work well for computations with overlapping
> phases, and I suspect that overlapping phases is going to be an
> important technique, so we should make sure not to settle into a
> synchronization model that makes it hard.

I have a draft scheme worked out to do what you called the "simple
version", though not in the first version I will post (the "naive
version").  I don't really want to go into all the details in this
thread, not least because I'm still working on it, but I do want to
point out that barriers as a synchronisation primitive are not at all
incompatible with overlapping work: on the contrary, they can help
coordinate it while keeping the code tractable.

Rough sketch*:  There will be a secondary hash table, used for
preloading the next batch.  As in the naive version, there is a
barrier after the hash table is loaded: you can't start probing an
incomplete hash table.  But after probing the primary table for batch
n, workers will immediately begin preloading the secondary table with
tuples for batch n + 1, until either work_mem is exhausted or batch n
+ 1 is entirely loaded.  Then will they synchronise, elect one worker
to promote the secondary table to primary, synchronise again, finish
loading the newly promoted primary table, and then rince and repeat:
synchronise to coordinate the beginning of probing for batch n + 1...

Of course there are also several other phases relating to
initialization, hashing, resizing and special cases for first and last
batches, as I will describe in a future thread with code.

*Actual patch may not resemble illustration.

>> A problem that I'm still grappling with is how to deal with workers
>> that fail to launch.  What I'm proposing so far is based on static
>> worker sets, where you can only give the number of workers at
>> initialisation time, just like pthread_barrier_init.  Some other
>> libraries allow for adjustable worker sets, and I'm wondering if a
>> parallel leader might need to be able to adjust the barrier when it
>> hears of a worker not starting.  More on that soon.
>
> I think tying this into the number of workers for the parallel context
> in general is going to get you into trouble.  For example, suppose
> that we have 

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

[Andres Freund]

On 2016-08-18 09:14:44 +0900, Michael Paquier wrote:
> On Thu, Aug 18, 2016 at 12:21 AM, Ryan Murphy  wrote:
> > I have created a better patch (attached) that correctly escapes the shell
> > arguments using PQExpBufferStr and the appendShellString function, as per
> > Michael and Andres' suggestions.
> >
> > Further suggestions welcome of course.
> 
> As far as I know, it is perfectly possible to have LF/CR in a path
> name (that's bad practice btw...), and your patch would make initdb
> fail in such cases. Do we want to authorize that?

I think that's actually a good thing to forbid.


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

[Andres Freund]

On 2016-08-03 20:22:03 -0700, Andres Freund wrote:
> On 2016-08-02 16:30:55 -0700, Andres Freund wrote:
> > > > Besides that I'm structurally wondering whether turning the original
> > > > query into a subquery is the right thing to do. It requires some kind of
> > > > ugly munching of Query->*, and has the above problem.
> > >
> > > It does not seem like it should be that hard, certainly no worse than
> > > subquery pullup.  Want to show code?
> >
> > It's not super hard, there's some stuff like pushing/not-pushing
> > various sortgrouprefs to the subquery. But I think we can live with it.
> >
> > Let me clean up the code some, hope to have something today or
> > tomorrow.
> 
> Here we go.  This *clearly* is a POC, not more.  But it mostly works.
> 
> 
> 0001 - adds some test, some of those change after the later patches
> 0002 - main SRF via ROWS FROM () implementation
> 0003 - Large patch removing now unused code. Most satisfying.
> 
> 
> The interesting bit is obviously 0002. What it basically does is, at the 
> beginning
> of subquery_planner():
> 1) unsrfify:
>move the jointree into a subquery
> 2) unsrfify_reference_subquery_mutator:
>process the old targetlist to reference the new subquery. If a
>TargetEntry doesn't contain a set, it's entirely moved into the
>subquery. Otherwise all Vars/Aggrefs/... it references are moved to
>the subquery, and referenced in the outer query's target list.
> 3) unsrfify_implement_srfs_mutator:
>Replace set returning functions in the targetlist with references to
>a new FUNCTION RTE. All non-nested tSRFs are part of the same RTE
>(i.e. the least common multiple behaviour is gone). all tSRFs in
>arguments are implemented as another FUNCTION RTE.
> 
> I discovered that we allow SRFs in UPDATE target lists. It's not clear
> to me what that's supposed to mean. Nor how exactly to implement that,
> given expand_targetlist(). Right now that fails with the patch, because
> it re-inserts Var's for the relation replaced by the subquery.
> 
> Note that I've not bothered to fix up the regression test output - I'm
> certain that explain output and such will still change.
> 
> Biggest questions / tasks:
> * General approach
> * DML handling
> * Operator implementation
> * SETOF record handling
> * correct handling of lateral dependency from RTE to subquery to force
>   evaluation order, instead of my RangeTblEntry->deps hack.
> * lot of cleanup
> 
> Comments?

Tom, do you think this is roughly going in the right direction? My plan
here is to develop two patches, to come before this:

a) Allow to avoid using a tuplestore for SRF_PERCALL SRFs in ROWS FROM -
   otherwise our performance would regress noticeably in some cases.
b) Allow ROWS FROM() to return SETOF RECORD type SRFs as one column,
   instead of expanded. That's important to be able move SETOF RECORD
   returning functions in the targetlist into ROWS FROM, which otherwise
   requires an explicit column list.

Greetings,

Andres Freund


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

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

[Michael Paquier]

On Thu, Aug 18, 2016 at 9:30 AM, Ryan Murphy  wrote:
(please avoid top-posting)

>> As far as I know, it is perfectly possible to have LF/CR in a path
>> name (that's bad practice btw...), and your patch would make initdb
>> fail in such cases. Do we want to authorize that? If we bypass the
>> error checks in appendShellString with an extra option, and have
>> initdb use that, the generated command would be actually correct.
>
> That's a fair point Michael.  I would be willing to make such a change, but
> since c doesn't have optional function arguments I'm not sure the least
> intrusive way to do that.  Do you have a suggestion?

You could just add a boolean flag to appendShellString called for
example no_error that the code path of initdb sets to true, and the
other ones to false.
--
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] CLUSTER, reform_and_rewrite_tuple(), and parallelism

[Andres Freund]

On 2016-08-17 17:35:32 -0700, Peter Geoghegan wrote:
> On Wed, Aug 17, 2016 at 5:20 PM, Andres Freund  wrote:
> > libc isn't compiled with -fno-omit-frame-pointer (and even if, it uses
> > assembly without setup of the frame pointer), so frame pointer based
> > call graphs are wrong through libc.  The attributions are based on
> > random stuff in the frame pointer at that point.  You either need to use
> > dwarf or lbr to get accurate ones.
> 
> Is it worth doing that here, and redoing the test, so that the glibc
> attributions are correct?

I'd say yes.


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

Re: [HACKERS] CLUSTER, reform_and_rewrite_tuple(), and parallelism

[Peter Geoghegan]

On Wed, Aug 17, 2016 at 5:20 PM, Andres Freund  wrote:
> libc isn't compiled with -fno-omit-frame-pointer (and even if, it uses
> assembly without setup of the frame pointer), so frame pointer based
> call graphs are wrong through libc.  The attributions are based on
> random stuff in the frame pointer at that point.  You either need to use
> dwarf or lbr to get accurate ones.

Is it worth doing that here, and redoing the test, so that the glibc
attributions are correct?

-- 
Peter Geoghegan


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

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

[Ryan Murphy]

That's a fair point Michael.  I would be willing to make such a change, but
since c doesn't have optional function arguments I'm not sure the least
intrusive way to do that.  Do you have a suggestion?

On Wednesday, August 17, 2016, Michael Paquier 
wrote:

> On Thu, Aug 18, 2016 at 12:21 AM, Ryan Murphy  > wrote:
> > I have created a better patch (attached) that correctly escapes the shell
> > arguments using PQExpBufferStr and the appendShellString function, as per
> > Michael and Andres' suggestions.
> >
> > Further suggestions welcome of course.
>
> As far as I know, it is perfectly possible to have LF/CR in a path
> name (that's bad practice btw...), and your patch would make initdb
> fail in such cases. Do we want to authorize that? If we bypass the
> error checks in appendShellString with an extra option, and have
> initdb use that, the generated command would be actually correct.
> --
> Michael
>

Re: [HACKERS] CLUSTER, reform_and_rewrite_tuple(), and parallelism

[Andres Freund]

On 2016-08-17 21:17:56 -0300, Alvaro Herrera wrote:
> Peter Geoghegan wrote:
> 
> > This doesn't seem that interesting, but not sure what you're looking for.
> > 
> > I also attach cycles flamegraph.
> 
> I may be blind, but what are those write() calls attributed to
> heap_form_tuple?

libc isn't compiled with -fno-omit-frame-pointer (and even if, it uses
assembly without setup of the frame pointer), so frame pointer based
call graphs are wrong through libc.  The attributions are based on
random stuff in the frame pointer at that point.  You either need to use
dwarf or lbr to get accurate ones.


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

Re: [HACKERS] CLUSTER, reform_and_rewrite_tuple(), and parallelism

[Alvaro Herrera]

Peter Geoghegan wrote:

> This doesn't seem that interesting, but not sure what you're looking for.
> 
> I also attach cycles flamegraph.

I may be blind, but what are those write() calls attributed to
heap_form_tuple?

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


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

Re: [HACKERS] CLUSTER, reform_and_rewrite_tuple(), and parallelism

[Andres Freund]

On 2016-08-17 16:58:49 -0700, Peter Geoghegan wrote:
> On Wed, Aug 17, 2016 at 4:28 PM, Andres Freund  wrote:
> > Could you also provide a strace -ttt -T -c and a cpu cycles flamegraph?
> 
> Here is the output from that strace invocation, plus a -p (to attach
> to the relevant backend):
> 
> strace: -t has no effect with -c
> strace: -T has no effect with -c

Ugh, swapped -c and -C, sorry.


> This doesn't seem that interesting, but not sure what you're looking for.

I'd like to know the amount of time spent in various syscalls, and how
much that varies.


> I also attach cycles flamegraph.

Thanks.

Andres


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

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

[Michael Paquier]

On Thu, Aug 18, 2016 at 12:21 AM, Ryan Murphy  wrote:
> I have created a better patch (attached) that correctly escapes the shell
> arguments using PQExpBufferStr and the appendShellString function, as per
> Michael and Andres' suggestions.
>
> Further suggestions welcome of course.

As far as I know, it is perfectly possible to have LF/CR in a path
name (that's bad practice btw...), and your patch would make initdb
fail in such cases. Do we want to authorize that? If we bypass the
error checks in appendShellString with an extra option, and have
initdb use that, the generated command would be actually correct.
-- 
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] CLUSTER, reform_and_rewrite_tuple(), and parallelism

[Peter Geoghegan]

On Wed, Aug 17, 2016 at 4:28 PM, Andres Freund  wrote:
> Could you also provide a strace -ttt -T -c and a cpu cycles flamegraph?

Here is the output from that strace invocation, plus a -p (to attach
to the relevant backend):

strace: -t has no effect with -c
strace: -T has no effect with -c
strace: Process 27986 attached
strace: Process 27986 detached
% time seconds  usecs/call callserrors syscall
-- --- --- - - 
 55.750.629331   179813516 unlink
 17.490.197422   0   2505449   write
 11.690.132000   1100012   fsync
  8.130.091799   0   2078837   read
  5.320.06   12000 5   ftruncate
  0.980.011011  24   460   brk
  0.640.0072181805 4   munmap
  0.000.50   0  6382   lseek
  0.000.00   058 5 open
  0.000.00   058   close
  0.000.00   014   stat
  0.000.00   0 4   mmap
  0.000.00   0 2   rt_sigprocmask
  0.000.00   012 6 rt_sigreturn
  0.000.00   0 1   select
  0.000.00   016   sendto
  0.000.00   0 2 1 recvfrom
  0.000.00   016   kill
  0.000.00   019   semop
  0.000.00   063   getrusage
  0.000.00   0 5   epoll_create
  0.000.00   0 9 4 epoll_wait
  0.000.00   010   epoll_ctl
-- --- --- - - 
100.001.128831   459147332 total

This doesn't seem that interesting, but not sure what you're looking for.

I also attach cycles flamegraph.

trace_sort indicated that the tuplesort CLUSTER takes just under 3
minutes (this includes writing out the new heap, of course).

-- 
Peter Geoghegan


cycles-cluster-presorted-flamegraph.svg.gz
Description: GNU Zip compressed data

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

Re: [HACKERS] PATCH: Exclude additional directories in pg_basebackup

[Michael Paquier]

On Thu, Aug 18, 2016 at 1:35 AM, Alvaro Herrera
 wrote:
> I don't remember how pg_snapshot works, but it's probably fine
> to start with an empty subdir (is it possible to export a snapshot from
> a prepared transaction?)

>From xact.c:
/*
 * Likewise, don't allow PREPARE after pg_export_snapshot.  This could be
 * supported if we added cleanup logic to twophase.c, but for now it
 * doesn't seem worth the trouble.
 */
if (XactHasExportedSnapshots())
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot PREPARE a transaction that has exported snapshots")));
So that's fine.
-- 
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] [RFC] Change the default of update_process_title to off

[Tsunakawa, Takayuki]

From: Magnus Hagander [mailto:mag...@hagander.net]
Applied and backpatched to 9.6.

Thank you very much.  I didn’t expect 9.6 to be patched, so I’m very happy.

Regards
Takayuki Tsunakawa

Re: [HACKERS] [GENERAL] C++ port of Postgres

[Craig Ringer]

On 18 August 2016 at 02:14, Gavin Flower 
wrote:


> My main language is Java, and there are a lot of very good reasons for
> rewriting Postgres in Java, but I'd never push that - as there are also
> many good reasons for NOT rewriting Postgres in Java!
>

I don't know why folks are jumping on the idea of a "rewrite". The original
post mentioned a C++ port, adding C++ compatibility and adopting some C++
features. Hardly a rewrite.

I'm not convinced that's a good idea either, unless it shows compelling
advantages in code clarity, performance, static checking, etc. But it's
hardly a "rewrite", that's just hyperbolic.

I think that to get anywhere with this you'll need to show a more concrete
plan for:

* What happens with libpq
* How this affects existing extensions and what changes they'll need
* How this affects PGXS
* What benefits this change offers. Concrete benefits with examples -
performance numbers, code snippets, etc
* Compatibility impact on platform and compiler support

Since there's approximately zero chance of a "one big commit" switchover,
you'll also need to present a transition plan, probably something like:

* Make all headers "extern "C"" conditionally if compiled as C++
* Add "extern "C"" to all C files conditionally if compiled as C++
* add a 'configure' option to compile as C++
* Progressively resolve C++-incompatibilities in C files and headers until
the whole database compiles as c++
* Resolve any runtime issues
* Add buildfarm client support for optionally building with c++
* Switch one or more buildfarm members to build with c++ or add new ones
* Identify and fix compatibility issues on other platforms

Then down the track we'd:
* Switch to C++ builds by default
* Define a C++ coding standard/policy that strictly identifies what C++
features are permissible
* Shut down the C-only buildfarm members and start permitting some C++
feature use where appropriate


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

Re: [HACKERS] CLUSTER, reform_and_rewrite_tuple(), and parallelism

[Andres Freund]

On 2016-08-17 16:23:29 -0700, Peter Geoghegan wrote:
> On Wed, Aug 17, 2016 at 4:16 PM, Andres Freund  wrote:
> >> Does anyone have any ideas on how to:
> >>
> >> 1). Directly address the reform_and_rewrite_tuple() bottleneck.
> >
> > What part of is actually the expensive bit? It does a whole lot of
> > things. Forming/Deforming tuples, the hash lookups in
> > rewrite_heap_tuple(), ...?
> 
> Perhaps the attached svg "flamegraph" will give you some idea. This is
> based on the "perf cache-misses" event type.

Could you also provide a strace -ttt -T -c and a cpu cycles flamegraph?


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

Re: [HACKERS] psql: tab completion for \l

[Ian Barwick]

Hi

On 8/17/16 2:41 PM, Gerdan Santos wrote:
> The following review has been posted through the commitfest application:
> make installcheck-world:  tested, passed
> Implements feature:   tested, passed
> Spec compliant:   tested, passed
> Documentation:tested, passed
> 
> I did some tests and found nothing special. The stated resource is 
> implemented correctly.
> He passes all regression tests and enables the use of the new features 
> specified.
> 
> The new status of this patch is: Ready for Committer

Thanks for taking the time to review this!


Regards

Ian Barwick


-- 
 Ian Barwick   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, RemoteDBA, 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] CLUSTER, reform_and_rewrite_tuple(), and parallelism

[Peter Geoghegan]

On Wed, Aug 17, 2016 at 4:16 PM, Andres Freund  wrote:
>> Does anyone have any ideas on how to:
>>
>> 1). Directly address the reform_and_rewrite_tuple() bottleneck.
>
> What part of is actually the expensive bit? It does a whole lot of
> things. Forming/Deforming tuples, the hash lookups in
> rewrite_heap_tuple(), ...?

Perhaps the attached svg "flamegraph" will give you some idea. This is
based on the "perf cache-misses" event type.

-- 
Peter Geoghegan


cache-misses-cluster-presorted-flamegraph.svg.gz
Description: GNU Zip compressed 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] CLUSTER, reform_and_rewrite_tuple(), and parallelism

[Andres Freund]

> Does anyone have any ideas on how to:
> 
> 1). Directly address the reform_and_rewrite_tuple() bottleneck.

What part of is actually the expensive bit? It does a whole lot of
things. Forming/Deforming tuples, the hash lookups in
rewrite_heap_tuple(), ...?

Andres


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

[HACKERS] CLUSTER, reform_and_rewrite_tuple(), and parallelism

[Peter Geoghegan]

During preliminary analysis of what it would take to produce a
parallel CLUSTER patch that is analogous of what I came up with for
CREATE INDEX, which in general seems quite possible, I identified
reform_and_rewrite_tuple() as a major bottleneck for the current
CLUSTER implementation.

Excluding the cost of the subsequent REINDEX of the clustered-on
index, reform_and_rewrite_tuple() appears to account for roughly 25% -
35% of both the cache misses, and instructions executed, for my test
case (this used a tuplesort, not an indexscan on the old heap
relation, of course). Merging itself was far less expensive (with my
optimization of how the heap is maintained during merging + 16
tapes/runs), so it would be reasonable to not parallelize that part,
just as it was for parallel CREATE INDEX. I don't think that it's
reasonable to not do anything about this reform_and_rewrite_tuple()
bottleneck, though.

Does anyone have any ideas on how to:

1). Directly address the reform_and_rewrite_tuple() bottleneck.

and/or:

2). Push down some or all of the reform_and_rewrite_tuple() work till
before tuples are passed to the tuplesort.

"2" would probably make it straightforward to have
reform_and_rewrite_tuple() work occur in parallel workers instead,
which buys us a lot.

-- 
Peter Geoghegan


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

Re: [HACKERS] regexp_match() returning text

[Tom Lane]

"David G. Johnston"  writes:
> I didn't compile either patch but given the scope and complexity I'd say it
> is ready for committer without that confirmed.  Tom usually touches the
> regexp code and I'm pretty sure he'll look at this with an eye no one else
> has.  Though I wouldn't expect anything until work on 10 begins in earnest.

Pushed with some cosmetic adjustments to the code and rather more
extensive work on the documentation.

I did *not* push the hunk in citext.sgml, since that was alleging support
that doesn't actually exist in this patch.  To make this work for citext,
we need to add wrapper functions similar to citext's wrappers for
regexp_matches.  And that in turn means a citext extension version bump,
which makes it more notationally tedious than I felt like dealing with
today.  I'm bouncing that requirement back to you to produce a separate
patch for.

regards, tom lane


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

Re: [HACKERS] PATCH: Exclude additional directories in pg_basebackup

[David Steele]

On 8/17/16 2:53 PM, Robert Haas wrote:
> On Wed, Aug 17, 2016 at 2:50 PM, David Steele  wrote:
>> Hi Robert,
>>
>> On 8/17/16 11:27 AM, Robert Haas wrote:
>>> On Mon, Aug 15, 2016 at 3:39 PM, David Steele  wrote:
 Recently a hacker proposed a patch to add pg_dynshmem to the list of
 directories whose contents are excluded in pg_basebackup.  I wasn't able
 to find the original email despite several attempts.

 That patch got me thinking about what else could be excluded and after
 some investigation I found the following: pg_notify, pg_serial,
 pg_snapshots, pg_subtrans.  These directories are all cleaned, zeroed,
 or rebuilt on server start.
>>>
>>> Eh ... I doubt very much that it's safe to blow away the entire
>>> contents of an SLRU between shutdown and startup, even if the data is
>>> technically transient data that won't be needed again after the system
>>> is reset.
>>
>> If you are correct it may indicate a problem anyway. Consider a standby
>> backup where the files in these directories may be incredibly stale
>> since they are not replicated.  Once restored to a master should we
>> trust anything in these files?
>>
>> pg_serial, pg_notify, pg_subtrans are not even fsync'd
>> (SlruCtl->do_fsync = false).  It's hard to imagine there's anything of
>> value in there or that it can be trusted if there is.
> 
> It's not just a question of whether the data has value; it's a
> question of whether the SLRU code will handle the situation correctly
> in all cases if the directory contains no files.  I don't think you
> can draw a firm conclusion on that without reading the code.

A good point, Robert.  I read the code but I should have shared my
findings in the original post. I'll only cover the the cases we have not
already decided were safe (those that explicitly delete files in their
init routines, pg_snapshots and pg_dynshmem).

First, SlruPhysicalWritePage() will create a file/page that does not
exist and SlruPhysicalReadPage() will return zeros for a file/page that
does not exist during recovery.  Not that I think the latter is
important for pg_serial, pg_notify, or pg_subtrans since they are not
WAL-logged.  As far as I can see the slru system is very resilient overall.

For pg_notify, AsyncShmemInit() explicitly deletes all files on startup.

For pg_subtrans, StartupSUBTRANS() explicitly zeroes all required pages
but does not flush them to disk.  Since SlruPhysicalWritePage() is
tolerant of missing files/pages this should be fine.

For pg_serial, OldSerXidInit() doesn't zero anything out since it
ignores the old transactions and they get truncated as the transaction
horizon moves.  The old data hangs around for a little while so it could
theoretically be used for debugging as Kevin notes.  Since pg_serial
would only be cleared after a restore I don't see that as a big issue.

-- 
-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] Why we lost Uber as a user

[Jim Nasby]

On 8/17/16 2:51 PM, Simon Riggs wrote:

On 17 August 2016 at 12:19, Greg Stark  wrote:

On Wed, Aug 17, 2016 at 1:36 AM, Jim Nasby  wrote:

Something I didn't see mentioned that I think is a critical point: last I
looked, HOT standby (and presumably SR) replays full page writes. That means
that *any* kind of corruption on the master is *guaranteed* to replicate to
the slave the next time that block is touched. That's completely the
opposite of trigger-based replication.


Yes, this is exactly what it should be doing and exactly why it's
useful. Physical replication accurately replicates the data from the
master including "corruption" whereas a logical replication system
will not, causing divergence and possible issues during a failover.


Yay! Completely agree.

Physical replication, as used by DRBD and all other block-level HA
solutions, and also used by other databases, such as Oracle.

Corruption on the master would often cause errors that would prevent
writes and therefore those changes wouldn't even be made, let alone be
replicated.


My experience has been that you discover corruption after it's already 
safely on disk, and more than once I've been able to recover by using 
data on a londiste replica.


As I said originally, it's critical to understand the different 
solutions and the pros and cons of each. There is no magic bullet.

--
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] Pluggable storage

[Alvaro Herrera]

Anastasia Lubennikova wrote:
> 13.08.2016 02:15, Alvaro Herrera:

> >To support this, we introduce StorageTuple and StorageScanDesc.
> >StorageTuples represent a physical tuple coming from some storage AM.
> >It is necessary to have a pointer to a StorageAmRoutine in order to
> >manipulate the tuple.  For heapam.c, a StorageTuple is just a HeapTuple.
> 
> StorageTuples concept looks really cool. I've got some questions on
> details of implementation.
> 
> Do StorageTuples have fields common to all implementations?
> Or StorageTuple is totally abstract structure that has nothing to do
> with data, except pointing to it?
> 
> I mean, now we already have HeapTupleData structure, which is a pretty
> good candidate to replace with StorageTuple.

I was planning to replace all uses of HeapTuple in the executor with
StorageTuple, actually.  But the main reason I would like to avoid
HeapTupleData itself is that it contains an assumption that there is a
single palloc chunk that contains the tuple (t_len and t_data).  This
might not be true in representations that split the tuple, for example
in columnar storage where you have one column in page A and another
column in page B, for the same tuple.  I suppose there might be some
point to keeping t_tableOid and t_self, though.

> And maybe add a "t_handler" field that points out to handler functions.
> I don't sure if it will be a name of StorageAm, or its OID, or maybe the
> main function itself. Although, If I'm not mistaken, we always have
> RelationData when we want to operate the tuple, so having t_handler
> in the StorageTuple is excessive.

Yeah, I think the RelationData (or more precisely the StorageAmRoutine)
is going to be available always, so I don't think we need a pointer in
the tuple itself.

> This approach allows to minimize code changes and ensure that we
> won't miss any function that handles tuples.
> 
> Do you see any weak points of the suggestion?
> What design do you use in your prototype?

It's currently a "void *" pointer in my prototype.

> >RelationData gains ->rd_stamroutine which is a pointer to the
> >StorageAmRoutine for the relation in question.  Similarly,
> >TupleTableSlot is augmented with a link to the StorageAmRoutine to
> >handle the StorageTuple it contains (probably in most cases it's set at
> >the same time as the tupdesc).  This implies that routines such as
> >ExecAssignScanType need to pass down the StorageAmRoutine from the
> >relation to the slot.
> 
> If we already have this pointer in t_handler as described below,
> we don't need to pass it between functions and slots.

I think it's better to have it in slots, so you can install multiple
tuples in the slot without having to change the routine pointers each
time.

> >The executor is modified so that instead of calling heap_insert etc
> >directly, it uses rel->rd_stamroutine to call these methods.  The
> >executor is still in charge of dealing with indexes, constraints, and
> >any other thing that's not the tuple storage itself (this is one major
> >point in which this differs from FDWs).  This all looks simple enough,
> >with one exception and a few notes:
> 
> That is exactly what I tried to describe in my proposal.
> Chapter "Relation management". I'm sure, you've already noticed
> that it will require huge source code cleaning. I've carefully read
> the sources and found "violators" of abstraction in src/backend/commands.
> The list is attached to the wiki page
> https://wiki.postgresql.org/wiki/HeapamRefactoring.
> 
> Except these, there are some pretty strange and unrelated functions in
> src/backend/catalog.
> I'm willing to fix them, but I'd like to synchronize our efforts.

I very much would like to stay away from touching src/backend/catalog,
which are the functions that deal with system catalogs.  We can simply
say that system catalogs are hardcoded to use heapam.c storage for now.
If we later see a need to enable some particular catalog using a
different storage implementation, we can change the code for that
specific catalog in src/backend/catalog and everywhere else, to use the
abstract API instead of hardcoding heap_insert etc.  But that can be
left for a second pass.  (This is my point "iv" further below, to which
you said "+1").


> Nothing to do, just substitute t_data with proper HeapTupleHeader
> representation. I think it's a job for StorageAm. Let's say each StorageAm
> must have stam_to_heaptuple() function and opposite function
> stam_from_heaptuple().

Hmm, yeah, that also works.  We'd have to check again whether it's more
convenient to start as a slot rather than a StorageTuple.  AFAICS the
trigger.c code is all starting from a slot, so it makes sense to have
the conversion use the slot code -- that way, there's no need for each
storageAM to re-implement conversion to HeapTuple.

> >note f) More widespread, MinimalTuples currently use a tweaked HeapTuple
> >format.  In the long run, it may be possible to replace them with a
> >separate storage 

[HACKERS] Add -c to rsync commands on SR tutorial wiki page

[Jim Nasby]

https://wiki.postgresql.org/wiki/Binary_Replication_Tutorial does not 
specify -c for any of the rsync commands. That's maybe safe for WAL, but 
I don't think it's safe for any of the other uses, right? I'd like 
someone to confirm before I just change the page... my intention is to 
just stick -c in all the commands.

--
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] Declarative partitioning - another take

[Robert Haas]

On Wed, Aug 17, 2016 at 2:21 AM, Amit Langote
 wrote:
> I am slightly tempted to eliminate the pg_partition catalog and associated
> syscache altogether and add a column to pg_class as Robert suggested.
> That way, all relid_is_partition() calls will be replaced by
> rel->rd_partbound != NULL check.  But one potential problem with that
> approach is that now whenever a parent relation is opened, all the
> partition relations must be opened to get the partbound value (to form the
> PartitionDesc to be stored in parent relation's rd_partdesc).  Whereas
> currently, we just look up the pg_partition catalog (or the associated
> cache) for every partition and that gets us the partbound.

Well, you could just look up the pg_class row without opening the
relation, too.  There is a system cache on pg_class.oid, after all.  I
think the issue is whether it's safe to read either one of those
things without a lock on the child relation.  If altering the
partitioning information for a relation requires holding only
AccessExclusiveLock on that relation, and no lock on the parent, then
you really can't read the information for any child relation without
taking at least AccessShareLock.  Otherwise, it might change under
you, and that would be bad.

I'm inclined to think that changing the partitioning information for a
child is going to require AccessExclusiveLock on both the child and
the parent.  That seems unfortunate from a concurrency point of view,
but we may be stuck with it: suppose you require only
ShareUpdateExclusiveLock on the parent.  Well, then a concurrent read
transaction might see the partition boundaries change when it does a
relcache rebuild, which would cause it to suddenly start expecting the
data to be in a different plan in mid-transaction, perhaps even in
mid-scan.  Maybe that's survivable with really careful coding, but it
seems like it's probably a bad thing.  For example, it would mean that
the executor would be unable to rely on the partitioning information
in the relcache remaining stable underneath it.  Moreover, the
relcache is always going to be scanned with the most recent possible
MVCC snapshot, but the transaction snapshot may be older, so such a
system creates all sorts of nasty possibilities for there to be skew
between the snapshot being used to via the data and the snapshot being
used to read the metadata that says where the data is.

This may need some more thought, but if we go with that approach of
requiring an AccessExclusiveLock on both parent and child, then it
seems to me that maybe we should consider the partitioning information
to be a property of the parent rather than the child.  Just take all
the partitioning information for all children and put it in one big
node tree and store it in the pg_class or pg_partition_root entry for
the parent as one big ol' varlena.  Now you can open the parent and
get all of the partitioning information for all of the children
without needing any lock on any child, and that's *really* good,
because it means that some day we might be able to do partition
elimination before locking any of the children!  That would be
excellent.

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


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

Re: [HACKERS] Why we lost Uber as a user

[Simon Riggs]

On 17 August 2016 at 12:19, Greg Stark  wrote:
> On Wed, Aug 17, 2016 at 1:36 AM, Jim Nasby  wrote:
>> Something I didn't see mentioned that I think is a critical point: last I
>> looked, HOT standby (and presumably SR) replays full page writes. That means
>> that *any* kind of corruption on the master is *guaranteed* to replicate to
>> the slave the next time that block is touched. That's completely the
>> opposite of trigger-based replication.
>
> Yes, this is exactly what it should be doing and exactly why it's
> useful. Physical replication accurately replicates the data from the
> master including "corruption" whereas a logical replication system
> will not, causing divergence and possible issues during a failover.

Yay! Completely agree.

Physical replication, as used by DRBD and all other block-level HA
solutions, and also used by other databases, such as Oracle.

Corruption on the master would often cause errors that would prevent
writes and therefore those changes wouldn't even be made, let alone be
replicated.

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


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

Re: [HACKERS] PATCH: Exclude additional directories in pg_basebackup

[Kevin Grittner]

On Wed, Aug 17, 2016 at 1:50 PM, David Steele  wrote:

>>> That patch got me thinking about what else could be excluded and after
>>> some investigation I found the following: pg_notify, pg_serial,
>>> pg_snapshots, pg_subtrans.  These directories are all cleaned, zeroed,
>>> or rebuilt on server start.
>>
>> Eh ... I doubt very much that it's safe to blow away the entire
>> contents of an SLRU between shutdown and startup, even if the data is
>> technically transient data that won't be needed again after the system
>> is reset.
>
> I've done pretty extensive testing in pgBackRest and haven't seen issues
> in any supported version (plus I audited each init() function for every
> version back to where it was introduced).  The patch also passes all the
> pg_basebackup TAP tests in master.
>
> If you are correct it may indicate a problem anyway. Consider a standby
> backup where the files in these directories may be incredibly stale
> since they are not replicated.  Once restored to a master should we
> trust anything in these files?
>
> pg_serial, pg_notify, pg_subtrans are not even fsync'd
> (SlruCtl->do_fsync = false).  It's hard to imagine there's anything of
> value in there or that it can be trusted if there is.

The contents of pg_serial could be deleted safely.  As I recall, I
initially had it cleaned out on startup, but Heikki (who was the
main committer for this feature) added SLRU buffer flushing for it
on checkpoint and took out the startup delete code with the
explanation that he thought someone might want to look at the file
contents for debugging purposes.  I would be a bit surprised if
anyone ever has used it for that, but that is the reason the
contents are not deleted just like pg_snapshot and pg_dynshmem.

--
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] Bug in intarray bench script

[Tom Lane]

"Andreas 'ads' Scherbaum"  writes:
> The patch changes the benchmark tool in a way that the explain output is 
> printed to standard out - what one would expect from the "-e" (explain) 
> option.

> The new status of this patch is: Ready for Committer

Pushed, thanks.

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] Use pread and pwrite instead of lseek + write and read

[Tom Lane]

Robert Haas  writes:
> I don't understand why you think this would create non-trivial
> portability issues.

The patch as submitted breaks entirely on platforms without pread/pwrite.
Yes, we can add a configure test and some shim functions to fix that,
but the argument that it makes the code shorter will get a lot weaker
once we do.

I agree that adding such functions is pretty trivial, but there are
reasons to think there are other hazards that are less trivial:

First, a self-contained shim function will necessarily do an lseek every
time, which means performance will get *worse* not better on non-pread
platforms.  And yes, the existing logic to avoid lseeks fires often enough
to be worthwhile, particularly in seqscans.

Second, I wonder whether this will break any kernel's readahead detection.
I wouldn't be too surprised if successive reads (not preads) without
intervening lseeks are needed to trigger readahead on at least some
platforms.  So there's a potential, both on platforms with pread and those
without, for this to completely destroy seqscan performance, with
penalties very far exceeding what we might save by avoiding some kernel
calls.

I'd be more excited about this if the claimed improvement were more than
1.5%, but you know as well as I do that that's barely above the noise
floor for most performance measurements.  I'm left wondering why bother,
and why take any risk of de-optimizing on some platforms.

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] Use pread and pwrite instead of lseek + write and read

[Claudio Freire]

On Wed, Aug 17, 2016 at 10:40 AM, Tom Lane  wrote:
> Oskari Saarenmaa  writes:
>> On my laptop a simple pgbench run (scale 100, 15 minutes) shows a 1.5%
>> performance improvement.
>
> I would have hoped for a lot better result before anyone would propose
> that we should deal with all the portability issues this'll create.
>
>> A 1.5% performance improvement is small but
>> measurable - and IMV more importantly it allows us to drop more than 100
>> lines of backwards (compatible?) code; maybe we could start targeting
>> more recent platforms in v10?
>
> That's basically nonsense: we'll end up adding way more than that to
> deal with platforms that haven't got these APIs.

Perhaps a better target would then be to try and make use of readv and
writev, which should both be useful for sequential access of various
kinds and network I/O. For instance, when reading 10 sequential
buffers, a readv could fill 10 buffers at a time.

I remember a project where we got a linear improvement in thoughput by
using them for network I/O, because we were limited by packet
thoughput rather than byte thoughput, and using the iovec utilities
reduced the overhead considerably.

But all this is anecdotal evidence in any case, YMMV.


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

Re: [HACKERS] PATCH: Exclude additional directories in pg_basebackup

[Robert Haas]

On Wed, Aug 17, 2016 at 2:50 PM, David Steele  wrote:
> Hi Robert,
>
> On 8/17/16 11:27 AM, Robert Haas wrote:
>> On Mon, Aug 15, 2016 at 3:39 PM, David Steele  wrote:
>>> Recently a hacker proposed a patch to add pg_dynshmem to the list of
>>> directories whose contents are excluded in pg_basebackup.  I wasn't able
>>> to find the original email despite several attempts.
>>>
>>> That patch got me thinking about what else could be excluded and after
>>> some investigation I found the following: pg_notify, pg_serial,
>>> pg_snapshots, pg_subtrans.  These directories are all cleaned, zeroed,
>>> or rebuilt on server start.
>>
>> Eh ... I doubt very much that it's safe to blow away the entire
>> contents of an SLRU between shutdown and startup, even if the data is
>> technically transient data that won't be needed again after the system
>> is reset.
>
> I've done pretty extensive testing in pgBackRest and haven't seen issues
> in any supported version (plus I audited each init() function for every
> version back to where it was introduced).  The patch also passes all the
> pg_basebackup TAP tests in master.
>
> If you are correct it may indicate a problem anyway. Consider a standby
> backup where the files in these directories may be incredibly stale
> since they are not replicated.  Once restored to a master should we
> trust anything in these files?
>
> pg_serial, pg_notify, pg_subtrans are not even fsync'd
> (SlruCtl->do_fsync = false).  It's hard to imagine there's anything of
> value in there or that it can be trusted if there is.

It's not just a question of whether the data has value; it's a
question of whether the SLRU code will handle the situation correctly
in all cases if the directory contains no files.  I don't think you
can draw a firm conclusion on that without reading the code.

> The files in pg_snapshot and pg_dynshmem are simply deleted on startup
> so that seems safe enough.

Agreed.

-- 
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] Curing plpgsql's memory leaks for statement-lifespan values

[Tom Lane]

Pavel Stehule  writes:
> I am sending a review of this patch:
> ...
> I'll mark this patch as ready for commiter

Pushed, thanks for the review.

regards, tom lane


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

Re: [HACKERS] PATCH: Exclude additional directories in pg_basebackup

[David Steele]

Hi Robert,

On 8/17/16 11:27 AM, Robert Haas wrote:
> On Mon, Aug 15, 2016 at 3:39 PM, David Steele  wrote:
>> Recently a hacker proposed a patch to add pg_dynshmem to the list of
>> directories whose contents are excluded in pg_basebackup.  I wasn't able
>> to find the original email despite several attempts.
>>
>> That patch got me thinking about what else could be excluded and after
>> some investigation I found the following: pg_notify, pg_serial,
>> pg_snapshots, pg_subtrans.  These directories are all cleaned, zeroed,
>> or rebuilt on server start.
>
> Eh ... I doubt very much that it's safe to blow away the entire
> contents of an SLRU between shutdown and startup, even if the data is
> technically transient data that won't be needed again after the system
> is reset.

I've done pretty extensive testing in pgBackRest and haven't seen issues
in any supported version (plus I audited each init() function for every
version back to where it was introduced).  The patch also passes all the
pg_basebackup TAP tests in master.

If you are correct it may indicate a problem anyway. Consider a standby
backup where the files in these directories may be incredibly stale
since they are not replicated.  Once restored to a master should we
trust anything in these files?

pg_serial, pg_notify, pg_subtrans are not even fsync'd
(SlruCtl->do_fsync = false).  It's hard to imagine there's anything of
value in there or that it can be trusted if there is.

The files in pg_snapshot and pg_dynshmem are simply deleted on startup
so that seems safe enough.

-- 
-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] Are these supported??

[Greg Stark]

On Wed, Aug 17, 2016 at 4:22 PM, Robert Haas  wrote:
> We've supported having joins in a DELETE since PostgreSQL 8.1.

Er, yes. Though he does say he's trying to use the same syntax as select...

-- 
greg


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

Re: [HACKERS] Use pread and pwrite instead of lseek + write and read

[Robert Haas]

On Wed, Aug 17, 2016 at 9:40 AM, Tom Lane  wrote:
> Oskari Saarenmaa  writes:
>> On my laptop a simple pgbench run (scale 100, 15 minutes) shows a 1.5%
>> performance improvement.
>
> I would have hoped for a lot better result before anyone would propose
> that we should deal with all the portability issues this'll create.
>
>> A 1.5% performance improvement is small but
>> measurable - and IMV more importantly it allows us to drop more than 100
>> lines of backwards (compatible?) code; maybe we could start targeting
>> more recent platforms in v10?
>
> That's basically nonsense: we'll end up adding way more than that to
> deal with platforms that haven't got these APIs.

I don't understand why you think this would create non-trivial
portability issues.

-- 
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] C++ port of Postgres

[Gavin Flower]

On 17/08/16 23:40, Aleksander Alekseev wrote:

I'm sure this wasn't your intent, but the tone of your response is
part of why people don't get involved with Postgres development...

Please note that you're the only person in the entire thread that's
said anything to the effect of a holy war...

OTOH, if the community takes the stance of "WTF WHY DO WE NEED
THIS?!",  we've just driven Joy and anyone else that's a C++ fan away.

I'm sorry for being maybe to emotional. It's was not (and never is!) my
intent to offend anyone. Also I would like to note that I don't speak
for community, I speak for myself.

What I saw was: "hey, lets rewrite PostgreSQL in C++ without any good
reason except (see [1] list)". Naturally I though (and still think) that
you people are just trolls. Or maybe "everything should be written in
C++ because it's the only right language and anyone who thinks
otherwise is wrong" type of fanatics. Thus I don't think you are here to
help.

Give a concrete reason. Like "hey, we rewrote this part of code in C++
and look, its much more readable, twice as fast as code in C (how to do
benchmarks right is a separate good topic!) and it still compiles fast
even on Raspberry Pi, works on all platforms you are supporting, etc".
Or "hey, we solved xid wraparound problem once and for all, but solution
is in C++, so its for you to decide whether to merge it or not".

If you really want to help, just solve _real_ problems using instruments
you prefer instead of reposting obviously holly war topic from general@
mailing list.

[1] https://en.wikipedia.org/wiki/List_of_fallacies

My main language is Java, and there are a lot of very good reasons for 
rewriting Postgres in Java, but I'd never push that - as there are also 
many good reasons for NOT rewriting Postgres in Java!


I am not an expert in C++, but I'm interested in its development and 
growing usage.  I've read enough about C++ to think it worthwhile to 
consider rewriting Postgres in C++.  If I had time, I would get deeper 
into C++, but for now pressures push me towards getting deeper into 
JavaScript - despite having an intense dislike for JavaScript!


The first 2 languages I used commercially were FORTRAN & COBOL back in 
the 70's, and I've been paid to teacht C to experienced programmers.


As far as I am concerned, there is no one language that is perfect.  I 
have written programs i over 20 different languages.


So I did not suggest C++, because I'm a C++ Fanatic!!!


Cheers,
Gavin




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

Re: [HACKERS] PATCH: Exclude additional directories in pg_basebackup

[Alvaro Herrera]

Robert Haas wrote:

> Eh ... I doubt very much that it's safe to blow away the entire
> contents of an SLRU between shutdown and startup, even if the data is
> technically transient data that won't be needed again after the system
> is reset.

Hmm.  At least async.c (pg_notify) deletes the whole directory at
startup so it's fine, but for the others (pg_serial, pg_subtrans) I
think it'd be saner to keep any "active" files (probably just the
latest).  I don't remember how pg_snapshot works, but it's probably fine
to start with an empty subdir (is it possible to export a snapshot from
a prepared transaction?)

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


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

Re: [HACKERS] drop src/backend/port/darwin/system.c ?

[Tom Lane]

Robert Haas  writes:
> On Wed, Aug 17, 2016 at 9:55 AM, Tom Lane  wrote:
>> Peter Eisentraut  writes:
>>> /* only needed in OS X 10.1 and possibly early 10.2 releases */
>>> Maybe it's time to let it go?

>> One part of me says it's not hurting anything, but another part
>> says that if it were broken we wouldn't know.  And it looks like
>> we can drop that whole subdirectory if we kill it, so let's.

> I've discovered that when I build the source tree with coverage
> enabled, the fact that that directory ends up containing a .o file
> with no code in it breaks things.  This is no doubt a bug in some part
> of the code coverage toolchain, but all the same I'll be happy if that
> file can go away.

Ah, well then it *is* hurting something.

Also, the early releases of OS X were rough enough that it's pretty hard
to believe anyone is still using them anywhere (certainly the buildfarm
isn't).  So the odds of anyone caring if we remove this file seem
negligible.  Let's nuke it.

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] drop src/backend/port/darwin/system.c ?

[Robert Haas]

On Wed, Aug 17, 2016 at 9:55 AM, Tom Lane  wrote:
> Peter Eisentraut  writes:
>> /* only needed in OS X 10.1 and possibly early 10.2 releases */
>> Maybe it's time to let it go?
>
> One part of me says it's not hurting anything, but another part
> says that if it were broken we wouldn't know.  And it looks like
> we can drop that whole subdirectory if we kill it, so let's.

I've discovered that when I build the source tree with coverage
enabled, the fact that that directory ends up containing a .o file
with no code in it breaks things.  This is no doubt a bug in some part
of the code coverage toolchain, but all the same I'll be happy if that
file can go away.

-- 
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] C++ port of Postgres

[Robert Haas]

On Wed, Aug 17, 2016 at 11:36 AM, Andrew Gierth
 wrote:
>> "Robert" == Robert Haas  writes:
>
>  Robert> Hmm, so sizeof() has different semantics in C vs. C++?
>
> No. '1' has different semantics in C vs C++. (In C, '1' is an int,
> whereas in C++ it's a char. It so happens that (sizeof '1') is the only
> case which is valid in both C and C++ where this makes a difference.)

OK.  Doesn't seem like a big problem.

-- 
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] Assertion failure in REL9_5_STABLE

[Alvaro Herrera]

Marko Tiikkaja wrote:

> Running one specific test from our application against REL9_5_STABLE
> (current as of today) gives me this:
> 
> #2  0x7effb59595be in ExceptionalCondition (
> conditionName=conditionName@entry=0x7effb5b27a88 "!(CritSectionCount > 0
> || TransactionIdIsCurrentTransactionId(( (!((tup)->t_infomask & 0x0800) &&
> ((tup)->t_infomask & 0x1000) && !((tup)->t_infomask & 0x0080)) ?
> HeapTupleGetUpdateXid(tup) : ( (tup)-"...,
> errorType=errorType@entry=0x7effb599b74b "FailedAssertion",
> fileName=fileName@entry=0x7effb5b2796c "combocid.c",
> lineNumber=lineNumber@entry=132) at assert.c:54
> #3  0x7effb598e19b in HeapTupleHeaderGetCmax (tup=0x7effa85714c8) at
> combocid.c:131

Can you please do
  frame 3
  print tup->t_ctid
in gdb?  That would print the tuple address with which to grep the xlog
sequence.

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


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

Re: [HACKERS] LWLocks in DSM memory

[Andres Freund]

On 2016-08-17 08:31:36 -0400, Robert Haas wrote:
> On Tue, Aug 16, 2016 at 5:03 PM, Andres Freund  wrote:
> > On 2016-08-15 18:15:23 -0400, Robert Haas wrote:
> >> On Thu, Aug 11, 2016 at 2:19 PM, Robert Haas  wrote:
> >> > Therefore, I plan to commit this patch, removing the #include
> >> >  unless someone convinces me we need it, shortly after
> >> > development for v10 opens, unless there are objections before then.
> >>
> >> Hearing no objections, done.
> >
> > I'd have objected, if I hadn't been on vacation.  While I intuitively
> > *do* think that the increased wait-list overhead won't be relevant, I
> > also know that my intuition has frequently been wrong around the lwlock
> > code.  This needs some benchmarks on a 4+ socket machine,
> > first. Something exercising the slow path obviously. E.g. a pgbench with
> > a small number of writers, and a large number of writers.
> 
> Amit just pointed out to me that you wrote "a small number of writers,
> and a large number of writers".  I assume one of those is supposed to
> say "readers"?  Probably the second one?

Yes. I want a long wait list, modified in bulk - which should be the
case with the above.


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

Re: [HACKERS] LWLocks in DSM memory

[Andres Freund]

On 2016-08-16 23:09:07 -0400, Robert Haas wrote:
> On Tue, Aug 16, 2016 at 5:03 PM, Andres Freund  wrote:
> > On 2016-08-15 18:15:23 -0400, Robert Haas wrote:
> >> On Thu, Aug 11, 2016 at 2:19 PM, Robert Haas  wrote:
> >> > Therefore, I plan to commit this patch, removing the #include
> >> >  unless someone convinces me we need it, shortly after
> >> > development for v10 opens, unless there are objections before then.
> >>
> >> Hearing no objections, done.
> >
> > I'd have objected, if I hadn't been on vacation.  While I intuitively
> > *do* think that the increased wait-list overhead won't be relevant, I
> > also know that my intuition has frequently been wrong around the lwlock
> > code.  This needs some benchmarks on a 4+ socket machine,
> > first. Something exercising the slow path obviously. E.g. a pgbench with
> > a small number of writers, and a large number of writers.
> 
> I have to admit that I totally blanked about you being on vacation.
> Thanks for mentioning the workload you think might be adversely
> affected, but to be honest, even if there's some workload where this
> causes a small regression, I'm not really sure what you think we
> should do instead.

Well, you convincingly argued against that approach in a nearby thread
;). Joking aside: I do think that we should make such a change
knowingly. It might also be possible to address it somehow.

I really hope there's no slowdown.


> Should we have a separate copy of lwlock.c just
> for parallel query and other stuff that uses DSM?

No, that'd be horrible.


> Or are you going to argue that parallel query doesn't really need
> LWLocks?

Definitely not.


- Andres


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

Re: [HACKERS] [Patch] RBTree iteration interface improvement

[Aleksander Alekseev]

> > Having said that, though: if the iteration state is not part of the
> > object, it's not very clear whether we can behave sanely if someone
> > changes the tree while an iteration is open.  It will need careful
> > thought as to what sort of guarantees we can make about that.  If
> > it's too weak, then a separated-state version would have enough
> > hazards of its own that it's not necessarily any safer.  
> 
> +1 to all of that.
> 

The old API assumes that tree doesn't change during iteration. And it
doesn't do any checks about this. The new API behaves the same way. In
this sense patch at least doesn't make anything worse.

> It seems clear to me that the existing arrangement is hazardous for
> any RBTree that hasn't got exactly one consumer.  I think
> Aleksander's plan to decouple the iteration state is probably a good
> one (NB: I've not read the patch, so this is not an endorsement of
> details).  I'd go so far as to say that we should remove the old API
> as being dangerous, rather than preserve it on
> backwards-compatibility grounds.  We make bigger changes than that in
> internal APIs all the time.

Glad to hear it! I would like to propose a patch that removes the old
API. I have a few questions though.

Would you like it to be a separate patch, or all-in-one patch is fine
in this case? I also would like to include unit/property-based tests in
this (these) patch (patches). However as I understand there are
currently no unit tests in PostgreSQL at all, only integration/system
tests. Any advice how to do it better?

-- 
Best regards,
Aleksander Alekseev


-- 
Sent 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] C++ port of Postgres

[Andrew Gierth]

> "Robert" == Robert Haas  writes:

 Robert> Hmm, so sizeof() has different semantics in C vs. C++?

No. '1' has different semantics in C vs C++. (In C, '1' is an int,
whereas in C++ it's a char. It so happens that (sizeof '1') is the only
case which is valid in both C and C++ where this makes a difference.)

-- 
Andrew (irc:RhodiumToad)


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

Re: [HACKERS] PATCH: Exclude additional directories in pg_basebackup

[Robert Haas]

On Mon, Aug 15, 2016 at 3:39 PM, David Steele  wrote:
> Recently a hacker proposed a patch to add pg_dynshmem to the list of
> directories whose contents are excluded in pg_basebackup.  I wasn't able
> to find the original email despite several attempts.
>
> That patch got me thinking about what else could be excluded and after
> some investigation I found the following: pg_notify, pg_serial,
> pg_snapshots, pg_subtrans.  These directories are all cleaned, zeroed,
> or rebuilt on server start.

Eh ... I doubt very much that it's safe to blow away the entire
contents of an SLRU between shutdown and startup, even if the data is
technically transient data that won't be needed again after the system
is reset.

-- 
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] Are these supported??

[Robert Haas]

On Wed, Aug 17, 2016 at 10:38 AM, Greg Stark  wrote:
> You can't have joins in a DELETE -- which table would it actually
> delete from? You can use a subselect to do look up information from
> other tables in your delete though.

We've supported having joins in a DELETE since PostgreSQL 8.1.

https://www.postgresql.org/docs/8.1/static/sql-delete.html

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


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

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

[Ryan Murphy]

I have created a better patch (attached) that correctly escapes the shell
arguments using PQExpBufferStr and the appendShellString function, as per
Michael and Andres' suggestions.

Further suggestions welcome of course.

Ryan

On Wed, Aug 17, 2016 at 8:28 AM, Ryan Murphy  wrote:

> That makes sense, Michael and Andres.
>
> I started to make a solution that uses a PQExpBuffer, appendShellString,
> etc.  I think it will work just fine, but I think I need to alter the
> Makefile as well, to get initdb.c to be compiled using
> -L../../../src/fe_utils -lpgfeutils.  Otherwise I am having issues linking:
>
> gcc -Wall -Wmissing-prototypes -Wpointer-arith
> -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute
> -Wformat-security -fno-strict-aliasing -fwrapv 
> -Wno-unused-command-line-argument
> -O2 initdb.o findtimezone.o localtime.o encnames.o  -L../../../src/port
> -L../../../src/common -Wl,-dead_strip_dylibs   -lpgcommon -lpgport -lz
> -lreadline -lm  -o initdb
> Undefined symbols for architecture x86_64:
>   "_appendPQExpBufferStr", referenced from:
>   _main in initdb.o
>   "_appendShellString", referenced from:
>   _main in initdb.o
>   "_createPQExpBuffer", referenced from:
>   _main in initdb.o
>   "_destroyPQExpBuffer", referenced from:
>   _main in initdb.o
> ld: symbol(s) not found for architecture x86_64
> clang: error: linker command failed with exit code 1 (use -v to see
> invocation)
>
> On Tue, Aug 16, 2016 at 10:00 PM, Michael Paquier <
> michael.paqu...@gmail.com> wrote:
>
>> On Wed, Aug 17, 2016 at 8:05 AM, Andres Freund 
>> wrote:
>> > ISTM that the correct fix would be to actually introduce something like
>> > quote_path_for_shell() which either adds proper quotes, or fails if
>> > that'd be hard (e.g. if the path contains quotes, and we're on
>> > windows).
>>
>> You are looking for appendShellString in fe_utils/string_utils.c.
>> --
>> Michael
>>
>
>


0001-initdb-quote-shell-args-in-final-pg_ctl-command.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] [GENERAL] C++ port of Postgres

[Robert Haas]

On Tue, Aug 16, 2016 at 5:08 PM, Piotr Stefaniak
 wrote:
> On 2016-08-16 18:33, Robert Haas wrote:
>> It wouldn't be that much work to maintain, either: we'd
>> just set up some buildfarm members that compiled using C++ and when
>> they turned red, we'd go fix it.
>
> I think that there exist subtle differences between C and C++ that
> without compile-time diagnostic could potentially lead to different
> run-time behavior. As an artificial example:
>
> $ cat ./test.c
> #include 
>
> int main(void) {
> FILE *f = fopen("test.bin", "w");
> if (f == NULL)
> return 1;
> fwrite("1", sizeof '1', 1, f);
> fclose(f);
> return 0;
> }
> $ clang ./test.c -o test
> $ ./test
> $ hexdump test.bin
> 000 0031 
> 004
> $ clang++ ./test.c -o test
> clang-3.9: warning: treating 'c' input as 'c++' when in C++ mode, this
> behavior is deprecated
> $ ./test
> $ hexdump test.bin
> 000 0031
> 001

Hmm, so sizeof() has different semantics in C vs. C++?

While that's a little alarming, I'm wondering whether this sort of
thing is likely to actually be a problem in practice.  We have such a
long laundry list of coding conventions already that I am inclined to
believe we could add a few more without breaking our ability to do
development.

-- 
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] C++ port of Postgres

[Dmitry Igrishin]

2016-08-17 14:40 GMT+03:00 Aleksander Alekseev :
>> I'm sure this wasn't your intent, but the tone of your response is
>> part of why people don't get involved with Postgres development...
>>
>> Please note that you're the only person in the entire thread that's
>> said anything to the effect of a holy war...
>>
>> OTOH, if the community takes the stance of "WTF WHY DO WE NEED
>> THIS?!",  we've just driven Joy and anyone else that's a C++ fan away.
>
> I'm sorry for being maybe to emotional. It's was not (and never is!) my
> intent to offend anyone. Also I would like to note that I don't speak
> for community, I speak for myself.
>
> What I saw was: "hey, lets rewrite PostgreSQL in C++ without any good
> reason except (see [1] list)". Naturally I though (and still think) that
> you people are just trolls. Or maybe "everything should be written in
> C++ because it's the only right language and anyone who thinks
> otherwise is wrong" type of fanatics. Thus I don't think you are here to
> help.
>
> Give a concrete reason. Like "hey, we rewrote this part of code in C++
> and look, its much more readable, twice as fast as code in C (how to do
> benchmarks right is a separate good topic!) and it still compiles fast
> even on Raspberry Pi, works on all platforms you are supporting, etc".
> Or "hey, we solved xid wraparound problem once and for all, but solution
> is in C++, so its for you to decide whether to merge it or not".
I doubt that someone will rush to rewrite PostgreSQL in C++. At now, the
reason to consider the refactoring of current codebase to make the C++
compilers happy, is to make the code more qualitative. I think, that only after
that step it is reasonable to consider the use some of C++ features.


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

Re: [HACKERS] Are these supported??

[Craig Ringer]

On 17 August 2016 at 11:33, Vince Vielhaber  wrote:

>
> I recently moved a mybb forum away from mysql to postgres.  Along the way
> I encountered a couple of things that either didn't seem to be supported or
> I'm just not doing it right. First, the server this is on is running
> version 8.4.22
>
>
8.4 is end-of-life, so it's time to move.

FYI, this isn't really the right place for these questions; pgsql-general
or Stack Overflow is more appropriate for topics not relating to PostgreSQL
code and design.

#1 Are joins supported in deletes?  The same join syntax works fine as
> a select.
>

Yes, but the syntax is a bit different and you can't use aliases on the
target table. You can also (unfortunately) only do inner joins.


> #2 is extract supported in a select statement dealing with a table?  To
> explain this one, here is the error I get:
>

If 'dateline' is an integer, you'll have to turn it into a timestamp or
date before you can extract the epoch.


> # select extract(epoch from timestamp dateline::timestamp) from
> mybb_adminlog limit 1;
>

You can't use the typed-literal syntax

TIMESTAMP 'something'

for a column reference, bind-parameter, etc. You can only use it for
literals. Use a CAST or the PostgreSQL :: shorthand. Just remove the
"timestamp" from "FROM timestamp".

Doesn't matter if I use epoch or day or anything else, they all fail with
> the same error.  And yes, dateline is a timestamp.


No, it isn't. The error message says so.

Please use pgsql-general or Stack Overflow.

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

Re: [HACKERS] Use pread and pwrite instead of lseek + write and read

[Oskari Saarenmaa]

17.08.2016, 16:40, Tom Lane kirjoitti:

Oskari Saarenmaa  writes:

On my laptop a simple pgbench run (scale 100, 15 minutes) shows a 1.5%
performance improvement.


I would have hoped for a lot better result before anyone would propose
that we should deal with all the portability issues this'll create.


AFAICT pread and pwrite are available on pretty much all operating 
systems released in 2000s; it was added to Linux in 1997.  Windows and 
HP-UX 10.20 don't have it, but we can just simulate it using lseek + 
read/write there without adding too much code.



A 1.5% performance improvement is small but
measurable - and IMV more importantly it allows us to drop more than 100
lines of backwards (compatible?) code; maybe we could start targeting
more recent platforms in v10?


That's basically nonsense: we'll end up adding way more than that to
deal with platforms that haven't got these APIs.


Attached an updated patch that adds a configure check and uses 
lseek+read/write instead pread/pwrite when the latter aren't available. 
The previous code ended up seeking anyway in most of the cases and 
pgbench shows no performance regression on my Linux box.


 8 files changed, 54 insertions(+), 168 deletions(-)

/ Oskari
diff --git configure configure
index 45c8eef..4d50b52 100755
--- configure
+++ configure
@@ -12473,7 +12473,7 @@ fi
 LIBS_including_readline="$LIBS"
 LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
 
-for ac_func in cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll pstat pthread_is_threaded_np readlink setproctitle setsid shm_open symlink sync_file_range towlower utime utimes wcstombs wcstombs_l
+for ac_func in cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll pread pstat pthread_is_threaded_np pwrite readlink setproctitle setsid shm_open symlink sync_file_range towlower utime utimes wcstombs wcstombs_l
 do :
   as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
 ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
diff --git configure.in configure.in
index c878b4e..5eddaca 100644
--- configure.in
+++ configure.in
@@ -1446,7 +1446,7 @@ PGAC_FUNC_WCSTOMBS_L
 LIBS_including_readline="$LIBS"
 LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
 
-AC_CHECK_FUNCS([cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll pstat pthread_is_threaded_np readlink setproctitle setsid shm_open symlink sync_file_range towlower utime utimes wcstombs wcstombs_l])
+AC_CHECK_FUNCS([cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll pread pstat pthread_is_threaded_np pwrite readlink setproctitle setsid shm_open symlink sync_file_range towlower utime utimes wcstombs wcstombs_l])
 
 AC_REPLACE_FUNCS(fseeko)
 case $host_os in
diff --git src/backend/access/heap/rewriteheap.c src/backend/access/heap/rewriteheap.c
index f9ce986..6dc5df3 100644
--- src/backend/access/heap/rewriteheap.c
+++ src/backend/access/heap/rewriteheap.c
@@ -918,7 +918,7 @@ logical_heap_rewrite_flush_mappings(RewriteState state)
 		 * Note that we deviate from the usual WAL coding practices here,
 		 * check the above "Logical rewrite support" comment for reasoning.
 		 */
-		written = FileWrite(src->vfd, waldata_start, len);
+		written = FileWriteAt(src->vfd, waldata_start, len, src->off);
 		if (written != len)
 			ereport(ERROR,
 	(errcode_for_file_access(),
diff --git src/backend/storage/file/buffile.c src/backend/storage/file/buffile.c
index 042be79..39482fa 100644
--- src/backend/storage/file/buffile.c
+++ src/backend/storage/file/buffile.c
@@ -60,12 +60,6 @@ struct BufFile
 	int			numFiles;		/* number of physical files in set */
 	/* all files except the last have length exactly MAX_PHYSICAL_FILESIZE */
 	File	   *files;			/* palloc'd array with numFiles entries */
-	off_t	   *offsets;		/* palloc'd array with numFiles entries */
-
-	/*
-	 * offsets[i] is the current seek position of files[i].  We use this to
-	 * avoid making redundant FileSeek calls.
-	 */
 
 	bool		isTemp;			/* can only add files if this is TRUE */
 	bool		isInterXact;	/* keep open over transactions? */
@@ -108,8 +102,6 @@ makeBufFile(File firstfile)
 	file->numFiles = 1;
 	file->files = (File *) palloc(sizeof(File));
 	file->files[0] = firstfile;
-	file->offsets = (off_t *) palloc(sizeof(off_t));
-	file->offsets[0] = 0L;
 	file->isTemp = false;
 	file->isInterXact = false;
 	file->dirty = false;
@@ -143,10 +135,7 @@ extendBufFile(BufFile *file)
 
 	file->files = (File *) repalloc(file->files,
 	(file->numFiles + 1) * sizeof(File));
-	file->offsets = (off_t *) repalloc(file->offsets,
-	   (file->numFiles + 1) * sizeof(off_t));
 	file->files[file->numFiles] = pfile;
-	file->offsets[file->numFiles] = 0L;
 	file->numFiles++;
 }
 
@@ -210,7 +199,6 @@ BufFileClose(BufFile *file)
 		FileClose(file->files[i]);
 	/* release the buffer space */
 	pfree(file->files);
-	pfree(file->offsets);
 	pfree(file);
 }
 

Re: [HACKERS] Are these supported??

[Greg Stark]

On Wed, Aug 17, 2016 at 4:33 AM, Vince Vielhaber  
wrote:
>
> I recently moved a mybb forum away from mysql to postgres.  Along the way I
> encountered a couple of things that either didn't seem to be supported or
> I'm just not doing it right.
>
> First, the server this is on is running version 8.4.22.  php is 5.6.22.


8.4 is very old. It's been unsupported for two years already.

You can't have joins in a DELETE -- which table would it actually
delete from? You can use a subselect to do look up information from
other tables in your delete though.

EXTRACT and date_part have no idea where the data they're passed came
from. They can come from tables or other functions or expressions. The
error you quoted is indicating that dateline is of type integer
however. The syntax for EXTRACT is confusing (blame the SQL
committee...) but you don't want the extra "timestamp" keyword before
the column there -- in the examples that's part of the literal being
used to have it be read as a timestamp.

https://www.postgresql.org/docs/8.4/static/functions-datetime.html#FUNCTIONS-DATETIME-EXTRACT


-- 
greg


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

Re: [HACKERS] Why we lost Uber as a user

[Craig Ringer]

On 17 August 2016 at 21:35, Bruce Momjian  wrote:


>
> > I saw from the Uber article that they weren't going to per-row logical
> > replication but _statement_ replication, which is very hard to do
> > because typical SQL doesn't record what concurrent transactions
> > committed before a new statement's transaction snapshot is taken, and
> > doesn't record lock order for row updates blocked by concurrent activity
> > --- both of which affect the final result from the query.
>
> I assume they can do SQL-level replication when there is no other
> concurrent activity on the table, and row-based in other cases?


I don't know, but wouldn't want to assume that. A quick search suggests
they probably define that away as nondeterministic behaviour that's allowed
to cause master/replica differences, but no time to look deeply.

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

Re: [HACKERS] Bug in to_timestamp().

[amul sul]

On Wednesday, August 17, 2016 5:15 PM, Artur Zakirov  
wrote:
>I attached new patch "0001-to-timestamp-format-checking-v2.patch". It 
>fixes behaviour for Amul's scenarious:

>
Great.
>
>> Following are few scenarios where we break existing behaviour:
>>
>> SELECT TO_TIMESTAMP('2015-12-31 13:43:36', ' MM DD HH24 MI SS');
>> SELECT TO_TIMESTAMP('2011$03!18 23_38_15', '-MM-DD HH24:MI:SS');
>> SELECT TO_TIMESTAMP('2011*03*18 23^38&15', '-MM-DD HH24:MI:SS');
>> SELECT TO_TIMESTAMP('2011*03!18 #%23^38$15', '-MM-DD$$$HH24:MI:SS');
>>
>> But current patch behaviour is not that much bad either at least we have 
>> errors, but I am not sure about community acceptance.
>>
>> I would like to divert communities' attention on following case:
>> SELECT TO_TIMESTAMP('2013--10-01', '-MM-DD');
>
>
>For queries above the patch gives an output without any error.
>
>
>> Another is, shouldn’t we have error in following cases?
>> SELECT TO_TIMESTAMP('2016-06-13 99:99:99', '-MM-DD HH24:MI:SS');
>> SELECT TO_TIMESTAMP('2016-02-30 15:43:36', '-MM-DD HH24:MI:SS');
>
>
>I attached second patch "0002-to-timestamp-validation-v2.patch". With it 
>PostgreSQL perform additional checks for date and time. But as I wrote 
>there is another patch in the thread "to_date_valid()" wich differs from 
>this patch.

>

Hmm. I haven't really looked into the code, but with applying both patches it 
looks precisely imitate Oracle's behaviour. Thanks.

Regards,
Amul Sul


-- 
Sent 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] C++ port of Postgres

[Serge Rielau]

> On Aug 16, 2016, at 10:16 PM, Craig Ringer  wrote:
> 
> On 17 August 2016 at 09:49, Andres Freund  > wrote:
>  
> 
> You need to include the files surrounded by extern "C" { }.
> 
> I'd really like to adopt the convention used by many libraries etc of doing 
> this automatically - detecting a c++ compiler in the preprocessor and 
> wrapping in "extern "C"" .
> 
> Having the codebase c++-clean enough to compile with a c++ compiler seems to 
> be the easiest way to maintain that, but means more "extern "C"" droppings in 
> the .c files, not just the headers. Still, pretty ignoreable.
> 
Big +1 here,
Just having community code compilable with a C++ compiler out of the box would 
go a long way.

Beyond that, on my end I have been working with PG now for a year and a half 
and here is a quick list of what I sorely miss from my C++ days:
* Overloading of functions (same as in SQL) keeps naming clean
* Named parameters (same as SQL) keeps code readable
* Adding new function parameters with defaults so I don’t need to pass in NULL, 
0, … at 20 places (again supported in SQL)
* Member functions greatly help organize code
* simple inheritance (as emulated today in node types)

At my old employer we used C++ for the DBMS in various degrees in different 
components.
That degree was agreed upon in coding standards, so we could pick what we like 
about C++ and blacklist what we didn’t.
E.g. C style exception handling was prohibited
Default memory management (new) was prohibited.
Instead new() was overloaded and hooked into the DBMS memory manager.
I see no reason why this couldn’t be done in PG.

I can’t comment of compiling on a Rasperry PI, but know that my former DBMS 
code compiled and ran on Windows, Linux, AIX, Sun, HP, and Mac.

But again, just having the community code compile so proprietary (for now) 
enhancements could be written in C++ would be huge.


Cheers
Serge
  

Re: [HACKERS] drop src/backend/port/darwin/system.c ?

[Tom Lane]

Peter Eisentraut  writes:
> /* only needed in OS X 10.1 and possibly early 10.2 releases */
> Maybe it's time to let it go?

One part of me says it's not hurting anything, but another part
says that if it were broken we wouldn't know.  And it looks like
we can drop that whole subdirectory if we kill it, so let's.

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] Why we lost Uber as a user

[Bruce Momjian]

On Wed, Aug 17, 2016 at 01:27:18PM +0800, Craig Ringer wrote:
> It's really bugging me that people are talking about "statement based"
> replication in MySQL as if it's just sending SQL text around. MySQL's 
> statemnet
> based replication is a lot smarter than that, and in the
> actually-works-properly form it's a hybrid of row and statement based
> replication ("MIXED" mode). As I understand it it lobs around something closer
> to parsetrees with some values pre-computed rather than SQL text where
> possible. It stores some computed values of volatile functions in the binlog
> and reads them from there rather than computing them again when running the
> statement on replicas, which is why AUTO_INCREMENT etc works. It also falls
> back to row based replication where necessary for correctness. Even then it 
> has
> a significant list of caveats, but it's pretty damn impressive. I didn't
> realise how clever the hybrid system was until recently.
> 
> I can see it being desirable to do something like that eventually as an
> optimisation to logical decoding based replication. Where we can show that the
> statement is safe or make it safe by doing things like evaluating and
> substituting volatile function calls, xlog a modified parsetree with oids
> changed to qualified object names etc, send that when decoding, and execute
> that on the downstream(s). If there's something we can't show to be safe then
> replay the logical rows instead. That's way down the track though; I think 
> it's
> more important to focus on completing logical row-based replication to the
> point where we handle table rewrites seamlessly and it "just works" first.

That was very interesting, and good to know.  I assume it also covers
concurrent activity issues which I wrote about in this thread, e.g.

> I saw from the Uber article that they weren't going to per-row logical
> replication but _statement_ replication, which is very hard to do
> because typical SQL doesn't record what concurrent transactions
> committed before a new statement's transaction snapshot is taken, and
> doesn't record lock order for row updates blocked by concurrent activity
> --- both of which affect the final result from the query.

I assume they can do SQL-level replication when there is no other
concurrent activity on the table, and row-based in other cases?

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


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

Re: [HACKERS] Use pread and pwrite instead of lseek + write and read

[Tom Lane]

Oskari Saarenmaa  writes:
> On my laptop a simple pgbench run (scale 100, 15 minutes) shows a 1.5% 
> performance improvement.

I would have hoped for a lot better result before anyone would propose
that we should deal with all the portability issues this'll create.

> A 1.5% performance improvement is small but 
> measurable - and IMV more importantly it allows us to drop more than 100 
> lines of backwards (compatible?) code; maybe we could start targeting 
> more recent platforms in v10?

That's basically nonsense: we'll end up adding way more than that to
deal with platforms that haven't got these APIs.

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] How to do failover in pglogical replication?

[roshan_myrepublic]

Hi,

I am currently exploring pglogical replication for my db servers. I would
like to know how can I automatically failover from Provider Node to
Subscriber Node, if the Provider node goes down for some reasons. How can I
redirect all the traffic to SubscriberNode automatically ? In the normal
replication, we use recovery_file and triggers to get this job done. Do we
have any similar alternative for pglogical replications as well?

Regards,
Roshan



--
View this message in context: 
http://postgresql.nabble.com/How-to-do-failover-in-pglogical-replication-tp5916769.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.


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

[HACKERS] Are these supported??

[Vince Vielhaber]

Hi all, been awhile!  Some may be saying "not long enough" but eh.

I recently moved a mybb forum away from mysql to postgres.  Along the way 
I encountered a couple of things that either didn't seem to be supported 
or I'm just not doing it right.


First, the server this is on is running version 8.4.22.  php is 5.6.22.

#1 Are joins supported in deletes?  The same join syntax works fine as
a select.

#2 is extract supported in a select statement dealing with a table?  To 
explain this one, here is the error I get:


# select date_part('epoch', dateline) from mybb_adminlog limit 1;ERROR: 
function date_part(unknown, integer) does not exist

LINE 1: select date_part('epoch', dateline) from mybb_adminlog limit...
   ^
HINT:  No function matches the given name and argument types. You might 
need to add explicit type casts.

#

or ...

# select extract(epoch from timestamp dateline::timestamp) from 
mybb_adminlog limit 1;

ERROR:  syntax error at or near "dateline"
LINE 1: select extract(epoch from timestamp dateline::timestamp) fro...
#

Doesn't matter if I use epoch or day or anything else, they all fail with 
the same error.  And yes, dateline is a timestamp.  WITH or WITHOUT 
timezone made no difference.


So my questions are..

Does Postgres not support joins in deletes?

If not, is there a reason?


Is EXTRACT and/or date_part not supported in select calls where a table is 
involved?


If not, is there a reason or a work around for selecting the epoch of a 
timestamp?



Thanks!!!  Miss you guys!!!
Vince.




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

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

[Ryan Murphy]

That makes sense, Michael and Andres.

I started to make a solution that uses a PQExpBuffer, appendShellString,
etc.  I think it will work just fine, but I think I need to alter the
Makefile as well, to get initdb.c to be compiled using
-L../../../src/fe_utils -lpgfeutils.  Otherwise I am having issues linking:

gcc -Wall -Wmissing-prototypes -Wpointer-arith
-Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute
-Wformat-security -fno-strict-aliasing -fwrapv
-Wno-unused-command-line-argument -O2 initdb.o findtimezone.o localtime.o
encnames.o  -L../../../src/port -L../../../src/common
-Wl,-dead_strip_dylibs   -lpgcommon -lpgport -lz -lreadline -lm  -o initdb
Undefined symbols for architecture x86_64:
  "_appendPQExpBufferStr", referenced from:
  _main in initdb.o
  "_appendShellString", referenced from:
  _main in initdb.o
  "_createPQExpBuffer", referenced from:
  _main in initdb.o
  "_destroyPQExpBuffer", referenced from:
  _main in initdb.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see
invocation)

On Tue, Aug 16, 2016 at 10:00 PM, Michael Paquier  wrote:

> On Wed, Aug 17, 2016 at 8:05 AM, Andres Freund  wrote:
> > ISTM that the correct fix would be to actually introduce something like
> > quote_path_for_shell() which either adds proper quotes, or fails if
> > that'd be hard (e.g. if the path contains quotes, and we're on
> > windows).
>
> You are looking for appendShellString in fe_utils/string_utils.c.
> --
> Michael
>

Re: [HACKERS] patch proposal

[Stephen Frost]

Venkata,

* Venkata B Nagothi (nag1...@gmail.com) wrote:
> Agreed. Additional option like "pause" would. As long as there is an option
> to ensure following happens if the recovery target is not reached -
> 
>  a) PG pauses the recovery at the end of the WAL
>  b) Generates a warning in the log file saying that recovery target point
> is not reached (there is a patch being worked upon on by Thom on this)
>  c) Does not open-up the database exiting from the recovery process by
> giving room to resume the replay of WALs

One thing to consider is just how different this is from simply bringing
PG up as a warm standby instead, with the warning added to indicate if
the recovery point wasn't reached.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] EXLCUDE constraints and Hash indexes

[Andrew Gierth]

> "Jeff" == Jeff Janes  writes:

 Jeff> From: https://www.postgresql.org/docs/9.4/static/sql-createtable.html

 Jeff> "The access method must support amgettuple (see Chapter 55); at
 Jeff> present this means GIN cannot be used. Although it's allowed, there is
 Jeff> little point in using B-tree or hash indexes with an exclusion
 Jeff> constraint, because this does nothing that an ordinary unique
 Jeff> constraint doesn't do better. So in practice the access method will
 Jeff> always be GiST or SP-GiST."

I also recently found a case where using btree exclusion constraints was
useful: a unique index on an expression can't be marked deferrable, but
the equivalent exclusion constraint can be.

-- 
Andrew (irc:RhodiumToad)


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

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

[Robert Haas]

On Tue, Aug 16, 2016 at 5:25 PM, Rahila Syed  wrote:
>>I think I like the option of having psql issue an error.  On the
>>server side, the transaction would still be open, but the user would
>>receive a psql error message and the autocommit setting would not be
>>changed.  So the user could type COMMIT or ROLLBACK manually and then
>>retry changing the value of the setting.
>
> Throwing psql error comes out to be most accepted outcome on this thread. I
> agree it is safer than guessing user intention.
>
> Although according to the default behaviour of psql, error will abort the
> current transaction and roll back all the previous commands.

A server error would do that, but a psql errror won't.

-- 
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] Pluggable storage

[Anastasia Lubennikova]

13.08.2016 02:15, Alvaro Herrera:

Many have expressed their interest in this topic, but I haven't seen any
design of how it should work.  Here's my attempt; I've been playing with
this for some time now and I think what I propose here is a good initial
plan.  This will allow us to write permanent table storage that works
differently than heapam.c.  At this stage, I haven't throught through
whether this is going to allow extensions to define new storage modules;
I am focusing on AMs that can coexist with heapam in core.

The design starts with a new row type in pg_am, of type "s" (for "storage").
The handler function returns a struct of node StorageAmRoutine.  This
contains functions for 1) scans (beginscan, getnext, endscan) 2) tuples
(tuple_insert/update/delete/lock, as well as set_oid, get_xmin and the
like), and operations on tuples that are part of slots (tuple_deform,
materialize).

To support this, we introduce StorageTuple and StorageScanDesc.
StorageTuples represent a physical tuple coming from some storage AM.
It is necessary to have a pointer to a StorageAmRoutine in order to
manipulate the tuple.  For heapam.c, a StorageTuple is just a HeapTuple.


StorageTuples concept looks really cool. I've got some questions on
details of implementation.

Do StorageTuples have fields common to all implementations?
Or StorageTuple is totally abstract structure that has nothing to do
with data, except pointing to it?

I mean, now we already have HeapTupleData structure, which is a pretty
good candidate to replace with StorageTuple.
It's already widely used in executor and moreover, it's the only structure
(except MinimalTuples and all those crazy optimizations) that works with
tuples, both extracted from the page or created on-the-fly in executor node.

typedef struct HeapTupleData
{
uint32t_len;   /* length of *t_data */
ItemPointerData t_self;/* SelfItemPointer */
Oidt_tableOid; /* table the tuple came from */
HeapTupleHeader t_data;/* -> tuple header and data */
} HeapTupleData;

We can simply change t_data type from HeapTupleHeader to Pointer.
And maybe add a "t_handler" field that points out to handler functions.
I don't sure if it will be a name of StorageAm, or its OID, or maybe the
main function itself. Although, If I'm not mistaken, we always have
RelationData when we want to operate the tuple, so having t_handler
in the StorageTuple is excessive.


typedef struct StorageTupleData
{
uint32t_len; /* length of *t_data */
ItemPointerData   t_self;/* SelfItemPointer */
Oid   t_tableOid;/* table the tuple came from */
Pointer   t_data;/* -> tuple header and data
  * This field should never be 
accessed directly,
  * only via StorageAm handler 
functions,
  * because we don't know 
underlying data structure.

  */
???   t_handler; /* StorageAm that knows what to do 
with the tuple */

} StorageTupleData
;

This approach allows to minimize code changes and ensure that we
won't miss any function that handles tuples.

Do you see any weak points of the suggestion?
What design do you use in your prototype?


RelationData gains ->rd_stamroutine which is a pointer to the
StorageAmRoutine for the relation in question.  Similarly,
TupleTableSlot is augmented with a link to the StorageAmRoutine to
handle the StorageTuple it contains (probably in most cases it's set at
the same time as the tupdesc).  This implies that routines such as
ExecAssignScanType need to pass down the StorageAmRoutine from the
relation to the slot.


If we already have this pointer in t_handler as described below,
we don't need to pass it between functions and slots.

The executor is modified so that instead of calling heap_insert etc
directly, it uses rel->rd_stamroutine to call these methods.  The
executor is still in charge of dealing with indexes, constraints, and
any other thing that's not the tuple storage itself (this is one major
point in which this differs from FDWs).  This all looks simple enough,
with one exception and a few notes:


That is exactly what I tried to describe in my proposal.
Chapter "Relation management". I'm sure, you've already noticed
that it will require huge source code cleaning. I've carefully read
the sources and found "violators" of abstraction in src/backend/commands.
The list is attached to the wiki page 
https://wiki.postgresql.org/wiki/HeapamRefactoring.


Except these, there are some pretty strange and unrelated functions in 
src/backend/catalog.

I'm willing to fix them, but I'd like to synchronize our efforts.


exception a) ExecMaterializeSlot needs special consideration.  This is
used in two different ways: a1) is the stated "make tuple independent
from any underlying storage" point, which is handled 

Re: [HACKERS] parallel.c is not marked as test covered

[Amit Kapila]

On Wed, Aug 17, 2016 at 1:34 AM, Peter Eisentraut
 wrote:
> On 6/20/16 11:16 PM, Tom Lane wrote:
>>> > I think this test would only fail if it runs out of workers, and that
>>> > would only happen in an installcheck run against a server configured in
>>> > a nonstandard way or that is doing something else -- which doesn't
>>> > happen on the buildfarm.
>> Um, if you're speaking of select_parallel, that already runs in parallel
>> with two other regression tests, and there is no annotation in the
>> parallel_schedule file suggesting that adding more scripts to that group
>> would be bad.  But yes, perhaps putting this test into its own standalone
>> group would be enough of a fix.
>
> Maybe now would be a good time to address this by applying the attached
> patch to master and seeing what happens?
>

+1.  Your patch looks good to me.


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


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

[HACKERS] drop src/backend/port/darwin/system.c ?

[Peter Eisentraut]

/* only needed in OS X 10.1 and possibly early 10.2 releases */

Maybe it's time to let it go?

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


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

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

[Robert Haas]

On Wed, Aug 17, 2016 at 7:25 AM, Amit Kapila  wrote:
> On Wed, Aug 10, 2016 at 11:27 AM, Dilip Kumar  wrote:
>> On Wed, Aug 10, 2016 at 10:04 AM, Dilip Kumar  wrote:
>>> This seems better, after checking at other places I found that for
>>> invalid type we are using ERRCODE_UNDEFINED_OBJECT and for invalid
>>> functions we are using ERRCODE_UNDEFINED_FUNCTION. So I have done the
>>> same way.
>>>
>>> Updated patch attached.
>>
>> I found some more places where we can get similar error and updated in
>> my v3 patch.
>>
>
> @@ -412,7 +412,9 @@ plpgsql_validator(PG_FUNCTION_ARGS)
>   /* Get the new function's pg_proc entry */
>   tuple = SearchSysCache1(PROCOID, ObjectIdGetDatum(funcoid));
>   if (!HeapTupleIsValid(tuple))
> - elog(ERROR, "cache lookup failed for function %u", funcoid);
> + ereport(ERROR,
> + (errcode(ERRCODE_UNDEFINED_FUNCTION),
> + errmsg("function with OID %u does not exist", funcoid)));
>
> If you are making changes in plpgsql_validator(), then shouldn't we
> make changes in plperl_validator() or plpython_validator()?  I see
> that you have made changes to function CheckFunctionValidatorAccess()
> which seems to be getting called from *_validator() (* indicates
> plpgsql/plperl/plpython) functions.  Is there a reason for changing
> the *_validator() function?

Yeah, when I glanced briefly at this patch, I found myself wondering
whether all of these call sites were actually reachable (and why the
patch contained no test cases to prove 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] LWLocks in DSM memory

[Robert Haas]

On Tue, Aug 16, 2016 at 5:03 PM, Andres Freund  wrote:
> On 2016-08-15 18:15:23 -0400, Robert Haas wrote:
>> On Thu, Aug 11, 2016 at 2:19 PM, Robert Haas  wrote:
>> > Therefore, I plan to commit this patch, removing the #include
>> >  unless someone convinces me we need it, shortly after
>> > development for v10 opens, unless there are objections before then.
>>
>> Hearing no objections, done.
>
> I'd have objected, if I hadn't been on vacation.  While I intuitively
> *do* think that the increased wait-list overhead won't be relevant, I
> also know that my intuition has frequently been wrong around the lwlock
> code.  This needs some benchmarks on a 4+ socket machine,
> first. Something exercising the slow path obviously. E.g. a pgbench with
> a small number of writers, and a large number of writers.

Amit just pointed out to me that you wrote "a small number of writers,
and a large number of writers".  I assume one of those is supposed to
say "readers"?  Probably the second one?

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


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

Re: [HACKERS] Bug in to_timestamp().

[Artur Zakirov]

I attached new patch "0001-to-timestamp-format-checking-v2.patch". It 
fixes behaviour for Amul's scenarious:



Following are few scenarios where we break existing behaviour:

SELECT TO_TIMESTAMP('2015-12-31 13:43:36', ' MM DD HH24 MI SS');
SELECT TO_TIMESTAMP('2011$03!18 23_38_15', '-MM-DD HH24:MI:SS');
SELECT TO_TIMESTAMP('2011*03*18 23^38&15', '-MM-DD HH24:MI:SS');
SELECT TO_TIMESTAMP('2011*03!18 #%23^38$15', '-MM-DD$$$HH24:MI:SS');

But current patch behaviour is not that much bad either at least we have 
errors, but I am not sure about community acceptance.

I would like to divert communities' attention on following case:
SELECT TO_TIMESTAMP('2013--10-01', '-MM-DD');


For queries above the patch gives an output without any error.


Another is, shouldn’t we have error in following cases?
SELECT TO_TIMESTAMP('2016-06-13 99:99:99', '-MM-DD HH24:MI:SS');
SELECT TO_TIMESTAMP('2016-02-30 15:43:36', '-MM-DD HH24:MI:SS');


I attached second patch "0002-to-timestamp-validation-v2.patch". With it 
PostgreSQL perform additional checks for date and time. But as I wrote 
there is another patch in the thread "to_date_valid()" wich differs from 
this patch.


Sincerely,
--
Artur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 7830334..559c55f 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -6083,9 +6083,12 @@ SELECT regexp_matches('abc01234xyz', '(?:(.*?)(\d+)(.*)){1,1}');
  
   
to_timestamp and to_date
-   skip multiple blank spaces in the input string unless the
+   skip multiple blank spaces and printable non letter and non digit
+   characters in the input string and in the formatting string unless the
FX option is used. For example,
-   to_timestamp('2000JUN', ' MON') works, but
+   to_timestamp('2000JUN', ' MON'),
+   to_timestamp('2000JUN', ' MON')
+   and to_timestamp('2000JUN', 'MON') work, but
to_timestamp('2000JUN', 'FX MON') returns an error
because to_timestamp expects one space only.
FX must be specified as the first item in
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index bbd97dc..b14678d 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -169,6 +169,8 @@ struct FormatNode
 #define NODE_TYPE_END		1
 #define NODE_TYPE_ACTION	2
 #define NODE_TYPE_CHAR		3
+#define NODE_TYPE_SEPARATOR	4
+#define NODE_TYPE_SPACE		5
 
 #define SUFFTYPE_PREFIX		1
 #define SUFFTYPE_POSTFIX	2
@@ -947,6 +949,7 @@ typedef struct NUMProc
 static const KeyWord *index_seq_search(char *str, const KeyWord *kw,
  const int *index);
 static const KeySuffix *suff_search(char *str, const KeySuffix *suf, int type);
+static bool is_char_separator(char *str);
 static void NUMDesc_prepare(NUMDesc *num, FormatNode *n);
 static void parse_format(FormatNode *node, char *str, const KeyWord *kw,
 			 const KeySuffix *suf, const int *index, int ver, NUMDesc *Num);
@@ -963,7 +966,6 @@ static void dump_node(FormatNode *node, int max);
 static const char *get_th(char *num, int type);
 static char *str_numth(char *dest, char *num, int type);
 static int	adjust_partial_year_to_2020(int year);
-static int	strspace_len(char *str);
 static void from_char_set_mode(TmFromChar *tmfc, const FromCharDateMode mode);
 static void from_char_set_int(int *dest, const int value, const FormatNode *node);
 static int	from_char_parse_int_len(int *dest, char **src, const int len, FormatNode *node);
@@ -1036,6 +1038,17 @@ suff_search(char *str, const KeySuffix *suf, int type)
 	return NULL;
 }
 
+static bool
+is_char_separator(char *str)
+{
+	return ((pg_mblen(str) == 1) &&
+			/* printable character, but not letter and digit */
+			((*str >= '!' && *str <= '/') ||
+			 (*str >= ':' && *str <= '@') ||
+			 (*str >= '[' && *str <= '`') ||
+			 (*str >= '{' && *str <= '~')));
+}
+
 /* --
  * Prepare NUMDesc (number description struct) via FormatNode struct
  * --
@@ -1237,9 +1250,10 @@ parse_format(FormatNode *node, char *str, const KeyWord *kw,
 {
 	const KeySuffix *s;
 	FormatNode *n;
+	bool		in_text = false,
+in_backslash = false;
 	int			node_set = 0,
-suffix,
-last = 0;
+suffix;
 
 #ifdef DEBUG_TO_FROM_CHAR
 	elog(DEBUG_elog_output, "to_char/number(): run parser");
@@ -1251,6 +1265,49 @@ parse_format(FormatNode *node, char *str, const KeyWord *kw,
 	{
 		suffix = 0;
 
+		/* Previous character was a backslash */
+		if (in_backslash)
+		{
+			/* After backslash should go non-space character */
+			if (isspace(*str))
+ereport(ERROR,
+		(errcode(ERRCODE_SYNTAX_ERROR),
+		 errmsg("invalid escape sequence")));
+			in_backslash = false;
+
+			n->type = NODE_TYPE_CHAR;
+			n->character = *str;
+			n->key = NULL;
+			n->suffix = 0;
+			n++;
+			str++;
+			continue;
+		}
+		/* Previous character was 

Re: [HACKERS] [GENERAL] C++ port of Postgres

[Aleksander Alekseev]

> I'm sure this wasn't your intent, but the tone of your response is
> part of why people don't get involved with Postgres development...
> 
> Please note that you're the only person in the entire thread that's
> said anything to the effect of a holy war...
>
> OTOH, if the community takes the stance of "WTF WHY DO WE NEED
> THIS?!",  we've just driven Joy and anyone else that's a C++ fan away.

I'm sorry for being maybe to emotional. It's was not (and never is!) my
intent to offend anyone. Also I would like to note that I don't speak
for community, I speak for myself.

What I saw was: "hey, lets rewrite PostgreSQL in C++ without any good
reason except (see [1] list)". Naturally I though (and still think) that
you people are just trolls. Or maybe "everything should be written in
C++ because it's the only right language and anyone who thinks
otherwise is wrong" type of fanatics. Thus I don't think you are here to
help.

Give a concrete reason. Like "hey, we rewrote this part of code in C++
and look, its much more readable, twice as fast as code in C (how to do
benchmarks right is a separate good topic!) and it still compiles fast
even on Raspberry Pi, works on all platforms you are supporting, etc".
Or "hey, we solved xid wraparound problem once and for all, but solution
is in C++, so its for you to decide whether to merge it or not".

If you really want to help, just solve _real_ problems using instruments
you prefer instead of reposting obviously holly war topic from general@
mailing list. 

[1] https://en.wikipedia.org/wiki/List_of_fallacies

-- 
Best regards,
Aleksander Alekseev


-- 
Sent 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 joinrels.c

[Amit Kapila]

On Wed, Aug 10, 2016 at 11:27 AM, Dilip Kumar  wrote:
> On Wed, Aug 10, 2016 at 10:04 AM, Dilip Kumar  wrote:
>> This seems better, after checking at other places I found that for
>> invalid type we are using ERRCODE_UNDEFINED_OBJECT and for invalid
>> functions we are using ERRCODE_UNDEFINED_FUNCTION. So I have done the
>> same way.
>>
>> Updated patch attached.
>
> I found some more places where we can get similar error and updated in
> my v3 patch.
>

@@ -412,7 +412,9 @@ plpgsql_validator(PG_FUNCTION_ARGS)
  /* Get the new function's pg_proc entry */
  tuple = SearchSysCache1(PROCOID, ObjectIdGetDatum(funcoid));
  if (!HeapTupleIsValid(tuple))
- elog(ERROR, "cache lookup failed for function %u", funcoid);
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_FUNCTION),
+ errmsg("function with OID %u does not exist", funcoid)));

If you are making changes in plpgsql_validator(), then shouldn't we
make changes in plperl_validator() or plpython_validator()?  I see
that you have made changes to function CheckFunctionValidatorAccess()
which seems to be getting called from *_validator() (* indicates
plpgsql/plperl/plpython) functions.  Is there a reason for changing
the *_validator() function?

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


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

Re: [HACKERS] Why we lost Uber as a user

[Greg Stark]

On Wed, Aug 17, 2016 at 1:36 AM, Jim Nasby  wrote:
> Something I didn't see mentioned that I think is a critical point: last I
> looked, HOT standby (and presumably SR) replays full page writes. That means
> that *any* kind of corruption on the master is *guaranteed* to replicate to
> the slave the next time that block is touched. That's completely the
> opposite of trigger-based replication.

Yes, this is exactly what it should be doing and exactly why it's
useful. Physical replication accurately replicates the data from the
master including "corruption" whereas a logical replication system
will not, causing divergence and possible issues during a failover.

Picture yourself as Delta, you have a fire in your data centre and go
to fail over to your secondary site. Your DBAs inform you that the
secondary site has "fixed" some corruption that you were unaware of
and wasn't causing any issues and now, in the middle of the business
crisis, is when you're going to need to spend time identifying and
repairing the problem because your business logic has suddenly started
running into problems.

Physical replication tries to solve the same use cases as physical
backups. They both provide you with exactly what you had prior to the
recovery. No more or less. That's what you want when recovering from a
disaster.

-- 
greg


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

Re: [HACKERS] Use pread and pwrite instead of lseek + write and read

[Victor Wagner]

On Wed, 17 Aug 2016 12:17:35 +0200
Magnus Hagander  wrote:

> On Wed, Aug 17, 2016 at 11:34 AM, Victor Wagner 
> wrote:

> > I don't think that all platforms, supported by PostgreSQL support
> > this API. Especially, I cannot find any mention of pread/pwrite in
> > the Win32 except this thread on stackoverflow:
> >
> >  
> Yeah, Windows does not have those API calls, but it shouldn't be
> rocket science to write a wrapper for it. The standard windows APIs
> can do the same thing -- but they'll need access to the HANDLE for
> the file and not the posix file descriptor.

There is _get_osfhandle function, which allows to find out Windows
HANDLE associated with posix file descriptor.

Really my question was - someone should write these wrappers into
src/port and add corresponding test to the configure and/or CMakefile 
for this patch to be complete.





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

Re: [HACKERS] Use pread and pwrite instead of lseek + write and read

[Magnus Hagander]

On Wed, Aug 17, 2016 at 12:41 PM, Dagfinn Ilmari Mannsåker <
ilm...@ilmari.org> wrote:

> Magnus Hagander  writes:
>
> [pread/pwrite]
>
> > Yeah, Windows does not have those API calls, but it shouldn't be rocket
> > science to write a wrapper for it. The standard windows APIs can do the
> > same thing -- but they'll need access to the HANDLE for the file and not
> > the posix file descriptor.
>

Just to be clear, I'm referring to the standard ReadFile() and WriteFile()
APIs here.



> >
> > It also has things like ReadFileScatter() (
> > https://msdn.microsoft.com/en-us/library/windows/desktop/
> aa365469(v=vs.85).aspx)
> > which is not the same, but might also be interesting as a future
> > improvement.
>
> That looks a lot like POSIX readv()
> (http://pubs.opengroup.org/onlinepubs/9699919799/functions/readv.html),
> and as far as I can tell it has the same issue as it in that it doesn't
> take an offset argument, but requires you to seek first.
>

Ah yeah, for some reason I keep getting readv() and pread(). Which solve a
different problem (see below about that function not having the same issues
on windows -- but it's still not the problem we're trying to solve here)



> Linux and modern BSDs however have preadv()
> (http://manpages.ubuntu.com/manpages/xenial/en/man2/preadv.2.html),
> which takes an offset and an iovec array.  I don't know if Windows and
> other platforms have anything similar.
>

ReadFileScatter() can take the offset from OVERLAPPED (same as regular
ReadFile) and an array of FILE_SEGMENT_ELEMENT, same as preadv(). But the
APIs start looking more different the further down the rabbithole you go, I
think. But the capability is definitely there (and has been for ages so is
in all supported version).

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

Re: [HACKERS] Use pread and pwrite instead of lseek + write and read

[Dagfinn Ilmari Mannsåker]

Magnus Hagander  writes:

[pread/pwrite]

> Yeah, Windows does not have those API calls, but it shouldn't be rocket
> science to write a wrapper for it. The standard windows APIs can do the
> same thing -- but they'll need access to the HANDLE for the file and not
> the posix file descriptor.
>
> It also has things like ReadFileScatter() (
> https://msdn.microsoft.com/en-us/library/windows/desktop/aa365469(v=vs.85).aspx)
> which is not the same, but might also be interesting as a future
> improvement.

That looks a lot like POSIX readv()
(http://pubs.opengroup.org/onlinepubs/9699919799/functions/readv.html),
and as far as I can tell it has the same issue as it in that it doesn't
take an offset argument, but requires you to seek first.

Linux and modern BSDs however have preadv()
(http://manpages.ubuntu.com/manpages/xenial/en/man2/preadv.2.html),
which takes an offset and an iovec array.  I don't know if Windows and
other platforms have anything similar.

-- 
- Twitter seems more influential [than blogs] in the 'gets reported in
  the mainstream press' sense at least.   - Matt McLeod
- That'd be because the content of a tweet is easier to condense down
  to a mainstream media article.  - Calle Dybedahl


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

Re: [HACKERS] Use pread and pwrite instead of lseek + write and read

[Magnus Hagander]

On Wed, Aug 17, 2016 at 11:34 AM, Victor Wagner  wrote:

> On Wed, 17 Aug 2016 10:58:09 +0300
> Oskari Saarenmaa  wrote:
>
>
>
> >
> > The attached patch replaces FileWrite and FileRead with FileWriteAt
> > and FileReadAt and removes most FileSeek calls.  FileSeek is still
> > around so we can find the end of a file, but it's not used for
> > anything else.
>
> It seems that configure test for availability of pread/pwrite functions
> and corresponding #define is needed.
>
> I don't think that all platforms, supported by PostgreSQL support this
> API. Especially, I cannot find any mention of pread/pwrite in the Win32
> except this thread on stackoverflow:
>
>
Yeah, Windows does not have those API calls, but it shouldn't be rocket
science to write a wrapper for it. The standard windows APIs can do the
same thing -- but they'll need access to the HANDLE for the file and not
the posix file descriptor.

It also has things like ReadFileScatter() (
https://msdn.microsoft.com/en-us/library/windows/desktop/aa365469(v=vs.85).aspx)
which is not the same, but might also be interesting as a future
improvement. That's clearly something different though, and out of scope
for this one. But IIRC that functionality was actually added for the sake
of SQLServer back in the days.

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

Re: [HACKERS] [GENERAL] C++ port of Postgres

[Aleksander Alekseev]

> Two big projects lately move to C++ from C:
> GCC, Mesa
> 
> You can read their reasons.
> Only C++ we can use without full rewrite currently. (or ObjectC maybe)
> If we wish fix C limitations. 
> 

I would like just to leave this link here:

https://en.wikipedia.org/wiki/List_of_fallacies

Long story short - no one cares who did what in other projects.
Recently I rewrote my OpenGL demo [1] from C++ to C. Uber recently
moved from PostgreSQL to MySQL. It proves literally nothing.

[1] https://github.com/afiskon/c-opengl-text/

-- 
Best regards,
Aleksander Alekseev


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

Re: [HACKERS] [Patch] Temporary tables that do not bloat pg_catalog (a.k.a fast temp tables)

[Aleksander Alekseev]

> That doesn't really solve the problem, because OTHER backends won't be
> able to see them.  So, if I create a fast temporary table in one
> session that depends on a permanent object, some other session can
> drop the permanent object.  If there were REAL catalog entries, that
> wouldn't work, because the other session would see the dependency.
> 

This is a good point. However current implementation doesn't allow to
do that. There is a related bug though, a minor one.

In session 1:

```
CREATE TABLE cities2 (name text, population float, altitude int);
CREATE FAST TEMPORARY TABLE capitals2 (state char(2)) INHERITS (cities2);
```

In session 2:

```
DROP TABLE cities2;

ERROR:  cache lookup failed for relation 16401
```

Instead of "cache lookup failed" probably a better error message
should be displayed. Something like "cannot drop table cities2 because
other objects depend on it". I will send a corrected patch shortly.

Everything else seems to work as expected.

If you discover any other bugs please let me know!

-- 
Best regards,
Aleksander Alekseev


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

Re: [HACKERS] Use pread and pwrite instead of lseek + write and read

[Victor Wagner]

On Wed, 17 Aug 2016 10:58:09 +0300
Oskari Saarenmaa  wrote:



> 
> The attached patch replaces FileWrite and FileRead with FileWriteAt
> and FileReadAt and removes most FileSeek calls.  FileSeek is still
> around so we can find the end of a file, but it's not used for
> anything else.

It seems that configure test for availability of pread/pwrite functions
and corresponding #define is needed.
 
I don't think that all platforms, supported by PostgreSQL support this
API. Especially, I cannot find any mention of pread/pwrite in the Win32 
except this thread on stackoverflow:


http://stackoverflow.com/questions/766477/are-there-equivalents-to-pread-on-different-platforms



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

Re: [HACKERS] [RFC] Change the default of update_process_title to off

[Magnus Hagander]

On Tue, Aug 16, 2016 at 3:37 PM, Tom Lane  wrote:

> Robert Haas  writes:
> > On Tue, Aug 16, 2016 at 5:53 AM, Magnus Hagander 
> wrote:
> >> What's our take on backpatching such changes? Should this be 9.6 only,
> or
> >> back further?
>
> > I would have thought this was a master-only change, although
> > back-patching it to 9.6 would be OK if it gets done RSN.  I don't
> > think changing GUC defaults in released branches is a good idea.
>
> I agree with fixing 9.6, but not further back.
>

Good, that pretty much aligns with what I was thinking.

Applied and backpatched to 9.6.

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

[HACKERS] Use pread and pwrite instead of lseek + write and read

[Oskari Saarenmaa]

Hi,

The Uber blog post, among other things, pointed out that PG uses lseek + 
read instead of pread.  I didn't see any discussion around that and my 
Google searches didn't find any posts about pread / pwrite for the past 
10 years.


With that plus the "C++ port" thread in mind, I was wondering if it's 
time to see if we could do better by just utilizing newer C and POSIX 
features.


The attached patch replaces FileWrite and FileRead with FileWriteAt and 
FileReadAt and removes most FileSeek calls.  FileSeek is still around so 
we can find the end of a file, but it's not used for anything else.


On my laptop a simple pgbench run (scale 100, 15 minutes) shows a 1.5% 
performance improvement.  A 1.5% performance improvement is small but 
measurable - and IMV more importantly it allows us to drop more than 100 
lines of backwards (compatible?) code; maybe we could start targeting 
more recent platforms in v10?


Obviously this patch needs some more work before it could be merged, and 
we probably still need a fallback for some platforms without pread and 
pwrite (afaik Windows doesn't implement them.)


/ Oskari
diff --git src/backend/access/heap/rewriteheap.c src/backend/access/heap/rewriteheap.c
index f9ce986..6dc5df3 100644
--- src/backend/access/heap/rewriteheap.c
+++ src/backend/access/heap/rewriteheap.c
@@ -918,7 +918,7 @@ logical_heap_rewrite_flush_mappings(RewriteState state)
 		 * Note that we deviate from the usual WAL coding practices here,
 		 * check the above "Logical rewrite support" comment for reasoning.
 		 */
-		written = FileWrite(src->vfd, waldata_start, len);
+		written = FileWriteAt(src->vfd, waldata_start, len, src->off);
 		if (written != len)
 			ereport(ERROR,
 	(errcode_for_file_access(),
diff --git src/backend/storage/file/buffile.c src/backend/storage/file/buffile.c
index 042be79..39482fa 100644
--- src/backend/storage/file/buffile.c
+++ src/backend/storage/file/buffile.c
@@ -60,12 +60,6 @@ struct BufFile
 	int			numFiles;		/* number of physical files in set */
 	/* all files except the last have length exactly MAX_PHYSICAL_FILESIZE */
 	File	   *files;			/* palloc'd array with numFiles entries */
-	off_t	   *offsets;		/* palloc'd array with numFiles entries */
-
-	/*
-	 * offsets[i] is the current seek position of files[i].  We use this to
-	 * avoid making redundant FileSeek calls.
-	 */
 
 	bool		isTemp;			/* can only add files if this is TRUE */
 	bool		isInterXact;	/* keep open over transactions? */
@@ -108,8 +102,6 @@ makeBufFile(File firstfile)
 	file->numFiles = 1;
 	file->files = (File *) palloc(sizeof(File));
 	file->files[0] = firstfile;
-	file->offsets = (off_t *) palloc(sizeof(off_t));
-	file->offsets[0] = 0L;
 	file->isTemp = false;
 	file->isInterXact = false;
 	file->dirty = false;
@@ -143,10 +135,7 @@ extendBufFile(BufFile *file)
 
 	file->files = (File *) repalloc(file->files,
 	(file->numFiles + 1) * sizeof(File));
-	file->offsets = (off_t *) repalloc(file->offsets,
-	   (file->numFiles + 1) * sizeof(off_t));
 	file->files[file->numFiles] = pfile;
-	file->offsets[file->numFiles] = 0L;
 	file->numFiles++;
 }
 
@@ -210,7 +199,6 @@ BufFileClose(BufFile *file)
 		FileClose(file->files[i]);
 	/* release the buffer space */
 	pfree(file->files);
-	pfree(file->offsets);
 	pfree(file);
 }
 
@@ -241,23 +229,12 @@ BufFileLoadBuffer(BufFile *file)
 	}
 
 	/*
-	 * May need to reposition physical file.
-	 */
-	thisfile = file->files[file->curFile];
-	if (file->curOffset != file->offsets[file->curFile])
-	{
-		if (FileSeek(thisfile, file->curOffset, SEEK_SET) != file->curOffset)
-			return;/* seek failed, read nothing */
-		file->offsets[file->curFile] = file->curOffset;
-	}
-
-	/*
 	 * Read whatever we can get, up to a full bufferload.
 	 */
-	file->nbytes = FileRead(thisfile, file->buffer, sizeof(file->buffer));
+	thisfile = file->files[file->curFile];
+	file->nbytes = FileReadAt(thisfile, file->buffer, sizeof(file->buffer), file->curOffset);
 	if (file->nbytes < 0)
 		file->nbytes = 0;
-	file->offsets[file->curFile] += file->nbytes;
 	/* we choose not to advance curOffset here */
 
 	pgBufferUsage.temp_blks_read++;
@@ -307,20 +284,10 @@ BufFileDumpBuffer(BufFile *file)
 bytestowrite = (int) availbytes;
 		}
 
-		/*
-		 * May need to reposition physical file.
-		 */
 		thisfile = file->files[file->curFile];
-		if (file->curOffset != file->offsets[file->curFile])
-		{
-			if (FileSeek(thisfile, file->curOffset, SEEK_SET) != file->curOffset)
-return;			/* seek failed, give up */
-			file->offsets[file->curFile] = file->curOffset;
-		}
-		bytestowrite = FileWrite(thisfile, file->buffer + wpos, bytestowrite);
+		bytestowrite = FileWriteAt(thisfile, file->buffer + wpos, bytestowrite, file->curOffset);
 		if (bytestowrite <= 0)
 			return;/* failed to write */
-		file->offsets[file->curFile] += bytestowrite;
 		file->curOffset += bytestowrite;
 		wpos += bytestowrite;
 
diff --git src/backend/storage/file/fd.c 

Re: [HACKERS] WIP: Covering + unique indexes.

[Andrew Borodin]

> That was a bug caused by high key truncation, that occurs when index has more 
> than 3 levels. Fixed.
Affirmative. I've tested index construction with 100M rows and
subsequent execution of select queries using index, works fine.

Best regards, Andrey Borodin, Octonica & Ural Federal University.


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