date:20150805

On Fri, Jun 26, 2015 at 11:40:27AM -0400, Robert Haas wrote:
> On Wed, Jun 24, 2015 at 4:53 AM, Peter Geoghegan  wrote:
> > On Wed, Jun 24, 2015 at 1:45 AM, Craig Ringer  wrote:
> >> WARNING: hash indexes are not crash-safe, not replicated, and their
> >> use is discouraged
> >
> > +1
> 
> I'm not wild about this rewording; I think that if users don't know
> what WAL is, they probably need to know that in order to make good
> decisions about whether to use hash indexes.  But I don't feel
> super-strongly about it.

Coming late to this, but I think Robert is right.  WAL is used for crash
recovery, PITR, and streaming replication, and I am not sure we want to
specify all of those in the warning message.

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

  + Everyone has their own god. +


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

Re: [HACKERS] [DESIGN] ParallelAppend

2015-08-05 Thread Amit Kapila

On Sat, Aug 1, 2015 at 6:39 PM, Kouhei Kaigai  wrote:
>
> > On Tue, Jul 28, 2015 at 6:08 PM, Kouhei Kaigai 
wrote:
> >
> > I am not sure, but what problem do you see in putting Funnel node
> > for one of the relation scans and not for the others.
> >
> At this moment, I'm not certain whether background worker can/ought
> to launch another background workers.
> If sub-Funnel node is executed by 10-processes then it also launch
> 10-processes for each, 100-processes will run for each?
>

Yes, that could be more work than current, but what I had in mind
is not that way, rather I was thinking that master backend will only
kick of workers for Funnel nodes in plan.

> > > If we pull Funnel here, I think the plan shall be as follows:
> > >   Funnel
> > >--> SeqScan on rel1
> > >--> PartialSeqScan on rel2
> > >--> IndexScan on rel3
> > >
> >
> > So if we go this route, then Funnel should have capability
> > to execute non-parallel part of plan as well, like in this
> > case it should be able to execute non-parallel IndexScan on
> > rel3 as well and then it might need to distinguish between
> > parallel and non-parallel part of plans.  I think this could
> > make Funnel node complex.
> >
> It is difference from what I plan now. In the above example,
> Funnel node has two non-parallel aware node (rel1 and rel3)
> and one parallel aware node, then three PlannedStmt for each
> shall be put on the TOC segment. Even though the background
> workers pick up a PlannedStmt from the three, only one worker
> can pick up the PlannedStmt for rel1 and rel3, however, rel2
> can be executed by multiple workers simultaneously.

Okay, now I got your point, but I think the cost of execution
of non-parallel node by additional worker is not small considering
the communication cost and setting up an addional worker for
each sub-plan (assume the case where out of 100-child nodes
only few (2 or 3) nodes actually need multiple workers).

> >
> > I think for a particular PlannedStmt, number of workers must have
> > been decided before start of execution, so if those many workers are
> > available to work on that particular PlannedStmt, then next/new
> > worker should work on next PlannedStmt.
> >
> My concern about pre-determined number of workers is, it depends on the
> run-time circumstances of concurrent sessions. Even if planner wants to
> assign 10-workers on a particular sub-plan, only 4-workers may be
> available on the run-time because of consumption by side sessions.
> So, I expect only maximum number of workers is meaningful configuration.
>

In that case, there is possibility that many of the workers are just
working on one or two of the nodes and other nodes execution might
get starved.  I understand this is tricky problem to allocate number
of workers for different nodes, however we should try to develop any
algorithm where there is some degree of fairness in allocation of workers
for different nodes.

> > 2. Execution of work by workers and Funnel node and then pass
> > the results back to upper node.  I think this needs some more
> > work in addition to ParallelSeqScan patch.
> >
> I expect we can utilize existing infrastructure here. It just picks
> up the records come from the underlying workers, then raise it to
> the upper node.
>

Sure, but still you need some work atleast in the area of making
workers understand different node types, I am guessing you need
to modify readfuncs.c to support new plan node if any for this
work.

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

Re: [HACKERS] Caching offsets of varlena attributes

2015-08-05 Thread Haribabu Kommi

On Thu, Aug 6, 2015 at 4:09 AM, Vignesh Raghunathan
 wrote:
> Hello,
>
> In the function heap_deform_tuple, there is a comment before caching varlena
> attributes specifying that the offset will be valid for either an aligned or
> unaligned value if there are no padding bytes. Could someone please
> elaborate on this?

In PostgreSQL tuple can contain two types of varlena headers. Those are
short varlena(doesn't need any alignment) and 4-byte varlena(needs alignment).

Refer the function "heap_fill_tuple" to see how the tuple is constructed.
For short varlena headers, even if the alignment suggests to do the alignment,
we shouldn't not do. Because of this reason instead of "att_align_nominal", the
"att_align_pointer" is called.

The following is the comment from "att_align_pointer" macro which gives the
details why we should use this macro instead of "att_align_nominal".

 * (A zero byte must be either a pad byte, or the first byte of a correctly
 * aligned 4-byte length word; in either case we can align safely.  A non-zero
 * byte must be either a 1-byte length word, or the first byte of a correctly
 * aligned 4-byte length word; in either case we need not align.)

> Also, why is it safe to call att_align_nominal if the attribute is not
> varlena? Couldn't att_align_pointer be called for both cases? I am not able
> to understand how att_align_nominal is faster.

All other types definitely needs either char or int or double alignment. Because
of this reason it is safe to use the att_align_nominal macro. Please refer the
function "heap_fill_tuple" for more details.

Regards,
Hari Babu
Fujitsu Australia

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

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

On Wed, Aug  5, 2015 at 11:57:48PM -0300, Alvaro Herrera wrote:
> Bruce Momjian wrote:
> 
> > This is why I suggested putting the new SQL function where it belongs
> > for consistency and then open a separate thread to discuss the future of
> > where we want diagnostic functions to be.  It is too complicated to talk
> > about both issues in the same thread.
> 
> Oh come on -- gimme a break.  We figure out much more complicated
> problems in single threads all the time.

Well, people are confused, as stated --- what more can I say?

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

  + Everyone has their own god. +


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

Re: [HACKERS] Raising our compiler requirements for 9.6

Andres Freund  writes:
> ... I'm going to reshuffle things in that direction tomorrow. I'll
> wait for other fallout first though. So far only gcc, xlc and clang (via
> gcc frontend) have run...

In the department of "other fallout", pademelon is not happy:

cc -Ae -g +O0 -Wp,-H16384 -I../../../src/include -D_XOPEN_SOURCE_EXTENDED  
-I/usr/local/libxml2-2.6.23/include/libxml2 -I/usr/local/include  -c -o 
pg_resetxlog.o pg_resetxlog.c
cc -Ae -g +O0 -Wp,-H16384 pg_resetxlog.o -L../../../src/port 
-L../../../src/common -L/usr/local/libxml2-2.6.23/lib -L/usr/local/lib -Wl,+b 
-Wl,'/home/bfarm/bf-data/HEAD/inst/lib'  -Wl,-z -lpgcommon -lpgport -lxnet 
-lxml2 -lz -lreadline -ltermcap -lm  -o pg_resetxlog
/usr/ccs/bin/ld: Unsatisfied symbols:
   pg_atomic_clear_flag_impl (code)
   pg_atomic_init_flag_impl (code)
   pg_atomic_compare_exchange_u32_impl (code)
   pg_atomic_fetch_add_u32_impl (code)
   pg_atomic_test_set_flag_impl (code)
   pg_atomic_init_u32_impl (code)
make[3]: *** [pg_resetxlog] Error 1

I'd have thought that port/atomics.c would be included in libpgport, but
it seems not.  (But is pademelon really the only buildfarm critter relying
on the "fallback" atomics implementation?)

Another possible angle is: what the heck does pg_resetxlog need with
atomic ops, anyway?  Should these things have a different, stub
implementation for FRONTEND code?

regards, tom lane


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

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

Bruce Momjian wrote:

> This is why I suggested putting the new SQL function where it belongs
> for consistency and then open a separate thread to discuss the future of
> where we want diagnostic functions to be.  It is too complicated to talk
> about both issues in the same thread.

Oh come on -- gimme a break.  We figure out much more complicated
problems in single threads all the time.

-- 
Á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] deparsing utility commands

Jim Nasby wrote:
> On 7/31/15 8:45 AM, Shulgin, Oleksandr wrote:

> >STATEMENT:  create view v1 as select * from t1 ;
> >ERROR:  operator does not exist: pg_catalog.oid = pg_catalog.oid at
> >character 52
> >HINT:  No operator matches the given name and argument type(s). You
> >might need to add explicit type casts.
> >QUERY:  SELECT * FROM pg_catalog.pg_rewrite WHERE ev_class = $1 AND
> >rulename = $2
> >CONTEXT:  PL/pgSQL function test_ddl_deparse() line 1 at FOR over SELECT
> >rows

> I'm not sure what test_ddl_deparse is doing, is that where the oid = oid is
> coming from?
> 
> It might be enlightening to replace = with OPERATOR(pg_catalog.=) and see if
> that works.

That worked, thanks!

-- 
Á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] Raising our compiler requirements for 9.6

2015-08-05 Thread Noah Misch

On Wed, Aug 05, 2015 at 10:32:48AM -0400, Tom Lane wrote:
> Andres Freund  writes:
> > Wasn't the point that 32 bit AIX as a whole hasn't been supported for a
> > couple years now? My willingness to expend effort for that is rather
> > limited.
> 
> Well, there's one in the buildfarm.  We don't generally turn off
> buildfarm critters just because the underlying OS is out of support
> (the population would be quite a bit lower if we did).

For the record, mandrill's OS and compiler are both in support and not so old
(June 2012 compiler, February 2013 OS).  The last 32-bit AIX *kernel* did exit
support in 2012, but 32-bit executables remain the norm.


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

