Re: [HACKERS] 10.0
On 20/06/2016 22:41, Alvaro Herrera wrote: > Tom Lane wrote: >> Robert Haas writes: >>> On Mon, Jun 20, 2016 at 4:00 PM, David G. Johnston >>> wrote: >>>> 10.x is the desired output. >> >>> 10.x is the output that some people desire. A significant number of >>> people, including me, would prefer to stick with the current >>> three-part versioning scheme, possibly with some change to the >>> algorithm for bumping the first digit (e.g. every 5 years like >>> clockwork). >> >> If we were going to do it like that, I would argue for "every ten years >> like clockwork", e.g. 10.0.x is next after 9.9.x. But in point of fact, >> Robert, you already made your case for that approach and nobody else >> cared for it. > > I voted for this approach initially too, and I think it has merit -- > notably, that it would stop this discussion. It was said that moving > to two-part numbers would stop all discussion, but it seems to have had > exactly the opposite effect. If voting is still possible, then I agree: no changes please! It won't make things easier to have a 10g or a 10.8 to explain, instead of a 10.0.8... and I'm not sure it'll make things easier to not have the chance to bump the 2 major parts if it happened to be interesting in the future like it was for 7.4->8 and 8.4->9 (9 is «new», it's the first time we go over .4 to bump first digit, but it's also the first time we have found a way to shorten release cycle) -- -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_receivexlog --create-slot-if-not-exists
Le 07/07/2015 14:55, Andres Freund a écrit : > On 2015-06-19 17:21:25 +0200, Cédric Villemain wrote: >>> To make slot usage in pg_receivexlog easier, should we add >>> --create-slot-if-not-exists? That'd mean you could run the same command >>> the first and later invocation. >> >> +1 (with a shorter name please, if you can find one... ) > > How about a seperate --if-not-exists modifier? Right now --create works > as an abbreviation for --create-slot, but --create-slot-if-not-exists > would break that. Having done a quick overview of other binaries provided by postgresql I realized that pg_receivexlog is distinct from other tools creating something at a sql level: for db, role and lang we have create and drop as distinct commands. Both dropdb and dropuser have a '--if-exists' (like in SQL) but have not for create operation. Maybe we should have used createslot/dropslot in the first place and use --if-exists --if-not-exists accordingly. Having all in one tool, adding one or both options looks good to me. The only alternative I see is more like "--force": return success even if the operation was not required (create or drop). -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_receivexlog --create-slot-if-not-exists
Le 19/06/2015 17:21, Andres Freund a écrit : > On 2015-06-19 11:19:23 -0400, Magnus Hagander wrote: >> On Fri, Jun 19, 2015 at 11:17 AM, Stephen Frost wrote: >> >>> * Andres Freund (and...@anarazel.de) wrote: >>>> To make slot usage in pg_receivexlog easier, should we add >>>> --create-slot-if-not-exists? That'd mean you could run the same command >>>> the first and later invocation. >>> >>> Yes, please. >> >> +1. > > Arguably we could do that for 9.5 - the whole slot stuff for receivexlog > is new, and this is just adjusting the API slightly. I won't fight much > for that opinion though. +1 for it in 9.5 -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_receivexlog --create-slot-if-not-exists
> To make slot usage in pg_receivexlog easier, should we add > --create-slot-if-not-exists? That'd mean you could run the same command > the first and later invocation. +1 (with a shorter name please, if you can find one... ) -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation -- Sent 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-vacuum is not running in 9.1.12
Le 17/06/2015 23:10, Alvaro Herrera a écrit : > Tom Lane wrote: >> launcher_determine_sleep() does have a minimum sleep time, and it seems >> like we could fairly cheaply guard against this kind of scenario by also >> enforcing a maximum sleep time (of say 5 or 10 minutes). Not quite >> convinced whether it's worth the trouble though. > > Yeah, the case is pretty weird and I'm not really sure that the server > ought to be expected to behave. But if this is actually the only part > of the server that misbehaves because of sudden gigantic time jumps, I > think it's fair to patch it. Here's a proposed patch. Does that still respect an autovacuum_naptime (GUC) greater than 5 minutes ? -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] checkpointer continuous flushing
Le 07/06/2015 16:53, Fabien COELHO a écrit :
> +» » /*·Others:·say·that·data·should·not·be·kept·in·memory...
> +» » ·*·This·is·not·exactly·what·we·want·to·say,·because·we·want·to·write
> +» » ·*·the·data·for·durability·but·we·may·need·it·later·nevertheless.
> +» » ·*·It·seems·that·Linux·would·free·the·memory·*if*·the·data·has
> +» » ·*·already·been·written·do·disk,·else·it·is·ignored.
> +» » ·*·For·FreeBSD·this·may·have·the·desired·effect·of·moving·the
> +» » ·*·data·to·the·io·layer.
> +» » ·*/
> +» » rc·=·posix_fadvise(context->fd,·context->offset,·context->nbytes,
> +» » » » » » ···POSIX_FADV_DONTNEED);
> +
It looks a bit hazardous, do you have a benchmark for freeBSD ?
Sources says:
case POSIX_FADV_DONTNEED:
/*
* Flush any open FS buffers and then remove pages
* from the backing VM object. Using vinvalbuf() here
* is a bit heavy-handed as it flushes all buffers for
* the given vnode, not just the buffers covering the
* requested range.
--
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fillfactor for GIN indexes
Le samedi 17 janvier 2015 08:23:44 Michael Paquier a écrit : > On Sat, Jan 17, 2015 at 2:40 AM, Robert Haas wrote: > > There's only value in adding a fillfactor parameter to GIN indexes > > if > > it improves performance. There are no benchmarks showing it does. > > So, why are we still talking about this? > > Indeed. There is no such benchmark as of now, and I am not sure I'll > get the time to work more on that soon, so let's reject the patch for > now. And sorry for the useless noise. Michael, I first didn't understood how GiN can benefits from the patch...thus my question. There were no noise for me, and I learn some more about GiN. So I thank you for your work! -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Fillfactor for GIN indexes
Le lundi 19 janvier 2015 08:24:08 Robert Haas a écrit : > On Sat, Jan 17, 2015 at 4:49 AM, Alexander Korotkov > > wrote: > > I already wrote quite detailed explanation of subject. Let mel try > > to > > explain in shortly. GIN is two level nested btree. Thus, GIN would > > have absolutely same benefits from fillfactor as btree. Lack of > > tests showing it is, for sure, fault. > > > > However, GIN posting trees are ordered by ItemPointer and this makes > > some specific. If you have freshly created table and do > > inserts/updates they would use the end of heap. Thus, inserts would > > go to the end of GIN posting tree and fillfactor wouldn't affect > > anything. Fillfactor would give benefits on HOT or heap space > > re-usage. > > Ah, OK. Those tests clarify things considerably; I see the point now. So I do. Alexander said: 1) In order to have fully correct support of fillfactor in GIN we need to rewrite GIN build algorithm. 2) Without rewriting GIN build algorithm, not much can be done with entry tree. However, you can implement some heuristics. The patch is 2), for the posting tree only ? Thanks! -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Fillfactor for GIN indexes
Le jeudi 27 novembre 2014 23:33:11 Michael Paquier a écrit : > On Fri, Nov 21, 2014 at 2:12 PM, Michael Paquier > > wrote: > > I am adding that to the commit fest of December. > > Here are updated patches. Alvaro notified me about an inconsistent > comment. what are the benefits of this patch ? (maybe you had some test case or a benchmark ?) -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Using Index-only scans to speed up count(*)
Le samedi 7 juin 2014 09:09:00 Gurjeet Singh a écrit : > On Sat, Jun 7, 2014 at 8:56 AM, Cédric Villemain wrote: > > Le samedi 7 juin 2014 08:35:27 Gurjeet Singh a écrit : > >> PS: Please note that I am not proposing to add support for the > >> optimizer hint embedded in Mitsuru's query. > >> > > :-) > > Even though I (sometimes) favor hints, and developed the optimizer > hints feature in EDB (PPAS), I know how much Postgres **hates** [1] > optimizer hints :) So just trying to wade off potential flamewar-ish > comments. There is a large benefits to users in preventing HINT in core: it makes it mandatory for PostgreSQL to keep improving, and it makes it mandatory for developers to find solutions around this constraint. There is at least planner_hint contribution (search Oleg website), and added to a postgresql hook on the parser you're done implementing HINT in userland. I'm not arguing pro/cons about the feature (as you said the question has been answered already) but arguing that arbitrary constraints challenge us and produces good things for PostgreSQL in return. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Using Index-only scans to speed up count(*)
Le samedi 7 juin 2014 08:35:27 Gurjeet Singh a écrit : > While reading [1] in context of Postgres Hibernator, I see that > Mitsuru mentioned one of the ways other RDBMS allows count(*) to be > driven by an index. > > > 'select /*+ INDEX(emp emp_pk) */ count(*) from emp;' to load index > > blocks > I am not sure if Postgres planner already allows this, but it would be > great if the planner considered driving a count(*) query using a > non-partial index, in the hopes that it turns into an index-only > scan, and hence returns count(*) result faster.The non-partial index > may not necessarily be the primary key index, it can be chosen purely > based on size, favouring smaller indexes. IIRC it is not (yet) possible to switch from index-scan to indexonly- scan on the fly because the structure used are different (indexonly scan needs to prepare a tuple struct to hold data, I'm not sure of the details). Indexonly scan is already used to answer count(*) but decision is done during planning. Now, it happens that this is an interesting idea which has already been discussed if not on postgresql-hacker at least during pre/post- conferences social events: being able to switch the plan during execution if things are not working as expected (in the same topic you have 'progress bar' for query execution, at least some mechanisms should be shared by both features). > PS: Please note that I am not proposing to add support for the > optimizer hint embedded in Mitsuru's query. :-) -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Proposing pg_hibernate
Le lundi 3 février 2014 19:18:54 Gurjeet Singh a écrit : > Possible enhancements: > - Ability to save/restore only specific databases. > - Control how many BlockReaders are active at a time; to avoid I/O > storms. - Be smart about lowered shared_buffers across the restart. > - Different modes of reading like pg_prewarm does. > - Include PgFincore functionality, at least for Linux platforms. Please note that pgfincore is working on any system where PostgreSQL prefetch is working, exactly like pg_prewarm. This includes linux, BSD and many unix-like. It *is not* limited to linux. I never had a single request for windows, but windows does provides an API for that too (however I have no windows offhand to test). Another side note is that currently BSD (at least freeBSD) have a more advanced mincore() syscall than linux and offers a better analysis (dirty status is known) and they implemented posix_fadvise... PS: There is a previous thread about that hibernation feature. Mitsuru IWASAKI did a patch, and it triggers some interesting discussions. Some notes in this thread are outdated now, but it's worth having a look at it: http://www.postgresql.org/message-id/20110504.231048.113741617.iwas...@jp.freebsd.org https://commitfest.postgresql.org/action/patch_view?idT9 -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] alternative back-end block formats
Le lundi 27 janvier 2014 13:42:29 Christian Convey a écrit : > On Sun, Jan 26, 2014 at 5:47 AM, Craig Ringer wrote: > > On 01/21/2014 07:43 PM, Christian Convey wrote: > > > Does anyone know if this has been done before with Postgres? I > > > would > > > have assumed yes, but I'm not finding anything in Google about > > > people > > > having done this. > > > > AFAIK (and I don't know much in this area) the storage manager isn't > > very pluggable compared to the rest of Pg. > > Thanks for the warning. Duly noted. As written in the meeting notes, Tom Lane revealed an interest in pluggable storage. So it might be interesting to check that. https://wiki.postgresql.org/wiki/PgCon_2013_Developer_Meeting -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] pg_rewarm status
Le jeudi 19 décembre 2013 03:08:59, Robert Haas a écrit : > On Wed, Dec 18, 2013 at 6:07 PM, Cédric Villemain wrote: > >> When the prefetch process starts up, it services requests from the > >> queue by reading the requested blocks (or block ranges). When the > >> queue is empty, it sleeps. If it receives no requests for some period > >> of time, it unregisters itself and exits. This is sort of a souped-up > >> version of the hibernation facility we already have for some auxiliary > >> processes, in that we don't just make the process sleep for a longer > >> period of time but actually get rid of it altogether. > > > > I'm just a bit skeptical about the starting time: backend will ReadBuffer > > very soon after requesting the Prefetch... > > Yeah, absolutely. The first backend that needs a prefetch probably > isn't going to get it in time. I think that's OK though. Once the > background process is started, response times will be quicker... > although possibly still not quick enough. We'd need to benchmark this > to determine how quickly the background process can actually service > requests. Does anybody have a good self-contained test case that > showcases the benefits of prefetching? Bitmap heap fetch, I haven't a selfcase here. I didn't CC Greg but I'm sure he has the material your asking. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: SQL objects UNITs (was: [HACKERS] Extension Templates S03E11)
Le jeudi 19 décembre 2013 14:01:17, Robert Haas a écrit :
> On Wed, Dec 18, 2013 at 10:05 AM, Alvaro Herrera
>
> wrote:
> > Stephen Frost escribió:
> >> * Dimitri Fontaine (dimi...@2ndquadrant.fr) wrote:
> >> > Basically with building `UNIT` we realise with hindsight that we
> >> > failed to build a proper `EXTENSION` system, and we send that message
> >> > to our users.
> >>
> >> Little difficult to draw conclusions about what out 'hindsight' will
> >> look like.
> >
> > I haven't been keeping very close attention to this, but I fail to see
> > why extensions are so much of a failure. Surely we can invent a new
> > "kind" of extensions, ones whose contents specifically are dumped by
> > pg_dump. Regular extensions, the kind we have today, still wouldn't,
> > but we could have a flag, say "CREATE EXTENSION ... (WITH DUMP)" or
> > something. That way you don't have to come up with UNIT at all (or
> > whatever). A whole new set of catalogs just to fix up a minor issue
> > with extensions sounds a bit too much to me; we can just add this new
> > thing on top of the existing infrastructure.
>
> Yep.
>
> I'm not very convinced that extensions are a failure. I've certainly
> had plenty of good experiences with them, and I think others have as
> well, so I believe Dimitri's allegation that we've somehow failed here
> is overstated. That having been said, having a flag we can set to
> dump the extension contents normally rather than just dumping a CREATE
> EXTENSION statement seems completely reasonable to me.
>
> ALTER EXTENSION foo SET (dump_members = true/false);
>
> It's even got use cases outside of what Dimitri wants to do, like
> dumping and restoring an extension that you've manually modified
> without losing your changes.
Isn't there some raw SQL extension author are supposed to be able to push in
order to dump partial configuration table and similar things (well, what we're
supposed to be able to change in an extension).
yes, it is:
SELECT pg_catalog.pg_extension_config_dump('my_config', 'WHERE NOT
standard_entry');
(it is raw SQL here, but it is not appreciated for Extension 'Templates' ....
I stopped trying to figure/undertand many arguments in those Extension email
threads)
Maybe something around that to have also the objects created by extension
dumped, and we're done. I even wnder if Dimitri has not already a patch for
that based on the work done for Extensions feature.
--
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation
signature.asc
Description: This is a digitally signed message part.
Re: [HACKERS] pg_rewarm status
Le mercredi 18 décembre 2013 18:40:09, Robert Haas a écrit : > Now that we have dynamic background workers, I've been thinking that > it might be possible to write a background worker to do asynchronous > prefetch on systems where we don't have OS support. We could store a > small ring buffer in shared memory, say big enough for 1k entries. > Each entry would consist of a relfilenode, a starting block number, > and a block count. We'd also store a flag indicating whether the > prefetch worker has been registered with the postmaster, and a pointer > to the PGPROC of any running worker. When a process wants to do a > prefetch, it locks the buffer, adds its prefetch request to the queue > (overwriting the oldest existing request if the queue is already > full), and checks the flag. If the flag is not set, it also registers > the background worker. Then, it releases the lock and sets the latch > of any running worker (whose PGPROC it remembered before releasing the > lock). Good idea. If the list is full it is probably that the system is busy, I suppose that in such case some alternative behavior can be interesting. Perhaps flush a part of the ring. Oldest entries are the less interesting, we're talking about prefetching after all. In the case of effective_io_concurrency, however, this may not work as well as expected, IIRC it is used to prefetch heap blocks, hopefully the requested blocks are contiguous but if there are too much holes it is enough to fill the ring very quickly (with the current max value of effective_io_concurrency). > When the prefetch process starts up, it services requests from the > queue by reading the requested blocks (or block ranges). When the > queue is empty, it sleeps. If it receives no requests for some period > of time, it unregisters itself and exits. This is sort of a souped-up > version of the hibernation facility we already have for some auxiliary > processes, in that we don't just make the process sleep for a longer > period of time but actually get rid of it altogether. I'm just a bit skeptical about the starting time: backend will ReadBuffer very soon after requesting the Prefetch... > All of this might be overkill; we could also do it with a permanent > auxiliary process. But it's sort of a shame to run an extra process > for a facility that might never get used, or might be used only > rarely. And I'm wary of cluttering things up with a thicket of > auxiliary processes each of which caters only to a very specific, very > narrow situation. Anyway, just thinking out loud here. For windows see the C++ PrefetchVirtualMemory() function. I really wonder if such a bgworker can improve the prefetching on !windows too if ring insert is faster than posix_fadvise call. If this is true, then effective_io_concurrency can be revisited. Maybe Greg Stark already did some benchmark of that... -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] pg_rewarm status
Le mardi 17 décembre 2013 21:14:44, Josh Berkus a écrit : > On 12/17/2013 06:34 AM, Robert Haas wrote: > > On Tue, Dec 17, 2013 at 12:09 AM, Amit Kapila wrote: > >> I have used pg_prewarm during some of work related to Buffer Management > >> and other performance related work. It is quite useful utility. > >> +1 for reviving this patch for 9.4 > > > > Any other votes? > > I still support this patch (as I did originally), and don't think that > the overlap with pgFincore is of any consequence. pgFincore does more > than pgrewarm ever will, but it's also platform-specific, so it still > makes sense for both to exist. Just for information, pgFincore is NOT limited to linux (the most interesting part, the memory snapshot, works also on BSD based kernels with mincore() syscall). Like for the PostgreSQL effective_io_concurrency (and pg_warm) it just doesn't work when posix_fadvise is not available. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] pg_rewarm status
Le mardi 17 décembre 2013 17:45:51, Robert Haas a écrit : > On Tue, Dec 17, 2013 at 11:02 AM, Jim Nasby wrote: > > On 12/17/13, 8:34 AM, Robert Haas wrote: > >> On Tue, Dec 17, 2013 at 12:09 AM, Amit Kapila > >> > >> wrote: > >>> I have used pg_prewarm during some of work related to Buffer Management > >>> and > >>> other performance related work. It is quite useful utility. > >>> +1 for reviving this patch for 9.4 > >> > >> Any other votes? > > > > We've had to manually code something that runs EXPLAIN ANALYZE SELECT * > > from a bunch of tables to warm our caches after a restart, but there's > > numerous flaws to that approach obviously. > > > > Unfortunately, what we really need to warm isn't the PG buffers, it's the > > FS cache, which I suspect this won't help. But I still see where just > > pg_buffers would be useful for a lot of folks, so +1. > > It'll do either one. For the FS cache, on Linux, you can also use > pgfincore. on Linux, *BSD (including OS X). like what's in postgresql. Only Windows is out of scope so far. and there is a solution for windows too, there is just no requirement from pgfincore users. Maybe you can add the windows support in PostgreSQL now ? -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] ERROR : 'tuple concurrently updated'
> > What PostgreSQL version is this? > > I'm using "Postgresql 9.2.4, compiled by Visual C++ build 1600, > 64-bit" > > Are there any triggers on any of these tables? > > There are no triggers. > > > Any noteworthy extensions installed? > > Here is the results returned by "select * from > pg_available_extensions" Those extensions are installed in the system, so you can install them in PostgreSQL. You may also have contrib run by servers without being pure extension. So the question is about used extensions or contrib. (it can be loaded by server, or in a session with LOAD, it can be auto-explain, pg_stat_statement, ). > > There are actually two places where that error can happen: > > simple_heap_update and simple_heap_delete. If you set the error > > verbosity to verbose, you should be able to see which function is at > > fault. The thing is, I don't see anything in that query which would > > update or delete any tuples, so there must be more to the story. If > > you have the ability to build from source, you could try setting a > > long sleep just before that error is thrown. Then run your test > > case > > until it hangs at that spot and get a stack backtrace. But that may > > be more troubleshooting than you want to get into. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
Le jeudi 10 octobre 2013 21:37:24 Peter Eisentraut a écrit : > On Mon, 2013-10-07 at 22:00 -0400, Andrew Dunstan wrote: > > The code has been sitting in HEAD for several months, and I > > committed on the back branches because it was wanted. > > New features normally go through a full development cycle including > extensive beta testing, especially when they contain changes to > external interfaces. The fact that the patch author wanted his > feature released immediately (of course) doesn't warrant a free pass > in this case, IMO. What's new feature ? PGXS break when 19e231b was commited, the patch around this special submake is trying to bugfix that. See http://www.postgresql.org/message-id/201305281410.32535.ced...@2ndquadrant.com There are also reports like this one (and thread): http://www.postgresql.org/message-id/cabrt9rcj6rvgmxbyebcgymwbwiobko_w6zvwrzn75_f+usp...@mail.gmail.com where you can read that I am not 'the' only guy who want to have this commited. And believeing this is worth a backpatch as it stands for a bugfix. Here also the problem is stated as something to fix. http://www.postgresql.org/message-id/20130516165318.gf15...@eldon.alvh.no-ip.org That's being said, you are correct about the problem you found and it's good to be able to release new version without a bug for OSX. I'm just sad you didn't get time to voice a bit before Andrew spent too much time on that. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
Le lundi 30 septembre 2013 00:10:09 Andrew Dunstan a écrit : > On 09/29/2013 07:09 PM, Andrew Dunstan wrote: > > On 09/03/2013 04:04 AM, Cédric Villemain wrote: > >>> Simple one, attached. > >>> I didn't document USE_VPATH, not sure how to explain that clearly. > >> > >> Just a remember that the doc is written and is waiting to be > >> commited. > >> > >> There is also an issue spoted by Christoph with the installdirs > >> prerequisite, > >> the attached patch fix that. > > > > I applied this one line version of the patch, which seemed more > > elegant than the later one supplied. > > > > I backpatched that and the rest of the VPATH changes for extensiuons > > to 9.1 where we first provided for extensions, so people can have a > > reasonably uniform build system for their extensions. > > I have reverted this in the 9.2 and 9.1 branches until I can work out > why it's breaking the buildfarm. 9.3 seems to be fine. Yes, please take the longer but better patch following the small one. There are eveidences that it fixes compilation to always build installdirs first. Previously installdirs may be build after copying files, so it fails. Chritstoph authored this good patch. I'm sorry not to have been clear on that. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] PostgreSQL 9.3 beta breaks some extensions "make install"
Le lundi 23 septembre 2013 11:43:13 Andrew Dunstan a écrit : > On 09/23/2013 11:31 AM, Alvaro Herrera wrote: > > Marti Raudsepp wrote: > >> On Fri, Sep 13, 2013 at 8:17 PM, Marti Raudsepp wrote: > >>> Oh I see, indeed commit 6697aa2bc25c83b88d6165340348a31328c35de6 > >>> "Improve support for building PGXS modules with VPATH" fixes the > >>> problem and I see it's not present in REL9_3_0. > >>> > >>> Andrew and others, does this seem safe enough to backport to > >>> 9.3.1? > >> > >> Ping? Will this be backported to 9.3 or should I report to > >> extension > >> authors to fix their Makefiles? > > > > I think this should be backpatched. > > I'm working on it. It appears to have a slight problem or two I want > to fix at the same time, rather than backpatch something broken. Would you mind sharing the problems you are facing ? You've noticed the problem about installdirs, I suppose. The attached patch is the fix currently applyed at debian. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et FormationMake the install targets depend on installdirs (not yet upstream, tbd) --- a/src/makefiles/pgxs.mk +++ b/src/makefiles/pgxs.mk @@ -132,29 +132,29 @@ $(INSTALL_PROGRAM) $(PROGRAM)$(X) '$(DESTDIR)$(bindir)' endif # PROGRAM -installcontrol: $(addsuffix .control, $(EXTENSION)) +installcontrol: $(addsuffix .control, $(EXTENSION)) | installdirs ifneq (,$(EXTENSION)) $(INSTALL_DATA) $^ '$(DESTDIR)$(datadir)/extension/' endif -installdata: $(DATA) $(DATA_built) +installdata: $(DATA) $(DATA_built) | installdirs ifneq (,$(DATA)$(DATA_built)) $(INSTALL_DATA) $^ '$(DESTDIR)$(datadir)/$(datamoduledir)/' endif -installdatatsearch: $(DATA_TSEARCH) +installdatatsearch: $(DATA_TSEARCH) | installdirs ifneq (,$(DATA_TSEARCH)) $(INSTALL_DATA) $^ '$(DESTDIR)$(datadir)/tsearch_data/' endif -installdocs: $(DOCS) +installdocs: $(DOCS) | installdirs ifdef DOCS ifdef docdir $(INSTALL_DATA) $^ '$(DESTDIR)$(docdir)/$(docmoduledir)/' endif # docdir endif # DOCS -installscripts: $(SCRIPTS) $(SCRIPTS_built) +installscripts: $(SCRIPTS) $(SCRIPTS_built) | installdirs ifdef SCRIPTS $(INSTALL_SCRIPT) $^ '$(DESTDIR)$(bindir)/' endif # SCRIPTS signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] PostgreSQL 9.3 beta breaks some extensions "make install"
> > Apt.pgdg got the patch present in postgresql head applyed. > > Erm, isn't apt.postgresql.org supposed to ship the *official* > PostgreSQL versions? Given that this issue affects all distros, I > don't see why Ubuntu/Debian need to be patched separately. Well, the patches are applyed on the debian packages (not only in apt.pgdg.org). The packages provided by apt.postgresql.org are based on the 'official packages' from debian. (if you allow me this circle) > Does anyone else think this is problematic? By slipping patches into > distro-specific packages, you're confusing users (like me) and > bypassing the PostgreSQL QA process. The QA is about distribution of packages here. Debian applies 14 patches on 9.3 builds, not only the things about vpath we're talking about. > PS: Where are the sources used to build packages on > apt.postgresql.org? 9.3: http://anonscm.debian.org/loggerhead/pkg-postgresql/postgresql-9.3/trunk/changes -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] PostgreSQL 9.3 beta breaks some extensions "make install"
Marti Raudsepp a écrit : >On Tue, May 14, 2013 at 4:12 AM, Marti Raudsepp >wrote: >> While testing out PostgreSQL 9.3beta1, I stumbled upon a problem > >> % make DESTDIR=/tmp/foo install > >> /usr/bin/install: will not overwrite just-created >> ‘/tmp/foo/usr/share/postgresql/extension/semver--0.3.0.sql’ with >> ‘./sql/semver--0.3.0.sql’ >> make: *** [install] Error 1 > >On Wed, May 15, 2013 at 4:49 PM, Peter Eisentraut >wrote: >> That said, I'm obviously outnumbered here. What about the following >> compromise: Use the configure-selected install program inside >> PostgreSQL (which we can test easily), and use install-sh under >> USE_PGXS? Admittedly, the make install time of extensions is >probably >> not an issue. > >Did we ever do anything about this? It looks like the thread got >distracted with VPATH builds and now I'm seeing this problem in 9.3.0. >:( > >This occurs in Arch Linux, but for some odd reason not on Ubuntu when >using apt.postgresql.org. Somehow the pgxs.mk supplied by >apt.postgresql.org differs from the one shipped in PostgreSQL. > >Is there a chance of getting this resolved in PostgreSQL or should we >get extension writers to fix their makefiles instead? Apt.pgdg got the patch present in postgresql head applyed. Andrew is about to commit (well...I hope) a doc patch about that and also a little fix. Imho this is a bugfix so I hope it will be applyed in older branches. -- Envoyé de mon téléphone. Excusez la brièveté. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] proposal: Set effective_cache_size to greater of .conf value, shared_buffers
Le jeudi 5 septembre 2013 17:14:37 Bruce Momjian a écrit : > On Thu, Sep 5, 2013 at 06:14:33PM +0200, Magnus Hagander wrote: > > > I have developed the attached patch which implements an auto-tuned > > > effective_cache_size which is 4x the size of shared buffers. I had to > > > set effective_cache_size to its old 128MB default so the EXPLAIN > > > regression tests would pass unchanged. > > > > That's not really autotuning though. ISTM that making the *default* 4 > > x shared_buffers might make perfect sense, but do we really need to > > hijack the value of "-1" for that? That might be useful for some time > > when we have actual autotuning, that somehow inspects the system and > > tunes it from there. > > > > I also don't think it should be called autotuning, when it's just a > > "smarter default value". > > > > I like the feature, though, just not the packaging. > > That "auto-tuning" text came from the wal_buffer documentation, which > does exactly this based on shared_buffers: > > The contents of the WAL buffers are written out to disk at every > transaction commit, so extremely large values are unlikely to > provide a significant benefit. However, setting this value to at > least a few megabytes can improve write performance on a busy > --> server where many clients are committing at once. The auto-tuning >--- > selected by the default setting of -1 should give reasonable > results in most cases. > > I am fine with rewording and not using -1, but we should change the > wal_buffer default and documentation too then. I am not sure what other > value than -1 to use? 0? I figure if we ever get better auto-tuning, > we would just remove this functionality and make it better. I'm fine with a -1 for auto-tune or inteligent default: it means (for me) that you don't need to care about this parameter in most case. A negative impact of the simpler multiplier might be that if suddendly someone reduce the shared_buffers size to fix some strange behavior, then he at the same needs to increase manualy the effective_cache_size (which remain the sum of the caches on the system, at least on a dedicated to PostgreSQL one). IMHO it is easy to know exactly how much of the memory is (or can be) used for/by PostgreSQL, we can compute that and update effective_cache_size at regular point int time. (just an idea, I know there are arguments against that too) Maybe the value for a 4x multiplier instead of 3x, is that the effective_cache_size usage can be larger than required. It's not a big trouble. With all things around NUMA we maybe just need to revisit that area (memory access cost non linear, double-triple caching, ...) . -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
> Simple one, attached. > I didn't document USE_VPATH, not sure how to explain that clearly. Just a remember that the doc is written and is waiting to be commited. There is also an issue spoted by Christoph with the installdirs prerequisite, the attached patch fix that. Also, the bugfixes were supposed to be backported. The behavior is not changed, nothing new, just fixing problem for some kind of extension builds. (However the USE_VPATH is new and is not needed in <9.3) -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation>From fb9d5ed99397b40e7fc33224fa31cd5c54c7b5d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Villemain?= Date: Tue, 3 Sep 2013 09:55:05 +0200 Subject: [PATCH] Add installdirs to PGXS order-only-prerequisites prerequisites are not ordered but installdirs is required by others. This patch is the simplest one, another solution being to remove completely installdirs prerequisite. Spotted by Christoph Berg with plr build. --- src/makefiles/pgxs.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk index 8618aa1..ac12f7d 100644 --- a/src/makefiles/pgxs.mk +++ b/src/makefiles/pgxs.mk @@ -124,7 +124,7 @@ all: all-lib endif # MODULE_big -install: all installdirs installcontrol installdata installdatatsearch installdocs installscripts +install: all installcontrol installdata installdatatsearch installdocs installscripts | installdirs ifdef MODULES $(INSTALL_SHLIB) $(addsuffix $(DLSUFFIX), $(MODULES)) '$(DESTDIR)$(pkglibdir)/' endif # MODULES -- 1.8.4.rc3 signature.asc Description: This is a digitally signed message part.
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
Le jeudi 29 août 2013 18:42:13 Stephen Frost a écrit : > On Thursday, August 29, 2013, Andres Freund wrote: > > If you don't want your installation to use it, tell you ops people not > > to do so. They are superusers, they need to have the ability to follow > > some rules you make up internally. > > The OPs people are the ones that will be upset with this because the DBAs > will be modifying configs which OPs rightfully claim as theirs. If they > have a way to prevent it then perhaps it's not terrible but they'd also > need to know to disable this new "feature". As for ALTER DATABASE- I would > be happier with encouraging use of that (or providing an ALTER CLUSTER) for > those thing it makes sense and works for and removing those from being in > postgresql.conf. I still feel things like listen_addresses shouldn't be > changed through this. ALTER ROLE ALL may be good enougth to handle every GUC that we can also remove from postgresql.conf (I suppose all GUC needing only a reload, not a restart). It may needs some improvement to handle changing default for ALL and adding new role. > > The energy wasted in a good part of this massive 550+ messages thread is > > truly saddening. We all (c|sh)ould have spent that time making PG more > > awesome instead. > > Perhaps not understood by all, but keeping PG awesome involves more than > adding every feature proposed- it also means saying no sometimes; to > features, to new GUCs, even to micro-optimizations when they're overly > complicated and offer only minimal or questionable improvements. Agreed, the current feature and proposal does not include pg_reload, and it introduces a full machinery we absolutely don't need. Grammar can be added later when the feature is stable. So far, we can achieve the goal by using adminpack, by using a file_fdw or a config_fdw. IMHO it is the job of a FDW to be able to handle atomic write or anything like that. I've commented one of the proposed patch adding some helpers to validate GUC change, I claimed this part was good enough to be added without ALTER SYSTEM (so a contrib can use it). -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] error out when building pg_xlogdump with pgxs
Le jeudi 29 août 2013 11:52:36 Andres Freund a écrit : > On 2013-08-29 11:49:00 +0200, Cédric Villemain wrote: > > Le mercredi 28 août 2013 00:12:22 Andres Freund a écrit : > > > Hi Alvaro, > > > > > > On 2013-08-27 14:47:49 -0400, Alvaro Herrera wrote: > > > > Andres Freund wrote: > > > > > pg_xlogdump cannot properly be built with pgxs since it needs a > > > > > sourcetree around. That already has confused some users... > > > > > > > > > > How about the attached patch which will tell it's not supported > > > > > instead > > > > > of an ominous build error about files that have no rules? > > > > > > > > Hmm, the other option is to ignore USE_PGXS completely and build > > > > assuming the source tree is always present. That way, if you build > > > > the > > > > whole contrib subdir with USE_PGXS=1, you will end up with all modules > > > > being built, not stop in the middle with an error. This seems more > > > > useful to me. We could just add a comment that USE_PGXS is ignored. > > > > > > What's the point in doing USE_PGXS builds with a full and configured > > > source present? The only thing I can think of is testing that pgxs > > > builds are working. In that case it doesn't seem helpful to fake > > > something into working which is then going to fail for real USE_PGXS > > > builds (where the original sourcetree won't be at that location > > > anymore). > > > > I had the same idea when Peter wished to remove PGXS from the contrib > > shiped with postgreSQL. > > > > I've been convinced that if we want to apply testing on pgxs makefile then > > we need something dedicated. Not abusing the current options. > > > > I'm in favor of removing PGXS from all contrib makefile, not only this > > one. > > Can we please discuss that in another thread? Anything like removing > PGXS wholesale is for sure not going to be something in 9.3 and this > specific issue should be fixed there as it already confused experienced > users. Andres, I was answering your question. Short and re-phrased: * we should not abuse make USE_PGXS to test the contrib build * I believe your patch is correct to issue an error when trying to build pg_xlogdump with PGXS, it is not possible, dot. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] error out when building pg_xlogdump with pgxs
Le mercredi 28 août 2013 00:12:22 Andres Freund a écrit : > Hi Alvaro, > > On 2013-08-27 14:47:49 -0400, Alvaro Herrera wrote: > > Andres Freund wrote: > > > pg_xlogdump cannot properly be built with pgxs since it needs a > > > sourcetree around. That already has confused some users... > > > > > > How about the attached patch which will tell it's not supported instead > > > of an ominous build error about files that have no rules? > > > > Hmm, the other option is to ignore USE_PGXS completely and build > > assuming the source tree is always present. That way, if you build the > > whole contrib subdir with USE_PGXS=1, you will end up with all modules > > being built, not stop in the middle with an error. This seems more > > useful to me. We could just add a comment that USE_PGXS is ignored. > > What's the point in doing USE_PGXS builds with a full and configured > source present? The only thing I can think of is testing that pgxs > builds are working. In that case it doesn't seem helpful to fake > something into working which is then going to fail for real USE_PGXS > builds (where the original sourcetree won't be at that location > anymore). I had the same idea when Peter wished to remove PGXS from the contrib shiped with postgreSQL. I've been convinced that if we want to apply testing on pgxs makefile then we need something dedicated. Not abusing the current options. I'm in favor of removing PGXS from all contrib makefile, not only this one. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Disabling ALTER SYSTEM SET WAS: Re: ALTER SYSTEM SET command to change postgresql.conf parameters
> There seemed to be agreement on having a config.d, though. Yes. Also, the validate_conf_parameter() (or some similar name) Amit added in his patch sounds useful if an extension can use it (to check a GUC someone want to change, to check a configuration file, ...) -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Disabling ALTER SYSTEM SET WAS: Re: ALTER SYSTEM SET command to change postgresql.conf parameters
> > Again, what are we trying to achieve?! > > no idea - wondering about that myself... It seems we are trying to add grammar for modifying postgresql.conf. Something we can already do easily in a standard extension, but without grammar changes. Maybe better to provide a contrib/ to modify config, then design what we can achieve more with an ALTER SYSTEM command. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
Le vendredi 2 août 2013 09:23:17, Amit Kapila a écrit : > On Friday, August 02, 2013 8:57 AM Stephen Frost wrote: > > * Andres Freund (and...@2ndquadrant.com) wrote: > > > FWIW, I think you've just put the final nail in the coffin of this > > > patch by raising the barriers unreasonably high. > > > > For my 2c, I don't think it's an unreasonable idea to actually > > *consider* what options are available through this mechanism rather > > than just presuming that it's a good idea to be able to modify > > anything, including things that you wouldn't be able to fix after a > > restart w/o hacking around in $PGDATA. > > I think if user has set any value wrong such that it doesn't allow server > to start, > we can provide an option similar to pg_resetxlog to reset the auto file. While guessing what changes are safe is hard, it is easy to discover the GUCs preventing PostgreSQL from restarting correctly. pg_ctl might be able to expose a clear message like : MSG: Params X and Y set by ALTER SYSTEM prevent PostgreSQL from starting HINT: Issue pg_ctl --ignore-bad-GUC start Note that the same may also allow postgresql to start with bad GUC value in postgresql.conf So this is another topic (IMHO). With the current idea of one-GUC-one-file in a PGDATA subdirectory it is *easy* to fix for any DBA or admin. However, note that I do prefer a simple 'include_auto_conf=here|start|end|off' in postgresql.conf (by default at end of file, with value 'end' set). -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
Le mardi 30 juillet 2013 05:27:12, Amit Kapila a écrit : > On Monday, July 29, 2013 7:15 PM Cédric Villemain wrote: > > Le lundi 29 juillet 2013 13:47:57, Amit Kapila a écrit : > > > On Sunday, July 28, 2013 11:12 AM Amit kapila wrote: > > > > On Friday, July 26, 2013 6:18 PM Tom Lane wrote: > > > > > > > > Alvaro Herrera writes: > > > > >> The main contention point I see is where conf.d lives; the two > > > > >> options are in $PGDATA or together with postgresql.conf. > > > > > > > > Tom > > > > > > > > >> and Robert, above, say it should be in $PGDATA; but this goes > > > > > > > > against > > > > > > > > >> Debian packaging and the Linux FHS (or whatever that thing is > > > > > > > > called). > > > > > > > > > Ordinarily, if postgresql.conf is not in $PGDATA, it will be > > > > > > > > somewhere > > > > > > > > > that the postmaster does not (and should not) have write > > > > > permissions for. I have no objection to inventiny a conf.d > > > > > subdirectory, I just > > > > > > > > say > > > > > > > > > that it must be under $PGDATA. The argument that this is against > > > > > FHS is utter nonsense, because anything we write there is not > > > > > static configuration, it's just data. > > > > > > > > > > Come to think of it, maybe part of the reason we're having such a > > > > > > > > hard > > > > > > > > > time getting to consensus is that people are conflating the > > > > "snippet" > > > > > > > part with the "writable" part? I mean, if you are thinking you > > > > > want system-management tools to be able to drop in configuration > > > > > fragments > > > > > > > > as > > > > > > > > > separate files, there's a case to be made for a conf.d > > > > > subdirectory > > > > > > > > that > > > > > > > > > lives somewhere that the postmaster can't necessarily write. We > > > > > just mustn't confuse that with support for ALTER SYSTEM SET. I > > > > > strongly believe that ALTER SYSTEM SET must not be designed to > > > > > write anywhere outside $PGDATA. > > > > > > > > I think if we can design conf.d separately for config files of > > > > management tools, then it is better to have postgresql.auto.conf to > > > > be in $PGDATA rather than in $PGDATA/conf.d > > > > > > > > Kindly let me know if you feel otherwise, else I will update and > > > > send patch tomorrow. > > > > > > Modified patch to have postgresql.auto.conf in $PGDATA. Changes are > > > > as > > > > > below: > > > > > > 1. initdb to create auto file in $PGDATA 2. ProcessConfigFile to open > > > auto file from data directory, special case handling for initdb 3. > > > AlterSystemSetConfigFile function to consider data directory as > > > reference for operating on auto file 4. modified comments in code and > > > docs to remove usage of config directory 5. modified function > > > write_auto_conf_file() such that even if there is no configuration > > > item to write, it should write header message. > > > > > >This is to handle case when there is only one parameter value and > > > > > > user set it to default, before this modification ,it > > > > > >will write empty file. > > > > I just read the patch, quickly. > > Thank you for review. > > > You may split the patch thanks to validate_conf_option(), however it is > > not a rule on postgresql-hacker. > > The review of the core functionality of patch has been done before the > introduction of function > validate_conf_option() in the patch. It was introduced because there > were some common parts in core implementation of AlterSystem and > set_config_option(). > I am really not sure, after having multiple round of reviews by > reviewers, it can add > significant value to split it. Yes, it just appeared that this part was a significant one in the patch. I understand that it is not interesting to split now. > > > Why not harcode in ParseConfigFp() that we should parse the a
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
Le lundi 29 juillet 2013 18:03:21, Alvaro Herrera a écrit : > > Why not harcode in ParseConfigFp() that we should parse the auto.conf > > file at the end (and/or if USE_AUTO_CONF is not OFF) instead of > > hacking ProcessConfigFile() with data_directory ? (data_directory should > > be set at this point) ... just thinking, a very convenient way to > > enable/disable that is just to add/remove the include directive in > > postgresql.conf. So no change should be required in ParseConf at all. > > Except maybe AbsoluteConfigLocation which should prefix the path to > > auto.conf.d with data_directory. What I like with the include directive > > is that Sysadmin can define some GUC *after* the auto.conf so he is sure > > those are not 'erased' by auto.conf (or by the DBA). > > Why do you think DBAs would like an option to disable this feature? I > see no point in that. And being able to relocate the parsing of > auto.conf to be in the middle of postgresql.conf instead of at the end > ... that seems nightmarish. I mean, things are *already* nontrivial to > follow, I don't see what would can come from a DBA running ALTER SYSTEM > and wondering why their changes don't take. I don't find that hard to do nor to understand, but if that has already reach a consensus then let's do that. > > Also, it looks very interesting to stick to an one-file-for-many-GUC when > > we absolutely don't care : this file should (MUST ?) not be edited by > > hand. The thing achieve is that it limits the access to ALTER SYSTEM. > > One file per GUC allows to LWlock only this GUC, isn't it ? (and also > > does not require machinery for holding old/new auto GUC, or at least > > more simple). > > This has already been debated, and we have already reached consensus > (one file to rule them all). I don't think it's a good idea to go over > all that discussion again. ok, I've only lost track for the consensus based on the technical objective. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
Le lundi 29 juillet 2013 13:47:57, Amit Kapila a écrit : > On Sunday, July 28, 2013 11:12 AM Amit kapila wrote: > > On Friday, July 26, 2013 6:18 PM Tom Lane wrote: > > > > Alvaro Herrera writes: > > >> The main contention point I see is where conf.d lives; > > >> the two options are in $PGDATA or together with postgresql.conf. > > > > Tom > > > > >> and Robert, above, say it should be in $PGDATA; but this goes > > > > against > > > > >> Debian packaging and the Linux FHS (or whatever that thing is > > > > called). > > > > > Ordinarily, if postgresql.conf is not in $PGDATA, it will be > > > > somewhere > > > > > that the postmaster does not (and should not) have write permissions > > > for. I have no objection to inventiny a conf.d subdirectory, I just > > > > say > > > > > that it must be under $PGDATA. The argument that this is against FHS > > > is utter nonsense, because anything we write there is not static > > > configuration, it's just data. > > > > > > Come to think of it, maybe part of the reason we're having such a > > > > hard > > > > > time getting to consensus is that people are conflating the "snippet" > > > part with the "writable" part? I mean, if you are thinking you want > > > system-management tools to be able to drop in configuration fragments > > > > as > > > > > separate files, there's a case to be made for a conf.d subdirectory > > > > that > > > > > lives somewhere that the postmaster can't necessarily write. We just > > > mustn't confuse that with support for ALTER SYSTEM SET. I strongly > > > believe that ALTER SYSTEM SET must not be designed to write anywhere > > > outside $PGDATA. > > > > I think if we can design conf.d separately for config files of > > management tools, then > > it is better to have postgresql.auto.conf to be in $PGDATA rather than > > in > > $PGDATA/conf.d > > > > Kindly let me know if you feel otherwise, else I will update and send > > patch > > tomorrow. > > Modified patch to have postgresql.auto.conf in $PGDATA. Changes are as > below: > > 1. initdb to create auto file in $PGDATA > 2. ProcessConfigFile to open auto file from data directory, special case > handling for initdb > 3. AlterSystemSetConfigFile function to consider data directory as > reference for operating on auto file > 4. modified comments in code and docs to remove usage of config directory > 5. modified function write_auto_conf_file() such that even if there is no > configuration item to write, it should write header message. >This is to handle case when there is only one parameter value and user > set it to default, before this modification ,it >will write empty file. I just read the patch, quickly. You may split the patch thanks to validate_conf_option(), however it is not a rule on postgresql-hacker. Why not harcode in ParseConfigFp() that we should parse the auto.conf file at the end (and/or if USE_AUTO_CONF is not OFF) instead of hacking ProcessConfigFile() with data_directory ? (data_directory should be set at this point) ... just thinking, a very convenient way to enable/disable that is just to add/remove the include directive in postgresql.conf. So no change should be required in ParseConf at all. Except maybe AbsoluteConfigLocation which should prefix the path to auto.conf.d with data_directory. What I like with the include directive is that Sysadmin can define some GUC *after* the auto.conf so he is sure those are not 'erased' by auto.conf (or by the DBA). Also, it looks very interesting to stick to an one-file-for-many-GUC when we absolutely don't care : this file should (MUST ?) not be edited by hand. The thing achieve is that it limits the access to ALTER SYSTEM. One file per GUC allows to LWlock only this GUC, isn't it ? (and also does not require machinery for holding old/new auto GUC, or at least more simple). It also prevent usage of ALTER SYSTEM for a cluster (as in replication) because this is not WAL logged. But it can be easier if trying to manage only one GUC at a time. I agree with Tom comment that this file(s) must be in data_directory. postgresql.auto.conf is useless, a "data_directory/auto.conf" (.d/ ?) is enough. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Re: [HACKERS] Wal sync odirect
Le lundi 22 juillet 2013 09:39:50, Craig Ringer a écrit : > On 07/22/2013 03:30 PM, Миша Тюрин wrote: > > > > i tell about wal_level is higher than MINIMAL > > OK, so you want to be able to force O_DIRECT for wal_level = archive ? > > I guess that makes sense if you expect the archive_command to read the > file out of the RAID controller's write cache before it gets flushed and > your archive_command can also use direct I/O to avoid pulling it into cache. > > You already know where to change to start experimenting with this. What > exactly are you trying to ask? I don't see any risk in forcing O_DIRECT > for higher wal_level, but I'm not an expert in WAL and recovery. I'd > recommend testing on a non-critical PostgreSQL instance. IIRC there is also some fadvise() call to flush the buffer cache when using 'minimal', but not when using archiving of WAL. I'm unsure how this has been tunned with streaming replication addition. see xlog.c|h -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation
Re: [Review] Re: [HACKERS] minor patch submission: CREATE CAST ... AS EXPLICIT
> >> Generally speaking, I agree with Robert's objection. The patch in > >> itself adds only one unnecessary keyword, which probably wouldn't be > >> noticeable, but the argument for it implies that we should be willing > >> to add a lot more equally-unnecessary keywords, which I'm not. gram.o > >> is already about 10% of the entire postgres executable, which probably > >> goes far towards explaining why its inner loop always shows up high in > >> profiling: cache misses are routine. And the size of those tables is > >> at least linear in the number of keywords --- perhaps worse than linear, > >> I'm not sure. Adding a bunch of keywords *will* cost us in performance. > >> I'm not willing to pay that cost for something that adds neither > >> features nor spec compliance. > > (1) Here are objective measures of the postgres stripped binary size > compiled from scratch on my laptop this morning: amd64, gcc version 4.7.3 (Debian 4.7.3-4) then gcc version 4.8.1 (Debian 4.8.1-6) with no option to configure, I got: > - without "AS EXPLICIT": 5286408 bytes >gram.o: 904936 bytes >keywords.o: 20392 bytes keywords.o : 20376 bytes gram.o: 909240 bytes keywords.o : 20376 bytes gram.o: 900504 bytes > > - with "AS EXPLICIT" : 5282312 bytes >gram.o: 901632 bytes >keywords.o: 20440 bytes keywords.o : 20424 bytes gram.o: 905904 bytes keywords.o : 20424 bytes gram.o: 897168 bytes > The executable binary is reduced by 4 KB with AS EXPLICIT, which > seems to come from some "ld" flucke really, because the only difference > I've found are the 8 bytes added to "keywords.o". This must be very > specific to the version and options used with gcc & ld on my laptop. same here, amd64. gcc to more impact, I didn't tryed with clang. > As for the general issue with the parser size: I work with languages and > compilers as a researcher. We had issues at one point with a scanner > because of too many keywords, and we solved it by removing keywords from > the scanner and checking them semantically in the parser with a hash > table, basically as suggested by Andres. The SQL syntax is pretty > redundant so that there are little choices at each point. Some tools can > generate perfect hash functions that can help (e.g. gperf). However I'm > not sure of the actual impact in size and time by following this path, I'm > just sure that there would be less keywords. IMHO, this issue is > orthogonal & independent from this patch. > > Finally, to answer your question directly, I'm really a nobody here, so > you can do whatever pleases you with the patch. I have no strong objection to the patch. It is only decoration and should not hurt. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
Le lundi 8 juillet 2013 21:46:39, Andrew Dunstan a écrit : > On 07/08/2013 03:40 PM, Josh Berkus wrote: > > On 07/04/2013 06:18 AM, Andrew Dunstan wrote: > >> On 07/04/2013 09:14 AM, Cédric Villemain wrote: > >>> ah yes, good catch, I though .control file were unique per contrib, > >>> but there aren't. > >> > >> It's already been fixed. > > > > Does it look like this patch will get into committable shape in the next > > couple of days? > > I think everything has been committed - as I think the CF app shows. The > only thing left in this srea from Cédric is the insallation of headers, > which Peter is down to review and is in "Waiting on Author" status. which is returned with feedback now, I can update if someone really wants it. > We are owed a docco patch though. Simple one, attached. I didn't document USE_VPATH, not sure how to explain that clearly. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation From 1380870e061362b871c375c25517005f82358dc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Villemain?= Date: Fri, 12 Jul 2013 23:39:11 +0200 Subject: [PATCH] add documentation for commit 6697aa2 Document that PGXS offers VPATH build and add a sample like in installation.sgml Also update the part about the PGXS global makefile inclusion. It was suggested to put it at the end, but if the Makefile contains a target before the include of pgxs then the default all: target is removed and the Makefile must define all: itself. --- doc/src/sgml/extend.sgml | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 60fa1a8..8f082e4 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -935,7 +935,7 @@ include $(PGXS) To use the PGXS infrastructure for your extension, you must write a simple makefile. In the makefile, you need to set some variables -and finally include the global PGXS makefile. +and include the global PGXS makefile. Here is an example that builds an extension module named isbn_issn, consisting of a shared library containing some C code, an extension control file, a SQL script, and a documentation @@ -1161,6 +1161,19 @@ include $(PGXS) or on the make command line. + +You can also run make in a directory outside the source +tree of your extension, if you want to keep the build directory separate. +This procedure is also called a +VPATHVPATH +build. Here's how: + + mkdir build_dir + cd build_dir + make -f /path/to/extension/source/tree/Makefile + make -f /path/to/extension/source/tree/Makefile install + + Changing PG_CONFIG only works when building -- 1.7.10.4 signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Millisecond-precision connect_timeout for libpq
Le lundi 8 juillet 2013 18:40:29, David E. Wheeler a écrit : > On Jul 8, 2013, at 7:44 AM, ivan babrou wrote: > >> Can you tell me why having ability to specify more accurate connect > >> timeout is a bad idea? > > > > Nobody answered my question yet. > > From an earlier post by Tom: > > What exactly is the use case for that? It seems like extra complication > > for something with little if any real-world usefulness. > > So the answer is: extra complication. for something that must go through a pooler anyway. You can have a look at pgbouncer: query_wait_timeout parameter for example. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
> > I'm not sure whether this is as simple as changing $< to $^ in the > > pgxs.mk's installcontrol rule, or if something more is required. > > Could you take a look? > > > > will do. ah yes, good catch, I though .control file were unique per contrib, but there aren't. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation
Re: [HACKERS] [PATCH] Remove useless USE_PGXS support in contrib
Le mercredi 3 juillet 2013 23:56:42, Josh Berkus a écrit : > Peter, Cedric, etc.: > > Where are we on this patch? Seems like discussion died out. Should it > be bounced? I for myself have been presuaded that it is a good idea. Things apparently loosed are not, it just outline that we need better coverage in PGXS area, so it is another thing to consider (TODO : add resgress test for PGXS ...) -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation
Re: [HACKERS] [9.4 CF 1] The Commitfest Slacker List
Le mercredi 3 juillet 2013 21:03:42, Christopher Browne a écrit : > On Wed, Jul 3, 2013 at 2:24 PM, Cédric Villemain wrote: > > > Clearly I ticked off a bunch of people by publishing "the list". On > > > the other hand, in the 5 days succeeding the post, more than a dozen > > > additional people signed up to review patches, and we got some of the > > > "ready for committer" patches cleared out -- something which nothing > > > else I did, including dozens of private emails, general pleas to this > > > mailing list, mails to the RRReviewers list, served to accomplish, in > > > this or previous CFs. > > > > Others rules appeared, like the 5 days limit. > > To me it outlines that some are abusing the CF app and pushing there > > useless > > patches (not still ready or complete, WIP, ... > > Seems to me that "useless" overstates things, but it does seem fair to > say that some patches are not sufficiently well prepared to be efficiently > added into Postgres. Oops! You just made me realized my choice of words was not good at all! I didn't considered under this angle, I just meant that some patches were added to CF to help patches authors, it was a good idea, but this had some unexpected corner case. Sorry for the confusion. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] [9.4 CF 1] The Commitfest Slacker List
> Clearly I ticked off a bunch of people by publishing "the list". On the > other hand, in the 5 days succeeding the post, more than a dozen > additional people signed up to review patches, and we got some of the > "ready for committer" patches cleared out -- something which nothing > else I did, including dozens of private emails, general pleas to this > mailing list, mails to the RRReviewers list, served to accomplish, in > this or previous CFs. Others rules appeared, like the 5 days limit. To me it outlines that some are abusing the CF app and pushing there useless patches (not still ready or complete, WIP, ...) > So, as an experiment, call it a mixed result. I would like to have some > other way to motivate reviewers than public shame. I'd like to have > some positive motivations for reviewers, such as public recognition by > our project and respect from hackers, but I'm doubting that those are > actually going to happen, given the feedback I've gotten on this list to > the idea. You're looking at a short term, big effect. And long term ? Will people listed still be interested to participate in a project which stamps people ? With or without review, it's a shame if people stop proposing patches because they are not sure to get time to review other things *in time*. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
Le samedi 29 juin 2013 22:00:34, Josh Berkus a écrit : > On 06/29/2013 11:42 AM, Andrew Dunstan wrote: > > I think we can survive for now without an additional tool. What I did > > was a proof of concept, it was not intended as a suggestion that we > > should install prep_buildtree. I am only suggesting that, in addition to > > your changes, if USE_VPATH is set then that path is used instead of a > > path inferred from the name of the Makefile. > > SO is this patch "returned with feedback"? Only the 0005-Allows-extensions-to-install-header-file.patch , others are in the hands of Andrew. Additional patch required for documentation. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
Le samedi 29 juin 2013 19:54:53, Andrew Dunstan a écrit : > On 06/26/2013 10:52 AM, Andrew Dunstan wrote: > > On 06/25/2013 11:29 AM, Cédric Villemain wrote: > >> Le mardi 25 juin 2013 17:18:51, Andrew Dunstan a écrit : > >>> On 06/24/2013 07:24 PM, Cédric Villemain wrote: > >>>> Le mardi 25 juin 2013 00:18:26, Andrew Dunstan a écrit : > >>>>> On 06/24/2013 04:02 PM, Cédric Villemain wrote: > >>>>>> WIth extension, we do have to set VPATH explicitely if we want to > >>>>>> use > >>>>>> VPATH (note that contribs/extensions must not care that > >>>>>> postgresql has > >>>>>> been built with or without VPATH set). My patches try to fix that. > >>>>> > >>>>> No, this is exactly what I'm objecting to. I want to be able to do: > >>>>> invoke_vpath_magic > >>>>> standard_make_commands_as_for_non_vpath > >>>>> > >>>>> Your patches have been designed to overcome your particular > >>>>> issues, but > >>>>> they don't meet the general case, IMNSHO. This is why I want to have > >>>>> more discussion about how vpath builds could work for PGXS modules. > >>>> > >>>> The patch does not restrict anything, it is not supposed to lead to > >>>> regression. > >>>> The assignment of VPATH and srcdir are wrong in the PGXS case, the > >>>> patch > >>>> correct them. You're still free to do "make VPATH=/mypath ..." the > >>>> VPATH > >>>> provided won't be erased by the pgxs.mk logic. > >>> > >>> I still think this is premature. I have spent some more time trying to > >>> make it work the way I think it should, so far without success. I think > >>> we need more discussion about how we'd like VPATH to work for PGXS > >>> would. There's no urgency about this - we've lived with the current > >>> situation for quite a while. > >> > >> Sure... > >> I did a quick and dirty patch (I just validate without lot of testing), > >> attached to this email to fix json_build (at least for make, make > >> install) > >> As you're constructing targets based on commands to execute in the > >> srcdir > >> directory, and because srcdir is only set in pgxs.mk, it is possible > >> to define > >> srcdir early in the json_build/Makefile and use it. > > > > This still doesn't do what I really want, which is to be able to > > invoke make without anything special in the invocation that triggers > > VPATH processing. > > > > Here's what I did that works like I think it should. > > > > First the attached patch on top of yours. > > > > Second, I did: > > mkdir vpath.json_build > > cd vpath.json_build > > sh/path/to/pgsource/config/prep_buildtree ../json_build . > > ln -s ../json_build/json_build.control . > > > > Then I created vpath.mk with these contents: > > ext_srcdir = ../json_build > > USE_VPATH = $(ext_srcdir) > > > > Finally I vpath-ized the Makefile (see attached). > > > > Given all of that, I was able to do, in the vpath directory: > > make > > make install > > make installcheck > > make clean > > > > and it all just worked, with exactly the same make invocations as work > > in an in-tree build. > > > > So what's lacking to make this nice is a tool to automate the second > > and third steps above. > > > > Maybe there are other ways of doing this, but this does what I'd like > > to see. > > I haven't seen a response to this. One thing we are missing is > documentation. Given that I'm inclined to commit all of this (i.e. > cedric's patches 1,2,3, and 4 plus my addition). I did so I sent the mail again. I believe your addition need some changes to allow the PGXS+VPATH case as it currently depends on the source-tree tool. So it needs to be in postgresql-server-devel packages, thus installed by postgresql "make install". I'm going to update the doc. It's probably worth to have some examples. > I'm also inclined to backpatch it, since without that it seems to me > unlikely packagers will be able to make practical use of it for several > years, and the risk is very low. Yes, it is valuable to help packagers here, thanks. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
[it seems my first email didn't make it, sent again] Le mercredi 26 juin 2013 16:52:01, Andrew Dunstan a écrit : > On 06/25/2013 11:29 AM, Cédric Villemain wrote: > > Le mardi 25 juin 2013 17:18:51, Andrew Dunstan a écrit : > >> On 06/24/2013 07:24 PM, Cédric Villemain wrote: > >>> Le mardi 25 juin 2013 00:18:26, Andrew Dunstan a écrit : > >>>> On 06/24/2013 04:02 PM, Cédric Villemain wrote: > >>>>> WIth extension, we do have to set VPATH explicitely if we want to use > >>>>> VPATH (note that contribs/extensions must not care that postgresql > >>>>> has been built with or without VPATH set). My patches try to fix > >>>>> that. > >>>> > >>>> No, this is exactly what I'm objecting to. I want to be able to do: > >>>> invoke_vpath_magic > >>>> standard_make_commands_as_for_non_vpath > >>>> > >>>> Your patches have been designed to overcome your particular issues, > >>>> but they don't meet the general case, IMNSHO. This is why I want to > >>>> have more discussion about how vpath builds could work for PGXS > >>>> modules. > >>> > >>> The patch does not restrict anything, it is not supposed to lead to > >>> regression. > >>> The assignment of VPATH and srcdir are wrong in the PGXS case, the > >>> patch correct them. You're still free to do "make VPATH=/mypath ..." > >>> the VPATH provided won't be erased by the pgxs.mk logic. > >> > >> I still think this is premature. I have spent some more time trying to > >> make it work the way I think it should, so far without success. I think > >> we need more discussion about how we'd like VPATH to work for PGXS > >> would. There's no urgency about this - we've lived with the current > >> situation for quite a while. > > > > Sure... > > I did a quick and dirty patch (I just validate without lot of testing), > > attached to this email to fix json_build (at least for make, make > > install) As you're constructing targets based on commands to execute in > > the srcdir directory, and because srcdir is only set in pgxs.mk, it is > > possible to define srcdir early in the json_build/Makefile and use it. > > This still doesn't do what I really want, which is to be able to invoke > make without anything special in the invocation that triggers VPATH > processing. I believe it is the case currently (with my patches applyed), we just have to invoke Makefile like we invoke configure for PostgreSQL, except that we don't need a configure stage with the contribs. Obviously it is not exactly the same because 'configure' is a script to execute, but 'make' is a command with a configuration file (the Makefile) For PostgreSQL it is: $ mkdir build_dir $ cd build_dir $ path/to/source/tree/configure [options go here] $ gmake For contribs it is: $ mkdir build_dir $ cd build_dir $ gmake -f /path/to/source/tree/Makefile [options go here] > Here's what I did that works like I think it should. > > First the attached patch on top of yours. > > Second, I did: > > mkdir vpath.json_build > cd vpath.json_build > sh/path/to/pgsource/config/prep_buildtree ../json_build . > ln -s ../json_build/json_build.control . OK, this creates supposedly required directories in the build process. This looks a bit wrong and more work than required: not all the source directories are required by the build process. But I understand the point. Second, config/prep_buildtree is not installed (by make install), so it is not suitable to use it with PGXS (PGXS is found with pg_config, when the postgresql source tree is not accessible anymore). We may add config/prep_buildtree to INSTALL_PROGRAM. The 'ln -s' can probably be copied by prep_buildtree. > Then I created vpath.mk with these contents: > > ext_srcdir = ../json_build > USE_VPATH = $(ext_srcdir) OK. > Finally I vpath-ized the Makefile (see attached). I don't see how this is more pretty than other solution. > Given all of that, I was able to do, in the vpath directory: > > make > make install > make installcheck > make clean > > and it all just worked, with exactly the same make invocations as work > in an in-tree build. It breaks others: mkdir /tmp/auth_delay && cd /tmp/auth_delay sh /path/prep_buildtree /path/contrib/auth_delay/ . (no .control, it's not an extension) make make: *** Pas de règle pour fabriquer la cible « auth_delay.so », nécessaire
Re: [HACKERS] Bugfix and new feature for PGXS
Le mercredi 26 juin 2013 16:52:01, Andrew Dunstan a écrit : > On 06/25/2013 11:29 AM, Cédric Villemain wrote: > > Le mardi 25 juin 2013 17:18:51, Andrew Dunstan a écrit : > >> On 06/24/2013 07:24 PM, Cédric Villemain wrote: > >>> Le mardi 25 juin 2013 00:18:26, Andrew Dunstan a écrit : > >>>> On 06/24/2013 04:02 PM, Cédric Villemain wrote: > >>>>> WIth extension, we do have to set VPATH explicitely if we want to use > >>>>> VPATH (note that contribs/extensions must not care that postgresql > >>>>> has been built with or without VPATH set). My patches try to fix > >>>>> that. > >>>> > >>>> No, this is exactly what I'm objecting to. I want to be able to do: > >>>> invoke_vpath_magic > >>>> standard_make_commands_as_for_non_vpath > >>>> > >>>> Your patches have been designed to overcome your particular issues, > >>>> but they don't meet the general case, IMNSHO. This is why I want to > >>>> have more discussion about how vpath builds could work for PGXS > >>>> modules. > >>> > >>> The patch does not restrict anything, it is not supposed to lead to > >>> regression. > >>> The assignment of VPATH and srcdir are wrong in the PGXS case, the > >>> patch correct them. You're still free to do "make VPATH=/mypath ..." > >>> the VPATH provided won't be erased by the pgxs.mk logic. > >> > >> I still think this is premature. I have spent some more time trying to > >> make it work the way I think it should, so far without success. I think > >> we need more discussion about how we'd like VPATH to work for PGXS > >> would. There's no urgency about this - we've lived with the current > >> situation for quite a while. > > > > Sure... > > I did a quick and dirty patch (I just validate without lot of testing), > > attached to this email to fix json_build (at least for make, make > > install) As you're constructing targets based on commands to execute in > > the srcdir directory, and because srcdir is only set in pgxs.mk, it is > > possible to define srcdir early in the json_build/Makefile and use it. > > This still doesn't do what I really want, which is to be able to invoke > make without anything special in the invocation that triggers VPATH > processing. I believe it is the case currently (with my patches applyed), we just have to invoke Makefile like we invoke configure for PostgreSQL, except that we don't need a configure stage with the contribs. Obviously it is not exactly the same because 'configure' is a script to execute, but 'make' is a command with a configuration file (the Makefile) For PostgreSQL it is: $ mkdir build_dir $ cd build_dir $ path/to/source/tree/configure [options go here] $ gmake For contribs it is: $ mkdir build_dir $ cd build_dir $ gmake -f /path/to/source/tree/Makefile [options go here] > Here's what I did that works like I think it should. > > First the attached patch on top of yours. > > Second, I did: > > mkdir vpath.json_build > cd vpath.json_build > sh/path/to/pgsource/config/prep_buildtree ../json_build . > ln -s ../json_build/json_build.control . OK, this creates supposedly required directories in the build process. This looks a bit wrong and more work than required: not all the source directories are required by the build process. But I understand the point. Second, config/prep_buildtree is not installed (by make install), so it is not suitable to use it with PGXS (PGXS is found with pg_config, when the postgresql source tree is not accessible anymore). We may add config/prep_buildtree to INSTALL_PROGRAM. The 'ln -s' can probably be copied by prep_buildtree. > Then I created vpath.mk with these contents: > > ext_srcdir = ../json_build > USE_VPATH = $(ext_srcdir) OK. > Finally I vpath-ized the Makefile (see attached). I don't see how this is more pretty than other solution. > Given all of that, I was able to do, in the vpath directory: > > make > make install > make installcheck > make clean > > and it all just worked, with exactly the same make invocations as work > in an in-tree build. It breaks others: mkdir /tmp/auth_delay && cd /tmp/auth_delay sh /path/prep_buildtree /path/contrib/auth_delay/ . (no .control, it's not an extension) make make: *** Pas de règle pour fabriquer la cible « auth_delay.so », nécessaire pour « all ». Arrêt. This works: cd /tmp/auth_delay rm
Re: [HACKERS] Bugfix and new feature for PGXS
Le mardi 25 juin 2013 17:18:51, Andrew Dunstan a écrit : > On 06/24/2013 07:24 PM, Cédric Villemain wrote: > > Le mardi 25 juin 2013 00:18:26, Andrew Dunstan a écrit : > >> On 06/24/2013 04:02 PM, Cédric Villemain wrote: > >>> WIth extension, we do have to set VPATH explicitely if we want to use > >>> VPATH (note that contribs/extensions must not care that postgresql has > >>> been built with or without VPATH set). My patches try to fix that. > >> > >> No, this is exactly what I'm objecting to. I want to be able to do: > >> invoke_vpath_magic > >> standard_make_commands_as_for_non_vpath > >> > >> Your patches have been designed to overcome your particular issues, but > >> they don't meet the general case, IMNSHO. This is why I want to have > >> more discussion about how vpath builds could work for PGXS modules. > > > > The patch does not restrict anything, it is not supposed to lead to > > regression. > > The assignment of VPATH and srcdir are wrong in the PGXS case, the patch > > correct them. You're still free to do "make VPATH=/mypath ..." the VPATH > > provided won't be erased by the pgxs.mk logic. > > I still think this is premature. I have spent some more time trying to > make it work the way I think it should, so far without success. I think > we need more discussion about how we'd like VPATH to work for PGXS > would. There's no urgency about this - we've lived with the current > situation for quite a while. Sure... I did a quick and dirty patch (I just validate without lot of testing), attached to this email to fix json_build (at least for make, make install) As you're constructing targets based on commands to execute in the srcdir directory, and because srcdir is only set in pgxs.mk, it is possible to define srcdir early in the json_build/Makefile and use it. > When I have more time I will work on it some more. Thank you -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation diff --git a/Makefile b/Makefile index ce10008..87582d1 100644 --- a/Makefile +++ b/Makefile @@ -1,24 +1,28 @@ EXTENSION= json_build -EXTVERSION = $(shell grep default_version $(EXTENSION).control | sed -e "s/default_version[[:space:]]*=[[:space:]]*'\([^']*\)'/\1/") -DATA = $(filter-out $(wildcard sql/*--*.sql),$(wildcard sql/*.sql)) -DOCS = $(wildcard doc/*.md) +srcdir = $(dir $(firstword $(MAKEFILE_LIST))) +EXTVERSION = $(shell grep default_version $(srcdir)/$(EXTENSION).control | sed -e "s/default_version[[:space:]]*=[[:space:]]*'\([^']*\)'/\1/") + +DATA = $(filter-out $(wildcard $(srcdir)/sql/*--*.sql),$(wildcard $(srcdir)/sql/*.sql)) +DOCS = $(wildcard $(srcdir)/doc/*.md) USE_MODULE_DB = 1 -TESTS= $(wildcard test/sql/*.sql) +TESTS= $(wildcard $(srcdir)/test/sql/*.sql) REGRESS_OPTS = --inputdir=test --outputdir=test \ --load-extension=$(EXTENSION) -REGRESS = $(patsubst test/sql/%.sql,%,$(TESTS)) +REGRESS = $(patsubst $(srcdir)/test/sql/%.sql,%,$(TESTS)) MODULE_big = $(EXTENSION) -OBJS = $(patsubst src/%.c,src/%.o,$(wildcard src/*.c)) +OBJS = $(patsubst $(srcdir)/src/%.c,$(srcdir)/src/%.o,$(wildcard $(srcdir)/src/*.c)) PG_CONFIG= pg_config all: sql/$(EXTENSION)--$(EXTVERSION).sql sql/$(EXTENSION)--$(EXTVERSION).sql: sql/$(EXTENSION).sql + $(MKDIR_P) sql cp $< $@ + DATA_built = sql/$(EXTENSION)--$(EXTVERSION).sql -DATA = $(filter-out sql/$(EXTENSION)--$(EXTVERSION).sql, $(wildcard sql/*--*.sql)) +DATA = $(filter-out $(srcdir)/sql/$(EXTENSION)--$(EXTVERSION).sql, $(wildcard $(srcdir)/sql/*--*.sql)) EXTRA_CLEAN = sql/$(EXTENSION)--$(EXTVERSION).sql PGXS := $(shell $(PG_CONFIG) --pgxs) signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
Le mardi 25 juin 2013 00:18:26, Andrew Dunstan a écrit : > On 06/24/2013 04:02 PM, Cédric Villemain wrote: > > WIth extension, we do have to set VPATH explicitely if we want to use > > VPATH (note that contribs/extensions must not care that postgresql has > > been built with or without VPATH set). My patches try to fix that. > > No, this is exactly what I'm objecting to. I want to be able to do: > > invoke_vpath_magic > standard_make_commands_as_for_non_vpath > > Your patches have been designed to overcome your particular issues, but > they don't meet the general case, IMNSHO. This is why I want to have > more discussion about how vpath builds could work for PGXS modules. The patch does not restrict anything, it is not supposed to lead to regression. The assignment of VPATH and srcdir are wrong in the PGXS case, the patch correct them. You're still free to do "make VPATH=/mypath ..." the VPATH provided won't be erased by the pgxs.mk logic. > > So the point is to be able to do exactly that : > > > > $ mkdir /tmp/json_build && cd /tmp/json_build > > $ make USE_PGXS=1 -f /path/to/json_build/Makefile > > It doesn't work: > > [andrew@emma vpath.json_build]$ PATH=../../inst.vpgxs.5706/bin:$PATH > make -f `pwd`/../json_build/Makefile > grep: json_build.control: No such file or directory > grep: json_build.control: No such file or directory > grep: json_build.control: No such file or directory > grep: json_build.control: No such file or directory > grep: json_build.control: No such file or directory > grep: json_build.control: No such file or directory > grep: json_build.control: No such file or directory > grep: json_build.control: No such file or directory > grep: json_build.control: No such file or directory > grep: json_build.control: No such file or directory > grep: json_build.control: No such file or directory > gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith > -Wdeclaration-after-statement -Wendif-labels > -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing > -fwrapv -fexcess-precision=standard -g -fpic -shared -o > json_build.so -L/home/pgl/npgl/inst.vpgxs.5706/lib -Wl,--as-needed > -Wl,-rpath,'/home/pgl/npgl/inst.vpgxs.5706/lib',--enable-new-dtags > cp > > /home/andrew/pgl/extns/vpath.json_build/../json_build/sql/json_build.sql > sql/json_build--.sql > cp: cannot create regular file `sql/json_build--.sql': No such file > or directory > make: *** [sql/json_build--.sql] Error 1 > [andrew@emma vpath.json_build]$ Ah, you make the point : your Makefile does not support VPATH or I failed to consider some cases. It seems to me that : EXTVERSION = $(shell grep default_version $(EXTENSION).control | sed -e "s/default_version[[:space:]]*=[[:space:]]*'\([^']*\)'/\1/") is not working. I'm going to check GNU Make guidelines about that case (should $(command) be executed on each path in the VPATH or not ?) [...] thinking a bit more... I suppose gmake expects the Makefile to list $(EXTENSION).control file as a prerequisite (thus its paths will be known during make invocation and your command will work) -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
Le lundi 24 juin 2013 19:40:19, Andrew Dunstan a écrit : > On 06/18/2013 09:52 AM, Cédric Villemain wrote: > > I've rebased the current set of pending patches I had, to fix pgxs and > > added 2 new patches. > > > > Bugfixes have already been presented and sent in another thread, except > > the last one which only fix comment in pgxs.mk. > > > > The new feature consists in a new variable to allow the installation of > > contrib header files. > > A new variable MODULEHEADER can be used in extension Makefile to list the > > header files to install. > > > > The installation path default to > > $includedir/$includedir_contrib/$extension. 2 new variables are set to > > define directories: $includedir_contrib and $includedir_extension, the > > former default to include/contrib and the later to > > $includedir_contrib/$extension ($extension is the name of the > > extension). > > > > This allows for example to install hstore header and be able to include > > them > > > > in another extension like that: > ># include "contrib/hstore/hstore.h" > > > > For packagers of PostgreSQL, this allows to have a > > postgresql-X.Y-contrib-dev package. > > For developers of extension it's killing the copy-and-paste-this-function > > dance previously required. > > > > I've not updated contribs' Makfefile: I'm not sure what we want to > > expose. > > > > I've 2 other patches to write to automatize a bit more the detection of > > things to do when building with USE_PGXS, based on the layout. Better > > get a good consensus on this before writing them. > > > > > > Bugfix: > > 0001-fix-SHLIB_PREREQS-when-building-with-USE_PGXS.patch > > 0002-Create-data-directory-if-extension-when-required.patch > > 0003-set-VPATH-for-out-of-tree-extension-builds.patch > > 0004-adds-support-for-VPATH-with-USE_PGXS.patch > > 0006-Fix-suggested-layout-for-extension.patch > > > > New feature: > > 0005-Allows-extensions-to-install-header-file.patch > > > > I'll do a documentation patch based on what is accepted in HEAD and/or > > previous branches. > > I have just spent an hour or two wrestling with the first four of these. > Items 5 and six are really separate items, which I'm not considering at > the moment. > > The trouble I have is that there are no instructions on how to modify > your Makefile or prepare a vpath tree, so I have been flying blind. I > started out doing what Postgres does, namely to prepare the tree using > config/prep_buildtree. This doesn't work at all - the paths don't get > set properly unless you invoke make with the path to the real makefile, > and I'm not sure they all get set correctly then. But that's not what we > expect of a vpath setup, or at least not what I expect. I expect that > with an appropriately set up make file and a correctly set up build tree > I can use an identical make invocation to the one I would use for an > in-tree build. That is, after setting up I should be able to ignore the > fact that it's a vpath build. > > FYI I deliberately chose a non-core extension with an uncommon layout > for testing: json_build <https://github.com/pgexperts/json_build> > > So, patches 1, 2 and 6 look sane enough, and I'm inclined to commit them > just to get them out of the way. That means two of the commitfest items > I am signed up for will be committed and two marked "Returned with > comments". I think we need some more discussion about how VPATH builds > should work with extensions, and subject to what limitations if any. Thank you for reviewing those patches. I'm sorry I haven't reproduced nor explained that much what is expected with VPATH. We have 2 main options with core postgresql: in-tree build or out-of-tree. The former is the classical build process, the later is what's frequently use for packaging because it allows not to pollute the source tree with intermediate or fully built files. You're right that we don't need to set VPATH explicitely, it is set by 'configure' when you exec configure out-of-source-tree. It is transparent to the user. Those 2 options are working as expected. Now, the extensions/contribs. We have 2^2 ways to build them. Either with PGXS or not, either with VPATH or not. NO_PGXS is how we build contribs shipped with postgresql. USE_PGXS is used by most of the others extensions (and contribs). WIth extension, we do have to set VPATH explicitely if we want to use VPATH (note that contribs/extensions must not care that postgresql has been built with or without
Re: [Review] Re: [HACKERS] minor patch submission: CREATE CAST ... AS EXPLICIT
Le lundi 24 juin 2013 11:44:21, Fabien COELHO a écrit : > >> Here it is. > > > > The patch does not apply and git also whines about trailing space. > > It needs a v3... > > The attachement here works for me. > Could you be more precise about the issue? > > postgresql> git branch test master > postgresql> git checkout test > Switched to branch 'test' > postgresql> patch -p1 < ../as_explicit-v2.patch > patching file doc/src/sgml/ref/create_cast.sgml > patching file src/backend/parser/gram.y > patching file src/include/parser/kwlist.h > patching file src/test/regress/expected/create_cast.out > patching file src/test/regress/sql/create_cast.sql Ah, got it. 'git apply' is more strict. Patch apply with patch -p1 ( I though I tryed, but it seems not) == patch -p1 < as_explicit-v2.patch (Stripping trailing CRs from patch.) patching file doc/src/sgml/ref/create_cast.sgml (Stripping trailing CRs from patch.) patching file src/backend/parser/gram.y (Stripping trailing CRs from patch.) patching file src/include/parser/kwlist.h (Stripping trailing CRs from patch.) patching file src/test/regress/expected/create_cast.out (Stripping trailing CRs from patch.) patching file src/test/regress/sql/create_cast.sql == -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [Review] Re: [HACKERS] minor patch submission: CREATE CAST ... AS EXPLICIT
Hello Fabien, > > I flag it 'return with feedback', please update the patch so it builds. > > Everything else is ok. > > Here it is. The patch does not apply and git also whines about trailing space. It needs a v3... Please note that a community-agreed behavior on this patch is not yet acquired, you should consider that too. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Implementing incremental backup
Le samedi 22 juin 2013 01:09:20, Jehan-Guillaume (ioguix) de Rorthais a écrit
:
> On 20/06/2013 03:25, Tatsuo Ishii wrote:
> >> On Wed, Jun 19, 2013 at 8:40 PM, Tatsuo Ishii
wrote:
> >>>> On Wed, Jun 19, 2013 at 6:20 PM, Stephen Frost
wrote:
> >>>>> * Claudio Freire (klaussfre...@gmail.com) wrote:
> [...]
>
> >> The only bottleneck here, is WAL archiving. This assumes you can
> >> afford WAL archiving at least to a local filesystem, and that the WAL
> >> compressor is able to cope with WAL bandwidth. But I have no reason to
> >> think you'd be able to cope with dirty-map updates anyway if you were
> >> saturating the WAL compressor, as the compressor is more efficient on
> >> amortized cost per transaction than the dirty-map approach.
> >
> > Thank you for detailed explanation. I will think more about this.
>
> Just for the record, I was mulling over this idea since a bunch of
> month. I even talked about that with Dimitri Fontaine some weeks ago
> with some beers :)
>
> My idea came from a customer during a training explaining me the
> difference between differential and incremental backup in Oracle.
>
> My approach would have been to create a standalone tool (say
> pg_walaggregate) which takes a bunch of WAL from archives and merge them
> in a single big file, keeping only the very last version of each page
> after aggregating all their changes. The resulting file, aggregating all
> the changes from given WAL files would be the "differential backup".
>
> A differential backup resulting from a bunch of WAL between W1 and Wn
> would help to recover much faster to the time of Wn than replaying all
> the WALs between W1 and Wn and saves a lot of space.
>
> I was hoping to find some time to dig around this idea, but as the
> subject rose here, then here are my 2¢!
something like that maybe :
./pg_xlogdump -b \
../data/pg_xlog/00010001 \
../data/pg_xlog/00010005| \
grep 'backup bkp' | awk '{print ($5,$9)}'
--
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation
signature.asc
Description: This is a digitally signed message part.
[Review] Re: [HACKERS] minor patch submission: CREATE CAST ... AS EXPLICIT
Le lundi 17 juin 2013 00:02:21, Fabien COELHO a écrit : > >> What activity would you expect? > > > > A patch which applies cleanly to git HEAD. This one doesn't for me, > > although I'm not really sure why, I don't see any obvious conflicts. > > Please find attached a freshly generated patch against current master. * Submission review: patch is in unified format and apply on HEAD. patch contains documentation, however I believe 'AS IMPLICIT' is a PostgreSQL extension with special behavior and 'AS EXPLICIT' respect the standard except that PostgreSQL adds only the expression 'AS EXPLICIT' (it is also the default in the standard). So maybe it is possible to rephrase this piece: @@ -411,8 +427,8 @@ CREATE CAST (bigint AS int4) WITH FUNCTION int4(bigint) AS ASSIGNMENT; SQL standard, except that SQL does not make provisions for binary-coercible types or extra arguments to implementation functions. - AS IMPLICIT is a PostgreSQL - extension, too. + AS IMPLICIT and AS EXPLICIT are + a PostgreSQL extension, too. After digging in the archive and the CF: original request is at https://commitfest.postgresql.org/action/patch_view?id=563 * Usability review ** Does the patch actually implement that? yes ** Do we want that? Back in 2012 Tom exposed arguments against it, or at least not a clear +1. The patch add nothing but more explicit creation statement, it has gone untouched for 2 years without interest so it is surely not something really important for PostgreSQL users. However we already have non-standard words for CREATE CAST, this new one is not very intrusive . ** Does it follow SQL spec, or the community-agreed behavior? It does not follow SQL spec. ** Does it include pg_dump support (if applicable)? Not but it is probably not interesting to add that to the pg_dump output: it increases incompatibility with SQL spec for no gain. The result is that the patch only allows to CREATE CAST..AS EXPLICIT without error. Then pg_dump won't know if the CAST has been created with the default or an 'explicit default'... ** Are there dangers? It seems no. * Feature test ** Does the feature work as advertised? Yes ** Are there corner cases the author has failed to consider? I think no, but my skills with the parser are limited (gram.y, ...) ** Are there any assertion failures or crashes? no * Performance review: not relevant. * Coding review Patch does not pass test: ./check_keywords.pl gram.y ../../../src/include/parser/kwlist.h I had to update the unreserved keyword list in order to be able to build postgresql. ** Does it follow the project coding guidelines? yes ** Are there portability issues? no (only for SQL) ** Will it work on Windows/BSD etc? yes ** Are the comments sufficient and accurate? Yes ** Does it do what it says, correctly? Yes ** Does it produce compiler warnings? don't build as is. Need patch update ** Can you make it crash? no * Architecture review ** Is everything done in a way that fits together coherently with other features/modules? Yes ** Are there interdependencies that can cause problems? No. I flag it 'return with feedback', please update the patch so it builds. Everything else is ok. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Why can't I use windowing functions over ordered aggregates?
Le vendredi 21 juin 2013 03:32:33, Josh Berkus a écrit : > Hackers, > > So, I can create a custom aggregate "first" and do this: > > SELECT first(val order by ts desc) ... > > And I can do this: > > SELECT first_value(val) OVER (order by ts desc) > > ... but I can't do this: > > SELECT first_value(val order by ts desc) > > ... even though under the hood, it's the exact same operation. First I'm not sure it is the same, in a window frame you have the notion of peer-rows (when you use ORDER BY). And also, first_value is a *window* function, not a simple aggregate function... See this example: # create table foo (i int, t timestamptz); # insert into foo select n, now() from generate_series(1,10) g(n); # select i, first_value(i) over (order by t desc) from foo; # select i, first_value(i) over (order by t desc ROWS between 0 PRECEDING and UNBOUNDED FOLLOWING) from foo; What do you expect "SELECT first(val order by ts desc)" to output ? -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
> Opinions all? > > * Give up on being able to use one extension's header from another > entirely, except in the special case of in-tree builds There are already a good number of use cases with only hstore and intarray, I'm in favor of this feature. > * Install install-enabled contrib headers into an include/contrib/ > that's always on the pgxs include path, so you can still just "#include > hstore.h" . For in-tree builds, explicit add other modules' contrib dirs > as Peter has done in the proposed plperl_hstore and plpython_hstore. > (Use unqualified names). I don't like the idea to share a flat directory with different header files, filenames can overlap. > * Install install-enabled contrib headers to > include/contrib/[modulename]/ . Within the module, use the unqualified > header name. When referring to it from outside the module, use #include > "contrib/modulename/header.h". Add $(top_srcdir) to the include path > when building extensions in-tree. not either, see my other mail. we still #include hstore.h when we want, we just add that the header so it is available for PGXS builds. Contrib Makefile still need to -I$(includedir_contrib)/hstore. What's new is that the header is installed, not that we don't have to set FLAGS. > Personally, I'm all for just using the local path when building the > module, and the qualified path elsewhere. It requires only a relatively > simple change, and I don't think that using "hstore.h" within hstore, > and "contrib/hstore/hstore.h" elsewhere is of great concern. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
Le jeudi 20 juin 2013 05:26:21, Peter Eisentraut a écrit :
> On Wed, 2013-06-19 at 20:58 +0200, Cédric Villemain wrote:
> > I believe he answered the proposal to put all headers on the same flat
> > directory, instead of a tree.
>
> The headers are used as
>
> #include "hstore.h"
> #include "ltree.h"
> etc.
>
> in the existing source code.
>
> If you want to install the for use by others, you can do one of three
> things:
>
> 1) Install them into $(pg_config --includedir-server), so other users
> will just include them in the same way as shown above.
I don't like this one because extension can overwrite sever headers.
> 2) Install them in a different directory, but keep the same #include
> lines. That would require PGXS changes, perhaps a new pg_config option,
> or something that produces the right -I option to find them.
Patch of PGXS is not a problem.
pg_config patch is a requirement only if users set their own
$(includedir_contrib) variable. I didn't though about it, but it is probably
better to add that. This looks trivial too.
To include hstore header we write «#include "hstore.h"» but we add :
-I$(includedir_contrib)/hstore to the FLAGS in the extension makefile which
does require hstore. It changes nothing to hstore Makefile itself.
The main difference is that before we had to -I$(top_srcdir)/../contrib/hstore
*iff* we are not building with PGXS (else we need to have the hstore source
available somewhere), now we can do -I$(includedir_contrib)/hstore with PGXS
(hstore need to be installed first, as we USE_PGXS)
Then PGXS offers to catch both case transparently so we can do :
-I${include_contrib_header) in Makefile
and pgxs.mk takes care of adjusting include_contrib_header (or whatever its
name) according to USE_PGXS value.
> 3) Install them in a different directory and require a different
> #include line. But then you have to change the existing uses as well,
> which would probably require moving files around.
Having to change existing source code do keep the same behavior is not
attractive at all.
> Both 2 and 3 require a lot of work, possibly compatibility breaks, for
> no obvious reason.
2 is a good solution and not a lot of work, IMO.
Removing the need of setting -I$(include...) in the contrib Makefile can be
done later by adding a PGXS variable to define the contrib requirements, this
is something different from the current feature (build contrib depending on
another(s) without full source tree)
--
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation
signature.asc
Description: This is a digitally signed message part.
Re: [HACKERS] [PATCH] Remove useless USE_PGXS support in contrib
Le mercredi 19 juin 2013 22:22:22, Andrew Dunstan a écrit : > On 06/19/2013 03:52 PM, Dimitri Fontaine wrote: > > Peter Eisentraut writes: > >> We could do something like > >> > >> PG_CONFIG = fake_intree_pg_config > >> PGXS := $(shell $(PG_CONFIG) --pgxs) > >> include $(PGXS) > > > > There's something to that idea. Of course we would need to offer a > > comment about the PG_CONFIG game and propose something else for real > > world extensions (PG_CONFIG ?= pg_config). > > > >> where fake_intree_pg_config is a purpose-built shell script that points > >> to the right places inside the source tree. > > > > If that works, that gets my preference over removing PGXS support in > > contrib modules. Setting an example is important, in-tree build is not > > a useful example for anyone but contributors to core. > > Not true - you're forgetting there is no pgxs for MSVC builds. PGXS + MSVC is still in the TODO list I won't be able to work on that. > If we're going to enable building of contrib modules using pgxs but > without an install we will make targets for that, and buildfarm support. With the set of patches I sent, contrib can be built with PGXS, there is no issue hereExcept maybe pg_xlogdump, and this one might be improved not to have to rebuild shared object from postgresql (IIRC it is a static build or something like that)... -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
Le mercredi 19 juin 2013 21:06:23, Alvaro Herrera a écrit : > Peter Eisentraut wrote: > > On 6/19/13 12:20 PM, Andrew Dunstan wrote: > > > So you're saying to install extension headers, but into the main > > > directory where we put server headers? > > > > Yes, if we choose to install some extension headers, that is where we > > should put them. > > The question of the name of the directory still stands. "contrib" would > be the easiest answer, but it's slightly wrong because > externally-supplied modules could also want to install headers. > "extension" might be it, but there are things that aren't extensions > (right? if not, that would be my choice). yes, I think the same. auth_delay for example is not an extension as in CREATE EXTENSION. So...it is probably better to postpone this decision and keep on the idea to just install headers where there should be will traditional name (contrib). -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
Le mercredi 19 juin 2013 18:48:21, Andrew Dunstan a écrit : > On 06/19/2013 12:32 PM, Cédric Villemain wrote: > > Le mercredi 19 juin 2013 18:20:11, Andrew Dunstan a écrit : > >> On 06/19/2013 11:26 AM, Peter Eisentraut wrote: > >>> On 6/19/13 10:19 AM, Andrew Dunstan wrote: > >>>> What are they going to be used for anyway? I rubbed up against this > >>>> not too long ago. Things will blow up if you use stuff from the > >>>> module and the module isn't already loaded. > >>> > >>> See transforms. > >> > >> So you're saying to install extension headers, but into the main > >> directory where we put server headers? > > > > At the same level than server headers, but I'm open for suggestion. > > > > $ tree -d include > > include > > ├── libpq > > └── postgresql > > > > ├── contrib > > │ └── hstore > > ├── informix > > │ └── esql > > ├── internal > > │ └── libpq > > └── server > > > > And all subidrs of server/ > > This is what Peter was arguing against, isn't it? Now I have a doubt :) I believe he answered the proposal to put all headers on the same flat directory, instead of a tree. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
Le mercredi 19 juin 2013 18:20:11, Andrew Dunstan a écrit :
> On 06/19/2013 11:26 AM, Peter Eisentraut wrote:
> > On 6/19/13 10:19 AM, Andrew Dunstan wrote:
> >> What are they going to be used for anyway? I rubbed up against this not
> >> too long ago. Things will blow up if you use stuff from the module and
> >> the module isn't already loaded.
> >
> > See transforms.
>
> So you're saying to install extension headers, but into the main
> directory where we put server headers?
At the same level than server headers, but I'm open for suggestion.
$ tree -d include
include
├── libpq
└── postgresql
├── contrib
│ └── hstore
├── informix
│ └── esql
├── internal
│ └── libpq
└── server
And all subidrs of server/
--
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation
signature.asc
Description: This is a digitally signed message part.
[Review] Re: [HACKERS] [PATCH] Remove useless USE_PGXS support in contrib
Le jeudi 13 juin 2013 05:16:48, Peter Eisentraut a écrit : > This has served no purpose except to > > 1. take up space > 2. confuse users > 3. produce broken external extension modules that take contrib as an > example 4. break builds of PostgreSQL when users try to fix 3. by > exporting USE_PGXS > > There is adequate material in the documentation and elsewhere (PGXN) on > how to write extensions and their makefiles, so this is not needed. > --- > pursuant to discussion here: > http://www.postgresql.org/message-id/512ceab8.9010...@gmx.net * Submission review: patch apply on HEAD, no doc or test required. * Usability review ** Does the patch actually implement that? yes ** Do we want that? Consensus is not complete: some use case raised. 1/ regression test: not a good excuse, see [1] 2/ being able to build contrib out of tree, it is unsure it is really needed on its own but was suggested. See [2] and [3] Arguments against removal are new features (extension layout, more work on PGXS shoulders, extension headers exported, clean regression test for PGXS) ** Does it follow the community-agreed behavior? Some people voiced against the idea. More answers might be better to confirm that this is wanted. Amul, Joe, Craig ? ** Are there dangers? The only I can see is packagers building contribs with PGXS, but as it is currently buggy I'm sure they can't do that. * Feature test: it deprecates a not-fully-implemented-feature (even fully implemented this may not be considered a feature at all) * Performance review: not relevant (contribs may build some µs faster...) * Coding review: OK * Architecture review: looks good too. The patch needs to reach consensus before commit. There is no status for that in CF, for me current status is: 'Ready, Waiting more feedback from community'. [1] http://www.postgresql.org/message- id/1371610695.13762.25.ca...@vanquo.pezone.net [2] http://www.postgresql.org/message- id/1371172850.79798.yahoomail...@web193505.mail.sg3.yahoo.com [3] http://www.postgresql.org/message-id/51bbe3a5.40...@2ndquadrant.com -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] [PATCH] Remove useless USE_PGXS support in contrib
Le mercredi 19 juin 2013 04:58:15, Peter Eisentraut a écrit : > On Mon, 2013-06-17 at 19:00 +0200, Cédric Villemain wrote: > > My only grief is to loose the perfect regression tests for PGXS those > > contribs are. > > I think they are neither perfect nor regression tests. If we want > tests, let's write tests. You are right. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
Le mercredi 19 juin 2013 04:48:11, Peter Eisentraut a écrit : > On Tue, 2013-06-18 at 15:52 +0200, Cédric Villemain wrote: > > This allows for example to install hstore header and be able to > > include them > > > > in another extension like that: > > # include "contrib/hstore/hstore.h" > > That's not going to work. hstore's header file is included as #include > "hstore.h" (cf. hstore source code). Having it included under different > paths in different contexts will be a mess. OK. At the beginning I though of putting headers in include/contrib but feared that some header may have the same name. If you think that it is suitable, I can do that ? (and include the clean: recipe that I missed on the first shot) Also, do we want to keep the word 'contrib' ? include/extension looks fine too... -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
[HACKERS] Bugfix and new feature for PGXS
I've rebased the current set of pending patches I had, to fix pgxs and added 2 new patches. Bugfixes have already been presented and sent in another thread, except the last one which only fix comment in pgxs.mk. The new feature consists in a new variable to allow the installation of contrib header files. A new variable MODULEHEADER can be used in extension Makefile to list the header files to install. The installation path default to $includedir/$includedir_contrib/$extension. 2 new variables are set to define directories: $includedir_contrib and $includedir_extension, the former default to include/contrib and the later to $includedir_contrib/$extension ($extension is the name of the extension). This allows for example to install hstore header and be able to include them in another extension like that: # include "contrib/hstore/hstore.h" For packagers of PostgreSQL, this allows to have a postgresql-X.Y-contrib-dev package. For developers of extension it's killing the copy-and-paste-this-function dance previously required. I've not updated contribs' Makfefile: I'm not sure what we want to expose. I've 2 other patches to write to automatize a bit more the detection of things to do when building with USE_PGXS, based on the layout. Better get a good consensus on this before writing them. Bugfix: 0001-fix-SHLIB_PREREQS-when-building-with-USE_PGXS.patch 0002-Create-data-directory-if-extension-when-required.patch 0003-set-VPATH-for-out-of-tree-extension-builds.patch 0004-adds-support-for-VPATH-with-USE_PGXS.patch 0006-Fix-suggested-layout-for-extension.patch New feature: 0005-Allows-extensions-to-install-header-file.patch I'll do a documentation patch based on what is accepted in HEAD and/or previous branches. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation From c23041f31b5a312702d79bbe759a56628f3e37e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Villemain?= Date: Tue, 28 May 2013 14:11:18 +0200 Subject: [PATCH 1/6] fix SHLIB_PREREQS when building with USE_PGXS commit 19e231b introduced SHLIB_PREREQS but failed to port that to PGXS build. The issue is that "submake-*" can not be built with PGXS, in this case they must check that expected files are present (and installed). Maybe it is better to only check if they have been built ? This fix the build of dblink and postgres_fdw (make USE_PGXS=1) --- src/Makefile.global.in | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/Makefile.global.in b/src/Makefile.global.in index 8bfb77d..c3c595e 100644 --- a/src/Makefile.global.in +++ b/src/Makefile.global.in @@ -415,13 +415,24 @@ libpq_pgport = -L$(top_builddir)/src/port -lpgport \ -L$(top_builddir)/src/common -lpgcommon $(libpq) endif - +# If PGXS is not defined, builds as usual: +# build dependancies required by SHLIB_PREREQS +# If the build is with PGXS, then any requirement is supposed to be already +# build and we just take care that the expected files exist +ifndef PGXS submake-libpq: $(MAKE) -C $(libpq_builddir) all +else +submake-libpq: $(libdir)/libpq.so ; +endif +ifndef PGXS submake-libpgport: $(MAKE) -C $(top_builddir)/src/port all $(MAKE) -C $(top_builddir)/src/common all +else +submake-libpgport: $(libdir)/libpgport.a $(libdir)/libpgcommon.a ; +endif .PHONY: submake-libpq submake-libpgport -- 1.7.10.4 From 3d3f4df6792c0e98b0a915b8704504f27738bf26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Villemain?= Date: Tue, 28 May 2013 14:17:04 +0200 Subject: [PATCH 2/6] Create data directory if extension when required There is a hack to link the regression data files from the srcdir to the builddir when doing 'make VPATH'. but it failed when used in conjunction with USE_PGXS and out-of-tree build of an extension. Issue is the absence of the data/ directory in the builddir. --- src/makefiles/pgxs.mk |1 + 1 file changed, 1 insertion(+) diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk index bbcfe04..6a19b0f 100644 --- a/src/makefiles/pgxs.mk +++ b/src/makefiles/pgxs.mk @@ -263,6 +263,7 @@ test_files_build := $(patsubst $(srcdir)/%, $(abs_builddir)/%, $(test_files_src) all: $(test_files_build) $(test_files_build): $(abs_builddir)/%: $(srcdir)/% + $(MKDIR_P) $(dir $@) ln -s $< $@ endif # VPATH -- 1.7.10.4 From 66b394ae867bde2ad968027f0708ae59a140d81b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Villemain?= Date: Tue, 28 May 2013 14:51:43 +0200 Subject: [PATCH 3/6] set VPATH for out-of-tree extension builds If the makefile is not in the current directory (where we launch 'make') then assume we are building out-of-src tree and set the VPATH to the directory of the first makefile... Thus it fixes: mkdir /tmp/a && cd /tmp/a make -f extension_src/Makefile USE_PGXS=1 --- src/makefiles/pgxs.mk |9
Re: [HACKERS] How do we track backpatches?
Le mardi 18 juin 2013 04:48:02, Peter Eisentraut a écrit : > On Mon, 2013-06-17 at 17:11 -0700, Josh Berkus wrote: > > Contributors, > > > > While going through this mailing list looking for missing 9.4 patches, I > > realized that we don't track backpatches (that is, fixes to prior > > versions) at all anywhere. Where backpatches are submitted by > > committers this isn't an issue, but we had a couple major ones (like the > > autovacuum fix) which were submitted by general contributors. The same > > goes for beta fixes. > > > > Should we add a "prior version" category to the CF to make sure these > > don't get dropped? Or do something else? > > A separate commit fest for tracking proposed backpatches might be > useful. Backpatches are bugs fix, isnt it ? I will like to have a mail based bug tracker like debbugs. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] [PATCH] Remove useless USE_PGXS support in contrib
Le lundi 17 juin 2013 18:41:32, Alvaro Herrera a écrit : > Joe Conway wrote: > > On 06/15/2013 11:28 AM, Alvaro Herrera wrote: > > > This use case seems too narrow to me to justify the burden of > > > keeping PGXS-enabled makefiles in contrib. > > > > What was the burden of it? > > Per http://www.postgresql.org/message- id/1371093408.309.5.ca...@vanquo.pezone.net : > : 1. take up space > : 2. confuse users > : 3. produce broken external extension modules that take contrib as an > : example 4. break builds of PostgreSQL when users try to fix 3. by > : exporting USE_PGXS But: 4. can be fixed (see patches I sent) so it is not an excuse. I agree for other points. My only grief is to loose the perfect regression tests for PGXS those contribs are. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] [PATCH] Remove useless USE_PGXS support in contrib
Le samedi 15 juin 2013 23:45:21, Andrew Dunstan a écrit : > On 06/15/2013 02:43 PM, David E. Wheeler wrote: > > On Jun 15, 2013, at 4:12 AM, Andrew Dunstan wrote: > >>REGRESS_OPTS = --inputdir=test --outputdir=test \ > >> > >> --load-extension=$(EXTENSION) > >> > >>... > >>override pg_regress_clean_files = test/results/ > >>test/regression.diffs test/regression.out tmp_check/ log/ > >> > >> That keeps the testing stuff out of the way quite nicely. > > > > I don't suppose there could be a way for the makefile to notice the > > --outputdir option and add those files to the clean target itself, could > > there? Having it hard-coded is slightly annoying. Maybe it could ask > > pg_regress where to find them? > > That doesn't sound like a promising line of development to me. Better > would be to provide a PGXS option to specify where tests are based, and > set the clean target accordingly. > > Then instead of the above you'd just be able to say something like > > MODULETEST = test or REGRESSDIR ? Also I suggest to remove the need to set REGRESS at all, and default to all sql files in REGRESSDIR/sql (if REGRESSDIR is set) Back to DOCS, we may also have PGXS default to find a README(.*) and rename it to README.$extension.$1 if MODULEDIR is not set. > REGRESS_OPTS = --load-extension=$(EXTENSION) > > Which would be a good deal cleaner. yes. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] [PATCH] Remove useless USE_PGXS support in contrib
> >> I don't really like the directory layout we use for these modules > >> anyway, so I'm not sure they constitute best practice for extension > >> builders. Lately I have been using an extension skeleton that looks > >> > >> something like this: > >> License > >> Readme.md > >> META.json (for pgxn) > >> extension.control > >> Makefile > >> doc/extension.md (soft linked to ../Readme.md) > > > > This makes mandatory to have a MODULEDIR defined or a rule to rename it > > with the extension name suffixed. > > Of course, for extension foo this would actually be foo.md. It installs > just fine like that. The makefile template has: > > DOCS = $(wildcard doc/*.md) Oh! yes, I missed the soft link. > >> src/extension.c > >> sql/extension.sql > > > > It is (was) the default place for regression testsI am not sure it is > > a good thing to shuffle that. Also, you don't do 'c/source.c' > > The sql here is the sql to install the extension, not part of the build > nor part of the tests. I am interested by this topic, since we have Extensions we invite users to increase the usage of them. So going a step forward with a better layout is definitively something to do. What do you suggest for the previous usage ? we have a hard rule to try to put libdir in *sql.in files for example. > Some time ago I fixed pg_regress to honor --inputdir and --outputdir > properly, so my Makefile template has this: > > REGRESS_OPTS = --inputdir=test --outputdir=test \ >--load-extension=$(EXTENSION) > ... > override pg_regress_clean_files = test/results/ > test/regression.diffs test/regression.out tmp_check/ log/ > > > That keeps the testing stuff out of the way quite nicely. > > You might not like this pattern, but I find it much saner that what we > currently use. I certainly don't claim it's perfect. I am interested by this topic, since we have Extensions we invite users to increase the usage of them. So going a step forward with a better layout is definitively something to do. I have no strong assumption on what the ideal layout is, 'your' and pgxn layout are good and I won't vote against suggesting to use them (and improve PGXS to match those suggestions). -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] [PATCH] Remove useless USE_PGXS support in contrib
Andrew Dunstan a écrit : > >On 06/14/2013 08:35 AM, Peter Eisentraut wrote: >> On 6/13/13 9:20 PM, amul sul wrote: >>> Agree, only if we consider these contrib module is always gonna >deployed with the postgresql. >>> But, what if user going to install such module elsewhere i.e. not >from contrib directory of pg source. >> Why would anyone do that? >> >> > >Maybe they wouldn't. > >I do think we need to make sure that we have at least buildfarm >coverage >of pgxs module building and testing. I have some coverage of a few >extensions I have written, which exercise that, so maybe that will >suffice. If not, maybe we need to have one module that only builds via >pgxs and is build after an install (i.e. not via the standard contrib >build). I agree, I found very useful to have all the provided extensions build with PGXS to debug it. It also offers a good set of natural regression tests. >I don't really like the directory layout we use for these modules >anyway, so I'm not sure they constitute best practice for extension >builders. Lately I have been using an extension skeleton that looks >something like this: > > License > Readme.md > META.json (for pgxn) > extension.control > Makefile > doc/extension.md (soft linked to ../Readme.md) This makes mandatory to have a MODULEDIR defined or a rule to rename it with the extension name suffixed. > src/extension.c > sql/extension.sql It is (was) the default place for regression testsI am not sure it is a good thing to shuffle that. Also, you don't do 'c/source.c' -- Envoyé de mon téléphone, excusez la brièveté. -- Sent 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 transforms feature
Peter Eisentraut a écrit : >A transform is an SQL object that supplies to functions for converting >between data types and procedural languages. For example, a transform >could arrange that hstore is converted to an appropriate hash or >dictionary object in PL/Perl or PL/Python. Nice ! >Continued from 2013-01 commit fest. All known open issues have been >fixed. You kept PGXS style makefile... -- Envoyé de mon téléphone excusez la brièveté. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposed patch: remove hard-coded limit MAX_ALLOCATED_DESCS
Le samedi 8 juin 2013 23:27:16, Tom Lane a écrit : > =?iso-8859-1?q?C=E9dric_Villemain?= writes: > > I'm not sure of expected value of "max_safe_fds". Your patch now > > initialize with 5 slots instead of 10, if max_safe_fds is large maybe it > > is better to double the size each time we need instead of jumping > > directly to the largest size ? > > I don't see the point particularly. At the default value of > max_files_per_process (1000), the patch can allocate at most 500 array > elements, which on a 64-bit machine would probably be 16 bytes each > or 8KB total. The point was only if the original comment was still relevant. It seems it is just not accurate anymore. With patch I can union 492 csv tables instead of 32 with beta1. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Proposed patch: remove hard-coded limit MAX_ALLOCATED_DESCS
> > I just wonder about this statement that you removed: > > * Since we don't want to encourage heavy use of those functions, > > * it seems OK to put a pretty small maximum limit on the number of > > * simultaneously allocated descs. > > > Is it now encouraged to use those functions, or at least that it seems less > > 'scary' than in the past ? > > Well, we could put back some weaker form of the statement, but I wasn't > sure how to word it. I'm not sure of the 'expected' problems... The only thing I can think of at the moment is the time spent to close file descriptors on abort/commit. I'm not sure of expected value of "max_safe_fds". Your patch now initialize with 5 slots instead of 10, if max_safe_fds is large maybe it is better to double the size each time we need instead of jumping directly to the largest size ? -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation
Re: [HACKERS] Proposed patch: remove hard-coded limit MAX_ALLOCATED_DESCS
Le samedi 8 juin 2013 19:06:58, Tom Lane a écrit : > Recently we had a gripe about how you can't read very many files > concurrently with contrib/file_fdw: > http://www.postgresql.org/message-id/OF419B5767.8A3C9ADB-ON85257B79.005491E > 9-85257b79.0054f...@isn.rtss.qc.ca > > The reason for this is that file_fdw goes through the COPY code, which > uses AllocateFile(), which has a wired-in assumption that not very many > files need to be open concurrently. I thought for a bit about trying to > get COPY to use a "virtual" file descriptor such as is provided by the > rest of fd.c, but that didn't look too easy, and in any case probably > nobody would be excited about adding additional overhead to the COPY > code path. What seems more practical is to relax the hard-coded limit > on the number of files concurrently open through AllocateFile(). Now, > we could certainly turn that array into some open-ended list structure, > but that still wouldn't let us have an arbitrary number of files open, > because at some point we'd run into platform-specific EMFILES or ENFILES > limits on the number of open file descriptors. In practice we want to > keep it under the max_safe_fds limit that fd.c goes to a lot of trouble > to determine. So it seems like the most useful compromise is to keep > the allocatedDescs[] array data structure as-is, but allow its size to > depend on max_safe_fds. In the attached proposed patch I limit it to > max_safe_fds / 2, so that there's still a reasonable number of FDs > available for fd.c's main pool of virtual FDs. On typical modern > platforms that should be at least a couple of hundred, thus making the > effective limit about an order of magnitude more than it is currently > (32). > > Barring objections or better ideas, I'd like to back-patch this as far > as 9.1 where file_fdw was introduced. I just wonder about this statement that you removed: * Since we don't want to encourage heavy use of those functions, * it seems OK to put a pretty small maximum limit on the number of * simultaneously allocated descs. Is it now encouraged to use those functions, or at least that it seems less 'scary' than in the past ? -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Configurable location for extension .control files
Hello > I am working with the NixOS Linux Distribution [nixos], which has a > fairly radical approach to package management. If you aren't familiar > with it, essentially all packages are installed in isolation - such that > packages cannot interfere with each other. good. > This is causing a bit of a problem with PostgreSQL extensions that are > usually installed via CREATE EXTENSION. For extensions to be used, a > .control file must be placed in SHAREDIR/extension, but this is not > possible under NixOS as once PostgreSQL is installed that directory is > essentially immutable. What about shared objects, .sql files and documentation an extension may have to install ? > What wolud work best for us is to allow this path to be configurable, > ideally through either an environment variable, command line switch, or > (and this is the least desirable) a postgresql.conf option. There is another point into allowing installation in different path : "make check" and "make installcheck" targets... > Would love to hear your thoughts. Once I get confirmation on the best > approach to take, I can try my hand at providing a patch. No idea on the best approach yet. But I am interested in this topic (for debian packaging). PS: I have a serie of bugfix and patches pending in the current commitfest (http://commitfest.postgresql.org) to help build with VPATH. You may be interested in them... -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] PostgreSQL 9.3 beta breaks some extensions "make install"
Le mardi 28 mai 2013 15:15:55, Cédric Villemain a écrit : > Le samedi 25 mai 2013 16:41:24, Cédric Villemain a écrit : > > > > If it seems to be on the right way, I'll keep fixing EXTENSION > > > > building with VPATH. > > > > > > I haven't tried the patch, but let me just say that Debian (and > > > apt.postgresql.org) would very much like the VPATH situation getting > > > improved. At the moment we seem to have to invent a new build recipe > > > for every extension around. > > Attached patch adds support for VPATH with USE_PGXS > It just change recipe for install: in pgxs.mk. > > I am unsure automatic variables can be used this way with all UNIX > variation of make... > > I also didn't touch MODULE and PROGRAM (yet) This patch can also be seen as a bugfix. The problem is that in case like this one: === FOO=bar.control installcontrol: $(FOO) $(INSTALL_DATA) $(FOO) \ '$(DESTDIR)$(datadir)/extension/' === INSTALL_DATA will install the file defined by FOO (=bar.control). But in the next case (the fix): === FOO=bar.control installcontrol: $(FOO) $(INSTALL_DATA) $< '$(DESTDIR)$(datadir)/extension/' === the $< contains the *filepath* where 'make' found the FOO file. I believe it is because recipes are read once target and prerequisite are set, so $< contains the full path to FOO, but FOO will still contain exactly what has been assigned to FOO. I choose to add targets for the variables that can be set when using PGXS. And say 'install:' target to depends on each. Maybe there is a smarter way to do it... my skills in Makefile are limited. So far the feedback is good for the set of patches: pg_buildext (the debian postgresql builder tool) is still working as expected and can be simplified. I can build each contrib provided with PostgreSQL and some others have been tried (like plr). IMHO this is mostly bugfixing and it just outline that we can support VPATH and PGXS build at the same time. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] PostgreSQL 9.3 beta breaks some extensions "make install"
Le mardi 28 mai 2013 14:10:14, Cédric Villemain a écrit : > > I just took time to inspect our contribs, USE_PGXS is not supported by > > all of them atm because of SHLIB_PREREQS (it used submake) I have a > > patch pending here to fix that. > > Attached patch fix SHLIB_PREREQS when building with USE_PGXS > > commit 19e231b introduced SHLIB_PREREQS but failed to port that to PGXS > build. > > The issue is that "submake-*" can not be built with PGXS, in this case they > must check that expected files are present (and installed). > Maybe it is better to only check if they have been built ? > > This fix the build of dblink and postgres_fdw (make USE_PGXS=1) This patch is a bugfix and IMO should be applied in 9.1 and 9.2. Else we should remove the PGXS support from the Makefile of the contribs dblink and postgres_fdw because it is not working currently. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] PostgreSQL 9.3 beta breaks some extensions "make install"
Le jeudi 30 mai 2013 14:32:46, Stefan Kaltenbrunner a écrit : > On 05/29/2013 06:08 PM, Cédric Villemain wrote: > >> I just took time to inspect our contribs, USE_PGXS is not supported by > >> all of them atm because of SHLIB_PREREQS (it used submake) I have a > >> patch pending here to fix that. Once all our contribs can build with > >> USE_PGXS I fix the VPATH. > > > > I've added 'most' of the patches to the commitfest... (I am not sure it > > is required, as this is more bugfix than anything else IMHO) > > See > > https://commitfest.postgresql.org/action/patch_view?id=1122 > > https://commitfest.postgresql.org/action/patch_view?id=1123 > > https://commitfest.postgresql.org/action/patch_view?id=1124 > > > > > > I stopped trying to add new item after too many failures from > > https://commitfest.postgresql.org/action/patch_form > > So one patch is not in the commitfest yet (fix_install_ext_vpath.patch) > > "failures"? what kind of issues did you experience? I didn't sent too much details as I am not sure if it was my setup which breaks things or not. [...] Just tested with Stephen, looks like a problem with something on my side, sorry for the noise. (rekonq 0 - chromium 1) -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] PostgreSQL 9.3 beta breaks some extensions "make install"
> I just took time to inspect our contribs, USE_PGXS is not supported by all > of them atm because of SHLIB_PREREQS (it used submake) I have a patch > pending here to fix that. Once all our contribs can build with USE_PGXS I > fix the VPATH. I've added 'most' of the patches to the commitfest... (I am not sure it is required, as this is more bugfix than anything else IMHO) See https://commitfest.postgresql.org/action/patch_view?id=1122 https://commitfest.postgresql.org/action/patch_view?id=1123 https://commitfest.postgresql.org/action/patch_view?id=1124 I stopped trying to add new item after too many failures from https://commitfest.postgresql.org/action/patch_form So one patch is not in the commitfest yet (fix_install_ext_vpath.patch) -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] PostgreSQL 9.3 beta breaks some extensions "make install"
Le samedi 25 mai 2013 16:41:24, Cédric Villemain a écrit : > > > If it seems to be on the right way, I'll keep fixing EXTENSION building > > > with VPATH. > > > > I haven't tried the patch, but let me just say that Debian (and > > apt.postgresql.org) would very much like the VPATH situation getting > > improved. At the moment we seem to have to invent a new build recipe > > for every extension around. Attached patch adds support for VPATH with USE_PGXS It just change recipe for install: in pgxs.mk. I am unsure automatic variables can be used this way with all UNIX variation of make... I also didn't touch MODULE and PROGRAM (yet) -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk index 31746f3..2575855 100644 --- a/src/makefiles/pgxs.mk +++ b/src/makefiles/pgxs.mk @@ -121,33 +121,40 @@ all: all-lib endif # MODULE_big -install: all installdirs -ifneq (,$(EXTENSION)) - $(INSTALL_DATA) $(addprefix $(srcdir)/, $(addsuffix .control, $(EXTENSION))) '$(DESTDIR)$(datadir)/extension/' -endif # EXTENSION -ifneq (,$(DATA)$(DATA_built)) - $(INSTALL_DATA) $(addprefix $(srcdir)/, $(DATA)) $(DATA_built) '$(DESTDIR)$(datadir)/$(datamoduledir)/' -endif # DATA -ifneq (,$(DATA_TSEARCH)) - $(INSTALL_DATA) $(addprefix $(srcdir)/, $(DATA_TSEARCH)) '$(DESTDIR)$(datadir)/tsearch_data/' -endif # DATA_TSEARCH +install: all installdirs installcontrol installdata installdatatsearch installdocs installscripts ifdef MODULES $(INSTALL_SHLIB) $(addsuffix $(DLSUFFIX), $(MODULES)) '$(DESTDIR)$(pkglibdir)/' endif # MODULES +ifdef PROGRAM + $(INSTALL_PROGRAM) $(PROGRAM)$(X) '$(DESTDIR)$(bindir)' +endif # PROGRAM + +installcontrol: $(addsuffix .control, $(EXTENSION)) +ifneq (,$(EXTENSION)) + $(INSTALL_DATA) $< '$(DESTDIR)$(datadir)/extension/' +endif + +installdata: $(DATA) $(DATA_built) +ifneq (,$(DATA)$(DATA_built)) + $(INSTALL_DATA) $^ '$(DESTDIR)$(datadir)/$(datamoduledir)/' +endif + +installdatatsearch: $(DATA_TSEARCH) +ifneq (,$(DATA_TSEARCH)) + $(INSTALL_DATA) $^ '$(DESTDIR)$(datadir)/tsearch_data/' +endif + +installdocs: $(DOCS) ifdef DOCS ifdef docdir - $(INSTALL_DATA) $(addprefix $(srcdir)/, $(DOCS)) '$(DESTDIR)$(docdir)/$(docmoduledir)/' + $(INSTALL_DATA) $^ '$(DESTDIR)$(docdir)/$(docmoduledir)/' endif # docdir endif # DOCS -ifdef PROGRAM - $(INSTALL_PROGRAM) $(PROGRAM)$(X) '$(DESTDIR)$(bindir)' -endif # PROGRAM + +installscripts: $(SCRIPTS) $(SCRIPTS_built) ifdef SCRIPTS - $(INSTALL_SCRIPT) $(addprefix $(srcdir)/, $(SCRIPTS)) '$(DESTDIR)$(bindir)/' + $(INSTALL_SCRIPT) $^ '$(DESTDIR)$(bindir)/' endif # SCRIPTS -ifdef SCRIPTS_built - $(INSTALL_SCRIPT) $(SCRIPTS_built) '$(DESTDIR)$(bindir)/' -endif # SCRIPTS_built ifdef MODULE_big install: install-lib signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] PostgreSQL 9.3 beta breaks some extensions "make install"
Le mardi 28 mai 2013 14:16:38, Cédric Villemain a écrit : > > Once all our contribs can build with USE_PGXS I > > fix the VPATH. > > > > The last step is interesting: installcheck/REGRESS. For this one, if I > > can know exactly what's required (for debian build for example), then I > > can also fix this target. > > There is a hack to link the regression data files from the srcdir > to the builddir when doing 'make VPATH'. but it failed when used in > conjunction with USE_PGXS and out-of-tree build of an extension. > > Issue is the absence of the data/ directory in the builddir. > > Attached patch fix that. use $(MKDIR_P) instead of mkdir -p -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk index bbcfe04..e8ff584 100644 --- a/src/makefiles/pgxs.mk +++ b/src/makefiles/pgxs.mk @@ -263,6 +263,7 @@ test_files_build := $(patsubst $(srcdir)/%, $(abs_builddir)/%, $(test_files_src) all: $(test_files_build) $(test_files_build): $(abs_builddir)/%: $(srcdir)/% + $(MKDIR_P) '$(dir $@)' ln -s $< $@ endif # VPATH signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] PostgreSQL 9.3 beta breaks some extensions "make install"
> Once all our contribs can build with USE_PGXS I > fix the VPATH. Attached patch set VPATH for out-of-tree extension builds If the makefile is not in the current directory (where we launch 'make') then assume we are building out-of-src tree and set the VPATH to the directory of the *first* makefile... Thus it fixes: mkdir /tmp/a && cd /tmp/a make -f extension_src/Makefile USE_PGXS=1 Note that the patch fix things. Still I am not really happy with the rule to get the srcdir. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk index e8ff584..64732ff 100644 --- a/src/makefiles/pgxs.mk +++ b/src/makefiles/pgxs.mk @@ -61,9 +61,18 @@ ifdef PGXS top_builddir := $(dir $(PGXS))../.. include $(top_builddir)/src/Makefile.global +# If Makefile is not in current directory we are building the extension with +# VPATH so we set the variable here +# XXX what about top_srcdir ? +ifeq ($(CURDIR),$(dir $(firstword $(MAKEFILE_LIST top_srcdir = $(top_builddir) srcdir = . VPATH = +else +top_srcdir = $(top_builddir) +srcdir = $(dir $(firstword $(MAKEFILE_LIST))) +VPATH = $(dir $(firstword $(MAKEFILE_LIST))) +endif # These might be set in Makefile.global, but if they were not found # during the build of PostgreSQL, supply default values so that users signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] PostgreSQL 9.3 beta breaks some extensions "make install"
> Once all our contribs can build with USE_PGXS I > fix the VPATH. > > The last step is interesting: installcheck/REGRESS. For this one, if I can > know exactly what's required (for debian build for example), then I can > also fix this target. There is a hack to link the regression data files from the srcdir to the builddir when doing 'make VPATH'. but it failed when used in conjunction with USE_PGXS and out-of-tree build of an extension. Issue is the absence of the data/ directory in the builddir. Attached patch fix that. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk index bbcfe04..e8ff584 100644 --- a/src/makefiles/pgxs.mk +++ b/src/makefiles/pgxs.mk @@ -263,6 +263,7 @@ test_files_build := $(patsubst $(srcdir)/%, $(abs_builddir)/%, $(test_files_src) all: $(test_files_build) $(test_files_build): $(abs_builddir)/%: $(srcdir)/% + mkdir -p $(dir $@) ln -s $< $@ endif # VPATH signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] PostgreSQL 9.3 beta breaks some extensions "make install"
> I just took time to inspect our contribs, USE_PGXS is not supported by all > of them atm because of SHLIB_PREREQS (it used submake) I have a patch > pending here to fix that. Attached patch fix SHLIB_PREREQS when building with USE_PGXS commit 19e231b introduced SHLIB_PREREQS but failed to port that to PGXS build. The issue is that "submake-*" can not be built with PGXS, in this case they must check that expected files are present (and installed). Maybe it is better to only check if they have been built ? This fix the build of dblink and postgres_fdw (make USE_PGXS=1) -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation diff --git a/src/Makefile.global.in b/src/Makefile.global.in index 89e39d2..1b13f85 100644 --- a/src/Makefile.global.in +++ b/src/Makefile.global.in @@ -415,13 +415,24 @@ libpq_pgport = -L$(top_builddir)/src/port -lpgport \ -L$(top_builddir)/src/common -lpgcommon $(libpq) endif - +# If PGXS is not defined, builds as usual: +# build dependancies required by SHLIB_PREREQS +# If the build is with PGXS, then any requirement is supposed to be already +# build and we just take care that the expected files exist +ifndef PGXS submake-libpq: $(MAKE) -C $(libpq_builddir) all +else +submake-libpq: $(libdir)/libpq.so ; +endif +ifndef PGXS submake-libpgport: $(MAKE) -C $(top_builddir)/src/port all $(MAKE) -C $(top_builddir)/src/common all +else +submake-libpgport: $(libdir)/libpgport.a $(libdir)/libpgcommon.a ; +endif .PHONY: submake-libpq submake-libpgport signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] PostgreSQL 9.3 beta breaks some extensions "make install"
> > If it seems to be on the right way, I'll keep fixing EXTENSION building > > with VPATH. > > I haven't tried the patch, but let me just say that Debian (and > apt.postgresql.org) would very much like the VPATH situation getting > improved. At the moment we seem to have to invent a new build recipe > for every extension around. I have been busy the last week. I just took time to inspect our contribs, USE_PGXS is not supported by all of them atm because of SHLIB_PREREQS (it used submake) I have a patch pending here to fix that. Once all our contribs can build with USE_PGXS I fix the VPATH. The last step is interesting: installcheck/REGRESS. For this one, if I can know exactly what's required (for debian build for example), then I can also fix this target. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] request a new feature in fuzzystrmatch
Hello Liming, > >> Sounds interesting. How can we build this over our current > >> implementation, or do we need to build it from scratch? > >> > > I know how to write the code, but just need approval of accepting into > > the new version. > > Well, acceptance depends largely on the implementation and actual > benefit statistics. I would suggest implementing a basic version and > then demonstrating its potential benefits here. It will lead to > clearer ideas for us and lead to improvements in the implementation. You can have a look at this page: http://wiki.postgresql.org/wiki/Submitting_a_Patch -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] PostgreSQL 9.3 beta breaks some extensions "make install"
Le jeudi 16 mai 2013 18:53:19, Alvaro Herrera a écrit : > Andrew Dunstan wrote: > > On 05/16/2013 10:39 AM, Cédric Villemain wrote: > > >Le jeudi 16 mai 2013 15:51:48, Tom Lane a écrit : > > >>Andrew Dunstan writes: > > >>>On 05/16/2013 05:41 AM, Dimitri Fontaine wrote: > > >>>>And VPATH building of extension is crucially important for me, as the > > >>>>easiest way I've found to build and package a given extension against > > >>>>all currently supported version of PostgreSQL. > > >>> > > >>>Is there documented support for VPATH builds? > > >> > > >>The core code certainly builds okay in VPATH mode, and I would agree > > >>with Dimitri that pgxs builds should as well. But this is more of an > > >>autoconf+make feature than ours, so I'm not sure why you'd expect us > > >>to document it. > > > > > >Extension does not support VPATH at 100% (well, pgxs.mk). > > >There is a minor hack for some REGRESS files but that's all. > > > > Right. My impression is that pgxs.mk actively works against vpath builds. > > That's my experience, yes. It would be great to get it fixed. OK. I've reduce that to the attached (WIP) patch . Problem is usage of $(srcdir) in the PGXS case. I try to remove it in favor of recipes for each case (DATA, DOCS, ...) It's a quick hack... It does not handle some cases yet (for example it does not find .o when come the time to build .so if the .o was already in VPATH), this later issue is a separate one I think. If it seems to be on the right way, I'll keep fixing EXTENSION building with VPATH. Comments ? -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk index bbcfe04..bc01e0c 100644 --- a/src/makefiles/pgxs.mk +++ b/src/makefiles/pgxs.mk @@ -112,32 +112,44 @@ all: all-lib endif # MODULE_big -install: all installdirs -ifneq (,$(EXTENSION)) - $(INSTALL_DATA) $(addprefix $(srcdir)/, $(addsuffix .control, $(EXTENSION))) '$(DESTDIR)$(datadir)/extension/' -endif # EXTENSION -ifneq (,$(DATA)$(DATA_built)) - $(INSTALL_DATA) $(addprefix $(srcdir)/, $(DATA)) $(DATA_built) '$(DESTDIR)$(datadir)/$(datamoduledir)/' -endif # DATA -ifneq (,$(DATA_TSEARCH)) - $(INSTALL_DATA) $(addprefix $(srcdir)/, $(DATA_TSEARCH)) '$(DESTDIR)$(datadir)/tsearch_data/' -endif # DATA_TSEARCH +install: all installdirs installcontrol installdata installdatatsearch installdocs installscript installscriptbuilt ifdef MODULES $(INSTALL_SHLIB) $(addsuffix $(DLSUFFIX), $(MODULES)) '$(DESTDIR)$(pkglibdir)/' endif # MODULES +ifdef PROGRAM + $(INSTALL_PROGRAM) $(PROGRAM)$(X) '$(DESTDIR)$(bindir)' +endif # PROGRAM + +installcontrol: $(addsuffix .control, $(EXTENSION)) +ifneq (,$(EXTENSION)) + $(INSTALL_DATA) $< '$(DESTDIR)$(datadir)/extension/' +endif + +installdata: $(DATA) $(DATA_built) +ifneq (,$(DATA)$(DATA_built)) + $(INSTALL_DATA) $^ '$(DESTDIR)$(datadir)/$(datamoduledir)/' +endif + +installdatatsearch: $(DATA_TSEARCH) +ifneq (,$(DATA_TSEARCH)) + $(INSTALL_DATA) $^ '$(DESTDIR)$(datadir)/tsearch_data/' +endif + +installdocs: $(DOCS) ifdef DOCS ifdef docdir - $(INSTALL_DATA) $(addprefix $(srcdir)/, $(DOCS)) '$(DESTDIR)$(docdir)/$(docmoduledir)/' + $(INSTALL_DATA) $^ '$(DESTDIR)$(docdir)/$(docmoduledir)/' endif # docdir endif # DOCS -ifdef PROGRAM - $(INSTALL_PROGRAM) $(PROGRAM)$(X) '$(DESTDIR)$(bindir)' -endif # PROGRAM + +installscript: $(SCRIPTS) ifdef SCRIPTS - $(INSTALL_SCRIPT) $(addprefix $(srcdir)/, $(SCRIPTS)) '$(DESTDIR)$(bindir)/' + $(INSTALL_SCRIPT) $^ '$(DESTDIR)$(bindir)/' endif # SCRIPTS -ifdef SCRIPTS_built - $(INSTALL_SCRIPT) $(SCRIPTS_built) '$(DESTDIR)$(bindir)/' + +installscriptbuilt: $(SCRIPTS_built) +ifdef SCRIPTS + $(INSTALL_SCRIPT) $^ '$(DESTDIR)$(bindir)/' endif # SCRIPTS_built ifdef MODULE_big signature.asc Description: This is a digitally signed message part.
Re: [GENERAL] [HACKERS] PLJava for Postgres 9.2.
> Yes, am aware PLJava is a 3rd party lib, just surprised the same party > hasn't built them given they seem to be built all the way to 9.1. > > My question was primarily about obtaining pgsx.mk file which is a part of > the PostgreSQL project. With linux you do something like that for pljava $ make PG_CONFIG=/usr/pgsql-9.2/bin/pg_config \ JAVA_HOME=/usr/java/default The pg_config is used to find the pgxs.mk (the real command is: «pg_config -- pgxs»). > > Paul > > > > From: Andrew Dunstan > To: Paul Hammond > Cc: "pgsql-hackers@postgresql.org" ; > "pgsql-gene...@postgresql.org" Sent: > Friday, 17 May 2013, 0:03 > Subject: Re: [GENERAL] [HACKERS] PLJava for Postgres 9.2. > > On 05/16/2013 05:59 PM, Paul Hammond wrote: > > Hi all, > > > > I've downloaded PLJava, the latest version, which doesn't seem to have > > a binary distribution at all for 9.2, so I'm trying to build it from > > the source for Postgres 9.2. I have the DB itself installed on Windows > > 7 64 bit as a binary install. I've had to do a fair bit of hacking > > with the makefiles on cygwin to get PLJava to build, but I have > > succeeded in compiling the Java and JNI code, the pljava_all and > > deploy_all targets effectively. > > Cygwin is not a recommended build platform for native Windows builds. > See the docs for the recommended ways to build Postgres. > > > But I'm coming unstuck at the next target where it's doing the target > > c_all. It's trying to find the following makefile in the Postgres dist: > > > > /lib/pgxs/src/makefiles/pgxs.mk: No such > > file or directory > > > > What do I need to do to obtain the required files, and does anybody > > know why, given Postgres 9.2 is out some time, and 9.3 is in beta, why > > no prebuild binary PLJavas exist for 9.2? > > Because nobody has built them? > > > FYI, PL/Java is not maintained by the PostgreSQL project. > > > cheers > > andrew -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] PostgreSQL 9.3 beta breaks some extensions "make install"
Le jeudi 16 mai 2013 15:51:48, Tom Lane a écrit : > Andrew Dunstan writes: > > On 05/16/2013 05:41 AM, Dimitri Fontaine wrote: > >> And VPATH building of extension is crucially important for me, as the > >> easiest way I've found to build and package a given extension against > >> all currently supported version of PostgreSQL. > > > > Is there documented support for VPATH builds? > > The core code certainly builds okay in VPATH mode, and I would agree > with Dimitri that pgxs builds should as well. But this is more of an > autoconf+make feature than ours, so I'm not sure why you'd expect us > to document it. Extension does not support VPATH at 100% (well, pgxs.mk). There is a minor hack for some REGRESS files but that's all. I think at least DOCS, DATA and REGRESS needs some addition on pgxs.mk to help extension author allows build out-of-tree (postgresql been built out or not). I'll work on a patch for that. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] PostgreSQL 9.3 beta breaks some extensions "make install"
Le mercredi 15 mai 2013 16:43:17, Andrew Dunstan a écrit : > On 05/15/2013 10:05 AM, Tom Lane wrote: > > Peter Eisentraut writes: > >> That said, I'm obviously outnumbered here. What about the following > >> compromise: Use the configure-selected install program inside > >> PostgreSQL (which we can test easily), and use install-sh under > >> USE_PGXS? Admittedly, the make install time of extensions is probably > >> not an issue. > > > > That works for me, since as you say we can easily fix any such bugs > > in the core code. The scary thing about this for extension authors > > is that they may very well see no bug in their own testing, only to > > have their packages fall over in the wild. We shouldn't make each > > author who's copied that code rediscover the problem for themselves > > that expensively. > > +1, although I will be renovating the Makefiles for all my extensions > along the lines of my previous email. pgxs.mk has some old rules to replace $libdir (and some few other maybe). Maybe we can try to find what make sense for most of the extension authors and add rules/target to pgxs.mk to reduce the useless copy/paste in the Makefile of extensions. So what's desirable ? -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] PostgreSQL 9.3 beta breaks some extensions "make install"
Le mardi 14 mai 2013 15:08:42, Andrew Dunstan a écrit : > On 05/14/2013 07:59 AM, Peter Eisentraut wrote: > > On 5/14/13 4:17 AM, Marti Raudsepp wrote: > >> On Tue, May 14, 2013 at 5:27 AM, Peter Eisentraut wrote: > >>> On Tue, 2013-05-14 at 04:12 +0300, Marti Raudsepp wrote: > >>>> It's caused by this common pattern in extension makefiles: > >>>> DATA = $(wildcard sql/*--*.sql) sql/$(EXTENSION)--$(EXTVERSION).sql > >>> > >>> What is the point of this? Why have the wildcard and then the > >>> non-wildcard term? > >> > >> Because the non-wildcard file is built by the same Makefile (it's > >> copied from the sql/$(EXTENSION).sql file). If it wasn't there, a > >> "make install" from a clean checkout would miss this file. > > > > If it's built, then it should be listed in DATA_built. > > So, AIUI, leaving aside stylistic arguments about use of wildcards, one > solution could be: > > DATA_built = sql/$(EXTENSION)--$(EXTVERSION).sql > DATA = $(filter-out sql/$(EXTENSION)--$(EXTVERSION).sql, $(wildcard > sql/*--*.sql)) > > Is that right? Yes. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] PostgreSQL 9.3 beta breaks some extensions "make install"
Le mardi 14 mai 2013 10:17:13, Marti Raudsepp a écrit : > On Tue, May 14, 2013 at 5:27 AM, Peter Eisentraut wrote: > > On Tue, 2013-05-14 at 04:12 +0300, Marti Raudsepp wrote: > >> It's caused by this common pattern in extension makefiles: > >> DATA = $(wildcard sql/*--*.sql) sql/$(EXTENSION)--$(EXTVERSION).sql > > > > What is the point of this? Why have the wildcard and then the > > non-wildcard term? > > Because the non-wildcard file is built by the same Makefile (it's > copied from the sql/$(EXTENSION).sql file). If it wasn't there, a > "make install" from a clean checkout would miss this file. > > all: sql/$(EXTENSION)--$(EXTVERSION).sql > > sql/$(EXTENSION)--$(EXTVERSION).sql: sql/$(EXTENSION).sql > cp $< $@ There is also something else here, the addition of the all: target. I believe it should be removed and the added target(s) be written after the include of pgxs makefile. If I follow your example, then I would rewrite http://manager.pgxn.org/howto From = PG91 = $(shell $(PG_CONFIG) --version | grep -qE " 8\.| 9\.0" && echo no || echo yes) ifeq ($(PG91),yes) all: sql/$(EXTENSION)--$(EXTVERSION).sql sql/$(EXTENSION)--$(EXTVERSION).sql: sql/$(EXTENSION).sql cp $< $@ DATA = $(wildcard sql/*--*.sql) sql/$(EXTENSION)--$(EXTVERSION).sql EXTRA_CLEAN = sql/$(EXTENSION)--$(EXTVERSION).sql endif PGXS := $(shell $(PG_CONFIG) --pgxs) include $(PGXS) To PG91 = $(shell $(PG_CONFIG) --version | grep -qE " 8\.| 9\.0" && echo no || echo yes) ifeq ($(PG91),yes) DATA = $(wildcard sql/*--*.sql) sql/$(EXTENSION)--$(EXTVERSION).sql EXTRA_CLEAN = sql/$(EXTENSION)--$(EXTVERSION).sql endif PGXS := $(shell $(PG_CONFIG) --pgxs) include $(PGXS) ifeq ($(PG91),yes) sql/$(EXTENSION)--$(EXTVERSION).sql: sql/$(EXTENSION).sql cp $< $@ endif because the default target from the PostgreSQL Makefile is «all:» and the addition of a target before inclusion of PostgreSQL makefile change the default (see DEFAULT_GOAL variable) Thought ? -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] PostgreSQL 9.3 beta breaks some extensions "make install"
Le mardi 14 mai 2013 10:17:13, Marti Raudsepp a écrit : > On Tue, May 14, 2013 at 5:27 AM, Peter Eisentraut wrote: > > On Tue, 2013-05-14 at 04:12 +0300, Marti Raudsepp wrote: > >> It's caused by this common pattern in extension makefiles: > >> DATA = $(wildcard sql/*--*.sql) sql/$(EXTENSION)--$(EXTVERSION).sql > > > > What is the point of this? Why have the wildcard and then the > > non-wildcard term? > > Because the non-wildcard file is built by the same Makefile (it's > copied from the sql/$(EXTENSION).sql file). If it wasn't there, a > "make install" from a clean checkout would miss this file. > > all: sql/$(EXTENSION)--$(EXTVERSION).sql > > sql/$(EXTENSION)--$(EXTVERSION).sql: sql/$(EXTENSION).sql > cp $< $@ > > > I think using wildcard is bad style and you should fix it. > > Perhaps, but fixing the extensions is not a solution at this point. A > large number of extensions use this exact code (it comes from David > Wheeler's template AFAIK). We might stand a chance in fixing the > public extensions on PGXN, but this would presumably still break > non-public extensions that people have written. My understanding is that the offending commit reveals possible bad written instruction in some Makefile. What's wrong ? -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Recovery target 'immediate'
Le vendredi 3 mai 2013 15:40:51, Heikki Linnakangas a écrit : > On 03.05.2013 16:29, Bruce Momjian wrote: > > On Fri, May 3, 2013 at 01:02:08PM +0200, Cédric Villemain wrote: > >>>>>> This changes the existing API which will confuse people that know it > >>>>>> and invalidate everything written in software and on wikis as to how > >>>>>> to do it. That means all the "in case of fire break glass" > >>>>>> instructions are all wrong and need to be rewritten and retested. > >>>>> > >>>>> Yes, *that* is the main reason *not* to make the change. It has a > >>>>> pretty bad cost in backwards compatibility loss. There is a gain, but > >>>>> I don't think it outweighs the cost. > >>>> > >>>> So, is there a way to add this feature without breaking the API? > >>> > >>> Yes, by adding a new parameter exclusively used to control this > >>> feature, something like recovery_target_immediate = 'on/off'. > >> > >> We just need to add a named restore point when ending the backup (in > >> pg_stop_backup() ?). > >> No API change required. Just document that some predefined target names > >> are set during backup. > > > > So we auto-add a restore point based on the backup label. Does that > > work for everyone? > > Unfortunately, no. There are cases where you want to stop right after > reaching consistency, but the point where you reach consistency is not > at the end of a backup. For example, if you take a backup using an > atomic filesystem snapshot, there are no pg_start/stop_backup calls, and > the system will reach consistency after replaying all the WAL in > pg_xlog. You might think that you can just not create a recovery.conf > file in that case, or create a dummy recovery.conf file with > restore_command='/bin/false'. However, then the system will not find the > existing timeline history files in the archive, and can pick a TLI > that's already in use. I found this out the hard way, and actually ended > up writing a restore_command that restore timeline history files > normally, but returns non-zero for any real other files; it wasn't pretty. OK. I missed that you wanted that outside of pg_start/stop_backup() dance. > If we want to avoid adding a new option for this, how about a magic > restore point called "consistent" or "immediate": > > recovery_target_name='immediate' > > That would stop recovery right after reaching consistency, but there > wouldn't be an actual restore point record in the WAL stream. Back to your first email then. +1 (as pointed by Simon, this is something we must document well: stopping at 'immediate' is sure to reduce your chance of recovering all the possible data ... opposite to recovery_target_name=ultimate, the default ;) ) -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Recovery target 'immediate'
Le vendredi 3 mai 2013 02:54:15, Michael Paquier a écrit : > On Fri, May 3, 2013 at 8:56 AM, Bruce Momjian wrote: > > On Thu, May 2, 2013 at 09:31:03AM +0200, Magnus Hagander wrote: > > > Actually, there is - I hear it quite often from people not so > > > experienced in PostgreSQL. Though in fairness, I'm not entirely sure > > > the new syntax would help - some of those need a tool to do it for > > > them, really (and such tools exist, I believe). > > > > > > That said, there is one property that's very unclear now and that's > > > that you can only set one of recovery_target_time, recovery_target_xid > > > and recovery_target_name. But they can be freely combined with > > > recovery_target_timeline and recovery_target_inclusive. That's quite > > > confusing. > > > > > > > This changes the existing API which will confuse people that know it > > > > and invalidate everything written in software and on wikis as to how > > > > to do it. That means all the "in case of fire break glass" > > > > instructions are all wrong and need to be rewritten and retested. > > > > > > Yes, *that* is the main reason *not* to make the change. It has a > > > pretty bad cost in backwards compatibility loss. There is a gain, but > > > I don't think it outweighs the cost. > > > > So, is there a way to add this feature without breaking the API? > > Yes, by adding a new parameter exclusively used to control this feature, > something like recovery_target_immediate = 'on/off'. We just need to add a named restore point when ending the backup (in pg_stop_backup() ?). No API change required. Just document that some predefined target names are set during backup. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Interesting post-mortem on a near disaster with git
Le lundi 25 mars 2013 19:35:12, Daniel Farina a écrit : > On Mon, Mar 25, 2013 at 11:07 AM, Stefan Kaltenbrunner > > wrote: > >> Back when we used CVS for quite a few years I kept 7 day rolling > >> snapshots of the CVS repo, against just such a difficulty as this. But > >> we seem to be much better organized with infrastructure these days so I > >> haven't done that for a long time. > > > > well there is always room for improvement(and for learning from others) > > - but I agree that this proposal seems way overkill. If people think we > > should keep online "delayed" mirrors we certainly have the resources to > > do that on our own if we want though... > > What about rdiff-backup? I've set it up for personal use years ago > (via the handy open source bash script backupninja) years ago and it > has a pretty nice no-frills point-in-time, self-expiring, file-based > automatic backup program that works well with file synchronization > like rsync (I rdiff-backup to one disk and rsync the entire > rsync-backup output to another disk). I've enjoyed using it quite a > bit during my own personal-computer emergencies and thought the > maintenance required from me has been zero, and I have used it from > time to time to restore, proving it even works. Hardlinks can be used > to tag versions of a file-directory tree recursively relatively > compactly. > > It won't be as compact as a git-aware solution (since git tends to to > rewrite entire files, which will confuse file-based incremental > differential backup), but the amount of data we are talking about is > pretty small, and as far as a lowest-common-denominator tradeoff for > use in emergencies, I have to give it a lot of praise. The main > advantage it has here is it implements point-in-time recovery > operations that easy to use and actually seem to work. That said, > I've mostly done targeted recoveries rather than trying to recover > entire trees. I have the same set up, and same feedback. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Temporal features in PostgreSQL
Hello, I'm also interested in this topic. > > I'm also interested in this topic and work on system-time temporal > > extension. Here I wrote down design of my solution few months ago > > https://wiki.postgresql.org/wiki/SQL2011Temporal. The idea is > > basically the same as in your solution with some minor differences. I've added a requirement in the system here: the table to be versioned must have a PK (I dislike _entry_id usage but this sounds good othwise). I then define a "EXCLUDE WITH GIST (pk with =, sys_period with &&)", thus getting expected UNIQUEness also in the history. Vlad, is your source code in a public versionning system (github, bucket, etc) ? It will ease the process to participate to your extension... -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation
Re: [HACKERS] Request for vote to move forward with recovery.conf overhaul
Le mardi 22 janvier 2013 01:54:50, Michael Paquier a écrit : > On Tue, Jan 22, 2013 at 9:27 AM, Robert Haas wrote: > > > On Mon, Jan 21, 2013 at 6:23 PM, Michael Paquier > > wrote: > > > Yes, that is one of the most important patches in the list, and I could > > put > > > some effort in it for either review or coding. > > > > I think it would be great if you could elaborate on your reasons for > > feeling that this patch is particularly important. > > > Sure. recovery.conf has been initially created for PITR management, but > since 9.0 and the introduction of streaming replication it is being used > for too many things that it was first targeting for, like now it can be > used to define where a slave can connect to a root node, fetch the > archives, etc. I am seeing for a long time on hackers (2010?) that postgres > should make the move on giving up recovery.conf and merge it with > postgresql.conf. > > I didn't know about the existence of a patch aimed to merge the parameters > of postgresql.conf and recovery.conf, and, just by looking at the patch, > half of the work looks to be already done. I thought it might be worth to > at least update the patch or provide some feedback. > > I agree that this might break backward-compatibility and that it would be > more suited for a 10.0(?), but as 9.3 development is already close to its > end, progressing on this discussion and decide whether this could be > shipped for 9.3 or later release is important. If it is decided to give up > on this feature, well let's do that later. If it is worth the shot, let's > put some effort for it. I though the idea is that for 9.3 we can have new feature, so everything can go in postgreql.conf, and also allows using recovery.conf so it does not break backward-compatibility. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation
Re: [HACKERS] Makefiles don't seem to remember to rebuild everything anymore
Le lundi 17 décembre 2012 17:42:04, Tom Lane a écrit : > =?iso-8859-15?q?C=E9dric_Villemain?= writes: > > Le lundi 17 décembre 2012 16:45:16, Tom Lane a écrit : > >> Having seen this, I now think .SECONDARY is broken across the board. > > > > make does what it is supposed to do here IIUC. > > Well, it's behaving as documented, but what this means is we need to be > very wary of what contexts we can use .SECONDARY in. I'd just as soon > try to avoid using it at all --- my example with hand-edited gram.c > points out that even rather short dependency chains can have unexpected > misbehaviors if the intermediate files are marked .SECONDARY. "touch gram.c" then "make" will build again the affected files. Which is different from "rm gram.c"... A "touch gram.y" will trigger a rebuild of "gram.c" (and "gram.o"). Maybe Makefile has an option to be a little bit more conservative and rebuild any removed intermediate file even if not required. > > What was the consensus when Peter did the change ? > > It was an experiment, nothing more; or at least that's how I saw it. ok. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Makefiles don't seem to remember to rebuild everything anymore
Le lundi 17 décembre 2012 15:29:09, Andrew Dunstan a écrit : > On 12/17/2012 08:46 AM, Peter Eisentraut wrote: > > On 12/15/12 11:23 AM, Tom Lane wrote: > >> =?iso-8859-15?q?C=E9dric_Villemain?= writes: > >>> Le vendredi 14 décembre 2012 23:02:11, Tom Lane a écrit : > >>>> $ rm gram.o > >>>> rm: remove regular file `gram.o'? y > >>>> $ make > >>>> make: Nothing to be done for `all'. > >>>> > >>>> WTF? > >>> > >>> A previous patch changed the ".SECONDARY" from an if() section to the > >>> main section of src/Makefile.global.in, > > > > Although it's a bit odd, it's not really a problem, I think. If you > > want to rebuild analyze.o, you should write "make analyze.o". If you > > want to rebuild postgres, run make in src/backend, and analyze.o (or > > whatever) will be rebuilt. > > That's a pretty nasty violation of the POLA. If our leading developer > thinks something about our build process is a problem, it's a problem. That's not so obvious. The current behavior is expected by .SECONDARY. In other words, if I just 'touch rewriteDefine.c' then rewriteDefine.o will be rebuilt by make (as expected). $ touch rewriteDefine.c $ make gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after- statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno- strict-aliasing -fwrapv -fexcess-precision=standard -I../../../src/include - D_GNU_SOURCE -c -o rewriteDefine.o rewriteDefine.c touch objfiles.txt It is maybe better to do a special case when you want to force rebuild, but in a more intuitive way than specifying each file you want to rebuild. Like that ?! : diff --git a/src/Makefile.global.in b/src/Makefile.global.in index 9cc14da..8597792 100644 --- a/src/Makefile.global.in +++ b/src/Makefile.global.in @@ -31,8 +31,10 @@ all: # started to update the file. .DELETE_ON_ERROR: +ifndef NOTSECONDARY # Never delete any intermediate files automatically. .SECONDARY: +endif $ rm rewriteDefine.o $ make nothing to do ... $ NOTSECONDARY=1 make gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after- statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno- strict-aliasing -fwrapv -fexcess-precision=standard -I../../../src/include - D_GNU_SOURCE -c -o rewriteDefine.o rewriteDefine.c touch objfiles.txt -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
