Re: [HACKERS] Review: query result history in psql
Hello 2013/7/1 ian link i...@ilink.io: Not sure about all of your suggestions. Let me see if I can clarify what you're looking for. * simply decision if content should be stored in history or not, Do you mean that the user should use a flag to place the result of a query into the history? like: --ans SELECT * FROM cities... Not sure if that's what you mean, but it seems kind of unnecesary. They can just hit the \ans flag beforehand. switching off on is not user friendly but maybe some interactive mode should be usefull - so after execution, and showing result, will be prompt if result should be saved or not. some like: \ans interactive SELECT * FROM pg_proc; result should be saved last result [y, n]? y result is saved in :ans22 * simply remove last entry (table) of history That could be useful. What do you think Maciej? yes, lot of queries is just +/- experiment and you don't would store result * queries should be joined to content, only name is not enough Don't know what you mean. Could you try re-wording that? yes, the names :ans01, :ans02, ... miss semantics - How I can join this name (and content) with some SQL query? I needs to reverese search in SQL of stored caches, and I need a information ans01 SELECT * FROM pg_proc ans02 SELECT * FROM ans02 WHERE ... ans03 ... Regards Pavel Ian On Fri, Jun 28, 2013 at 8:49 AM, Pavel Stehule pavel.steh...@gmail.com wrote: Hello I am not sure, this interface is really user friendly there is not possible searching in history, and not every query push to history some interesting content. It require: * simply decision if content should be stored in history or not, * simply remove last entry (table) of history * queries should be joined to content, only name is not enough Regards Pavel 2013/6/28 Maciej Gajewski maciej.gajews...@gmail.com: Thanks for checking the patch! So what's left to fix? * Moving the escaping-related functions to separate module, * applying your corrections. Did I missed anything? I'll submit corrected patch after the weekend. M -- 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] plpython implementation
On 07/01/2013 07:53 AM, Claudio Freire wrote: On Mon, Jul 1, 2013 at 2:29 AM, james ja...@mansionfamily.plus.com wrote: On 01/07/2013 02:43, Claudio Freire wrote: In essence, you'd have to use another implementation. CPython guys have left it very clear they don't intend to fix that, as they don't consider it a bug. It's just how it is. Given how useful it is to have a scripting language that can be used outside of the database as well as inside it, would it be reasonable to consider 'promoting' pllua? My understanding is that it (lua) is much cleaner under the hood (than CPython). Although I do recognise that Python as a whole has always had more traction. Well, that, or you can use another implementation. There are many, and PyPy should be seriously considered given its JIT and how much faster it is for raw computation power, which is what a DB is most likely going to care about. OTOH, pypy startup time is bigger than CPython. It is also generally slower at running small on-call functions before JIT kicks in. I bet PyPy's sandboxing is a lot better as well. pypy sandbox implementation seems to be a sound one, as it delegates all unsafe operations to outside controller at bytecode level. The outside controller usually being a standard CPython wrapper. Of course this makes any such operations slower, but this is the price to pay for sandboxing. Making a postgres-interphasing pypy fork I guess would be a nice project, it's as simple as implementing all of plpy's API in RPython and translating a C module out of it. I have some ideas about allowing new pl-s to be written in pl/pythonu If any of you interested in this are at Europython come talk to me about this after my presentations ;) No, I'm not volunteering ;-) Neither am I, at least not yet -- Hannu Krosing PostgreSQL Consultant Performance, Scalability and High Availability 2ndQuadrant Nordic OÜ -- 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] Request for Patch Feedback: Lag Lead Window Functions Can Ignore Nulls
On 1 July 2013 03:07, Nicholas White n.j.wh...@gmail.com wrote: Alternatively, it might be trivial to make all aggregate functions work with ignore nulls in a window context This is a good idea, but I'd like to keep the scope of this patch limited for the time being Agreed. - I'll look at doing this (along with the first / last / nth value window functions) for a later release. On the other hand, perhaps this is not worth doing for aggregates, since in that case IGNORE NULLS is just a special case of FILTER (WHERE ...). Making IGNORE NULLS work for the other window functions is probably more useful, as you say, in a future patch. Regards, Dean -- 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] Support for RANGE ... PRECEDING windows in OVER
Definitely not this week. Hopefully for next commit fest. On Sun, Jun 30, 2013 at 9:56 PM, Josh Berkus j...@agliodbs.com wrote: On 06/30/2013 08:54 PM, ian link wrote: I found some time and I think I am up to speed now. I finally figured out how to add new operator strategies and made a little test operator for myself. It seems pretty clear that assuming '+' and '-' are addition and subtraction is a bad idea. I don't think it would be too tricky to add support for new operator strategies. Andrew Gierth suggested calling these new strategies offset - and offset +, which I think describes it pretty well. I assigned the operator itself to be @+ and @- but that can obviously be changed. If this sounds like a good path to you guys, I will go ahead and implement the operators for the appropriate types. Please let me know if I am misunderstanding something - I am still figuring stuff out :) Aside from the opclass stuff, there were some other important issues mentioned with the original RANGE support. I think I will address those after the opclass stuff is done. Are these things you plan to get done this week, or for next CommitFest? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Re: [HACKERS] Block write statistics WIP
Hi, I'm looking into this patch as a reviewer. (2013/05/24 10:33), Heikki Linnakangas wrote: On 23.05.2013 19:10, Greg Smith wrote: On 5/20/13 7:51 AM, Heikki Linnakangas wrote: It strikes me as kind of weird that the read side and write side are not more symmetrical. It might seem weird if you expect the API to be similar to POSIX read() and write(), where you can read() an arbitrary block at any location, and write() an arbitrary block at any location. A better comparison would be e.g open() and close(). open() returns a file descriptor, which you pass to other functions to operate on the file. When you're done, you call close(fd). The file descriptor is completely opaque to the user program, you do all the operations through the functions that take the fd as argument. Similarly, ReadBuffer() returns a Buffer, which is completely opaque to the caller, and you do all the operations through various functions and macros that operate on the Buffer. When you're done, you release the buffer with ReleaseBuffer(). (sorry for the digression from the original topic, I don't have any problem with what adding an optional Relation argument to MarkBuffer if you need that :-) ) Or should we add some pointer, which is accociated with the Relation, into BufferDesc? Maybe OID? I'm thinking of FlushBuffer() too. How can we count physical write for each relation in FlushBuffer()? Or any other appropriate place? BTW, Greg's first patch could not be applied to the latest HEAD. I guess it need to have some fix, I couldn't clafiry it though. Regards, -- Satoshi Nagayasu sn...@uptime.jp Uptime Technologies, LLC. http://www.uptime.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review: query result history in psql
but maybe some interactive mode should be usefull - so after execution, and showing result, will be prompt if result should be saved or not. I like the idea, in addition to the ordinary mode. Personally, I would use the ordinary mode, but I can see how 'interactive' would be useful. yes, the names :ans01, :ans02, ... miss semantics - How I can join this name (and content) with some SQL query? That makes sense. I think having part of / the whole query string would be very helpful. Great suggestion! Maciej, would you be able/have time to implement these? Or do you need any help getting them done? On Sun, Jun 30, 2013 at 11:35 PM, Pavel Stehule pavel.steh...@gmail.comwrote: Hello 2013/7/1 ian link i...@ilink.io: Not sure about all of your suggestions. Let me see if I can clarify what you're looking for. * simply decision if content should be stored in history or not, Do you mean that the user should use a flag to place the result of a query into the history? like: --ans SELECT * FROM cities... Not sure if that's what you mean, but it seems kind of unnecesary. They can just hit the \ans flag beforehand. switching off on is not user friendly but maybe some interactive mode should be usefull - so after execution, and showing result, will be prompt if result should be saved or not. some like: \ans interactive SELECT * FROM pg_proc; result should be saved last result [y, n]? y result is saved in :ans22 * simply remove last entry (table) of history That could be useful. What do you think Maciej? yes, lot of queries is just +/- experiment and you don't would store result * queries should be joined to content, only name is not enough Don't know what you mean. Could you try re-wording that? yes, the names :ans01, :ans02, ... miss semantics - How I can join this name (and content) with some SQL query? I needs to reverese search in SQL of stored caches, and I need a information ans01 SELECT * FROM pg_proc ans02 SELECT * FROM ans02 WHERE ... ans03 ... Regards Pavel Ian On Fri, Jun 28, 2013 at 8:49 AM, Pavel Stehule pavel.steh...@gmail.com wrote: Hello I am not sure, this interface is really user friendly there is not possible searching in history, and not every query push to history some interesting content. It require: * simply decision if content should be stored in history or not, * simply remove last entry (table) of history * queries should be joined to content, only name is not enough Regards Pavel 2013/6/28 Maciej Gajewski maciej.gajews...@gmail.com: Thanks for checking the patch! So what's left to fix? * Moving the escaping-related functions to separate module, * applying your corrections. Did I missed anything? I'll submit corrected patch after the weekend. M
Re: [HACKERS] Department of Redundancy Department: makeNode(FuncCall) division
On Mon, Jul 1, 2013 at 6:16 AM, David Fetter da...@fetter.org wrote: On Fri, Jun 28, 2013 at 01:28:35PM -0400, Peter Eisentraut wrote: On 6/28/13 11:30 AM, Robert Haas wrote: On Fri, Jun 28, 2013 at 10:31 AM, Tom Lane t...@sss.pgh.pa.us wrote: David Fetter da...@fetter.org writes: Please find attached the latest patch. I remain of the opinion that this is simply a bad idea. It is unlike our habits for constructing other types of nodes, and makes it harder not easier to find all the places that need to be updated when adding another field to FuncCall. I think it's a nice code cleanup. I don't understand your objection. Yeah, I was reading the patch thinking, yes, finally someone cleans that up. Please find enclosed a patch reflecting the changes that de-reserved OVER as a keyword. I have re-validated this new patch and it looks good to go in now. I saw that it's already marked ready for committer. Thanks David Cheers, David. -- David Fetter da...@fetter.org 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 -- Jeevan B Chalke Senior Software Engineer, RD EnterpriseDB Corporation The Enterprise PostgreSQL Company Phone: +91 20 30589500 Website: www.enterprisedb.com EnterpriseDB Blog: http://blogs.enterprisedb.com/ Follow us on Twitter: http://www.twitter.com/enterprisedb This e-mail message (and any attachment) is intended for the use of the individual or entity to whom it is addressed. This message contains information from EnterpriseDB Corporation that may be privileged, confidential, or exempt from disclosure under applicable law. If you are not the intended recipient or authorized to receive this for the intended recipient, any use, dissemination, distribution, retention, archiving, or copying of this communication is strictly prohibited. If you have received this e-mail in error, please notify the sender immediately by reply e-mail and delete this message.
Re: [HACKERS] plpython implementation
On 2013-06-30 22:43:52 -0300, Claudio Freire wrote: Not only that, the CPython interpreter is rather fuzzy about the division between interpreters. You can initialize multiple interpreters, but they share a lot of state, so you can never fully separate them. You'd have some state from the untrusted interpreter spill over into the trusted one within the same session, which is not ideal at all (and in fact can be exploited). In essence, you'd have to use another implementation. CPython guys have left it very clear they don't intend to fix that, as they don't consider it a bug. It's just how it is. Doesn't zope's RestrictedPython have a history of working reasonably well? Now, you sure pay a price for that, but ... 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: simple date constructor from numeric values
Hello 2013/6/29 Pavel Stehule pavel.steh...@gmail.com: Hello long time I am thinking about simple function for creating date or timestamp values based on numeric types without necessity to create format string. some like ansi_date(year, month, day) and ansi_timestamp(year, month, day, hour, minuts, sec, msec, offset) What do you think about this idea? I found so same idea was discussed three years ago http://www.postgresql.org/message-id/14107.1276443...@sss.pgh.pa.us and it is a part of our ToDo: Add function to allow the creation of timestamps using parameters so we can have a functions with signatures CREATE OR REPLACE FUNCTION construct_date(year int, month int DEFAULT 1, day int DEFAULT 1) RETURNS date; CREATE OR REPLACE FUNCTION construct_time(hour int DEFAULT 0, mi int DEFAULT 0, sec int DEFAULT 0, ms float DEFAULT 0.0); CREATE OR REPLACE FUNCTION construct_timestap(year int, month int DEFAULT CREATE OR REPLACE FUNCTION construct_timestamp_with_timezone(year int, month int DEFAULT1, ... ??? Regards Pavel Stehule Regards Pavel Stehule -- 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] Optimizing pglz compressor
On 26.06.2013 16:37, Amit Kapila wrote: On Wednesday, June 26, 2013 2:15 AM Heikki Linnakangas wrote: Can you also try the attached patch, please? It's the same as before, but in this version, I didn't replace the prev and next pointers in PGLZ_HistEntry struct with int16s. That avoids some table lookups, at the expense of using more memory. It's closer to what we have without the patch, so maybe that helps on your system. Yes it helped a lot on my system. Ok, good. Strange, I did not expect such a big difference. There was minor problem in you patch, in one of experiments it crashed. Fix is not to access 0th history entry in function pglz_find_match(), modified patch is attached. Thanks, good catch! I thought that a pointer to the 0th entry would never make it into the prev/next fields, but it does. In fact, we never store a NULL there anymore, a pointer to the 0th entry is now always used to mean 'invalid'. I adjusted the patch to remove the NULL check, and only check for the 0th entry. Committed. - Heikki -- 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: query result history in psql
I'm not really bought into some of the ideas. but maybe some interactive mode should be usefull - so after execution, and showing result, will be prompt if result should be saved or not. I like the idea, in addition to the ordinary mode. Personally, I would use the ordinary mode, but I can see how 'interactive' would be useful. This would require a complex change to the client code. And the result would eventually become annoying: an interactive question after each and every query. Currently, when turned on, every result is stored and simple notification is printed. yes, the names :ans01, :ans02, ... miss semantics - How I can join this name (and content) with some SQL query? That makes sense. I think having part of / the whole query string would be very helpful. Great suggestion! The naming is obscure and non-informative, I agree. If you have a nice idea how to make it better, I'd love to discuss it. But please remember that it has one huge advantage: simplicity. The client is a classical command-line tool, and as such it delegates some of the functionality to external programs, like pager or terminal. I'm pretty sure that your terminal emulator has a 'find' function that would allow you to quickly locate the variable and associated query in the scrollback. M
Re: [HACKERS] Review: query result history in psql
2013/7/1 Maciej Gajewski maciej.gajews...@gmail.com: I'm not really bought into some of the ideas. but maybe some interactive mode should be usefull - so after execution, and showing result, will be prompt if result should be saved or not. I like the idea, in addition to the ordinary mode. Personally, I would use the ordinary mode, but I can see how 'interactive' would be useful. This would require a complex change to the client code. And the result would eventually become annoying: an interactive question after each and every query. Currently, when turned on, every result is stored and simple notification is printed. . When I tested this feature, I had 30 caches per 5 minutes, and only a few from these queries had a sense. Switch between off and on is not user friendly. I believe so there can be other solution than mine, but a possibility to friendly clean unwanted caches is necessary. yes, the names :ans01, :ans02, ... miss semantics - How I can join this name (and content) with some SQL query? That makes sense. I think having part of / the whole query string would be very helpful. Great suggestion! The naming is obscure and non-informative, I agree. If you have a nice idea how to make it better, I'd love to discuss it. But please remember that it has one huge advantage: simplicity. The client is a classical command-line tool, and as such it delegates some of the functionality to external programs, like pager or terminal. Personally, I don't see a strong price for all users without friendly interface. Regards Pavel I'm pretty sure that your terminal emulator has a 'find' function that would allow you to quickly locate the variable and associated query in the scrollback. M -- 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] Bugfix and new feature for PGXS
Le samedi 29 juin 2013 22:00:34, Josh Berkus a écrit : On 06/29/2013 11:42 AM, Andrew Dunstan wrote: I think we can survive for now without an additional tool. What I did was a proof of concept, it was not intended as a suggestion that we should install prep_buildtree. I am only suggesting that, in addition to your changes, if USE_VPATH is set then that path is used instead of a path inferred from the name of the Makefile. SO is this patch returned with feedback? Only the 0005-Allows-extensions-to-install-header-file.patch , others are in the hands of Andrew. Additional patch required for documentation. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] [PATCH] add --progress option to pgbench (submission 3)
(2013/06/28 3:17), Fabien COELHO wrote: Attached is patch version 5. It includes this solution for fork emulation, one report per thread instead of a global report. Some code duplication for that. It's good coding. I test configure option with --disable-thread-safety and not. My test results were same as your proposal. It fix problems that compatiblity and progress time is off to the side, too. Here is the test results. * with --disable-thread-safety [mitsu-ko@localhost postgresql]$ bin/pgbench -T 600 -c10 -j5 -P 5 starting vacuum...end. progress 1: 5.0 s, 493.8 tps, 4.050 ms lat progress 2: 5.0 s, 493.2 tps, 4.055 ms lat progress 3: 5.0 s, 474.6 tps, 4.214 ms lat progress 4: 5.0 s, 479.1 tps, 4.174 ms lat progress 0: 5.0 s, 469.5 tps, 4.260 ms lat * without --disable-thread-safety (normal) [mitsu-ko@localhost postgresql]$ bin/pgbench -T 600 -c10 -j5 -P 5 starting vacuum...end. progress: 5.0 s, 2415.0 tps, 4.141 ms lat progress: 10.0 s, 2445.5 tps, 4.089 ms lat progress: 15.0 s, 2442.2 tps, 4.095 ms lat progress: 20.0 s, 2414.3 tps, 4.142 ms lat Finally, I've added a latency measure as defended by Mitsumasa. However the formula must be updated for the throttling patch. Thanks! In benchmark test, it is not good to over throttle. It is difficult to set appropriate options which are number of client or number of threads. These result will help to set appropriate throttle options. We can easy to search by these information which is indicated as high tps and low latency as possible. I have small comments. I think that 'lat' is not generally abbreviation of 'latency'. But I don't know good abbreviation. If you have any good abbreviation, please send us revise version. And, please fix under following code. It might be degrade by past your patches. --P, --progress SEC show thread progress report every SEC seconds\n +-P, --progress NUM show thread progress report every NUM seconds\n - tps = 100.0 * (count-last_count) / run; + tps = 100.0 * (count - last_count) / run; My comments are that's all. If you send latest patch, I'm going to set ready for commiter. I also test your throttle patch. My impression of this patch is good, but it does not necessary to execute with progress option. Because, in the first place, throttle patch is controlling transaction of pgbench, and it does not need to display progress which will be same information which is expected by a user. A user who uses throttle patch will think that throttle patch can control transaction exactly, and it is not debugging option. So I think that it had better to increase the accuracy of throttle patch, and it does not need to exist together of both patches. If you think that it cannot exist together, I suggest that forbidding simultaneously progress option and throttle option. Best regards, -- Mitsumasa KONDO NTT Open Source Software Center -- 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] GIN improvements part 3: ordering in index
On Sun, Jun 30, 2013 at 3:09 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 25.06.2013 21:18, Alexander Korotkov wrote: On Tue, Jun 25, 2013 at 7:31 PM, Heikki Linnakangashlinnakangas@** vmware.com hlinnakan...@vmware.com wrote: In summary: The test case you presented as motivation for this patch is a bit of a worst-case scenario for the current tidbitmap implementation. The speedup from your patch comes from avoiding the tidbitmap. However, it would be fairly easy to optimize the tidbitmap to handle this scenario better, which would benefit all kinds of queries that use bitmap scans. There is really no reason to complicate the GIN API for this. Let's just optimize tidbitmap. I'm not sure if I fullly understand your patch, though. Is there some other test scenario where it performs significantly better, which can not be attributed to a tidbitmap overhead? I'm assuming 'no' for now, and marking this patch as rejected in the commitfest app, but feel free to reopen if there is. So, it's likely I've positioned this patch wrong from the begging, because my examples were focused on CPU time improvement. But initial purpose of this patch was to decrease IO. Ok. Storing the additional information bloats the index considerably, so it's clearly not going to be a win in all cases. So whether you store the additional information or not needs to configurable somehow. Yes, I think we should have two distinct opclasses. -- With best regards, Alexander Korotkov.
Re: [HACKERS] GIN improvements part 1: additional information
On Sun, Jun 30, 2013 at 2:50 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 29.06.2013 20:08, Heikki Linnakangas wrote: On 29.06.2013 11:56, Heikki Linnakangas wrote: I made one significant change: I removed the 'freespace' field you added to GinpageOpaque. Instead, on data leaf pages the offset from the beginning of the page where the packed items end is stored in place of the 'maxoff' field. This allows for quick calculation of the free space, but there is no count of item pointers stored on the page anymore, so some code that looped through all the item pointers relying on 'maxoff' had to be changed to work with the end offset instead. I'm not 100% wedded on this, but I'd like to avoid adding the redundant freespace field on pages that don't need it, because it's confusing and you have to remember to keep them in sync. Ah, crap, looks like I sent the wrong patch, and now I can't find the correct one anymore. The patch I sent didn't include the changes store end offset instead of freespace. I'll rewrite that part.. Here's the correct version. I've probably broken things, sorry about that. I'm going to mark this as returned with feedback now. The code still needs a lot of general cleanup, including comment and README updates. Also, please take some time to consider the open questions I listed here: archives.postgresql.org/**message-id/51CEA13C.8040103@**vmware.comhttp://archives.postgresql.org/message-id/51cea13c.8040...@vmware.com . Thanks! So, we have a lot of stuff and you give the points for further work. Could you please verify my plan of work on these patches: 1) Solving questions of archives.postgresql.org/** message-id/51CEA13C.8040103@**vmware.comhttp://archives.postgresql.org/message-id/51cea13c.8040...@vmware.com for packed postinglists. 2) Extract additional info patch based on packed postinglists. 3) Rewrite interface of fast scan. Do CPU and IO benchmarking. 4) Do IO benchmarking of index ordering. Cleanup, comments and READMEs are assumed in each item. -- With best regards, Alexander Korotkov.
Re: [HACKERS] GIN improvements part 1: additional information
On Thu, Jun 27, 2013 at 6:20 PM, Antonin Houska antonin.hou...@gmail.comwrote: On 06/25/2013 12:03 AM, Alexander Korotkov wrote: New revision of patch is attached. Now it includes some docs. Hi, I was curious about the new layout of the data page, so I spent a while looking into the code. It's interesting, but I suspect 2 things are not o.k.: * gindatapage.c:**dataIsEnoughSpace() - 'i++' in the for loop should probably be 'j++', otherwise it loops forever * gin_private.h:**ginDataPageLeafRead() - fetch_att() is used to retrieve the additional info, but per the definition and comments in tupmacs.h it expects aligned pointer. * gindatapage.c:**ginCheckPlaceToDataPageLeaf() - comment if leaf data page should probably be on a leaf data page or so. Hi! Thanks for pointing these. -- With best regards, Alexander Korotkov.
Re: [HACKERS] [PATCH] add --progress option to pgbench (submission 3)
Dear Mitsumasa, I have small comments. I think that 'lat' is not generally abbreviation of 'latency'. But I don't know good abbreviation. If you have any good abbreviation, please send us revise version. I needed something short, because I may add a lag time as well under throttling. No better idea. And, please fix under following code. It might be degrade by past your patches. Done. I've also put the long option definition at its right place in the alphabetical order. My comments are that's all. If you send latest patch, I'm going to set ready for commiter. Please find attached version 6. I also test your throttle patch. My impression of this patch is good, but it does not necessary to execute with progress option. [...] I agree that it is not necessary. However for my use case it would be useful to have both throttling progress at the same time, in particular to check the effect of other concurrent operations (eg. pg_dump, pg_basebackup) while a bench is running. -- Fabien.diff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.c index 80203d6..4e8c607 100644 --- a/contrib/pgbench/pgbench.c +++ b/contrib/pgbench/pgbench.c @@ -74,7 +74,7 @@ static int pthread_join(pthread_t th, void **thread_return); #include pthread.h #else /* Use emulation with fork. Rename pthread identifiers to avoid conflicts */ - +#define PTHREAD_FORK_EMULATION #include sys/wait.h #define pthread_tpg_pthread_t @@ -164,6 +164,8 @@ bool use_log; /* log transaction latencies to a file */ bool use_quiet; /* quiet logging onto stderr */ int agg_interval; /* log aggregates instead of individual * transactions */ +int progress = 0; /* thread progress report every this seconds */ +int progress_nclients = 0; /* number of clients for progress report */ bool is_connect; /* establish connection for each transaction */ bool is_latencies; /* report per-command latencies */ int main_pid; /* main process id used in log filename */ @@ -352,6 +354,7 @@ usage(void) (default: simple)\n -n, --no-vacuum do not run VACUUM before tests\n -N, --skip-some-updates skip updates of pgbench_tellers and pgbench_branches\n + -P, --progress NUM show thread progress report every NUM seconds\n -r, --report-latencies report average latency per command\n -s, --scale=NUM report this scale factor in output\n -S, --select-onlyperform SELECT-only transactions\n @@ -2119,6 +2122,7 @@ main(int argc, char **argv) {log, no_argument, NULL, 'l'}, {no-vacuum, no_argument, NULL, 'n'}, {port, required_argument, NULL, 'p'}, + {progress, required_argument, NULL, 'P'}, {protocol, required_argument, NULL, 'M'}, {quiet, no_argument, NULL, 'q'}, {report-latencies, no_argument, NULL, 'r'}, @@ -2202,7 +2206,7 @@ main(int argc, char **argv) state = (CState *) pg_malloc(sizeof(CState)); memset(state, 0, sizeof(CState)); - while ((c = getopt_long(argc, argv, ih:nvp:dqSNc:j:Crs:t:T:U:lf:D:F:M:, long_options, optindex)) != -1) + while ((c = getopt_long(argc, argv, ih:nvp:dqSNc:j:Crs:t:T:U:lf:D:F:M:P:, long_options, optindex)) != -1) { switch (c) { @@ -2357,6 +2361,16 @@ main(int argc, char **argv) exit(1); } break; + case 'P': +progress = atoi(optarg); +if (progress = 0) +{ + fprintf(stderr, + thread progress delay (-P) must not be negative (%s)\n, + optarg); + exit(1); +} +break; case 0: /* This covers long options which take no argument. */ break; @@ -2482,6 +2496,7 @@ main(int argc, char **argv) * changed after fork. */ main_pid = (int) getpid(); + progress_nclients = nclients; if (nclients 1) { @@ -2733,6 +2748,11 @@ threadRun(void *arg) int nstate = thread-nstate; int remains = nstate; /* number of remaining clients */ int i; + /* for reporting progress: */ + int64 thread_start = INSTR_TIME_GET_MICROSEC(thread-start_time); + int64 last_report = thread_start; + int64 next_report = last_report + progress * 100; + int64 last_count = 0; AggVals aggs; @@ -2896,6 +2916,68 @@ threadRun(void *arg) st-con = NULL; } } + +#ifdef PTHREAD_FORK_EMULATION + /* each process reports its own progression */ + if (progress) + { + instr_time now_time; + int64 now; + INSTR_TIME_SET_CURRENT(now_time); + now = INSTR_TIME_GET_MICROSEC(now_time); + if (now = next_report) + { +/* generate and show report */ +int64 count = 0; +int64 run = now - last_report; +float tps, total_run, latency; + +for (i = 0 ; i nstate ; i++) + count += state[i].cnt; + +total_run = (now - thread_start) / 100.0; +tps = 100.0 * (count - last_count) / run; +latency = 1000.0 * nstate / tps; + +fprintf(stderr, progress %d: %.1f s, %.1f tps, %.3f ms lat\n, + thread-tid, total_run, tps,
Re: [HACKERS] Review: query result history in psql
When I tested this feature, I had 30 caches per 5 minutes, and only a few from these queries had a sense. Switch between off and on is not user friendly. I believe so there can be other solution than mine, but a possibility to friendly clean unwanted caches is necessary. If you know that you'll need the result of a query beforehand, you can always use SELECT ... INTO ... . No client-side features required. This feature is intended for people running plenty of ad-hoc queries, when every result could potentially be useful.
Re: [HACKERS] Review: query result history in psql
2013/7/1 Maciej Gajewski maciej.gajews...@gmail.com: When I tested this feature, I had 30 caches per 5 minutes, and only a few from these queries had a sense. Switch between off and on is not user friendly. I believe so there can be other solution than mine, but a possibility to friendly clean unwanted caches is necessary. If you know that you'll need the result of a query beforehand, you can always use SELECT ... INTO ... . No client-side features required. This feature is intended for people running plenty of ad-hoc queries, when every result could potentially be useful. a idea is good, but I don't think, it can be useful with current implementation. How I can identify, what is correct answer for my query? Have I remember twenty numbers and twenty queries? Regards Pavel -- 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 --progress option to pgbench (submission 3)
Hi, Febien Thanks for your fast response and fix! I set your patch ready for commiter now. (2013/07/01 19:49), Fabien COELHO wrote: I have small comments. I think that 'lat' is not generally abbreviation of 'latency'. But I don't know good abbreviation. If you have any good abbreviation, please send us revise version. I needed something short, because I may add a lag time as well under throttling. No better idea. OK. We have no idea:-) And, please fix under following code. It might be degrade by past your patches. Done. I've also put the long option definition at its right place in the alphabetical order. Oh, I leak it in my review. Thanks. I also test your throttle patch. My impression of this patch is good, but it does not necessary to execute with progress option. [...] I agree that it is not necessary. However for my use case it would be useful to have both throttling progress at the same time, in particular to check the effect of other concurrent operations (eg. pg_dump, pg_basebackup) while a bench is running. It is very dicreet checking! I think it is important for momentous systems, too. If I have time for reviewing throttle patch for more detail, I will send you comment. I hope both patches are commited. Best regards, -- Mitsumasa KONDO NTT Open Source Software Center -- 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] Request for Patch Feedback: Lag Lead Window Functions Can Ignore Nulls
On Sun, Jun 30, 2013 at 10:07 PM, Nicholas White n.j.wh...@gmail.com wrote: I've attached another iteration of the patch that fixes the multiple-window bug and adds ( uses) a function to create a Bitmapset using a custom allocator. I don't think there's any outstanding problems with it now. I think the right way to do this is to temporarily set the current memory context to winobj-winstate-partcontext while creating or manipulating the Bitmapset and restore it afterwards. Maybe someone will say that's a modularity violation, but surely this is worse... -- 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] Review: Display number of changed rows since last analyze
On Mon, Jun 17, 2013 at 1:49 PM, Albe Laurenz laurenz.a...@wien.gv.at wrote: This is a review of the patch in 5192d7d2.8020...@catalyst.net.nz The patch applies cleanly (with the exception of catversion.h of course), compiles without warnings and passes the regression tests. It contains enough documentation, though I'd prefer Estimated number of rows modified since the table was last analyzed to Estimated number of row changes (inserts + updates + deletes) since the last analyze The patch works as it should, and I think that this is a useful addition. It only exposes a value that is already available internally, so there shouldn't be any penalties. I think that the column name is ok as it is, even if it is a bit long - I cannot come up with a more succinct idea. Perhaps n_changed_since_analyze could be shortened to n_mod_since_analyze, but that's not much of an improvement. AFAICT it's related to n_live_tup, and n_dead_tup. How about just n_mod_tup? Though that doesn't convey that it's since the last analyze, I guess. But given that both n_dead_tup and n_live_tup don't really indicate that they're not since the beginning of stats in the name (which other stats counters are), I'm not sure that's a problem? It would be a name that sounds more similar to the rest of the table. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] Request for Patch Feedback: Lag Lead Window Functions Can Ignore Nulls
On 1 July 2013 03:07, Nicholas White n.j.wh...@gmail.com wrote: I've attached another iteration of the patch that fixes the multiple-window bug and adds ( uses) a function to create a Bitmapset using a custom allocator. I don't think there's any outstanding problems with it now. I just realised there is another issue (sorry). pg_get_viewdef() needs to be updated so that dumping and restoring a view that uses ignore/respect nulls works properly. Regards, Dean -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] LDAP: bugfix and deprecated OpenLDAP API
On Tue, Feb 5, 2013 at 10:39 AM, Albe Laurenz laurenz.a...@wien.gv.at wrote: I found a small bug in the implementation of LDAP connection parameter lookup. As documented in http://www.postgresql.org/docs/current/static/libpq-ldap.html processing should continue after a failed attempt to connect to an LDAP server. The code in src/interfaces/libpq/fe-connect.c defines a timeout of two seconds so that this failure won't block the libpq connection attempt for a long time. As coded now, the timeout won't work - if the LDAP server is down, ldap_simple_bind will wait for the network timeout, which will be quite longer than 2 seconds. The attached patch ldap-bug.patch fixes this problem; unfortunately I found no way that works both with OpenLDAP and Windows LDAP, so I had to add an #ifdef. I think that this patch should be applied and backpatched. So just to be clear - the difference is we're going from implicit anonymous bind, to an explicit one? We're not actually causing an extra bind compared to previous versions? I also tried to fix the problem mentioned in http://www.postgresql.org/message-id/CA+TgmoYnj=Es3L_0Q8+ijR4tVhvztW1fb=7c9k9gemzwqhp...@mail.gmail.com that we use deprecated OpenLDAP functions, see the attached ldap-undeprecate.patch. I added a file ldap.c in src/port with my own implementation of some of the functions that OpenLDAP has deprecated. With that, the code changes necessary are pretty minimal. Doesn't this need a version check against OpenSSL at some point, or a configure check? Are we just assuming that all versions that people ever use have the function deprecated? (That's probably not entirely unreasonable, just double checking) -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] LDAP: bugfix and deprecated OpenLDAP API
On 7/1/13 7:58 AM, Magnus Hagander wrote: I also tried to fix the problem mentioned in http://www.postgresql.org/message-id/CA+TgmoYnj=Es3L_0Q8+ijR4tVhvztW1fb=7c9k9gemzwqhp...@mail.gmail.com that we use deprecated OpenLDAP functions, see the attached ldap-undeprecate.patch. I added a file ldap.c in src/port with my own implementation of some of the functions that OpenLDAP has deprecated. With that, the code changes necessary are pretty minimal. Doesn't this need a version check against OpenSSL at some point, or a configure check? Are we just assuming that all versions that people ever use have the function deprecated? (That's probably not entirely unreasonable, just double checking) Btw., I just checked the source code of Apache, PHP, and PAM, and they are all unconditionally building with LDAP_DEPRECATED. So maybe there is no hurry about this. -- 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: Display number of changed rows since last analyze
Magnus Hagander wrote: On Mon, Jun 17, 2013 at 1:49 PM, Albe Laurenz laurenz.a...@wien.gv.at wrote: This is a review of the patch in 5192d7d2.8020...@catalyst.net.nz The patch applies cleanly (with the exception of catversion.h of course), compiles without warnings and passes the regression tests. It contains enough documentation, though I'd prefer Estimated number of rows modified since the table was last analyzed to Estimated number of row changes (inserts + updates + deletes) since the last analyze The patch works as it should, and I think that this is a useful addition. It only exposes a value that is already available internally, so there shouldn't be any penalties. I think that the column name is ok as it is, even if it is a bit long - I cannot come up with a more succinct idea. Perhaps n_changed_since_analyze could be shortened to n_mod_since_analyze, but that's not much of an improvement. AFAICT it's related to n_live_tup, and n_dead_tup. How about just n_mod_tup? Though that doesn't convey that it's since the last analyze, I guess. But given that both n_dead_tup and n_live_tup don't really indicate that they're not since the beginning of stats in the name (which other stats counters are), I'm not sure that's a problem? It would be a name that sounds more similar to the rest of the table. I don't get that. As far as I know, n_dead_tup and n_live_tup are estimates for the total number of live and dead tuples, period. Both numbers are not reset to zero when ANALYZE (or even VACUUM) takes place. Yours, Laurenz Albe -- 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: Display number of changed rows since last analyze
On Mon, Jul 1, 2013 at 2:48 PM, Albe Laurenz laurenz.a...@wien.gv.at wrote: Magnus Hagander wrote: On Mon, Jun 17, 2013 at 1:49 PM, Albe Laurenz laurenz.a...@wien.gv.at wrote: This is a review of the patch in 5192d7d2.8020...@catalyst.net.nz The patch applies cleanly (with the exception of catversion.h of course), compiles without warnings and passes the regression tests. It contains enough documentation, though I'd prefer Estimated number of rows modified since the table was last analyzed to Estimated number of row changes (inserts + updates + deletes) since the last analyze The patch works as it should, and I think that this is a useful addition. It only exposes a value that is already available internally, so there shouldn't be any penalties. I think that the column name is ok as it is, even if it is a bit long - I cannot come up with a more succinct idea. Perhaps n_changed_since_analyze could be shortened to n_mod_since_analyze, but that's not much of an improvement. AFAICT it's related to n_live_tup, and n_dead_tup. How about just n_mod_tup? Though that doesn't convey that it's since the last analyze, I guess. But given that both n_dead_tup and n_live_tup don't really indicate that they're not since the beginning of stats in the name (which other stats counters are), I'm not sure that's a problem? It would be a name that sounds more similar to the rest of the table. I don't get that. As far as I know, n_dead_tup and n_live_tup are estimates for the total number of live and dead tuples, period. Both numbers are not reset to zero when ANALYZE (or even VACUUM) takes place. No, but they are zero *until* vacuum runs. The point I was trying to make was that they are showing an absolute number. Unlike for example n_tup_inserted and friends which show the total number of event since stat reset. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] New regression test time
On Monday, July 01, 2013 8:37 AM Josh Berkus wrote: On 06/30/2013 12:33 AM, Amit kapila wrote: On Sunday, June 30, 2013 11:37 AM Fabien COELHO wrote: If we had a different set of tests, that would be a valid argument. But we don't, so it's not. And nobody has offered to write a feature to split our tests either. I have done a POC. See: https://commitfest.postgresql.org/action/patch_view?id=1170 I think it is better to submit for next commit fest which is at below link: https://commitfest.postgresql.org/action/commitfest_view?id=19 I would argue for doing this in this CF, just so that we can have the benefit of the extra tests for the next 3 months, and so that Andrew can work on the buildfarm additions. My mail was just a suggestion, as in general after start of CF all new patch submissions for review are done in next CF. However as for this particular patch, there are quite a few other dependent patches, so it will be good if this patch gets committed in this CF only. Also as a CFM, you are better person to decide about it. 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: Display number of changed rows since last analyze
Magnus Hagander wrote: On Mon, Jun 17, 2013 at 1:49 PM, Albe Laurenz laurenz.a...@wien.gv.at wrote: I think that the column name is ok as it is, even if it is a bit long - I cannot come up with a more succinct idea. Perhaps n_changed_since_analyze could be shortened to n_mod_since_analyze, but that's not much of an improvement. AFAICT it's related to n_live_tup, and n_dead_tup. How about just n_mod_tup? Though that doesn't convey that it's since the last analyze, I guess. But given that both n_dead_tup and n_live_tup don't really indicate that they're not since the beginning of stats in the name (which other stats counters are), I'm not sure that's a problem? It would be a name that sounds more similar to the rest of the table. I don't get that. As far as I know, n_dead_tup and n_live_tup are estimates for the total number of live and dead tuples, period. Both numbers are not reset to zero when ANALYZE (or even VACUUM) takes place. No, but they are zero *until* vacuum runs. The point I was trying to make was that they are showing an absolute number. Unlike for example n_tup_inserted and friends which show the total number of event since stat reset. Ok, I understand you now. All the old names are fairly intuitive in my opinion. Number of life tuples since the statistics were reset doesn't make a lot of sense to me, so I would automatically read that as an absolute number. But it would not be clear to me that n_mod_tuples are counted since the last ANALYZE (different from other columns); I would jump to the conclusion that it is a sum of n_tup_ins, n_tup_upd and n_tup_del. So I think that a name that it less likely to cause confusion would be better that a short, but misleading name. Yours, Laurenz Albe -- 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] XLogInsert scaling, revisited
On 2013-06-26 18:52:30 +0300, Heikki Linnakangas wrote: * Could you document the way slots prevent checkpoints from occurring when XLogInsert rechecks for full page writes? I think it's correct - but not very obvious on a glance. There's this in the comment near the top of the file: * To update RedoRecPtr or fullPageWrites, one has to make sure that all * subsequent inserters see the new value. This is done by reserving all the * insertion slots before changing the value. XLogInsert reads RedoRecPtr and * fullPageWrites after grabbing a slot, so by holding all the slots * simultaneously, you can ensure that all subsequent inserts see the new * value. Those fields change very seldom, so we prefer to be fast and * non-contended when they need to be read, and slow when they're changed. Does that explain it well enough? XLogInsert holds onto a slot while it rechecks for full page writes. I am a bit worried about that logic. We're basically reverting to the old logic whe xlog writing is an exlusive affair. We will have to wait for all the other queued inserters before we're finished. I am afraid that that will show up latencywise. I have two ideas to improve on that: a) Queue the backend that does WALInsertSlotAcquire(true) at the front of the exclusive waiters in *AcquireOne. That should be fairly easy. b) Get rid of WALInsertSlotAcquire(true) by not relying on blocking all slot acquiration. I think with some trickery we can do that safely: In CreateCheckpoint() we first acquire the insertpos_lck and read CurrBytePos as a recptr. Set some shared memory variable, say, PseudoRedoRecPtr, that's now used to check whether backup blocks need to be made. Release insertpos_lck. Then acquire each slot once, but without holding the other slots. That guarantees that all XLogInsert()ing backends henceforth see our PseudoRedoRecPtr value. Then just proceed in CreateCheckpoint() as we're currently doing except computing RedoRecPtr under a spinlock. If a backend reads PseudoRedoRecPtr before we've set RedoRecPtr accordingly, all that happens is that we possibly have written a FPI too early. Makes sense? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] MVCC catalog access
On 2013-06-28 23:14:23 -0400, Robert Haas wrote: Here's a further update of this patch. In this version, I added some mechanism to send a new kind of sinval message that is sent when a catalog without catcaches is updated; it doesn't apply to all catalogs, just to whichever ones we want to have this treatment. That means we don't need to retake snapshots for those catalogs on every access, so backend startup requires just one extra MVCC snapshot as compared with current master. Assorted cleanup has been done, along with the removal of a few more SnapshotNow references. This is really cool stuff. It's still possible to construct test cases that perform badly by pounding the server with 1000 clients running Andres's readonly-busy.sql. Consider the following test case: use a DO block to create a schema with 10,000 functions in it and then DROP .. CASCADE. When the server is unloaded, the extra MVCC overhead is pretty small. Well, now the create is 52% slower and the drop is a whopping 4.7x slower. It's worth digging into the reasons just a bit. I was able to speed up this case quite a bit - it was 30x slower a few hours ago - by adding a few new relations to the switch in RelationInvalidatesSnapshotsOnly(). But the code still takes one MVCC snapshot per object dropped, because deleteOneObject() calls CommandCounterIncrement() and that, as it must, invalidates our previous snapshot. I have to say, if the thing that primarily suffers is pretty extreme DDL in extreme situations I am not really worried. Anybody running anything close to the territory of such concurrency won't perform that much DDL. We could, if we were inclined to spend the effort, probably work out that although we need to change curcid, the rest of the snapshot is still OK, but I'm not too convinced that it's worth adding an even-more-complicated mechanism for this. We could probably also optimize the delete code to increment the command counter fewer times, but I'm not convinced that's worth doing either. I am pretty convinced we shouldn't do either for now. Something picked up when quickly scanning over the last version of the patch: +/* + * Staleness detection for CatalogSnapshot. + */ +static bool CatalogSnapshotStale = true; /* * These are updated by GetSnapshotData. We initialize them this way @@ -177,6 +188,9 @@ GetTransactionSnapshot(void) else CurrentSnapshot = GetSnapshotData(CurrentSnapshotData); + /* Don't allow catalog snapshot to be older than xact snapshot. */ + CatalogSnapshotStale = true; + FirstSnapshotSet = true; return CurrentSnapshot; } @@ -184,6 +198,9 @@ GetTransactionSnapshot(void) if (IsolationUsesXactSnapshot()) return CurrentSnapshot; + /* Don't allow catalog snapshot to be older than xact snapshot. */ + CatalogSnapshotStale = true; + CurrentSnapshot = GetSnapshotData(CurrentSnapshotData); return CurrentSnapshot; @@ -207,6 +224,54 @@ GetLatestSnapshot(void) } Do we really need to invalidate snapshots in either situation? Isn't it implied, that if it's still valid, according to a) no invalidation via local invalidation messages b) no invalidations from other backends, there shouldn't be any possible differences when you only look at the catalog? And if it needs to change, we could copy the newly generated snapshot to the catalog snapshot if it's currently valid. 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] Documentation/help for materialized and recursive views
On 6/28/13 2:27 PM, David Fetter wrote: You can run \! man from within psql, And if you're on Windows, you're Sadly Out of Luck with that. Is there an equivalent we could #ifdef in for that platform? If you are using psql on Windows extensively, you probably have one of mingw, cygwin, or pgadmin handy, all of which can get you to the documentation. I don't think it's worth devising a mechanism for those not covered by this. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_ctl promote exit status
On 6/28/13 10:50 PM, Bruce Momjian wrote: On Mon, Jan 28, 2013 at 09:46:32AM -0500, Peter Eisentraut wrote: On 1/26/13 4:44 PM, Aaron W. Swenson wrote: You are right. Had I read a little further down, it seems that the exit status should actually be 7. 7 is OK for not running, but what should we use when the server is not in standby mode? Using the idempotent argument that we are discussing for the stop action, promoting a server that is not a standby should be a noop and exit successfully. Not sure if that is what we want, though. I looked at all the LSB return codes listed here and mapped them to pg_ctl error situations: https://refspecs.linuxbase.org/LSB_3.1.0/LSB-Core-generic/LSB-Core-generic/iniscrptact.html Patch attached. I did not touch the start/stop return codes. Approximately none of these changes seem correct to me. For example, why is failing to open the PID file 6, or failing to start the server 7? -- 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] New regression test time
On Sat, Jun 29, 2013 at 02:59:35PM -0700, Josh Berkus wrote: On 06/29/2013 02:14 PM, Andrew Dunstan wrote: AIUI: They do test feature use and errors that have cropped up in the past that we need to beware of. They don't test every bug we've ever had, nor do they exercise every piece of code. If we don't have a test for it, then we can break it in the future and not know we've broken it until .0 is released. Is that really a direction we're happy going in? Maybe there is a good case for these last two in a different set of tests. If we had a different set of tests, that would be a valid argument. But we don't, so it's not. And nobody has offered to write a feature to split our tests either. With utmost respect, this just isn't true. There is a make coverage target that probably doesn't get enough exercise, but it's just the kind of infrastructure you're describing. I have to say, I'm really surprised at the level of resistance people on this list are showing to the idea of increasing test coverage. I thought that Postgres was all about reliability? For a project as mature as we are, our test coverage is abysmal, and I think I'm starting to see why. Burdening hackers with extra time in ordinary compile cycles is the wrong direction. If anything, we should probably look at what tests only routinely get run by our CI system--currently the buildfarm--and which ones developers could reasonably be expected to wait until post-push to run in day-to-day development. Cheers, David. -- David Fetter da...@fetter.org 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
Re: [HACKERS] LDAP: bugfix and deprecated OpenLDAP API
Magnus Hagander wrote: On Tue, Feb 5, 2013 at 10:39 AM, Albe Laurenz laurenz.a...@wien.gv.at wrote: I found a small bug in the implementation of LDAP connection parameter lookup. [...] As coded now, the timeout won't work - if the LDAP server is down, ldap_simple_bind will wait for the network timeout, which will be quite longer than 2 seconds. The attached patch ldap-bug.patch fixes this problem; unfortunately I found no way that works both with OpenLDAP and Windows LDAP, so I had to add an #ifdef. I think that this patch should be applied and backpatched. So just to be clear - the difference is we're going from implicit anonymous bind, to an explicit one? We're not actually causing an extra bind compared to previous versions? No, it was an explicit bind before as well. The bug I discovered is that if you ldap_simple_bind, the timeout set in ldap_result does not work for network timeouts. It always waits until the TCP connection is established or fails, which can take much longer than PGLDAP_TIMEOUT seconds. With OpenLDAP you can set the LDAP_OPT_NETWORK_TIMEOUT option to enforce a timeout, but on Windows you have to use the nonstandard function ldap_connect. I also tried to fix the problem mentioned in http://www.postgresql.org/message-id/CA+TgmoYnj=Es3L_0Q8+ijR4tVhvztW1fb=7c9k9gemzwqhp...@mail.gmail.com that we use deprecated OpenLDAP functions, see the attached ldap-undeprecate.patch. I added a file ldap.c in src/port with my own implementation of some of the functions that OpenLDAP has deprecated. With that, the code changes necessary are pretty minimal. Doesn't this need a version check against OpenSSL at some point, or a configure check? Are we just assuming that all versions that people ever use have the function deprecated? (That's probably not entirely unreasonable, just double checking) I checked, and it should work fine since OpenLDAP 2.0.0, released in 2000. The current version is 2.4.35. In 2.0.0 the old API was not yet deprecated, but the new functions are already there so that PostgreSQL should work. I don't know if that would require a check, but it should be simple to add a configure test if we are sufficiently paranoid. I'll be on vacation from Wednesday on until July 20th. Yours, Laurenz Albe -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] LDAP: bugfix and deprecated OpenLDAP API
Peter Eisentraut wrote: Btw., I just checked the source code of Apache, PHP, and PAM, and they are all unconditionally building with LDAP_DEPRECATED. So maybe there is no hurry about this. I don't think that the old API functions will go away until there is a new standard for the LDAP C API, but I have no inside knowledge of OpenLDAP. Yours, Laurenz Albe -- 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] Documentation/help for materialized and recursive views
On Mon, Jul 01, 2013 at 10:05:24AM -0400, Peter Eisentraut wrote: On 6/28/13 2:27 PM, David Fetter wrote: You can run \! man from within psql, And if you're on Windows, you're Sadly Out of Luck with that. Is there an equivalent we could #ifdef in for that platform? If you are using psql on Windows extensively, you probably have one of mingw, cygwin, or pgadmin handy, all of which can get you to the documentation. I don't think it's worth devising a mechanism for those not covered by this. With deepest respect, failing to provide documentation to users on our widest-deployed platform seems pretty hostile to me. There was an earlier suggestion that we provide URLs, which seems like a decent way forward as those environments so locked down as to disallow outbound HTTP are pretty rare, and non-networked computers are even more rare. Cheers, David. -- David Fetter da...@fetter.org 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
Re: [HACKERS] [PATCH] big test separation POC
Hi Fabien, On Mon, Jul 1, 2013 at 10:42 AM, Fabien COELHO coe...@cri.ensmp.fr wrote: - I do not understand why the makefile specifies $(srcdir) before local files in some places. For VPATH builds :-) Here is a v2 which is more likely to work under VPATH. I really appreciate your efforts. I am reviewing your patch. While testing patch, I found that make installcheck breaks with your patch and gives following error: == running regression test queries== pg_regress: could not open file ./serial_schedule for reading: No such file or directory looks like you forgot to add entry for serial_schedule. Following sequence of commands will reproduces this error: 1) ./configure 2) make 3) make install 4) initialize the database cluster (initdb) 5) start the server 6) make installcheck I will post more review comments if there are any. -- Regards, Samrat Revgade
Re: [HACKERS] New regression test time
On 2013-07-01 07:14:23 -0700, David Fetter wrote: If we had a different set of tests, that would be a valid argument. But we don't, so it's not. And nobody has offered to write a feature to split our tests either. With utmost respect, this just isn't true. There is a make coverage target that probably doesn't get enough exercise, but it's just the kind of infrastructure you're describing. Uh? Isn't make coverage a target for collecting the generated coverage data? Afaik it itself does *NOT* depend on any checks being run. And it only does something sensible if --enable-coverage is passed to ./configure anyway. 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] Documentation/help for materialized and recursive views
On 7/1/13 10:20 AM, David Fetter wrote: On Mon, Jul 01, 2013 at 10:05:24AM -0400, Peter Eisentraut wrote: On 6/28/13 2:27 PM, David Fetter wrote: You can run \! man from within psql, And if you're on Windows, you're Sadly Out of Luck with that. Is there an equivalent we could #ifdef in for that platform? If you are using psql on Windows extensively, you probably have one of mingw, cygwin, or pgadmin handy, all of which can get you to the documentation. I don't think it's worth devising a mechanism for those not covered by this. With deepest respect, failing to provide documentation to users on our widest-deployed platform seems pretty hostile to me. As I argue above, I don't think this is the widest-deployed platform. The actual platform in use is either mingw, which has man, or click-and-drool, which has pgadmin, both of which provide the documentation. There was an earlier suggestion that we provide URLs, which seems like a decent way forward as those environments so locked down as to disallow outbound HTTP are pretty rare, and non-networked computers are even more rare. Does clicking on links in cmd.exe do anything useful? -- 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] checking variadic any argument in parser - should be array
Pavel Stehule escribió: Hello updated patch - precious Assert, more comments Pavel, can you please remove quoted text from messages you reply to? This message has 10kb of completely useless text. Thanks, -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] checking variadic any argument in parser - should be array
2013/6/29 Pavel Stehule pavel.steh...@gmail.com: Hello updated patch - precious Assert, more comments Regards Pavel stripped variadic_any_parser_check-3.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] request a new feature in fuzzystrmatch
Joe Conway escribió: Actually, given that this change will create version 1.1 of the extension, I believe the 1.0 versions of the sql scripts should probably be removed entirely. Can someone with more knowledge of the extension facility comment on that? Besides what Michael said, another thing is that you need to ensure that if the functions are run with the 1.0 definitions, they don't crash (i.e. you need to check the number of arguments actually passed to function(s), and ensure no changes are made to the types of previously existing arguments). You can test this by installing the 1.0 version in a 9.3 database, then pg_upgrade, and test the functions without running the ALTER EXTENSION UPGRADE. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Shorter iterations of join_info_list
As far as I understand, deconstruct_recurse() ensures that SpecialJoinInfo of a new join always gets added to higher position in join_info_list than SJ infos of all joins located below the new join in the tree. I wonder if we can rely on that fact sometimes. One possible use case could be placeholder.c:update_placeholder_eval_levels(): 1. The first (in terms of position in join_info_list) join above phinfo-ph_var-phrels can be marked as the (exclusive) upper bound for all iterations. 2. The first join for which particular iteration of SJ infos identifies the necessity to extend eval_at can be marked in join_info_list as (exclusive) lower bound for the next iteration. This is because that addition can only affect parents of the join whose relations we just added to eval_at. And these essentially can't be located at lower positions in join_info_list. The lower limit could also be used in initsplan.c:check_outerjoin_delay(). Is this worth a patch? It's not much coding but I'd appreciate some feedback before I try to do anything. Thanks, Antonin Houska (Tony) -- 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] Documentation/help for materialized and recursive views
On Mon, Jul 01, 2013 at 10:52:55AM -0400, Peter Eisentraut wrote: On 7/1/13 10:20 AM, David Fetter wrote: On Mon, Jul 01, 2013 at 10:05:24AM -0400, Peter Eisentraut wrote: On 6/28/13 2:27 PM, David Fetter wrote: You can run \! man from within psql, And if you're on Windows, you're Sadly Out of Luck with that. Is there an equivalent we could #ifdef in for that platform? If you are using psql on Windows extensively, you probably have one of mingw, cygwin, or pgadmin handy, all of which can get you to the documentation. I don't think it's worth devising a mechanism for those not covered by this. With deepest respect, failing to provide documentation to users on our widest-deployed platform seems pretty hostile to me. As I argue above, I don't think this is the widest-deployed platform. The actual platform in use is either mingw, which has man, or click-and-drool, which has pgadmin, both of which provide the documentation. I'm not going to get into a big definitional wrangle here. Has available (as in you could install software if you wanted to) is a pretty long distance from actually handy, which URLs are a bit closer to. There was an earlier suggestion that we provide URLs, which seems like a decent way forward as those environments so locked down as to disallow outbound HTTP are pretty rare, and non-networked computers are even more rare. Does clicking on links in cmd.exe do anything useful? Apparently start URL works for some large class of URLs. More details here: http://www.dwheeler.com/essays/open-files-urls.html Cheers, David. -- David Fetter da...@fetter.org 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
Re: [HACKERS] fallocate / posix_fallocate for new WAL file creation (etc...)
On Sun, Jun 30, 2013 at 11:52 PM, Greg Smith g...@2ndquadrant.com wrote: On 6/30/13 9:28 PM, Jon Nelson wrote: The performance of the latter (new) test sometimes seems to perform worse and sometimes seems to perform better (usually worse) than either of the other two. In all cases, posix_fallocate performs better, but I don't have a sufficiently old kernel to test with. This updated test program looks reliable now. The numbers are very tight when I'd expect them to be, and there's nowhere with the huge differences I saw in the earlier test program. Here's results from a few sets of popular older platforms: If you found yourself with a spare moment, could you run these again with the number of open/close cycles set high (say, 100) and the number of rewrites set to 0 and also to 1? Most of the time spent is actually spent overwriting the files so by reducing or eliminating that aspect it might be easier to get a handle on the actual performance difference. -- Jon -- 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] Passing fdw_private data from PlanForeignScan to PlanForeignModify
--On 13. Juni 2013 18:12:05 -0400 Tom Lane t...@sss.pgh.pa.us wrote: What i tried before was to access (in PlanForeignModify) the RelOptInfo structure through PlannerInfo-simple_rel_array, assuming the the resultRelation index points to the right array member. However, that didn't work, the fdw_private List is not the one filled by GetForeignPlan...is there another way to get back the RelOptInfo worked on earlier? It should work ... *if* there was in fact a RelOptInfo worked on earlier. There sometimes isn't. You might need to do something like what make_modifytable() has to do to call you in the first place: Sorry for the late feedback, didn't manage to get back to this earlier. This works indeed, the RelOptInfo structure stored in simple_rel_array (if present) allows a FDW to access its earlier scan state, assigned in GetForeignPlan() for example. Thanks for the hints! -- Bernd -- 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] Randomisation for ensuring nlogn complexity in quicksort
My PostgresSQL (9.2) is crashing after certain load tests. Currently, postgressql is crashing when simulatenously 800 to 1000 threads are run on a 10 million records schema. Not sure, if we have to tweak some more parameters of postgres. - jasmine -- View this message in context: http://postgresql.1045698.n5.nabble.com/Randomisation-for-ensuring-nlogn-complexity-in-quicksort-tp5761907p5762011.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.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: FILTER for aggregates [was Re: [HACKERS] Department of Redundancy Department: makeNode(FuncCall) division]
On 1 July 2013 01:44, David Fetter da...@fetter.org wrote: On Fri, Jun 28, 2013 at 09:22:52PM +0100, Dean Rasheed wrote: On 21 June 2013 06:16, David Fetter da...@fetter.org wrote: Please find attached a patch which allows subqueries in the FILTER clause and adds regression testing for same. This needs re-basing/merging following Robert's recent commit to make OVER unreserved. Please find attached. Thanks, Andrew Gierth! In this one, FILTER is no longer a reserved word. Looking at this patch again, it appears to be in pretty good shape. - Applies cleanly to head. - Compiles with no warnings. - Includes regression test cases and doc updates. - Compatible with the relevant part of T612, Advanced OLAP operations. - Includes pg_dump support. - Code changes all look reasonable, and I can't find any corner cases that have been missed. - Appears to work as expected. I tested everything I could think of and couldn't break it. AFAICT all the bases have been covered. As mentioned upthread, I would have preferred a few more regression test cases, and a couple of the tests don't appear to return anything interesting, but I'll leave that for the committer to decide whether they're sufficient for regression tests. I have a few suggestions to improve the docs: 1). In syntax.sgml: The aggregate_name can also be suffixed with FILTER as described below. It's not really a suffix to the aggregate name, since it follows the function arguments and optional order by clause. Perhaps it would be more consistent with the surrounding text to say something like replaceableexpression/replaceable is any value expression that does not itself contain an aggregate expression or a window function call, and !replaceableorder_by_clause/replaceable and !replaceablefilter_clause/replaceable are optional !literalORDER BY/ and literalFILTER/ clauses as described below. 2). In syntax.sgml: ... or when a FILTER clause is present, each row matching same. In the context of that paragraph this suggests that the filter clause only applies to the first form, since that paragraph is a description of the 4 forms of the aggregate function. I don't think it's worth mentioning FILTER in this paragraph at all --- it's adequately described below that. 3). In syntax.sgml: Adding a FILTER clause to an aggregate specifies which values of the expression being aggregated to evaluate. How about something a little more specific, along the lines of If literalFILTER/ is specified, then only input rows for which the replaceablefilter_clause/replaceable evaluates to true are fed to the aggregate function; input rows for which the replaceablefilter_clause/replaceable evaluates to false or the null value are discarded. For example... 4). In select.sgml: In the absence of a FILTER clause, aggregate functions It doesn't seem right to refer to the FILTER clause at the top level here because it's not part of the SELECT syntax being described on this page. Also I think this should include a cross-reference to the aggregate function syntax section, perhaps something like: Aggregate functions, if any are used, are computed across all rows making up each group, producing a separate value for each group (whereas without literalGROUP BY/literal, an aggregate produces a single value computed across all the selected rows). +The set of rows fed to the aggregate function can be further filtered +by attaching a literalFILTER/literal clause to the aggregate +function call, see xref ... for more information. When literalGROUP BY/literal is present, it is not valid for the commandSELECT/command list expressions to refer to ungrouped columns except within aggregate functions or if the ungrouped column is functionally dependent on the grouped columns, since there would otherwise be more than one possible value to return for an ungrouped column. A functional dependency exists if the grouped columns (or a subset thereof) are the primary key of the table containing the ungrouped column. Regards, Dean -- 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] Randomisation for ensuring nlogn complexity in quicksort
On Mon, Jul 01, 2013 at 04:42:04AM -0700, jasmine wrote: My PostgresSQL (9.2) is crashing after certain load tests. Currently, postgressql is crashing when simulatenously 800 to 1000 threads are run on a 10 million records schema. Not sure, if we have to tweak some more parameters of postgres. Jasmine, Please start your own thread rather than replying to some unrelated thread. In the particular case above, please also to provide reproducible test cases including the exact version(s) of PostgreSQL to which they apply (9.2.4, e.g.) along with output from same so other people can see what you're talking about and actually help. Cheers, David. -- David Fetter da...@fetter.org 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
Re: [HACKERS] fallocate / posix_fallocate for new WAL file creation (etc...)
On Sun, 2013-06-30 at 16:09 -0400, Andrew Dunstan wrote: It was originally generated. Since then it's been maintained by hand. What is the procedure for maintaining it by hand? Why are HAVE_POSIX_SIGNALS and HAVE_SYNC_FILE_RANGE in there (though commented out), but not HAVE_POSIX_FADVISE? Regards, Jeff Davis -- 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] in-catalog Extension Scripts and Control parameters (templates?)
Very minor comment here: these SGML id tags: + refentry id=SQL-ALTEREXTENSIONTEMPLATE are pretty important, because they become the URL for the specific page in the reference docs. So I think you should fix them to be the correct spelling of the command alter template for extension, and also perhaps add an hyphen or two. Maybe SQL-ALTER-EXTENSION-FOR-TEMPLATE. (We're inconsistent about adding hyphens; most URLs don't have hyphens after then sql- bit, so sql-altertablespace, but we have some examples of the opposite such as sql-commit-prepared and sql-drop-owned.) -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_ctl and -h/help
On Sun, Jun 30, 2013 at 02:29:20PM +0900, Michael Paquier wrote: On Sat, Jun 29, 2013 at 10:45 PM, Bruce Momjian br...@momjian.us wrote: In studying pg_upgrade's handling of --help, I noticed that pg_ctl supports -h for help, but it is the only tool to do so, and -h is not documented. I propose we remove -h for help in pg_ctl, and have it support only -? and --help. I suppose that it doesn't hurt to have it, but for yes the sake of consistency with the other binaries it would make sense to remove it. Btw, not even the docs, it is also not listed in the --help message findable in code. Agreed --- attached patch applied. I also noticed that we sometimes test for -? then --help, but other times do things in the opposite order, and the same for -V/--version, so I made that consistent. However, I also noticed that while we document -? before --help, we test for --help before -?, and the same for -V/--version. Should I make those even more consistent by always testing for the single-letter option first? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + diff --git a/contrib/pg_test_fsync/pg_test_fsync.c b/contrib/pg_test_fsync/pg_test_fsync.c new file mode 100644 index b978d9e..53f600f *** a/contrib/pg_test_fsync/pg_test_fsync.c --- b/contrib/pg_test_fsync/pg_test_fsync.c *** handle_args(int argc, char *argv[]) *** 146,153 if (argc 1) { ! if (strcmp(argv[1], --help) == 0 || strcmp(argv[1], -h) == 0 || ! strcmp(argv[1], -?) == 0) { printf(Usage: %s [-f FILENAME] [-s SECS-PER-TEST]\n, progname); exit(0); --- 146,152 if (argc 1) { ! if (strcmp(argv[1], --help) == 0 || strcmp(argv[1], -?) == 0) { printf(Usage: %s [-f FILENAME] [-s SECS-PER-TEST]\n, progname); exit(0); diff --git a/contrib/pg_test_timing/pg_test_timing.c b/contrib/pg_test_timing/pg_test_timing.c new file mode 100644 index 0bf9127..e44c535 *** a/contrib/pg_test_timing/pg_test_timing.c --- b/contrib/pg_test_timing/pg_test_timing.c *** handle_args(int argc, char *argv[]) *** 49,56 if (argc 1) { ! if (strcmp(argv[1], --help) == 0 || strcmp(argv[1], -h) == 0 || ! strcmp(argv[1], -?) == 0) { printf(Usage: %s [-d DURATION]\n, progname); exit(0); --- 49,55 if (argc 1) { ! if (strcmp(argv[1], --help) == 0 || strcmp(argv[1], -?) == 0) { printf(Usage: %s [-d DURATION]\n, progname); exit(0); diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c new file mode 100644 index 9045e00..9e909ae *** a/src/bin/pg_ctl/pg_ctl.c --- b/src/bin/pg_ctl/pg_ctl.c *** main(int argc, char **argv) *** 2002,2014 /* support --help and --version even if invoked as root */ if (argc 1) { ! if (strcmp(argv[1], -h) == 0 || strcmp(argv[1], --help) == 0 || ! strcmp(argv[1], -?) == 0) { do_help(); exit(0); } ! else if (strcmp(argv[1], -V) == 0 || strcmp(argv[1], --version) == 0) { puts(pg_ctl (PostgreSQL) PG_VERSION); exit(0); --- 2002,2013 /* support --help and --version even if invoked as root */ if (argc 1) { ! if (strcmp(argv[1], --help) == 0 || strcmp(argv[1], -?) == 0) { do_help(); exit(0); } ! else if (strcmp(argv[1], --version) == 0 || strcmp(argv[1], -V) == 0) { puts(pg_ctl (PostgreSQL) PG_VERSION); exit(0); diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c new file mode 100644 index 1c9f7a5..b2264c9 *** a/src/bin/psql/startup.c --- b/src/bin/psql/startup.c *** parse_psql_options(int argc, char *argv[ *** 558,564 break; case '?': /* Actual help option given */ ! if (strcmp(argv[optind - 1], -?) == 0 || strcmp(argv[optind - 1], --help) == 0) { usage(); exit(EXIT_SUCCESS); --- 558,564 break; case '?': /* Actual help option given */ ! if (strcmp(argv[optind - 1], --help) == 0 || strcmp(argv[optind - 1], -?) == 0) { usage(); exit(EXIT_SUCCESS); -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_ctl promote exit status
On Mon, Jul 1, 2013 at 10:11:23AM -0400, Peter Eisentraut wrote: On 6/28/13 10:50 PM, Bruce Momjian wrote: On Mon, Jan 28, 2013 at 09:46:32AM -0500, Peter Eisentraut wrote: On 1/26/13 4:44 PM, Aaron W. Swenson wrote: You are right. Had I read a little further down, it seems that the exit status should actually be 7. 7 is OK for not running, but what should we use when the server is not in standby mode? Using the idempotent argument that we are discussing for the stop action, promoting a server that is not a standby should be a noop and exit successfully. Not sure if that is what we want, though. I looked at all the LSB return codes listed here and mapped them to pg_ctl error situations: https://refspecs.linuxbase.org/LSB_3.1.0/LSB-Core-generic/LSB-Core-generic/iniscrptact.html Patch attached. I did not touch the start/stop return codes. Approximately none of these changes seem correct to me. For example, why is failing to open the PID file 6, or failing to start the server 7? Well, according to that URL, we have: 6 program is not configured 7 program is not running I just updated the pg_ctl.c comments to at least point to a valid URL for this. I think we can just call this item closed because I am still unclear if these return codes should be returned by pg_ctl or the start/stop script. Anyway, while I do think pg_ctl could pass a little more information back about failure via its return code, I am unclear if LSB is the right approach. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] Optimizing pglz compressor
On Mon, Jul 1, 2013 at 11:05:37AM +0300, Heikki Linnakangas wrote: On 26.06.2013 16:37, Amit Kapila wrote: On Wednesday, June 26, 2013 2:15 AM Heikki Linnakangas wrote: Can you also try the attached patch, please? It's the same as before, but in this version, I didn't replace the prev and next pointers in PGLZ_HistEntry struct with int16s. That avoids some table lookups, at the expense of using more memory. It's closer to what we have without the patch, so maybe that helps on your system. Yes it helped a lot on my system. Ok, good. Strange, I did not expect such a big difference. There was minor problem in you patch, in one of experiments it crashed. Fix is not to access 0th history entry in function pglz_find_match(), modified patch is attached. Thanks, good catch! I thought that a pointer to the 0th entry would never make it into the prev/next fields, but it does. In fact, we never store a NULL there anymore, a pointer to the 0th entry is now always used to mean 'invalid'. I adjusted the patch to remove the NULL check, and only check for the 0th entry. Committed. Does someone have new tests comparing this patch with the new compression methods being considered? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] fallocate / posix_fallocate for new WAL file creation (etc...)
On Sun, 2013-06-30 at 18:55 -0400, Greg Smith wrote: This makes platform level testing a lot easier, thanks. Attached is an updated copy of that program with some error checking. If the files it creates already existed, the code didn't notice, and a series of write errors happened. If you set the test up right it's not a problem, but it's better if a bad setup is caught. I wrapped the whole test with a shell script, also attached, which insures the right test sequence and checks. Thank you. That's glibc helpfully converting your call to posix_fallocate into small writes, because the OS doesn't provide a better way in that kernel. It's not hard to imagine this being slower than what the WAL code is doing right now. I'm not worried about correctness issues anymore, but my gut paranoia about this not working as expected on older systems was justified. Everyone who thought I was just whining owes me a cookie. So your theory is that it may be slower because there are twice as many syscalls (one per 4K page rather than one per 8K page)? Interesting observation. This is what I plan to benchmark specifically next. In the interest of keeping this patch moving forward, do you have an estimate for when this testing will be complete? If the posix_fallocate approach is actually slower than what's done now when it's not getting kernel acceleration, which is the case on RHEL5 era kernels, we might need to make the configure time test more complicated. Whether posix_fallocate is defined isn't sensitive enough; on Linux it may be the case that this only is usable when fallocate() is also there. I'd say that if posix_fallocate is slower than the existing code on pretty much any platform, we shouldn't commit the patch at all. I would be quite surprised if that was the case, however. Regards, Jeff Davis -- 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] in-catalog Extension Scripts and Control parameters (templates?)
I think this is unlikely to work reliably: + PG_TRY(); + { + ExtensionControl *control = read_extension_control_file(extname); + + if (control) + { + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_OBJECT), +errmsg(extension \%s\ is already available, extname))); + } + } + PG_CATCH(); + { + /* no control file found is good news for us */ + } + PG_END_TRY(); What if read_extension_control_file() fails because of an out-of-memory error? I think you need to extend that function to have a more useful API, not rely on it raising a specific error. There is at least one more case when you're calling that function in the same way. It'd probably work to have a boolean return (found/not found), and return the ExtensionControl structure through a pointer. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Randomisation for ensuring nlogn complexity in quicksort
On Mon, Jul 1, 2013 at 10:02 PM, David Fetter da...@fetter.org wrote: On Mon, Jul 01, 2013 at 04:42:04AM -0700, jasmine wrote: My PostgresSQL (9.2) is crashing after certain load tests. Currently, postgressql is crashing when simulatenously 800 to 1000 threads are run on a 10 million records schema. Not sure, if we have to tweak some more parameters of postgres. Jasmine, Please start your own thread rather than replying to some unrelated thread. In the particular case above, please also to provide reproducible test cases including the exact version(s) of PostgreSQL to which they apply (9.2.4, e.g.) along with output from same so other people can see what you're talking about and actually help. Not sure if it belongs to -general instead of -hackers. Regards, Atri -- Regards, Atri l'apprenant -- 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] fallocate / posix_fallocate for new WAL file creation (etc...)
On Tue, Jul 2, 2013 at 1:55 AM, Jeff Davis pg...@j-davis.com wrote: On Sun, 2013-06-30 at 18:55 -0400, Greg Smith wrote: This makes platform level testing a lot easier, thanks. Attached is an updated copy of that program with some error checking. If the files it creates already existed, the code didn't notice, and a series of write errors happened. If you set the test up right it's not a problem, but it's better if a bad setup is caught. I wrapped the whole test with a shell script, also attached, which insures the right test sequence and checks. Thank you. That's glibc helpfully converting your call to posix_fallocate into small writes, because the OS doesn't provide a better way in that kernel. It's not hard to imagine this being slower than what the WAL code is doing right now. I'm not worried about correctness issues anymore, but my gut paranoia about this not working as expected on older systems was justified. Everyone who thought I was just whining owes me a cookie. So your theory is that it may be slower because there are twice as many syscalls (one per 4K page rather than one per 8K page)? Interesting observation. This is what I plan to benchmark specifically next. In the interest of keeping this patch moving forward, do you have an estimate for when this testing will be complete? If the posix_fallocate approach is actually slower than what's done now when it's not getting kernel acceleration, which is the case on RHEL5 era kernels, we might need to make the configure time test more complicated. Whether posix_fallocate is defined isn't sensitive enough; on Linux it may be the case that this only is usable when fallocate() is also there. I'd say that if posix_fallocate is slower than the existing code on pretty much any platform, we shouldn't commit the patch at all. Even in that case, if a user can easily know which platform posix_fallocate should be used in, we can commit the patch with the configurable GUC parameter. Regards, -- Fujii Masao -- 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] Minor inheritance/check bug: Inconsistent behavior
On Sun, Jun 30, 2013 at 06:57:10AM +, Amit kapila wrote: I have done the initial analysis and prepared a patch, don't know if anything more I can do until someone can give any suggestions to further proceed on this bug. So, I guess we never figured this out. I can submit this bug-fix for next commitfest if there is no objection for doing so. What is your opinion? Yes, good idea. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] Eliminating PD_ALL_VISIBLE, take 2
On Sun, 2013-06-30 at 22:58 -0400, Robert Haas wrote: I thought that Jeff withdrew this patch. No -- was there a reason you thought that? I know it could use another round of testing before commit, and there may be a couple other things to clear up. But I don't want to invest a lot of time there right now, because, as I understand it, you still object to the patch anyway. I am still not entirely clear on the objections to this patch: 1. Contention was a concern, but I believe I have mitigated it. Strictly speaking, additional pins may be acquired, but the cost of those pin operations will be spread over a lot of other work. 2. There are quite a few different ideas about where we're going with PD_ALL_VISIBLE and freezing, but it seems like removing PD_ALL_VISIBLE is potentially compatible with most of them. Any others? The patch reduces code complexity and reduces writes during a data load. Regards, Jeff Davis -- 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] Outputting UTC offset with to_char()
Applied. I referenced macros for some of the new constants, e.g. SECS_PER_HOUR. --- On Fri, Jun 28, 2013 at 10:04:49PM -0400, Bruce Momjian wrote: On Sun, Oct 21, 2012 at 05:40:40PM -0400, Andrew Dunstan wrote: I'm not sure if this has come up before. A client was just finding difficulties because to_char() doesn't support formatting the timezone part of a timestamptz numerically (i.e. as +-hhmm) instead of using a timezone name. Is there any reason for that? Would it be something worth having? Great idea! I have developed the attached patch to do this: test= SELECT to_char(current_timestamp, 'OF'); to_char - -04 (1 row) test= SELECT to_char(current_timestamp, 'TMOF'); to_char - -04 (1 row) test= SET timezone = 'Asia/Calcutta'; SET test= SELECT to_char(current_timestamp, 'OF'); to_char - +05:30 (1 row) test= SELECT to_char(current_timestamp, 'FMOF'); to_char - +5:30 (1 row) I went with the optional colon and minutes because this is how we output it: test= SELECT current_timestamp; now --- 2013-06-28 22:02:24.773587-04 --- (1 row) test= set timezone = 'Asia/Calcutta'; SET test= SELECT current_timestamp; now -- 2013-06-29 07:32:29.157565+05:30 -- (1 row) -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml new file mode 100644 index 7c009d8..5765ddf *** a/doc/src/sgml/func.sgml --- b/doc/src/sgml/func.sgml *** SELECT SUBSTRING('XY1234Z', 'Y*?([0-9]{1 *** 5645,5650 --- 5645,5654 entryliteraltz/literal/entry entrylower case time-zone name/entry /row +row + entryliteralOF/literal/entry + entrytime-zone offset/entry +/row /tbody /tgroup /table diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c new file mode 100644 index 7b85406..4c272ef *** a/src/backend/utils/adt/formatting.c --- b/src/backend/utils/adt/formatting.c *** typedef enum *** 600,605 --- 600,606 DCH_MS, DCH_Month, DCH_Mon, + DCH_OF, DCH_P_M, DCH_PM, DCH_Q, *** static const KeyWord DCH_keywords[] = { *** 746,751 --- 747,753 {MS, 2, DCH_MS, TRUE, FROM_CHAR_DATE_NONE}, {Month, 5, DCH_Month, FALSE, FROM_CHAR_DATE_GREGORIAN}, {Mon, 3, DCH_Mon, FALSE, FROM_CHAR_DATE_GREGORIAN}, + {OF, 2, DCH_OF, FALSE, FROM_CHAR_DATE_NONE}, /* O */ {P.M., 4, DCH_P_M, FALSE, FROM_CHAR_DATE_NONE}, /* P */ {PM, 2, DCH_PM, FALSE, FROM_CHAR_DATE_NONE}, {Q, 1, DCH_Q, TRUE, FROM_CHAR_DATE_NONE}, /* Q */ *** static const int DCH_index[KeyWord_INDEX *** 874,880 -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, DCH_A_D, DCH_B_C, DCH_CC, DCH_DAY, -1, ! DCH_FX, -1, DCH_HH24, DCH_IDDD, DCH_J, -1, -1, DCH_MI, -1, -1, DCH_P_M, DCH_Q, DCH_RM, DCH_, DCH_TZ, DCH_US, -1, DCH_WW, -1, DCH_Y_YYY, -1, -1, -1, -1, -1, -1, -1, DCH_a_d, DCH_b_c, DCH_cc, DCH_day, -1, DCH_fx, -1, DCH_hh24, DCH_iddd, DCH_j, -1, -1, DCH_mi, --- 876,882 -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, DCH_A_D, DCH_B_C, DCH_CC, DCH_DAY, -1, ! DCH_FX, -1, DCH_HH24, DCH_IDDD, DCH_J, -1, -1, DCH_MI, -1, DCH_OF, DCH_P_M, DCH_Q, DCH_RM, DCH_, DCH_TZ, DCH_US, -1, DCH_WW, -1, DCH_Y_YYY, -1, -1, -1, -1, -1, -1, -1, DCH_a_d, DCH_b_c, DCH_cc, DCH_day, -1, DCH_fx, -1, DCH_hh24, DCH_iddd, DCH_j, -1, -1, DCH_mi, *** DCH_to_char(FormatNode *node, bool is_in *** 2502,2507 --- 2504,2519 s += strlen(s); } break; + case DCH_OF: + INVALID_FOR_INTERVAL; + sprintf(s, %+0*ld, S_FM(n-suffix) ? 0 : 3, tm-tm_gmtoff / 3600); + s += strlen(s); + if (tm-tm_gmtoff % 3600 != 0) + { + sprintf(s, :%02ld, (tm-tm_gmtoff %
Re: [HACKERS] changeset generation v5-01 - Patches git tree
Since this discussion seems to have stalled, let me do a quick summary. The goal of this subset of patches is to allow retroactive look up of relations starting from a WAL record. Currently, the WAL record only tracks the relfilenode that it affects, so there are two possibilities: 1. we add some way to find out the relation OID from the relfilenode, 2. we augment the WAL record with the relation OID. Each solution has its drawbacks. For the former, * we need a new cache * we need a new pg_class index * looking up the relation OID still requires some CPU runtime and memory to keep the caches in; run invalidations, etc. For the latter, * each WAL record would become somewhat bigger. For WAL records with a payload of 25 bytes (say insert a tuple which is 25 bytes long) this means about 7% overhead. There are some other issues, but these can be solved. For instance Tom doesn't want a syscache on top of a non-unique index, and I agree on that. But if we agree on this way forward, we can just go a different route by keeping a separate cache layer. So the question is, do we take the overhead of the new index (which means overhead on DML operations -- supposedly rare) or do we take the overhead of larger WAL records (which means overhead on all DDL operations)? Note we can make either thing apply to only people running logical replication. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] changeset generation v5-01 - Patches git tree
Alvaro Herrera alvhe...@2ndquadrant.com writes: So the question is, do we take the overhead of the new index (which means overhead on DML operations -- supposedly rare) or do we take the overhead of larger WAL records (which means overhead on all DDL operations)? Note we can make either thing apply to only people running logical replication. I don't believe you can have or not have an index on pg_class as easily as all that. The choice would have to be frozen at initdb time, so people would have to pay the overhead if they thought there was even a small possibility that they'd want logical replication later. Flipping the content of WAL records might not be a terribly simple thing to do either, but at least in principle it could be done during a postmaster restart, without initdb. 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] fallocate / posix_fallocate for new WAL file creation (etc...)
On Tue, 2013-07-02 at 02:13 +0900, Fujii Masao wrote: Even in that case, if a user can easily know which platform posix_fallocate should be used in, we can commit the patch with the configurable GUC parameter. I disagree here. We're not talking about a huge win; this speedup may not even be detectable on a running system. I think Robert summarized the reason for the patch best: I mean, if posix_fallocate() is faster, then it's just faster, right?. But if we need a new GUC, and DBAs now have one more thing they need to test about their platform, then that argument goes out the window. Regards, Jeff Davis -- 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] Move unused buffers to freelist
On Sun, Jun 30, 2013 at 3:24 AM, Amit kapila amit.kap...@huawei.com wrote: Do you think it will be sufficient to just wake bgwriter when the buffers in freelist drops below low watermark, how about it's current job of flushing dirty buffers? Well, the only point of flushing dirty buffers in the background writer is to make sure that backends can allocate buffers quickly. If there are clean buffers already in the freelist, that's not a concern. So... I mean to ask that if for some scenario where there are sufficient buffers in freelist, but most other buffers are dirty, will delaying flush untill number of buffers fall below low watermark is okay. ...I think this is OK, or at least we should assume it's OK until we have evidence that it isn't. -- 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] review: Non-recursive processing of AND/OR lists
On Sun, Jun 30, 2013 at 1:08 PM, Pavel Stehule pavel.steh...@gmail.com wrote: 2013/6/30 Gurjeet Singh gurj...@singh.im: On Sun, Jun 30, 2013 at 11:46 AM, Pavel Stehule pavel.steh...@gmail.com wrote: 2013/6/30 Gurjeet Singh gurj...@singh.im: On Sun, Jun 30, 2013 at 11:13 AM, Pavel Stehule pavel.steh...@gmail.com wrote: How about naming those 3 variables as follows: root_expr_kind root_expr_name root_bool_expr_type +1 Thanks. Attached is the patch with that change. I'll update the commitfest entry with a link to this email. ok I chechecked it - patched without warnings, all tests passed It is ready for commit I think it's a waste of code to try to handle bushy trees. A list is not a particularly efficient representation of the pending list; this will probably be slower than recusing in the common case. I'd suggest keeping the logic to handle left-deep trees, which I find rather elegant, but ditching the pending list. -- 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] Support for RANGE ... PRECEDING windows in OVER
On Sun, Jun 30, 2013 at 11:54 PM, ian link i...@ilink.io wrote: I found some time and I think I am up to speed now. I finally figured out how to add new operator strategies and made a little test operator for myself. It seems pretty clear that assuming '+' and '-' are addition and subtraction is a bad idea. I don't think it would be too tricky to add support for new operator strategies. Andrew Gierth suggested calling these new strategies offset - and offset +, which I think describes it pretty well. I assigned the operator itself to be @+ and @- but that can obviously be changed. If this sounds like a good path to you guys, I will go ahead and implement the operators for the appropriate types. Please let me know if I am misunderstanding something - I am still figuring stuff out :) I don't think I understand the design you have in mind. I'm actually not clear that it would be all that bad to assume fixed operator names, as we apparently do in a few places despite the existence of operator classes. But if that is bad, then I don't know how using @+ and @- instead helps anything. -- 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 generation v5-01 - Patches git tree
On 2013-07-01 14:16:55 -0400, Tom Lane wrote: Alvaro Herrera alvhe...@2ndquadrant.com writes: So the question is, do we take the overhead of the new index (which means overhead on DML operations -- supposedly rare) or do we take the overhead of larger WAL records (which means overhead on all DDL operations)? Note we can make either thing apply to only people running logical replication. I don't believe you can have or not have an index on pg_class as easily as all that. The choice would have to be frozen at initdb time, so people would have to pay the overhead if they thought there was even a small possibility that they'd want logical replication later. It should be possible to create the index in a single database when we start logical replication in that database? Running the index creation with a fixed oid shouldn't require too much code. The oid won't be reused by other pg_class entries since it would be a system one. Alternatively we could always create the index's pg_class/index entry but mark it as !indislive when logical replication isn't active for that database. Then activating it would just require rebuilding that index. But then, I am not fully convinced that's worth the trouble since I don't think pg_class index maintenance is the painspot in DDL atm. Flipping the content of WAL records might not be a terribly simple thing to do either, but at least in principle it could be done during a postmaster restart, without initdb. The main patch combines various booleans in the heap wal records into a flags variable, so there should be enough space to keep track of it without increasing size. Makes size calculations a bit more annoying though as we use the xlog record length to calculate the heap tuple's length, but that's not a large problem. So we could just set the XLOG_HEAP_CONTAINS_CLASSOID flag if wal_level = WAL_LEVEL_LOGICAL. Wal decoding then can throw a tantrum if it finds a record without it and we're done. We could even make that per database, but that seems to be something for the future. 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] Department of Redundancy Department: makeNode(FuncCall) division
On Mon, Jul 1, 2013 at 3:19 AM, Jeevan Chalke jeevan.cha...@enterprisedb.com wrote: I have re-validated this new patch and it looks good to go in now. I saw that it's already marked ready for committer. I don't normally like to commit things over another committer's objections, but this has +1 votes from four other committers (Stephen, Noah, Peter E, myself) so I think that's enough reason to move forward. So committed. -- 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] big test separation POC
While testing patch, I found that make installcheck breaks with your patch and gives following error: Indeed, I did not put the dependency for that target, I really tested check bigcheck. The attached patch adds the needed dependency for installcheck, and I could run it. I checked that no other target seems to be missing a dependency... -- Fabien.diff --git a/src/test/regress/GNUmakefile b/src/test/regress/GNUmakefile index 7309b00..35d29a4 100644 --- a/src/test/regress/GNUmakefile +++ b/src/test/regress/GNUmakefile @@ -86,7 +86,7 @@ regress_data_files = \ $(wildcard $(srcdir)/output/*.source) \ $(filter-out $(addprefix $(srcdir)/,$(input_files)),$(wildcard $(srcdir)/sql/*.sql)) \ $(wildcard $(srcdir)/data/*.data) \ - $(srcdir)/parallel_schedule $(srcdir)/serial_schedule $(srcdir)/resultmap + $(srcdir)/parallel_schedule $(srcdir)/big_schedule $(srcdir)/resultmap install-tests: all install install-lib installdirs-tests $(MAKE) -C $(top_builddir)/contrib/spi install @@ -132,13 +132,33 @@ tablespace-setup: ## Run tests ## +# derive schedules +derived_schedules = serial_schedule parallel_big_schedule serial_big_schedule + +serial_schedule: parallel_schedule + echo '# this file is automatically generated, do not edit!' $@ + egrep '^(test|ignore):' $ | \ + while read op list ; do \ + for test in $$list ; do \ + echo $$op $$test ; \ + done ; \ + done $@ + +parallel_big_schedule: parallel_schedule big_schedule + echo '# this file is automatically generated, do not edit!' $@ + cat $^ $@ + +serial_big_schedule: serial_schedule big_schedule + echo '# this file is automatically generated, do not edit!' $@ + cat $^ $@ + REGRESS_OPTS = --dlpath=. $(EXTRA_REGRESS_OPTS) check: all tablespace-setup $(pg_regress_check) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) $(TEMP_CONF) $(EXTRA_TESTS) -installcheck: all tablespace-setup - $(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/serial_schedule $(EXTRA_TESTS) +installcheck: all tablespace-setup serial_schedule + $(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=./serial_schedule $(EXTRA_TESTS) installcheck-parallel: all tablespace-setup $(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) $(EXTRA_TESTS) @@ -152,11 +172,11 @@ runcheck: check runtest: installcheck runtest-parallel: installcheck-parallel -bigtest: all tablespace-setup - $(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/serial_schedule numeric_big +bigtest: all tablespace-setup serial_big_schedule + $(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=./serial_big_schedule -bigcheck: all tablespace-setup - $(pg_regress_check) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) numeric_big +bigcheck: all tablespace-setup parallel_big_schedule + $(pg_regress_check) $(REGRESS_OPTS) --schedule=./parallel_big_schedule $(MAXCONNOPT) ## @@ -166,7 +186,7 @@ bigcheck: all tablespace-setup clean distclean maintainer-clean: clean-lib # things built by `all' target rm -f $(OBJS) refint$(DLSUFFIX) autoinc$(DLSUFFIX) dummy_seclabel$(DLSUFFIX) - rm -f pg_regress_main.o pg_regress.o pg_regress$(X) + rm -f pg_regress_main.o pg_regress.o pg_regress$(X) $(derived_schedules) # things created by various check targets rm -f $(output_files) $(input_files) rm -rf testtablespace diff --git a/src/test/regress/big_schedule b/src/test/regress/big_schedule new file mode 100644 index 000..4058499 --- /dev/null +++ b/src/test/regress/big_schedule @@ -0,0 +1,3 @@ +# these are big tests not run by default +# these test are expected serial, only put one test per line +test: numeric_big diff --git a/src/test/regress/serial_schedule b/src/test/regress/serial_schedule deleted file mode 100644 index d6eaa7a..000 --- a/src/test/regress/serial_schedule +++ /dev/null @@ -1,139 +0,0 @@ -# src/test/regress/serial_schedule -# This should probably be in an order similar to parallel_schedule. -test: tablespace -test: boolean -test: char -test: name -test: varchar -test: text -test: int2 -test: int4 -test: int8 -test: oid -test: float4 -test: float8 -test: bit -test: numeric -test: txid -test: uuid -test: enum -test: money -test: rangetypes -test: strings -test: numerology -test: point -test: lseg -test: box -test: path -test: polygon -test: circle -test: date -test: time -test: timetz -test: timestamp -test: timestamptz -test: interval -test: abstime -test: reltime -test: tinterval -test: inet -test: macaddr -test: tstypes -test: comments -test: geometry -test: horology -test: regex -test: oidjoins -test: type_sanity -test: opr_sanity -test: insert -test: create_function_1 -test: create_type -test: create_table -test: create_function_2 -test: copy -test: copyselect -test: create_misc -test: create_operator -test: create_index -test: create_view -test: create_aggregate -test: create_function_3 -test: create_cast -test: constraints -test: triggers -test: inherit
Re: [HACKERS] MVCC catalog access
On Mon, Jul 1, 2013 at 10:04 AM, Andres Freund and...@2ndquadrant.com wrote: This is really cool stuff. Thanks. I have to say, if the thing that primarily suffers is pretty extreme DDL in extreme situations I am not really worried. Anybody running anything close to the territory of such concurrency won't perform that much DDL. /me wipes brow. Something picked up when quickly scanning over the last version of the patch: +/* + * Staleness detection for CatalogSnapshot. + */ +static bool CatalogSnapshotStale = true; /* * These are updated by GetSnapshotData. We initialize them this way @@ -177,6 +188,9 @@ GetTransactionSnapshot(void) else CurrentSnapshot = GetSnapshotData(CurrentSnapshotData); + /* Don't allow catalog snapshot to be older than xact snapshot. */ + CatalogSnapshotStale = true; + FirstSnapshotSet = true; return CurrentSnapshot; } @@ -184,6 +198,9 @@ GetTransactionSnapshot(void) if (IsolationUsesXactSnapshot()) return CurrentSnapshot; + /* Don't allow catalog snapshot to be older than xact snapshot. */ + CatalogSnapshotStale = true; + CurrentSnapshot = GetSnapshotData(CurrentSnapshotData); return CurrentSnapshot; @@ -207,6 +224,54 @@ GetLatestSnapshot(void) } Do we really need to invalidate snapshots in either situation? Isn't it implied, that if it's still valid, according to a) no invalidation via local invalidation messages b) no invalidations from other backends, there shouldn't be any possible differences when you only look at the catalog? I had the same thought, removed that code, and then put it back. The problem is that if we revive an older snapshot from the dead, so to speak, our backend's advertised xmin might need to go backwards, and that seems unsafe - e.g. suppose another backend has updated a tuple but not yet committed. We don't see any invalidation messages so decide reuse our existing (old) snapshot and begin a scan. After we've looked at the page containing the new tuple (and decided not to see it), vacuum nukes the old tuple (which we then also don't see). Bad things ensue. It might be possible to avoid the majority of problems in this area via an appropriate set of grotty hacks, but I don't want to go there. And if it needs to change, we could copy the newly generated snapshot to the catalog snapshot if it's currently valid. Yeah, I think there's room for further fine-tuning there. But I think it would make sense to push the patch at this point, and then if we find cases that can be further improved, or things that it breaks, we can fix them. This area is complicated enough that I wouldn't be horribly surprised if we end up having to fix a few more problem cases or even revert the whole thing, but I think we've probably reached the point where further review has less value than getting the code out there in front of more people and seeing where (if anywhere) the wheels come off out in the wild. -- 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] Documentation/help for materialized and recursive views
On Mon, Jul 1, 2013 at 10:20 AM, David Fetter da...@fetter.org wrote: With deepest respect, failing to provide documentation to users on our widest-deployed platform seems pretty hostile to me. Yes, that would be pretty hostile. However, we don't do anything that remotely resembles that statement, nor has anyone proposed any such thing. Personally, I think this whole thread is much ado about nothing. Magnus is basically arguing that people might expect that CREATE VIEW ought to tell you about CREATE MATERIALIZED VIEW also, but I don't find that argument to have a whole lot of merit. Views and materialized views are pretty different things; it is a bit like asking why Googling for dog does not give you information on hot dogs. The output of psql's \h command is intended to be a succinct synopsis summarizing the salient syntax (try saying that five times fast), not a comprehensive reference. If you want the latter, read the fine manual. I admit that this particular case is slightly more prone to confusion than some, but I'm just not that exercised about it. Every bit of detail we add to the \h output is better for the people who otherwise would have been unhappy, but it's worse for all the people who did need it because it's more to read through. Regardless of whether you agree with or disagree with the above statement, building a high-quality documentation reader into psql so that users who are running Windows but not mingw, cygwin, or pgAdmin can access the documentation more easily doesn't seem like the correct solution to this problem. I don't really object if somebody wants to do it (although someone else may object) but it's certainly taking the long way around as far as this particular confusion is concerned. -- 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] Support for RANGE ... PRECEDING windows in OVER
Robert Haas escribió: On Sun, Jun 30, 2013 at 11:54 PM, ian link i...@ilink.io wrote: It seems pretty clear that assuming '+' and '-' are addition and subtraction is a bad idea. I don't think it would be too tricky to add support for new operator strategies. Andrew Gierth suggested calling these new strategies offset - and offset +, which I think describes it pretty well. I assigned the operator itself to be @+ and @- but that can obviously be changed. If this sounds like a good path to you guys, I will go ahead and implement the operators for the appropriate types. Please let me know if I am misunderstanding something - I am still figuring stuff out :) I don't think I understand the design you have in mind. I'm actually not clear that it would be all that bad to assume fixed operator names, as we apparently do in a few places despite the existence of operator classes. But if that is bad, then I don't know how using @+ and @- instead helps anything. Yeah. Currently, all operator classes are tied to access methods. Since nobody seems to have any great idea about creating an access method that requires addition and subtraction, would it make sense to have operator classes that exist solely to support keeping track of such operators for the various datatypes? -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Randomisation for ensuring nlogn complexity in quicksort
On Sun, Jun 30, 2013 at 8:30 AM, Atri Sharma atri.j...@gmail.com wrote: I have been reading the recent discussion and was researching a bit, and I think that we should really go with the idea of randomising the input data(if it is not completely presorted), to ensure that we do not get quadratic complexity. That doesn't ensure any such thing. It just makes it less likely. But we have existing guards that also make that unlikely, so I'm not sure what we'd be gaining. One easy way to do that could be to take a sample of the data set, and take a pivot out of it. Still a better way could be to take multiple samples which are spread of the data set, select a value from each of them, and then take a cumulative pivot(median,maybe). We pretty much do that already. This shouldn't be too complex, and should give us a fixed nlogn complexity even for wild data sets, without affecting existing normal data sets that are present in every day transactions. I even believe that those data sets will also benefit from the above optimisation. The only method of selecting a pivot for quicksort that obtain O(n lg n) run time with 100% certainty is have a magical oracle inside the computer that tells you in fixed time and with perfect accuracy which pivot you should select. If you want to get a useful response to your emails, consider including a statement of what you think the problem is and why you think your proposed changes will help. Consider offering a test case that performs badly and an analysis of the reason why. -- 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] Documentation/help for materialized and recursive views
On 07/01/2013 03:26 PM, Robert Haas wrote: Regardless of whether you agree with or disagree with the above statement, building a high-quality documentation reader into psql so that users who are running Windows but not mingw, cygwin, or pgAdmin can access the documentation more easily doesn't seem like the correct solution to this problem. I don't really object if somebody wants to do it (although someone else may object) but it's certainly taking the long way around as far as this particular confusion is concerned. FWIW, in my reasonably substantial experience, virtually every user on Windows uses pgadmin3. Use of psql is fairly rare. 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] Support for RANGE ... PRECEDING windows in OVER
On Mon, Jul 1, 2013 at 3:28 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Robert Haas escribió: On Sun, Jun 30, 2013 at 11:54 PM, ian link i...@ilink.io wrote: It seems pretty clear that assuming '+' and '-' are addition and subtraction is a bad idea. I don't think it would be too tricky to add support for new operator strategies. Andrew Gierth suggested calling these new strategies offset - and offset +, which I think describes it pretty well. I assigned the operator itself to be @+ and @- but that can obviously be changed. If this sounds like a good path to you guys, I will go ahead and implement the operators for the appropriate types. Please let me know if I am misunderstanding something - I am still figuring stuff out :) I don't think I understand the design you have in mind. I'm actually not clear that it would be all that bad to assume fixed operator names, as we apparently do in a few places despite the existence of operator classes. But if that is bad, then I don't know how using @+ and @- instead helps anything. Yeah. Currently, all operator classes are tied to access methods. Since nobody seems to have any great idea about creating an access method that requires addition and subtraction, would it make sense to have operator classes that exist solely to support keeping track of such operators for the various datatypes? I suppose if we really wanted to do this, it would make more sense to have a new kind of object, maybe CREATE TYPE INTERFACE, rather than shoehorning it into the operator class machinery. It seems like a fairly heavyweight solution, however. -- 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] fallocate / posix_fallocate for new WAL file creation (etc...)
On Mon, Jul 1, 2013 at 2:17 PM, Jeff Davis pg...@j-davis.com wrote: On Tue, 2013-07-02 at 02:13 +0900, Fujii Masao wrote: Even in that case, if a user can easily know which platform posix_fallocate should be used in, we can commit the patch with the configurable GUC parameter. I disagree here. We're not talking about a huge win; this speedup may not even be detectable on a running system. I think Robert summarized the reason for the patch best: I mean, if posix_fallocate() is faster, then it's just faster, right?. But if we need a new GUC, and DBAs now have one more thing they need to test about their platform, then that argument goes out the window. Yeah. If the patch isn't going to be a win on RHEL 5, I'd consider that a good reason to scrap it for now and revisit it in 3 years. There are still a LOT of people running RHEL 5, and the win isn't big enough to engineer a more complex solution. But ultimately RHEL 5, like any other old system, will go away. However, Greg's latest testing results seem to show that there isn't much to worry about, so we may be fine anyway. -- 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] extensible external toast tuple support
On 2013-06-28 11:25:50 -0400, Robert Haas wrote: On Fri, Jun 28, 2013 at 10:53 AM, Andres Freund and...@2ndquadrant.com wrote: Why does toast_insert_or_update() need to go through all the rigamarole in toast_datum_differs()? I would have thought that it could simply treat any external-indirect value as needing to be detoasted and retoasted, since the destination is the disk anyhow. We could do that, yes. But I think it might be better not to: If we simplify the tuples used in a query to not reference ondisk tuples anymore and we then UPDATE using that new version I would rather not retoast all the unchanged columns. I can e.g. very well imagine that we decide to resolve toasted Datums to indirect Datums during an UPDATE if there are multiple BEFORE UPDATE triggers to avoid detoasting in each and every one of them. Such a tuple will then passed to heap_update... I must be missing something. At that point, yes, you'd like to avoid re-toasting unnecessarily, but ISTM you've already bought the farm. Unless I'm misunderstanding the code as written, you'd just end up writing the indirect pointer back out to disk in that scenario. That's not gonna work... You didn't misunderstand anything unfortunately. I broke that somewhere along the road, probably when factoring things out to toast_datum_differs(). Fixed... Which shows that we need tests. I've added some using a function in regress.c that makes all datums indirect. Together with a triggers that allows some testing. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services From 7b2c83aa337e639d2467359ab7b0cc00a2f36cde Mon Sep 17 00:00:00 2001 From: Andres Freund and...@anarazel.de Date: Tue, 11 Jun 2013 23:25:26 +0200 Subject: [PATCH] Add support for multiple kinds of external toast datums There are several usecases where our current representation of external toast datums is limiting: * adding new compression schemes * avoidance of repeated detoasting * externally decoded toast tuples For that support 'tags' on external (varattrib_1b_e) varlenas which recoin the current va_len_1be field to store the tag (or type) of a varlena. To determine the actual length a macro VARTAG_SIZE(tag) is added which can be used to map from a tag to the actual length. This patch adds support for 'indirect' tuples which point to some externally allocated memory containing a toast tuple. It also implements the stub for a different compression algorithm. 0.3 --- src/backend/access/heap/tuptoaster.c | 189 --- src/include/access/tuptoaster.h | 5 + src/include/postgres.h | 84 +++--- src/test/regress/expected/indirect_toast.out | 151 ++ src/test/regress/input/create_function_1.source | 5 + src/test/regress/output/create_function_1.source | 4 + src/test/regress/parallel_schedule | 2 +- src/test/regress/regress.c | 92 +++ src/test/regress/serial_schedule | 1 + src/test/regress/sql/indirect_toast.sql | 61 10 files changed, 552 insertions(+), 42 deletions(-) create mode 100644 src/test/regress/expected/indirect_toast.out create mode 100644 src/test/regress/sql/indirect_toast.sql diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.c index fc37ceb..fd3362f 100644 --- a/src/backend/access/heap/tuptoaster.c +++ b/src/backend/access/heap/tuptoaster.c @@ -44,9 +44,6 @@ #undef TOAST_DEBUG -/* Size of an EXTERNAL datum that contains a standard TOAST pointer */ -#define TOAST_POINTER_SIZE (VARHDRSZ_EXTERNAL + sizeof(struct varatt_external)) - /* * Testing whether an externally-stored value is compressed now requires * comparing extsize (the actual length of the external data) to rawsize @@ -87,11 +84,11 @@ static struct varlena *toast_fetch_datum_slice(struct varlena * attr, * heap_tuple_fetch_attr - * * Public entry point to get back a toasted value from - * external storage (possibly still in compressed format). + * external source (possibly still in compressed format). * * This will return a datum that contains all the data internally, ie, not - * relying on external storage, but it can still be compressed or have a short - * header. + * relying on external storage or memory, but it can still be compressed or + * have a short header. -- */ struct varlena * @@ -99,13 +96,35 @@ heap_tuple_fetch_attr(struct varlena * attr) { struct varlena *result; - if (VARATT_IS_EXTERNAL(attr)) + if (VARATT_IS_EXTERNAL_ONDISK(attr)) { /* * This is an external stored plain value */ result = toast_fetch_datum(attr); } + else if (VARATT_IS_EXTERNAL_INDIRECT(attr)) + { + /* + * copy into the caller's memory context. That's not required in all + * cases but sufficient for now since this
Re: [HACKERS] Randomisation for ensuring nlogn complexity in quicksort
On Mon, Jul 1, 2013 at 4:32 PM, Robert Haas robertmh...@gmail.com wrote: This shouldn't be too complex, and should give us a fixed nlogn complexity even for wild data sets, without affecting existing normal data sets that are present in every day transactions. I even believe that those data sets will also benefit from the above optimisation. The only method of selecting a pivot for quicksort that obtain O(n lg n) run time with 100% certainty is have a magical oracle inside the computer that tells you in fixed time and with perfect accuracy which pivot you should select. Doesn't a linear median algorithm (say median of medians) get you O(n lg n)? Granted, with a huge constant (I think 4)... but it should still be O(n lg n). -- 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] Documentation/help for materialized and recursive views
On Mon, Jul 1, 2013 at 9:26 PM, Robert Haas robertmh...@gmail.com wrote: On Mon, Jul 1, 2013 at 10:20 AM, David Fetter da...@fetter.org wrote: With deepest respect, failing to provide documentation to users on our widest-deployed platform seems pretty hostile to me. Yes, that would be pretty hostile. However, we don't do anything that remotely resembles that statement, nor has anyone proposed any such thing. Personally, I think this whole thread is much ado about nothing. Magnus is basically arguing that people might expect that CREATE VIEW ought to tell you about CREATE MATERIALIZED VIEW also, but I don't find that argument to have a whole lot of merit. Views and materialized views are pretty different things; it is a bit like How different are they really? Yes, they are very different from an implementation standpoint, from an enduser perspective they really are not. If they were, they'd probably be called something else.. asking why Googling for dog does not give you information on hot dogs. The output of psql's \h command is intended to be a succinct I'd personally say it's more like googling for dog gives me hits specifically around dog breeding and not just dogs themselves. synopsis summarizing the salient syntax (try saying that five times fast), not a comprehensive reference. If you want the latter, read the fine manual. I admit that this particular case is slightly more prone to confusion than some, but I'm just not that exercised about it. Every bit of detail we add to the \h output is better for the people who otherwise would have been unhappy, but it's worse for all the people who did need it because it's more to read through. True. Regardless of whether you agree with or disagree with the above statement, building a high-quality documentation reader into psql so that users who are running Windows but not mingw, cygwin, or pgAdmin can access the documentation more easily doesn't seem like the correct solution to this problem. I don't really object if somebody wants to do it (although someone else may object) but it's certainly taking the long way around as far as this particular confusion is concerned. I still think a better option to that would be to get psql to provide a link to the full documentation there. pgAdmin could also do that, but doesn't - it gets you a link to the main documentation, but not a context sensitive one IIRC. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] fallocate / posix_fallocate for new WAL file creation (etc...)
On 7/1/13 12:34 PM, Jeff Davis wrote: On Sun, 2013-06-30 at 16:09 -0400, Andrew Dunstan wrote: It was originally generated. Since then it's been maintained by hand. What is the procedure for maintaining it by hand? Edit away. Why are HAVE_POSIX_SIGNALS and HAVE_SYNC_FILE_RANGE in there (though commented out), but not HAVE_POSIX_FADVISE? Because maintaining it by hand is prone to inconsistencies. I wouldn't worry about 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] Eliminating PD_ALL_VISIBLE, take 2
On Mon, Jul 1, 2013 at 1:21 PM, Jeff Davis pg...@j-davis.com wrote: On Sun, 2013-06-30 at 22:58 -0400, Robert Haas wrote: I thought that Jeff withdrew this patch. No -- was there a reason you thought that? I thought I remembered you saying you were going to abandon it in the face of objections. I know it could use another round of testing before commit, and there may be a couple other things to clear up. But I don't want to invest a lot of time there right now, because, as I understand it, you still object to the patch anyway. I am still not entirely clear on the objections to this patch: 1. Contention was a concern, but I believe I have mitigated it. Strictly speaking, additional pins may be acquired, but the cost of those pin operations will be spread over a lot of other work. 2. There are quite a few different ideas about where we're going with PD_ALL_VISIBLE and freezing, but it seems like removing PD_ALL_VISIBLE is potentially compatible with most of them. Any others? The patch reduces code complexity and reduces writes during a data load. Well, I don't believe there's any way to really eliminate the contention concern completely. There's no way around the fact that it means more access to the visibility map, and I've seen recent (albeit circumstantial thus far) evidence that that can be a real problem. The buffer mapping locks are a problem, too, so anything that means more page accesses can't be taken lightly. I agree your proposed changes reduce the chances of problems; I don't agree that they eliminate them. The other concern I remember being expressed (and not just by me, but by a number of people) is that your patch turns loss of a visibility map bit into a data corruption scenario, which it currently isn't. Right now, if your visibility map gets corrupted, you can always recover by deleting it. Under your proposal that would no longer be possible. I think that's sufficient grounds to reject the patch by itself, even if there were no other issues. If that doesn't strike you as very dangerous, I'm baffled as to why not. -- 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] Documentation/help for materialized and recursive views
On Mon, Jul 1, 2013 at 3:57 PM, Magnus Hagander mag...@hagander.net wrote: How different are they really? Yes, they are very different from an implementation standpoint, from an enduser perspective they really are not. If they were, they'd probably be called something else. They're different because they consume storage, require effort to update, and aren't always up-to-date. Those things are all quite user-visible. I still think a better option to that would be to get psql to provide a link to the full documentation there. It seems like clutter to me, but I'll defer to whatever the consensus is. -- 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] Documentation/help for materialized and recursive views
On 07/01/2013 07:20 AM, David Fetter wrote: On Mon, Jul 01, 2013 at 10:05:24AM -0400, Peter Eisentraut wrote: On 6/28/13 2:27 PM, David Fetter wrote: You can run \! man from within psql, And if you're on Windows, you're Sadly Out of Luck with that. Is there an equivalent we could #ifdef in for that platform? If you are using psql on Windows extensively, you probably have one of mingw, cygwin, or pgadmin handy, all of which can get you to the documentation. I don't think it's worth devising a mechanism for those not covered by this. With deepest respect, failing to provide documentation to users on our widest-deployed platform seems pretty hostile to me. There was an earlier suggestion that we provide URLs, which seems like a decent way forward as those environments so locked down as to disallow outbound HTTP are pretty rare, and non-networked computers are even more rare. Although I agree with the sentiment the idea that postgres more widely deployed on windows than other platforms is rather laughable. The only metrics we have are downloads which doesn't count cause linux ships with postgres with a simple yum or apt-get. Whatever solution we decide, we should not push this responsibility off on pgadmin as pgadmin is not part of PostgreSQL but a third party tool. The standard postgresql client is psql (for good or bad) and we should support psql fully on all platforms. Sincerely, Joshua D. Drake -- Command Prompt, Inc. - http://www.commandprompt.com/ 509-416-6579 PostgreSQL Support, Training, Professional Services and Development High Availability, Oracle Conversion, Postgres-XC, @cmdpromptinc For my dreams of your image that blossoms a rose in the deeps of my heart. - W.B. Yeats -- 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] Randomisation for ensuring nlogn complexity in quicksort
On Mon, Jul 1, 2013 at 3:54 PM, Claudio Freire klaussfre...@gmail.com wrote: On Mon, Jul 1, 2013 at 4:32 PM, Robert Haas robertmh...@gmail.com wrote: This shouldn't be too complex, and should give us a fixed nlogn complexity even for wild data sets, without affecting existing normal data sets that are present in every day transactions. I even believe that those data sets will also benefit from the above optimisation. The only method of selecting a pivot for quicksort that obtain O(n lg n) run time with 100% certainty is have a magical oracle inside the computer that tells you in fixed time and with perfect accuracy which pivot you should select. Doesn't a linear median algorithm (say median of medians) get you O(n lg n)? Granted, with a huge constant (I think 4)... but it should still be O(n lg n). No. Thinking about this a little more, I believe the way it works out is that any algorithm for picking the median that guarantees that a certain *percentage* of the tuples will be in the smaller partition will have O(n lg n) complexity, but any algorithm that only guarantees that a fixed *number* of tuples in the smaller partition is still quadratic in complexity. In the case of a median algorithm, you're only guaranteed to have 2 elements in the smaller partition, which is a constant. If you take a median of medians, you're guaranteed to have 8 elements in the smaller partition, which is bigger, but still a constant. The reason why this doesn't matter much in practice is because the data distribution that causes quadratic behavior for median-of-medians is not one which is likely to occur in the real world and will probably only come up if chosen by an adversary who is attempting to make your life miserable. -- 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] pg_ctl promote exit status
On 7/1/13 12:47 PM, Bruce Momjian wrote: Approximately none of these changes seem correct to me. For example, why is failing to open the PID file 6, or failing to start the server 7? Well, according to that URL, we have: 6 program is not configured 7 program is not running There is also 4 user had insufficient privilege I just updated the pg_ctl.c comments to at least point to a valid URL for this. I think we can just call this item closed because I am still unclear if these return codes should be returned by pg_ctl or the start/stop script. Anyway, while I do think pg_ctl could pass a little more information back about failure via its return code, I am unclear if LSB is the right approach. Yeah, a lot of these things are unclear and not used in practice, so it's probably better to stick to exit code 1, unless there is a clear use case. The status case is different, because there the exit code can be passed out by the init script directly. -- 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] GIN improvements part 1: additional information
On 01.07.2013 13:28, Alexander Korotkov wrote: Thanks! So, we have a lot of stuff and you give the points for further work. Could you please verify my plan of work on these patches: 1) Solving questions of archives.postgresql.org/** message-id/51CEA13C.8040103@**vmware.comhttp://archives.postgresql.org/message-id/51cea13c.8040...@vmware.com for packed postinglists. 2) Extract additional info patch based on packed postinglists. 3) Rewrite interface of fast scan. Do CPU and IO benchmarking. 4) Do IO benchmarking of index ordering. Cleanup, comments and READMEs are assumed in each item. Yep, sounds good! - Heikki -- 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] plpython implementation
On 7/1/13 1:29 AM, james wrote: Given how useful it is to have a scripting language that can be used outside of the database as well as inside it, would it be reasonable to consider 'promoting' pllua? You can start promoting pllua by making it work with current PostgreSQL versions. It hasn't been updated in 5 years, and doesn't build cleanly last I checked. Having a well-maintained and fully featured pllua available would surely be welcome by many. -- 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] Randomisation for ensuring nlogn complexity in quicksort
On Mon, Jul 1, 2013 at 5:12 PM, Robert Haas robertmh...@gmail.com wrote: On Mon, Jul 1, 2013 at 3:54 PM, Claudio Freire klaussfre...@gmail.com wrote: On Mon, Jul 1, 2013 at 4:32 PM, Robert Haas robertmh...@gmail.com wrote: This shouldn't be too complex, and should give us a fixed nlogn complexity even for wild data sets, without affecting existing normal data sets that are present in every day transactions. I even believe that those data sets will also benefit from the above optimisation. The only method of selecting a pivot for quicksort that obtain O(n lg n) run time with 100% certainty is have a magical oracle inside the computer that tells you in fixed time and with perfect accuracy which pivot you should select. Doesn't a linear median algorithm (say median of medians) get you O(n lg n)? Granted, with a huge constant (I think 4)... but it should still be O(n lg n). No. Thinking about this a little more, I believe the way it works out is that any algorithm for picking the median that guarantees that a certain *percentage* of the tuples will be in the smaller partition will have O(n lg n) complexity, but any algorithm that only guarantees that a fixed *number* of tuples in the smaller partition is still quadratic in complexity. In the case of a median algorithm, you're only guaranteed to have 2 elements in the smaller partition, which is a constant. If you take a median of medians, you're guaranteed to have 8 elements in the smaller partition, which is bigger, but still a constant. What? A median of medians algorithm will guarantee floor(N/2) elements on the smaller. That's the definition of median. Note that I'm referring to picking the actual median of all tuples, not just a sample. That's slow, but it guarantees O(n log n). -- 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] Bugfix and new feature for PGXS
On 6/29/13 1:54 PM, Andrew Dunstan wrote: I haven't seen a response to this. One thing we are missing is documentation. Given that I'm inclined to commit all of this (i.e. cedric's patches 1,2,3, and 4 plus my addition). Could someone post an updated set of patches that is currently under consideration? I'm also inclined to backpatch it, since without that it seems to me unlikely packagers will be able to make practical use of it for several years, and the risk is very low. Actually, the risk of makefile changes is pretty high, especially in cases involving advanced features such as vpath. GNU make hasn't been as stable is one might think, lately. We should carefully consider exactly which parts are worth backpatching. -- 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: simple date constructor from numeric values
On 7/1/13 3:47 AM, Pavel Stehule wrote: and it is a part of our ToDo: Add function to allow the creation of timestamps using parameters so we can have a functions with signatures I would just name them date(...), time(...), etc. CREATE OR REPLACE FUNCTION construct_date(year int, month int DEFAULT 1, day int DEFAULT 1) RETURNS date; I would not use default values for this one. CREATE OR REPLACE FUNCTION construct_time(hour int DEFAULT 0, mi int DEFAULT 0, sec int DEFAULT 0, ms float DEFAULT 0.0); If we are using integer datetime storage, we shouldn't use floats to construct them. -- 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] Add regression tests for DISCARD
On 17 June 2013 18:14, Marko Kreen mark...@gmail.com wrote: Perhaps existing tests in guc.sql should be merged into it? Thanks Marko for pointing out about guc.sql. Please find attached a patch to move DISCARD related tests from guc.sql to discard.sql. It adds an extra test for a DISCARD PLANS line, although I amn't sure on how to validate that its working. Personally, I wouldn't call this a great patch, since most of the tests were already running, although in a generic script. The separation of DISCARD related tests to another file is arguably good for the long-term though. -- Robins Tharakan regress_discard_v3.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_resetxlog -m documentation not up to date
On 6/19/13 9:57 PM, Alvaro Herrera wrote: Peter Eisentraut wrote: Ping. This ought to be fixed before 9.3 goes out. Will fix. On Sat, 2013-04-27 at 21:22 -0400, Peter Eisentraut wrote: The pg_resetxlog -m option was changed from The man page lists the -m option as -m mxid,mxid, but the --help output has -m XID,XID. Is that correct? Do we consider MXIDs to be a subset of XIDs? -- 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] Bugfix and new feature for PGXS
On 07/01/2013 04:39 PM, Peter Eisentraut wrote: On 6/29/13 1:54 PM, Andrew Dunstan wrote: I haven't seen a response to this. One thing we are missing is documentation. Given that I'm inclined to commit all of this (i.e. cedric's patches 1,2,3, and 4 plus my addition). Could someone post an updated set of patches that is currently under consideration? See what I actually committed today. I'm also inclined to backpatch it, since without that it seems to me unlikely packagers will be able to make practical use of it for several years, and the risk is very low. Actually, the risk of makefile changes is pretty high, especially in cases involving advanced features such as vpath. GNU make hasn't been as stable is one might think, lately. We should carefully consider exactly which parts are worth backpatching. These changes are fairly small and mostly non-invasive, but if I've broken something we should find out about it fairly quickly, I hope. 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] changeset generation v5-01 - Patches git tree
On 27 June 2013 23:18, Tom Lane t...@sss.pgh.pa.us wrote: Exactly what is the argument that says performance of this function is sufficiently critical to justify adding both the maintenance overhead of a new pg_class index, *and* a broken-by-design syscache? I think we all agree on changing the syscache. I'm not clear why adding a new permanent index to pg_class is such a problem. It's going to be a very thin index. I'm trying to imagine a use case that has pg_class index maintenance as a major part of its workload and I can't. An extra index on pg_attribute and I might agree with you. The pg_class index would only be a noticeable % of catalog rows for very thin temp tables, but would still even then be small; that isn't even necessary work since we all agree that temp table overheads could and should be optimised away somwhere. So blocking a new index because of that sounds strange. What issues do you foresee? How can we test them? Or perhaps we should just add the index and see if we later discover a measurable problem workload? -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services
Re: [HACKERS] fallocate / posix_fallocate for new WAL file creation (etc...)
On 7/1/13 3:44 PM, Robert Haas wrote: Yeah. If the patch isn't going to be a win on RHEL 5, I'd consider that a good reason to scrap it for now and revisit it in 3 years. There are still a LOT of people running RHEL 5, and the win isn't big enough to engineer a more complex solution. I'm still testing, expect to have this wrapped up on my side by tomorrow. So much of the runtime here is the file setup/close that having a 2:1 difference in number of writes, what happens on the old platforms, it is hard to get excited about. I don't think the complexity to lock out RHEL5 here is that bad even if it turns out to be a good idea. Just add another configure check for fallocate, and on Linux if it's not there don't use posix_fallocate either. Maybe 5 lines of macro code? RHEL5 sure isn't going anyway anytime soon, but at the same time there won't be that many 9.4 deployments on that version. I've been digging into the main situation where this feature helps, and it won't be easy to duplicate in a benchmark situation. Using Linux's fallocate works as a hint that the whole 16MB should be allocated at once, and therefore together on disk if feasible. The resulting WAL files should be less prone to fragmentation. That's actually the biggest win of this approach, but I can't easily duplicate the sort of real-world fragmentation I see on live servers here.Given that, I'm leaning toward saying that unless there's a clear regression on older platforms, above the noise floor, this is still the right thing to do. I fully agree that this needs to fully automatic--no GUC--before it's worth committing. If we can't figure out the right thing to do now, there's little hope anyone else will in a later tuning expedition. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.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] Randomisation for ensuring nlogn complexity in quicksort
On Mon, Jul 1, 2013 at 12:32 PM, Robert Haas robertmh...@gmail.com wrote: I have been reading the recent discussion and was researching a bit, and I think that we should really go with the idea of randomising the input data(if it is not completely presorted), to ensure that we do not get quadratic complexity. That doesn't ensure any such thing. It just makes it less likely. But we have existing guards that also make that unlikely, so I'm not sure what we'd be gaining. +1 -- 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] pg_resetxlog -m documentation not up to date
Peter Eisentraut wrote: On 6/19/13 9:57 PM, Alvaro Herrera wrote: Peter Eisentraut wrote: Ping. This ought to be fixed before 9.3 goes out. Will fix. On Sat, 2013-04-27 at 21:22 -0400, Peter Eisentraut wrote: The pg_resetxlog -m option was changed from The man page lists the -m option as -m mxid,mxid, but the --help output has -m XID,XID. Is that correct? Do we consider MXIDs to be a subset of XIDs? Yeah, mxids are stored in places that normally store Xids (namely a tuples' Xmax field). That said, I don't see any reason not to have --help show -m MXID,MXID. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers