Re: [HACKERS] WITHIN GROUP patch
On Sun, Jan 5, 2014 at 12:00 PM, Tom Lane wrote: > > > Looking at this example makes me wonder if it wouldn't be worthwhile to > provide a way to reset and re-use a tuplesort object, instead of redoing > all the lookup work involved. Or maybe just find a way to cache the > catalog lookups that are happening inside tuplesort_begin_datum, which are > about 50% of that function's cost it looks like. We're paying this same > kind of price for repeated tuplesort setup in the existing nodeAgg code, > if we have an aggregate with ORDER BY or DISTINCT in a grouped query with > many groups. > > This sounds very similar to: http://www.postgresql.org/message-id/caaphdvrbq348m8dyj-7o4vae5ps9zoq_34rgvaan1qyxl2s...@mail.gmail.com A reset function was added in the next patch which improved performance in my test case by about 5 times. Perhaps they can make use of the same function. Regards David Rowley > regards, tom lane > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers >
Re: [HACKERS] [bug fix] multibyte messages are displayed incorrectly on the client
From: "Noah Misch" I agree that English consistently beats mojibake. I question whether that makes up for the loss of translation when encodings do happen to match, particularly for non-technical errors like a mistyped password. The everything-UTF8 scenario appears often, perhaps explaining infrequent complaints about the status quo. If 90% of translated message users have client_encoding != server_encoding, then +1 for your patch's strategy. If the figure is only 60%, I'd vote for holding out for a more-extensive fix that allows us to encoding-convert localized authentication failure messages. I agree with you. It would be more friendly to users if more messages are localized. Then, as a happy medium, how about disabling message localization only if the client encoding differs from the server one? That is, compare the client_encoding value in the startup packet with the result of GetPlatformEncoding(). If they don't match, call disable_message_localization(). Regards MauMau -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [bug fix] "pg_ctl stop" times out when it should respond quickly
From: "Peter Eisentraut" On 12/25/13, 6:40 AM, MauMau wrote: pg_regress must wait for postgres to terminate by calling waitpid(), because it invoked postgres directly. The attached pg_regress_pg_stop.patch does this. If you like the combination of this and the original fix for pg_ctl in one patch, please use pg_stop_fail_v3.patch. This patch doesn't apply. Is that true? Today, I downloaded the following file (whose timestamp was 2014-Jan-04), and could apply pg_stop_fail_v3.patch cleanly. The "make check" succeeded. http://ftp.postgresql.org/pub/snapshot/dev/postgresql-snapshot.tar.gz Could you confirm again and tell me what problem is happening? Regards MauMau -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [bug fix] ECPG app crashes due to SIGBUS on SPARC Solaris
From: "Michael Meskes" On Sat, Dec 28, 2013 at 08:04:09AM +0900, MauMau wrote: OK, I'll run the ECPG regression test on Solaris without the patch. Please wait until Jan 6 2014 or so, because we've just entered new year holidays here in Japan. Sure, we're no in a particular hurry. I ran the ECPG regression test with the unpatched 64-bit PostgreSQL 9.2.4 on SPARC Solaris, and it succeeded (all 54 tests passed). For information, I did as follows: configure --prefix=... CC='/bin/cc -xtarget=generic64' ... gmake gmake install initdb -E UTF8 --no-locale pg_ctl start cd postgresql-9.2.4/src/interfaces/ecpg gmake check I guess that's that's because the regression test doesn't have a test case which specifies SQL descriptor name with a host variable. Regards MauMau -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ERROR: missing chunk number 0 for toast value
On Fri, Jan 3, 2014 at 9:05 AM, Amit Kapila wrote: > On Fri, Jan 3, 2014 at 12:51 AM, Heikki Linnakangas > wrote: >> On 01/02/2014 02:24 PM, Rushabh Lathia wrote: >>> Do you think we should detoast the local variable before >>> RollbackAndReleaseCurrentSubTransaction ? Or any other options ? >> >> >> Hmm, that would fix this particular test case, but not the other case where >> you DROP or TRUNCATE the table in the same transaction. >> >> The simplest fix would be to just detoast everything on assignment but that >> was rejected on performance grounds in that previous thread. I don't see any >> other realistic way to fix this, however, so maybe we should just bite the >> bullet and do it anyway. For simple variables like, in your test case, it's >> a good bet to detoast the value immediately; it'll be detoasted as soon as >> you try to do anything with it anyway. But it's not a good bet for record or >> row variables, because you often fetch the whole row into a variable but >> only access a field or two. > > Yeah, this is exactly what came to my mind as well the first time I saw this > problem that for row and record variables it can be penalty which user might > not expect as he might not be using toasted values. > > However is it possible that we do detoasting on assignment when the > variable of function is declared with some specific construct. After reading about handling for similar problem in other databases and thinking more on it, I wonder if we can make a rule such that values lesser than some threshold (8K or 16K or 32K) can be allowed to be retrieved in plpgsql variables. So with this, we can always detoast on assignment if the value is less than threshold and return error otherwise. I think this will help in reducing the performance impact and allow users to retrieve values (which are of less than threshold) in plpgsql variables without worrying about the behaviour reported in this and similar thread. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WITHIN GROUP patch
Andrew Gierth writes: > "Tom" == Tom Lane writes: > Tom> I've committed this after significant editorialization --- most > Tom> notably, I pushed control of the sort step into the aggregate > Tom> support functions. > Initial tests suggest that your version is ~40% slower than ours on > some workloads. I poked at this a bit with perf and oprofile, and concluded that probably the difference comes from ordered_set_startup() repeating lookups for each group that could be done just once per query. I'm not sure I believe the 40% figure; on this particular test case, oprofile breaks down the costs of ordered_set_startup() like this: 290.0756 postgres advance_transition_function 3830799.9244 postgres ordered_set_transition 1445 1.0808 postgres ordered_set_startup 3141879.4990 postgres tuplesort_begin_datum 4056 10.2632 postgres get_typlenbyvalalign 1445 3.6564 postgres ordered_set_startup [self] 734 1.8573 postgres MemoryContextAllocZero 510 1.2905 postgres RegisterExprContextCallback 283 0.7161 postgres exprType 247 0.6250 postgres get_sortgroupclause_tle 235 0.5946 postgres exprCollation 920.2328 postgres ReleaseSysCache 780.1974 postgres SearchSysCache 710.1797 postgres AggGetAggref 630.1594 postgres AggCheckCallContext 580.1468 postgres AllocSetAlloc 460.1164 postgres PrepareSortSupportFromOrderingOp 430.1088 postgres AggGetPerAggEContext 400.1012 postgres get_typlenbyval 390.0987 postgres palloc0 350.0886 postgres MemoryContextAlloc 170.0430 postgres get_sortgroupref_tle 100.0253 postgres tuplesort_begin_common The tuplesort_begin_datum calls are needed regardless --- your version was just doing them inside nodeAgg rather than externally. However, get_typlenbyvalalign and some of the other calls here are to fetch information that doesn't change across groups; probably we could arrange to cache that info instead of recomputing it each time. Still, it doesn't look like that could save more than 20% of ordered_set_startup, which itself is still only a few percent of the total query time. Looking at this example makes me wonder if it wouldn't be worthwhile to provide a way to reset and re-use a tuplesort object, instead of redoing all the lookup work involved. Or maybe just find a way to cache the catalog lookups that are happening inside tuplesort_begin_datum, which are about 50% of that function's cost it looks like. We're paying this same kind of price for repeated tuplesort setup in the existing nodeAgg code, if we have an aggregate with ORDER BY or DISTINCT in a grouped query with many groups. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [ANNOUNCE] IMCS: In Memory Columnar Store for PostgreSQL
On 01/04/2014 11:11 PM, Tom Lane wrote: knizhnik writes: On 01/04/2014 12:05 PM, David Fetter wrote: Is there some way not to use shared memory for it? No, IMCS ("In-Memory Columnar Store") is storing data in shared memory. It would probably be better if it made use of the dynamic shared memory features that exist in HEAD. regards, tom lane Thank you, I will try it. But I have some concerns: 1. I want IMCS to work with PostgreSQL versions not supporting DSM (dynamic shared memory), like 9.2, 9.3.1,... 2. IMCS is using PostgreSQL hash table implementation (ShmemInitHash, hash_search,...) May be I missed something - I just noticed DSM and have no chance to investigate it, but looks like hash table can not be allocated in DSM... 3. IMCS is allocating memory using ShmemAlloc. In case of using DSM I have to provide own allocator (although creation of non-releasing memory allocator should not be a big issue). 4. Current implementation of DSM still suffers from 256Gb problem. Certainly I can create multiple segments and so provide workaround without using huge pages, but it complicates allocator. 5. I wonder if I dynamically add new DSM segment - will it be available for other PostgreSQL processes? For example I run query which loads data in IMCS and so needs more space and allocates new DSM segment. Then another query is executed by other PostgreSQL process which tries to access this data. This process is not forked from the process created this new DSM segment, so I do not understand how this segment will be mapped to the address space of this process, preserving address... Certainly I can prohibit dynamic extension of IMCS storage (hoping that in this case there will be no such problem with DSM). But in this case we will loose the main advantage of using DSM instead of old schema of plugin's private shared memory. 6. IMCS has some configuration parameters which has to be set through postgresql.conf. So in any case user has to edit postgresql.conf file. In case of using DSM it will be not necessary to add IMCS to shared_preload_libraries list. But I do not think that it is so restrictive and critical requirement, is it? -- 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] Store Extension Options
Andres Freund writes: > On 2014-01-04 14:06:19 -0500, Tom Lane wrote: >> And if we have ext. as a prefix, exactly what prevents conflicts in the >> second part of the name? Nothing, that's what. It's useless. > Uh? We are certainly not going to add core code that defines relation > options with ext. in the name like we've introduced toast.fillfactor et > al? If this feature is of any use, surely we should assume that more than one extension will use it. If those extensions are separately developed, there's nothing preventing name conflicts. I would rank the odds of two people writing "my_replication_extension" a lot higher than the odds of the core code deciding to use such a prefix. What's more, what happens if we decide to migrate some such extension into core? A hard and fast division between names allowed to external and internal features is just going to bite us on the rear eventually. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [ANNOUNCE] IMCS: In Memory Columnar Store for PostgreSQL
knizhnik writes: > On 01/04/2014 12:05 PM, David Fetter wrote: >> Is there some way not to use shared memory for it? > No, IMCS ("In-Memory Columnar Store") is storing data in shared memory. It would probably be better if it made use of the dynamic shared memory features that exist in HEAD. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Store Extension Options
On 2014-01-04 14:06:19 -0500, Tom Lane wrote: > Andres Freund writes: > > On 2014-01-04 13:00:03 -0500, Tom Lane wrote: > >> Assuming that such examples are forthcoming, though, I think my main > >> objection to this proposal is the "ext." prefix, which seems precisely > >> 100% useless, not to mention inconsistent with the naming of custom GUCs, > >> which the same extension might well have some of. > > > Well, the argument is/was that it avoid conflicts with future core code > > adding more namespaces - like the already existing toast. prefix. If we > > say we can live with the possibility of such conflicts, it seems > > appropriate not to use ext. as a prefix. > > And if we have ext. as a prefix, exactly what prevents conflicts in the > second part of the name? Nothing, that's what. It's useless. Uh? We are certainly not going to add core code that defines relation options with ext. in the name like we've introduced toast.fillfactor et al? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Store Extension Options
Andres Freund writes: > On 2014-01-04 13:00:03 -0500, Tom Lane wrote: >> Assuming that such examples are forthcoming, though, I think my main >> objection to this proposal is the "ext." prefix, which seems precisely >> 100% useless, not to mention inconsistent with the naming of custom GUCs, >> which the same extension might well have some of. > Well, the argument is/was that it avoid conflicts with future core code > adding more namespaces - like the already existing toast. prefix. If we > say we can live with the possibility of such conflicts, it seems > appropriate not to use ext. as a prefix. And if we have ext. as a prefix, exactly what prevents conflicts in the second part of the name? Nothing, that's what. It's useless. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Store Extension Options
On 2014-01-04 13:00:03 -0500, Tom Lane wrote: > Andres Freund writes: > > On 2014-01-04 11:54:46 -0500, Robert Haas wrote: > >> Well, as I said before, somebody can make their own configuration > >> table and store stuff there, rather than using pg_class.reloptions. > >> As I recall, the only response to that proposal was "well, they might > >> not want to do it that way", which does not strike me as a sufficient > >> reason. > > > Well, there's some things you get by that integration: > > * Proper dependency tracking when relations are dropped & renamed > > * Sensible integration into pg_dump, with only the relevant options > > being dumped/restored on partial dump/restores > > * Caching of values, with proper cache invalidation > > If you have some settings that need to be table-specific, then > I agree that the reloptions infrastructure is a nice place to track them. > What's actually missing here is some compelling examples of such settings > for plausible extensions. I don't know about others, but I would like to use it for bdr to configure which table is replicated where. I.e. something like bdr.replication_set=a,b,c, > (The original example was pure handwaving, as > I don't believe it's possible to build a "replication extension" with no > core-code changes. As long as you need some of those, patching in a few > more standard reloptions is hardly a deal-breaker.) Well, slony et al exist, so it's certainly possible. And even if you want to talk about logical replication, there aren't that much core changes required - and all of them have been submitted. They might not make it into 9.4 but I certainly plan to pursue things further so it's possible to stuff without patching core. > Assuming that such examples are forthcoming, though, I think my main > objection to this proposal is the "ext." prefix, which seems precisely > 100% useless, not to mention inconsistent with the naming of custom GUCs, > which the same extension might well have some of. Well, the argument is/was that it avoid conflicts with future core code adding more namespaces - like the already existing toast. prefix. If we say we can live with the possibility of such conflicts, it seems appropriate not to use ext. as a prefix. > I would suggest addressing Robert's concern about lack of error checking > by refusing to allow a custom reloption to be set unless the relevant > extension is loaded and checks it. Unlike the postgresql.conf problem, > I don't see any very good use-case for allowing an unchecked ALTER TABLE Fine with me. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] proposal: extensible plpgsql executor - related to assertions
2014/1/4 Tom Lane > Pavel Stehule writes: > > I propose a enhance the PLpgSQL_plugin struct by a new hook > > void (*pragma)(PLpgSQL_execstate *estate, PLpgSQL_pragma > > *pragma_stmt) > > I repeat what I said a couple of days ago: it's a very bad idea to be > enabling more plpgsql plugins as long as the infrastructure can only > support one. We need to fix that *first* before we go merrily designing > more. > It means a some additional mechanism to find_rendezvous_variable. What about two new rendezvous variables? One for publishing a PLpgSQL internal API, and second a list of plpgsql_plugin structures? It would be very nice, if we can better access to other plpgsql public functions. A implementation in plpgsql_lint and plpgsql_check is working, but I agree so it is ugly designed (with some disadvantages) - and any change can be better. I minimize these bad references to plpgsq - but plpgsql requires "plpgsql_compile" and "plpgsql_parser_setup" still. Other possibility is new V1 function for plugin registration. > > I don't like the notion of a pragma statement in this form anyway, > because you've essentially made it an executable statement; usually > pragmas are compile-time things. It appears to me that you've basically > commandeered the word "pragma" for "SET affecting a plugin's variable". > It should not be named pragma - I have not better name now. It should not be used as plugin's variable primary. It should to invoke a external routine - with or without additional parameters. When I would to support tracking, then user should to explicitly set point, where tracking is defined - same is with assertions. > If we're inventing new callbacks for plugins, why not one that will extend > an existing statement having the right kind of semantics? yes, it is possible - I can to image some like PERFORM assert(exprlist) and inside callback, we can check a expression and when we find a expected pattern, we can change a semantic. I plan to use this workaround for first plpgsql dumper and tracking extension. But it can have some performance (probably minimal) impact - and it is difficult to implement a mode when this functionality is disabled without any performance impact. So special statement can simplify life to plugin' developers. > Or actually, > why would it not be better for the plugin to be using a custom GUC for > its variable? There's a large amount of infrastructure that custom GUCs > can take advantage of, which you'd otherwise have to reinvent piece > by piece. > GUC doesn't help me with marking some position in source code important for plugin. Regards Pavel > > regards, tom lane >
Re: [HACKERS] comment typo in postgres_fdw/postgres_fdw.c
Ian Lawrence Barwick writes: > Presumably "classifyConditions", not "classifyClauses", is meant. Hm, I think this got missed in a bout of renaming. Will apply, thanks. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Wrong comment for bitncmp function in network.c
Emre Hasegeli writes: > I am not sure it worth reporting but it took me a while to find out > what is wrong with comparing two values returned from > the bitncmp function. It does not return -1, 1 or 0 as it is written > on the comment when n % 8 == 0. Yeah, should say "<0, >0, or 0", I guess. Will fix, thanks. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Store Extension Options
Andres Freund writes: > On 2014-01-04 11:54:46 -0500, Robert Haas wrote: >> Well, as I said before, somebody can make their own configuration >> table and store stuff there, rather than using pg_class.reloptions. >> As I recall, the only response to that proposal was "well, they might >> not want to do it that way", which does not strike me as a sufficient >> reason. > Well, there's some things you get by that integration: > * Proper dependency tracking when relations are dropped & renamed > * Sensible integration into pg_dump, with only the relevant options > being dumped/restored on partial dump/restores > * Caching of values, with proper cache invalidation If you have some settings that need to be table-specific, then I agree that the reloptions infrastructure is a nice place to track them. What's actually missing here is some compelling examples of such settings for plausible extensions. (The original example was pure handwaving, as I don't believe it's possible to build a "replication extension" with no core-code changes. As long as you need some of those, patching in a few more standard reloptions is hardly a deal-breaker.) Assuming that such examples are forthcoming, though, I think my main objection to this proposal is the "ext." prefix, which seems precisely 100% useless, not to mention inconsistent with the naming of custom GUCs, which the same extension might well have some of. I think that custom reloptions should just have names like "extension_name.option_name", the same as custom GUCs do. We have enough experience now with custom GUCs that I don't think it's unreasonable to model the behavior of custom reloptions on them as closely as possible. I would suggest addressing Robert's concern about lack of error checking by refusing to allow a custom reloption to be set unless the relevant extension is loaded and checks it. Unlike the postgresql.conf problem, I don't see any very good use-case for allowing an unchecked ALTER TABLE to occur. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] proposal: extensible plpgsql executor - related to assertions
Pavel Stehule writes: > I propose a enhance the PLpgSQL_plugin struct by a new hook > void (*pragma)(PLpgSQL_execstate *estate, PLpgSQL_pragma > *pragma_stmt) I repeat what I said a couple of days ago: it's a very bad idea to be enabling more plpgsql plugins as long as the infrastructure can only support one. We need to fix that *first* before we go merrily designing more. I don't like the notion of a pragma statement in this form anyway, because you've essentially made it an executable statement; usually pragmas are compile-time things. It appears to me that you've basically commandeered the word "pragma" for "SET affecting a plugin's variable". If we're inventing new callbacks for plugins, why not one that will extend an existing statement having the right kind of semantics? Or actually, why would it not be better for the plugin to be using a custom GUC for its variable? There's a large amount of infrastructure that custom GUCs can take advantage of, which you'd otherwise have to reinvent piece by piece. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] truncating pg_multixact/members
Robert Haas writes: > As far as back-patching the GUCs, my thought would be to back-patch > them but mark them GUC_NOT_IN_SAMPLE in 9.3, so we don't have to touch > the default postgresql.conf. That seems bizarre and pointless. Keep in mind that 9.3 is still wet behind the ears and many many people haven't adopted it yet. If we do what you're suggesting then we're creating a completely useless inconsistency that will nonetheless affect all those future adopters ... while accomplishing nothing much for those who have already installed 9.3. The latter are not going to have these GUCs in their existing postgresql.conf, true, but there's nothing we can do about that. (Hint: GUC_NOT_IN_SAMPLE doesn't actually *do* anything, other than prevent the variable from being shown by SHOW ALL, which is not exactly helpful here.) regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Store Extension Options
On 2014-01-04 11:54:46 -0500, Robert Haas wrote: > On Sat, Jan 4, 2014 at 11:07 AM, Fabrizio Mello > wrote: > >> I continue to think that the case for having this feature at all has > >> not been well-made. > > > > We can use this feature to store any custom GUC for relations, attributes > > and functions also. > > > > Some use cases: > > * extension options > > * config for external apps (frameworks, third part software) > > > > Comments? > > Well, as I said before, somebody can make their own configuration > table and store stuff there, rather than using pg_class.reloptions. > As I recall, the only response to that proposal was "well, they might > not want to do it that way", which does not strike me as a sufficient > reason. Well, there's some things you get by that integration: * Proper dependency tracking when relations are dropped & renamed * Sensible integration into pg_dump, with only the relevant options being dumped/restored on partial dump/restores * Caching of values, with proper cache invalidation Sure, you can implement both using event triggers and relcache invalidation callbacks, but that's not something we want several extensions to do independently. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Store Extension Options
On Sat, Jan 4, 2014 at 11:07 AM, Fabrizio Mello wrote: >> I continue to think that the case for having this feature at all has >> not been well-made. > > We can use this feature to store any custom GUC for relations, attributes and > functions also. > > Some use cases: > * extension options > * config for external apps (frameworks, third part software) > > Comments? Well, as I said before, somebody can make their own configuration table and store stuff there, rather than using pg_class.reloptions. As I recall, the only response to that proposal was "well, they might not want to do it that way", which does not strike me as a sufficient reason. What we've basically settled into for GUCs is that you can register a custom GUC, but unless the module is loaded we'll accept any value for that GUC without checking it. We'd presumably need a similar mechanism here, or maybe you're proposing that we accept any reloption at all with any associated value whatsoever, so long as the prefix is ext. The first seems like an extension of an existing kludge of which I'm not overly found, and the second is an even larger kludge. In my experience as a software developer, there are very few places where it's useful to accept and store user input without any validation whatsoever, and I doubt that this is one of them. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Store Extension Options
Enviado via iPhone > Em 02/01/2014, às 22:16, Robert Haas escreveu: > >> On Thu, Jan 2, 2014 at 4:19 AM, Andres Freund wrote: >> On 2013-12-31 13:37:59 +0100, Pavel Stehule wrote: We use the namespace "ext" to the internal code (src/backend/access/common/reloptions.c) skip some validations and store the custom GUC. Do you think we don't need to use the "ext" namespace? >>> >>> yes - there be same mechanism as we use for GUC >> >> There is no existing mechanism to handle conflicts for GUCs. The >> difference is that for GUCs nearly no "namespaced" GUCs exist (plperl, >> plpgsql have some), but postgres defines at least autovacuum. and >> toast. namespaces for relation options. > > I continue to think that the case for having this feature at all has > not been well-made. > We can use this feature to store any custom GUC for relations, attributes and functions also. Some use cases: * extension options * config for external apps (frameworks, third part software) Comments? Regards, Fabrízio Mello -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] proposal: extensible plpgsql executor - related to assertions
Hello I am thinking how to implement a assertions and tracking to plpgsql. This area is not strict and clear - everybody would little bit different functionality - and implementation can be different if someone use a psql as main client or some GUI. So I am sceptical to implement assertion directly to plpgsql. Now I finished plpgsql_check_function as extension and I had to use a plpgsql plugin API again. This API is good enough to enable extensible plpgsql executor. It can be implemened via stmt_beg call, although a special hook can be better. typedef struct { /* Function pointers set up by the plugin */ void(*func_setup) (PLpgSQL_execstate *estate, PLpgSQL_function *func); void(*func_beg) (PLpgSQL_execstate *estate, PLpgSQL_function *func); void(*func_end) (PLpgSQL_execstate *estate, PLpgSQL_function *func); void(*stmt_beg) (PLpgSQL_execstate *estate, PLpgSQL_stmt *stmt); void(*stmt_end) (PLpgSQL_execstate *estate, PLpgSQL_stmt *stmt); /* Function pointers set by PL/pgSQL itself */ void(*error_callback) (void *arg); void(*assign_expr) (PLpgSQL_execstate *estate, PLpgSQL_datum *target, PLpgSQL_expr *expr); } PLpgSQL_plugin; What is missing is a possibility to invoke some custom functionality (implemented in plugin-extension) from plpgsql code. What example, I can try to print function parameters, local variables at some point, assertions, tracking, ... Usually we would to ignore any action, when plugin is not active. This ensure a minimal slowdown on production when we don´t would to enable assertions, but we don´t would to modify source code. I propose a enhance the PLpgSQL_plugin struct by a new hook void (*pragma)(PLpgSQL_execstate *estate, PLpgSQL_pragma *pragma_stmt) typedef struct { int cmd_type; int lineno; char *refname; List *exprs; } pragma_stmt; PLpgSQL executor should to execute this statement only when pragma hook is defined. Syntax of PL/pgSQL should be enhanced about statement pragma Syntax: pragma pragmaname [ [ ( ] expr, [expr .. ] [ ) ] ] This syntax should be checked by plpgsql compiler, but and it is important - it is not executed by plpgsql executor ever (minimally in this proposal). So if you don´t would active assertions, then you don´t load a plugin with assertion implementation. Expressions are executed by plugin, and it is on plugin responsibility. So with this functionality we can implement tracking, variable dumping, assertions pragma dump_local_variables; pragma dump_parameters; pragma assert(a = 10, ´variable a is not 10´); pragma warning(a > 100, ´a > 100´); pragma notice a > 100, format('a = "%s"', a); ... This proposal should not break any current plpgsql code and it is generic - require only minimal changes in plpgsql runtime. Any current plpgsql plugins should be workable after compilation (but recompilation is required for any new version). Pragma can be used for other plpgsql plugin as other way how to push a some complex (or simple) parameters to plpgsql related extensions pragma disable_plpgsql_check; Opinions, ideas? Regards Pavel
Re: [HACKERS] RFC: Async query processing
On Fri, Jan 03, 2014 at 04:46:23PM +0100, Florian Weimer wrote: > On 01/03/2014 04:20 PM, Tom Lane wrote: > > >I think Florian has a good point there, and the reason is this: what > >you are talking about will be of exactly zero use to applications that > >want to see the results of one query before launching the next. Which > >eliminates a whole lot of apps. I suspect that almost the *only* > >common use case in which a stream of queries can be launched without > >feedback is going to be bulk data loading. It's not clear at all > >that pipelining the PQexec code path is the way to better performance > >for that --- why not use COPY, instead? > > The data I encounter has to be distributed across multiple tables. > Switching between the COPY TO commands would again need client-side > buffering and heuristics for sizing these buffers. Lengths of runs > vary a lot in my case. Why switch between COPY commands, why could you not do it in one? For example: COPY table1(col1, col2, ...), table2(col1, col2, ...) FROM STDIN WITH (tableoids); tableoid1col1col2... tableoid2... ... \. There's no especially good reason why a COPY can only write to one table. In this way you provide, per row, which table this row should go to. There's always the issue of generated primary keys. So you could, if you wanted to, do: COPY table1(col1, col2, ...) RETURNING pkey1, table2(col1, col2, ...) FROM STDIN WITH (tableoids); tableoid1col1col2... tableoid2\K ... ... \. So, like we have \N for NULL, a \K oid which be the value of the RETURNING column for the table with that oid. This may be way outfield, but we have a streaming interface, there's no reason why we can't extend it. There's also the idea of WITH x AS (COPY ... FROM STDIN) which you could do with a similar multiplexing of rows in one stream. This would be extremely powerful together with MERGE. Have a nice way, -- Martijn van Oosterhout http://svana.org/kleptog/ > He who writes carelessly confesses thereby at the very outset that he does > not attach much importance to his own thoughts. -- Arthur Schopenhauer signature.asc Description: Digital signature
Re: [HACKERS] truncating pg_multixact/members
On Fri, Jan 3, 2014 at 9:11 AM, Alvaro Herrera wrote: > Robert Haas escribió: >> On Mon, Dec 30, 2013 at 10:59 PM, Alvaro Herrera >> wrote: >> > One problem I see is length of time before freezing multis: they live >> > for far too long, causing the SLRU files to eat way too much disk space. >> > I ran burnmulti in a loop, creating multis of 3 members each, with a min >> > freeze age of 50 million, and this leads to ~770 files in >> > pg_multixact/offsets and ~2900 files in pg_multixact/members. Each file >> > is 32 pages long. 256kB apiece. Probably enough to be bothersome. >> > >> > I think for computing the freezing point for multis, we should slash >> > min_freeze_age by 10 or something like that. Or just set a hardcoded >> > one million. >> >> Yeah. Since we expect mxids to be composed at a much lower rate than >> xids, we can keep pg_multixact small without needing to increase the >> rate of full table scans. However, it seems to me that we ought to >> have GUCs for mxid_freeze_table_age and mxid_freeze_min_age. There's >> no principled way to derive those values from the corresponding values >> for XIDs, and I can't see any reason to suppose that we know how to >> auto-tune brand new values better than we know how to auto-tune their >> XID equivalents that we've had for years. >> >> One million is probably a reasonable default for mxid_freeze_min_age, though. > > I didn't want to propose having new GUCs, but if there's no love for my > idea of deriving it from the Xid freeze policy, I guess it's the only > solution. Just keep in mind we will need to back-patch these new GUCs > to 9.3. Are there objections to this? > > Also, what would be good names? Peter E. complained recently about the > word MultiXactId being exposed in some error messages; maybe "mxid" is > too short an abbreviation of that. Perhaps > multixactid_freeze_min_age = 1 million > multixactid_freeze_table_age = 3 million > ? > I imagine this stuff would be described somewhere in the docs, perhaps > within the "routine maintenance" section somewhere. Yeah, this stuff is definitely underdocumented relative to vacuum right now. As far as back-patching the GUCs, my thought would be to back-patch them but mark them GUC_NOT_IN_SAMPLE in 9.3, so we don't have to touch the default postgresql.conf. Also, while multixactid_freeze_min_age should be low, perhaps a million as you suggest, multixactid_freeze_table_age should NOT be lowered to 3 million or anything like it. If you do that, people who are actually doing lots of row locking will start getting many more full-table scans. We want to avoid that at all cost. I'd probably make the default the same as for vacuum_freeze_table_age, so that mxids only cause extra full-table scans if they're being used more quickly than xids. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Changeset Extraction Interfaces
On Fri, Jan 3, 2014 at 10:12 AM, Andres Freund wrote: > On 2013-12-12 10:01:21 -0500, Robert Haas wrote: >> On Thu, Dec 12, 2013 at 7:04 AM, Andres Freund >> wrote: >> > I think there'll always be a bit of a difference between slots for >> > physical and logical data, even if 90% of the implementation is the >> > same. We can signal that difference by specifying logical/physical as an >> > option or having two different sets of commands. >> > >> > Maybe? >> > >> > ACQUIRE_REPLICATION_SLOT slot_name PHYSICAL physical_opts >> > ACQUIRE_REPLICATION_SLOT slot_name LOGICAL logical_opts >> > -- already exists without slot, PHYSICAL arguments >> > START_REPLICATION [SLOT slot] [PHYSICAL] RECPTR opt_timeline >> > START_REPLICATION SLOT LOGICAL slot plugin_options >> > RELEASE_REPLICATION_SLOT slot_name >> >> I assume you meant START_REPLICATION SLOT slot LOGICAL plugin_options, >> but basically this seems OK to me. > > When writing the code for this, I decided that I need to reneg a bit on > those names - they don't work nicely enough on the C level for > me. Specifically during a START_REPLICATION we need to temporarily mark > the slot as being actively used and mark it unused again > afterwards. That's much more Acquire/Release like than the persistent > Acquire/Release above for me. > > The C names in the version I am working on currently are: > extern void ReplicationSlotCreate(const char *name); > extern void ReplicationSlotDrop(const char *name); > extern void ReplicationSlotAcquire(const char *name); > extern void ReplicationSlotRelease(void); > extern void ReplicationSlotSave(void); > > which would make the walsender ones > > CREATE_REPLICATION_SLOT ... > START_REPLICATION [SLOT slot] [LOGICAL | PHYSICAL] ... > DROP_REPLICATION_SLOT ... > > where START_REPLICATION internally does acquire/release on the passed > SLOT. > > Does that work for you? Yep, no objections. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for pg_stat_archiver view
Hi Magnus, Il 04/01/14 13:25, Magnus Hagander ha scritto: > My first reaction was that exactly those two things were missing. And > then I read your whole email :) :) > With those two, I think it would make much sense to have a view like > this. Ok, I will prepare version 2 with those. > I'd suggest making the view on top of an SRF like pg_stat_replication > and pg_stat_activity (for example), instead of a whole lot of separate > function calls like the older stats views. Ok, good idea. > in pgarch_ArchiveDone() you seem to be increasing the m_archived_vals > value for each call and then sending it off. And then you add that > number in the stats collector. Isn't that going to add the wrong > number in the end - after a while, the archiver is going to send "add > 100" when it's just sent one file? ISTM that pgstat_recv_archiver > should just do ++ on the value? You are right. The purpose was to set it to 1 in ArchiveDone (I might have missed that change), so that I can manage the failed counters in the same way. I will fix this in version 2. > Oh, and you need to change the format id number of the stats file. I have not found any instruction on how to set it. I assume you are talking about this: PGSTAT_FILE_FORMAT_ID0x01A5BC9B Any suggestion is welcome. > There's a quick review you for ;) I think it's definitely worthwhile > with those things fixed (and a proper review, that was just a quick > one-over) Thanks for that. It already means a lot if you agree too it is worth it. Ciao, Gabriele -- Gabriele Bartolini - 2ndQuadrant Italia PostgreSQL Training, Services and Support gabriele.bartol...@2ndquadrant.it | www.2ndQuadrant.it -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] comment typo in postgres_fdw/postgres_fdw.c
Presumably "classifyConditions", not "classifyClauses", is meant. Patch attached. Regards Ian Barwick diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index 246a3a9..46ea032 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -723,7 +723,7 @@ postgresGetForeignPlan(PlannerInfo *root, /* * Separate the scan_clauses into those that can be executed remotely and * those that can't. baserestrictinfo clauses that were previously -* determined to be safe or unsafe by classifyClauses are shown in +* determined to be safe or unsafe by classifyConditions are shown in * fpinfo->remote_conds and fpinfo->local_conds. Anything else in the * scan_clauses list should be a join clause that was found safe by * postgresGetForeignPaths. -- 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] Support for pg_stat_archiver view
On Sat, Jan 4, 2014 at 1:33 AM, Gabriele Bartolini < gabriele.bartol...@2ndquadrant.it> wrote: > Hello, > > please find attached the patch that adds basic support for the > pg_stat_archiver system view, which allows users that have continuous > archiving procedures in place to keep track of some important metrics > and information. > > Currently, pg_stat_archiver displays: > > * archived_wals: number of successfully archived WAL files since start > (or the last reset) > * last_archived_wal: last successfully archived WAL file > * last_archived_wal_time: timestamp of the latest successful WAL archival > * stats_reset: time of last stats reset > > This is an example of output: > > postgres=# select * from pg_stat_archiver ; > -[ RECORD 1 ]--+-- > archived_wals | 1 > last_archived_wal | 00010001 > last_archived_wal_time | 2014-01-04 01:01:08.858648+01 > stats_reset| 2014-01-04 00:59:25.895034+01 > > Similarly to pg_stat_bgwriter, it is possible to reset statistics just > for this context, calling the pg_stat_reset_shared('archiver') function. > > The patch is here for discussion and has been prepared against HEAD. > It includes also changes in the documentation and the rules.out test. > > I plan to add further information to the pg_stat_archiver view, > including the number of failed attempts of archival and the WAL and > timestamp of the latest failure. However, before proceeding, I'd like to > get some feedback on this small patch as well as advice on possible > regression tests to be added. > > My first reaction was that exactly those two things were missing. And then I read your whole email :) With those two, I think it would make much sense to have a view like this. I'd suggest making the view on top of an SRF like pg_stat_replication and pg_stat_activity (for example), instead of a whole lot of separate function calls like the older stats views. in pgarch_ArchiveDone() you seem to be increasing the m_archived_vals value for each call and then sending it off. And then you add that number in the stats collector. Isn't that going to add the wrong number in the end - after a while, the archiver is going to send "add 100" when it's just sent one file? ISTM that pgstat_recv_archiver should just do ++ on the value? Oh, and you need to change the format id number of the stats file. There's a quick review you for ;) I think it's definitely worthwhile with those things fixed (and a proper review, that was just a quick one-over) -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
[HACKERS] Wrong comment for bitncmp function in network.c
I am not sure it worth reporting but it took me a while to find out what is wrong with comparing two values returned from the bitncmp function. It does not return -1, 1 or 0 as it is written on the comment when n % 8 == 0. -- 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] [ANNOUNCE] IMCS: In Memory Columnar Store for PostgreSQL
On 01/04/2014 12:05 PM, David Fetter wrote: I'm sorry I misunderstood about the extension you wrote. Is there some way not to use shared memory for it? No, IMCS ("In-Memory Columnar Store") is storing data in shared memory. Certainly I could allocate shared memory myself, but due to portability and easy maintenance reasons I decided to reuse PostgreSQL mechanism of shared memory. The only requirement is that IMSC extension (as well as pg_stat_statements extension) should be included in "shared_preload_libraries" list in postgresql.conf. IMCS memory is not somehow interleave with shared memory used for PostgreSQL shared buffers. And the only limitation is this 2567Gb limit at Linux, which can be resolved using the patch included in IMCS distributive. Cheers, David. On Sat, Jan 04, 2014 at 11:46:25AM +0400, knizhnik wrote: Hi David, Sorry, but I do not completely understand your suggestions: 1. IMCS really contains single patch file sysv_shmem.patch. Applying this patch is not mandatory for using IMCS: it just solves the problem with support of > 256Gb of shared memory. Right now PostgreSQL is not able to use more than 256Gb shared buffers at Linux with standard 4kb pages. I have found proposal for using MAP_HUGETLB flag in commit fest: http://www.postgresql.org/message-id/20131125032920.ga23...@toroid.org but unfortunately it was rejected. Hugepages are intensively used by Oracle and I think that them will be useful for improving performance of PorstreSQL. So not just IMCS can benefit from this patch. My patch is much more simple - I specially limited scope of this patch to one file. Certainly switch huge tlb on/off should be done through postgresql.conf configuration file. In any case - IMCS can be used without this patch: you just could not use more than 256Gb memory, even if your system has more RAM. 2. I do not understand "The add-on is not formatted as an EXTENSION" IMCS was created as standard extension - I just look at the examples of other PostgreSQL extensions included in PostgreSQL distribution (for example pg_stat_statements). It can be added using "create extension imcs" and removed "drop extension imcs" commands. If there are some violations of PostgreSQL extensions rules, please let me know, I will fix them. But I thought that I have done everything in legal way. On 01/04/2014 03:21 AM, David Fetter wrote: On Thu, Jan 02, 2014 at 08:48:24PM +0400, knizhnik wrote: I want to announce implementation of In-Memory Columnar Store extension for PostgreSQL. Vertical representation of data is stored in PostgreSQL shared memory. Thanks for the hard work! I noticed a couple of things about this that probably need some improvement. 1. There are unexplained patches against other parts of PostgreSQL, which means that they may break other parts of PostgreSQL in equally inexplicable ways. Please rearrange the patch so it doesn't require this. This leads to: 2. The add-on is not formatted as an EXTENSION, which would allow people to add it or remove it cleanly. Would you be so kind as to fix these? Cheers, David.
Re: [HACKERS] [ANNOUNCE] IMCS: In Memory Columnar Store for PostgreSQL
I'm sorry I misunderstood about the extension you wrote. Is there some way not to use shared memory for it? Cheers, David. On Sat, Jan 04, 2014 at 11:46:25AM +0400, knizhnik wrote: > Hi David, > > Sorry, but I do not completely understand your suggestions: > > 1. IMCS really contains single patch file sysv_shmem.patch. > Applying this patch is not mandatory for using IMCS: it just solves > the problem with support of > 256Gb of shared memory. > Right now PostgreSQL is not able to use more than 256Gb shared > buffers at Linux with standard 4kb pages. > I have found proposal for using MAP_HUGETLB flag in commit fest: > > http://www.postgresql.org/message-id/20131125032920.ga23...@toroid.org > > but unfortunately it was rejected. Hugepages are intensively used by > Oracle and I think that them will be useful for improving > performance of PorstreSQL. So not just IMCS can benefit from this > patch. My patch is much more simple - I specially limited scope of > this patch to one file. Certainly switch huge tlb on/off should be > done through postgresql.conf configuration file. > > In any case - IMCS can be used without this patch: you just could > not use more than 256Gb memory, even if your system has more RAM. > > 2. I do not understand "The add-on is not formatted as an EXTENSION" > IMCS was created as standard extension - I just look at the examples > of other PostgreSQL extensions included in PostgreSQL distribution > (for example pg_stat_statements). It can be added using "create > extension imcs" and removed "drop extension imcs" commands. > > If there are some violations of PostgreSQL extensions rules, please > let me know, I will fix them. > But I thought that I have done everything in legal way. > > > > > > > On 01/04/2014 03:21 AM, David Fetter wrote: > >On Thu, Jan 02, 2014 at 08:48:24PM +0400, knizhnik wrote: > >>I want to announce implementation of In-Memory Columnar Store > >>extension for PostgreSQL. > >>Vertical representation of data is stored in PostgreSQL shared memory. > >Thanks for the hard work! > > > >I noticed a couple of things about this that probably need some > >improvement. > > > >1. There are unexplained patches against other parts of PostgreSQL, > >which means that they may break other parts of PostgreSQL in equally > >inexplicable ways. Please rearrange the patch so it doesn't require > >this. This leads to: > > > >2. The add-on is not formatted as an EXTENSION, which would allow > >people to add it or remove it cleanly. > > > >Would you be so kind as to fix these? > > > >Cheers, > >David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers