[HACKERS] plpgsql_check_function - implementation
Hello I try to play with different implementations of plpgsql deep checking. The most important task of deep checking is creating plans for all queries and expressions in function. The prerequisite for this task is knowledge of data types of all variables. Record and row types is break, but there is workaround - we are able to derive data types from plans and we can assign with high success rate valid types to this kind variables. We are not able to do with result of dynamic SQL and temporary tables still - just we are not able to detect possible errors for dynamic queries ever. There are four possible implementations: 0) special recursive check routine + derivation data types from plans: + zero impact on current code, readability, - one other long recursive routine a) enhance parser + derivation data types from plans: + no new recursive routine, - order of check depends on bison processing order, result needs a final sort b) enhance executor nodes + take data types from fake execution: + relative less new code, - decrease readability of executor code, 20% slowdown of CPU bottle neck code (new code is on critical path) I tested code (this is a worst situation) - patch is in attachment (it is WIP - just for test of impact new code to performance) CREATE OR REPLACE FUNCTION public.test() RETURNS integer LANGUAGE plpgsql IMMUTABLE AS $function$ declare i int; declare j int; begin i := 1; while i 1 loop j := 1; while j 1000 loop j := j + 1; end loop; i := i + 1; end loop; return i; end; $function$ c) merge checking and dumping and derivation data from plans: + zero impact on current code, readability, - some new code my @0 works well, but was repeatedly rejected by Tom and Heikki, @a needs final sort - so it needs more complex infrastructure for creating result tuplestore, @b has mensurable performance impact (from 9454 to 11274 ms), so there are only @c. comments, notices? Regards Pavel plpgsql_check_implementation.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] posix_fadvise missing in the walsender
In access/transam/xlog.c we give the OS buffer caching a hint that we won't need a WAL file any time soon with posix_fadvise(openLogFile, 0, 0, POSIX_FADV_DONTNEED); before closing the WAL file, but only if we don't have walsenders. That's reasonable because the walsender will reopen that same file shortly after. However the walsender doesn't call posix_fadvise once it's done with the file and I'm proposing to add this to walsender.c for consistency as well. Since there could be multiple walsenders, only the slowest one should call this function. Finding out the slowest walsender can be done by inspecting the shared memory and looking at the sentPtr of each walsender. Any comments? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] overlapping strncpy/memcpy errors via valgrind
==24373== Source and destination overlap in strncpy(0x28b892f5, 0x28b892f5, 64) ==24373==at 0x402A8F2: strncpy (mc_replace_strmem.c:477) ==24373==by 0x7D563F: namestrcpy (name.c:221) ==24373==by 0x46DF31: TupleDescInitEntry (tupdesc.c:473) ==24373==by 0x889EC3: resolve_polymorphic_tupdesc (funcapi.c:573) ==24373==by 0x889873: internal_get_result_type (funcapi.c:322) ==24373==by 0x8896A2: get_expr_result_type (funcapi.c:235) ==24373==by 0x594577: addRangeTableEntryForFunction (parse_relation.c:1206) ==24373==by 0x57D81E: transformRangeFunction (parse_clause.c:550) ==24373==by 0x57DBE1: transformFromClauseItem (parse_clause.c:658) ==24373==by 0x57CF01: transformFromClause (parse_clause.c:120) ==24373==by 0x54F9A5: transformSelectStmt (analyze.c:925) ==24373==by 0x54E4E9: transformStmt (analyze.c:242) ==24373== Source and destination overlap in memcpy(0x546abc0, 0x546abc0, 24) ==24373==at 0x402B930: memcpy (mc_replace_strmem.c:883) ==24373==by 0x853BAB: uniqueentry (tsvector.c:127) ==24373==by 0x8541A5: tsvectorin (tsvector.c:262) ==24373==by 0x888781: InputFunctionCall (fmgr.c:1916) ==24373==by 0x888A7D: OidInputFunctionCall (fmgr.c:2047) ==24373==by 0x59B6D7: stringTypeDatum (parse_type.c:592) ==24373==by 0x580E14: coerce_type (parse_coerce.c:303) ==24373==by 0x580AD4: coerce_to_target_type (parse_coerce.c:101) ==24373==by 0x58B802: transformTypeCast (parse_expr.c:) ==24373==by 0x587484: transformExprRecurse (parse_expr.c:208) ==24373==by 0x587156: transformExpr (parse_expr.c:116) ==24373==by 0x5975CC: transformTargetEntry (parse_target.c:94) I didn't check out the tsvector case but the resolve_polymorphic_tupdesc/TupleDescInitEntry is clearly bogusly coded. Do we care? strncpy'ing a string over itself isn't defined... 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] JSON Function Bike Shedding
On 02/16/2013 07:50 PM, David E. Wheeler wrote: On Feb 16, 2013, at 12:47 PM, Andrew Dunstan and...@dunslane.net wrote: To answer David's point, there is no point in having both get(json,text) get(json, variadic text[]) since the second can encompass the first, and having both would make calls ambiguous. Oh. Well then how about get(json, int) get(json, text) get(json, text[]) ? No, then we don't have a variadic version. You are going to have to accept that we can't make one function name cover all of this. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] overlapping strncpy/memcpy errors via valgrind
Peter G is sitting near me and reminded me that this issue came up in the past. Iirc the conclusion then is that we're calling memcpy where the source and destination pointers are sometimes identical. Tom decided there was really no realistic architecture where that wouldn't work. We're not calling it on overlapping nonidentical pointers. On Feb 17, 2013 2:22 PM, Andres Freund and...@2ndquadrant.com wrote: ==24373== Source and destination overlap in strncpy(0x28b892f5, 0x28b892f5, 64) ==24373==at 0x402A8F2: strncpy (mc_replace_strmem.c:477) ==24373==by 0x7D563F: namestrcpy (name.c:221) ==24373==by 0x46DF31: TupleDescInitEntry (tupdesc.c:473) ==24373==by 0x889EC3: resolve_polymorphic_tupdesc (funcapi.c:573) ==24373==by 0x889873: internal_get_result_type (funcapi.c:322) ==24373==by 0x8896A2: get_expr_result_type (funcapi.c:235) ==24373==by 0x594577: addRangeTableEntryForFunction (parse_relation.c:1206) ==24373==by 0x57D81E: transformRangeFunction (parse_clause.c:550) ==24373==by 0x57DBE1: transformFromClauseItem (parse_clause.c:658) ==24373==by 0x57CF01: transformFromClause (parse_clause.c:120) ==24373==by 0x54F9A5: transformSelectStmt (analyze.c:925) ==24373==by 0x54E4E9: transformStmt (analyze.c:242) ==24373== Source and destination overlap in memcpy(0x546abc0, 0x546abc0, 24) ==24373==at 0x402B930: memcpy (mc_replace_strmem.c:883) ==24373==by 0x853BAB: uniqueentry (tsvector.c:127) ==24373==by 0x8541A5: tsvectorin (tsvector.c:262) ==24373==by 0x888781: InputFunctionCall (fmgr.c:1916) ==24373==by 0x888A7D: OidInputFunctionCall (fmgr.c:2047) ==24373==by 0x59B6D7: stringTypeDatum (parse_type.c:592) ==24373==by 0x580E14: coerce_type (parse_coerce.c:303) ==24373==by 0x580AD4: coerce_to_target_type (parse_coerce.c:101) ==24373==by 0x58B802: transformTypeCast (parse_expr.c:) ==24373==by 0x587484: transformExprRecurse (parse_expr.c:208) ==24373==by 0x587156: transformExpr (parse_expr.c:116) ==24373==by 0x5975CC: transformTargetEntry (parse_target.c:94) I didn't check out the tsvector case but the resolve_polymorphic_tupdesc/TupleDescInitEntry is clearly bogusly coded. Do we care? strncpy'ing a string over itself isn't defined... 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] Materialized views WIP patch
Noah Misch n...@leadboat.com wrote: On Sat, Feb 16, 2013 at 09:53:14AM -0800, Kevin Grittner wrote: I agree that making the dump fail on this account is bad. I would argue that this is an overstatement of the issue except for restores that use the single-transaction switch and pg_upgrade without the hard link option. In all other cases, one could just issue REFRESH statements after the dump successfully completed all the other work. But those two cases are important enough that the issue must be seriously considered. (1) Force mva to refresh on restore, even though it was empty and not scannable on dump. This may delay completion of the restore for an extended time. It would leave both mva and mvb populated. This is reasonable. If the user doesn't like it, he can presumably use an edited dump list to remove the REFRESHes. Overall, I recommend option 1. I'm OK with that approach, and in the absence of anyone pushing for another direction, will make that change to pg_dump. I'm thinking I would only do this for materialized views which were not scannable, but which cause REFRESH failures on other materialized views if not refreshed first (recursively evaluated), rather than just automatically refreshing all MVs on restore. The reason this seems important is that some MVs may take a long time to refresh, and a user might want a dump/restore to get to a point where they can use the rest of the database while building the contents of some MVs in the background or during off hours. None of the options will furnish the desire of every database, Agreed. but the DBA can always tailor the outcome by replacing the dumped REFRESH statements with his own. ... or by issuing TRUNCATE or REFRESH statements before the dump to avoid the issue. I'm not envisioning that MVs left invalid for the long term will be a typical thing, anyway. Agreed. I think this will be an infrequent issue caused by unusual user actions; but it would be bound to come up occasionally. -Kevin -- 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 PQconninfoParseParams and PQconninfodefaultsMerge to libpq
On Sun, Feb 17, 2013 at 1:35 AM, Amit kapila amit.kap...@huawei.com wrote: On Tuesday, February 12, 2013 2:49 AM Heikki Linnakangas wrote: On 04.02.2013 17:32, Alvaro Herrera wrote: Phil Sorber wrote: On Mon, Feb 4, 2013 at 10:16 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Phil Sorber wrote: On Mon, Feb 4, 2013 at 9:13 AM, Alvaro Herreraalvhe...@2ndquadrant.com wrote: I think this patch would simplift the patch to pass a connection string to pg_basebackup and pg_receivexlog: http://www.postgresql.org/message-id/005501cdfa45$6b0eec80$412cc580$@ko...@huawei.com. I note that pg_dumpall also has a similar issue as pg_basebackup and pg_receivexlog; there's no way to pass a connection string to it either. I have looked into both patches and my analysis is as below: In pg_basebackup patch, we have connection string and list of keywords (individual options specified by user), in the current available patch it has combined connection string and list of keywords as connection string and called PQconnectdb() which takes connection string as input. Now the patch of Phil Sober provides 2 new API's PQconninfoParseParams(), and PQconninfodefaultsMerge(), using these API's I can think of below way for patch pass a connection string to pg_basebackup, ... 1. Call existing function PQconinfoParse() with connection string input by user and get PQconninfoOption. 2. Now use the existing keywords (individual options specified by user) and extract the keywords from PQconninfoOption structure and call new API PQconninfoParseParams() which will return PQconninfoOption. The PQconninfoOption structure returned in this step will contain all keywords 3. Call PQconninfodefaultsMerge() to merge any default values if exist. Not sure if this step is required? 4. Extract individual keywords from PQconninfoOption structure and call PQconnectdbParams. Is this inline with what you have in mind or you have thought of some other simpler way of using new API's? With Regards, Amit Kapila. I think what would be nice is an additional connect function that took PQconninfoOption as a parameter. Or at least something that can convert from PQconninfoOption to a connection string or keyword/value arrays. -- 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] overlapping strncpy/memcpy errors via valgrind
On 2013-02-17 15:10:35 +, Greg Stark wrote: Peter G is sitting near me and reminded me that this issue came up in the past. Iirc the conclusion then is that we're calling memcpy where the source and destination pointers are sometimes identical. Tom decided there was really no realistic architecture where that wouldn't work. I am not so convinced that that is safe if libc turns that into some optimized string instructions or even PCMPSTR... We're not calling it on overlapping nonidentical pointers. Yup, the backtrace shows that... 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] overlapping strncpy/memcpy errors via valgrind
Andres Freund and...@2ndquadrant.com writes: On 2013-02-17 15:10:35 +, Greg Stark wrote: Peter G is sitting near me and reminded me that this issue came up in the past. Iirc the conclusion then is that we're calling memcpy where the source and destination pointers are sometimes identical. Tom decided there was really no realistic architecture where that wouldn't work. I am not so convinced that that is safe if libc turns that into some optimized string instructions or even PCMPSTR... What would you envision happening that would be bad? The reason overlapping source/dest is undefined is essentially that the function is allowed to copy bytes in an unspecified order. But if the pointers are the same, then no matter what it does, no individual store will replace a byte with a different value than it had, and so it's not possible for the order of operations to matter. I don't think it's worth contorting the source code to suppress this complaint. In the case of resolve_polymorphic_tupdesc, for instance, dodging this warning would require that we not use TupleDescInitEntry to update the data type of an attribute, but instead have this code know all about the subsidiary fields that get set from the datatype. I'm not seeing that as an improvement ... 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] overlapping strncpy/memcpy errors via valgrind
2013-02-17 16:32 keltezéssel, Tom Lane írta: Andres Freund and...@2ndquadrant.com writes: On 2013-02-17 15:10:35 +, Greg Stark wrote: Peter G is sitting near me and reminded me that this issue came up in the past. Iirc the conclusion then is that we're calling memcpy where the source and destination pointers are sometimes identical. Tom decided there was really no realistic architecture where that wouldn't work. I am not so convinced that that is safe if libc turns that into some optimized string instructions or even PCMPSTR... What would you envision happening that would be bad? The reason overlapping source/dest is undefined is essentially that the function is allowed to copy bytes in an unspecified order. But if the pointers are the same, then no matter what it does, no individual store will replace a byte with a different value than it had, and so it's not possible for the order of operations to matter. Then, why isn't memcpy() skipped if the source and dest are the same? It would be a micro-optimization but a valid one. I don't think it's worth contorting the source code to suppress this complaint. In the case of resolve_polymorphic_tupdesc, for instance, dodging this warning would require that we not use TupleDescInitEntry to update the data type of an attribute, but instead have this code know all about the subsidiary fields that get set from the datatype. I'm not seeing that as an improvement ... regards, tom lane -- -- Zoltán Böszörményi Cybertec Schönig Schönig GmbH Gröhrmühlgasse 26 A-2700 Wiener Neustadt, Austria Web: http://www.postgresql-support.de http://www.postgresql.at/ -- 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] overlapping strncpy/memcpy errors via valgrind
Boszormenyi Zoltan z...@cybertec.at writes: Then, why isn't memcpy() skipped if the source and dest are the same? It would be a micro-optimization but a valid one. No, it'd be more like a micro-pessimization, because the test would be wasted effort in the vast majority of calls. The *only* reason to do this would be to shut up valgrind, and that seems annoying. I wonder if anyone's tried filing a bug against valgrind to say that it shouldn't complain about this case. 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] FDW for PostgreSQL
Shigeru Hanada shigeru.han...@gmail.com writes: On Sun, Feb 17, 2013 at 8:44 AM, Tom Lane t...@sss.pgh.pa.us wrote: These don't seem to me like names that we ought to be exposing at the SQL command level. Why not just schema, table, column? Or perhaps schema_name, table_name, column_name if you feel it's essential to distinguish that these are names. I think not-shortened names (words used in documents of conversations) are better now. I prefer table_name to table, because it would be easy to distinguish as name, even if we add new options like table_foo. Yeah. I doubt that these options will be commonly used anyway --- surely it's easier and less confusing to choose names that match the remote table in the first place. So there's no very good reason to keep the option names short. I'll go with schema_name, table_name, column_name unless someone comes along with a contrary opinion. In psql \d+ result for postgres_fdw foreign tables, table and column are quoted, but schema is not. Is this behavior of quote_ident() intentional? That's probably a consequence of these being keywords of different levels of reserved-ness. If we go with the longer names it won't happen. 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] [RFC] indirect toast tuple support
On Sat, Feb 16, 2013 at 4:42 PM, Andres Freund and...@2ndquadrant.com wrote: I propose extending the EXTERNAL varlenas to be able to point to memory instead just to disk. It seem apt to use EXTERNAL for this as they aren't stored in the normal heap tuple but somewhere else. Unfortunately there is no backward-compatible flag space in varattrib_1b_e either to nicely denote this and we sure don't want to break on-disk compatibility for this. Thus I propose to distinguish on-disk and in-memory tuples via the varattrib_1b_e.va_len_1be. The intention was to use va_len_1be to allow extensibility with different external toast types. We were originally not going to have it at all and just before committing we became concerned that we wanted to avoid painting ourselves into a corner where we wouldn't be able to come up with any new formats for external toast types. So we added this one byte. i suggest thinking of it more as a type field that happens to be the length of the toast pointer by convention than an actual length header. Just as an example I have at various times proposed a column compression method that would store a dictionary of common values and the toast pointer would be a pointer to this dictionary and an index into it. I have no problem using it for this case since it's using up only one particular value for this field. As long as you don't want to use up all the possible values for a single type of external pointer it seems in line with the plan. -- greg -- 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] overlapping strncpy/memcpy errors via valgrind
On Sun, Feb 17, 2013 at 4:26 PM, Tom Lane t...@sss.pgh.pa.us wrote: No, it'd be more like a micro-pessimization, because the test would be wasted effort in the vast majority of calls. The *only* reason to do this would be to shut up valgrind, and that seems annoying. In terms of runtime I strongly suspect the effect would be 0 due to branch prediction. The effect on the code cleanliness seems like a stronger argument but I have a hard time getting upset about a single one-line if statement in namestrcpy. I suspect the argument may have been that we have no reason to believe namestrcpy is the only place this can happen. -- greg -- 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] JSON Function Bike Shedding
On Feb 17, 2013, at 6:33 AM, Andrew Dunstan and...@dunslane.net wrote: No, then we don't have a variadic version. You are going to have to accept that we can't make one function name cover all of this. Well, for me, I would rather specify an array than call a function with a different name. But it’s six of one, half-dozen of another, really, as long as it all works. D -- 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] overlapping strncpy/memcpy errors via valgrind
Tom Lane t...@sss.pgh.pa.us schrieb: Andres Freund and...@2ndquadrant.com writes: On 2013-02-17 15:10:35 +, Greg Stark wrote: Peter G is sitting near me and reminded me that this issue came up in the past. Iirc the conclusion then is that we're calling memcpy where the source and destination pointers are sometimes identical. Tom decided there was really no realistic architecture where that wouldn't work. I am not so convinced that that is safe if libc turns that into some optimized string instructions or even PCMPSTR... What would you envision happening that would be bad? Afair some of the optimized instructions (like movdqa) don't necessarily work if source an target are in the same location. Not sure about it bit I wouldn't want to depend on it. Andres --- Please excuse brevity and formatting - I am writing this on my mobile phone. -- 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] overlapping strncpy/memcpy errors via valgrind
Tom Lane t...@sss.pgh.pa.us schrieb: Boszormenyi Zoltan z...@cybertec.at writes: Then, why isn't memcpy() skipped if the source and dest are the same? It would be a micro-optimization but a valid one. No, it'd be more like a micro-pessimization, because the test would be wasted effort in the vast majority of calls. The *only* reason to do this would be to shut up valgrind, and that seems annoying. I wonder if anyone's tried filing a bug against valgrind to say that it shouldn't complain about this case. You already need a suppression file to use valgrind sensibly, its easy enough to add it there. Perhaps we should add one to the tree? --- Please excuse brevity and formatting - I am writing this on my mobile phone. -- 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] overlapping strncpy/memcpy errors via valgrind
On 17 February 2013 18:52, anara...@anarazel.de and...@anarazel.de wrote: You already need a suppression file to use valgrind sensibly, its easy enough to add it there. Perhaps we should add one to the tree? Perhaps you should take the time to submit a proper Valgrind patch first. The current approach of applying the patch that Noah Misch originally wrote (but did not publicly submit, iirc) on an ad-hoc basis isn't great. That is what you've done here, right? -- Regards, Peter Geoghegan -- 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] overlapping strncpy/memcpy errors via valgrind
Peter Geoghegan peter.geoghega...@gmail.com schrieb: On 17 February 2013 18:52, anara...@anarazel.de and...@anarazel.de wrote: You already need a suppression file to use valgrind sensibly, its easy enough to add it there. Perhaps we should add one to the tree? Perhaps you should take the time to submit a proper Valgrind patch first. The current approach of applying the patch that Noah Misch originally wrote (but did not publicly submit, iirc) on an ad-hoc basis isn't great. That is what you've done here, right? What patch are you talking about? I have no knowledge about any pending valgrind patches except one I made upstream apply to make pg inside valgrind work on amd64. Andres --- Please excuse brevity and formatting - I am writing this on my mobile phone. -- 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] overlapping strncpy/memcpy errors via valgrind
On 2013-02-17 19:52:16 +, Peter Geoghegan wrote: On 17 February 2013 19:39, anara...@anarazel.de and...@anarazel.de wrote: What patch are you talking about? I have no knowledge about any pending valgrind patches except one I made upstream apply to make pg inside valgrind work on amd64. Noah wrote a small patch, which he shared with me privately, which added Valgrind hook macros to aset.c and mcxt.c. The resulting Valgrind run threw up some things that were reported publicly [1]. I documented much of his work on the wiki. I was under the impression that this was the best way to get Valgrind to work with Postgres (apparently there were problems with many false positives otherwise). [1] http://www.postgresql.org/message-id/20110312133224.ga7...@tornado.gateway.2wire.net Nice, I wasn't aware of that work. I always wanted to add that instrumentation but never got arround to it. PG runs without problems for me with the exception of some warnings that I suppress. Would be nice to get that upstream... For reasons that have yet to be ascertained, it is necessary to run the regression tests with autovacuum = 'off'. Otherwise, Postgres will segfault within an autovacuum worker's elog() call. That's the bug I was referring to, its fixed at least in svn. It failed in far more places than that, basically everywhere an instruction that required the stack to be properly aligned was executed. The problem was that valgrind didn't align the new stack properly after a fork if the fork was executed inside a signal handler. Which pg happens to do... 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: PATCH: Split stats file per database WAS: [HACKERS] autovacuum stress-testing our system
On 17.2.2013 06:46, Alvaro Herrera wrote:
Tomas Vondra wrote:
I've been thinking about this (actually I had a really weird dream about
it this night) and I think it might work like this:
(1) check the timestamp of the global file - if it's too old, we need
to send an inquiry or wait a bit longer
(2) if it's new enough, we need to read it a look for that particular
database - if it's not found, we have no info about it yet (this is
the case handled by the dummy files)
(3) if there's a database stat entry, we need to check the timestamp
when it was written for the last time - if it's too old, send an
inquiry and wait a bit longer
(4) well, we have a recent global file, it contains the database stat
entry and it's fresh enough - tadaa, we're done
Hmm, yes, I think this is what I was imagining. I had even considered
that the timestamp would be removed from the per-db file as you suggest
here.
So, here's v10 of the patch (based on the v9+v9a), that implements the
approach described above.
It turned out to be much easier than I expected (basically just a
rewrite of the pgstat_read_db_statsfile_timestamp() function.
I've done a fair amount of testing (and will do some more next week) but
it seems to work just fine - no errors, no measurable decrease of
performance etc.
regards
Tomas Vondra
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 9b92ebb..36c0d8b 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -38,6 +38,7 @@
#include access/xact.h
#include catalog/pg_database.h
#include catalog/pg_proc.h
+#include lib/ilist.h
#include libpq/ip.h
#include libpq/libpq.h
#include libpq/pqsignal.h
@@ -66,8 +67,9 @@
* Paths for the statistics files (relative to installation's $PGDATA).
* --
*/
-#define PGSTAT_STAT_PERMANENT_FILENAME global/pgstat.stat
-#define PGSTAT_STAT_PERMANENT_TMPFILE global/pgstat.tmp
+#define PGSTAT_STAT_PERMANENT_DIRECTORY pg_stat
+#define PGSTAT_STAT_PERMANENT_FILENAME pg_stat/global.stat
+#define PGSTAT_STAT_PERMANENT_TMPFILE pg_stat/global.tmp
/* --
* Timer definitions.
@@ -115,6 +117,8 @@ int pgstat_track_activity_query_size = 1024;
* Built from GUC parameter
* --
*/
+char *pgstat_stat_directory = NULL;
+int pgstat_stat_dbfile_maxlen = 0;
char *pgstat_stat_filename = NULL;
char *pgstat_stat_tmpname = NULL;
@@ -219,11 +223,16 @@ static int localNumBackends = 0;
*/
static PgStat_GlobalStats globalStats;
-/* Last time the collector successfully wrote the stats file */
-static TimestampTz last_statwrite;
+/* Write request info for each database */
+typedef struct DBWriteRequest
+{
+ Oid databaseid; /* OID of the database to write */
+ TimestampTz request_time; /* timestamp of the last write request */
+ slist_node next;
+} DBWriteRequest;
-/* Latest statistics request time from backends */
-static TimestampTz last_statrequest;
+/* Latest statistics request times from backends */
+static slist_head last_statrequests = SLIST_STATIC_INIT(last_statrequests);
static volatile bool need_exit = false;
static volatile bool got_SIGHUP = false;
@@ -252,11 +261,16 @@ static void pgstat_sighup_handler(SIGNAL_ARGS);
static PgStat_StatDBEntry *pgstat_get_db_entry(Oid databaseid, bool create);
static PgStat_StatTabEntry *pgstat_get_tab_entry(PgStat_StatDBEntry *dbentry,
Oid tableoid, bool create);
-static void pgstat_write_statsfile(bool permanent);
-static HTAB *pgstat_read_statsfile(Oid onlydb, bool permanent);
+static void pgstat_write_statsfiles(bool permanent, bool allDbs);
+static void pgstat_write_db_statsfile(PgStat_StatDBEntry * dbentry, bool permanent);
+static HTAB *pgstat_read_statsfile(Oid onlydb, bool permanent, bool deep);
+static void pgstat_read_db_statsfile(Oid databaseid, HTAB *tabhash, HTAB *funchash, bool permanent);
static void backend_read_statsfile(void);
static void pgstat_read_current_status(void);
+static bool pgstat_write_statsfile_needed(void);
+static bool pgstat_db_requested(Oid databaseid);
+
static void pgstat_send_tabstat(PgStat_MsgTabstat *tsmsg);
static void pgstat_send_funcstats(void);
static HTAB *pgstat_collect_oids(Oid catalogid);
@@ -285,7 +299,6 @@ static void pgstat_recv_recoveryconflict(PgStat_MsgRecoveryConflict *msg, int le
static void pgstat_recv_deadlock(PgStat_MsgDeadlock *msg, int len);
static void pgstat_recv_tempfile(PgStat_MsgTempFile *msg, int len);
-
/*
* Public functions called from postmaster follow
*
@@ -541,16 +554,40 @@ startup_failed:
}
/*
+ * subroutine for pgstat_reset_all
+ */
+static void
+pgstat_reset_remove_files(const char *directory)
+{
+ DIR * dir;
+ struct dirent * entry;
+ char fname[MAXPGPATH];
+
+ dir = AllocateDir(pgstat_stat_directory);
+ while ((entry = ReadDir(dir,
Re: [HACKERS] Materialized views WIP patch
On 16 February 2013 01:01, Kevin Grittner kgri...@ymail.com wrote: Unless something else comes up in review or I get feedback to the contrary I plan to deal with the above-mentioned issues and commit this within a week or two. At the moment it's not possible to rename a column without using ALTER TABLE on an MV. Also, shouldn't we have the ability to set the collation on a column of the MV? And the inconsistency between regular views and MVs is still present, where MVs always dump with column parameters in their definition, and views never do. Not a show-stopper, but curious nonetheless. -- Thom -- 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] JSON Function Bike Shedding
On 02/17/2013 01:19 PM, David E. Wheeler wrote:
On Feb 17, 2013, at 6:33 AM, Andrew Dunstan and...@dunslane.net wrote:
No, then we don't have a variadic version. You are going to have to accept that
we can't make one function name cover all of this.
Well, for me, I would rather specify an array than call a function with a
different name. But it’s six of one, half-dozen of another, really, as long as
it all works.
I am going to go the way that involves the least amount of explicit
casting or array construction. So get_path() stays, but becomes
non-variadic. get() can take an int or variadic text[], so you can do:
get(myjson,0)
get(myjson,'f1')
get(myjson,'f1','2','f3')
get_path(myjson,'{f1,2,f3}')
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [RFC] ideas for a new Python DBAPI driver (was Re: [HACKERS] libpq test suite)
On Thursday 14 February 2013, Manlio Perillo wrote: Il 14/02/2013 14:06, Albe Laurenz ha scritto: Manlio Perillo wrote: Sorry for the question, but where can I find the libpq test suite? I can not find it in the PostgreSQL sources; it seems that there are only some examples, in src/test/examples. For my Python DBAPI2 PostgreSQL driver I plan the following optimizations: 1) always use PQsendQueryParams functions. This will avoid having to escape parameters, as it is done in psycopg2 (IMHO it still use simple query protocol for compatibility purpose) 2) when the driver detects a Python string is being sent to the database, use binary format. As a special case, this will avoid having to use PQescapeByteaConn when sending binary string (e.g. byte strings in Python 3.x) Perhaps you could also see some attempt I'd made to support binary protocol inside psycopg2, some time ago: https://github.com/xrg/psycopg/tree/execparams2 -- Say NO to spam and viruses. Stop using Microsoft Windows! -- 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] Temporal features in PostgreSQL
Hi, On 02/15/2013 10:46 PM, Cédric Villemain wrote: 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. I use similar constraints for application-time period tables but not for system versioned. Because they are automatically controlled by a trigger, there should be no need for additional integrity checks. If you want to speed up queries against historical data, you can create GIST index or an exclusion constraint. Vlad, is your source code in a public versionning system (github, bucket, etc) ? It will ease the process to participate to your extension... Yes, I uploaded it on github https://github.com/arkhipov/temporal_tables/ The extension is also available on PGXN http://pgxn.org/dist/temporal_tables/1.0.0/ -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation
Re: [HACKERS] [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq
On Sunday, February 17, 2013 8:44 PM Phil Sorber wrote: On Sun, Feb 17, 2013 at 1:35 AM, Amit kapila amit.kap...@huawei.com wrote: On Tuesday, February 12, 2013 2:49 AM Heikki Linnakangas wrote: On 04.02.2013 17:32, Alvaro Herrera wrote: Phil Sorber wrote: On Mon, Feb 4, 2013 at 10:16 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Phil Sorber wrote: On Mon, Feb 4, 2013 at 9:13 AM, Alvaro Herreraalvhe...@2ndquadrant.com wrote: I think this patch would simplift the patch to pass a connection string to pg_basebackup and pg_receivexlog: http://www.postgresql.org/message- id/005501cdfa45$6b0eec80$412cc580$@ko...@huawei.com. I note that pg_dumpall also has a similar issue as pg_basebackup and pg_receivexlog; there's no way to pass a connection string to it either. I have looked into both patches and my analysis is as below: In pg_basebackup patch, we have connection string and list of keywords (individual options specified by user), in the current available patch it has combined connection string and list of keywords as connection string and called PQconnectdb() which takes connection string as input. Now the patch of Phil Sober provides 2 new API's PQconninfoParseParams(), and PQconninfodefaultsMerge(), using these API's I can think of below way for patch pass a connection string to pg_basebackup, ... 1. Call existing function PQconinfoParse() with connection string input by user and get PQconninfoOption. 2. Now use the existing keywords (individual options specified by user) and extract the keywords from PQconninfoOption structure and call new API PQconninfoParseParams() which will return PQconninfoOption. The PQconninfoOption structure returned in this step will contain all keywords 3. Call PQconninfodefaultsMerge() to merge any default values if exist. Not sure if this step is required? 4. Extract individual keywords from PQconninfoOption structure and call PQconnectdbParams. Is this inline with what you have in mind or you have thought of some other simpler way of using new API's? With Regards, Amit Kapila. I think what would be nice is an additional connect function that took PQconninfoOption as a parameter. Or at least something that can convert from PQconninfoOption to a connection string or keyword/value arrays. Yes, it would be better if we would like to use new API's, I think it can save step-4 and some part in step-2. I am not sure for this purpose would it be okay to introduce new Connect API? I also feel it will increase the scope of patch. Heikki, would you be more specific that what in the patch you want to see simplified. Is the combining of keywords and connection string makes you feel the area where patch can be improved. With Regards, Amit Kapila. -- 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] review: ALTER ROLE ALL SET
On Wed, 2013-02-13 at 08:38 +0100, Pavel Stehule wrote: 5) Open question * I think so doc is not fully correct http://www.postgresql.org/message-id/CAFj8pRBf6suKewDCiXiGy=NeYY_Ns9CAZemomvRYsAQ=upl...@mail.gmail.com Fixed that and committed. * syntax when I try some variants I got not user friendly error message postgres=# alter role set work_mem = '1MB'; ERROR: unrecognized role option work_mem LINE 1: alter role set work_mem = '1MB'; ^ This issue already existed before. I don't think we can do much about it with one token lookahead. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