Re: [HACKERS] nodes/*funcs.c inconsistencies

2015-08-05 Thread Noah Misch

On Mon, Aug 03, 2015 at 09:57:52AM -0700, Joe Conway wrote:
> On 08/03/2015 09:55 AM, Stephen Frost wrote:
> > * Noah Misch (n...@leadboat.com) wrote:
> >> On Sun, Aug 02, 2015 at 11:31:16PM -0400, Tom Lane wrote:
> >>> That being the case, it would probably be a good idea to get
> >>> them done before alpha2, as there may not be a good opportunity
> >>> afterwards.
> >> 
> >> Freedom to bump catversion after alpha2 will be
> >> barely-distinguishable from freedom to do so now.  I have planned
> >> to leave my usual comment period of a few days, though skipping
> >> that would be rather innocuous in this case.
> > 
> > This is clearly necessary, of course, and I wouldn't be surprised
> > if we have another necessary bump post-alpha2, but at the same
> > time, it'd certainly be nice if we are able to put out alpha2 and
> > then a beta1 in the future without needing to do a bump.
> > 
> > In other words, +1 from me for going ahead and committing this for 
> > alpha2, if possible.
> 
> +1

Rather than commit on an emergency basis in the few hours between these +1's
and the wrap, I kept to my original schedule.  FYI, if it hadn't required
emergency procedures (cancelling the day's plans so I could get to a notebook
computer), I would have committed early based on the three +1s.


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

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

On Wed, Aug  5, 2015 at 10:58:00AM -0700, Josh Berkus wrote:
> On 08/05/2015 10:46 AM, Alvaro Herrera wrote:
> > 1. Add the functions as a builtins.
> >This is what the current patch does.  Simon seems to prefer this,
> >because he wants the function to be always available in production;
> >but I don't like this option because adding functions as builtins
> >makes it impossible to move later to extensions.
> >Bruce doesn't like this option either.
> 
> Why would we want to move them later to extensions?  Do you anticipate
> not needing them in the future?  If we don't need them in the future,
> why would they continue to exist at all?
> 
> I'm really not getting this.
  

This is why I suggested putting the new SQL function where it belongs
for consistency and then open a separate thread to discuss the future of
where we want diagnostic functions to be.  It is too complicated to talk
about both issues in the same thread.

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

  + Everyone has their own god. +


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

Re: [HACKERS] Doubt about AccessExclusiveLock in ALTER TABLE .. SET ( .. );

2015-08-05 Thread Michael Paquier

On Thu, Aug 6, 2015 at 3:06 AM, Fabrízio de Royes Mello wrote:
> On Wed, Aug 5, 2015 at 9:31 AM, Robert Haas wrote:
>> Agreed.  I think we're making a mountain out of a molehill here.  As
>> long as the locks that are actually used are monotonic, just use > and
>> stick a comment in there explaining that it could need adjustment if
>> we use other lock levels in the future.  I presume all the lock-levels
>> used for DDL are, and will always be, self-exclusive, so why all this
>> hand-wringing?
>>
>
> New version attached with suggested changes.

Thanks!

+# SET autovacuum_* options needs a ShareUpdateExclusiveLock
+# so we mix reads with it to see what works or waits
s/needs/need/ and I think you mean mixing "writes", not "reads".

Those are minor things though, and from my point of view a committer
can look at it.
Regards,
-- 
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] libpq: Allow specifying multiple host names to try to connect to

2015-08-05 Thread Michael Paquier

On Wed, Aug 5, 2015 at 11:53 PM, Bruce Momjian  wrote:
> On Wed, Jul  8, 2015 at 12:24:37PM -0400, Robbie Harwood wrote:
>> > You update the documentation just for  psql but your change effects any
>> > libpq application if we go forward with this patch we should update the
>> > documentation for libpq as well.
>> >
>> > This approach seems to work with the url style of conninfo
>> >
>> > For example
>> >   postgres://some-down-host.info,some-other-host.org:5435/test1
>> >
>> > seems to work as expected but I don't like that syntax I would rather see
>> > postgres://some-down-host.info:5435/test1,postgres://some-other-host.org:5435/test1
>> >
>> > This would be a more invasive change but I think the syntax is more usable.
>>
>> I agree with this; it seems to me that it's more powerful to be able to
>> specify complete urls for when they may differ.
>>
>> For the non-url case though, I don't see a clean way of doing this.  We
>> could always, e.g., locally bind port specification to the closest host
>> specification, but that seems nasty, and is still less powerful than
>> passing urls (or we could just do the same for all parameters, but
>> that's just a mess).
>>
>> Might it be reasonable to only allow the multi-host syntax in the
>> url-style and not otherwise?
>
> First, I agree this is a very useful feature that we want.  Many NoSQL
> databases are promoting multi-host client libraries as HA, which is kind
> of humorous, and also makes sense because many NoSQL solution are
> multi-host.
> I can see this feature benefitting us for clients to auto-failover
> without requiring a pooler or virtual IP reassignment, and also useful
> for read-only connections that want to connect to a read-only slave, but
> don't care which one.  The idea of randomly selecting a host from the
> list might be a future feature.

Yep. The JDBC driver is doing it as well.

> I realize this is libpq-feature-creep, but considering the complexities
> of a pooler and virtual IP address reassignment, I think adding this
> The fact that other DBs are doing it, including I think
> VMWare's libpq, supports the idea of adding this simple specification.

Not exactly (the change has been open-sourced). Some extra logic has
been added in pghost parsing handling so as it is possible to grab
from it an ldap search filter, and then override pghostaddr using the
result found.
-- 
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] raw output from copy

2015-08-05 Thread Andrew Dunstan



On 08/05/2015 04:59 PM, Heikki Linnakangas wrote:

On 07/27/2015 02:28 PM, Pavel Stehule wrote:

2015-07-27 10:41 GMT+02:00 Heikki Linnakangas :

What about input? This is a whole new feature, but it would be nice 
to be

able to pass the file contents as a query parameter. Something like:

\P /tmp/foo binary
INSERT INTO foo VALUES (?);


The example of input is strong reason, why don't do it via inserts. Only
parsing some special "?" symbol needs lot of new code.


Sorry, I meant $1 in place of the ?. No special parsing needed, psql 
can send the query to the server as is, with the parameters that are 
given by this new mechanism.



In this case, I don't see any advantage of  psql based solution. COPY is
standard interface for input/output from/to files, and it should be used
there.


I'm not too happy with the COPY approach, although I won't object is 
one of the other committers feel more comfortable with it. However, we 
don't seem to be making progress here, so I'm going to mark this as 
Returned with Feedback. I don't feel good about that either, because I 
don't actually have any great suggestions on how to move this forward. 
Which is a pity because this is a genuine problem for users.





This is really only a psql problem, IMNSHO. Inserting and extracting 
binary data is pretty trivial for most users of client libraries (e.g. 
it's a couple of lines of code in a DBD::Pg program), but it's hard in psql.


I do agree that the COPY approach feels more than a little klunky.

cheers

andrew




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

Re: [HACKERS] raw output from copy

2015-08-05 Thread Heikki Linnakangas


On 07/27/2015 02:28 PM, Pavel Stehule wrote:

2015-07-27 10:41 GMT+02:00 Heikki Linnakangas :


What about input? This is a whole new feature, but it would be nice to be
able to pass the file contents as a query parameter. Something like:

\P /tmp/foo binary
INSERT INTO foo VALUES (?);


The example of input is strong reason, why don't do it via inserts. Only
parsing some special "?" symbol needs lot of new code.


Sorry, I meant $1 in place of the ?. No special parsing needed, psql can 
send the query to the server as is, with the parameters that are given 
by this new mechanism.



In this case, I don't see any advantage of  psql based solution. COPY is
standard interface for input/output from/to files, and it should be used
there.


I'm not too happy with the COPY approach, although I won't object is one 
of the other committers feel more comfortable with it. However, we don't 
seem to be making progress here, so I'm going to mark this as Returned 
with Feedback. I don't feel good about that either, because I don't 
actually have any great suggestions on how to move this forward. Which 
is a pity because this is a genuine problem for users.


- Heikki



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

Re: [HACKERS] pgbench - allow backslash-continuations in custom scripts

2015-08-05 Thread Heikki Linnakangas


On 07/24/2015 11:36 AM, Kyotaro HORIGUCHI wrote:

At Fri, 24 Jul 2015 07:39:16 +0200 (CEST), Fabien COELHO  wrote 
in 

- backslash commands is handled as the same as before: multiline
  is not allowed.


Hmm... that is really the feature I wanted to add initially, too bad
it is the dropped one:-)


Ouch. The story has been derailed somewhere.

Since SQL statments could be multilined without particluar
marker, we cannot implement multilined backslash commands in the
same way..


I don't think we actually want backslash-continuations. The feature we 
want is "allow SQL statements span multiple lines", and using the psql 
lexer solves that. We don't need the backslash-continuations when we 
have that.


On 07/25/2015 05:53 PM, Fabien COELHO wrote:

I don't have idea how to deal with the copy of psqlscan.[lh] from
psql. Currently they are simply the dead copies of those of psql.


I think that there should be no copies, but it should use relative
symbolic links so that the files are kept synchronized.


Yeah, I think so but symlinks could harm on git and Windows.
The another way would be make copies it from psql directory. They live
next door to each other.


Indeed there are plenty of links already which are generated by makefiles
(see src/bin/pg_xlogdump/*), and probably a copy is made on windows. There
should no file duplication within the source tree.


Yeah, following the example of pg_xlogdump and others is the way to go.

Docs need updating, and there's probably some cleanup to do before this 
is ready for committing, but overall I think this is definitely the 
right direction.


I complained upthread that this makes it impossible to use 
"multi-statements" in pgbench, as they would be split into separate 
statements, but looking at psqlscan.l there is actually a syntax for 
that in psql already. You escape the semicolon as \;, e.g. "SELECT 1 \; 
SELECT 2;", and then both queries will be sent to the server as one. So 
even that's OK.


- Heikki


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

Re: [HACKERS] brin index vacuum versus transaction snapshots

Alvaro Herrera wrote:

> Here's the promised patch.

Pushed to master and 9.5.  Thanks for reporting!

-- 
Á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] BRIN trivial tweaks

Kevin Grittner wrote:
> I happened to notice two declarations of functions for BRIN that
> are not actually defined, and a comment that looked like it was
> incompletely edited.  Attached patch is a suggestion, but I leave
> it to those working on the feature to commit, since there might be
> a reason I'm missing for the declarations and my wording might not
> be ideal.

Thanks, pushed this alongside together with the fix for the other BRIN
bug you opened.

-- 
Á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] [DOCS] max_worker_processes on the standby

2015-08-05 Thread Petr Jelinek


On 2015-08-05 00:13, Alvaro Herrera wrote:

Robert Haas wrote:

On Tue, Aug 4, 2015 at 12:41 AM, Alvaro Herrera
 wrote:



The alternative is to turn the feature on automatically if it sees that
the master also has it on, i.e. the value would not be what the config
file says it is.  Doing this would be a bit surprising IMO, but given
the behavior above maybe it's better than the current behavior.


I think it's totally reasonable for the standby to follow the master's
behavior rather than the config file.  That should be documented, but
otherwise, no problem.  If it were technologically possible for the
standby to follow the config file rather than the master in all cases,
that would be fine, too.  But the current behavior is somewhere in the
middle, and that doesn't seem like a good plan.


So I discussed this with Petr.  He points out that if we make the
standby follows the master, then the problem would be the misbehavior
that results once the standby is promoted: at that point the standby
would no longer "follow the master" and would start with the feature
turned off, which could be disastrous (depending on what are you using
the commit timestamps for).

Given this, we're leaning towards the idea that the standby should not
try to follow the master at all.  Instead, an extension that wants to
use this stuff can check the value for itself, and raise a fatal error
if it's not already turned on the config file.  That way, a lot of the
strange corner cases disappear.



Actually, after thinking bit more about this I think the behavior of 
these two will be similar - you suddenly lose the commit timestamp info. 
The difference is that with fist option you'll lose it after restart 
while with second one you lose it immediately after promotion since 
there was never any info on the slave.


Extensions can do sanity checking in both scenarios.

The way I see it the first option has following advantages:
- it's smaller change
- it's more consistent with how wal_log_hints behaves
- fixing the config does not require server restart since the in-memory 
state was set from WAL record automatically


However the second option has also some:
- one can have slave which doesn't have overhead of the commit timestamp 
SLRU if they don't need it there
- it's theoretically easier to notice that the track_commit_timestamps 
is off in config because the the SQL interface will complain if called 
on the slave


So +0.5 from me towards following master and removing the error message

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


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

Re: [HACKERS] deparsing utility commands

2015-08-05 Thread Jim Nasby


On 7/31/15 8:45 AM, Shulgin, Oleksandr wrote:

While running deparsecheck suite I'm getting a number of oddly looking
errors:

WARNING:  state: 42883 errm: operator does not exist: pg_catalog.oid =
pg_catalog.oid

This is caused by deparsing create view, e.g.:

STATEMENT:  create view v1 as select * from t1 ;
ERROR:  operator does not exist: pg_catalog.oid = pg_catalog.oid at
character 52
HINT:  No operator matches the given name and argument type(s). You
might need to add explicit type casts.
QUERY:  SELECT * FROM pg_catalog.pg_rewrite WHERE ev_class = $1 AND
rulename = $2
CONTEXT:  PL/pgSQL function test_ddl_deparse() line 1 at FOR over SELECT
rows

The pg_rewrite query comes from ruleutils.c, while ddl_deparse.c calls
it through pg_get_viewdef_internal() but don't understand how is it
different from e.g., select pg_get_viewdef(...), and that last one is
not affected.


I'm not sure what test_ddl_deparse is doing, is that where the oid = oid 
is coming from?


It might be enlightening to replace = with OPERATOR(pg_catalog.=) and 
see if that works.

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


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

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

2015-08-05 Thread Andreas Seltenreich

Tom Lane writes:

>> On 08/01/2015 05:59 PM, Tom Lane wrote:
>>> Well, I certainly think all of these represent bugs:
 1 | ERROR:  could not find pathkey item to sort
>
> Hmm ... I see no error with these queries as of today's HEAD or
> back-branch tips.  I surmise that this was triggered by one of the other
> recently-fixed bugs, though the connection isn't obvious offhand.

I still see this error in master as of b8cbe43, but the queries are
indeed a pita to reproduce.  The one below is the only one so far that
is robust against running ANALYZE on the regression db, and also
reproduces when I run it as an EXTRA_TEST with make check.

regards,
Andreas

select
  rel_217088662.a as c0,
  rel_217088554.a as c1,
  rel_217088662.b as c2,
  subq_34235266.c0 as c3,
  rel_217088660.id2 as c4,
  rel_217088660.id2 as c5
from
  public.clstr_tst as rel_217088554
inner join (select
   rel_217088628.a as c0
 from
   public.rtest_vview3 as rel_217088628
 where (rel_217088628.b !~ rel_217088628.b)
   and rel_217088628.b ~~* rel_217088628.b)
 or (rel_217088628.b ~* rel_217088628.b))
   or (rel_217088628.b <> rel_217088628.b))
 or (rel_217088628.b = rel_217088628.b))) as subq_34235266
 inner join public.num_exp_mul as rel_217088660
   inner join public.onek2 as rel_217088661
   on (rel_217088660.id1 = rel_217088661.unique1 )
 on (subq_34235266.c0 = rel_217088660.id1 )
  inner join public.main_table as rel_217088662
  on (rel_217088661.unique2 = rel_217088662.a )
on (rel_217088554.b = rel_217088660.id1 )
where rel_217088554.d = rel_217088554.c
fetch first 94 rows only;


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

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

2015-08-05 Thread Petr Jelinek


On 2015-08-05 20:09, Alvaro Herrera wrote:

Josh Berkus wrote:

On 08/05/2015 10:46 AM, Alvaro Herrera wrote:

1. Add the functions as a builtins.
This is what the current patch does.  Simon seems to prefer this,
because he wants the function to be always available in production;
but I don't like this option because adding functions as builtins
makes it impossible to move later to extensions.
Bruce doesn't like this option either.


Why would we want to move them later to extensions?


Because it's not nice to have random stuff as builtins.



Extensions have one nice property, they provide namespacing so not 
everything has to be in pg_catalog which already has about gazilion 
functions. It's nice to have stuff you don't need for day to day 
operations separate but still available (which is why src/extensions is 
better than contrib).


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


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

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

Piotr Stefaniak  writes:
> On 08/05/2015 02:24 AM, Tom Lane wrote:
>> Piotr Stefaniak  writes:
>>> Set join_collapse_limit = 32 and you should see the error.

>> Ah ... now I get that error on the smaller query, but the larger one
>> (that you put in an attachment) still doesn't show any problem.
>> Do you have any other nondefault settings?

> Sorry, that needs from_collapse_limit = 32 as well.

Yeah, I assumed as much, but it still doesn't happen for me.  Possibly
something platform-dependent in statistics, or something like that.

Anyway, I fixed the problem exposed by the smaller query; would you
check that the larger one is okay for you now?

regards, tom lane

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

[HACKERS] Removing unreferenced files

2015-08-05 Thread Ron Farrer


Hello,

This is in regards to the patch[0] posted in 2006 based on previous
works[1]. Below is a summary of the issues, at present, as I understand
it along with some questions.

Initial questions that had no consensus in previous discussions:
1. Approach on file handling undecided
2. Startup vs standalone tool
3. If startup: how to determine when to run

Outstanding problems (point-for-point with original 2005 post):
1. Does not work with non-standard tablespaces. The check will even
report all files are stale, etc.
2. Has issues with stale subdirs of a tablespace (subdirs corresponding
to a nonexistent database) [appears related to #1 because of maintenance
mode and not failing]
3. Assumes relfilenode is unique database-wide when it’s only safe
tablespace-wide
4. Does not examine table segment files such as “nnn.1” - it should
instead complain when “nnn” does not match a hash entry
5. It loads every value of relfilenode in pg_class into the hash table
without checking that it is meaningful or not - needs to check.
6. strol vs strspn (or other) [not sure what the problem here is. If
errors are handled correctly this should not be an issue]
7. No checks for readdir failure [this should be easy to check for]

Other thoughts:
1. What to do if problem happens during drop table/index and the files
that should be removed are still there.. the DBA needs to know when this
happens somehow
2.  What happened to pgfsck: was that a better approach? why was that
abandoned?
3.  What to do about stale files and missing files

References:
0 -
http://www.postgresql.org/message-id/200606081508.k58f85m29...@candle.pha.pa.us
1 - http://www.postgresql.org/message-id/8291.1115340...@sss.pgh.pa.us


Ron

-- 
Command Prompt, Inc. http://www.commandprompt.com/ +1-800-492-2240
PostgreSQL Centered full stack support, consulting, and development.



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

Re: [HACKERS] LWLock deadlock and gdb advice

On 2015-08-05 11:22:55 -0700, Jeff Janes wrote:
> On Sun, Aug 2, 2015 at 8:05 AM, Andres Freund  wrote:
> 
> > On 2015-08-02 17:04:07 +0200, Andres Freund wrote:
> > > I've attached a version of the patch that should address Heikki's
> > > concern. It imo also improves the API and increases debuggability by not
> > > having stale variable values in the variables anymore. (also attached is
> > > a minor optimization that Heikki has noticed)
> >
> >
> I've run these (from your git commit) for over 60 hours with no problem, so
> I think it's solved.

Cool! Thanks for the testing.

> If there are still problems, hopefully they will come out in other tests.
> I don't know why the gin test was efficient at provoking the problem while
> none of the other ones (btree-upsert, gist, brin, btree-foreign key,
> btree-prepare transaction) I've played around with.  Perhaps it is just due
> to the amount of WAL which gin generates.

I'm not sure either... I've been able to reproduce a version of the
issue by judiciously sprinkling pg_usleep()'s over the code.

Regards,

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] RFC: replace pg_stat_activity.waiting with something more descriptive

On Wed, Aug 5, 2015 at 1:10 PM, Ildus Kurbangaliev
 wrote:
> About `memcpy`, PgBackendStatus struct already have a bunch of multi-byte
> variables,  so it will be
> not consistent anyway if somebody will want to copy it in that way. On the
> other hand two bytes in this case
> give less overhead because we can avoid the offset calculations. And as I've
> mentioned before the class
> of wait will be useful when monitoring of waits will be extended.

You're missing the point.  Those multi-byte fields have additional
synchronization requirements, as I explained in some detail in my
previous email. You can't just wave that away.

When people raise points in review, you need to respond to those with
discussion, not just blast out a new patch version that may or may not
have made some changes in that area.  Otherwise, you're wasting the
time of the people who are reviewing, which is not nice.

>> 1. have you tested this under -DEXEC_BACKEND ?  I wonder if those
>> initializations are going to work on Windows.
>
> No, it wasn't tested on Windows

I don't think it's going to work on Windows.
CreateSharedMemoryAndSemaphores() is called once only from the
postmaster on non-EXEC_BACKEND builds, but on EXEC_BACKEND builds
(i.e. Windows) it's called for every process startup.  Thus, it's got
to be idempotent: if the shared memory structures it's looking for
already exist, it must not try to recreate them.  You have, for
example, InitBufferPool() calling LWLockCreateTranche(), which
unconditionally assigns a new tranche ID.  It can't do that; all of
the processes in the system have to agree on what the tranche IDs are.

The general problem that I see with splitting apart the main lwlock
array into many tranches is that all of those tranches need to get set
up properly - with matching tranche IDs - in every backend.  In
non-EXEC_BACKEND builds, that's basically free, but on EXEC_BACKEND
builds it isn't.  I think that's OK; this won't be the first piece of
state where EXEC_BACKEND builds incur some extra overhead.  But we
should make an effort to keep that overhead small.

The way to do that, I think, is to store some state in shared memory
that, in EXEC_BACKEND builds, will allow new postmaster children to
correctly re-register all of the tranches.  It seems to me that we can
do this as follows:

1. Have a compiled-in array of built-in tranche names.
2. In LWLockShmemSize(), work out the number of lwlocks each tranche
should contain.
3. In CreateLWLocks(), if IsUnderPostmaster, grab enough shared memory
for all the lwlocks themselves plus enough extra shared memory for one
pointer per tranche.  Store pointers to the start of each tranche in
shared memory, and initialize all the lwlocks.
4. In CreateLWLocks(), if tranche registration has not yet been done
(either because we are the postmaster, or because this is the
EXEC_BACKEND case) loop over the array of built-in tranche names and
register each one with the corresponding address grabbed from shared
memory.

A more radical redesign would be to have each tranche of LWLocks as a
separate chunk of shared memory, registered with ShmemInitStruct(),
and let EXEC_BACKEND children find those chunks by name using
ShmemIndex.  But that seems like more work to me for not much benefit,
especially since there's this weird thing where lwlocks are
initialized before ShmemIndex gets set up.

Yet another possible approach is to have each module register its own
tranche and track its own tranche ID using the same kind of strategy
that replication origins and WAL insert locks already employ.  That
may well be the right long-term strategy, but it seems like sort of a
pain to all that effort right now for this project, so I'm inclined to
hack on the approach described above and see if I can get that working
for now.

-- 
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] LWLock deadlock and gdb advice

2015-08-05 Thread Jeff Janes

On Sun, Aug 2, 2015 at 8:05 AM, Andres Freund  wrote:

> On 2015-08-02 17:04:07 +0200, Andres Freund wrote:
> > I've attached a version of the patch that should address Heikki's
> > concern. It imo also improves the API and increases debuggability by not
> > having stale variable values in the variables anymore. (also attached is
> > a minor optimization that Heikki has noticed)
>
>
I've run these (from your git commit) for over 60 hours with no problem, so
I think it's solved.

If there are still problems, hopefully they will come out in other tests.
I don't know why the gin test was efficient at provoking the problem while
none of the other ones (btree-upsert, gist, brin, btree-foreign key,
btree-prepare transaction) I've played around with.  Perhaps it is just due
to the amount of WAL which gin generates.

Cheers,

Jeff

[HACKERS] Caching offsets of varlena attributes

2015-08-05 Thread Vignesh Raghunathan

Hello,

In the function heap_deform_tuple, there is a comment before caching
varlena attributes specifying that the offset will be valid for either an
aligned or unaligned value if there are no padding bytes. Could someone
please elaborate on this?

Also, why is it safe to call att_align_nominal if the attribute is not
varlena? Couldn't att_align_pointer be called for both cases? I am not able
to understand how att_align_nominal is faster.

Thanks,
Vignesh

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

Josh Berkus wrote:
> On 08/05/2015 10:46 AM, Alvaro Herrera wrote:
> > 1. Add the functions as a builtins.
> >This is what the current patch does.  Simon seems to prefer this,
> >because he wants the function to be always available in production;
> >but I don't like this option because adding functions as builtins
> >makes it impossible to move later to extensions.
> >Bruce doesn't like this option either.
> 
> Why would we want to move them later to extensions?

Because it's not nice to have random stuff as builtins.

-- 
Á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] Doubt about AccessExclusiveLock in ALTER TABLE .. SET ( .. );

2015-08-05 Thread Fabrízio de Royes Mello

On Wed, Aug 5, 2015 at 9:31 AM, Robert Haas  wrote:
>
> On Tue, Aug 4, 2015 at 1:15 PM, Alvaro Herrera 
wrote:
> > That opens up for lock escalation and deadlocks, doesn't it?  You are
> > probably thinking that it's okay to ignore those but I don't necessarily
> > agree with that.
>
> Agreed.  I think we're making a mountain out of a molehill here.  As
> long as the locks that are actually used are monotonic, just use > and
> stick a comment in there explaining that it could need adjustment if
> we use other lock levels in the future.  I presume all the lock-levels
> used for DDL are, and will always be, self-exclusive, so why all this
> hand-wringing?
>

New version attached with suggested changes.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 1c1c181..ad985cd 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -543,6 +543,10 @@ ALTER TABLE ALL IN TABLESPACE name
   of ALTER TABLE that forces a table rewrite.
  
 
+ 
+  Changing autovacuum storage parameters acquires a SHARE UPDATE EXCLUSIVE lock.
+ 
+
  
   
While CREATE TABLE allows OIDS to be specified
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 180f529..c39b878 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -57,7 +57,8 @@ static relopt_bool boolRelOpts[] =
 		{
 			"autovacuum_enabled",
 			"Enables autovacuum in this relation",
-			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
+			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST,
+			ShareUpdateExclusiveLock
 		},
 		true
 	},
@@ -65,7 +66,8 @@ static relopt_bool boolRelOpts[] =
 		{
 			"user_catalog_table",
 			"Declare a table as an additional catalog table, e.g. for the purpose of logical replication",
-			RELOPT_KIND_HEAP
+			RELOPT_KIND_HEAP,
+			AccessExclusiveLock
 		},
 		false
 	},
@@ -73,7 +75,8 @@ static relopt_bool boolRelOpts[] =
 		{
 			"fastupdate",
 			"Enables \"fast update\" feature for this GIN index",
-			RELOPT_KIND_GIN
+			RELOPT_KIND_GIN,
+			AccessExclusiveLock
 		},
 		true
 	},
@@ -81,7 +84,8 @@ static relopt_bool boolRelOpts[] =
 		{
 			"security_barrier",
 			"View acts as a row security barrier",
-			RELOPT_KIND_VIEW
+			RELOPT_KIND_VIEW,
+			AccessExclusiveLock
 		},
 		false
 	},
@@ -95,7 +99,8 @@ static relopt_int intRelOpts[] =
 		{
 			"fillfactor",
 			"Packs table pages only to this percentage",
-			RELOPT_KIND_HEAP
+			RELOPT_KIND_HEAP,
+			AccessExclusiveLock
 		},
 		HEAP_DEFAULT_FILLFACTOR, HEAP_MIN_FILLFACTOR, 100
 	},
@@ -103,7 +108,8 @@ static relopt_int intRelOpts[] =
 		{
 			"fillfactor",
 			"Packs btree index pages only to this percentage",
-			RELOPT_KIND_BTREE
+			RELOPT_KIND_BTREE,
+			AccessExclusiveLock
 		},
 		BTREE_DEFAULT_FILLFACTOR, BTREE_MIN_FILLFACTOR, 100
 	},
@@ -111,7 +117,8 @@ static relopt_int intRelOpts[] =
 		{
 			"fillfactor",
 			"Packs hash index pages only to this percentage",
-			RELOPT_KIND_HASH
+			RELOPT_KIND_HASH,
+			AccessExclusiveLock
 		},
 		HASH_DEFAULT_FILLFACTOR, HASH_MIN_FILLFACTOR, 100
 	},
@@ -119,7 +126,8 @@ static relopt_int intRelOpts[] =
 		{
 			"fillfactor",
 			"Packs gist index pages only to this percentage",
-			RELOPT_KIND_GIST
+			RELOPT_KIND_GIST,
+			AccessExclusiveLock
 		},
 		GIST_DEFAULT_FILLFACTOR, GIST_MIN_FILLFACTOR, 100
 	},
@@ -127,7 +135,8 @@ static relopt_int intRelOpts[] =
 		{
 			"fillfactor",
 			"Packs spgist index pages only to this percentage",
-			RELOPT_KIND_SPGIST
+			RELOPT_KIND_SPGIST,
+			AccessExclusiveLock
 		},
 		SPGIST_DEFAULT_FILLFACTOR, SPGIST_MIN_FILLFACTOR, 100
 	},
@@ -135,7 +144,8 @@ static relopt_int intRelOpts[] =
 		{
 			"autovacuum_vacuum_threshold",
 			"Minimum number of tuple updates or deletes prior to vacuum",
-			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
+			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST,
+			ShareUpdateExclusiveLock
 		},
 		-1, 0, INT_MAX
 	},
@@ -143,7 +153,8 @@ static relopt_int intRelOpts[] =
 		{
 			"autovacuum_analyze_threshold",
 			"Minimum number of tuple inserts, updates or deletes prior to analyze",
-			RELOPT_KIND_HEAP
+			RELOPT_KIND_HEAP,
+			ShareUpdateExclusiveLock
 		},
 		-1, 0, INT_MAX
 	},
@@ -151,7 +162,8 @@ static relopt_int intRelOpts[] =
 		{
 			"autovacuum_vacuum_cost_delay",
 			"Vacuum cost delay in milliseconds, for autovacuum",
-			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
+			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST,
+			ShareUpdateExclusiveLock
 		},
 		-1, 0, 100
 	},
@@ -159,7 +171,8 @@ static relopt_int intRelOpts[] =
 		{
 			"autovacuum_vacuum_cost_limit",
 			"Vacuum cost amount available before napping, for autovacuum",
-			RELOPT_KIND

Re: [HACKERS] Raising our compiler requirements for 9.6

On 2015-08-05 17:19:05 +0200, Andres Freund wrote:
> On 2015-08-05 11:12:34 -0400, Tom Lane wrote:
> > Andres Freund  writes:
> > > Ok, lets' do it that way then. It seems the easiest way to test for this
> > > is to use something like
> > 
> > > # "IBM XL C/C++ for AIX, V12.1" miscompiles, for 32-bit, some inline
> > > # expansions of ginCompareItemPointers() "long long" arithmetic.  To
> > > # take advantage of inlining, build a 64-bit PostgreSQL.
> > > test $(getconf HARDWARE_BITMODE) == '32' then
> > >CFLAGS="$CFLAGS -DPG_FORCE_DISABLE_INLINE"
> > > fi

So that approach doesn't work out well because the 32 bit xlc can be
installed on the 64 bit system.

> > Actually, much the easiest way to convert what Noah did would be to add
> > 
> > #if defined(__ILP32__) && defined(__IBMC__)
> > #define PG_FORCE_DISABLE_INLINE
> > #endif
> > 
> > in src/include/port/aix.h.

Therefore I'm going to reshuffle things in that direction tomorrow. I'll
wait for other fallout first though. So far only gcc, xlc and clang (via
gcc frontend) have run...

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

On 08/05/2015 10:46 AM, Alvaro Herrera wrote:
> 1. Add the functions as a builtins.
>This is what the current patch does.  Simon seems to prefer this,
>because he wants the function to be always available in production;
>but I don't like this option because adding functions as builtins
>makes it impossible to move later to extensions.
>Bruce doesn't like this option either.

Why would we want to move them later to extensions?  Do you anticipate
not needing them in the future?  If we don't need them in the future,
why would they continue to exist at all?

I'm really not getting this.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.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] Freeze avoidance of very large table.

Josh Berkus wrote:
> On 08/05/2015 10:26 AM, Bruce Momjian wrote:

> > I don't care what we do, but I do think we should be consistent. 
> > Frankly I am unclear why I am even having to make this point, as cases
> > where we have chosen expediency over consistency have served us badly in
> > the past.
> 
> Saying "it's stupid to be consistent with a bad old rule", and making a
> new rule is not "expediency".

So I discussed this with Bruce on IM a bit.  I think there are basically
four ways we could go about this:

1. Add the functions as a builtins.
   This is what the current patch does.  Simon seems to prefer this,
   because he wants the function to be always available in production;
   but I don't like this option because adding functions as builtins
   makes it impossible to move later to extensions.
   Bruce doesn't like this option either.

2. Add the functions to contrib, keep them there for the foreesable future.
   Simon is against this option, because the functions will be
   unavailable when needed in production.  I am of the same position.
   Bruce opines this option is acceptable.

3. a) Add the function to some extension in contrib now, by using a
  slightly modified version of the current patch, and
   b) Apply some later patch to move said extension to src/extension.

4. a) Patch some extension(s) to move it to src/extension,
   b) Apply a version of this patch that adds the new functions to said
  extension

Essentially 3 and 4 are the same thing except the order is reversed;
they both result in the functions being shipped in some "core extension"
(a concept we do not have today).  Bruce says either of these is fine
with him.  I am fine with either of them also.  As long as we do 3b
during 9.6 timeframe, the outcome of either 3 and 4 seems to be
acceptable for Simon also.

Robert seems to be saying that he doesn't care about moving extensions
to core at all.

What do others think?

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

On 08/05/2015 10:26 AM, Bruce Momjian wrote:
> On Wed, Aug  5, 2015 at 10:22:48AM -0700, Josh Berkus wrote:
>> On 08/05/2015 10:00 AM, Alvaro Herrera wrote:
>>> Anyway, the patch as proposed puts the new functions in core as builtins
>>> (which is what Bruce seems to be objecting to).  Maybe instead of
>>> proposing moving existing extensions in core, it would be better to have
>>> this patch put those two new functions alone as a single new extension
>>> in src/extension, and not move anything else.  I don't necessarily
>>> resist adding these functions as builtins, but if we do that then
>>> there's no going back to having them as an extension instead, which is
>>> presumably more in line with what we want in the long run.
>>
>> For my part, I am unclear on why we are putting *any* diagnostic tools
>> in /contrib today.  Either the diagnostic tools are good quality and
>> necessary for a bunch of users, in which case we ship them in core, or
>> they are obscure and/or untested, in which case they go in an external
>> project and/or on PGXN.
>>
>> Yes, for tools with overhead we might want to require enabling them in
>> pg.conf.  But that's very different from requiring the user to install a
>> separate package.
> 
> I don't care what we do, but I do think we should be consistent. 
> Frankly I am unclear why I am even having to make this point, as cases
> where we have chosen expediency over consistency have served us badly in
> the past.

Saying "it's stupid to be consistent with a bad old rule", and making a
new rule is not "expediency".

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.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] Draft Alpha 2 announcement

On 08/05/2015 10:29 AM, Joe Conway wrote:
> On 08/05/2015 10:15 AM, Josh Berkus wrote:
>> * Make pg_dump work back up Row Level Security policies
> 
> There were far more than this single bugfix related to RLS. You might
> want to say something like:
> 
> * Numerous Row Level Security related bug fixes

I have a line like that (but I like "numerous", will use that); I wanted
also to call out pg_dump because it is a user-visible fix in need of
testing.


-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.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] Draft Alpha 2 announcement

2015-08-05 Thread Joe Conway

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 08/05/2015 10:15 AM, Josh Berkus wrote:
> * Make pg_dump work back up Row Level Security policies

There were far more than this single bugfix related to RLS. You might
want to say something like:

* Numerous Row Level Security related bug fixes

- -- 
Joe Conway
Crunchy Data
Enterprise PostgreSQL
http://crunchydata.com
-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJVwkf1AAoJEDfy90M199hlEnwP/RNObNwpdzN9MxWTEzLQfB5o
PdQ4kjE4tY/PdYrLv2G7a12G9i/Lfkza1grHc+FaAuc7QPToOPLEut/punZgPlU2
+Zf9Bn1Pjkl9h37BbW1H978qAq6vbS4R+9mp8qamMKfmYlJptCSOrG549wTqOesM
640ZfFsnB/3L0ApIcfrg/EYZPrl/Ue9PntFK1cKwcu9BAPC+Sc/kM5P/nxyz2Avo
Ovetv1AUlSWTLARDBDyXDiCaB9ADWLrlPvfNIt6DuzkEKADzh8R49YLCOkhdWW+a
z9LYqINtlRfwN9oh7ob/OJD3FT1SPjHOiBwQB8bGJGSAqeMSTY5owuBZOnDlcZx5
cK7CFUQioIY831o5z/nyLdzYR/LRzn9tr4yLoVIDB0ZERdBtu7xI2pM3S2TB9yJk
U2PySM2LFTBLlXTI1al38/yKVZCK+aLcF3jqZxEbEOE5SsaudoDQUcrR3MeSSwoy
bnUYDez4sLa96j4seF0yiRSlYYUZFxE/BW+VW8QF0e9l8JX0WWoZCTMC7m28eUMN
HMa5skbQrE40taJt1kbNz9FGR8u5EeF2cbe5A9ys7dvijDqt1fEYLNQCY4FZ3g3/
yPFip0txcL2kJ8qGUILxWn4dGeNXQTcBQg1L41ZkOEmTW+oEVvUZ0P2TtRBYXKG1
2+cebmtLgaO8O3cY4O+7
=9lWg
-END PGP SIGNATURE-


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

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

On Wed, Aug  5, 2015 at 10:22:48AM -0700, Josh Berkus wrote:
> On 08/05/2015 10:00 AM, Alvaro Herrera wrote:
> > Anyway, the patch as proposed puts the new functions in core as builtins
> > (which is what Bruce seems to be objecting to).  Maybe instead of
> > proposing moving existing extensions in core, it would be better to have
> > this patch put those two new functions alone as a single new extension
> > in src/extension, and not move anything else.  I don't necessarily
> > resist adding these functions as builtins, but if we do that then
> > there's no going back to having them as an extension instead, which is
> > presumably more in line with what we want in the long run.
> 
> For my part, I am unclear on why we are putting *any* diagnostic tools
> in /contrib today.  Either the diagnostic tools are good quality and
> necessary for a bunch of users, in which case we ship them in core, or
> they are obscure and/or untested, in which case they go in an external
> project and/or on PGXN.
> 
> Yes, for tools with overhead we might want to require enabling them in
> pg.conf.  But that's very different from requiring the user to install a
> separate package.

I don't care what we do, but I do think we should be consistent. 
Frankly I am unclear why I am even having to make this point, as cases
where we have chosen expediency over consistency have served us badly in
the past.

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

  + Everyone has their own god. +


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

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

On 08/05/2015 10:00 AM, Alvaro Herrera wrote:
> Anyway, the patch as proposed puts the new functions in core as builtins
> (which is what Bruce seems to be objecting to).  Maybe instead of
> proposing moving existing extensions in core, it would be better to have
> this patch put those two new functions alone as a single new extension
> in src/extension, and not move anything else.  I don't necessarily
> resist adding these functions as builtins, but if we do that then
> there's no going back to having them as an extension instead, which is
> presumably more in line with what we want in the long run.

For my part, I am unclear on why we are putting *any* diagnostic tools
in /contrib today.  Either the diagnostic tools are good quality and
necessary for a bunch of users, in which case we ship them in core, or
they are obscure and/or untested, in which case they go in an external
project and/or on PGXN.

Yes, for tools with overhead we might want to require enabling them in
pg.conf.  But that's very different from requiring the user to install a
separate package.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.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] Draft Alpha 2 announcement

Hackers,

Please check over the attached for errors.

Also, please suggest major fixes/changes since Alpha 1 I might have
missed. Thanks!

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com
PostgreSQL 9.5 Alpha 2 Released
===

The PostgreSQL Global Development Group announces today that the second alpha release of PostgreSQL 9.5 is available for download. This release contains previews of all of the features which will be available in the final release of version 9.5, although some details will change before then. Please download, test, and report what you find.

Changes Since Alpha1

Many bugs and issues reported by our users and contributors have been fixed since the release of Alpha1. These include:

* Make pg_dump work back up Row Level Security policies
* Make pg_rewind work with symlinks
* Fix several issue with locking
* Multiple changes and improvements to Row Level Security
* Correct some oversights in BRIN indexes
* Fix behavior of JSON negative array subscripts
* Correct strxfrm() behavior on buggy platforms
* Multiple documentation changes and additions

If you reported an issue while testing PostgreSQL 9.5, please download Alpha2 and test whether that issue has been fixed. If you have not yet tested version 9.5, now is the time for you to help out PostgreSQL development.

Alpha and Beta Schedule
---

This is the alpha release of version 9.5, indicating that some changes to features are still possible before release. The PostgreSQL Project will release 9.5 another alpha or beta in September, and then periodically release alphas or betas as required for testing until the final release in late 2015. For more information, and suggestions on how to test the alpha and betas, see the Beta Testing page.

Full documentation and release notes of the new version is available online and also installs with PostgreSQL. Also see the What's New page for details on some features.

Links
-

* [Downloads Page](http://www.postgresql.org/download/)
* [Beta Testing Information](http://www.postgresql.org/developer/alpha)
* [9.5 Alpha Release Notes](http://www.postgresql.org/docs/devel/static/release-9-5.html)
* [What's New in 9.5](https://wiki.postgresql.org/wiki/What%27s_new_in_PostgreSQL_9.5)

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

Re: [HACKERS] RFC: replace pg_stat_activity.waiting with something more descriptive

2015-08-05 Thread Ildus Kurbangaliev


On 08/04/2015 11:47 PM, Robert Haas wrote:

On Tue, Aug 4, 2015 at 4:37 PM, Ildus Kurbangaliev
 wrote:

A new version of the patch. I used your idea with macros, and with tranches that
allowed us to remove array with names (they can be written directly to the 
corresponding
tranche).

You seem not to have addressed a few of the points I brought up here:

http://www.postgresql.org/message-id/CA+TgmoaGqhah0VTamsfaOMaE9uOrCPYSXN8hCS9=wirupjs...@mail.gmail.com



About `memcpy`, PgBackendStatus struct already have a bunch of 
multi-byte variables,  so it will be
not consistent anyway if somebody will want to copy it in that way. On 
the other hand two bytes in this case
give less overhead because we can avoid the offset calculations. And as 
I've mentioned before the class

of wait will be useful when monitoring of waits will be extended.

Other things from that patch already changed in latest patch.

On 08/04/2015 11:53 PM, Alvaro Herrera wrote:

Just a bystander here, I haven't reviewed this patch at all, but I have
two questions,

1. have you tested this under -DEXEC_BACKEND ?  I wonder if those
initializations are going to work on Windows.

No, it wasn't tested on Windows

--
Ildus Kurbangaliev
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company



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

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

Robert Haas wrote:
> On Wed, Aug 5, 2015 at 12:36 PM, Alvaro Herrera
>  wrote:
> > Bruce Momjian wrote:
> >> I understand the desire for a diagnostic function in core, but we have
> >> to be consistent.  Just because we are adding this function now doesn't
> >> mean we should use different rules from what we did previously for
> >> diagnostic functions.  Either their is logic to why this function is
> >> different from the other diagnostic functions in contrib, or we need to
> >> have a separate discussion of whether diagnostic functions belong in
> >> contrib or core.
> >
> > Then let's start moving some extensions to src/extension/.
> 
> That seems like yet another separate issue.
> 
> FWIW, it seems to me that we've done a heck of a lot of moving stuff
> out of contrib over the last few releases.  A bunch of things moved to
> src/test/modules and a bunch of things went to src/bin.  We can move
> more, of course, but this code reorganization has non-trivial costs
> and I'm not clear what benefits we hope to realize and whether we are
> in fact realizing those benefits.  At this point, the overwhelming
> majority of what's in contrib is extensions; we're not far from being
> able to put the whole thing in src/extensions if it really needs to be
> moved at all.

There are a number of things in contrib that are not extensions, and
others are not core-quality yet.  I don't think we should move
everything; at least not everything in one go.  I think there are a
small number of diagnostic extensions that would be useful to have in
core (pageinspect, pg_buffercache, pg_stat_statements).

> But I don't think it's fair to conflate that with Bruce's question,
> which it seems to me is both a fair question and a different one.

Well, there was no question as such.  If the question is "should we
instead put it in contrib just to be consistent?" then I think the
answer is no.  I value consistency as much as every other person, but I
there are other things I value more, such as availability.  If stuff is
in contrib and servers don't have it installed because of package
policies and it takes three management layers' approval to get it
installed in a dying server, then I prefer to have it in core.

If the question was "why are we not using the rule we previously had
that diagnostic tools were in contrib?" then I think the answer is that
we have evolved and we now know better.  We have evolved in the sense
that we have more stuff in production now that needs better diagnostic
tooling to be available; and we know better now in the sense that we
have realized there's this company policy bureaucracy that things in
contrib are not always available for reasons that are beyond us.

Anyway, the patch as proposed puts the new functions in core as builtins
(which is what Bruce seems to be objecting to).  Maybe instead of
proposing moving existing extensions in core, it would be better to have
this patch put those two new functions alone as a single new extension
in src/extension, and not move anything else.  I don't necessarily
resist adding these functions as builtins, but if we do that then
there's no going back to having them as an extension instead, which is
presumably more in line with what we want in the long run.

(It would be a shame to delay this patch, which messes with complex
innards, just because of a discussion about the placement of two
smallish diagnostic functions.)

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

On Wed, Aug 5, 2015 at 12:36 PM, Alvaro Herrera
 wrote:
> Bruce Momjian wrote:
>> I understand the desire for a diagnostic function in core, but we have
>> to be consistent.  Just because we are adding this function now doesn't
>> mean we should use different rules from what we did previously for
>> diagnostic functions.  Either their is logic to why this function is
>> different from the other diagnostic functions in contrib, or we need to
>> have a separate discussion of whether diagnostic functions belong in
>> contrib or core.
>
> Then let's start moving some extensions to src/extension/.

That seems like yet another separate issue.

FWIW, it seems to me that we've done a heck of a lot of moving stuff
out of contrib over the last few releases.  A bunch of things moved to
src/test/modules and a bunch of things went to src/bin.  We can move
more, of course, but this code reorganization has non-trivial costs
and I'm not clear what benefits we hope to realize and whether we are
in fact realizing those benefits.  At this point, the overwhelming
majority of what's in contrib is extensions; we're not far from being
able to put the whole thing in src/extensions if it really needs to be
moved at all.

But I don't think it's fair to conflate that with Bruce's question,
which it seems to me is both a fair question and a different 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] Freeze avoidance of very large table.

Bruce Momjian wrote:

> I understand the desire for a diagnostic function in core, but we have
> to be consistent.  Just because we are adding this function now doesn't
> mean we should use different rules from what we did previously for
> diagnostic functions.  Either their is logic to why this function is
> different from the other diagnostic functions in contrib, or we need to
> have a separate discussion of whether diagnostic functions belong in
> contrib or core.

Then let's start moving some extensions to src/extension/.

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

On Wed, Jul  8, 2015 at 02:31:04PM +0100, Simon Riggs wrote:
> On 7 July 2015 at 18:45, Sawada Masahiko  wrote:
> 
> On Wed, Jul 8, 2015 at 12:37 AM, Andres Freund  wrote:
> > On 2015-07-07 16:25:13 +0100, Simon Riggs wrote:
> >> I don't think pg_freespacemap is the right place.
> >
> > I agree that pg_freespacemap sounds like an odd location.
> >
> >> I'd prefer to add that as a single function into core, so we can write
> >> formal tests.
> >
> > With the advent of src/test/modules it's not really a prerequisite for
> > things to be builtin to be testable. I think there's fair arguments for
> > moving stuff like pg_stattuple, pg_freespacemap, pg_buffercache into
> > core at some point, but that's probably a separate discussion.
> >
> 
> I understood.
> So I will place bunch of test like src/test/module/visibilitymap_test,
> which contains  some tests regarding this feature,
> and gather them into one patch.
> 
> 
> Please place it in core. I see value in having a diagnostic function for
> general use on production systems.

Sorry to be coming to this discussion late.

I understand the desire for a diagnostic function in core, but we have
to be consistent.  Just because we are adding this function now doesn't
mean we should use different rules from what we did previously for
diagnostic functions.  Either their is logic to why this function is
different from the other diagnostic functions in contrib, or we need to
have a separate discussion of whether diagnostic functions belong in
contrib or core.

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

  + Everyone has their own god. +


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

Re: [HACKERS] Dependency between bgw_notify_pid and bgw_flags

Ashutosh Bapat wrote:

> Looking at larger picture, we should also enable this feature to be
> used by auxilliary processes. It's very hard to add a new auxilliary
> process in current code.  One has to go add code at many places to
> make sure that the auxilliary processes die and are re-started
> correctly.

No kidding.

> Even tougher to add a parent auxilliary process, which spawns multiple
> worker processes.

I'm not sure about this point.  The autovacuum launcher, which is an
obvious candidate to have "children" processes, does not do that but
instead talks to postmaster to do it instead.  We did it that way to
avoid the danger of children processes connected to shared memory that
would not be direct children of postmaster; that could cause reliability
problems if the intermediate process fails to signal its children for
some reason.  Along the same lines I would suggest that other bgworker
processes should also be controllers only, that is it can ask postmaster
to start other processes but not start them directly.

> That
> would be whole lot simpler if we could allow the auxilliary processes to
> use background worker infrastructure (which is what they are utlimately).
> 
> About the flags BGWORKER_BACKEND_DATABASE_CONNECTION and
> BGWORKER_SHMEM_ACCESS
>  BGWORKER_BACKEND_DATABASE_CONNECTION is used at seven places in the code:
> one is assertion, two check existence of this flag, when backend actually
> connects to a database, fourth checks whether BGWORKER_SHMEM_ACCESS is also
> set, fifth creates parallel workers with this flag, sixth uses the flag to
> add backend to the backed list (which you have removed). Seventh usage is
> only usage which installs signal handlers based on this flag, which too I
> guess can be overridden (or set correctly) by the actual background worker
> code.
> 
> BGWORKER_SHMEM_ACCESS has similar usage, except that it resets the on exit
> callbacks and detaches the shared memory segment from the background
> worker. That avoids a full cluster restart when one of those worker which
> can not corrupt shared memory dies. But I do not see any check to prevent
> such backend from calling PGSharedMemoryReattach()
> 
> So it looks like, it suffices to assume that background worker either needs
> to access shared memory or doesn't. Any background worker having shared
> memory access can also access database and thus becomes part of the backend
> list. Or may be we just avoid these flags and treat every background worker
> as if it passed both these flags. That will simplify a lot of code.

If you want to make aux processes use the bgworker infrastructure (a
worthy goal I think), it is possible that more uses would be found for
those flags.  For instance, avlauncher is equivalent to a worker that
has SHMEM_ACCESS but no DATABASE_CONNECTION; maybe some of the code
would differ depending on whether or not DATABASE_CONNECTION is
specified.  The parallel to the avlauncher was the main reason I decided
to separate those two flags.  Therefore I wouldn't try to simplify just
yet.  If you succeed in making the avlauncher (and more generally all of
autovacuum) use bgworker code, perhaps that would be the time to search
for simplifications to make, because we would know more about how it
would be used.

>From your list above it doesn't sound like DATABASE_CONNECTION actually
does anything useful other than sanity checks.  I wonder if the behavior
that avlauncher becomes part of the backend list would be sane (this is
what would happen if you simply remove the DATABASE_CONNECTION flag and
then turn avlauncher into a regular bgworker).

On further thought, given that almost every aux process has particular
timing needs for start/stop under different conditions, I am doubtful
that you could turn any of them into a bgworker.  Maybe pgstat would be
the only exception, perhaps bgwriter, not sure.

-- 
Á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] Raising our compiler requirements for 9.6

On 2015-08-05 11:23:22 -0400, Tom Lane wrote:
> Andres Freund  writes:
> > I'm ok with that too, although I do like the warning at configure
> > time. I'd go with the template approach due to that, but I don't feel
> > strongly about it.
> 
> Meh.  I did *not* like the way you proposed doing that: it looked far too
> dependent on autoconf internals (the kind that they change regularly).

Hm, I'd actually checked that as_echo worked back till 9.0. But it
doesn't exist in much older releases.

> If you can think of a way of emitting a warning that isn't going to break
> in a future autoconf release, then ok.

echo "$as_me: WARNING: disabling inlining on 32 bit aix due to a bug in xlc" 
2>&1

then. That'd have worked back in 7.4 and the worst that'd happen is that
$as_me is empty.



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

Re: [HACKERS] Raising our compiler requirements for 9.6

Andres Freund  writes:
> I'm ok with that too, although I do like the warning at configure
> time. I'd go with the template approach due to that, but I don't feel
> strongly about it.

Meh.  I did *not* like the way you proposed doing that: it looked far too
dependent on autoconf internals (the kind that they change regularly).
If you can think of a way of emitting a warning that isn't going to break
in a future autoconf release, then ok.

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] Raising our compiler requirements for 9.6

On 2015-08-05 11:12:34 -0400, Tom Lane wrote:
> Andres Freund  writes:
> > Ok, lets' do it that way then. It seems the easiest way to test for this
> > is to use something like
> 
> > # "IBM XL C/C++ for AIX, V12.1" miscompiles, for 32-bit, some inline
> > # expansions of ginCompareItemPointers() "long long" arithmetic.  To
> > # take advantage of inlining, build a 64-bit PostgreSQL.
> > test $(getconf HARDWARE_BITMODE) == '32' then
> >CFLAGS="$CFLAGS -DPG_FORCE_DISABLE_INLINE"
> > fi
> 
> > in the xlc part of the template?

(there's a ; missing and it should be CPPFLAGS rather than CFLAGS)

> Actually, much the easiest way to convert what Noah did would be to add
> 
> #if defined(__ILP32__) && defined(__IBMC__)
> #define PG_FORCE_DISABLE_INLINE
> #endif
> 
> in src/include/port/aix.h.

I'm ok with that too, although I do like the warning at configure
time. I'd go with the template approach due to that, but I don't feel
strongly about it.

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] Raising our compiler requirements for 9.6

Andres Freund  writes:
> Ok, lets' do it that way then. It seems the easiest way to test for this
> is to use something like

> # "IBM XL C/C++ for AIX, V12.1" miscompiles, for 32-bit, some inline
> # expansions of ginCompareItemPointers() "long long" arithmetic.  To
> # take advantage of inlining, build a 64-bit PostgreSQL.
> test $(getconf HARDWARE_BITMODE) == '32' then
>CFLAGS="$CFLAGS -DPG_FORCE_DISABLE_INLINE"
> fi

> in the xlc part of the template?

Actually, much the easiest way to convert what Noah did would be to add

#if defined(__ILP32__) && defined(__IBMC__)
#define PG_FORCE_DISABLE_INLINE
#endif

in src/include/port/aix.h.

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] Reduce ProcArrayLock contention

2015-08-05 Thread Amit Kapila

On Tue, Aug 4, 2015 at 8:59 PM, Robert Haas  wrote:
>
>
> I'm not entirely happy with the name "nextClearXidElem" but apart from
> that I'm fairly happy with this version.  We should probably test it
> to make sure I haven't broken anything;

I have verified the patch and it is fine.  I have tested it via manual
tests; for long pgbench tests, results are quite similar to previous
versions of patch.

Few changes, I have made in patch:

1.

+static void

+ProcArrayGroupClearXid(PGPROC *proc, TransactionId latestXid)

+{

+ volatile PROC_HDR *procglobal = ProcGlobal;

+ uint32 nextidx;

+ uint32 wakeidx;

+ int extraWaits = -1;

+

+ /* We should definitely have an XID to clear. */

+ Assert(TransactionIdIsValid(pgxact->xid));

Here Assert is using pgxact which is wrong.

2. Made ProcArrayEndTransactionInternal as inline function as
suggested by you.

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

group-xid-clearing-v5.patch
Description: Binary data

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

Re: [HACKERS] [PATCH] libpq: Allow specifying multiple host names to try to connect to

On Wed, Jul  8, 2015 at 12:24:37PM -0400, Robbie Harwood wrote:
> > You update the documentation just for  psql but your change effects any 
> > libpq application if we go forward with this patch we should update the 
> > documentation for libpq as well.
> >
> > This approach seems to work with the url style of conninfo
> >
> > For example
> >   postgres://some-down-host.info,some-other-host.org:5435/test1
> >
> > seems to work as expected but I don't like that syntax I would rather see
> > postgres://some-down-host.info:5435/test1,postgres://some-other-host.org:5435/test1
> >
> > This would be a more invasive change but I think the syntax is more usable.
> 
> I agree with this; it seems to me that it's more powerful to be able to
> specify complete urls for when they may differ.
> 
> For the non-url case though, I don't see a clean way of doing this.  We
> could always, e.g., locally bind port specification to the closest host
> specification, but that seems nasty, and is still less powerful than
> passing urls (or we could just do the same for all parameters, but
> that's just a mess).
> 
> Might it be reasonable to only allow the multi-host syntax in the
> url-style and not otherwise?

First, I agree this is a very useful feature that we want.  Many NoSQL
databases are promoting multi-host client libraries as HA, which is kind
of humorous, and also makes sense because many NoSQL solution are
multi-host.

I can see this feature benefitting us for clients to auto-failover
without requiring a pooler or virtual IP reassignment, and also useful
for read-only connections that want to connect to a read-only slave, but
don't care which one.  The idea of randomly selecting a host from the
list might be a future feature.

I agree we should allow the specification of multiple hosts, e.g. -h
"host1,host2", but anything more complex should require the URL syntax,
and require full URLs separated by commas, not commas inside a single
URL to specify multiple host names, as shown above.  If repeating
information inside each URL is a problem, the user can still use
connections-specific options to controls things, e.g. by using -p 5433,
it is not necessary to specify the port number in the URLs:

$ psql -p 5433  postgres://localhost/test,postgres://localhost/test2

I realize this is libpq-feature-creep, but considering the complexities
of a pooler and virtual IP address reassignment, I think adding this
makes sense.  The fact that other DBs are doing it, including I think
VMWare's libpq, supports the idea of adding this simple specification.

Can someone work on a patch to implement this?

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

  + Everyone has their own god. +

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

Re: [HACKERS] Raising our compiler requirements for 9.6

On 2015-08-05 10:32:48 -0400, Tom Lane wrote:
> Andres Freund  writes:
> > Wasn't the point that 32 bit AIX as a whole hasn't been supported for a
> > couple years now? My willingness to expend effort for that is rather
> > limited.
> 
> Well, there's one in the buildfarm.

Oh nice. That's new. I see it has been added less than a week ago ;)

> We don't generally turn off buildfarm critters just because the
> underlying OS is out of support (the population would be quite a bit
> lower if we did).

I didn't know there was a buildfarm animal. We'd been pleading a bunch
of people over the last few years to add one. If there's an actual way
to see platforms breaking I'm more accepting to try and keep them alive.

> > I mean I'd otherwise ok with a PG_FORCE_DISABLE_INLINE flag that takes
> > effect in c.h or so. That could easily be set in src/template/aix. Might
> > also be useful for investigatory purposes every couple years or so.
> 
> +1, that could have other use-cases.

Ok, lets' do it that way then. It seems the easiest way to test for this
is to use something like

# "IBM XL C/C++ for AIX, V12.1" miscompiles, for 32-bit, some inline
# expansions of ginCompareItemPointers() "long long" arithmetic.  To
# take advantage of inlining, build a 64-bit PostgreSQL.
test $(getconf HARDWARE_BITMODE) == '32' then
   CFLAGS="$CFLAGS -DPG_FORCE_DISABLE_INLINE"
fi

in the xlc part of the template?

do we want to add something like
$as_echo "$as_me: WARNING: disabling inlining on 32 bit aix due to compiler 
bugs"
? Seems like a good idea to me.

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] Raising our compiler requirements for 9.6

Andres Freund  writes:
> Wasn't the point that 32 bit AIX as a whole hasn't been supported for a
> couple years now? My willingness to expend effort for that is rather
> limited.

Well, there's one in the buildfarm.  We don't generally turn off
buildfarm critters just because the underlying OS is out of support
(the population would be quite a bit lower if we did).

> I mean I'd otherwise ok with a PG_FORCE_DISABLE_INLINE flag that takes
> effect in c.h or so. That could easily be set in src/template/aix. Might
> also be useful for investigatory purposes every couple years or so.

+1, that could have other use-cases.

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] Raising our compiler requirements for 9.6

On 2015-08-05 10:23:31 -0400, Tom Lane wrote:
> Andres Freund  writes:
> > How about moving that error check into into the aix template file and
> > erroring out there? Since this is master I think it's perfectly fine to
> > refuse to work with the buggy unsupported 32 bit compiler. The argument
> > not to do so was that PG previously worked in the back branches
> > depending on the minor version, but that's not an argument on master.
> 
> The check as Noah wrote it rejects *all* 32-bit IBM compilers, not just
> buggy ones.  That was okay when the effect was only a rather minor
> performance loss, but refusing to build at all would raise the stakes
> quite a lot.  Unless you are volunteering to find out how to tell broken
> compilers from fixed ones more accurately, I think you need to confine
> the effects of the check to disabling inlining.

