Re: [HACKERS] [PATCH] pg_sleep(interval)
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)
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
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
> On 16-Oct-2013, at 3:45 pm, Andres Freund 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
From: "Andres Freund" I've seen several sites shutting down because of forgotten prepared transactions causing bloat and anti-wraparound shutdowns. From: "Magnus Hagander" 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'
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
On Tue, Oct 15, 2013 at 4:17 PM, Kohei KaiGai 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
On Tue, Oct 15, 2013 at 11:17 PM, Peter Eisentraut 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
On 2013-10-16 08:39:10 -0400, Robert Haas wrote: > On Tue, Oct 15, 2013 at 11:17 PM, Peter Eisentraut 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
On Tue, Oct 15, 2013 at 4:02 PM, Kohei KaiGai 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
On Wed, Oct 16, 2013 at 8:54 AM, Andres Freund wrote: > On 2013-10-16 08:39:10 -0400, Robert Haas wrote: >> On Tue, Oct 15, 2013 at 11:17 PM, Peter Eisentraut 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
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
On Wed, Oct 16, 2013 at 9:37 AM, Andres Freund 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
On 2013-10-16 09:44:32 -0400, Robert Haas wrote: > On Wed, Oct 16, 2013 at 9:37 AM, Andres Freund 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)
* 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
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 : > On Wed, 2013-09-04 at 14:35 +, Robert Haas wrote: >> >> On Fri, Aug 30, 2013 at 3:43 PM, Tom Lane 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 -- Sent 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)
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)
On Wed, Oct 16, 2013 at 11:18 AM, Stephen Frost 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
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
---
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
On Tue, Oct 15, 2013 at 8:33 AM, Andres Freund 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
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
On 2013-10-16 12:26:28 -0400, Robert Haas wrote: > On Tue, Oct 15, 2013 at 8:33 AM, Andres Freund 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
On Sat, Oct 12, 2013 at 8:46 PM, Andres Freund 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/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
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 http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] removing old ports and architectures
On 10/16/2013 07:04 PM, Robert Haas wrote: > On Sat, Oct 12, 2013 at 8:46 PM, Andres Freund 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)
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
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
>> - 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
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
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
> > 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 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, "", 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 > . > > 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 > > tak
Re: [HACKERS] Triggers on foreign tables
2013/10/16 Robert Haas : > On Tue, Oct 15, 2013 at 4:17 PM, Kohei KaiGai 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 -- Sent 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
On Wed, Oct 16, 2013 at 1:52 PM, Andres Freund 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
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 http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] better atomics
On Wed, Oct 16, 2013 at 12:52 PM, Andres Freund 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
On Wed, Oct 16, 2013 at 2:20 PM, Kohei KaiGai 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
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)
On Wed, Oct 16, 2013 at 1:26 PM, Josh Berkus 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
Bruce Momjian 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
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
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 http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Record comparison compiler warning
On Wed, Oct 16, 2013 at 11:49:13AM -0700, Kevin Grittner wrote: > Bruce Momjian 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: default: -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] removing old ports and architectures
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
On Wed, Oct 16, 2013 at 3:26 PM, Andres Freund 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/> 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 --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. Obviously,
Re: [HACKERS] Auto-tuning work_mem and maintenance_work_mem
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
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 http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_sleep(interval)
Robert Haas 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
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/ those (though I don't see the code you're talking about wrt/32bit 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
Andres Freund 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
On Wed, Oct 16, 2013 at 5:30 PM, Bruce Momjian 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
On 2013-10-16 22:39:07 +0200, Tom Lane wrote: > Andres Freund 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
On 2013-10-16 16:10:18 -0400, Robert Haas wrote: > (though I don't see the code you're talking about wrt/32bit 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
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)
On 10/16/13 2:40 PM, Robert Haas wrote:
> On Wed, Oct 16, 2013 at 1:26 PM, Josh Berkus 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?
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?
On Wed, Oct 16, 2013 at 8:35 PM, 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). 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)
On Wed, Oct 16, 2013 at 4:34 PM, Kevin Grittner wrote: > Robert Haas 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
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
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
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?
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
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
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 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
On Wed, Oct 16, 2013 at 4:30 AM, Gibheer wrote: > On Mon, 14 Oct 2013 11:52:57 +0530 > Amit Kapila wrote: > >> On Sun, Oct 13, 2013 at 2:08 PM, Gibheer >> wrote: >> > On Sun, 13 Oct 2013 11:38:17 +0530 >> > Amit Kapila wrote: >> > >> >> On Thu, Oct 10, 2013 at 3:17 AM, Gibheer >> >> wrote: >> >> > On Mon, 7 Oct 2013 11:39:55 +0530 >> >> > Amit Kapila wrote: >> >> >> Robert Haas wrote: >> >> >> On Mon, Aug 5, 2013 at 2:04 AM, Andres Freund >> >> >> 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. >> >> +superuser_reserved_connections >> >> +max_wal_senders 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(Reser
Re: [HACKERS] FDW API / flow charts for the docs?
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
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
