Re: [HACKERS] Triggers on foreign tables

2013-10-16 Thread Ronan Dunklau
Le mardi 15 octobre 2013 09:47:31 Robert Haas a écrit :
 On Mon, Oct 14, 2013 at 5:24 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
  And, I also want some comments from committers, not only from mine.
  
  +1
  
  +1
 
 /me pokes head up.  I know I'm going to annoy people with this
 comment, but I feel like it's going to have to be made at some point
 by somebody, so here goes: I don't see the point of this feature.  If
 you want a trigger on a table, why not set it on the remote side?  A
 trigger on the foreign table won't be enforced consistently; it'll
 only work when the update is routed through the foreign table, not
 when people access the underlying table on the remote side through any
 other mechanism.  The number of useful things you can do this way
 seems fairly small.  Perhaps you could use a row-level trigger for
 RLS, to allow only certain rows on the foreign side to be updated, but
 even that seems like a slightly strange design: generally it'd be
 better to enforce the security as close to the target object as
 possible.
 
 There's another issue that concerns me here also: performance.  IIUC,
 an update of N tuples on the remote side currently requires N+1 server
 round-trips.  That is unspeakably awful, and we really oughta be
 looking for ways to make that number go down, by pushing the whole
 update to the remote side.  But obviously that won't be possible if
 there's a per-row trigger that has to be evaluated on the local side.
 Now, assuming somebody comes up with code that implements that
 optimization, we can just disable it when there are local-side
 triggers.  But, then you're back to having terrible performance.  So
 even if the use case for this seemed really broad, I tend to think
 performance concerns would sink most of the possible real-world uses.
 
 I could, of course, be all wet

Even if the row-level trigger functionality is controversial, in terms of 
provided features VS performance, the original patch I submitted enables 
statement-level triggers. 

Although my goal was to implement row-level triggers and I hit a wall pretty 
quickly, would it make sense to have statement-level triggers without their 
row counterparts ?

I also think that pushing the whole update to the remote side will not be 
possible in all cases, and like Kohei wrote, it may be an acceptable loss to 
not be able to push it when a trigger is present. If we want to push the whole 
update, we will have to cope with local joins and function evaluations in all 
cases. IMO, triggers are just a special case of these limiting factors.

signature.asc
Description: This is a digitally signed message part.


Re: [HACKERS] Triggers on foreign tables

2013-10-16 Thread Ronan Dunklau

 Sorry, I might call it something like primary key, instead of 'tupleid'.
 Apart from whether we can uniquely identify a particular remote record with
 an attribute, what FDW needs here is something to identify a remote
 record. So, we were talking about same concept with different names.

Ah, that makes sense: I was understanding tupleid as a synonym for ctid.


  Does the incomplete tuple mean a tuple image but some of columns
  are replaced with NULL due to optimization, as postgres_fdw is doing,
  doesn't it?
  If so, a solution is to enforce FDW driver to fetch all the columns if
  its
  managing foreign table has row level triggers for update / delete.
  
  What I meant is that a DELETE statement, for example, does not build a
  scan- stage reltargetlist consisting of the whole set of attributes: the
  only attributes that are fetched are the ones built by
  addForeignUpdateTargets, and whatever additional attributes are involved
  in the DELETE statement (in the WHERE clause, for example).
 
 DELETE statement indeed does not (need to) construct a complete tuple
 image on scan stage, however, it does not prohibit FDW driver to keep the
 latest record being fetched from remote server.
 FDW driver easily knows whether relation has row-level trigger preliminary,
 so here is no matter to keep a complete image internally if needed.
 Or, it is perhaps available to add additional target-entries that is
 needed to construct a complete tuple using AddForeignUpdateTargets()
 callback.

I think that the additional target-entries should be added in core, because 
that would require additional work from FDW drivers, and the code would be 
duplicated amongst all FDW drivers that wish to support triggers.


  I like this idea, but I don't really see all the implications of such a
  design.
  Where would this tuple be stored ?
  From what I understand, after-triggers are queued for later execution,
  using the old/new tupleid for later retrieval (in the
  AfterTriggerEventData struct). This would need a major change to work
  with foreign tables. Would that involve a special path for executing
  those triggers ASAP ?
 
 Ops, I oversight here. AfterTriggerSaveEvent() indeed saves only ctid of
 tuples in regular tables, because it can re-construct a complete tuple
 image from a particular ctid any time.
 On the other hand, it is not available on foreign table due to its nature.
 All we can do seems to me, probably, to save copy of before/after tuple
 image on local memory, even though it consumes much memory than
 regular tables.

Or, as suggested above, add a special code path for foreign tables which would 
execute the trigger as soon as possible instead of queuing it.  

 The feature we need here might become a bit clear little by little.
 A complete tuple image shall be fetched on the scan stage, if foreign
 table has row-level trigger. The planSlot may make sense if it contains
 (junk) attributes that is sufficient to re-construct an old tuple image.
 Target-list of DELETE statement contains a junk ctid attribute. Here
 is no reason why we cannot add junk attributes that reference each
 regular attribute, isn't it? Also, target-list of UPDATE statement
 contains new tuple image, however, it is available to add junk attributes
 that just reference old value, as a carrier of old values from scan stage
 to modify stage.
 I want core PostgreSQL to support a part of these jobs. For example,
 it may be able to add junk attribute to re-construct old tuple image on
 rewriteTargetListUD(), if target relation has row-level triggers. Also, it
 may be able to extract these old values from planSlot to construct
 an old tuple image on GetTupleForTrigger(), if relation is foreign table.

So, if I understand you, the target list would be built as follow:


 - core builds the target list, including every attribute
 - this target list is updated by the FDW to add any junk attributes it wishes 
to use 
 - in the case of an UPDATE, core duplicates the whole list of attributes 
(including the added junk attributes), as another set of junk attributes. 
Maybe we could duplicate only the updated attributes.
 


 Unfortunately, I have no special idea to handle old/new remote tuple
 on AfterTriggerSaveEvent(), except for keeping it as copy, but it is
 straightforward.

Maybe avoid it altogether in the case of a trigger on a remote foreign table ?

 
  And, I also want some comments from committers, not only from mine.
  
  +1
 
 +1
 
 Thanks,

signature.asc
Description: This is a digitally signed message part.


Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-16 Thread Fabien COELHO


Hello Vik,


Yes, I understand you are trying to help, and I appreciate it!  My
opinion, and that of others as well from the original thread, is that
this patch should either go in as is and break that one case, or not go
in at all.  I'm fine with either (although clearly I would prefer it
went in otherwise I wouldn't have written the patch).


I see this is marked as rejected in the commitfest app, but I don't see
any note about who did it or why.  I don't believe there is consensus
for rejection on this list. In fact I think the opposite is true.

May we have an explanation please from the person who rejected this
without comment?


I did it, on the basis that you stated that you prefered not adding 
pg_sleep(TEXT) to answer Robert Haas concern about preserving 
pg_sleep('10') current functionality, and that no other solution was 
suggested to tackle this issue.


If I'm mistaken, feel free to change the state back to what is 
appropriate.


My actual opinion is that breaking pg_sleep('10') is no big deal, but I'm 
nobody here, and Robert is somebody, so I think that his concern must be 
addressed.


--
Fabien.


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


Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-16 Thread Vik Fearing
On 10/16/2013 10:57 AM, Fabien COELHO wrote:

 Hello Vik,

 I see this is marked as rejected in the commitfest app, but I don't see
 any note about who did it or why.  I don't believe there is consensus
 for rejection on this list. In fact I think the opposite is true.

 May we have an explanation please from the person who rejected this
 without comment?

 I did it, on the basis that you stated that you prefered not adding
 pg_sleep(TEXT) to answer Robert Haas concern about preserving
 pg_sleep('10') current functionality, and that no other solution was
 suggested to tackle this issue.

The suggested solution is to ignore the issue.

 If I'm mistaken, feel free to change the state back to what is
 appropriate.

I'm not really sure what the proper workflow is for marking a patch as
rejected, actually.  I wouldn't mind some clarification on this from the
CFM or somebody.

In the meantime I've set it to Ready for Committer because in my mind
there is a consensus for it (see below) and you don't appear to have
anything more to say about the patch except for the do-we/don't-we issue.

 My actual opinion is that breaking pg_sleep('10') is no big deal, but
 I'm nobody here, and Robert is somebody, so I think that his concern
 must be addressed.

Tom Lane is somebody, too, and his opinion is to break it or reject it
although he refrains from picking a side[1].  Josh Berkus and Stephen
Frost are both somebodies and they are on the break it side[2][3]. 
Peter Eisentraut gave no opinion at all but did say that Robert's
argument was not very good.  I am for it because I wrote the patch, and
you seem not to care.  So the way I see it we have:

For: Josh, Stephen, me
Against: Robert
Neutral: Tom, you

I don't know if that's enough of a consensus to commit it, but I do
think it's not nearly enough of a consensus to reject it.

[1] http://www.postgresql.org/message-id/16727.1376697147%40sss.pgh.pa.us
[2] http://www.postgresql.org/message-id/520ec584.3050...@agliodbs.com
[3]
http://www.postgresql.org/message-id/20130820013033.gu2...@tamriel.snowman.net

-- 
Vik



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


Re: [HACKERS] Standby catch up state change

2013-10-16 Thread Andres Freund
On 2013-10-16 11:03:12 +0530, Pavan Deolasee wrote:
 I think you are right. Someone who understands the replication code very
 well advised us to use that log message as a way to measure how much time
 it takes to send all the missing WAL to a remote standby on a slow WAN
 link. While it worked well for all measurements, when we use a middleware
 which caches a lot of traffic on the sender side, this log message was very
 counter intuitive. It took several more minutes for the standby to actually
 receive all the WAL files and catch up after the message was displayed on
 the master side. But then as you said, may be relying on the message was
 not the best way to measure the time.

Query pg_stat_replication instead, that has the flush position.

Greetings,

Andres Freund

-- 
 Andres Freund 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] Standby catch up state change

2013-10-16 Thread Pavan Deolasee


 On 16-Oct-2013, at 3:45 pm, Andres Freund and...@2ndquadrant.com wrote:
 
 On 2013-10-16 11:03:12 +0530, Pavan Deolasee wrote:
 I think you are right. Someone who understands the replication code very
 well advised us to use that log message as a way to measure how much time
 it takes to send all the missing WAL to a remote standby on a slow WAN
 link. While it worked well for all measurements, when we use a middleware
 which caches a lot of traffic on the sender side, this log message was very
 counter intuitive. It took several more minutes for the standby to actually
 receive all the WAL files and catch up after the message was displayed on
 the master side. But then as you said, may be relying on the message was
 not the best way to measure the time.
 
 Query pg_stat_replication instead, that has the flush position.
 

Yeah, that's what we are doing now.

Thanks,
Pavan

 Greetings,
 
 Andres Freund
 
 -- 
 Andres Freund   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] Auto-tuning work_mem and maintenance_work_mem

2013-10-16 Thread MauMau

From: Andres Freund and...@2ndquadrant.com

I've seen several sites shutting down because of forgotten prepared
transactions causing bloat and anti-wraparound shutdowns.


From: Magnus Hagander mag...@hagander.net

I would say *using* an external transaction manager *is* the irregular
thing. The current default *is* friendly for normal users, for example
see the comments from Andres about what happens if you make a mistake.
So I definitely agree with your sentiment that we should be more
friendly for normal users - but in this case we are.

If I look through all the customers I've worked with, only a handful
have actually used a transaction manager. And of those, at least half
of them were using it even though they didn't need it, because they
didn't know what it was.


I understand that you mean by *irregular* that there are few people who use 
distributed transactions.  I guess so too: there are not many users who 
require distributed transactions in the real world.  I meant by *irregular* 
that almost all users should use distributed transactions through an 
external transaction manager incorporated in Java EE application servers or 
MSDTC.


The distributed transaction features like XA and Java Transaction API (JTA) 
are established.  They are not irregular for those who need them; they were 
developed and exist for a long time, because they were/are needed.


I don't think the default value of zero for max_prepared_transactions is 
friendly for normal and not-normal users.  Normal users, who properly use 
external transaction manager, won't be caught by the trouble Andres 
mentioned, because the external transaction manager soon resolves prepared 
(in-doubt) transactions.


Not-normal users, who uses PREPARE TRANSACTION statement or 
XAResource.prepare() directly from their applications without using external 
transaction manager or without need based on proper understanding, won't 
escape from Andres's concern.  They will see the following message and 
follow it blindly to make their applications succeed.


ERROR:  prepared transactions are disabled
HINT:  Set max_prepared_transactions to a nonzero value.

So, the current default of zero is not only unfriendly for normal users but 
also non-helpful for those who make mistakes.


Regards
MauMau



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


[HACKERS] ERROR : 'tuple concurrently updated'

2013-10-16 Thread Stéphan BEUZE
The following query is performed concurrently by two threads logged in 
with two different users:


WITH raw_stat AS (
SELECT
   host(client_addr) as client_addr,
   pid ,
   usename
FROM
   pg_stat_activity
WHERE
   usename = current_user
)
INSERT INTO my_stat(id, client_addr, pid, usename)
SELECT
 nextval('mystat_sequence'), t.client_addr, t.pid, t.usename
FROM (
SELECT
client_addr, pid, usename
FROM
raw_stat s
WHERE
NOT EXISTS (
   SELECT
  NULL
   FROM
  my_stat u
   WHERE
  current_date = u.creation
   AND
  s.pid = u.pid
   AND
  s.client_addr = u.client_addr
   AND
  s.usename = u.usename
)
) t;

From time to time, I get the following error: tuple concurrently updated

I can't figure out what throw  this error and why this error is thrown. 
Can you shed a light ?


---
Here is the sql definition of the table mystat.

**mystats.sql**

CREATE TABLE mystat
(
  id bigint NOT NULL,
  creation date NOT NULL DEFAULT current_date,

  client_addr text NOT NULL,
  pid integer NOT NULL,
  usename name NOT NULL,
  CONSTRAINT statistiques_connexions_pkey PRIMARY KEY (id)
)
WITH (
  OIDS=FALSE
);


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


Re: [HACKERS] Triggers on foreign tables

2013-10-16 Thread Robert Haas
On Tue, Oct 15, 2013 at 4:17 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 One reason we should support local triggers is that not all the data
 source of FDW support remote trigger. It required FDW drivers to
 have RDBMS as its backend, but no realistic assumption.
 For example, file_fdw is unavailable to implement remote triggers.