Wasn't the point that 32 bit AIX as a whole hasn't been supported for a
couple years now? My willingness to expend effort for that is rather
limited.

I mean I'd otherwise ok with a PG_FORCE_DISABLE_INLINE flag that takes
effect in c.h or so. That could easily be set in src/template/aix. Might
also be useful for investigatory purposes every couple years or so.


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] Raising our compiler requirements for 9.6

Andres Freund  writes:
> On 2015-08-05 10:08:10 -0400, Tom Lane wrote:
>> Hmm.  I notice that this removes Noah's hack from commit c53f73879f552a3c.
>> Do we care about breaking old versions of xlc, and if so, how are we going
>> to fix that?  (I assume it should be possible to override AC_C_INLINE's
>> result, but I'm not sure where would be a good place to do so.)

> Hm. That's a good point.

> How about moving that error check into into the aix template file and
> erroring out there? Since this is master I think it's perfectly fine to
> refuse to work with the buggy unsupported 32 bit compiler. The argument
> not to do so was that PG previously worked in the back branches
> depending on the minor version, but that's not an argument on master.

The check as Noah wrote it rejects *all* 32-bit IBM compilers, not just
buggy ones.  That was okay when the effect was only a rather minor
performance loss, but refusing to build at all would raise the stakes
quite a lot.  Unless you are volunteering to find out how to tell broken
compilers from fixed ones more accurately, I think you need to confine
the effects of the check to disabling inlining.

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] Raising our compiler requirements for 9.6

On 2015-08-05 10:08:10 -0400, Tom Lane wrote:
> Andres Freund  writes:
> > On 2015-08-05 14:05:24 +0200, Andres Freund wrote:
> >> So unless somebody protests I'm going to prepare (and commit after
> >> posting) a patch to rip out the bits of code that currently depend on
> >> PG_USE_INLINE.
> 
> > Here's that patch. I only removed code dependant on PG_USE_INLINE. We
> > might later want to change some of the harder to maintain macros to
> > inline functions, but that seems better done separately.
> 
> Hmm.  I notice that this removes Noah's hack from commit c53f73879f552a3c.
> Do we care about breaking old versions of xlc, and if so, how are we going
> to fix that?  (I assume it should be possible to override AC_C_INLINE's
> result, but I'm not sure where would be a good place to do so.)

Hm. That's a good point.

How about moving that error check into into the aix template file and
erroring out there? Since this is master I think it's perfectly fine to
refuse to work with the buggy unsupported 32 bit compiler. The argument
not to do so was that PG previously worked in the back branches
depending on the minor version, but that's not an argument on master.


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

Re: [HACKERS] Raising our compiler requirements for 9.6

Andres Freund  writes:
> On 2015-08-05 14:05:24 +0200, Andres Freund wrote:
>> So unless somebody protests I'm going to prepare (and commit after
>> posting) a patch to rip out the bits of code that currently depend on
>> PG_USE_INLINE.

> Here's that patch. I only removed code dependant on PG_USE_INLINE. We
> might later want to change some of the harder to maintain macros to
> inline functions, but that seems better done separately.

Hmm.  I notice that this removes Noah's hack from commit c53f73879f552a3c.
Do we care about breaking old versions of xlc, and if so, how are we going
to fix that?  (I assume it should be possible to override AC_C_INLINE's
result, but I'm not sure where would be a good place to do so.)

regards, tom lane

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

Re: [HACKERS] Raising our compiler requirements for 9.6

On 2015-08-05 15:08:29 +0200, Andres Freund wrote:
> We might later want to change some of the harder to maintain macros to
> inline functions, but that seems better done separately.

Here's a conversion for fastgetattr() and heap_getattr(). Not only is
the resulting code significantly more readable, but the conversion also
shrinks the code size:

$ ls -l src/backend/postgres_stock src/backend/postgres
-rwxr-xr-x 1 andres andres 37054832 Aug  5 15:18 src/backend/postgres_stock
-rwxr-xr-x 1 andres andres 37209288 Aug  5 15:23 src/backend/postgres

$ size src/backend/postgres_stock src/backend/postgres
   textdata bss dec hex filename
7026843   49982  298584 7375409  708a31 src/backend/postgres_stock
7023851   49982  298584 7372417  707e81 src/backend/postgres

stock is the binary compiled without the conversion.

A lot of the size difference is debugging information (which now needs
less duplicated information on each callsite), but you can see that the
text section (the actual executable code) shrank by 3k.

stripping the binary shows exactly that:
-rwxr-xr-x 1 andres andres 7076760 Aug  5 15:44 src/backend/postgres_s
-rwxr-xr-x 1 andres andres 7079512 Aug  5 15:45 src/backend/postgres_stock_s

To be sure this doesn't cause problems I ran a readonly pgbench. There's
a very slight, but reproducible, performance benefit. I don't think
that's a reason for the change, I just wanted to make sure there's no
regressions.

Regards,

Andres

>From cca830c39a6c46caf0c68b340399cf60f04ad31f Mon Sep 17 00:00:00 2001
From: Andres Freund 
Date: Wed, 5 Aug 2015 15:30:20 +0200
Subject: [PATCH] Convert fastgetattr() and heap_getattr() into inline
 functions.

The current macro is very hard to read. Now that we can unconditionally
use inline functions there's no reason to keep them that way.

In my gcc 5 based environment this shaves of nearly 200k of the final
binary size. A lot of that is debugging information, but the .text
section alone shrinks by 3k.
---
 src/backend/access/heap/heapam.c  |  46 ---
 src/include/access/htup_details.h | 157 ++
 2 files changed, 75 insertions(+), 128 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 3701d8e..ff93b3c 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -983,52 +983,6 @@ heapgettup_pagemode(HeapScanDesc scan,
 }
 
 
-#if defined(DISABLE_COMPLEX_MACRO)
-/*
- * This is formatted so oddly so that the correspondence to the macro
- * definition in access/htup_details.h is maintained.
- */
-Datum
-fastgetattr(HeapTuple tup, int attnum, TupleDesc tupleDesc,
-			bool *isnull)
-{
-	return (
-			(attnum) > 0 ?
-			(
-			 (*(isnull) = false),
-			 HeapTupleNoNulls(tup) ?
-			 (
-			  (tupleDesc)->attrs[(attnum) - 1]->attcacheoff >= 0 ?
-			  (
-			   fetchatt((tupleDesc)->attrs[(attnum) - 1],
-		(char *) (tup)->t_data + (tup)->t_data->t_hoff +
-		(tupleDesc)->attrs[(attnum) - 1]->attcacheoff)
-			   )
-			  :
-			  nocachegetattr((tup), (attnum), (tupleDesc))
-			  )
-			 :
-			 (
-			  att_isnull((attnum) - 1, (tup)->t_data->t_bits) ?
-			  (
-			   (*(isnull) = true),
-			   (Datum) NULL
-			   )
-			  :
-			  (
-			   nocachegetattr((tup), (attnum), (tupleDesc))
-			   )
-			  )
-			 )
-			:
-			(
-			 (Datum) NULL
-			 )
-		);
-}
-#endif   /* defined(DISABLE_COMPLEX_MACRO) */
-
-
 /* 
  *	 heap access method interface
  * 
diff --git a/src/include/access/htup_details.h b/src/include/access/htup_details.h
index 8dd530bd..9d49c5e 100644
--- a/src/include/access/htup_details.h
+++ b/src/include/access/htup_details.h
@@ -673,6 +673,38 @@ struct MinimalTupleData
 #define HeapTupleSetOid(tuple, oid) \
 		HeapTupleHeaderSetOid((tuple)->t_data, (oid))
 
+/* prototypes for functions in common/heaptuple.c */
+extern Size heap_compute_data_size(TupleDesc tupleDesc,
+	   Datum *values, bool *isnull);
+extern void heap_fill_tuple(TupleDesc tupleDesc,
+Datum *values, bool *isnull,
+char *data, Size data_size,
+uint16 *infomask, bits8 *bit);
+extern bool heap_attisnull(HeapTuple tup, int attnum);
+extern Datum nocachegetattr(HeapTuple tup, int attnum,
+			   TupleDesc att);
+extern Datum heap_getsysattr(HeapTuple tup, int attnum, TupleDesc tupleDesc,
+bool *isnull);
+extern HeapTuple heap_copytuple(HeapTuple tuple);
+extern void heap_copytuple_with_tuple(HeapTuple src, HeapTuple dest);
+extern Datum heap_copy_tuple_as_datum(HeapTuple tuple, TupleDesc tupleDesc);
+extern HeapTuple heap_form_tuple(TupleDesc tupleDescriptor,
+Datum *values, bool *isnull);
+extern HeapTuple heap_modify_tuple(HeapTuple tuple,
+  TupleDesc tupleDesc,
+  Datum *replValues,
+  bool *replIsnull,
+  bool *doReplace);
+extern void heap_deform_tuple(HeapTuple tuple, TupleDesc tupleDesc,
+  Datum *valu

Re: [HACKERS] Reduce ProcArrayLock contention

On Wed, Aug 5, 2015 at 8:30 AM, Pavan Deolasee  wrote:
> I actually even thought if we can define some macros or functions to work on
> atomic list of PGPROCs. What we need is:
>
> - Atomic operation to add a PGPROC to list list and return the head of the
> list at the time of addition
> - Atomic operation to delink a list from the head and return the head of the
> list at that time
> - Atomic operation to remove a PGPROC from the list and return next element
> in the list
> - An iterator to work on the list.

The third operation is unsafe because of the A-B-A problem.  That's
why the patch clears the whole list instead of popping an individual
entry.

> This will avoid code duplication when this infrastructure is used at other
> places. Any mistake in the sequence of read/write/cas can lead to a hard to
> find bug.
>
> Having said that, it might be ok if we go with the current approach and then
> revisit this if and when other parts require similar logic.

Yeah, I don't think we should assume this will be a generic facility.
We can make it one later if needed.

-- 
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] Raising our compiler requirements for 9.6

On 2015-08-05 14:05:24 +0200, Andres Freund wrote:
> So unless somebody protests I'm going to prepare (and commit after
> posting) a patch to rip out the bits of code that currently depend on
> PG_USE_INLINE.

Here's that patch. I only removed code dependant on PG_USE_INLINE. We
might later want to change some of the harder to maintain macros to
inline functions, but that seems better done separately.

Regards,

Andres
>From 72025848dc6de01c5ce014a4fde12d7a8610252d Mon Sep 17 00:00:00 2001
From: Andres Freund 
Date: Wed, 5 Aug 2015 15:02:56 +0200
Subject: [PATCH] Rely on inline functions even if that causes warnings in
 older compilers.

So far we have worked around the fact that some very old compilers do
not support 'inline' functions by only using inline functions
conditionally (or not at all). Since such compilers are very rare by
now, we have decided to rely on inline functions from 9.6 onwards.

To avoid breaking these old compilers inline is defined away when not
supported. That'll cause "function x defined but not used" type of
warnings, but since nobody develops on such compilers anymore that's
ok.

This change in policy will allow us to more easily employ inline
functions.

I chose to remove code previously conditional on PG_USE_INLINE as it
seemed confusing to have code dependent on a define that's always
defined.

Discussion: 20150701161447.gb30...@awork2.anarazel.de
---
 config/c-compiler.m4  |  34 --
 config/test_quiet_include.h   |  18 -
 configure |  38 ---
 configure.in  |   2 +-
 src/backend/lib/ilist.c   |   3 -
 src/backend/nodes/list.c  |   3 -
 src/backend/port/atomics.c|   7 --
 src/backend/utils/adt/arrayfuncs.c|   3 -
 src/backend/utils/mmgr/mcxt.c |   3 -
 src/backend/utils/sort/sortsupport.c  |   3 -
 src/include/access/gin_private.h  |   8 +--
 src/include/c.h   |  28 
 src/include/lib/ilist.h   | 106 --
 src/include/nodes/pg_list.h   |  17 ++---
 src/include/pg_config.h.in|   4 --
 src/include/pg_config.h.win32 |   6 +-
 src/include/port/atomics.h| 101 
 src/include/port/atomics/arch-x86.h   |   4 --
 src/include/port/atomics/fallback.h   |   5 --
 src/include/port/atomics/generic-acc.h|   8 +--
 src/include/port/atomics/generic-gcc.h|   8 +--
 src/include/port/atomics/generic-msvc.h   |   8 ---
 src/include/port/atomics/generic-sunpro.h |   4 --
 src/include/port/atomics/generic-xlc.h|   8 ---
 src/include/port/atomics/generic.h|   4 --
 src/include/utils/arrayaccess.h   |  19 +-
 src/include/utils/palloc.h|  11 +---
 src/include/utils/sortsupport.h   |  18 +
 28 files changed, 70 insertions(+), 411 deletions(-)
 delete mode 100644 config/test_quiet_include.h

diff --git a/config/c-compiler.m4 b/config/c-compiler.m4
index 050bfa5..397e1b0 100644
--- a/config/c-compiler.m4
+++ b/config/c-compiler.m4
@@ -17,40 +17,6 @@ fi])# PGAC_C_SIGNED
 
 
 