True, but gosh, updates via file_fdw are gonna be so slow I can't
believe anybody'd use it for something real

 One thing I'd like to know is, where is the goal of FDW feature.
 It seems to me, FDW goes into a feature to manage external data
 set as if regular tables. If it is right understanding, things we try to
 support on foreign table is things we're supporting on regular tables,
 such as triggers.

I generally agree with that.

 We often have some case that we cannot apply fully optimized path
 because of some reasons, like view has security-barrier, qualifier
 contained volatile functions, and so on...
 Trigger may be a factor to prevent fully optimized path, however,
 it depends on the situation which one shall be prioritized; performance
 or functionality.

Sure.  I mean, I guess if there are enough people that want this, I
suppose I ought not stand in the way.  It just seems like a lot of
work for a feature of very marginal utility.

-- 
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] buildfarm failures on smew and anole

2013-10-16 Thread Robert Haas
On Tue, Oct 15, 2013 at 11:17 PM, Peter Eisentraut pete...@gmx.net wrote:
 On Mon, 2013-10-14 at 18:14 -0400, Robert Haas wrote:
  I cleaned the semaphores on smew, but they came back.  Whatever is
  crashing is leaving the semaphores lying around.

 Ugh.  When did you do that exactly?  I thought I fixed the problem
 that was causing that days ago, and the last 4 days worth of runs all
 show the too many clients error.

 I did it a few times over the weekend.  At least twice less than 4 days
 ago.  There are currently no semaphores left around, so whatever
 happened in the last run cleaned it up.

That seems to suggest I've introduced some bug.  I'm at a loss as to
what it is, though.  :-(

-- 
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] buildfarm failures on smew and anole

2013-10-16 Thread Andres Freund
On 2013-10-16 08:39:10 -0400, Robert Haas wrote:
 On Tue, Oct 15, 2013 at 11:17 PM, Peter Eisentraut pete...@gmx.net wrote:
  On Mon, 2013-10-14 at 18:14 -0400, Robert Haas wrote:
   I cleaned the semaphores on smew, but they came back.  Whatever is
   crashing is leaving the semaphores lying around.
 
  Ugh.  When did you do that exactly?  I thought I fixed the problem
  that was causing that days ago, and the last 4 days worth of runs all
  show the too many clients error.
 
  I did it a few times over the weekend.  At least twice less than 4 days
  ago.  There are currently no semaphores left around, so whatever
  happened in the last run cleaned it up.
 
 That seems to suggest I've introduced some bug.  I'm at a loss as to
 what it is, though.  :-(

Ah. I see the issue. To reproduce do something like
# mkdir /tmp/empty
# mount --bind /tmp/empty /dev/shm/
and then run initdb.

The issue is that test_config_settings determines max_connections
without disabling dynamic shared memory which consequently chooses posix
which doesn't work. Setting it to none during the test makes it work.

Greetings,

Andres Freund

-- 
 Andres Freund 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] background workers, round three

2013-10-16 Thread Robert Haas
On Tue, Oct 15, 2013 at 4:02 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 Hmm. It probably allows to clean-up smaller fraction of data structure
 constructed on dynamic shared memory segment, if we map / unmap
 for each transactions.

I think the primary use of dynamic shared memory will be for segments
that get created for a single transaction, used, and then destroyed.
Perhaps people will find other uses for them that involve keeping them
mapped for longer periods of time, and that is fine.  But whether or
long or short, it seems clear to me that shared memory data structures
will need cleanup on detach, just as backends must clean up the main
shared memory segment before they detach.

 I assumed smaller chunks allocated on static or dynamic shared
 memory segment to be used for communicate between main
 process and worker processes because of my motivation.
 When we move a chunk of data to co-processor using asynchronous
 DMA transfer, API requires the source buffer is mlock()'ed to avoid
 unintentional swap out during DMA transfer. On the other hand,
 cost of mlock() operation is not ignorable, so it may be a reasonable
 design to lock a shared memory segment on start-up time then
 continue to use it, without unmapping.
 So, I wondered how to handle the situation when extension tries
 to manage a resource with smaller granularity than the one
 managed by PostgreSQL core.

I'm still not sure exactly what your concern is here, but I agree
there are some thorny issues around resource management here.  I've
spent a lot of the last couple months trying to sort through them.  As
it seems to me, the problem is basically that we're introducing major
new types of resources (background workers, dynamic shared memory
segments, and perhaps other things) and they all require management -
just as our existing types of resources (locks, buffer pins, etc.)
require management.  But the code to manage existing resources has
been around for so long that we just rely on it without thinking about
it, so when we suddenly DO need to think about it, it's an unpleasant
surprise.

As far as shared memory resources specifically, one consideration is
that some systems (e.g. some versions of HP-UX) have fairly small
limits ( 15) on the number of shared memory segments that can be
mapped, and all 32-bit systems are prone to running out of usable
address space.  So portable code is going to have to use these
resources judiciously.  On the other hand, I think that people wanting
to write code that only needs to run on 64-bit Linux will be able to
pretty much go nuts.

Unless there are further objections to the terminate patch as written,
I'm going to go ahead and commit that, with the additional of
documentation (as pointed out by Michael) and a change to the lock
mode (as pointed out by KaiGai).  The other patches, at least for the
time being, are withdrawn.

-- 
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] buildfarm failures on smew and anole

2013-10-16 Thread Robert Haas
On Wed, Oct 16, 2013 at 8:54 AM, Andres Freund and...@2ndquadrant.com wrote:
 On 2013-10-16 08:39:10 -0400, Robert Haas wrote:
 On Tue, Oct 15, 2013 at 11:17 PM, Peter Eisentraut pete...@gmx.net wrote:
  On Mon, 2013-10-14 at 18:14 -0400, Robert Haas wrote:
   I cleaned the semaphores on smew, but they came back.  Whatever is
   crashing is leaving the semaphores lying around.
 
  Ugh.  When did you do that exactly?  I thought I fixed the problem
  that was causing that days ago, and the last 4 days worth of runs all
  show the too many clients error.
 
  I did it a few times over the weekend.  At least twice less than 4 days
  ago.  There are currently no semaphores left around, so whatever
  happened in the last run cleaned it up.

 That seems to suggest I've introduced some bug.  I'm at a loss as to
 what it is, though.  :-(

 Ah. I see the issue. To reproduce do something like
 # mkdir /tmp/empty
 # mount --bind /tmp/empty /dev/shm/
 and then run initdb.

 The issue is that test_config_settings determines max_connections
 without disabling dynamic shared memory which consequently chooses posix
 which doesn't work. Setting it to none during the test makes it work.

Gah.  I fixed one instance of that problem in test_config_settings(),
but missed the other.

Thanks.

-- 
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] Compression of full-page-writes

2013-10-16 Thread k...@rice.edu
On Wed, Oct 16, 2013 at 01:42:34PM +0900, KONDO Mitsumasa wrote:
 (2013/10/15 22:01), k...@rice.edu wrote:
 Google's lz4 is also a very nice algorithm with 33% better compression
 performance than snappy and 2X the decompression performance in some
 benchmarks also with a bsd license:
 
 https://code.google.com/p/lz4/
 If we judge only performance, we will select lz4. However, we should think
  another important factor which is software robustness, achievement, bug
 fix history, and etc... If we see unknown bugs, can we fix it or improve
 algorithm? It seems very difficult, because we only use it and don't
 understand algorihtms. Therefore, I think that we had better to select
 robust and having more user software.
 
 Regards,
 --
 Mitsumasa KONDO
 NTT Open Source Software
 
Hi,

Those are all very good points. lz4 however is being used by Hadoop. It
is implemented natively in the Linux 3.11 kernel and the BSD version of
the ZFS filesystem supports the lz4 algorithm for on-the-fly compression.
With more and more CPU cores available in modern system, using an
algorithm with very fast decompression speeds can make storing data, even
in memory, in a compressed form can reduce space requirements in exchange
for a higher CPU cycle cost. The ability to make those sorts of trade-offs
can really benefit from a plug-able compression algorithm interface.

Regards,
Ken


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


Re: [HACKERS] buildfarm failures on smew and anole

2013-10-16 Thread Robert Haas
On Wed, Oct 16, 2013 at 9:37 AM, Andres Freund and...@2ndquadrant.com wrote:
 On 2013-10-16 09:35:46 -0400, Robert Haas wrote:
 Gah.  I fixed one instance of that problem in test_config_settings(),
 but missed the other.

 Maybe it'd be better to default to none, just as max_connections
 defaults to 1 and shared_buffers to 16? As we write out the value in the
 config file, everything should still continue to work.

Hmm, possibly.  But how would we document that?  It seems strange to
say that the default is none, but the actual setting probably won't be
none on your system because we hack up postgresql.conf.
shared_buffers pretty much just glosses over the distinction between
default and what you probably have configured, but I'm not sure
that's actually great policy.

Trivial fixed pushed, 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] buildfarm failures on smew and anole

2013-10-16 Thread Andres Freund
On 2013-10-16 09:44:32 -0400, Robert Haas wrote:
 On Wed, Oct 16, 2013 at 9:37 AM, Andres Freund and...@2ndquadrant.com wrote:
  On 2013-10-16 09:35:46 -0400, Robert Haas wrote:
  Gah.  I fixed one instance of that problem in test_config_settings(),
  but missed the other.
 
  Maybe it'd be better to default to none, just as max_connections
  defaults to 1 and shared_buffers to 16? As we write out the value in the
  config file, everything should still continue to work.
 
 Hmm, possibly.  But how would we document that?  It seems strange to
 say that the default is none, but the actual setting probably won't be
 none on your system because we hack up postgresql.conf.
 shared_buffers pretty much just glosses over the distinction between
 default and what you probably have configured, but I'm not sure
 that's actually great policy.

I can't remember somebody actually being confused by that with s_b or
max_connections. So maybe it's just ok not to document it. But yes, I
can't come up with a succinct description of that behaviour either.

Greetings,

Andres Freund

-- 
 Andres Freund 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] [PATCH] pg_sleep(interval)

2013-10-16 Thread Stephen Frost
* Vik Fearing (vik.fear...@dalibo.com) wrote:
 I don't know if that's enough of a consensus to commit it, but I do
 think it's not nearly enough of a consensus to reject it.

This is actually a problem that I think we have today- the expectation
that *everyone* has to shoot down an idea for it to be rejected, but
one individual saying oh, that's a good idea means it must be
committed.

That's not how it works and there's no notion of pending further
discussion in the CF; imv that equates to returned with feedback.
Marking this patch as 'Ready for Committer' when multiple committers
have already commented on it doesn't strike me as moving things forward
either.

As it relates to this specific patch for this CF, I'd go with 'Returned
with Feedback' and encourage you to consider the arguments for and
against, and perhaps try to find existing usage which would break due to
this change and consider the impact of changing it.  For example, what
do the various languages and DB abstraction layers do today?  Would
users of Hibernate likely be impacted or no?  What about PDO?
Personally, I'm still on-board with the change in general, but it'd
really help to know that normal/obvious usage through various languages
won't be busted by the change.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] [v9.4] row level security

2013-10-16 Thread Kohei KaiGai
Because of CF-2nd end, it was moved to the next commit-fest.

In my personal opinion, it probably needs a few groundwork to get RLS
commitable,
prior to RLS itself.

One is a correct handling towards the scenario that Korotkov pointed
out. (How was
it concluded?) I think it is a problem we can fix with reasonable way.
Likely, solution
is to prohibit to show number of rows being filtered if plan node is
underlying a sub-
query with security-barrier.

One other stuff I'm concerned about is, existing implementation
assumes a certain
rtable entry performs as a source relation, also result relation in same time on
DELETE and UPDATE.
We usually scan a regular relation to fetch ctid of the tuples to be
modified, then
ModifyTable deletes or updates the tuple being identified. Here has
been no matter
for long period, even if same rtable entry is used to point out a
relation to be scanned
and to be modified.
It however makes RLS implementation complicated, because this feature tries to
replace a rtable entry to relation with row-level security policy by a
simple sub-query
with security-barrier attribute. Our implementation does not assume a sub-query
performs as a result relation, so I had to have some ad-hoc coding,
like adjustment
on varno/varattno of Var objects, to avoid problem.
I think, solution is to separate a rtable entry of result-relation
from rtable entry to
be scanned. It makes easier to implement RLS feature because all we need to do
is just replacement of rtable entry for scanning stage, and no need to
care about
ModifyTable operations towards sub-query.

Is it a right direction to go?

Thanks,

2013/10/10 Marc Munro m...@bloodnok.com:
 On Wed, 2013-09-04 at 14:35 +, Robert Haas wrote:

 On Fri, Aug 30, 2013 at 3:43 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  I think it's entirely sensible to question whether we should reject
 (not
  hold up) RLS if it has major covert-channel problems.

 We've already had this argument before, about the security_barrier
 [ . . . ]

 Sorry for following up on this so late, I have just been trying to catch
 up with the mailing lists.

 I am the developer of Veil, which this thread mentioned a number of
 times.  I wanted to state/confirm a number of things:

 Veil is not up to date wrt Postgres versions.  I didn't release a new
 version for 9.2, and when no-one complained I figured no-one other than
 me was using it.  I'll happily update it if anyone wants it.

 Veil makes no attempt to avoid covert channels.  It can't.

 Veil is a low-level toolset designed for optimising queries about
 privileges.  It allows you to build RLS with reasonable performance, but
 it is not in itself a solution for RLS.

 I wish the Postgres RLS project well and look forward to its release in
 Postgres 9.4.

 __
 Marc





-- 
KaiGai Kohei kai...@kaigai.gr.jp


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


Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-16 Thread Andres Freund
On 2013-10-16 11:18:55 -0400, Stephen Frost wrote:
 This is actually a problem that I think we have today- the expectation
 that *everyone* has to shoot down an idea for it to be rejected, but
 one individual saying oh, that's a good idea means it must be
 committed.

But neither does a single objection mean it cannot get committed. I
don't see either scenario being present here though.

Greetings,

Andres Freund

-- 
 Andres Freund 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] [PATCH] pg_sleep(interval)