-# PGAC_C_INLINE
-# -
-# Check if the C compiler understands inline functions without being
-# noisy about unused static inline functions. Some older compilers
-# understand inline functions (as tested by AC_C_INLINE) but warn about
-# them if they aren't used in a translation unit.
-#
-# This test used to just define an inline function, but some compilers
-# (notably clang) got too smart and now warn about unused static
-# inline functions when defined inside a .c file, but not when defined
-# in an included header. Since the latter is what we want to use, test
-# to see if the warning appears when the function is in a header file.
-# Not pretty, but it works.
-#
-# Defines: inline, PG_USE_INLINE
-AC_DEFUN([PGAC_C_INLINE],
-[AC_C_INLINE
-AC_CACHE_CHECK([for quiet inline (no complaint if unreferenced)], pgac_cv_c_inline_quietly,
-  [pgac_cv_c_inline_quietly=no
-  if test "$ac_cv_c_inline" != no; then
-pgac_c_inline_save_werror=$ac_c_werror_flag
-ac_c_werror_flag=yes
-AC_LINK_IFELSE([AC_LANG_PROGRAM([#include "$srcdir/config/test_quiet_include.h"],[])],
-   [pgac_cv_c_inline_quietly=yes])
-ac_c_werror_flag=$pgac_c_inline_save_werror
-  fi])
-if test "$pgac_cv_c_inline_quietly" != no; then
-  AC_DEFINE_UNQUOTED([PG_USE_INLINE], 1,
-[Define to 1 if "static inline" works without unwanted warnings from ]
-[compilations where static inline functions are defined but not called.])
-fi
-])# PGAC_C_INLINE
-
-
 # PGAC_C_PRINTF_ARCHETYPE
 # ---
 # Set the format archetype used by gcc to check printf type functions.  We
diff --git a/config/test_quiet_include.h b/config/test_quiet_include.h
deleted file mode 100644
index 732b231..000
--- a/config/test_quiet_include.

Re: [HACKERS] WIP: Make timestamptz_out less slow.

2015-08-05 Thread David Rowley

On 5 August 2015 at 12:51, David Rowley 
wrote:

> On 29 July 2015 at 03:25, Andres Freund  wrote:
>
>> On 2015-07-29 03:10:41 +1200, David Rowley wrote:
>> > timestamp_out() = 2015-07-29 02:24:33.34 in 3.506000
>> > timestamp_out_old() = 2015-07-29 02:24:33.034 in 64.518000
>> > timestamp_out_af() = 2015-07-29 02:24:33.034 in 2.981000
>> >
>> > timestamp_out_old is master's version, the timestamp_out_af() is yours,
>> and
>> > timestamp_out() is my one. times are in seconds to perform 100 million
>> > calls.
>>
>> That looks good.
>>
>> > So it appears your version is a bit faster than mine, but we're both
>> about
>> > 20 times faster than the current one.
>> > Also mine needs fixed up as the fractional part is not padded the same
>> as
>> > yours, but I doubt that'll affect the performance by much.
>>
>> Worthwhile to finish that bit and try ;)
>>
>> > My view: It's probably not worth going quite as far as you've gone for a
>> > handful of nanoseconds per call, but perhaps something along the lines
>> of
>> > mine can be fixed up.
>>
>> Yes, I agreee that your's is probably going to be fast enough.
>>
>> > Have you thought about what to do when HAVE_INT64_TIMESTAMP is not
>> defined?
>>
>> I don't think it's actually important. The only difference vs float
>> timestamps is that in the latter case we set fsecs to zero BC.
>>
>> Unless we want to slow down the common case it seems not unlikely that
>> we're going to end up with a separate slow path anyway. E.g. neither
>> your version nor mine handles 5 digit years (which is why I fell back to
>> the slow path in that case in my patch).
>>
>
> It occurred to me that handling the 5 digit year is quite a simple change
> to my code:
>
> We simply just need to check if there was any 'num' left after consuming
> the given space. If there's any left then just use pg_uint2str().
> This keeps things very fast for the likely most common case where the year
> is 4 digits long.
>
> I've not thought about negative years. The whole function should perhaps
> take signed ints instead of unsigned.
>
>
I've made a few changes to this to get the fractional seconds part working
as it should.

It also now works fine with 5 digit years.

It's still in the form of the test program, but it should be simple enough
to pull out what's required from that and put into Postgres.

I've also changed my version of AppendSeconds() so that it returns a
pointer to the new end of string. This should be useful as I see some
strlen() calls to get the new end of string. It'll easy to remove those now
which will further increase performance.

timestamp_out() is the proposed new version
timestamp_out_old() is master's version
timestamp_out_af() is your version

Performance is as follows:

With Clang
david@ubuntu:~/C$ clang timestamp_out.c -o timestamp_out -O2
david@ubuntu:~/C$ ./timestamp_out
timestamp_out() = 2015-07-29 02:24:33.034 in 0.313686
timestamp_out_old() = 2015-07-29 02:24:33.034 in 5.048472
timestamp_out_af() = 2015-07-29 02:24:33.034 in 0.198147

With gcc
david@ubuntu:~/C$ gcc timestamp_out.c -o timestamp_out -O2
david@ubuntu:~/C$ ./timestamp_out
timestamp_out() = 2015-07-29 02:24:33.034 in 0.405795
timestamp_out_old() = 2015-07-29 02:24:33.034 in 4.678918
timestamp_out_af() = 2015-07-29 02:24:33.034 in 0.270557

Regards

David Rowley
--
 David Rowley   http://www.2ndQuadrant.com/

 PostgreSQL Development, 24x7 Support, Training & Services
#include 
#include 
#include 
#include 

struct pg_tm
{
	int			tm_sec;
	int			tm_min;
	int			tm_hour;
	int			tm_mday;
	int			tm_mon;			/* origin 0, not 1 */
	int			tm_year;		/* relative to 1900 */
	int			tm_wday;
	int			tm_yday;
	int			tm_isdst;
	long int	tm_gmtoff;
	const char *tm_zone;
};

static char *pg_uint2str(char *str, unsigned int value);
static char *pg_uint2str_padding(char *str, unsigned int value, unsigned int padding);

static char *
AppendSeconds2(char *cp, int sec, unsigned int fsec, int precision, char fillzeros)
{
	
	if (fillzeros)
		cp = pg_uint2str_padding(cp, abs(sec), 2);
	else
		cp = pg_uint2str(cp, abs(sec));
	
	if (fsec != 0)
	{
		unsigned int value = fsec;
		char *end = &cp[precision + 1];
		int gotnonzero = 0;

		*cp++ = '.';
	
		/*
		 * Append the fractional seconds part. Note that we don't want any
		 * trailing zeros here, so since we're building the number in reverse
		 * we'll skip appending any zeros, unless we've seen a non-zero.
		 */
		while (precision--)
		{
			int		remainder;
			int		oldval = value;

			value /= 10;
			remainder = oldval - value * 10;
			
			/* check if we got a non-zero */
			if (remainder)
gotnonzero = 1;
		
			if (gotnonzero)
cp[precision] = '0' + remainder;
			else
end = &cp[precision];
		}
		
		/*
		 * if we have a non-zero value then precision must have not been enough
		 * to print the number, we'd better have another go. There won't be any
		 * zero padding, so we can just use pg_uint2str()
		 */
		if (value > 0)
			return pg_uin

Re: [HACKERS] Doubt about AccessExclusiveLock in ALTER TABLE .. SET ( .. );

On Tue, Aug 4, 2015 at 1:15 PM, Alvaro Herrera  wrote:
> That opens up for lock escalation and deadlocks, doesn't it?  You are
> probably thinking that it's okay to ignore those but I don't necessarily
> agree with that.

Agreed.  I think we're making a mountain out of a molehill here.  As
long as the locks that are actually used are monotonic, just use > and
stick a comment in there explaining that it could need adjustment if
we use other lock levels in the future.  I presume all the lock-levels
used for DDL are, and will always be, self-exclusive, so why all this
hand-wringing?

-- 
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] Reduce ProcArrayLock contention

2015-08-05 Thread Pavan Deolasee

On Tue, Aug 4, 2015 at 8:59 PM, Robert Haas  wrote:

>
>
> I spent some time looking at this patch today and found that a good
> deal of cleanup work seemed to be needed.  Attached is a cleaned-up
> version which makes a number of changes:
>
>
> I'm not entirely happy with the name "nextClearXidElem" but apart from
> that I'm fairly happy with this version.

Can we just call it "nextAtomicListElem" and the one in PROC_HDR
"headAtomicList" or something like that? Basically we can use the same list
later at other places requiring similar treatment. I don't see them
anything specific to clearXid stuff. Rather it is just some sort of atomic
list of PGPROC

I actually even thought if we can define some macros or functions to work
on atomic list of PGPROCs. What we need is:

- Atomic operation to add a PGPROC to list list and return the head of the
list at the time of addition
- Atomic operation to delink a list from the head and return the head of
the list at that time
- Atomic operation to remove a PGPROC from the list and return next element
in the list
- An iterator to work on the list.

This will avoid code duplication when this infrastructure is used at other
places. Any mistake in the sequence of read/write/cas can lead to a hard to
find bug.

Having said that, it might be ok if we go with the current approach and
then revisit this if and when other parts require similar logic.

Thanks,
Pavan

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

Re: [HACKERS] RFC: replace pg_stat_activity.waiting with something more descriptive

On Tue, Aug 4, 2015 at 8:46 PM, Josh Berkus  wrote:
> On 06/25/2015 07:50 AM, Tom Lane wrote:
>> To do that, we'd have to change the semantics of the 'waiting' column so
>> that it becomes true for non-heavyweight-lock waits.  I'm not sure whether
>> that's a good idea or not; I'm afraid there may be client-side code that
>> expects 'waiting' to indicate that there's a corresponding row in
>> pg_locks.  If we're willing to do that, then I'd be okay with
>> allowing wait_status to be defined as "last thing waited for"; but the
>> two points aren't separable.
>
> Speaking as someone who writes a lot of monitoring and alerting code,
> changing the meaning of the waiting column is OK as long as there's
> still a boolean column named "waiting" and it means "query blocked" in
> some way.
>
> Users are used to pg_stat_activity.waiting failing to join against
> pg_locks ... for one thing, there's timing issues there.  So pretty much
> everything I've seen uses outer joins anyway.

All of that is exactly how I feel about it, too.  It's not totally
painless to redefine waiting, but we're not proposing a *big* change
in semantics.  The way I see it, if we change this now, some people
will need to adjust, but it won't really be a big deal.  If we insist
that "waiting" is graven in stone, then in 5 years people will still
be wondering why the "waiting" column is inconsistent with the
"wait_state" column.  That's not going to be a win.

-- 
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] Raising our compiler requirements for 9.6

On 2015-08-04 16:55:12 -0400, Robert Haas wrote:
> On Tue, Aug 4, 2015 at 3:55 PM, Andres Freund  wrote:
> > On 2015-08-04 15:45:44 -0400, Tom Lane wrote:
> >> I'm not sure that there's any great urgency about changing the instances
> >> that exist now; the real point of this discussion is that we will allow
> >> new code to use static inlines in headers.
> >
> > I agree that we don't have to (and probably shouldn't) make the required
> > configure changes and change definitions.  But I do think some of the
> > current macro usage deserves to be cleaned up at some point. I,
> > somewhere during 9.4 or 9.5, redefined some of the larger macros into
> > static inlines and it both reduced the binary size and gave minor
> > performance benefits.
> 
> We typically recommend that people write their new code like the
> existing code.  If we say that the standards for new code are now
> different from old code in this one way, I don't think that's going to
> be very helpful to anyone.

I'm coming around to actually changing more code initially. While I
don't particularly buy the severity of the "make it look like existing
code" issue, I do think it'll be rather confusing to have code dependent
on PG_USE_INLINE when it's statically defined.

So unless somebody protests I'm going to prepare (and commit after
posting) a patch to rip out the bits of code that currently depend on
PG_USE_INLINE.

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] more-helpful-izing a debug message

On 2015-08-04 16:38:58 -0400, Robert Haas wrote:
> On Wed, Jul 8, 2015 at 5:38 AM, Marko Tiikkaja  wrote:
> > One of the debug messages related to logical replication could be more
> > helpful than it currently is.  The attached patch reorders the two
> > operations to make it so.
> >
> > Please consider patching and back-patching.
> 
> Andres, this looks like a bug fix to me.  What do you think?

Yes, definitely. Sorry for letting this fall by the wayside. Pushed.

Regards,

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] Reduce ProcArrayLock contention