2013-10-16 Thread Robert Haas
On Wed, Oct 16, 2013 at 11:18 AM, Stephen Frost sfr...@snowman.net wrote:
 * Vik Fearing (vik.fear...@dalibo.com) wrote:
 I don't know if that's enough of a consensus to commit it, but I do
 think it's not nearly enough of a consensus to reject it.

 This is actually a problem that I think we have today- the expectation
 that *everyone* has to shoot down an idea for it to be rejected, but
 one individual saying oh, that's a good idea means it must be
 committed.

 That's not how it works and there's no notion of pending further
 discussion in the CF; imv that equates to returned with feedback.
 Marking this patch as 'Ready for Committer' when multiple committers
 have already commented on it doesn't strike me as moving things forward
 either.

 As it relates to this specific patch for this CF, I'd go with 'Returned
 with Feedback' and encourage you to consider the arguments for and
 against, and perhaps try to find existing usage which would break due to
 this change and consider the impact of changing it.  For example, what
 do the various languages and DB abstraction layers do today?  Would
 users of Hibernate likely be impacted or no?  What about PDO?
 Personally, I'm still on-board with the change in general, but it'd
 really help to know that normal/obvious usage through various languages
 won't be busted by the change.

I generally agree, although I'm not as sanguine as you about the
overall prospects for the patch.  The bottom line is that there are
cases, like pg_sleep('42') that will be broken by this, and if you fix
those by some hack there will be other cases that break instead.
Recall what happened with pg_size_pretty(), which did not turn out
nearly as well as I thought it would, though I advocated for it at the
time.  There's just no such thing as a free lunch: we *can't* change
the API for functions that have been around for many releases without
causing some pain for users that are depending on those functions.

Now, of course, sometimes it's worth it.  We can and should change
things when there's enough benefit to justify the pain that comes from
breaking backward compatibility.  But I don't see that as being the
case here.  Anyone who actually wants this in their environment can
add the overloaded function in their environment with a one-line SQL
function: pg_sleep(interval) as proposed here is just
pg_sleep(extract(epoch from now() + $1) - extract(epoch from now())).
Considering how easy that is, I don't see why we need to have it in
core.

In general, I'm a big fan of composibility as a design principle.  If
you have a function that does A and a function that does B, it's
reasonable to say that people should use them together, rather than
providing a third function that does AB.  Otherwise, you just end up
with too many functions.  Of course, there are exceptions: if A and B
are almost always done together, a convenience function may indeed be
warranted.  But I don't see how you can argue that here; there are
doubtless many existing users of pg_sleep(double) that are perfectly
happy with the existing behavior.

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


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


[HACKERS] [PATCH] hstore_to_json: empty hstores must return empty json objects

2013-10-16 Thread Oskari Saarenmaa
hstore_to_json used to return an empty string for empty hstores, but
that is not correct as an empty string is not valid json and calling
row_to_json() on rows which have empty hstores will generate invalid
json for the entire row.  The right thing to do is to return an empty
json object.

Signed-off-by: Oskari Saarenmaa o...@ohmu.fi
---
This should probably be applied to 9.3 tree as well as master.

# select row_to_json(r.*) from (select ''::hstore as d) r;
 {d:}

# select hstore_to_json('')::text::json;
ERROR:  invalid input syntax for type json

 contrib/hstore/expected/hstore.out | 12 
 contrib/hstore/hstore_io.c |  8 
 contrib/hstore/sql/hstore.sql  |  2 ++
 3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/contrib/hstore/expected/hstore.out 
b/contrib/hstore/expected/hstore.out
index 2114143..3280b5e 100644
--- a/contrib/hstore/expected/hstore.out
+++ b/contrib/hstore/expected/hstore.out
@@ -1472,6 +1472,18 @@ select hstore_to_json_loose('a key =1, b = t, c = 
null, d= 12345, e = 012
  {b: true, c: null, d: 12345, e: 012345, f: 1.234, g: 2.345e+4, 
a key: 1}
 (1 row)
 
+select hstore_to_json('');
+ hstore_to_json 
+
+ {}
+(1 row)
+
+select hstore_to_json_loose('');
+ hstore_to_json_loose 
+--
+ {}
+(1 row)
+
 create table test_json_agg (f1 text, f2 hstore);
 insert into test_json_agg values ('rec1','a key =1, b = t, c = null, d= 
12345, e = 012345, f= 1.234, g= 2.345e+4'),
('rec2','a key =2, b = f, c = null, d= -12345, e = 012345.6, 
f= -1.234, g= 0.345e-4');
diff --git a/contrib/hstore/hstore_io.c b/contrib/hstore/hstore_io.c
index d3e67dd..3781a71 100644
--- a/contrib/hstore/hstore_io.c
+++ b/contrib/hstore/hstore_io.c
@@ -1241,8 +1241,8 @@ hstore_to_json_loose(PG_FUNCTION_ARGS)
 
if (count == 0)
{
-   out = palloc(1);
-   *out = '\0';
+   out = palloc(3);
+   memcpy(out, {}, 3);
PG_RETURN_TEXT_P(cstring_to_text(out));
}
 
@@ -1370,8 +1370,8 @@ hstore_to_json(PG_FUNCTION_ARGS)
 
if (count == 0)
{
-   out = palloc(1);
-   *out = '\0';
+   out = palloc(3);
+   memcpy(out, {}, 3);
PG_RETURN_TEXT_P(cstring_to_text(out));
}
 
diff --git a/contrib/hstore/sql/hstore.sql b/contrib/hstore/sql/hstore.sql
index 68b74bf..47cbfad 100644
--- a/contrib/hstore/sql/hstore.sql
+++ b/contrib/hstore/sql/hstore.sql
@@ -335,6 +335,8 @@ select count(*) from testhstore where h = 'pos=98, 
line=371, node=CBA, indexe
 select hstore_to_json('a key =1, b = t, c = null, d= 12345, e = 012345, 
f= 1.234, g= 2.345e+4');
 select cast( hstore  'a key =1, b = t, c = null, d= 12345, e = 012345, 
f= 1.234, g= 2.345e+4' as json);
 select hstore_to_json_loose('a key =1, b = t, c = null, d= 12345, e = 
012345, f= 1.234, g= 2.345e+4');
+select hstore_to_json('');
+select hstore_to_json_loose('');
 
 create table test_json_agg (f1 text, f2 hstore);
 insert into test_json_agg values ('rec1','a key =1, b = t, c = null, d= 
12345, e = 012345, f= 1.234, g= 2.345e+4'),
-- 
1.8.3.1



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


Re: [HACKERS] removing old ports and architectures

2013-10-16 Thread Robert Haas
On Tue, Oct 15, 2013 at 8:33 AM, Andres Freund and...@2ndquadrant.com wrote:
 On 2013-10-13 16:56:12 +0200, Tom Lane wrote:
 More to the point for this specific case, it seems like our process
 ought to be
 (1) select a preferably-small set of gcc atomic intrinsics that we
 want to use.

 I suggest:
 * pg_atomic_load_u32(uint32 *)
 * uint32 pg_atomic_store_u32(uint32 *)

We currently assume simple assignment suffices for this.

 * uint32 pg_atomic_exchange_u32(uint32 *ptr, uint32 val)
 * bool pg_atomic_compare_exchange_u32(uint32 *ptr, uint32 *expected, uint32 
 newval)
 * uint32 pg_atomic_fetch_add_u32(uint32 *ptr, uint32 add)
 * uint32 pg_atomic_fetch_sub_u32(uint32 *ptr, uint32 add)
 * uint32 pg_atomic_fetch_and_u32(uint32 *ptr, uint32 add)
 * uint32 pg_atomic_fetch_or_u32(uint32 *ptr, uint32 add)

Do we really need all of those?  Compare-and-swap is clearly needed,
and fetch-and-add is also very useful.  I'm not sure about the rest.
Not that I necessarily object to having them, but it will be a lot
more work.

 * u64 variants of the above
 * bool pg_atomic_test_set(void *ptr)
 * void pg_atomic_clear(void *ptr)

What are the intended semantics here?

 Ontop of that we can generically implement:
 * pg_atomic_add_until_u32(uint32 *ptr, uint32 val, uint32 limit)
 * pg_atomic_(add|sub|and|or)_fetch_u32()
 * u64 variants of the above

Are these really generally needed?

 We might also want to provide a generic implementation of the math
 operations based on pg_atomic_compare_exchange() to make it easier to
 bring up a new architecture.

+1.

I have a related problem, which is that some code I'm currently
working on vis-a-vis parallelism can run lock-free on platforms with
atomic 8 bit assignment but needs a spinlock or two elsewhere.  So I'd
want to use pg_atomic_store_u64(), but I'd also need a clean way to
test, at compile time, whether it's available.

More general question: how do we move the ball down the field in this
area?  I'm willing to do some of the legwork, but I doubt I can do all
of it, and I really think we need to make some progress here soon, as
it seems that you and I and Ants are all running into the same
problems in slightly different ways.  What's our strategy for getting
something done here?

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


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


[HACKERS] Turning recovery.conf into GUCs

2013-10-16 Thread Jaime Casanova
Hi everyone,

Seems that the latest patch in this series is:
http://www.postgresql.org/message-id/CAB7nPqRLWLH1b0YsvqYF-zOFjrv4FRiurQ6yqcP3wjBp=tj...@mail.gmail.com

= Applying =

Reading the threads it seems there is consensus in this happening, so
let's move in that direction.
The patch applies almost cleanly except for a minor change in xlog.c

= Building =

In pg_basebackup we have now 2 unused functions:
escapeConnectionParameter and escape_quotes. the only use of these
functions was when that tool created the recovery.conf file so they
aren't needed anymore.

= Functionality =

I have been testing functionality and it looks good so far, i still
need to test a few things but i don't expect anything bad.

I have 2 complaints though:

1) the file to trigger recovery is now called standby.enabled. I know
this is because we use the file to also make the node a standby.
Now, reality is that the file doesn't make the node a standby but the
combination of this file (which starts recovery) + standby_mode=on.
so, i agree with the suggestion of naming the file: recovery.trigger

2) This patch removes the dual existence of recovery.conf: as a state
file and as a configuration file

- the former is accomplished by changing the name of the file that
triggers recovery (from recovery.conf to standby.enabled)
- the latter is done by moving all parameters to postgresql.conf and
*not reading* recovery.conf anymore

so, after applying this patch postgres don't use recovery.conf for
anything... except for complaining.
it's completely fair to say we are no longer using that file and
ignoring its existence, but why we should care if users want to use a
file with that name for inclusion in postgresql.conf or where they put
that hypotetic file?

after this patch, recovery.conf will have no special meaning anymore
so let's the user put it whatever files he wants, wherever he wants.
if we really want to warn the user, use WARNING instead of FATAL

= Code  functionality =

why did you changed the context in which we can set archive_command?
please, let it as a SIGHUP parameter

-   {archive_command, PGC_SIGHUP, WAL_ARCHIVING,
+   {archive_command, PGC_POSTMASTER, WAL_ARCHIVING,


All parameters moved from recovery.conf has been marked as
PGC_POSTMASTER, but most of them are clearly PGC_SIGHUP candidates

+   {restore_command, PGC_POSTMASTER, WAL_ARCHIVE_RECOVERY,
+   {archive_cleanup_command, PGC_POSTMASTER, WAL_ARCHIVE_RECOVERY,
+   {recovery_end_command, PGC_POSTMASTER, WAL_ARCHIVE_RECOVERY,
+   {recovery_target_xid, PGC_POSTMASTER, WAL_RECOVERY_TARGET,
+   {recovery_target_name, PGC_POSTMASTER, WAL_RECOVERY_TARGET,
+   {recovery_target_time, PGC_POSTMASTER, WAL_RECOVERY_TARGET,
+   {trigger_file, PGC_POSTMASTER, REPLICATION_STANDBY,

Not sure about these ones

+   {recovery_target_timeline, PGC_POSTMASTER, WAL_RECOVERY_TARGET,
+   {primary_conninfo, PGC_POSTMASTER, REPLICATION_STANDBY,

This is the only one i agree, should be PGC_POSTMASTER only

+   {standby_mode, PGC_POSTMASTER, REPLICATION_STANDBY,

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] removing old ports and architectures

2013-10-16 Thread Andres Freund
On 2013-10-16 12:26:28 -0400, Robert Haas wrote:
 On Tue, Oct 15, 2013 at 8:33 AM, Andres Freund and...@2ndquadrant.com wrote:
  On 2013-10-13 16:56:12 +0200, Tom Lane wrote:
  More to the point for this specific case, it seems like our process
  ought to be
  (1) select a preferably-small set of gcc atomic intrinsics that we
  want to use.
 
  I suggest:
  * pg_atomic_load_u32(uint32 *)
  * uint32 pg_atomic_store_u32(uint32 *)
 
 We currently assume simple assignment suffices for this.

Partially that only works because we sprinkle 'volatile's on lots of
places. That can actually hurt performance... it'd usually be something
like:
#define pg_atomic_load(uint32) (*(volatile uint32 *)(atomic))

Even if not needed in some places because a variable is already
volatile, it's very helpful in pointing out what happens and where you
need to be careful.

  * uint32 pg_atomic_exchange_u32(uint32 *ptr, uint32 val)
  * bool pg_atomic_compare_exchange_u32(uint32 *ptr, uint32 *expected, uint32 
  newval)
  * uint32 pg_atomic_fetch_add_u32(uint32 *ptr, uint32 add)
  * uint32 pg_atomic_fetch_sub_u32(uint32 *ptr, uint32 add)
  * uint32 pg_atomic_fetch_and_u32(uint32 *ptr, uint32 add)
  * uint32 pg_atomic_fetch_or_u32(uint32 *ptr, uint32 add)
 
 Do we really need all of those?  Compare-and-swap is clearly needed,
 and fetch-and-add is also very useful.  I'm not sure about the rest.
 Not that I necessarily object to having them, but it will be a lot
 more work.

Well, _sub can clearly be implemented with _add generically. I find code
using _sub much easier to read than _add(-whatever), but that's maybe
just me.

But _and, _or are really useful because they can be used to atomically
clear and set flag bits.

 
  * u64 variants of the above
  * bool pg_atomic_test_set(void *ptr)
  * void pg_atomic_clear(void *ptr)
 
 What are the intended semantics here?

It's basically TAS() without defining whether setting a value that needs
to be set is a 1 or a 0. Gcc provides this and I think we should make
our spinlock implementation use it if there is no special cased
implementation available.

  Ontop of that we can generically implement:
  * pg_atomic_add_until_u32(uint32 *ptr, uint32 val, uint32 limit)
  * pg_atomic_(add|sub|and|or)_fetch_u32()
  * u64 variants of the above
 
 Are these really generally needed?

_add_until() is very useful for implementing thinks like usagecount
where you don't want to increase values too high.

The lwlock scaling thing needs the add_fetch variant because we need to
know what the lockcount is *after* we've registered. I think lots of
lockless algorithm have similar requirements.

Since those are either wrappers around fetch_op or compare_swap and thus
can be implemented generically I don't really see a problem with
providing them.

 I have a related problem, which is that some code I'm currently
 working on vis-a-vis parallelism can run lock-free on platforms with
 atomic 8 bit assignment but needs a spinlock or two elsewhere.  So I'd
 want to use pg_atomic_store_u64(), but I'd also need a clean way to
 test, at compile time, whether it's available.

Yes, definitely. There should be a couple of #defines that declare
whether non-prerequisite options are supported. I'd guess we want at least:
* 8byte math
* 16byte compare_and_swap

 More general question: how do we move the ball down the field in this
 area?  I'm willing to do some of the legwork, but I doubt I can do all
 of it, and I really think we need to make some progress here soon, as
 it seems that you and I and Ants are all running into the same
 problems in slightly different ways.  What's our strategy for getting
 something done here?

That's a pretty good question.

I am willing to write the gcc implementation, plus the generic wrappers
and provide the infrastructure to override it per-platform. I won't be
able to do anything about non-linux, non-gcc platforms in that timeframe
though.

I was thinking of something like:
include/storage/atomic.h
include/storage/atomic-gcc.h
include/storage/atomic-msvc.h
include/storage/atomic-acc-ia64.h
where atomic.h first has a list of #ifdefs including the specialized
files and then lots of #ifndef providing generic variants if not
already provided by the platorm specific file.

We could provide not only per-compiler files but also compiler
independent files for some arches so we could e.g. define the
restrictions for arm once. I think whether that's useful will be visible
when writing the stuff.

Makes sense?

Greetings,

Andres Freund

-- 
 Andres Freund 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] removing old ports and architectures

2013-10-16 Thread Robert Haas
On Sat, Oct 12, 2013 at 8:46 PM, Andres Freund and...@2ndquadrant.com wrote:
 I think we should remove support the following ports:
 - IRIX
 - UnixWare
 - Tru64

According to http://en.wikipedia.org/wiki/IRIX, IRIX has been
officially retired.  The last release of IRIX was in 2006 and support
will end in December of 2013.  Therefore, it will be unsupported by
the time PostgreSQL 9.4 is released.

According to http://en.wikipedia.org/wiki/UnixWare, UnixWare is not
dead, although there have been no new releases in 5 years.

According to http://en.wikipedia.org/wiki/Tru64_UNIX, Tru64 has been
officially retired.  Support ended in December, 2012.  This seems safe
to remove.

So I vote for removing IRIX and Tru64 immediately, but I'm a little
more hesitant about shooting UnixWare, since it's technically still
supported.

 Neither of those are relevant.

 I think we should remove support for the following architectures:
 - VAX

According to http://en.wikipedia.org/wiki/VAX#History, all
manufacturing of VAX computers ceased in 2005, but according to
http://en.wikipedia.org/wiki/OpenVMS#Major_release_timeline, OpenVMS
is still releasing new versions.  I'm not sure what to make of that.

 - univel (s_lock support remaining)

We removed the univel port in 9.2 and didn't get any complaints, but
the s_lock support is still used by the SCO and UnixWare ports, except
under GCC.

 - sinix (s_lock support remaining)

This seems to be quite thoroughly dead.  The best information I can
find indicates that development ended in 2002 and support in 2008.  I
think we can remove this.

 - sun3 (I think it's just s_lock support remaining)
 - natsemi 32k

Both of these are so old I can hardly find any information on them.
Seems clear to nuke these.

 - superH

Support for spinlocks on SuperH was only recently added, in 9.0.  I
don't think we can assume that no one cares any more.

 - ALPHA (big pain in the ass to get right, nobody uses it anymore)

It seems somehow a shame to let this one go, but I agree it's a big
pain in the ass to get it right.

 - m86k (doesn't have a useable CAS on later iterations like coldfire)

I don't think we can desupport it just because it doesn't have CAS.
CAS is very useful and I think we should start using it, but I think
we should anticipate a --disable-cas or --disable-atomics option and
regularly test that our code works without it.  IOW, we can rely on it
as an optimization, but not for correctness.  Eventually, we can
probably desupport all platforms that don't have CAS, but I see that
as something that's probably 5 or 10 years away, not something we can
do tomorrow.

I'm also not sure that this is dead enough to kill.

 - M32R (no userspace CAS afaics)

Support was added for this in 8.3 and it doesn't seem to be particularly dead.

 - mips for anything but gcc  4.4, using gcc's atomics support
 - s390 for anything but gcc  4.4, using gcc's atomics support

I'm not clearly how broadly this would sweep, but MIPS doesn't appear dead.

 - 32bit/v9 sparc (doesn't have proper atomics, old)

Seems fine to remove this.

-- 
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] insert throw error when year field len 4 for timestamptz datatype

2013-10-16 Thread Bruce Momjian
On Tue, Oct  8, 2013 at 09:05:37AM -0400, Bruce Momjian wrote:
 On Tue, Oct  8, 2013 at 05:08:17PM +0530, Rushabh Lathia wrote:
  This
  might be a case where throwing an error is actually better than trying
  to make sense of the input.
  
  I don't feel super-strongly about this, but I offer it as a question
  for reflection.
  
  
  
  At the same time I do agree fixing this kind of issue in postgres datetime
  module 
  is bit difficult without some assumption. Personally I feel patch do add 
  some
  value but not fully compatible with all kind of year field format.
  
  Bruce,
  
  Do you have any thought/suggestion ?
 
 I think Robert is asking the right question:  Is it better to accept
 5-digit years, or throw an error?  Doing anything new with 6-digit years
 is going to break the much more common use of YMD or HMS.
 
 The timestamp data type only supports values to year 294276, so the full
 6-digit range isn't even supported.  ('DATE' does go higher.)
 
 The entire date/time processing allows imprecise input, so throwing an
 error on clear 5-digit years seems wrong.  Basically, we have gone down
 the road of interpreting date/time input liberally, so throwing an error
 on a clear 5-digit year seems odd.
 
 On the other hand, this has never come up before because no one cared
 about 5-digit years, so you could argue that 5-digit years require
 precise specification, which would favor throwing an error.

Patch applied to support 5+ digit years in non-ISO timestamp/date
strings, where appropriate.

-- 
  Bruce Momjian  br...@momjian.ushttp://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] removing old ports and architectures

2013-10-16 Thread Stefan Kaltenbrunner
On 10/16/2013 07:04 PM, Robert Haas wrote:
 On Sat, Oct 12, 2013 at 8:46 PM, Andres Freund and...@2ndquadrant.com wrote:
 I think we should remove support the following ports:
 - IRIX
 - UnixWare
 - Tru64
 
 According to http://en.wikipedia.org/wiki/IRIX, IRIX has been
 officially retired.  The last release of IRIX was in 2006 and support
 will end in December of 2013.  Therefore, it will be unsupported by
 the time PostgreSQL 9.4 is released.
 
 According to http://en.wikipedia.org/wiki/UnixWare, UnixWare is not
 dead, although there have been no new releases in 5 years.
 
 According to http://en.wikipedia.org/wiki/Tru64_UNIX, Tru64 has been
 officially retired.  Support ended in December, 2012.  This seems safe
 to remove.
 
 So I vote for removing IRIX and Tru64 immediately, but I'm a little
 more hesitant about shooting UnixWare, since it's technically still
 supported.
 
 Neither of those are relevant.

agreed


 I think we should remove support for the following architectures:
 - VAX
 
 According to http://en.wikipedia.org/wiki/VAX#History, all
 manufacturing of VAX computers ceased in 2005, but according to
 http://en.wikipedia.org/wiki/OpenVMS#Major_release_timeline, OpenVMS
 is still releasing new versions.  I'm not sure what to make of that.

VAX is also an officially supported OpenBSD port (see
http://www.openbsd.org/vax.html)





Stefan


-- 
Sent 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] pg_sleep(interval)

2013-10-16 Thread Josh Berkus
On 10/16/2013 08:18 AM, Stephen Frost wrote:
 As it relates to this specific patch for this CF, I'd go with 'Returned
 with Feedback' and encourage you to consider the arguments for and
 against, and perhaps try to find existing usage which would break due to
 this change and consider the impact of changing it.  For example, what
 do the various languages and DB abstraction layers do today?  Would
 users of Hibernate likely be impacted or no?  What about PDO?
 Personally, I'm still on-board with the change in general, but it'd
 really help to know that normal/obvious usage through various languages
 won't be busted by the change.

I'm fairly sure that the only language likely to be impacted chronically
is PHP.  Java, Ruby and Python, as a rule, properly data-type their
parameters; PHP is the only language I know of where developers *and
frameworks* chronically pass everything as a string.  IIRC, the primary
complainers when we removed a bunch of implicit casts in 8.3 were PHP devs.

One thing to keep in mind, though, is that few developers actually use
pg_sleep(), and I'd be surprised to find in in any canned web framework.
 Generally, if a web developer is going to sleep, they do it in the
application.  So we're really only talking about stored procedure
writers here.  And, as a rule, we can expect stored procedure writers to
not gratuitously use strings in place of integers.

We generally don't bounce PLpgSQL features which break unsupported
syntax in PLpgSQL, since we expect people who write functions to have a
better knowledge of SQL syntax than people who don't.  I think
pg_sleep(interval) falls under the same test.  So I'd vote to accept it.

Also, as Tom pointed out, at some point we have to either say we really
support overloading or we don't.

-- 
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] removing old ports and architectures

2013-10-16 Thread Andres Freund
On 2013-10-16 13:04:23 -0400, Robert Haas wrote:
 So I vote for removing IRIX and Tru64 immediately, but I'm a little
 more hesitant about shooting UnixWare, since it's technically still
 supported.

I think if somebody wants to have it supported they need to provide a
buildfarm member and probably a bit of help. Doing any change in this
area for a platform that nobody has access to in any way seems
pointless because it will be broken anyway.

  I think we should remove support for the following architectures:
  - VAX
 
 According to http://en.wikipedia.org/wiki/VAX#History, all
 manufacturing of VAX computers ceased in 2005, but according to
 http://en.wikipedia.org/wiki/OpenVMS#Major_release_timeline, OpenVMS
 is still releasing new versions.  I'm not sure what to make of that.

And the last revisions are from 2000 even. I don't think anybody will
actually run a new postgres installation on it.
I also doubt our current version actually compiles there.

  - superH
 
 Support for spinlocks on SuperH was only recently added, in 9.0.  I
 don't think we can assume that no one cares any more.

Yes, I since revised my opinion somewhere downthread. It's pretty much
linux and gcc only, so it's really not that problematic.

Note that our current implementation is broken on many older SuperH (
SH4) CPUs since their tas isn't safe...

  - m86k (doesn't have a useable CAS on later iterations like coldfire)

 I don't think we can desupport it just because it doesn't have CAS.
 CAS is very useful and I think we should start using it, but I think
 we should anticipate a --disable-cas or --disable-atomics option and
 regularly test that our code works without it.  IOW, we can rely on it
 as an optimization, but not for correctness.  Eventually, we can
 probably desupport all platforms that don't have CAS, but I see that
 as something that's probably 5 or 10 years away, not something we can
 do tomorrow.

I think that will result in loads of barely tested duplicative code. If
there were any even remotely popular platforms requiring this, ok. But
unless I miss something there really isn't.

We're talking about CPUs with mostly less than 100MHZ here, mostly with
directly soldered RAM in the one digit MB range. I really don't think
there's a usecase for running PG on them. And I doubt it still works on
many of the architectures we advocate.

 I'm also not sure that this is dead enough to kill.
 
  - M32R (no userspace CAS afaics)
 
 Support was added for this in 8.3 and it doesn't seem to be particularly dead.

It's not? All I've read seems to point into a different direction.

The newest supported CPU seems to be
http://www.renesas.com/products/mpumcu/m32r/m32r_ecu/32196/index.jsp
sporting a whopping 1024Kb of programmable RAM and only single precision
FPU.

  - mips for anything but gcc  4.4, using gcc's atomics support
  - s390 for anything but gcc  4.4, using gcc's atomics support
 
 I'm not clearly how broadly this would sweep, but MIPS doesn't appear dead.

Downthread I noticed it's gcc 4.2 not 4.4. There's some API change in
gcc's atomics support in 4.4 which is why I thought of 4.4 but it
shouldn't affect us after looking in more detail.
Mips seems to only be used with gcc these days, so I think that's ok.

Greetings,

Andres Freund

-- 
 Andres Freund 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] removing old ports and architectures

2013-10-16 Thread Robert Haas
 - sinix (s_lock support remaining)
 - sun3 (I think it's just s_lock support remaining)
 - natsemi 32k

Patch removing spinlock support for these three ports is attached.
This is not to say we couldn't remove more later, but these seem to be
the three spinlock implementations that are most sincerely dead.
Absent objections, I'll apply this tomorrow.

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


desupport-ancient-spinlocks.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] removing old ports and architectures

2013-10-16 Thread Andres Freund
On 2013-10-16 13:55:20 -0400, Robert Haas wrote:
  - sinix (s_lock support remaining)
  - sun3 (I think it's just s_lock support remaining)
  - natsemi 32k
 
 Patch removing spinlock support for these three ports is attached.
 This is not to say we couldn't remove more later, but these seem to be
 the three spinlock implementations that are most sincerely dead.
 Absent objections, I'll apply this tomorrow.

Looks good to me.

Greetings,

Andres Freund

-- 
 Andres Freund 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] removing old ports and architectures

2013-10-16 Thread Andres Freund
On 2013-10-16 13:04:23 -0400, Robert Haas wrote:
  - m86k (doesn't have a useable CAS on later iterations like coldfire)
 
 I don't think we can desupport it just because it doesn't have CAS.

Btw, if necessary we could easily support the pre coldfire
variants. Note that e.g. debian doesn't support coldfire either. Well,
the unofficial m68k port after it has been dropped from core debian in
*2007*.

Greetings,

Andres Freund

-- 
 Andres Freund 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] [SQL] Comparison semantics of CHAR data type

2013-10-16 Thread Bruce Momjian
  You can see the UTF8 case is fine because \n is considered greater
  than space, but in the C locale, where \n is less than space, the
  false return value shows the problem with
  internal_bpchar_pattern_compare() trimming the string and first
  comparing on lengths.  This is exactly the problem you outline, where
  space trimming assumes everything is less than a space.
 
 For collations other than C some of those issues that have to do with
 string comparisons might simply be hidden, depending on how strcoll()
 handles inputs off different lengths: If strcoll() applies implicit
 space padding to the shorter value, there won't be any visible
 difference in ordering between bpchar and varchar values.  If strcoll()
 does not apply such space padding, the right-trimming of bpchar values
 causes very similar issues even in a en_US collation.
 
 For example, this seems to be the case on OS X:
 
 select 'ab '::char(10) collate en_US  E'ab\n'::char(10)
 collate en_US;
  ?column?
 --
  t
 (1 row)
 
 select 'ab '::char(10) collate C  E'ab\n'::char(10) collate C;
  ?column?
 --
  t
 (1 row)
 
 select 'ab '::varchar(10) collate en_US 
 E'ab\n'::varchar(10) collate en_US;
  ?column?
 --
  f
 (1 row)

The above query returns true on Linux, so there certainly is a
platform-specific difference there.  The others are the same.

 select 'ab '::varchar(10) collate C  E'ab\n'::varchar(10)
 collate C;
  ?column?
 --
  f
 (1 row)
 
 So here there's actually not only the same \n/space issue as in the C
 collation (which would go away if the bpchar value weren't trimmed).
 It also shows that there might be slight differences in behavior,
 depending which platform you're running on.
 
 On Fri, Oct 11, 2013 at 4:58 PM, Kevin Grittner kgri...@ymail.com wrote:
  What matters in general isn't where the characters fall when comparing
  individual bytes, but how the strings containing them sort according
  to the applicable collation.  That said, my recollection of the spec
  is that when two CHAR(n) values are compared, the shorter should be
  blank-padded before making the comparison.
 
 Not necessarily.  The SQL Standard actually ties this to the collation
 sequence that is in use.  Without a lot of context, this is from
 Subclause 8.2, comparison predicate, General Rule 3)b):
 
   b) If the length in characters of X is not equal to the length in
  characters of Y, then the shorter string is effectively replaced,
  for the purposes of comparison, with a copy of itself that has been
  extended to the length of the longer string by concatenation on the
  right of one or more pad characters, where the pad character is
  chosen based on CS. If CS has the NO PAD characteristic, then the
  pad character is an implementation-dependent character different
  from any character in the character set of X and Y that collates
  less than any string under CS. Otherwise, the pad character is a
  space.
 
 In my opinion, that's just a lot of handwaving, to the extent that in
 practice different vendors interpret this clause differently.  It seems
 that SQLServer and DB2 do PAD semantics across the board, whereas Oracle
 has uses NO PAD semantics whenever there's a VARCHAR type involved in
 the comparison.
 
 But all that is actually a whole different can of worms, and slightly
 besides the point of my original question.  How to properly compare
 strings with different lentgths has been discussed before, see for
 instance the thread in [1].  My intention was not to get that started
 again.  As far as I can see, the consensus seems to be that when using
 the C locale, string comparisons should be done using NO PAD semantics.
 (It sure gives some strange semantics if you have varchars with trailing
 spaces, but it's perfectly legal.)
 
 The point is that my testcase deals with strings of the same length.
 Thus, the above clause doesn't really apply.  The standard, to my
 understanding, says that fixed-length character values are padded when
 the row is constructed.  And once that happens, those spaces become part
 of the value.  It's invalid to strip them, unless done explicitly.

Yes, there are three types of comparisons that are important here:

1. 'a'::CHAR(3)  'a'::CHAR(3)
2. 'a '::CHAR(3)  E'a\n'::CHAR(3)
3. 'a'::CHAR(3)  'a'::CHAR(4)

You are saying it is only #3 where we can substitute the special
always-lower pad character, while it appears that Postgres does this in
cases #2 and #3.

  Since we only have the CHAR(n) type to improve compliance with the SQL
  specification, and we don't generally encourage its use, I think we
  should fix any non-compliant behavior.  That seems to mean that if you
  take two CHAR values and compare them, it should give the same result
  as comparing the same 

Re: [HACKERS] Triggers on foreign tables

2013-10-16 Thread Kohei KaiGai
2013/10/16 Robert Haas robertmh...@gmail.com:
 On Tue, Oct 15, 2013 at 4:17 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 One reason we should support local triggers is that not all the data
 source of FDW support remote trigger. It required FDW drivers to
 have RDBMS as its backend, but no realistic assumption.
 For example, file_fdw is unavailable to implement remote triggers.

 True, but gosh, updates via file_fdw are gonna be so slow I can't
 believe anybody'd use it for something real

How about another example? I have implemented a column-oriented
data store with read/write capability, using FDW APIs.
The role of FDW driver here is to translate its data format between
column-oriented and row-oriented, but no trigger capability itself.

 One thing I'd like to know is, where is the goal of FDW feature.
 It seems to me, FDW goes into a feature to manage external data
 set as if regular tables. If it is right understanding, things we try to
 support on foreign table is things we're supporting on regular tables,
 such as triggers.

 I generally agree with that.

 We often have some case that we cannot apply fully optimized path
 because of some reasons, like view has security-barrier, qualifier
 contained volatile functions, and so on...
 Trigger may be a factor to prevent fully optimized path, however,
 it depends on the situation which one shall be prioritized; performance
 or functionality.

 Sure.  I mean, I guess if there are enough people that want this, I
 suppose I ought not stand in the way.  It just seems like a lot of
 work for a feature of very marginal utility.

The reason why I'm interested in this feature is, row-level triggers on
foreign tables will become a piece to implement table partitioning
that contains multiple foreign tables.
Probably, it also connects to the effort of custom-plan node (in the
future) that enables to replace Append node by custom logic, to kick
multiple concurrent remote queries on behalf of partitioned foreign table.

Thanks,
-- 
KaiGai Kohei kai...@kaigai.gr.jp


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


Re: [HACKERS] removing old ports and architectures

2013-10-16 Thread Robert Haas
On Wed, Oct 16, 2013 at 1:52 PM, Andres Freund and...@2ndquadrant.com wrote:
 On 2013-10-16 13:04:23 -0400, Robert Haas wrote:
 So I vote for removing IRIX and Tru64 immediately, but I'm a little
 more hesitant about shooting UnixWare, since it's technically still
 supported.

 I think if somebody wants to have it supported they need to provide a
 buildfarm member and probably a bit of help. Doing any change in this
 area for a platform that nobody has access to in any way seems
 pointless because it will be broken anyway.

I don't agree with that policy.  Sure, 97% of our users are probably
running Linux, Windows, MacOS X, or one of the fairly-popular BSD
variants. But I think a part of the appeal of PostgreSQL is that it is
cross-platform, and doesn't require a lot of special hardware support,
and I'm loathe to give that up.  It's one thing to say, gee, we don't
know whether this is actually going to compile and work on your
platform, because it's not tested.  It's quite something else to say,
anything that's not on this short list of supported platforms has zero
chance of working without jumping through major hoops.

Commit b8ed4cc9627de437e5eafdb81631a0d0f063abb3, from April of this
year, updated our support for --disable-spinlocks.  Spinlocks are a
far more basic facility than the sorts of advanced primitives we're
talking about requiring here.  If we're going to support compiling
under --disable-spinlocks, then we certainly can't rely on this more
advanced stuff always being present.  Even if we get rid of that, it's
throwing up one more barrier in front of people who want to get
PostgreSQL running on a non-mainstream architecture.  I've spent
enough time over the years working on odd hardware to have sympathy
for people who want to compile stuff there and have it work, or at
least work after some non-huge amount of hacking.

 I don't think we can desupport it just because it doesn't have CAS.
 CAS is very useful and I think we should start using it, but I think
 we should anticipate a --disable-cas or --disable-atomics option and
 regularly test that our code works without it.  IOW, we can rely on it
 as an optimization, but not for correctness.  Eventually, we can
 probably desupport all platforms that don't have CAS, but I see that
 as something that's probably 5 or 10 years away, not something we can
 do tomorrow.

 I think that will result in loads of barely tested duplicative code. If
 there were any even remotely popular platforms requiring this, ok. But
 unless I miss something there really isn't.

 We're talking about CPUs with mostly less than 100MHZ here, mostly with
 directly soldered RAM in the one digit MB range. I really don't think
 there's a usecase for running PG on them. And I doubt it still works on
 many of the architectures we advocate.

If stuff is completely ancient and obsolete, I think it's fine to kill
it; it probably doesn't work anyway, and nobody's likely to try.  But
I think a lot of the stuff you're talking about is not in that
category.

 I'm also not sure that this is dead enough to kill.

  - M32R (no userspace CAS afaics)

 Support was added for this in 8.3 and it doesn't seem to be particularly 
 dead.

 It's not? All I've read seems to point into a different direction.

 The newest supported CPU seems to be
 http://www.renesas.com/products/mpumcu/m32r/m32r_ecu/32196/index.jsp
 sporting a whopping 1024Kb of programmable RAM and only single precision
 FPU.

Well, so, they're an embedded chip.  Yes, it's low spec.  But then
why'd somebody bother adding spinlock support for it in 2007?  It's
not as if that was a high-end chip *then* either.

It's hard to say where to draw the line here.  I don't want the
illusion of support for platforms that don't in fact have a prayer of
working to prevent us from making needed improvements.  On the other
hand, I also don't want to blithely rip out support for architectures
that people may well still be using and where, in some cases, people
have gone out of their way to add that support.  If we get to the
point where some relatively-obscure architecture is the only thing
standing between us and improvement X, then we can weigh those things
against each other and decide.  But I don't really want to go rip out
support for half-a-dozen semi-supported architectures without some
clear goal in mind.  That just doesn't seem friendly.

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


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


[HACKERS] Record comparison compiler warning

2013-10-16 Thread Bruce Momjian
I am seeing this compiler warning in git head:

rowtypes.c: In function 'record_image_cmp':
rowtypes.c:1433: warning: 'cmpresult' may be used
uninitialized in this function rowtypes.c:1433: note: 'cmpresult' was 
declared here

-- 
  Bruce Momjian  br...@momjian.ushttp://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


[HACKERS] better atomics

2013-10-16 Thread Robert Haas
On Wed, Oct 16, 2013 at 12:52 PM, Andres Freund and...@2ndquadrant.com wrote:
 Partially that only works because we sprinkle 'volatile's on lots of
 places. That can actually hurt performance... it'd usually be something
 like:
 #define pg_atomic_load(uint32) (*(volatile uint32 *)(atomic))

 Even if not needed in some places because a variable is already
 volatile, it's very helpful in pointing out what happens and where you
 need to be careful.

That's fine with me.

  * uint32 pg_atomic_exchange_u32(uint32 *ptr, uint32 val)
  * bool pg_atomic_compare_exchange_u32(uint32 *ptr, uint32 *expected, 
  uint32 newval)
  * uint32 pg_atomic_fetch_add_u32(uint32 *ptr, uint32 add)
  * uint32 pg_atomic_fetch_sub_u32(uint32 *ptr, uint32 add)
  * uint32 pg_atomic_fetch_and_u32(uint32 *ptr, uint32 add)
  * uint32 pg_atomic_fetch_or_u32(uint32 *ptr, uint32 add)

 Do we really need all of those?  Compare-and-swap is clearly needed,
 and fetch-and-add is also very useful.  I'm not sure about the rest.
 Not that I necessarily object to having them, but it will be a lot
 more work.

 Well, _sub can clearly be implemented with _add generically. I find code
 using _sub much easier to read than _add(-whatever), but that's maybe
 just me.

Yeah, I wouldn't bother with _sub.

 But _and, _or are really useful because they can be used to atomically
 clear and set flag bits.

Until we have some concrete application that requires this
functionality for adequate performance, I'd be inclined not to bother.
 I think we have bigger fish to fry, and (to extend my nautical
metaphor) trying to get this working everywhere might amount to trying
to boil the ocean.

  * u64 variants of the above
  * bool pg_atomic_test_set(void *ptr)
  * void pg_atomic_clear(void *ptr)

 What are the intended semantics here?

 It's basically TAS() without defining whether setting a value that needs
 to be set is a 1 or a 0. Gcc provides this and I think we should make
 our spinlock implementation use it if there is no special cased
 implementation available.

I'll reserve judgement until I see the patch.

 More general question: how do we move the ball down the field in this
 area?  I'm willing to do some of the legwork, but I doubt I can do all
 of it, and I really think we need to make some progress here soon, as
 it seems that you and I and Ants are all running into the same
 problems in slightly different ways.  What's our strategy for getting
 something done here?

 That's a pretty good question.

 I am willing to write the gcc implementation, plus the generic wrappers
 and provide the infrastructure to override it per-platform. I won't be
 able to do anything about non-linux, non-gcc platforms in that timeframe
 though.

 I was thinking of something like:
 include/storage/atomic.h
 include/storage/atomic-gcc.h
 include/storage/atomic-msvc.h
 include/storage/atomic-acc-ia64.h
 where atomic.h first has a list of #ifdefs including the specialized
 files and then lots of #ifndef providing generic variants if not
 already provided by the platorm specific file.

Seems like we might want to put some of this in one of the various
directories called port, or maybe a new subdirectory of one of them.
 This basically sounds sane, but I'm unwilling to commit to dropping
support for all obscure platforms we currently support unless there
are about 500 people +1ing the idea, so I think we need to think about
what happens when we don't have platform support for these primitives.

-- 
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] Triggers on foreign tables

2013-10-16 Thread Robert Haas
On Wed, Oct 16, 2013 at 2:20 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 True, but gosh, updates via file_fdw are gonna be so slow I can't
 believe anybody'd use it for something real

 How about another example? I have implemented a column-oriented
 data store with read/write capability, using FDW APIs.
 The role of FDW driver here is to translate its data format between
 column-oriented and row-oriented, but no trigger capability itself.

OK, that's a believable example.  Call me convinced.  :-)

-- 
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] Turning recovery.conf into GUCs

2013-10-16 Thread Josh Berkus
Jaime,
 = Building =

 In pg_basebackup we have now 2 unused functions:
 escapeConnectionParameter and escape_quotes. the only use of these
 functions was when that tool created the recovery.conf file so they
 aren't needed anymore.

Except that we'll want 9.4's -R to do something, probably create a file
called conf.d/replication.conf.  Mind you, it won't need the same wonky
quoting stuff.

 1) the file to trigger recovery is now called standby.enabled. I know
 this is because we use the file to also make the node a standby.
 Now, reality is that the file doesn't make the node a standby but the
 combination of this file (which starts recovery) + standby_mode=on.
 so, i agree with the suggestion of naming the file: recovery.trigger
 
 2) This patch removes the dual existence of recovery.conf: as a state
 file and as a configuration file
 
 - the former is accomplished by changing the name of the file that
 triggers recovery (from recovery.conf to standby.enabled)
 - the latter is done by moving all parameters to postgresql.conf and
 *not reading* recovery.conf anymore
 
 so, after applying this patch postgres don't use recovery.conf for
 anything... except for complaining.
 it's completely fair to say we are no longer using that file and
 ignoring its existence, but why we should care if users want to use a
 file with that name for inclusion in postgresql.conf or where they put
 that hypotetic file?

So this is a bit of a catch-22.  If we allow the user to use a file
named recovery.conf in $PGDATA, then the user may be unaware that the
*meaning* of the file has changed, which can result in unplanned
promotion of replicas after upgrade.

*on the other hand*, if we prevent creation of a configuration file
named recovery.conf, then we block efforts to write
backwards-compatible management utilities.

AFAIK, there is no good solution for this, which is why it's taken so
long for us to resolve the general issue.  Given that, I think it's
better to go for the breakage and all the warnings.  If a user wants a
file called recovery.conf, put it in the conf.d directory.

I don't remember, though: how does this patch handle PITR recovery?

 = Code  functionality =

 +   {restore_command, PGC_POSTMASTER, WAL_ARCHIVE_RECOVERY,
 +   {archive_cleanup_command, PGC_POSTMASTER, WAL_ARCHIVE_RECOVERY,
 +   {recovery_end_command, PGC_POSTMASTER, WAL_ARCHIVE_RECOVERY,
 +   {recovery_target_xid, PGC_POSTMASTER, WAL_RECOVERY_TARGET,
 +   {recovery_target_name, PGC_POSTMASTER, WAL_RECOVERY_TARGET,
 +   {recovery_target_time, PGC_POSTMASTER, WAL_RECOVERY_TARGET,
 +   {trigger_file, PGC_POSTMASTER, REPLICATION_STANDBY,
 
 Not sure about these ones
 
 +   {recovery_target_timeline, PGC_POSTMASTER, WAL_RECOVERY_TARGET,
 +   {primary_conninfo, PGC_POSTMASTER, REPLICATION_STANDBY,

It would be really nice to change these on the fly; it would help a lot
of issues with minor changes to replication config.  I can understand,
though, that the replication code might not be prepared for that.



-- 
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] [PATCH] pg_sleep(interval)

2013-10-16 Thread Robert Haas
On Wed, Oct 16, 2013 at 1:26 PM, Josh Berkus j...@agliodbs.com wrote:
 Also, as Tom pointed out, at some point we have to either say we really
 support overloading or we don't.

We clearly do support overloading.  I don't think that's at issue.
But as we all know, using it can cause formerly unambiguous queries to
become ambiguous and stop working.

-- 
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] Record comparison compiler warning

2013-10-16 Thread Kevin Grittner
Bruce Momjian br...@momjian.us wrote:

 I am seeing this compiler warning in git head:

 rowtypes.c: In function 'record_image_cmp':
 rowtypes.c:1433: warning: 'cmpresult' may be used
 uninitialized in this function rowtypes.c:1433: note: 'cmpresult' was 
declared here

I had not gotten a warning under either gcc or clang, but that was
probably because I was doing assert-enabled builds, and the
Assert(false) saved me.  That seemed a little marginal anyway, so
how about this?:

diff --git a/src/backend/utils/adt/rowtypes.c b/src/backend/utils/adt/rowtypes.c
index ae007cf..0332593 100644
--- a/src/backend/utils/adt/rowtypes.c
+++ b/src/backend/utils/adt/rowtypes.c
@@ -1508,7 +1508,11 @@ record_image_cmp(FunctionCallInfo fcinfo)
    break;
 #endif
    default:
-   Assert(false);  /* cannot happen */
+   /* cannot happen */
+   elog(ERROR,
+    unexpected length of %i found comparing columns 
of type %s,
+    (int) tupdesc1-attrs[i1]-attlen,
+    format_type_be(tupdesc1-attrs[i1]-atttypid));
    }
    }
    else

Does that fix the warning for you?

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


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


Re: [HACKERS] better atomics

2013-10-16 Thread Andres Freund
On 2013-10-16 14:30:34 -0400, Robert Haas wrote:
  But _and, _or are really useful because they can be used to atomically
  clear and set flag bits.
 
 Until we have some concrete application that requires this
 functionality for adequate performance, I'd be inclined not to bother.
  I think we have bigger fish to fry, and (to extend my nautical
 metaphor) trying to get this working everywhere might amount to trying
 to boil the ocean.

Well, I actually have a very, very preliminary patch using them in the
course of removing the spinlocks in PinBuffer() (via LockBufHdr()).

I think it's also going to be easier to add all required atomics at once
than having to go over several platforms in independent feature
patches adding atomics one by one.

  I was thinking of something like:
  include/storage/atomic.h
  include/storage/atomic-gcc.h
  include/storage/atomic-msvc.h
  include/storage/atomic-acc-ia64.h
  where atomic.h first has a list of #ifdefs including the specialized
  files and then lots of #ifndef providing generic variants if not
  already provided by the platorm specific file.

 Seems like we might want to put some of this in one of the various
 directories called port, or maybe a new subdirectory of one of them.

Hm. I'd have to be src/include/port, right? Not sure if putting some
files there will be cleaner, but I don't mind it either.

  This basically sounds sane, but I'm unwilling to commit to dropping
 support for all obscure platforms we currently support unless there
 are about 500 people +1ing the idea, so I think we need to think about
 what happens when we don't have platform support for these primitives.

I think the introduction of the atomics really isn't much dependent on
the removal. The only thing I'd touch around platforms in that patch is
adding a generic fallback pg_atomic_test_and_set() to s_lock.h and
remove the special case usages of __sync_lock_test_and_set() (Arm64,
some 32 bit arms).
It's the patches that would like to rely on atomics that will have the
problem :(

Greetings,

Andres Freund

-- 
 Andres Freund 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] removing old ports and architectures

2013-10-16 Thread Bruce Momjian
On Sat, Oct 12, 2013 at 06:35:00PM -0700, Peter Geoghegan wrote:
  - ALPHA (big pain in the ass to get right, nobody uses it anymore)
 
 Yes, for many years now ALPHA has only been useful as a way of
 illustrating how bad it's possible for CPU memory operation reordering
 considerations to get. So I quite agree.

Are there any optimizations we have avoided, or 'volatile' designations
added, only for Alpha?  Could we improve other things if Alpha support
was dropped?

-- 
  Bruce Momjian  br...@momjian.ushttp://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] Record comparison compiler warning

2013-10-16 Thread Bruce Momjian
On Wed, Oct 16, 2013 at 11:49:13AM -0700, Kevin Grittner wrote:
 Bruce Momjian br...@momjian.us wrote:
 
  I am seeing this compiler warning in git head:
 
  rowtypes.c: In function 'record_image_cmp':
  rowtypes.c:1433: warning: 'cmpresult' may be used
  uninitialized in this function rowtypes.c:1433: note: 'cmpresult' was 
 declared here
 
 I had not gotten a warning under either gcc or clang, but that was
 probably because I was doing assert-enabled builds, and the
 Assert(false) saved me.  That seemed a little marginal anyway, so
 how about this?:

Would you please send the file as ASCII, e.g. not:

A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0 
default:

-- 
  Bruce Momjian  br...@momjian.ushttp://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] removing old ports and architectures

2013-10-16 Thread Andres Freund
On 2013-10-16 15:49:54 -0400, Bruce Momjian wrote:
 On Sat, Oct 12, 2013 at 06:35:00PM -0700, Peter Geoghegan wrote:
   - ALPHA (big pain in the ass to get right, nobody uses it anymore)
  
  Yes, for many years now ALPHA has only been useful as a way of
  illustrating how bad it's possible for CPU memory operation reordering
  considerations to get. So I quite agree.
 
 Are there any optimizations we have avoided, or 'volatile' designations
 added, only for Alpha?

I am somewhat sure that some of the code we added in the last years
isn't actually correct for alpha (and others actually). It's just that
nobody actually runs on alpha anymore, so nobody notices.

 Could we improve other things if Alpha support was dropped?

I think the major thing is that if we're going to add more algorithms
that use less locks - which we'll have to, otherwise our scalability
will get more and more problematic - we'll have to adhere to the
weakest cache coherency model we support. And at least I am not
intelligent/experienced enough to blindly write correct code for Alpha.

Greetings,

Andres Freund

-- 
 Andres Freund 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] removing old ports and architectures

2013-10-16 Thread Robert Haas
On Wed, Oct 16, 2013 at 3:26 PM, Andres Freund and...@2ndquadrant.com wrote:
 I don't agree with that policy.  Sure, 97% of our users are probably
 running Linux, Windows, MacOS X, or one of the fairly-popular BSD
 variants. But I think a part of the appeal of PostgreSQL is that it is
 cross-platform, and doesn't require a lot of special hardware support,
 and I'm loathe to give that up.  It's one thing to say, gee, we don't
 know whether this is actually going to compile and work on your
 platform, because it's not tested.  It's quite something else to say,
 anything that's not on this short list of supported platforms has zero
 chance of working without jumping through major hoops.

 Well, I am not advocating to break platforms just because they are not
 on the buildfarm, but I don't see how we can introduce new facilities
 that require platform support if we don't have any way to test them on
 some platforms.

Well, on that we agree.  But my answer to that is - any facilities
that require additional platform support had better be optional.  If
we can't do it without raising the bar for platform support, then we
don't do it.

 I think by using architecture independent, compiler provided intrinsics
 we take care of that for most non-mainstream platforms. Just about
 everything even remotely new that is/will be out there is/will be using gcc
 or something compatible to it (e.g. llvm). Those provide the __sync_*
 intrinsics we'd wrap.
 That's incidentally why I am advocating including pg_atomic_clear and
 pg_atomic_test_set in the set of supported atomics. With those porting
 to a new platform will often require exactly zero changes since
 spinlocks will just use them.

I grant that helps a great deal.  The fact that gcc has added those
atomics makes this a lot more feasible to contemplate that it would be
otherwise. Now, given some of our other experiences with gcc, I am
slightly worried that there may be bugs.  But if that turns out to be
the case we can figure out what to do about it.  It's very nice to at
least have a place to start.

On the other hand, I'm not convinced that we don't need to give any
thought to UNIX vendors that are still pushing their proprietary
compilers.  Many of the old players are dead, but IBM's ICC and HP's
aCC definitely aren't, and I wouldn't be surprised to find one or two
other big ones as well, plus maybe half-a-dozen others that are
clinging to life.

 Which? We only seem to disagree about M32R and m68k, right? I've
 recanted mips and superh days ago ;).
 If you find alpha important, that's fine, that's supported by gcc. I
 just doubt we'll get it even remotely right without tests...

As far as spinlock support goes, you proposed removing VAX, univel,
sinix, sun3, natsemi 32k, superH, ALPHA, m86k, M32R, mips non-GCC,
s390 non-GCC, and 32bit/v9 sparc.  I agreed unequivocally with 4 of
those (though I don't see the code you're talking about wrt/32bitv9
sparc), and you withdrew 2 of those suggestions, so I think there are
6 that are still in doubt: vax, univel, ALPHA, m32r, m68k, and s390
non-GCC.

 It's hard to say where to draw the line here.  I don't want the
 illusion of support for platforms that don't in fact have a prayer of
 working to prevent us from making needed improvements.  On the other
 hand, I also don't want to blithely rip out support for architectures
 that people may well still be using and where, in some cases, people
 have gone out of their way to add that support.  If we get to the
 point where some relatively-obscure architecture is the only thing
 standing between us and improvement X, then we can weigh those things
 against each other and decide.  But I don't really want to go rip out
 support for half-a-dozen semi-supported architectures without some
 clear goal in mind.  That just doesn't seem friendly.

 Well, I only started to look at this somewhat seriously because more and
 more people in the last year or so, both onlist and towards 2ndq were
 complaining about massive spinlock contention (up to 97% spent in
 s_lock) on somewhat bigger machines. The primary offender in many
 workloads is the lwlock internal spinlock, quite often for LW_SHARED
 acquisition where we shouldn't need to block.

 That triggered developing the wait-free LW_SHARED patch
 http://www.postgresql.org/message-id/20130926225545.gb26...@awork2.anarazel.de
 which indeed shows quite some promise. Unfortunately it pretty
 fundamentally requires compare_swap and fetch_add. Now we could just
 implement lwlocks in two pretty much independent ways, but that seems to
 be pretty bad from a maintainability POV.

 The next big thing after that is getting rid of spinlocks around buffer
 pinning (and by that in buffer hdr locking). That would end up in quite
 some #ifdef's sprinkled around.

I do understand that it's going to be painful to carry multiple
implementations of some of these facilities.  But at least from where
I sit I'm not sure we really have a choice.  If we have a

Re: [HACKERS] Auto-tuning work_mem and maintenance_work_mem

2013-10-16 Thread Andrew Dunstan


On 10/09/2013 11:06 AM, Andrew Dunstan wrote:




The assumption that each connection won't use lots of work_mem is also 
false, I think, especially in these days of connection poolers.






Andres has just been politely pointing out to me that my knowledge of 
memory allocators is a little out of date (i.e. by a decade or two), and 
that this memory is not in fact likely to be held for a long time, at 
least on most modern systems. That undermines completely my reasoning above.


Given that, it probably makes sense for us to be rather more liberal in 
setting work_mem that I was suggesting.


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] Auto-tuning work_mem and maintenance_work_mem

2013-10-16 Thread Bruce Momjian
On Wed, Oct 16, 2013 at 04:25:37PM -0400, Andrew Dunstan wrote:
 
 On 10/09/2013 11:06 AM, Andrew Dunstan wrote:
 
 
 
 The assumption that each connection won't use lots of work_mem is
 also false, I think, especially in these days of connection
 poolers.
 
 
 
 
 Andres has just been politely pointing out to me that my knowledge
 of memory allocators is a little out of date (i.e. by a decade or
 two), and that this memory is not in fact likely to be held for a
 long time, at least on most modern systems. That undermines
 completely my reasoning above.
 
 Given that, it probably makes sense for us to be rather more liberal
 in setting work_mem that I was suggesting.

Ah, yes, this came up last year (MMAP_THRESHOLD):

http://www.postgresql.org/message-id/20120730161416.gb10...@momjian.us

-- 
  Bruce Momjian  br...@momjian.ushttp://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] [PATCH] pg_sleep(interval)

2013-10-16 Thread Kevin Grittner
Robert Haas robertmh...@gmail.com wrote:

 Anyone who actually wants this in their environment can
 add the overloaded function in their environment with a one-line SQL
 function: pg_sleep(interval) as proposed here is just
 pg_sleep(extract(epoch from now() + $1) - extract(epoch from now())).

You're making it sound way harder than it is.  Why not just:

create function my_sleep(delay interval)
  returns void language sql
  as 'select pg_sleep(extract(epoch from $1))';

... or,  of course, named to match the existing function.

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


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


Re: [HACKERS] removing old ports and architectures

2013-10-16 Thread Andres Freund
On 2013-10-16 16:10:18 -0400, Robert Haas wrote:
 On the other hand, I'm not convinced that we don't need to give any
 thought to UNIX vendors that are still pushing their proprietary
 compilers.  Many of the old players are dead, but IBM's ICC and HP's
 aCC definitely aren't, and I wouldn't be surprised to find one or two
 other big ones as well, plus maybe half-a-dozen others that are
 clinging to life.

acc provides intrinsics for IA64, so that's easily supported. For IBM,
do you mean XLC? If so, that provides intrinsics as well. So does sun
studio.
ICC as in Intel's compiler provides intrinsics as well. In fact, that's
where gcc cribbed most of it's API from.

  Which? We only seem to disagree about M32R and m68k, right? I've
  recanted mips and superh days ago ;).
  If you find alpha important, that's fine, that's supported by gcc. I
  just doubt we'll get it even remotely right without tests...
 
 As far as spinlock support goes, you proposed removing VAX, univel,
 sinix, sun3, natsemi 32k, superH, ALPHA, m86k, M32R, mips non-GCC,
 s390 non-GCC, and 32bit/v9 sparc.  I agreed unequivocally with 4 of
 those (though I don't see the code you're talking about wrt/32bitv9
 sparc), and you withdrew 2 of those suggestions, so I think there are
 6 that are still in doubt: vax, univel, ALPHA, m32r, m68k, and s390
 non-GCC.

Well, you didn't sound like you deemed vax and alpha to be that
important and I was only talking about architectures, not OSs...

But anyway hardware architecture wise, all but m32r, m68k and pa-risc
have the required hardware support for atomic add and cmpxchg.

univel/unixware: Supports only x86 (these days at least), so writing the
required assembly is trivial if it comes to that. Testing on the other hand...

s390, s390x: We only support linux anyway, so I don't see the
restriction to gcc being problematic.

alpha: We use gcc inline assembly currently, so it's only gcc again. It
is supported by gcc's intrinsics. We can easily support it if we trust
ourselves to understand the cache coherency.

mips: Besides gcc we only support IRIX, since you voted to remove that,
the restriction to gcc doesn't cost anything.

m68k: Only coldfire chips don't have working CAS. All support atomic add.
m32r: Doesn't have CAS, doesn't support atomic add.
pa-risc: Doesn't have CAS, doesn't support atomic add.


 I do understand that it's going to be painful to carry multiple
 implementations of some of these facilities.  But at least from where
 I sit I'm not sure we really have a choice.  If we have a
 --disable-atomics option, there's no reason we can't have regular
 buildfarm coverage of that code; broken lwlocks tend to make things
 die pretty horribly, so I'm relatively confident bugs will show up.

I am pretty sure lots of that code will only be noticeably under
noticeable concurrency. And we don't exercise that all that much in the
regression tets.

Greetings,

Andres Freund

-- 
 Andres Freund 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] better atomics

2013-10-16 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes:
 On 2013-10-16 14:30:34 -0400, Robert Haas wrote:
 But _and, _or are really useful because they can be used to atomically
 clear and set flag bits.

 Until we have some concrete application that requires this
 functionality for adequate performance, I'd be inclined not to bother.
 I think we have bigger fish to fry, and (to extend my nautical
 metaphor) trying to get this working everywhere might amount to trying
 to boil the ocean.

 Well, I actually have a very, very preliminary patch using them in the
 course of removing the spinlocks in PinBuffer() (via LockBufHdr()).

I think we need to be very, very wary of making our set of required
atomics any larger than it absolutely has to be.  The more stuff we
require, the closer we're going to be to making PG a program that only
runs well on Intel.  I am not comforted by the gcc will provide good
implementations of the atomics everywhere argument, because in fact
it won't.  See for example the comment associated with our only existing
use of gcc atomics:

 * On ARM, we use __sync_lock_test_and_set(int *, int) if available, and if
 * not fall back on the SWPB instruction.  SWPB does not work on ARMv6 or
 * later, so the compiler builtin is preferred if available.  Note also that
 * the int-width variant of the builtin works on more chips than other widths.
   ^^

That's not a track record that should inspire much faith that complete
sets of atomics will exist elsewhere.  What's more, we don't just need
atomics that *work*, we need atomics that are *fast*, else the whole
exercise turns into pessimization not improvement.  The more atomic ops
we depend on, the more likely it is that some of them will involve kernel
support on some chips, destroying whatever performance improvement we
hoped to get.

 ... The only thing I'd touch around platforms in that patch is
 adding a generic fallback pg_atomic_test_and_set() to s_lock.h and
 remove the special case usages of __sync_lock_test_and_set() (Arm64,
 some 32 bit arms).

Um, if we had a generic pg_atomic_test_and_set(), s_lock.h would be
about one line long.  The above quote seems to me to be exactly the kind
of optimism that is not warranted in this area.

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] Auto-tuning work_mem and maintenance_work_mem

2013-10-16 Thread Claudio Freire
On Wed, Oct 16, 2013 at 5:30 PM, Bruce Momjian br...@momjian.us wrote:
 On Wed, Oct 16, 2013 at 04:25:37PM -0400, Andrew Dunstan wrote:

 On 10/09/2013 11:06 AM, Andrew Dunstan wrote:
 
 
 
 The assumption that each connection won't use lots of work_mem is
 also false, I think, especially in these days of connection
 poolers.
 
 


 Andres has just been politely pointing out to me that my knowledge
 of memory allocators is a little out of date (i.e. by a decade or
 two), and that this memory is not in fact likely to be held for a
 long time, at least on most modern systems. That undermines
 completely my reasoning above.

 Given that, it probably makes sense for us to be rather more liberal
 in setting work_mem that I was suggesting.

 Ah, yes, this came up last year (MMAP_THRESHOLD):

 http://www.postgresql.org/message-id/20120730161416.gb10...@momjian.us


Beware of depending on that threshold. It varies wildly among platforms.

I've seen implementations with the threshold well above 64MB.


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


Re: [HACKERS] better atomics

2013-10-16 Thread Andres Freund
On 2013-10-16 22:39:07 +0200, Tom Lane wrote:
 Andres Freund and...@2ndquadrant.com writes:
  On 2013-10-16 14:30:34 -0400, Robert Haas wrote:
  But _and, _or are really useful because they can be used to atomically
  clear and set flag bits.
 
  Until we have some concrete application that requires this
  functionality for adequate performance, I'd be inclined not to bother.
  I think we have bigger fish to fry, and (to extend my nautical
  metaphor) trying to get this working everywhere might amount to trying
  to boil the ocean.
 
  Well, I actually have a very, very preliminary patch using them in the
  course of removing the spinlocks in PinBuffer() (via LockBufHdr()).
 
 I think we need to be very, very wary of making our set of required
 atomics any larger than it absolutely has to be.  The more stuff we
 require, the closer we're going to be to making PG a program that only
 runs well on Intel.

Well, essentially I am advocating to support basically three operations:
* compare and swap
* atomic add (and by that sub)
* test and set

The other operations are just porcelain around that.

With compare and swap both the others can be easily implemented if
neccessary.

Note that e.g. linux - running on all platforms we're talking about but
VAX - exensively and unconditionally uses atomic operations widely. So I
think we don't have to be too afraid about non-intel performance here.

  I am not comforted by the gcc will provide good
 implementations of the atomics everywhere argument, because in fact
 it won't.  See for example the comment associated with our only existing
 use of gcc atomics:
 
  * On ARM, we use __sync_lock_test_and_set(int *, int) if available, and if
  * not fall back on the SWPB instruction.  SWPB does not work on ARMv6 or
  * later, so the compiler builtin is preferred if available.  Note also that
  * the int-width variant of the builtin works on more chips than other widths.
^^

 That's not a track record that should inspire much faith that complete
 sets of atomics will exist elsewhere.  What's more, we don't just need
 atomics that *work*, we need atomics that are *fast*, else the whole
 exercise turns into pessimization not improvement.  The more atomic ops
 we depend on, the more likely it is that some of them will involve kernel
 support on some chips, destroying whatever performance improvement we
 hoped to get.

Hm. I can't really follow. We *prefer* to use __sync_lock_test_and_set
in contrast to our own open-coded implementation, right? And the comment
about some hardware only supporting int-width matches that I only want to
require u32 atomics support, right?

I completely agree that we cannot rely on 8byte math or similar (16byte
cmpxchg) to be supported by all platforms. That indeed would require
kernel fallbacks on e.g. ARM.

  ... The only thing I'd touch around platforms in that patch is
  adding a generic fallback pg_atomic_test_and_set() to s_lock.h and
  remove the special case usages of __sync_lock_test_and_set() (Arm64,
  some 32 bit arms).
 
 Um, if we had a generic pg_atomic_test_and_set(), s_lock.h would be
 about one line long.  The above quote seems to me to be exactly the kind
 of optimism that is not warranted in this area.

Well, I am somewhat hesitant to change s_lock for more platforms than
necessary. So I proposed to restructure it in a way that leaves all
existing implementations in place that do not already rely on
__sync_lock_test_and_set().

There's also SPIN_DELAY btw, which I don't see any widely provided
intrinsics for.

Greetings,

Andres Freund

-- 
 Andres Freund 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] removing old ports and architectures

2013-10-16 Thread Andres Freund
On 2013-10-16 16:10:18 -0400, Robert Haas wrote:
 (though I don't see the code you're talking about wrt/32bitv9
 sparc)

 v9 sparc doesn't support compare-and-swap like operations, that's the
background.

Greetings,

Andres Freund

-- 
 Andres Freund 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] Auto-tuning work_mem and maintenance_work_mem

2013-10-16 Thread Josh Berkus
On 10/16/2013 01:25 PM, Andrew Dunstan wrote:
 Andres has just been politely pointing out to me that my knowledge of
 memory allocators is a little out of date (i.e. by a decade or two), and
 that this memory is not in fact likely to be held for a long time, at
 least on most modern systems. That undermines completely my reasoning
 above.

Except that Opensolaris and FreeBSD still have the old memory allocation
behavior, as do older Linux kernels, many of which will remain in
production for years.  I have no idea what Windows' memory management
behavior is.

So this is a case of needing to know considerably more than the
available RAM to determine a good setting.

-- 
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] [PATCH] pg_sleep(interval)

2013-10-16 Thread Peter Eisentraut
On 10/16/13 2:40 PM, Robert Haas wrote:
 On Wed, Oct 16, 2013 at 1:26 PM, Josh Berkus j...@agliodbs.com wrote:
 Also, as Tom pointed out, at some point we have to either say we really
 support overloading or we don't.
 
 We clearly do support overloading.  I don't think that's at issue.
 But as we all know, using it can cause formerly unambiguous queries to
 become ambiguous and stop working.

But that's not really what this is.  It's one thing to be wary about
adding foo(bigint, int, smallint) when there are already three other
permutations in the system.  (At least in other languages, compilers
will give you warnings or errors when this creates an ambiguity, so
there's no guessing.)  But the problem here is that if there already is a

foo(type1)

then the proposal to add

foo(type2)

can always be shot down by

But this will break foo('type1val').

That can't be in the spirit of overloading.

The only way to fix this is that at the time when you add foo(type1) you
need to prevent people from being able to call foo('type1val') and
instead require the full syntax foo(type1 'type1val').  The only way to
do that, I think, is to add some other foo(type3) at the same time.
There is just something wrong with that.



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


[HACKERS] FDW API / flow charts for the docs?

2013-10-16 Thread Tomas Vondra
Hi,

I've been experimenting with the new reworked FDW API to get familiar
with it. The postgres_fdw is a great source of knowledge (huge thanks to
Shigeru Hanada, KaiGai Kohei and everyone else who made this happen),
but in the end I had to draw some flow charts in Dia, to understand how
exactly do the functions interact, pass private data etc. in various
scenarios.

Attached is the set of flow charts, showing the sequence of callbacks
for all the supported commands (i.e. SELECT, INSERT, UPDATE, DELETE,
ANALYZE). Wouldn't it be useful to put something like this into the
docs? I mean, the FDW API is not going to get any simpler, and for me
this was a significant help.

regards
Tomas


fdw.dia
Description: application/dia-diagram

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


Re: [HACKERS] FDW API / flow charts for the docs?

2013-10-16 Thread Fabrízio de Royes Mello
On Wed, Oct 16, 2013 at 8:35 PM, Tomas Vondra t...@fuzzy.cz wrote:

 [...]

 Attached is the set of flow charts, showing the sequence of callbacks
 for all the supported commands (i.e. SELECT, INSERT, UPDATE, DELETE,
 ANALYZE).

Thank you very much... this flow charts will help many people, including me
;-)


 Wouldn't it be useful to put something like this into the
 docs? I mean, the FDW API is not going to get any simpler, and for me
 this was a significant help.


+1 to add into docs.

I think we can add this flow charts to [1].

Regards,

[1] http://www.postgresql.org/docs/9.3/interactive/fdwhandler.html

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
 Timbira: http://www.timbira.com.br
 Blog sobre TI: http://fabriziomello.blogspot.com
 Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
 Twitter: http://twitter.com/fabriziomello


Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-16 Thread Robert Haas
On Wed, Oct 16, 2013 at 4:34 PM, Kevin Grittner kgri...@ymail.com wrote:
 Robert Haas robertmh...@gmail.com wrote:

 Anyone who actually wants this in their environment can
 add the overloaded function in their environment with a one-line SQL
 function: pg_sleep(interval) as proposed here is just
 pg_sleep(extract(epoch from now() + $1) - extract(epoch from now())).

 You're making it sound way harder than it is.  Why not just:

 create function my_sleep(delay interval)
   returns void language sql
   as 'select pg_sleep(extract(epoch from $1))';

 ... or,  of course, named to match the existing function.

Because that might or might not do the right thing if the interval is 1 month.

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


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


[HACKERS] libpgport vs libpgcommon

2013-10-16 Thread Peter Eisentraut
I wonder whether it was ever consciously decided what the dependency
relationship between libpgport and libpgcommon would be.  When I added
asprintf(), I had intuitively figured that libpgport would be the lower
layer, and so psprintf() in libpgcommon depends on vasprintf() in
libpgport.  I still think that is sound.  But working through the
buildfarm issues now it turns out that wait_result_to_str() in libpgport
depends on pstrdup() in libpgcommon.  That doesn't seem ideal.  I think
in this case we could move wait_error.c to libpgcommon.  But I would
like to know what the consensus on the overall setup is.



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


Re: [HACKERS] LDAP: bugfix and deprecated OpenLDAP API

2013-10-16 Thread Peter Eisentraut
On Tue, 2013-09-24 at 15:07 +, Albe Laurenz wrote:
 --- 3511,3534 
   }
   
   /*
 !  * Perform an explicit anonymous bind.
 !  * This is not necessary in principle, but we want to set a timeout
 !  * of PGLDAP_TIMEOUT seconds and return 2 if the connection fails.
 !  * Unfortunately there is no standard conforming way to do that.
*/

This comment has become a bit confusing.  What exactly is nonstandard?
Setting a timeout, or returning 2?  The code below actually returns 3.

 + #ifdef HAVE_LIBLDAP
 + /* in OpenLDAP, use the LDAP_OPT_NETWORK_TIMEOUT option */

We don't use HAVE_LIBLDAP anywhere else to mean OpenLDAP.  Existing
LDAP-related code uses #ifdef WIN32.

 + #else

There should be a comment here indicating what this #else belongs to
(#else /* HAVE_LIBLDAP */, or whatever we end up using).

 + /* the nonstandard ldap_connect function performs an anonymous bind */
 + if (ldap_connect(ld, time) != LDAP_SUCCESS)
 + {
 + /* error or timeout in ldap_connect */
 + free(url);
 + ldap_unbind(ld);
 + return 2;
 + }
 + #endif

here too

Bonus: Write a commit message for your patch.  (Consider using git
format-patch.)




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


Re: [HACKERS] libpgport vs libpgcommon

2013-10-16 Thread Noah Misch
On Wed, Oct 16, 2013 at 09:41:20PM -0400, Peter Eisentraut wrote:
 I wonder whether it was ever consciously decided what the dependency
 relationship between libpgport and libpgcommon would be.  When I added
 asprintf(), I had intuitively figured that libpgport would be the lower
 layer, and so psprintf() in libpgcommon depends on vasprintf() in
 libpgport.  I still think that is sound.  But working through the
 buildfarm issues now it turns out that wait_result_to_str() in libpgport
 depends on pstrdup() in libpgcommon.  That doesn't seem ideal.  I think
 in this case we could move wait_error.c to libpgcommon.  But I would
 like to know what the consensus on the overall setup is.

Interesting.  I, too, would have figured that libpgport is lower-level,
because any higher-level library might need the libc functions it replaces.
Moving wait_error.c to libpgcommon makes sense.  dirmod.c perhaps deserves a
split into libpgcommon parts (e.g. pgfnames()) and libpgport parts
(e.g. pgrename()).  Hopefully there's not much more.

Thanks,
nm

-- 
Noah Misch
EnterpriseDB http://www.enterprisedb.com


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


Re: [HACKERS] FDW API / flow charts for the docs?

2013-10-16 Thread Alvaro Herrera
Tomas Vondra wrote:

 Attached is the set of flow charts, showing the sequence of callbacks
 for all the supported commands (i.e. SELECT, INSERT, UPDATE, DELETE,
 ANALYZE). Wouldn't it be useful to put something like this into the
 docs? I mean, the FDW API is not going to get any simpler, and for me
 this was a significant help.

Please see this thread
http://www.postgresql.org/message-id/4bb9e69f.9080...@usit.uio.no

-- 
Álvaro Herrerahttp://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] psql tab completion for updatable foreign tables

2013-10-16 Thread Peter Eisentraut
On Mon, 2013-07-08 at 16:04 +, Dean Rasheed wrote:
 There was concern that pg_relation_is_updatable() would end up opening
 every relation in the database, hammering performance. I now realise
 that these tab-complete queries have a limit (1000 by default) so I
 don't think this is such an issue. I tested it by creating 10,000
 randomly named auto-updatable views on top of a table, and didn't see
 any performance problems.

Even if performance isn't a problem, do we really want tab completion
interfering with table locking?  That might be a step too far.

Personally, I think this is too fancy anyway.  I'd just complete all
views and foreign tables and be done with it.  We don't inspect
permissions either, for example.  This might be too confusing for users.




-- 
Sent 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] Add an ldapoption to disable chasing LDAP referrals

2013-10-16 Thread James Sewell
Hey All,

I had missed these emails, sorry.

The search+bind mode issue is one of documentation location, I have fixed
it by moving the section to the applied to both list. As the patch is to do
with post-auth response this is correct.

As far as the issue when something other than 0 or 1 is set I am happy
throw an error (although this doesn't seem to be how option such as LDAPTLS
work: 1 if 1 else 0). I assume I would use the ereport() function to do
this (using the second example from this page
http://www.postgresql.org/docs/9.2/static/error-message-reporting.html)?

Cheers,
James


James Sewell,
PostgreSQL Team Lead / Solutions Architect
__


 Level 2, 50 Queen St, Melbourne VIC 3000

*P *(+61) 3 8370 8000 * **W* www.lisasoft.com  *F *(+61) 3 8370 8099



On Thu, Sep 19, 2013 at 12:56 AM, Peter Eisentraut pete...@gmx.net wrote:

 On 7/8/13 9:33 PM, James Sewell wrote:
  New patch attached. I've moved from using a boolean to an enum trivalue.

 When ldapreferrals is set to something other than 0 or 1 exactly, it
 just ignores the option.  That's not good, I think.  It should probably
 be an error.



-- 


--
The contents of this email are confidential and may be subject to legal or 
professional privilege and copyright. No representation is made that this 
email is free of viruses or other defects. If you have received this 
communication in error, you may not copy or distribute any part of it or 
otherwise disclose its contents to anyone. Please advise the sender of your 
incorrect receipt of this correspondence.


Re: [HACKERS] Patch for reserved connections for replication users

2013-10-16 Thread Amit Kapila
On Wed, Oct 16, 2013 at 4:30 AM, Gibheer gibh...@zero-knowledge.org wrote:
 On Mon, 14 Oct 2013 11:52:57 +0530
 Amit Kapila amit.kapil...@gmail.com wrote:

 On Sun, Oct 13, 2013 at 2:08 PM, Gibheer gibh...@zero-knowledge.org
 wrote:
  On Sun, 13 Oct 2013 11:38:17 +0530
  Amit Kapila amit.kapil...@gmail.com wrote:
 
  On Thu, Oct 10, 2013 at 3:17 AM, Gibheer
  gibh...@zero-knowledge.org wrote:
   On Mon, 7 Oct 2013 11:39:55 +0530
   Amit Kapila amit.kapil...@gmail.com wrote:
   Robert Haas wrote:
   On Mon, Aug 5, 2013 at 2:04 AM, Andres Freund
   andres(at)2ndquadrant(dot)com wrote:
Hmm.  It seems like this match is making MaxConnections no
longer mean the maximum number of connections, but rather
the maximum number of non-replication connections.  I don't
think I support that definitional change, and I'm kinda
surprised if this is sufficient to implement it anyway
(e.g. see InitProcGlobal()).
   
I don't think the implementation is correct, but why don't
you like the definitional change? The set of things you can
do from replication connections are completely different
from a normal connection. So using separate pools for them
seems to make sense. That they end up allocating similar
internal data seems to be an implementation detail to me.
  
Because replication connections are still connections.  If I
tell the system I want to allow 100 connections to the server,
it should allow 100 connections, not 110 or 95 or any other
number.
  
   I think that to reserve connections for replication, mechanism
   similar to superuser_reserved_connections be used rather than
   auto vacuum workers or background workers.
   This won't change the definition of MaxConnections. Another
   thing is that rather than introducing new parameter for
   replication reserved connections, it is better to use
   max_wal_senders as it can serve the purpose.
  
   Review for replication_reserved_connections-v2.patch,
   considering we are going to use mechanism similar to
   superuser_reserved_connections and won't allow definition of
   MaxConnections to change.
  
 
  
   Hi,
  
   I took the time and reworked the patch with the feedback till
   now. Thank you very much Amit!
  
   So this patch uses max_wal_senders together with the idea of the
   first patch I sent. The error messages are also adjusted to make
   it obvious, how it is supposed to be and all checks work, as far
   as I could tell.
 
  If I understand correctly, now the patch has implementation such
  that a. if the number of connections left are (ReservedBackends +
  max_wal_senders), then only superusers or replication connection's
  will be allowed
  b. if the number of connections left are ReservedBackend, then only
  superuser connections will be allowed.
 
  That is correct.
 
  So it will ensure that max_wal_senders is used for reserving
  connection slots from being used by non-super user connections. I
  find new usage of max_wal_senders acceptable, if anyone else thinks
  otherwise, please let us know.
 
 
  1.
  +varnamesuperuser_reserved_connections/varname
  +varnamemax_wal_senders/varname only superuser and WAL
  connections
  +are allowed.
 
  Here minus seems to be missing before max_wal_senders and I think
  it will be better to use replication connections rather than WAL
  connections.
 
  This is fixed.
 
  2.
  -new replication connections will be accepted.
  +new WAL or other connections will be accepted.
 
  I think as per new implementation, we don't need to change this
  line.
 
  I reverted that change.
 
  3.
  + * reserved slots from max_connections for wal senders. If the
  number of free
  + * slots (max_connections - max_wal_senders) is depleted.
 
   Above calculation (max_connections - max_wal_senders) needs to
  include super user reserved connections.
 
  My first thought was, that I would not add it here. When superuser
  reserved connections are not set, then only max_wal_senders would
  count.
  But you are right, it has to be set, as 3 connections are reserved
  by default for superusers.

 + * slots (max_connections - superuser_reserved_connections -
 max_wal_senders) here it should be ReservedBackends rather than
 superuser_reserved_connections.

 fixed

  4.
  + /*
  + * Although replication connections currently require superuser
  privileges, we
  + * don't allow them to consume the superuser reserved slots, which
  are
  + * intended for interactive use.
*/
if ((!am_superuser || am_walsender) 
ReservedBackends  0 
!HaveNFreeProcs(ReservedBackends))
ereport(FATAL,
(errcode(ERRCODE_TOO_MANY_CONNECTIONS),
  - errmsg(remaining connection slots are reserved for
  non-replication superuser connections)));
  + errmsg(remaining connection slots are reserved for superuser
  connections)));
 
  Will there be any problem if we do the above check before the check
  for wal senders and reserved replication 

Re: [HACKERS] FDW API / flow charts for the docs?

2013-10-16 Thread Stephen Frost
Alvaro,

* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
 Tomas Vondra wrote:
  Attached is the set of flow charts, showing the sequence of callbacks
  for all the supported commands (i.e. SELECT, INSERT, UPDATE, DELETE,
  ANALYZE). Wouldn't it be useful to put something like this into the
  docs? I mean, the FDW API is not going to get any simpler, and for me
  this was a significant help.
 
 Please see this thread
 http://www.postgresql.org/message-id/4bb9e69f.9080...@usit.uio.no

The conclusion of that thread appears to be use dia, which was done
here..  Am I missing something there (full disclosure- I haven't looked
at the dia yet)?

Also, for my part, I'd suggest putting it on the wiki initially anyway,
as then it can be seen directly (load it as a png or what-have-you) and
it becomes immediately available to users.  The .dia should also be on
the wiki, of course, and then included in the PG tree eventually if it's
added as part of the official docs.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] removing old ports and architectures

2013-10-16 Thread Noah Misch
On Wed, Oct 16, 2013 at 10:04:29PM +0200, Andres Freund wrote:
 On 2013-10-16 15:49:54 -0400, Bruce Momjian wrote:
  On Sat, Oct 12, 2013 at 06:35:00PM -0700, Peter Geoghegan wrote:
- ALPHA (big pain in the ass to get right, nobody uses it anymore)
   
   Yes, for many years now ALPHA has only been useful as a way of
   illustrating how bad it's possible for CPU memory operation reordering
   considerations to get. So I quite agree.
  
  Are there any optimizations we have avoided, or 'volatile' designations
  added, only for Alpha?
 
 I am somewhat sure that some of the code we added in the last years
 isn't actually correct for alpha (and others actually). It's just that
 nobody actually runs on alpha anymore, so nobody notices.
 
  Could we improve other things if Alpha support was dropped?
 
 I think the major thing is that if we're going to add more algorithms
 that use less locks - which we'll have to, otherwise our scalability
 will get more and more problematic - we'll have to adhere to the
 weakest cache coherency model we support. And at least I am not
 intelligent/experienced enough to blindly write correct code for Alpha.

Removing support for alpha is a different animal compared to removing support
for non-gcc MIPS and most of the others in your list.  A hacker wishing to
restore support for another MIPS compiler would fill in the assembly code
blanks, probably using code right out of an architecture manual.  A hacker
wishing to restore support for alpha would find himself auditing every
lock-impoverished algorithm in the backend.  At the same time, as you suggest,
the benefit to general PostgreSQL development from dropping alpha is greater
in the same way.  Dropping non-gcc MIPS reduces effort to complete the initial
patch that adds the atomic primitives; dropping alpha reduces effort to
implement every future algorithm that uses barriers.  I do think dropping
support for alpha is the right decision.  That's a firm and likely one-way
downgrade of support, much like we've done for compilers with no 64-bit type.

Concerning cases like non-gcc MIPS, I'll mostly echo Tom's comments[1].  I'm
comfortable with the project saying We've added atomics for the architectures
we have.  Help us with the rest!  It's reasonable to introduce an improvement
that entails architecture-dependent code without requiring that the initial
patch and initial author address every interesting target.  The bar to restore
support, even years later, will be pretty low.  In that light, I don't favor
ripping out longstanding architecture-specific code for borderline platforms.
Doing so raises the bar for restoring support, without proximate benefit.

[1] http://www.postgresql.org/message-id/4694.1381676...@sss.pgh.pa.us

-- 
Noah Misch
EnterpriseDB http://www.enterprisedb.com


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