Re: [HACKERS] Move unused buffers to freelist
On Tuesday, July 02, 2013 12:00 AM Robert Haas wrote: 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. Sure, after completing my other review work of Commit Fest, I will devise the solution for the suggestions summarized in previous mail and then start a discussion about same. 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] Optimizing pglz compressor
On Monday, July 01, 2013 1:36 PM 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. Thanks, will update the WAL Optimization patch based on this and post the new patch and data on the corresponding thread. 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: query result history in psql
Maciej - I can see your resistance to some kind of interactive mode. It would definitely be more code and create a less simple user interface. I would be perfectly happy if we left that part as it is now. However, I think it would be important to have a way of displaying the query history. Yes, you can search through your console backwards. You can still search through your old queries, but this would be a good alternative. What if the user reconnects after working for a while? Their old bash history might be gone. This would leave them with a big query history and no point of reference. Personally, I would find this feature very worthwhile. The query history wouldn't be crippled without it, but it would be a lot less flexible. Pavel Stehule Monday, July 01, 2013 4:05 AM a idea is good, but I don't think, it can be useful with currentimplementation. How I can identify, what is correct answer for myquery? Have I remember twenty numbers and twenty queries?RegardsPavel Maciej Gajewski Monday, July 01, 2013 4:01 AM 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. Pavel Stehule Monday, July 01, 2013 1:31 AM 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 Maciej Gajewski Monday, July 01, 2013 1:23 AM 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 besaved 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 ian link Monday, July 01, 2013 12:19 AM but maybe some interactive mode should be usefull - so after execution, and showing result, will be prompt if result should besaved or not. I
[HACKERS] pgsql_tmp and external sort
Dear Hackers Recently I find about pgsql_tmp directory. As I think, this directory is a area for making temporal files which are used for external sort. I wonder if anyone could help me to find the module of pg that is responsible for the temporal space and external sort in the PG source code. Regards Soroosh Sardari
Re: [HACKERS] refresh materialized view concurrently
On Thu, Jun 27, 2013 at 12:19 AM, Hitoshi Harada umi.tan...@gmail.comwrote: On Wed, Jun 26, 2013 at 1:38 AM, Kevin Grittner kgri...@ymail.com wrote: New version attached. Will take another look. Oops! drop materialized view if exists mv; drop table if exists foo; create table foo(a, b) as values(1, 10); create materialized view mv as select * from foo; create unique index on mv(a); insert into foo select * from foo; refresh materialized view mv; refresh materialized view concurrently mv; test=# refresh materialized view mv; ERROR: could not create unique index mv_a_idx DETAIL: Key (a)=(1) is duplicated. test=# refresh materialized view concurrently mv; REFRESH MATERIALIZED VIEW Here's one more. create table foo(a, b, c) as values(1, 2, 3); create materialized view mv as select * from foo; create unique index on mv (a); create unique index on mv (b); create unique index on mv (c); insert into foo values(2, 3, 4); insert into foo values(3, 4, 5); refresh materialized view concurrently mv; test=# refresh materialized view concurrently mv; ERROR: syntax error at or near FROM LINE 1: UPDATE public.mv x SET FROM pg_temp_2.pg_temp_16615_2 d WHE... ^ QUERY: UPDATE public.mv x SET FROM pg_temp_2.pg_temp_16615_2 d WHERE d.tid IS NOT NULL AND x.ctid = d.tid Other than these, I've found index is opened with NoLock, relying on ExclusiveLock of parent matview, and ALTER INDEX SET TABLESPACE or something similar can run concurrently, but it is presumably safe. DROP INDEX, REINDEX would be blocked by the ExclusiveLock. -- Hitoshi Harada
Re: [HACKERS] [GENERAL] Floating point error
At 2013-06-24 10:16:33 +, laurenz.a...@wien.gv.at wrote: What do you think of the attached version? I'm not exactly fond of it, but I can't come up with a better version either. It's slightly better if but may not accurately represent the stored value is removed. Does anyone else have suggestions? -- Abhijit -- 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] [9.4 CF 1] The Commitfest Slacker List
Sorry for joining the thread this late, but I didn't really expect to see myself listed as a slacker on a public list. Additionally, the following committers are not listed as reviewers on any patch. Note that I have no way to search which ones might be *committers* on a patch, so these folks are not necessarily slackers This means I'm a slacker because I'm not reviewing or committing a patch, right? Do we have a written rule somewhere? Or some other communication about this? I would have liked to know this requirement before getting singled out in public. Also I'd like to know who made the decision to require a patch review from each committer as I certainly missed it. Was the process public? Where can I find more about it? In general I find it difficult to digest that other people make decisions about my spare time without me having a word in the discussion. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org Jabber: michael.meskes at gmail dot com VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL -- 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
2013/7/1 Peter Eisentraut pete...@gmx.net: 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. +1 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. I have no problem with it 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. so possible signature signature should be CREATE FUNCTION time(hour int, mi int, sec int, used int) ?? and CREATE FUNCTION timetz(hour int, mi int, sec int, isec int, tz int) ?? 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] pgsql_tmp and external sort
On 02.07.2013 10:55, Soroosh Sardari wrote: Dear Hackers Recently I find about pgsql_tmp directory. As I think, this directory is a area for making temporal files which are used for external sort. I wonder if anyone could help me to find the module of pg that is responsible for the temporal space and external sort in the PG source code. See src/backend/utils/sort/tuplesort.c and src/backend/storage/file/buffile.c for starters. - 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] [9.4 CF 1] The Commitfest Slacker List
Folks, For 9.2, we adopted it as policy that anyone submitting a patch to a commitfest is expected to review at least one patch submitted by someone else. And that failure to do so would affect the attention your patches received in the future. For that reason, I'm publishing the list below of people who submitted patches and have not yet claimed any patch in the commitfest to review. For those of you who are contributing patches for your company, please let your boss know that reviewing is part of contributing, and that if you don't do the one you may not be able to do the other. The two lists below, idle submitters and committers, constitutes 26 people. This *outnumbers* the list of contributors who are busy reviewing patches -- some of them four or more patches. If each of these people took just *one* patch to review, it would almost entirely clear the list of patches which do not have a reviewer. If these folks continue not to do reviews, this commitfest will drag on for at least 2 weeks past its closure date. Andrew Geirth Nick White Peter Eisentrout Alexander Korotkov Etsuro Fujita Hari Babu Jameison Martin Jon Nelson Oleg Bartunov Chris Farmiloe Samrat Revagade Alexander Lakhin Mark Kirkwood Liming Hu Maciej Gajewski Josh Kuperschmidt Mark Wong Gurjeet Singh Robins Tharakan Tatsuo Ishii Karl O Pinc It took me for while before realizing that I am on the list because I posted this: http://www.postgresql.org/message-id/20130610.091605.250603479334631505.t-is...@sraoss.co.jp Because I did not register the patch into CF page myself. I should have not posted it until I find any patch which I can take care of. Sorry for this. -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese: http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [BUGS] BUG #7873: pg_restore --clean tries to drop tables that don't exist
Hello 2013/3/8 Josh Kupershmidt schmi...@gmail.com: On Fri, Mar 8, 2013 at 2:23 AM, Pavel Stehule pavel.steh...@gmail.com wrote: 2013/3/8 Josh Kupershmidt schmi...@gmail.com: Cool. I think it would also be useful to check that --clean may only be used with --format=p to avoid any confusion there. (This issue could be addressed in a separate patch if you'd rather not lard this patch.) no some people - like we in our company would to use this feature for quiet and strict mode for plain text format too. enabling this feature has zero overhead so there are no reason block it anywhere. I'm not sure I understand what you're getting at, but maybe my proposal wasn't so clear. Right now, you can specify --clean along with -Fc to pg_dump, and pg_dump will not complain even though this combination is nonsense. I am proposing that pg_dump error out in this case, i.e. $ pg_dump -Fc --file=test.dump --clean -d test pg_dump: option --clean only valid with plain format dump Although this lack of an error a (IMO) misfeature of existing pg_dump, so if you'd rather leave this issue aside for your patch, that is fine. I tested last patch and I am thinking so this patch has sense for custom format too [postgres@localhost ~]$ /usr/local/pgsql/bin/pg_dump --clean -t a -Fc postgres dump [postgres@localhost ~]$ psql -c drop table a DROP TABLE [postgres@localhost ~]$ pg_restore --clean -d postgres dump pg_restore: [archiver (db)] Error while PROCESSING TOC: pg_restore: [archiver (db)] Error from TOC entry 171; 1259 16462 TABLE a postgres pg_restore: [archiver (db)] could not execute query: ERROR: table a does not exist Command was: DROP TABLE public.a; WARNING: errors ignored on restore: 1 [postgres@localhost ~]$ /usr/local/pgsql/bin/pg_dump --if-exist --clean -t a -Fc postgres dump [postgres@localhost ~]$ psql -c drop table a DROP TABLE [postgres@localhost ~]$ pg_restore --clean -d postgres dump So limit for plain format is not too strict Regards Pavel Josh -- 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
On Mon, Jul 1, 2013 at 8:36 PM, Pavel Stehule pavel.steh...@gmail.comwrote: 2013/6/29 Pavel Stehule pavel.steh...@gmail.com: Hello updated patch - precious Assert, more comments Regards Pavel stripped Thanks. Patch looks good to me now. Revalidated and didn't see any issue so marking Ready For Committer. Thanks Pavel. -- Jeevan B Chalke Senior Software Engineer, RD EnterpriseDB Corporation The Enterprise PostgreSQL Company
Re: [HACKERS] Review: query result history in psql
The query history is stored within the client, so once the user stops the client, it is gone. But yes, it would be useful to have some tool that would allow you to see what's in there. I could be a command (\showans ?) that would list all :ansXXX variables, together with the query text and the size of the answer. It would probably look ugly for very long queries, but could be useful anyway. I'm not sure if I'll be able to implement this feature any time soon, as I'm very busy at the moment and going for a business trip in few days. In the meantime, I've applied your suggestions and moved the sting-manipulating functions to stringutils. Also fixed a tiny bug. Patch attached. M psql-ans.3.diff 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] pgsql_tmp and external sort
From: Soroosh Sardari soroosh.sard...@gmail.com I wonder if anyone could help me to find the module of pg that is responsible for the temporal space and external sort in the PG source code. See src/backend/utils/sort/ for sort implementation. That uses BufFile in src/backend/storage/file/buffile.c, which in turn uses OpenTemporaryFile in src/backend/storage/file/fd.c. Regards MauMau -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] signed vs. unsigned in plpy_procedure.c
Hi, src/postgresql/src/pl/plpython/plpy_procedure.c: In function ‘PLy_procedure_munge_source’: src/postgresql/src/pl/plpython/plpy_procedure.c:507:2: warning: comparison of unsigned expression = 0 is always true [-Wtype-limits] Assert(plen = 0 plen mlen); Which has a point, we assign sprintf()s result to a size_t and then check for a negative value. Which doesn't work. Obviously the impact is miniscule, but removing legitimate warnings is a good thing. Trivial patch attached. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services diff --git a/src/pl/plpython/plpy_procedure.c b/src/pl/plpython/plpy_procedure.c index 5007e77..d278d6e 100644 --- a/src/pl/plpython/plpy_procedure.c +++ b/src/pl/plpython/plpy_procedure.c @@ -494,8 +494,8 @@ PLy_procedure_munge_source(const char *name, const char *src) char *mrc, *mp; const char *sp; - size_t mlen, -plen; + size_t mlen; + int plen; /* * room for function source and the def statement -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [BUGS] BUG #7873: pg_restore --clean tries to drop tables that don't exist
Hello 2013/3/8 Josh Kupershmidt schmi...@gmail.com: [Moving to -hackers] On Sat, Feb 23, 2013 at 2:51 PM, Pavel Stehule pavel.steh...@gmail.com wrote: so * --conditional-drops replaced by --if-exists Thanks for the fixes, I played around with the patch a bit. I was sort of expecting this example to work (after setting up the regression database with `make installcheck`) pg_dump --clean --if-exists -Fp -d regression --file=regression.sql createdb test psql -v ON_ERROR_STOP=1 --single-transaction -d test -f regression.sql But it fails, first at: ... DROP TRIGGER IF EXISTS tsvectorupdate ON public.test_tsvector; ERROR: relation public.test_tsvector does not exist This seems like a shortcoming of DROP TRIGGER ... IF EXISTS, and it looks like DROP RULE ... IF EXISTS has the same problem. I recall DROP ... IF EXISTS being fixed recently for not to error out if the schema specified for the object does not exist, and ISTM the same arguments could be made in favor of fixing DROP TRIGGER/TABLE ... IF EXISTS not to error out if the table doesn't exist. yes, I am thinking so it is probably best solution. Without it I should to generate a DO statement with necessary conditions. :( I'll prepare patch and proposal. Working further through the dump of the regression database, these also present problems for --clean --if-exists dumps: DROP CAST IF EXISTS (text AS public.casttesttype); ERROR: type public.casttesttype does not exist DROP OPERATOR IF EXISTS public.% (point, widget); ERROR: type widget does not exist DROP FUNCTION IF EXISTS public.pt_in_widget(point, widget); ERROR: type widget does not exist I'm not sure whether DROP CAST/OPERATOR/FUNCTION IF EXISTS should be more tolerant of nonexistent types, of if the mess could perhaps be avoided by dump reordering. we can raise a warning instead error ? Regards Pavel Note, this usability problem affects unpatched head as well: pg_dump -Fc -d regression --file=regression.dump pg_restore --clean -1 -d regression regression.dump ... pg_restore: [archiver (db)] could not execute query: ERROR: type widget does not exist Command was: DROP FUNCTION public.widget_out(widget); (The use here is a little different than the first example above, but I would still expect this case to work.) The above problems with IF EXISTS aren't really a problem of the patch per se, but IMO it would be nice to straighten all the issues out together for 9.4. * -- additional check, available only with -c option Cool. I think it would also be useful to check that --clean may only be used with --format=p to avoid any confusion there. (This issue could be addressed in a separate patch if you'd rather not lard this patch.) Some comments on the changes: 1. There is at least one IF EXISTS check missing from pg_dump.c, see for example this statement from a dump of the regression database with --if-exists: ALTER TABLE public.nv_child_2010 DROP CONSTRAINT nv_child_2010_d_check; 2. Shouldn't pg_restore get --if-exists as well? 3. + printf(_( --if-exists don't report error if cleaned object doesn't exist\n)); This help output bleeds just over our de facto 80-character limit. Also contractions seem to be avoided elsewhere. It's a little hard to squeeze a decent explanation into one line, but perhaps: Use IF EXISTS when dropping objects would be better. The sgml changes could use some wordsmithing and grammar fixes. I could clean these up for you in a later version if you'd like. 4. There seem to be spurious whitespace changes to the function prototype and declaration for _printTocEntry. That's all I've had time for so far... Josh -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] proposal: fault tolerant DROP XXXX IF name
Hello I would to continue on implementation --if-exists option (using conditional DROP statements) for pg_dump related discussion http://www.postgresql.org/message-id/cafj8prdwqtxnc+reuavc4h5hx1f_0hkna-9f+2fgcu18axt...@mail.gmail.com Actually, we are not able to do simple implementation due double check (dependency check) in DROP STATEMENTS small example create or replace function fg() returns setof omega as $$ select * from omega $$ language sql; create or replace function fg(a omega) returns setof omega as $$ select * from omega $$ language sql; There is dependency fg function on table omega. Dump cleanup section: DROP FUNCTION IF EXISTS public.fg(a omega); DROP TABLE IF EXISTS public.omega; DROP EXTENSION IF EXISTS plpgsql; DROP SCHEMA IF EXISTS public; But DROP FUNCTION fails due missing table omega SET ERROR: type omega does not exist so we are not able to reload dump inside transaction. so my proposal: two ways * do conditional drops fully fault tolerant - it show only notice instead error or * decrease level of exceptions in conditional drops to WARNINGS Ideas, comments?? 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
[HACKERS] Re: [BUGS] BUG #7873: pg_restore --clean tries to drop tables that don't exist
Hello remastered patch still there is a issue with dependencies Regards Pavel Stehule 2013/6/17 Josh Kupershmidt schmi...@gmail.com: On Fri, Mar 8, 2013 at 11:58 AM, Pavel Stehule pavel.steh...@gmail.com wrote: I'll see - please, stay tuned to 9.4 first commitfest Hi Pavel, Just a reminder, I didn't see this patch in the current commitfest. I would be happy to spend some more time reviewing if you wish to pursue the patch. Josh conditional-drops.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] Randomisation for ensuring nlogn complexity in quicksort
On Tue, Jul 2, 2013 at 12:33 AM, Atri Sharma atri.j...@gmail.com wrote: 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. Right, thanks for that. I will keep that in mind. I was thinking about *mostly sorted* datasets, consider the following: 10 11 12 4 5 6 1 2 I think if you'll try it you'll find that we perform quite well on data sets of this kind - and if you read the code you'll see 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] Eliminating PD_ALL_VISIBLE, take 2
On 2013-07-01 19:52:57 -0700, Jeff Davis wrote:
2. Can you be more specific about the scenarios that you *are* concerned
about? Preferably in a form that could be tested on a 64-core box; but
at least some kind of analysis involving numbers. More page accesses
is scary, but completely meaningless without saying how *many* more and
in which situations.
Ok, so you want some benchmark results. I spent 20 minutes concocting some
quick tests. Here you go:
master (384f933046dc9e9a2b416f5f7b3be30b93587c63):
tps = 155075.448341 (including connections establishing)
tps = 155259.752267 (excluding connections establishing)
dev (384f933046dc9e9a2b416f5f7b3be30b93587c63 + patch):
tps = 151450.387021 (including connections establishing)
tps = 152512.741161 (excluding connections establishing)
That's about a 3% regression.
Testsetup:
~/build/postgres/{master,dev}-optimize/vpath/src/backend/postgres \
-D /srv/dev/pdav-{master,dev}/ \
-c shared_buffers=1GB
-c max_connections=150
Data loaded: load.sql.
Test run: SELECT key, data FROM kv WHERE key = 'key-17';
Test: pgbench postgres -n -S -M prepared -f /tmp/query.sql -T 100 -c 100 -j 100
So basically we're just scanning a smalling relation that's all visible
rather frequently. With lookup tables - that might even be accessed in a
correlated subquery - that's not a unrealistic scenario.
I am pretty sure it's not all that hard to create a test where the loss
is bigger due to the loss of all_visible on small relations (the
SCAN_VM_THRESHOLD thing).
I am not sure whether that's big enough to say the idea of
SCAN_VM_THRESHOLD is dead, but it's not small enough to dismiss either.
So, running the same test with 'kv' having 36 pages/5500 tuples instead
of just 1 page/100 tuples:
master:
tps = 171260.444722 (including connections establishing)
tps = 173335.031802 (excluding connections establishing)
dev:
tps = 170809.702066 (including connections establishing)
tps = 171730.291712 (excluding connections establishing)
~1%
With SELECT count(*) FROM kv;
master:
tps = 13740.652186 (including connections establishing)
tps = 13779.507250 (excluding connections establishing)
dev:
tps = 13409.639239 (including connections establishing)
tps = 13466.905051 (excluding connections establishing)
~2%.
All that isn't a too big regression, but it shows that this isn't free
lunch either. Would be interesting to see whether that shows up worse on
bigger hardware than mine (2 x E5520).
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] Eliminating PD_ALL_VISIBLE, take 2
On 2013-07-02 14:02:22 +0200, Andres Freund wrote: Data loaded: load.sql. Attached... Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services DROP TABLE IF EXISTS kv; CREATE TABLE kv( id serial primary key, key text NOT NULL UNIQUE, data text NOT NULL ); INSERT INTO kv(key, data) SELECT 'key-'||g.i::text, 'abcdefg' FROM generate_series(1, 100) g(i); VACUUM (ANALYZE, VERBOSE, FREEZE) kv; VACUUM (ANALYZE, VERBOSE) kv; -- 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 Tue, Jul 2, 2013 at 5:20 PM, Robert Haas robertmh...@gmail.com wrote: On Tue, Jul 2, 2013 at 12:33 AM, Atri Sharma atri.j...@gmail.com wrote: 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. Right, thanks for that. I will keep that in mind. I was thinking about *mostly sorted* datasets, consider the following: 10 11 12 4 5 6 1 2 I think if you'll try it you'll find that we perform quite well on data sets of this kind - and if you read the code you'll see why. Right, let me read the code again from that viewpoint. Thanks a ton for your help! 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] Review: query result history in psql
On 7/2/13 5:53 AM, Maciej Gajewski wrote: In the meantime, I've applied your suggestions and moved the sting-manipulating functions to stringutils. Also fixed a tiny bug. Patch attached. I haven't been able to find any real documentation on this feature, other than the additions to the psql help. Could someone write some mock documentation at least, so we know what the proposed interfaces and intended ways of use are? What is ans? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add an ldapoption to disable chasing LDAP referrals
On 7/2/13 12:20 AM, James Sewell wrote:
Hey All,
This patch request grew from this post (of mine) to pgsql-general:
http://www.postgresql.org/message-id/cabuevezouae-g1_oejagujjmem675dnystwybp4d_wz6om+...@mail.gmail.com
The patch adds another available LDAP option (ldapnochaseref) for
search+bind mode in the pg_hba.conf fil. If set to 1 (0 is default) then
it performs a ldap_set_option which disables chasing of any LDAP
references which are returned as part of the search LDIF.
This appears to be the same as the referrals option in pam_ldap
(http://linux.die.net/man/5/pam_ldap). So it seems legitimate.
For consistency, I would name the option ldapreferrals={0|1}. I prefer
avoiding double negatives.
Do you know of a standard way to represent this option in an LDAP URL,
perhaps as an extension?
--
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-07-01 15:02:41 -0400, Robert Haas wrote: * 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; + 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. Yes, I thought about that and I think it's a problem that can be solved without too ugly hacks. But, as you say: 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. I am pretty sure that we will have to fix more stuff, but luckily we're in the beginning of the cycle. And while I'd prefer more eyes on the patch before it gets applied, especially ones knowledgeable about the implications this has, I don't really see that happening soon. So applying is more likely to lead to more review than waiting. So, from me: +1. Some things that might be worth changing when committing: * Could you add a Assert(!RelationHasSysCache(relid)) to RelationInvalidatesSnapshotsOnly? It's not unlikely that it will be missed by the next person adding a syscache and that seems like it could have ugly and hard to detect consequences. * maybe use bsearch(3) instead of open coding the binary search? We already use it in the backend. * possibly paranoid, but I'd add a Assert to heap_beginscan_catalog or GetCatalogSnapshot ensuring we're dealing with a catalog relation. The consistency mechanisms in GetCatalogSnapshot() only work for those, so there doesn't seem to be a valid usecase for non-catalog relations. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for fail-back without fresh backup
On Tue, Jul 2, 2013 at 2:45 PM, Amit Kapila amit.kap...@huawei.com wrote: On Friday, June 28, 2013 10:41 AM Sawada Masahiko wrote: On Wed, Jun 26, 2013 at 1:40 PM, Amit Kapila amit.kap...@huawei.com wrote: On Tuesday, June 25, 2013 10:23 AM Amit Langote wrote: Hi, So our proposal on this problem is that we must ensure that master should not make any file system level changes without confirming that the corresponding WAL record is replicated to the standby. How will you take care of extra WAL on old master during recovery. If it plays the WAL which has not reached new-master, it can be a problem. I am trying to understand how there would be extra WAL on old master that it would replay and cause inconsistency. Consider how I am picturing it and correct me if I am wrong. 1) Master crashes. So a failback standby becomes new master forking the WAL. 2) Old master is restarted as a standby (now with this patch, without a new base backup). 3) It would try to replay all the WAL it has available and later connect to the new master also following the timeline switch (the switch might happen using archived WAL and timeline history file OR the new switch-over-streaming-replication-connection as of 9.3, right?) * in (3), when the new standby/old master is replaying WAL, from where is it picking the WAL? Yes, this is the point which can lead to inconsistency, new standby/old master will replay WAL after the last successful checkpoint, for which he get info from control file. It is picking WAL from the location where it was logged when it was active (pg_xlog). Does it first replay all the WAL in pg_xlog before archive? Should we make it check for a timeline history file in archive before it starts replaying any WAL? I have really not thought what is best solution for problem. * And, would the new master, before forking the WAL, replay all the WAL that is necessary to come to state (of data directory) that the old master was just before it crashed? I don't think new master has any correlation with old master's data directory, Rather it will replay the WAL it has received/flushed before start acting as master. when old master fail over, WAL which ahead of new master might be broken data. so that when user want to dump from old master, there is possible to fail dump. it is just idea, we extend parameter which is used in recovery.conf like 'follow_master_force'. this parameter accepts 'on' and 'off', is effective only when standby_mode is set to on. if both parameters 'follow_master_force' and 'standby_mode' is set to 'on', 1. when standby server starts and starts to recovery, standby server skip to apply WAL which is in pg_xlog, and request WAL from latest checkpoint LSN to master server. 2. master server receives LSN which is standby server latest checkpoint, and compare between LSN of standby and LSN of master latest checkpoint. if those LSN match, master will send WAL from latest checkpoint LSN. if not, master will inform standby that failed. 3. standby will fork WAL, and apply WAL which is sent from master continuity. Please consider if this solution has the same problem as mentioned by Robert Hass in below mail: http://www.postgresql.org/message-id/ca+tgmoy4j+p7jy69ry8gposmmdznyqu6dtionprcxavg+sp...@mail.gmail.com in this approach, user who want to dump from old master will set 'off' to follow_master_force and standby_mode, and gets the dump of old master after master started. OTOH, user who want to starts replication force will set 'on' to both parameter. I think before going into solution of this problem, it should be confirmed by others whether such a problem needs to be resolved as part of this patch. I have seen that Simon Riggs is a reviewer of this Patch and he hasn't mentioned his views about this problem. So I think it's not worth inventing a solution. Rather I think if all other things are resolved for this patch, then may be in end we can check with Committer, if he thinks that this problem needs to be solved as a separate patch. thank you for feedback. yes, we can consider separately those problem. and we need to judge that whether it is worth to invent a solution. I think that solving the fundamental of this problem is complex. it might be needs to big change to architecture of replication. so I'm thinking that I'd like to deal of something when we do recovery. if so, I think that if we deal at recovery time, impact to performance is ignored. Regards, --- Sawada Masahiko -- 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 for fail-back without fresh backup
On Tuesday, July 02, 2013 11:16 AM Amit Kapila wrote: On Friday, June 28, 2013 10:41 AM Sawada Masahiko wrote: On Wed, Jun 26, 2013 at 1:40 PM, Amit Kapila amit.kap...@huawei.com wrote: On Tuesday, June 25, 2013 10:23 AM Amit Langote wrote: Hi, So our proposal on this problem is that we must ensure that master should not make any file system level changes without confirming that the corresponding WAL record is replicated to the standby. How will you take care of extra WAL on old master during recovery. If it plays the WAL which has not reached new-master, it can be a problem. I am trying to understand how there would be extra WAL on old master that it would replay and cause inconsistency. Consider how I am picturing it and correct me if I am wrong. 1) Master crashes. So a failback standby becomes new master forking the WAL. 2) Old master is restarted as a standby (now with this patch, without a new base backup). 3) It would try to replay all the WAL it has available and later connect to the new master also following the timeline switch (the switch might happen using archived WAL and timeline history file OR the new switch-over-streaming-replication-connection as of 9.3, right?) * in (3), when the new standby/old master is replaying WAL, from where is it picking the WAL? Yes, this is the point which can lead to inconsistency, new standby/old master will replay WAL after the last successful checkpoint, for which he get info from control file. It is picking WAL from the location where it was logged when it was active (pg_xlog). Does it first replay all the WAL in pg_xlog before archive? Should we make it check for a timeline history file in archive before it starts replaying any WAL? I have really not thought what is best solution for problem. * And, would the new master, before forking the WAL, replay all the WAL that is necessary to come to state (of data directory) that the old master was just before it crashed? I don't think new master has any correlation with old master's data directory, Rather it will replay the WAL it has received/flushed before start acting as master. when old master fail over, WAL which ahead of new master might be broken data. so that when user want to dump from old master, there is possible to fail dump. it is just idea, we extend parameter which is used in recovery.conf like 'follow_master_force'. this parameter accepts 'on' and 'off', is effective only when standby_mode is set to on. if both parameters 'follow_master_force' and 'standby_mode' is set to 'on', 1. when standby server starts and starts to recovery, standby server skip to apply WAL which is in pg_xlog, and request WAL from latest checkpoint LSN to master server. 2. master server receives LSN which is standby server latest checkpoint, and compare between LSN of standby and LSN of master latest checkpoint. if those LSN match, master will send WAL from latest checkpoint LSN. if not, master will inform standby that failed. 3. standby will fork WAL, and apply WAL which is sent from master continuity. Please consider if this solution has the same problem as mentioned by Robert Hass in below mail: Sorry typo error, it's Robert Haas mail: http://www.postgresql.org/message- id/ca+tgmoy4j+p7jy69ry8gposmmdznyqu6dtionprcxavg+sp...@mail.gmail.com in this approach, user who want to dump from old master will set 'off' to follow_master_force and standby_mode, and gets the dump of old master after master started. OTOH, user who want to starts replication force will set 'on' to both parameter. I think before going into solution of this problem, it should be confirmed by others whether such a problem needs to be resolved as part of this patch. I have seen that Simon Riggs is a reviewer of this Patch and he hasn't mentioned his views about this problem. So I think it's not worth inventing a solution. Rather I think if all other things are resolved for this patch, then may be in end we can check with Committer, if he thinks that this problem needs to be solved as a separate patch. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] MVCC catalog access
On Tue, Jul 2, 2013 at 9:02 AM, Andres Freund and...@2ndquadrant.com wrote: Some things that might be worth changing when committing: * Could you add a Assert(!RelationHasSysCache(relid)) to RelationInvalidatesSnapshotsOnly? It's not unlikely that it will be missed by the next person adding a syscache and that seems like it could have ugly and hard to detect consequences. There's a cross-check in InitCatalogCache() for that very issue. * maybe use bsearch(3) instead of open coding the binary search? We already use it in the backend. I found comments elsewhere indicating that bsearch() was slower than open-coding it, so I copied the logic used for ScanKeywordLookup(). * possibly paranoid, but I'd add a Assert to heap_beginscan_catalog or GetCatalogSnapshot ensuring we're dealing with a catalog relation. The consistency mechanisms in GetCatalogSnapshot() only work for those, so there doesn't seem to be a valid usecase for non-catalog relations. It'll actually work find as things stand; it'll just take a new snapshot every time. I have a few ideas for getting rid of the remaining uses of SnapshotNow that I'd like to throw out there: - In pgrowlocks and pgstattuple, I think it would be fine to use SnapshotSelf instead of SnapshotNow. The only difference is that it includes changes made by the current command that wouldn't otherwise be visible until CommandCounterIncrement(). That, however, is not really a problem for their usage. - In genam.c and execUtils.c, we treat SnapshotNow as a kind of default snapshot. That seems like a crappy idea. I propose that we either set that pointer to NULL and let the server core dump if the snapshot doesn't get set or (maybe better) add a new special snapshot called SnapshotError which just errors out if you try to use it for anything, and initialize to that. - I'm not quite sure what to do about get_actual_variable_range(). Taking a new MVCC snapshot there seems like it might be pricey on some workloads. However, I wonder if we could use SnapshotDirty. Presumably, even uncommitted tuples affect the amount of index-scanning we have to do, so that approach seems to have some theoretical justification. But I'm worried there could be unfortunate consequences to looking at uncommitted data, either now or in the future. SnapshotSelf seems less likely to have that problem, but feels wrong somehow. - currtid_byreloid() and currtid_byrelname() use SnapshotNow as an argument to heap_get_latest_tid(). I don't know what these functions are supposed to be good for, but taking a new snapshot for every function call seems to guarantee that someone will find a way to use these functions as a poster child for how to brutalize PGXACT, so I don't particularly want to do that. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Custom gucs visibility
Hi, So, if I have a contrib module which adds in a custom guc via DefineCustomIntVariable() for example. Now that custom guc is not visible via a show command unless that corresponding .so has been loaded into memory. E.g. postgres=# show pg_buffercache.fancy_custom_guc; ERROR: unrecognized configuration parameter pg_buffercache.fancy_custom_guc Should we do something about this or we are ok with the current behavior? I would prefer to get to see the config parameter value if I so happen to want to see it before the load of that contrib module. Thoughts? Regards, Nikhils
Re: [HACKERS] MVCC catalog access
On 2013-07-02 09:31:23 -0400, Robert Haas wrote: On Tue, Jul 2, 2013 at 9:02 AM, Andres Freund and...@2ndquadrant.com wrote: Some things that might be worth changing when committing: * Could you add a Assert(!RelationHasSysCache(relid)) to RelationInvalidatesSnapshotsOnly? It's not unlikely that it will be missed by the next person adding a syscache and that seems like it could have ugly and hard to detect consequences. There's a cross-check in InitCatalogCache() for that very issue. Great. * maybe use bsearch(3) instead of open coding the binary search? We already use it in the backend. I found comments elsewhere indicating that bsearch() was slower than open-coding it, so I copied the logic used for ScanKeywordLookup(). Hm. Ok. * possibly paranoid, but I'd add a Assert to heap_beginscan_catalog or GetCatalogSnapshot ensuring we're dealing with a catalog relation. The consistency mechanisms in GetCatalogSnapshot() only work for those, so there doesn't seem to be a valid usecase for non-catalog relations. It'll actually work find as things stand; it'll just take a new snapshot every time. Ok. Doesn't really change my opinion that it's a crappy idea to use it otherwise ;) - In genam.c and execUtils.c, we treat SnapshotNow as a kind of default snapshot. That seems like a crappy idea. I propose that we either set that pointer to NULL and let the server core dump if the snapshot doesn't get set or (maybe better) add a new special snapshot called SnapshotError which just errors out if you try to use it for anything, and initialize to that. I vote for SnapshotError. - I'm not quite sure what to do about get_actual_variable_range(). Taking a new MVCC snapshot there seems like it might be pricey on some workloads. However, I wonder if we could use SnapshotDirty. Presumably, even uncommitted tuples affect the amount of index-scanning we have to do, so that approach seems to have some theoretical justification. But I'm worried there could be unfortunate consequences to looking at uncommitted data, either now or in the future. SnapshotSelf seems less likely to have that problem, but feels wrong somehow. I don't like using SnapshotDirty either. Can't we just use the currently active snapshot? Unless I miss something this always will be called while we have one since when we plan we've done an explicit PushActiveSnapshot() and if we need to replan stuff during execution PortalRunSelect() will have pushed one. - currtid_byreloid() and currtid_byrelname() use SnapshotNow as an argument to heap_get_latest_tid(). I don't know what these functions are supposed to be good for, but taking a new snapshot for every function call seems to guarantee that someone will find a way to use these functions as a poster child for how to brutalize PGXACT, so I don't particularly want to do that. Heikki mentioned that at some point they were added for the odbc driver. I am not particularly inclined to worry about taking too many snapshots here. 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] New regression test time
Reviewing this thread, I think that the following people are in favor of adding the tests to the existing schedule: Josh Berkus, Stephen Frost, Fabien Coelho, Dann Corbit, and Jeff Janes. And I think that the following people are in favor of a new schedule: Andres Freund, Andrew Dunstan, David Fetter, and possibly Alvaro Herrera. I believe Tom Lane has also expressed objections, though not on this thread. So I think the first question we need to answer is: Should we just reject Robins' patches en masse? If we do that, then the rest of this is moot. If we don't do that, then the second question is whether we should try to introduce a new schedule, and if so, whether we should split out that new schedule before or after committing these patches as they stand. Here are my opinions, for what they are worth. First, I think that rejecting these new tests is a bad idea, although I've looked them over a bit and I think there might be individual things we might want to take out. Second, I think that creating a new schedule is likely to cost developers more time than it saves them. Sure, you'll be able to run the tests slightly faster, but when you commit something, break the buildfarm (which does run those tests), and then have to go back and fix the tests (or your patch), you'll waste more time doing that than you saved by avoiding those few extra seconds of runtime. Third, I think if we do want to create a new schedule, it makes more sense to commit the tests first and then split out the new schedule according to some criteria that we devise. There should be a principled reason for putting tests in one schedule or the other; all the tests that Robins Tharakan wrote is not a good filter criteria. I'm willing to put effort into going through these patches and figuring out which parts are worth committing, and commit them. However, I don't want to (and should not) do that if the consensus is to reject the patches altogether; or if people are not OK with the approach proposed above, namely, commit it first and then, if it causes problems, decide how to fix it. Please help me understand what the way forward is for this patch set. Thanks, -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add regression tests for DISCARD
On Mon, Jul 1, 2013 at 5:59 PM, Robins Tharakan thara...@gmail.com wrote: 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, You must add this new test case called discard to src/test/regress/parallel_schedule and src/test/regress/serial_schedule, because if we do make check the new discard test case is not executed. Regards, -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Blog sobre TI: http://fabriziomello.blogspot.com Perfil Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello
Re: [HACKERS] Large object + FREEZE?
On Tue, Jul 2, 2013 at 12:16:18PM +0900, Tatsuo Ishii wrote: Now that we have COPY FREEZE, I'm thinking about adding similar option to creating large objects. In 9.3 the maximum size of large objects are increased. That means, the first access to a large object will trigger more writes because of hint bit updation. Also subsequent VACUUM may trigger that as well. If we could freeze arge objects while creating, it could reduce the writes dramatically as COPY FREEZE already does. I was not aware that large objects had to feeze each 8k segment. Do they? -- 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] MVCC catalog access
On Tue, Jul 2, 2013 at 9:52 AM, Andres Freund and...@2ndquadrant.com wrote: * possibly paranoid, but I'd add a Assert to heap_beginscan_catalog or GetCatalogSnapshot ensuring we're dealing with a catalog relation. The consistency mechanisms in GetCatalogSnapshot() only work for those, so there doesn't seem to be a valid usecase for non-catalog relations. It'll actually work find as things stand; it'll just take a new snapshot every time. Ok. Doesn't really change my opinion that it's a crappy idea to use it otherwise ;) I agree, but I don't see an easy way to write the assertion you want using only the OID. - In genam.c and execUtils.c, we treat SnapshotNow as a kind of default snapshot. That seems like a crappy idea. I propose that we either set that pointer to NULL and let the server core dump if the snapshot doesn't get set or (maybe better) add a new special snapshot called SnapshotError which just errors out if you try to use it for anything, and initialize to that. I vote for SnapshotError. OK. - I'm not quite sure what to do about get_actual_variable_range(). Taking a new MVCC snapshot there seems like it might be pricey on some workloads. However, I wonder if we could use SnapshotDirty. Presumably, even uncommitted tuples affect the amount of index-scanning we have to do, so that approach seems to have some theoretical justification. But I'm worried there could be unfortunate consequences to looking at uncommitted data, either now or in the future. SnapshotSelf seems less likely to have that problem, but feels wrong somehow. I don't like using SnapshotDirty either. Can't we just use the currently active snapshot? Unless I miss something this always will be called while we have one since when we plan we've done an explicit PushActiveSnapshot() and if we need to replan stuff during execution PortalRunSelect() will have pushed one. We could certainly do that, but I'd be a bit reluctant to do so without input from Tom. I imagine there might be cases where it could cause a regression. - currtid_byreloid() and currtid_byrelname() use SnapshotNow as an argument to heap_get_latest_tid(). I don't know what these functions are supposed to be good for, but taking a new snapshot for every function call seems to guarantee that someone will find a way to use these functions as a poster child for how to brutalize PGXACT, so I don't particularly want to do that. Heikki mentioned that at some point they were added for the odbc driver. I am not particularly inclined to worry about taking too many snapshots here. Well, if it uses it with any regularity I think it's a legitimate concern. We have plenty of customers who use ODBC and some of them allow frightening numbers of concurrent server connections. Now you may say that's a bad idea, but so is 1000 backends doing txid_current() in a loop. -- 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] New regression test time
On 07/02/2013 10:17 AM, Robert Haas wrote: Reviewing this thread, I think that the following people are in favor of adding the tests to the existing schedule: Josh Berkus, Stephen Frost, Fabien Coelho, Dann Corbit, and Jeff Janes. And I think that the following people are in favor of a new schedule: Andres Freund, Andrew Dunstan, David Fetter, and possibly Alvaro Herrera. I believe Tom Lane has also expressed objections, though not on this thread. So I think the first question we need to answer is: Should we just reject Robins' patches en masse? If we do that, then the rest of this is moot. If we don't do that, then the second question is whether we should try to introduce a new schedule, and if so, whether we should split out that new schedule before or after committing these patches as they stand. Here are my opinions, for what they are worth. First, I think that rejecting these new tests is a bad idea, although I've looked them over a bit and I think there might be individual things we might want to take out. Second, I think that creating a new schedule is likely to cost developers more time than it saves them. Sure, you'll be able to run the tests slightly faster, but when you commit something, break the buildfarm (which does run those tests), and then have to go back and fix the tests (or your patch), you'll waste more time doing that than you saved by avoiding those few extra seconds of runtime. Third, I think if we do want to create a new schedule, it makes more sense to commit the tests first and then split out the new schedule according to some criteria that we devise. There should be a principled reason for putting tests in one schedule or the other; all the tests that Robins Tharakan wrote is not a good filter criteria. I'm willing to put effort into going through these patches and figuring out which parts are worth committing, and commit them. However, I don't want to (and should not) do that if the consensus is to reject the patches altogether; or if people are not OK with the approach proposed above, namely, commit it first and then, if it causes problems, decide how to fix it. Please help me understand what the way forward is for this patch set. I think I'm probably a bit misrepresented here. The question of what tests we have is distinct from the question of what schedule(s) they are in. We already have tests that are in NO schedule, IIRC. What is more, it's entirely possibly to invoke pg_regress with multiple --schedule arguments, so we could, for example, have a makefile target that would run both the check and some other schedule of longer running tests. So my $0.02 says you should assess these tests on their own merits, and we can debate the schedule arrangements later. 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] [9.4 CF 1] The Commitfest Slacker List
On Tue, Jul 2, 2013 at 10:52:26AM +0200, Michael Meskes wrote: Sorry for joining the thread this late, but I didn't really expect to see myself listed as a slacker on a public list. Additionally, the following committers are not listed as reviewers on any patch. Note that I have no way to search which ones might be *committers* on a patch, so these folks are not necessarily slackers This means I'm a slacker because I'm not reviewing or committing a patch, right? Do we have a written rule somewhere? Or some other communication about this? I would have liked to know this requirement before getting singled out in public. Also I'd like to know who made the decision to require a patch review from each committer as I certainly missed it. Was the process public? Where can I find more about it? In general I find it difficult to digest that other people make decisions about my spare time without me having a word in the discussion. I understand. You could wear slacker as a badge of honor: ;-) http://momjian.us/main/img/main/slacker.jpg -- 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] signed vs. unsigned in plpy_procedure.c
On 02.07.2013 13:39, Andres Freund wrote: src/postgresql/src/pl/plpython/plpy_procedure.c: In function ‘PLy_procedure_munge_source’: src/postgresql/src/pl/plpython/plpy_procedure.c:507:2: warning: comparison of unsigned expression= 0 is always true [-Wtype-limits] Assert(plen= 0 plen mlen); Which has a point, we assign sprintf()s result to a size_t and then check for a negative value. Which doesn't work. Obviously the impact is miniscule, but removing legitimate warnings is a good thing. Trivial patch attached. Thanks, applied. - 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] Large object + FREEZE?
On Mon, Jul 1, 2013 at 11:16 PM, Tatsuo Ishii is...@postgresql.org wrote: Now that we have COPY FREEZE, I'm thinking about adding similar option to creating large objects. In 9.3 the maximum size of large objects are increased. That means, the first access to a large object will trigger more writes because of hint bit updation. Also subsequent VACUUM may trigger that as well. If we could freeze arge objects while creating, it could reduce the writes dramatically as COPY FREEZE already does. Opinions? COPY FREEZE only works if the table was created in the same transaction. Otherwise, an error midway through could leave the table and index inconsistent. I think that's a killer for this proposal, because obviously when a large object is created, pg_largeobject will be pre-existing. -- 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] Custom gucs visibility
On Tue, Jul 2, 2013 at 9:32 AM, Nikhil Sontakke nikkh...@gmail.com wrote: So, if I have a contrib module which adds in a custom guc via DefineCustomIntVariable() for example. Now that custom guc is not visible via a show command unless that corresponding .so has been loaded into memory. E.g. postgres=# show pg_buffercache.fancy_custom_guc; ERROR: unrecognized configuration parameter pg_buffercache.fancy_custom_guc Should we do something about this or we are ok with the current behavior? I would prefer to get to see the config parameter value if I so happen to want to see it before the load of that contrib module. Thoughts? If we haven't loaded the .so yet, where would we get the list of custom GUCs from? -- 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] refresh materialized view concurrently
On Tue, Jul 2, 2013 at 4:02 AM, Hitoshi Harada umi.tan...@gmail.com wrote: Other than these, I've found index is opened with NoLock, relying on ExclusiveLock of parent matview, and ALTER INDEX SET TABLESPACE or something similar can run concurrently, but it is presumably safe. DROP INDEX, REINDEX would be blocked by the ExclusiveLock. I doubt very much that this is safe. And even if it is safe today, I think it's a bad idea, because we're likely to try to reduce lock levels in the future. Taking no lock on a relation we're opening, even an index, seems certain to be a bad idea. -- 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-07-01 17:46:51 -0400, Robert Haas wrote:
But backing up a minute, this is really a significant behavior change
that is independent of the purpose of the rest of this patch. What
you're proposing here is that every time we consider toasting a value
on update, we should first check whether it's byte-for-byte equivalent
to the old value. That may or may not be a good idea - it will suck
if, for example, a user repeatedly updates a very long string by
changing only the last character thereof, but it will win in other
cases.
In that case the old value will rather likely just have been read just
before, so the price of rereading should be relatively low.
But:
Whether it's a good idea or not, I think it deserves to be a
separate patch. For purposes of this patch, I think you should just
assume that any external-indirect value needs to be retoasted, just as
we currently assume for untoasted values.
is a rather valid point. I've split the patch accordingly. The second
patch is *not* supposed to be applied together with patch 1 but rather
included for reference.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
From 9f69c755c4e69495847138e3b7ce47dee78b6eb0 Mon Sep 17 00:00:00 2001
From: Andres Freund and...@anarazel.de
Date: Tue, 11 Jun 2013 23:25:26 +0200
Subject: [PATCH 1/2] 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.4
---
src/backend/access/heap/tuptoaster.c | 110 ++---
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, 472 insertions(+), 43 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..e759502 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 is mainly used when we need
+ * to persist a Datum for unusually long time, like in a HOLD cursor.
+ */
+ struct varatt_indirect redirect;
+ VARATT_EXTERNAL_GET_POINTER(redirect, attr);
+ attr = (struct varlena *)redirect.pointer;
+
+ /* nested indirect Datums aren't allowed */
+ Assert(!VARATT_IS_EXTERNAL_INDIRECT(attr));
+
+ /* doesn't make much sense, but better handle it */
+ if
Re: [HACKERS] MVCC catalog access
On 2013-07-02 10:38:17 -0400, Robert Haas wrote:
On Tue, Jul 2, 2013 at 9:52 AM, Andres Freund and...@2ndquadrant.com wrote:
* possibly paranoid, but I'd add a Assert to heap_beginscan_catalog or
GetCatalogSnapshot ensuring we're dealing with a catalog relation. The
consistency mechanisms in GetCatalogSnapshot() only work for those, so
there doesn't seem to be a valid usecase for non-catalog relations.
It'll actually work find as things stand; it'll just take a new
snapshot every time.
Ok. Doesn't really change my opinion that it's a crappy idea to use it
otherwise ;)
I agree, but I don't see an easy way to write the assertion you want
using only the OID.
Let's add
/*
* IsSystemRelationId
* True iff the relation is a system catalog relation.
*/
bool
IsSystemRelationId(Oid relid)
{
return relid FirstNormalObjectId;
}
and change IsSystemRelation() to use that instead of what it does now...
- currtid_byreloid() and currtid_byrelname() use SnapshotNow as an
argument to heap_get_latest_tid(). I don't know what these functions
are supposed to be good for, but taking a new snapshot for every
function call seems to guarantee that someone will find a way to use
these functions as a poster child for how to brutalize PGXACT, so I
don't particularly want to do that.
Heikki mentioned that at some point they were added for the odbc
driver. I am not particularly inclined to worry about taking too many
snapshots here.
Well, if it uses it with any regularity I think it's a legitimate
concern. We have plenty of customers who use ODBC and some of them
allow frightening numbers of concurrent server connections.
I've quickly verified that it indeed uses them. I wish I hadn't. Brrr. I
can't even guess what that should do from the surrounding code/function
names. Except that it looks broken under concurrency as long as
SnapshotNow is used (because the query's snapshot won't be as new as
SnapshotNow, even in read committed mode).
Heikki, do you understand the code well enough to explain it without
investing time?
Now you may say that's a bad idea, but so is 1000 backends doing
txid_current() in a loop.
Hehe ;).
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] New regression test time
What is more, it's entirely possibly to invoke pg_regress with multiple --schedule arguments, so we could, for example, have a makefile target that would run both the check and some other schedule of longer running tests. I missed this fact, because I've not seen any example of multiple schedule option in regress, so in the split test POC I concatenated files for the same purpose, but multiple schedule options looks like a much better alternative. -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Custom gucs visibility
Robert Haas robertmh...@gmail.com writes: On Tue, Jul 2, 2013 at 9:32 AM, Nikhil Sontakke nikkh...@gmail.com wrote: Should we do something about this or we are ok with the current behavior? I would prefer to get to see the config parameter value if I so happen to want to see it before the load of that contrib module. Thoughts? If we haven't loaded the .so yet, where would we get the list of custom GUCs from? This has come up before. We could show the string value of the GUC, if it's been set in postgresql.conf, but we do not have correct values for any of the other columns in pg_settings; nor are we even sure that the module will think the value is valid once it does get loaded. So the consensus has been that allowing the GUC to be printed would be more misleading than helpful. 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] Randomisation for ensuring nlogn complexity in quicksort
On Tue, Jul 2, 2013 at 5:04 AM, Atri Sharma atri.j...@gmail.com wrote: I think if you'll try it you'll find that we perform quite well on data sets of this kind - and if you read the code you'll see why. Right, let me read the code again from that viewpoint. In my opinion, it would be worthwhile reading the original Bentley and McIlroy paper [1] and using what you learned to write a patch that adds comments throughout the canonical qsort_arg, and perhaps the other variants. [1] http://www.enseignement.polytechnique.fr/informatique/profs/Luc.Maranget/421/09/bentley93engineering.pdf -- 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] MVCC catalog access
On 02.07.2013 18:24, Andres Freund wrote: I've quickly verified that it indeed uses them. I wish I hadn't. Brrr. I can't even guess what that should do from the surrounding code/function names. Except that it looks broken under concurrency as long as SnapshotNow is used (because the query's snapshot won't be as new as SnapshotNow, even in read committed mode). Heikki, do you understand the code well enough to explain it without investing time? No, sorry. I think it has something to do with updateable cursors, but I don't understand the details. - 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] Randomisation for ensuring nlogn complexity in quicksort
On Tue, Jul 2, 2013 at 12:36 PM, Peter Geoghegan p...@heroku.com wrote: On Tue, Jul 2, 2013 at 5:04 AM, Atri Sharma atri.j...@gmail.com wrote: I think if you'll try it you'll find that we perform quite well on data sets of this kind - and if you read the code you'll see why. Right, let me read the code again from that viewpoint. In my opinion, it would be worthwhile reading the original Bentley and McIlroy paper [1] and using what you learned to write a patch that adds comments throughout the canonical qsort_arg, and perhaps the other variants. [1] http://www.enseignement.polytechnique.fr/informatique/profs/Luc.Maranget/421/09/bentley93engineering.pdf That's weird, it doesn't seem as sophisticated as even libc's introsort. Perhaps an introsort[1] approach wouldn't hurt: do the quick and dirty median selection pg is already doing (or a better one if a better one is found), but check recursion depth/input size ratios. When across K recursive calls the input set hasn't been halved in size, switch to median of medians to guard off against quadratic complexity. [1] http://en.wikipedia.org/wiki/Selection_algorithm#Introselect -- 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] Custom gucs visibility
On Tue, Jul 02, 2013 at 11:37:13AM -0400, Tom Lane wrote: Robert Haas robertmh...@gmail.com writes: On Tue, Jul 2, 2013 at 9:32 AM, Nikhil Sontakke nikkh...@gmail.com wrote: Should we do something about this or we are ok with the current behavior? I would prefer to get to see the config parameter value if I so happen to want to see it before the load of that contrib module. Thoughts? If we haven't loaded the .so yet, where would we get the list of custom GUCs from? This has come up before. We could show the string value of the GUC, if it's been set in postgresql.conf, but we do not have correct values for any of the other columns in pg_settings; nor are we even sure that the module will think the value is valid once it does get loaded. So the consensus has been that allowing the GUC to be printed would be more misleading than helpful. How about printing them with something along the lines of, Please load extension foobar for details or (less informative, but possibly easier to code) libfoobar.so not loaded. ? 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] Large object + FREEZE?
On Mon, Jul 1, 2013 at 11:16 PM, Tatsuo Ishii is...@postgresql.org wrote: Now that we have COPY FREEZE, I'm thinking about adding similar option to creating large objects. In 9.3 the maximum size of large objects are increased. That means, the first access to a large object will trigger more writes because of hint bit updation. Also subsequent VACUUM may trigger that as well. If we could freeze arge objects while creating, it could reduce the writes dramatically as COPY FREEZE already does. Opinions? COPY FREEZE only works if the table was created in the same transaction. Otherwise, an error midway through could leave the table and index inconsistent. I think that's a killer for this proposal, because obviously when a large object is created, pg_largeobject will be pre-existing. Argh. You are right. -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese: http://www.sraoss.co.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] XLogInsert scaling, revisited
On 27.06.2013 15:27, Andres Freund wrote: Hi, 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. Yes. Earlieron that was explained in XLogInsert() - maybe point to the documentation ontop of the file? The file is too big to expect everyone to reread the whole file in a new release... * The read of Insert-RedoRecPtr while rechecking whether it's out of date now is unlocked, is that correct? And why? Same here, XLogInsert holds the slot while rechecking Insert-RedoRecptr. I was wondering whether its guaranteed that we don't read a cached value, but I didn't think of the memory barriers due to the spinlock in Release/AcquireSlot. I think I thought that release wouldn't acquire the spinlock at all. * Have you measured whether it works to just keep as many slots as max_backends requires around and not bothering with dynamically allocating them to inserters? That seems to require to keep actually waiting slots in a separate list which very well might make that too expensive. Correctness wise the biggest problem for that probably is exlusive acquiration of all slots CreateCheckpoint() does... Hmm. It wouldn't be much different, each backend would still need to reserve its own dedicated slot, because it might be held by the a backend that grabbed all the slots. Also, bgwriter and checkpointer need to flush the WAL, so they'd need slots too. More slots make WaitXLogInsertionsToFinish() more expensive, as it has to loop through all of them. IIRC some earlier pgbench tests I ran didn't show much difference in performance, whether there were 40 slots or 100, as long as there was enough of them. I can run some more tests with even more slots to see how it behaves. In a very quick test I ran previously I did see the slot acquiration in the profile when using short connections that all did some quick inserts - which isn't surprising since they tend to all start with the same slot so they will all fight for the first slots. Maybe we could ammeliorate that by initializing MySlotNo to (MyProc-pgprocno % num_xloginsert_slots) when uninitialized? That won't be completely fair since the number of procs won't usually be a multiple of num_insert_slots, but I doubt that will be problematic. * What about using some sort of linked list of unused slots for WALInsertSlotAcquireOne? Everytime we're done we put it to the *end* of the list so it's less likely to have been grabbed by somebody else so we can reuse it. a) To grab a new one go to the head of the linked list spinlock it and recheck whether it's still free. If not, restart. Otherwise, remove from list. b) To reuse a previously used slot That way we only have to do the PGSemaphoreLock() dance if there really aren't any free slots. That adds a spinlock acquisition / release into the critical path, to protect the linked list. I'm trying very hard to avoid serialization points like that. Hm. We already have at least one spinlock in that path? Or are you thinking of a global one protecting the linked list? If so, I think we should be able to get away with locking individual slots. IIRC if you never need to delete list elements you can even get away with a lockless implementation. While profiling the tests I've done, finding a free slot hasn't shown up much. So I don't think it's a problem performance-wise as it is, and I don't think it would make the code simpler. It sure wouldn't make it simpler. As I said above, I saw the slot acquiration in a profile when using -C and a short pgbench script (that just inserts 10 rows). * The queuing logic around slots seems to lack documentation. It's complex enough to warrant that imo. Yeah, it's complex. I expanded the comments above XLogInsertSlot, to explain how xlogInsertingAt works. Did that help, or was it some other part of that that needs more docs? What I don't understand so far is why we don't reset xlogInsertingAt during SlotReleaseOne. There are places documenting that we don't do so, but not why unless I missed it. We could reset it in SlotReleaseOne. However, the next inserter that
Re: [HACKERS] Eliminating PD_ALL_VISIBLE, take 2
On Tue, 2013-07-02 at 14:02 +0200, Andres Freund wrote: Ok, so you want some benchmark results. I spent 20 minutes concocting some quick tests. Here you go: master (384f933046dc9e9a2b416f5f7b3be30b93587c63): tps = 155075.448341 (including connections establishing) tps = 155259.752267 (excluding connections establishing) dev (384f933046dc9e9a2b416f5f7b3be30b93587c63 + patch): tps = 151450.387021 (including connections establishing) tps = 152512.741161 (excluding connections establishing) That's about a 3% regression. I had a little trouble reproducing this result on my workstation, and my previous tests on the 64-core box didn't seem to show a difference (although I didn't spend a lot of time on it, so perhaps I could try again). I did see some kind of difference, I think. But the fastest run without the patch beat the slowest run with the patch by about 1.4%. The delta generally seemed closer to 0.5%. The noise seemed to be around 2.6%. Why did you do this as a concurrent test? The difference between reading hints and PD_ALL_VISIBLE doesn't seem to have much to do with concurrency. Regardless, this is at least a concrete issue that I can focus on, and I appreciate that. Are scans of small tables the primary objection to this patch, or are there others? If I solve it, will this patch make real progress? 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] XLogInsert scaling, revisited
On 2013-07-02 19:48:40 +0300, Heikki Linnakangas wrote: If so, why isn't it sufficient to initialize it in ReserveXLogInsertLocation? It would be, but then ReserveXLogInsertLocation would need to hold the slot's spinlock at the same time with insertpos_lck, so that it could atomically read the current CurrBytePos value and copy it to xlogInsertingAt. It's important to keep ReserveXLogInsertLocation() as lightweight as possible, to maximize concurrency. If you make it so that you always acquire the slot's spinlock first and insertpos_lck after, the scalability shouldn't be any different from now? Both the duration during which insertpos_lck is held and the overall amount of atomic ops should be the same? 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] [GENERAL] Floating point error
Abhijit Menon-Sen escribió: At 2013-06-24 10:16:33 +, laurenz.a...@wien.gv.at wrote: What do you think of the attached version? I'm not exactly fond of it, but I can't come up with a better version either. It's slightly better if but may not accurately represent the stored value is removed. Does anyone else have suggestions? Pushed backpatched. If there are more suggestions, I'm happy to tweak it. -- Á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] Eliminating PD_ALL_VISIBLE, take 2
On 2013-07-02 10:12:31 -0700, Jeff Davis wrote: On Tue, 2013-07-02 at 14:02 +0200, Andres Freund wrote: Ok, so you want some benchmark results. I spent 20 minutes concocting some quick tests. Here you go: master (384f933046dc9e9a2b416f5f7b3be30b93587c63): tps = 155075.448341 (including connections establishing) tps = 155259.752267 (excluding connections establishing) dev (384f933046dc9e9a2b416f5f7b3be30b93587c63 + patch): tps = 151450.387021 (including connections establishing) tps = 152512.741161 (excluding connections establishing) That's about a 3% regression. I had a little trouble reproducing this result on my workstation, and my previous tests on the 64-core box didn't seem to show a difference (although I didn't spend a lot of time on it, so perhaps I could try again). I did see some kind of difference, I think. But the fastest run without the patch beat the slowest run with the patch by about 1.4%. The delta generally seemed closer to 0.5%. The noise seemed to be around 2.6%. It was more reliably reproduceable here, I ran every test 5 times and the differences between runs weren't big. But I wouldn't be surprised if it's dependent on the exact CPU. Why did you do this as a concurrent test? The difference between reading hints and PD_ALL_VISIBLE doesn't seem to have much to do with concurrency. Primarily because I didn't spend too much time on it and just wanted to get a feeling for things. I originally wanted to check how much the additional pin/buffer reference on a small table (i.e. ~33+ pages) is noticeable, but noticed this before. Also, cache issues are often easier to notice in concurrent scenarios where the CPUs are overcommitted since increased cache usage shows up more prominently and intelligence on the cpu level can save less. Regardless, this is at least a concrete issue that I can focus on, and I appreciate that. Are scans of small tables the primary objection to this patch, or are there others? If I solve it, will this patch make real progress? Well, I still have my doubts that it's a good idea to remove this knowledge from the page level. And that's not primarily related to performance. Unfortunately a good part of judging architectural questions is gut feeling... The only real argument for the removal I see - removal of one cycle of dirtying - wouldn't really weigh much if we would combine it with freezing (which we can do if Robert's forensic freezing patch makes it in). 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] extensible external toast tuple support
On Tue, Jul 2, 2013 at 11:06 AM, Andres Freund and...@2ndquadrant.com wrote: In that case the old value will rather likely just have been read just before, so the price of rereading should be relatively low. Maybe in terms of I/O, but there's still CPU. is a rather valid point. I've split the patch accordingly. The second patch is *not* supposed to be applied together with patch 1 but rather included for reference. OK, committed patch 1. -- 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] New regression test time
On Tue, Jul 02, 2013 at 10:17:08AM -0400, Robert Haas wrote: So I think the first question we need to answer is: Should we just reject Robins' patches en masse? If we do that, then the rest of this is moot. If we don't do that, then the second question is whether we should try to introduce a new schedule, and if so, whether we should split out that new schedule before or after committing these patches as they stand. It's sad to simply reject meaningful automated tests on the basis of doubt that they're important enough to belong in every human-in-the-loop test run. I share the broader vision for automated testing represented by these patches. Here are my opinions, for what they are worth. First, I think that rejecting these new tests is a bad idea, although I've looked them over a bit and I think there might be individual things we might want to take out. Second, I think that creating a new schedule is likely to cost developers more time than it saves them. Sure, you'll be able to run the tests slightly faster, but when you commit something, break the buildfarm (which does run those tests), and then have to go back and fix the tests (or your patch), you'll waste more time doing that than you saved by avoiding those few extra seconds of runtime. Third, I think if we do want to create a new schedule, it makes more sense to commit the tests first and then split out the new schedule according to some criteria that we devise. There should be a principled reason for putting tests in one schedule or the other; all the tests that Robins Tharakan wrote is not a good filter criteria. +1 for that plan. I don't know whether placing certain tests outside the main test sequence would indeed cost more time than it saves. I definitely agree that if these new tests should appear elsewhere, some of our existing tests should also move there. Thanks, nm -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch: clean up addRangeTableEntryForFunction
On Tue, Jan 22, 2013 at 11:45 AM, David Fetter da...@fetter.org wrote: I've been working with Andrew Gierth (well, mostly he's been doing the work, as usual) to add WITH ORDINALITY as an option for set-returning functions. In the process, he found a minor opportunity to clean up the interface for $SUBJECT, reducing the call to a Single Point of Truth for lateral-ness, very likely improving the efficiency of calls to that function. Please find attached the patch. I think this patch is utterly pointless. I recommend we reject it. If this were part of some larger refactoring that was going in some direction we could agree on, it might be worth it. But as it is, I think it's just a shot in the dark whether this change will end up being better or worse, and my personal bet is that it won't make any difference whatsoever. -- 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] Eliminating PD_ALL_VISIBLE, take 2
On Tue, 2013-07-02 at 19:34 +0200, Andres Freund wrote: Well, I still have my doubts that it's a good idea to remove this knowledge from the page level. And that's not primarily related to performance. Unfortunately a good part of judging architectural questions is gut feeling... The only real argument for the removal I see - removal of one cycle of dirtying - wouldn't really weigh much if we would combine it with freezing (which we can do if Robert's forensic freezing patch makes it in). I'll have to take a closer look at the relationship between Robert's and Heikki's proposals. I have a gut feeling that the complexity we go through to maintain PD_ALL_VISIBLE is unnecessary and will cause us problems later. If we could combine freezing and marking all-visible, and use WAL for PD_ALL_VISIBLE in a normal fashion, then I'd be content with that. Even better would be if we could eliminate the freeze I/O with Heikki's proposal, and eliminate the PD_ALL_VISIBLE I/O with my patch. But, that's easier said than done. 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] [9.4 CF 1] The Commitfest Slacker List
On 07/02/2013 11:30 AM, Tatsuo Ishii wrote: Folks, For 9.2, we adopted it as policy that anyone submitting a patch to a commitfest is expected to review at least one patch submitted by someone else. And that failure to do so would affect the attention your patches received in the future. For that reason, I'm publishing the list below of people who submitted patches and have not yet claimed any patch in the commitfest to review. For those of you who are contributing patches for your company, please let your boss know that reviewing is part of contributing, and that if you don't do the one you may not be able to do the other. The two lists below, idle submitters and committers, constitutes 26 people. This *outnumbers* the list of contributors who are busy reviewing patches -- some of them four or more patches. If each of these people took just *one* patch to review, it would almost entirely clear the list of patches which do not have a reviewer. If these folks continue not to do reviews, this commitfest will drag on for at least 2 weeks past its closure date. Andrew Geirth Nick White Peter Eisentrout Alexander Korotkov Etsuro Fujita Hari Babu Jameison Martin Jon Nelson Oleg Bartunov Chris Farmiloe Samrat Revagade Alexander Lakhin Mark Kirkwood Liming Hu Maciej Gajewski Josh Kuperschmidt Mark Wong Gurjeet Singh Robins Tharakan Tatsuo Ishii Karl O Pinc It took me for while before realizing that I am on the list because I posted this: http://www.postgresql.org/message-id/20130610.091605.250603479334631505.t-is...@sraoss.co.jp Because I did not register the patch into CF page myself. I should have not posted it until I find any patch which I can take care of. Hi Tatsuo-san I guess whoever registered it with CF should also take your place on the slackers list ;) Regards Hannu Krosing PS. I am also currently witholding a patch from CF for the same reason Sorry for this. -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese: http://www.sraoss.co.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: Patch to compute Max LSN of Data Pages
On Tue, Jun 25, 2013 at 1:42 PM, Andres Freund and...@2ndquadrant.com wrote:
I think the usecase for this utility isn't big enough to be included in
postgres since it really can only help in a very limited
circumstances. And I think it's too likely to be misused for stuff it's
not useable for (e.g. remastering).
The only scenario I see is that somebody deleted/corrupted
pg_controldata. In that scenario the tool is supposed to be used to find
the biggest lsn used so far so the user then can use pg_resetxlog to set
that as the wal starting point.
But that can be way much easier solved by just setting the LSN to
something very, very high. The database cannot be used for anything
reliable afterwards anyway.
I guess this is true, but I think I'm mildly in favor of including
this anyway. I think I would have used it once or twice, if it had
been there - maybe not even to feed into pg_resetxlog, but just to
check for future LSNs. We don't have anything like a suite of
diagnostic tools in bin or contrib today, for use by database
professionals in fixing things that fall strictly in the realm of
don't try this at home, and I think we should have such a thing.
Unfortunately this covers about 0.1% of the space I'd like to see
covered, which might be a reason to reject it or at least insist that
it be enhanced first.
At any rate, I don't think this is anywhere near committable as it
stands today. Some random review comments:
remove_parent_refernces() is spelled wrong.
Why does this patch need all of this fancy path-handling logic -
specifically, remove_parent_refernces() and make_absolute_path()?
Surely its needs are not that different from pg_ctl or pg_resetxlog or
pg_controldata. If they all need it and it's duplicated, we should
fix that. Otherwise, why the special logic here?
I don't think we need getLinkPath() either. The server has no trouble
finding its files by just using a pathname that follows the symlink.
Why do we need anything different here? The whole point of symlinks
is that you can traverse them transparently, without knowing where
they point.
Duplicating PageHeaderIsValid doesn't seem acceptable. Moreover,
since the point of this is to be able to use it on a damaged cluster,
why is that logic even desirable? I think we'd be better off assuming
the pages to be valid.
The calling convention for this utility seems quite baroque. There's
no real need for all of this -p/-P stuff. I think the syntax should
just be:
pg_computemaxlsn file_or_directory...
For each argument, we determine whether it's a file or a directory.
If it's a file, we assume it's a PostgreSQL data file and scan it. If
it's a directory, we check whether it looks like a data directory. If
it does, we recurse through the whole tree structure and find the data
files, and process them. If it doesn't look like a data directory, we
scan each plain file in that directory whose name looks like a
PostgreSQL data file name. With this approach, there's no need to
limit ourselves to a single input argument and no need to specify what
kind of argument it is; the tool just figures it out.
I think it would be a good idea to have a mode that prints out the max
LSN found in *each data file* scanned, and then prints out the overall
max found at the end. In fact, I think that should perhaps be the
default mode, with -q, --quiet to disable it. When printing out the
per-file data, I think it would be useful to track and print the block
number where the highest LSN in that file was found. I have
definitely had cases where I suspected, but was not certain of,
corruption. One could use a tool like this to hunt for problems, and
then use something like pg_filedump to dump the offending blocks.
That would be a lot easier than running pg_filedump on the whole file
and grepping through the output.
Similarly, I see no reason for the restriction imposed by
check_path_belongs_to_pgdata(). I've had people mail me one data
file; why shouldn't I be able to run this tool on it? It's a
read-only utility.
- if (0 != FindMaxLSNinDir(pathbuf, maxlsn, false)) and similar is not
PostgreSQL style.
+ printf(_(%s compute the maximum LSN in PostgreSQL data
pages.\n\n), progname);
s/compute/computes/
+ /*
+* Don't allow pg_computemaxlsn to be run as root, to avoid overwriting
+* the ownership of files in the data directory. We need only check for
+* root -- any other user won't have sufficient permissions to modify
+* files in the data directory.
+*/
+ #ifndef WIN32
+ if (geteuid() == 0)
+ {
+ fprintf(stderr, _(%s: cannot be executed by \root\.\n),
+ progname);
+ fprintf(stderr, _(You must run %s as the PostgreSQL
superuser.\n),
+ progname);
+ exit(1);
+ }
+ #endif
This utility only reads files; it does not modify them. So this seems
unnecessary. I assume it's
Re: [HACKERS] [9.4 CF 1] The Commitfest Slacker List
On Tue, Jul 2, 2013 at 3:00 PM, Hannu Krosing ha...@krosing.net wrote: I guess whoever registered it with CF should also take your place on the slackers list ;) Yeah, I recommend that, in the future, CF managers do NOT go and add patches to the CF. Pinging newbies to see if they just forgot is sensible, but if an experienced hacker hasn't put something in the CF, there's probably a reason. Also, I recommend that nobody get too worked up about being on the slacker list. Life is short, PostgreSQL is awesome, and nobody can make you review patches against your will. Don't take it for more than what Josh meant it as. -- 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 REINDEX CONCURRENTLY
On Fri, Jun 28, 2013 at 4:30 PM, Michael Paquier
michael.paqu...@gmail.com wrote:
On Wed, Jun 26, 2013 at 1:06 AM, Fujii Masao masao.fu...@gmail.com wrote:
Thanks for updating the patch!
And thanks for taking time to look at that. I updated the patch
according to your comments, except for the VACUUM FULL problem. Please
see patch attached and below for more details.
When I ran VACUUM FULL, I got the following error.
ERROR: attempt to apply a mapping to unmapped relation 16404
STATEMENT: vacuum full;
This can be reproduced when doing a vacuum full on pg_proc,
pg_shdescription or pg_db_role_setting for example, or relations that
have no relfilenode (mapped catalogs), and a toast relation. I still
have no idea what is happening here but I am looking at it. As this
patch removes reltoastidxid, could that removal have effect on the
relation mapping of mapped catalogs? Does someone have an idea?
Could you let me clear why toast_save_datum needs to update even invalid
toast
index? It's required only for REINDEX CONCURRENTLY?
Because an invalid index might be marked as indisready, so ready to
receive inserts. Yes this is a requirement for REINDEX CONCURRENTLY,
and in a more general way a requirement for a relation that includes
in rd_indexlist indexes that are live, ready but not valid. Just based
on this remark I spotted a bug in my patch for tuptoaster.c where we
could insert a new index tuple entry in toast_save_datum for an index
live but not ready. Fixed that by adding an additional check to the
flag indisready before calling index_insert.
@@ -1573,7 +1648,7 @@ toastid_valueid_exists(Oid toastrelid, Oid valueid)
toastrel = heap_open(toastrelid, AccessShareLock);
- result = toastrel_valueid_exists(toastrel, valueid);
+ result = toastrel_valueid_exists(toastrel, valueid, AccessShareLock);
toastid_valueid_exists() is used only in toast_save_datum(). So we should use
RowExclusiveLock here rather than AccessShareLock?
Makes sense.
+ * toast_open_indexes
+ *
+ * Get an array of index relations associated to the given toast
relation
+ * and return as well the position of the valid index used by the toast
+ * relation in this array. It is the responsability of the caller of
this
Typo: responsibility
Done.
toast_open_indexes(Relation toastrel,
+ LOCKMODE lock,
+ Relation **toastidxs,
+ int *num_indexes)
+{
+ int i = 0;
+ int res = 0;
+ boolfound = false;
+ List *indexlist;
+ ListCell *lc;
+
+ /* Get index list of relation */
+ indexlist = RelationGetIndexList(toastrel);
What about adding the assertion which checks that the return value
of RelationGetIndexList() is not NIL?
Done.
When I ran pg_upgrade for the upgrade from 9.2 to HEAD (with patch),
I got the following error. Without the patch, that succeeded.
command: /dav/reindex/bin/pg_dump --host /dav/reindex --port 50432
--username postgres --schema-only --quote-all-identifiers
--binary-upgrade --format=custom
--file=pg_upgrade_dump_12270.custom postgres
pg_upgrade_dump_12270.log 21
pg_dump: query returned 0 rows instead of one: SELECT c.reltoastrelid,
t.indexrelid FROM pg_catalog.pg_class c LEFT JOIN pg_catalog.pg_index
t ON (c.reltoastrelid = t.indrelid) WHERE c.oid =
'16390'::pg_catalog.oid AND t.indisvalid;
This issue is reproducible easily by having more than 1 table using
toast indexes in the cluster to be upgraded. The error was on pg_dump
side when trying to do a binary upgrade. In order to fix that, I
changed the code binary_upgrade_set_pg_class_oids:pg_dump.c to fetch
the index associated to a toast relation only if there is a toast
relation. This adds one extra step in the process for each having a
toast relation, but makes the code clearer. Note that I checked
pg_upgrade down to 8.4...
Why did you remove the check of indisvalid from the --binary-upgrade SQL?
Without this check, if there is the invalid toast index, more than one rows are
returned and ExecuteSqlQueryForSingleRow() would cause the error.
+ foreach(lc, indexlist)
+ *toastidxs[i++] = index_open(lfirst_oid(lc), lock);
*toastidxs[i++] should be (*toastidxs)[i++]. Otherwise, segmentation fault can
happen.
For now I've not found any other big problem except the above.
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] proposal: enable new error fields in plpgsql (9.4)
On Fri, Jun 28, 2013 at 12:08:21PM -0400, Noah Misch wrote: On Fri, Jun 28, 2013 at 05:21:29PM +0200, Pavel Stehule wrote: 2013/6/28 Noah Misch n...@leadboat.com: Okay. I failed to note the first time through that while the patch uses the same option names for RAISE and GET STACKED DIAGNOSTICS, the existing option lists for those commands differ: --RAISE option----GET STACKED DIAGNOSTICS option-- ERRCODE RETURNED_SQLSTATE MESSAGE MESSAGE_TEXT DETAIL PG_EXCEPTION_DETAIL HINTPG_EXCEPTION_HINT CONTEXT PG_EXCEPTION_CONTEXT To be consistent with that pattern, I think we would use COLUMN, CONSTRAINT, TABLE, TYPE and SCHEMA as the new RAISE options. I understand to your motivation, but I am not sure. Minimally word TYPE is too general. I have not strong opinion in this area. maybe DATATYPE ?? I'm not positive either. DATATYPE rather than TYPE makes sense. Here's a revision bearing the discussed name changes and protocol documentation tweaks, along with some cosmetic adjustments. If this seems good to you, I will commit it. -- Noah Misch EnterpriseDB http://www.enterprisedb.com *** a/doc/src/sgml/libpq.sgml --- b/doc/src/sgml/libpq.sgml *** *** 2712,2720 char *PQresultErrorField(const PGresult *res, int fieldcode); termsymbolPG_DIAG_TABLE_NAME//term listitem para ! If the error was associated with a specific table, the name of ! the table. (When this field is present, the schema name field ! provides the name of the table's schema.) /para /listitem /varlistentry --- 2712,2720 termsymbolPG_DIAG_TABLE_NAME//term listitem para ! If the error was associated with a specific table, the name of the ! table. (Refer to the schema name field for the name of the ! table's schema.) /para /listitem /varlistentry *** *** 2723,2731 char *PQresultErrorField(const PGresult *res, int fieldcode); termsymbolPG_DIAG_COLUMN_NAME//term listitem para ! If the error was associated with a specific table column, the ! name of the column. (When this field is present, the schema ! and table name fields identify the table.) /para /listitem /varlistentry --- 2723,2731 termsymbolPG_DIAG_COLUMN_NAME//term listitem para ! If the error was associated with a specific table column, the name ! of the column. (Refer to the schema and table name fields to ! identify the table.) /para /listitem /varlistentry *** *** 2734,2742 char *PQresultErrorField(const PGresult *res, int fieldcode); termsymbolPG_DIAG_DATATYPE_NAME//term listitem para ! If the error was associated with a specific data type, the name ! of the data type. (When this field is present, the schema name ! field provides the name of the data type's schema.) /para /listitem /varlistentry --- 2734,2742 termsymbolPG_DIAG_DATATYPE_NAME//term listitem para ! If the error was associated with a specific data type, the name of ! the data type. (Refer to the schema name field for the name of ! the data type's schema.) /para /listitem /varlistentry *** *** 2745,2755 char *PQresultErrorField(const PGresult *res, int fieldcode); termsymbolPG_DIAG_CONSTRAINT_NAME//term listitem para ! If the error was associated with a specific constraint, ! the name of the constraint. The table or domain that the ! constraint belongs to is reported using the fields listed ! above. (For this purpose, indexes are treated as constraints, ! even if they weren't created with constraint syntax.) /para /listitem /varlistentry --- 2745,2755 termsymbolPG_DIAG_CONSTRAINT_NAME//term listitem para ! If the error was associated with a specific constraint, the name ! of the constraint. Refer to fields listed above for the ! associated table or domain. (For this purpose, indexes are ! treated as constraints, even if they weren't created with ! constraint syntax.) /para /listitem /varlistentry *** *** 2787,2795 char *PQresultErrorField(const PGresult
Re: [HACKERS] preserving forensic information when we freeze
On Mon, Jun 24, 2013 at 8:12 AM, Andres Freund and...@2ndquadrant.com wrote: Agreed. And it also improves the status quo when debugging. I wish this would have been the representation since the beginning. Some remarks: * I don't really like that HeapTupleHeaderXminFrozen() now is frequently performed without checking for FrozenTransactionId. I think the places where that's done are currently safe, but it seems likely that we introduce bugs due to people writing similar code. I think replacing *GetXmin by a wrapper that returns FrozenTransactionId if the hint bit tell us so would be a good idea. Then add *GetRawXmin for the rest (probably mostly display). Seems like it would make the patch actually smaller. I did think about this approach, but it seemed like it would add cycles in a significant number of places. For example, consider the HeapTupleSatisfies() routines, which are perhaps PostgreSQL's finest example of a place where you DON'T want to add any cycles. All of the checks on xmin are conditional on HeapTupleHeaderXminCommitted() having been found already to be false. That implies that the frozen bits aren't set either, so if HeapTupleHeaderGetXmin() were to recheck the bits it would be a waste. As I got to the end of the patch I was a little dismayed by the number of places that did need adjustment to check both things, but there are still plenty of important places that don't. * The PLs imo should set fn_xmin to FrozenTransactionId if the hint bit tell us the tuple is frozen. Why? I thought about that, but it seemed to me that the purpose of that caching was to avoid confusing two functions whose pg_proc tuples ended up at the same TID. So there's a failure mechanism there: the tuple can get vacuumed away and replaced with a new tuple which then gets frozen, and everything (bogusly) matches. If the actual XID-prior-to-freezing has to match, ISTM that the chances of a false match are far lower. You have to get the new tuple at the same TID slot *and* the XID counter has to wrap back around to the exactly-right XID to get a false match on XID, within the lifetime of a single backend. That's not bloody likely. Remember, the whole point of the patch is to start freezing things sooner, so the scenario where both the original and replacement tuples are frozen is going to become more common. We also don't particularly *want* the freezing of a pg_proc tuple to force recompilations in every backend. * I think rewrite_heap_dead_tuple needs to check for a frozen xmin and store that. We might looking at a chain which partially was done in 9.4. Not sure if that's a realistic scenario, but I'd rather be safe. IIUC, you're talking about the scenario where we have an update chain X - Y, where X is dead but not actually removed and Y is (forensically) frozen. We're examining tuple Y and trying to determine whether X has been entered in rs_unresolved_tups. If, as I think you're proposing, we consider the xmin of Y to be FrozenTransactionId, we will definitely not find it - because the way it got into the table in the first place was based on the value returned by HeapTupleHeaderGetUpdateXid(). And that value is certain not to be FrozenTransactionId, because we never set the xmax of a tuple to FrozenTransactionId. There's no possibility of getting confused here; if X is still around at all, it's xmax is of the same generation as Y's xmin. Otherwise, we've had an undetected XID wraparound. -- 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 REINDEX CONCURRENTLY
On Wed, Jul 3, 2013 at 5:43 AM, Michael Paquier michael.paqu...@gmail.com wrote: On Wed, Jul 3, 2013 at 5:22 AM, Fujii Masao masao.fu...@gmail.com wrote: Why did you remove the check of indisvalid from the --binary-upgrade SQL? Without this check, if there is the invalid toast index, more than one rows are returned and ExecuteSqlQueryForSingleRow() would cause the error. + foreach(lc, indexlist) + *toastidxs[i++] = index_open(lfirst_oid(lc), lock); *toastidxs[i++] should be (*toastidxs)[i++]. Otherwise, segmentation fault can happen. For now I've not found any other big problem except the above. system_views.sql -GROUP BY C.oid, N.nspname, C.relname, T.oid, X.oid; +GROUP BY C.oid, N.nspname, C.relname, T.oid, X.indexrelid; I found another problem. X.indexrelid should be X.indrelid. Otherwise, when there is the invalid toast index, more than one rows are returned for the same relation. OK cool, updated version attached. If you guys think that the attached version is fine (only the reltoasyidxid removal part), perhaps it would be worth committing it as Robert also committed the MVCC catalog patch today. So we would be able to focus on the core feature asap with the 2nd patch, and the removal of AccessExclusiveLock at swap step. Yep, will do. Maybe today. 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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
Amit Kapila escribió: I have changed the file name to postgresql.auto.conf and I have changed the name of SetPersistentLock to AutoFileLock. Zoltan, Could you please once check the attached Patch if you have time, as now all the things are resolved except for small doubt in syntax extensibility which can be changed based on final decision. There are a bunch of whitespace problems, as you would notice with git diff --check or git show --color. Could you please clean that up? Also, some of the indentation in various places is not right; perhaps you could fix that by running pgindent over the source files you modified (this is easily visible by eyeballing the git diff output; stuff is misaligned in pretty obvious ways.) Random minor other comments: + use of xref linkend=SQL-ALTER SYSTEM this SGML link cannot possibly work. Please run make check in the doc/src/sgml directory. Examples in alter_system.sgml have a typo SYTEM. Same file has or or. Also in that file, The name of an configuration parameter that exist in filenamepostgresql.conf/filename. the parameter needn't exist in that file to be settable, right? refnamediv refnameALTER SYSTEM/refname refpurposechange a server configuration parameter/refpurpose /refnamediv Perhaps permanently change ..? Also, some parameters require a restart, not a reload; maybe we should direct the user somewhere else to learn how to freshen up the configuration after calling the command. + /* skip auto conf temporary file */ + if (strncmp(de-d_name, + PG_AUTOCONF_FILENAME ., + sizeof(PG_AUTOCONF_FILENAME)) == 0) + continue; What's the dot doing there? + if ((stat(AutoConfFileName, st) == -1)) + ereport(elevel, + (errmsg(configuration parameters changed with ALTER SYSTEM command before restart of server + will not be effective as \%s\ file is not accessible., PG_AUTOCONF_FILENAME))); + else + ereport(elevel, + (errmsg(configuration parameters changed with ALTER SYSTEM command before restart of server + will not be effective as default include directive for \%s\ folder is not present in postgresql.conf, PG_AUTOCONF_DIR))); These messages should be split. Perhaps have the will not be effective in the errmsg() and the reason as part of errdetail()? Also, I'm not really sure about doing another stat() on the file after parsing failed; I think the parse routine should fill some failure context information, instead of having this code trying to reproduce the failure to know what to report. Maybe something like the SlruErrorCause enum, not sure. Similarly, + /* + * record if the file currently being parsed is postgresql.auto.conf, + * so that it can be later used to give warning if it doesn't parse + * it. +*/ + snprintf(Filename,sizeof(Filename),%s/%s, PG_AUTOCONF_DIR, PG_AUTOCONF_FILENAME); + ConfigAutoFileName = AbsoluteConfigLocation(Filename, ConfigFileName); + if (depth == 1 strcmp(ConfigAutoFileName, config_file) == 0) + *is_config_dir_parsed = true; this seems very odd. The whole is_config_dir_parsed mechanism smells strange to me, really. I think the caller should be in charge of keeping track of this, but I'm not sure. ParseConfigFp() would have no way of knowing, and in one place we're calling that routine precisely to parse the auto file. 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] [9.4 CF 1] The Commitfest Slacker List
On 2013/07/02, at 23:44, Bruce Momjian br...@momjian.us wrote: I understand. You could wear slacker as a badge of honor: ;-) http://momjian.us/main/img/main/slacker.jpg This picture could make a nice T-shirt btw. -- Michael -- 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] warn_unused_result in pgbench
On Wed, Jun 19, 2013 at 9:38 PM, Peter Eisentraut pete...@gmx.net wrote: When building without thread safety, I get the following compiler warning in pgbench: pgbench.c:2612:2: error: ignoring return value of function declared with warn_unused_result attribute [-Werror,-Wunused-result] write(th-pipes[1], ret, sizeof(TResult)); That should be fixed, I think. We can stick a cast-to-void in there if that helps. Or we could have the process exit(1) or exit(42) or something in that case. But then pthread_join() would also need to get smarter, as would its callers. -- 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: query result history in psql
I haven't been able to find any real documentation on this feature, other than the additions to the psql help. You're right, I missed that in my review. I agree that it needs some more documentation. What is ans? We covered that earlier in the email thread, but it's the current name for the query history. I think we are going to change it to something a little more descriptive. I was thinking qh for short or query-history. I'm not sure if I'll be able to implement this feature any time soon, as I'm very busy at the moment and going for a business trip in few days. I can try implementing the feature, I might have some time. On Tue, Jul 2, 2013 at 5:36 AM, Peter Eisentraut pete...@gmx.net wrote: On 7/2/13 5:53 AM, Maciej Gajewski wrote: In the meantime, I've applied your suggestions and moved the sting-manipulating functions to stringutils. Also fixed a tiny bug. Patch attached. I haven't been able to find any real documentation on this feature, other than the additions to the psql help. Could someone write some mock documentation at least, so we know what the proposed interfaces and intended ways of use are? What is ans?
Re: [HACKERS] Support for RANGE ... PRECEDING windows in OVER
I'm fine with moving the operators over to functions. I just don't want to implement anything that is against best practice. If we are OK with that direction, I'll go ahead and start on the new patch. Ian On Mon, Jul 1, 2013 at 9:03 PM, Tom Lane t...@sss.pgh.pa.us wrote: Craig Ringer cr...@2ndquadrant.com writes: On 07/02/2013 02:39 AM, Robert Haas wrote: 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. Personally I'm not clear why it's bad to reserve certain fundamental operators like '+' and '-', requiring that they have particular semantics. It is bad. It's against project policy, not least because we have assorted *existing* datatypes for which obvious operator names like = do not have all the properties you might expect. If you need a more concrete example of why that sort of thinking is bad, you might consider the difference between and ~~ for type text. If we hard-wired knowledge about operator behavior to operator names, it would be impossible for the system to understand that both of those operators represent sorting-related behaviors. Or to be even more concrete: if we allow RANGE to suppose that there's only one possible definition of + for a datatype, we're effectively supposing that there's only one possible sort ordering for that type. Which is already a wrong assumption, and has been since Postgres was still at Berkeley. If you go this way, you won't be able to support both WINDOW ... ORDER BY foo USING RANGE ... and WINDOW ... ORDER BY foo USING ~~ RANGE ... because you won't know which addition operator to apply. (And yeah, I'm aware that the SQL standard only expects RANGE to support sort keys that are of numeric, datetime, or interval type. I would hope that we have higher expectations than that. Even if we don't, it's not exactly hard to credit that people might have multiple ideas about how to sort interval values.) There are indeed still some places where we rely on operator names to mean something, but we need to get away from that idea not add more. Ideally, any property the system understands about an operator or function should be explicitly declared through opclass membership or some similar representation. We've made substantial progress in that direction in the last fifteen years. I don't want to reverse that progress in the name of minor expediencies, especially not ones that fail to support flexibility that has been in the system for a couple or three decades already. regards, tom lane
Re: [HACKERS] updated emacs configuration
Updated files with changes: - adjusted fill-column to 78, per Noah - added c-file-style, per Andrew - support both postgresql and postgres directory names - use defun instead of lambda, per Dimitri - put Perl configuration back into emacs.samples, for Tom I also added configuration of c-auto-align-backslashes as well as label and statement-case-open to c-offsets-alist. With those changes, the result of indent-region is now very very close to pgindent, with the main exception of the end-of-line de-indenting that pgindent does, which nobody likes anyway. ;; see also src/tools/editors/emacs.samples for more complete settings ((c-mode . ((c-basic-offset . 4) (c-file-style . bsd) (fill-column . 78) (indent-tabs-mode . t) (tab-width . 4))) (dsssl-mode . ((indent-tabs-mode . nil))) (nxml-mode . ((indent-tabs-mode . nil))) (perl-mode . ((perl-indent-level . 4) (perl-continued-statement-offset . 4) (perl-continued-brace-offset . 4) (perl-brace-offset . 0) (perl-brace-imaginary-offset . 0) (perl-label-offset . -2) (tab-width . 4))) (sgml-mode . ((fill-column . 78) (indent-tabs-mode . nil ;; -*- mode: emacs-lisp -*- ;; This file contains code to set up Emacs to edit PostgreSQL source ;; code. Copy these snippets into your .emacs file or equivalent, or ;; use load-file to load this file directly. ;; ;; Note also that there is a .dir-locals.el file at the top of the ;; PostgreSQL source tree, which contains many of the settings shown ;; here (but not all, mainly because not all settings are allowed as ;; local variables). So for light editing, you might not need any ;; additional Emacs configuration. ;;; C files ;; Style that matches the formatting used by ;; src/tools/pgindent/pgindent. Many extension projects also use this ;; style. (c-add-style postgresql '(bsd (c-auto-align-backslashes . nil) (c-basic-offset . 4) (c-offsets-alist . ((case-label . +) (label . -) (statement-case-open . +))) (fill-column . 78) (indent-tabs-mode . t) (tab-width . 4))) (add-hook 'c-mode-hook (defun postgresql-c-mode-hook () (when (string-match /postgres\\(ql\\)?/ buffer-file-name) (c-set-style postgresql ;;; Perl files ;; Style that matches the formatting used by ;; src/tools/pgindent/perltidyrc. (defun pgsql-perl-style () Perl style adjusted for PostgreSQL project (interactive) (setq perl-brace-imaginary-offset 0) (setq perl-brace-offset 0) (setq perl-continued-brace-offset 4) (setq perl-continued-statement-offset 4) (setq perl-indent-level 4) (setq perl-label-offset -2) (setq tab-width 4)) (add-hook 'perl-mode-hook (defun postgresql-perl-mode-hook () (when (string-match /postgres\\(ql\\)?/ buffer-file-name) (pgsql-perl-style ;;; documentation files (add-hook 'sgml-mode-hook (defun postgresql-sgml-mode-hook () (when (string-match /postgres\\(ql\\)?/ buffer-file-name) (setq fill-column 78) (setq indent-tabs-mode nil) (setq sgml-basic-offset 1 ;;; Makefiles ;; use GNU make mode instead of plain make mode (add-to-list 'auto-mode-alist '(/postgresql/.*Makefile.* . makefile-gmake-mode)) (add-to-list 'auto-mode-alist '(/postgresql/.*\\.mk\\' . makefile-gmake-mode)) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add an ldapoption to disable chasing LDAP referrals
Hey Peter,
You are correct, it is the same as the referrals option in pam_ldap. It's
also the -C (sometimes -R - it seems ldapsearch options are pretty
non-standard) option in ldapsearch.
As far as I'm aware you can't pass this in an LDAP URL, primarily because
this never gets sent to the LDAP server. The server always returns an LDIF
with inline references, this just determines if you chase them client side
or just list them as is.
I could be missing something here, but using:
ldapreferrals={0|1}
Would require a three state type, as we need a way of not interfering with
the library defaults? To 'enable' the new behavior here using a boolean you
would need to set ldapreferrals=false - which with the normal way of
dealing with config booleans would alter the default behavior if the option
was not specified.
How do you feel about:
ldapdisablereferrals=(0|1)
Cheers,
James Sewell
James Sewell
PostgreSQL Team Lead / Solutions Architect
_
[image:
http://www.lisasoft.com/sites/lisasoft/files/u1/2013hieghtslogan_0.png]
Level 2, 50 Queen St,
Melbourne, VIC, 3000
P: 03 8370 8000 F: 03 8370 8099 W: www.lisasoft.com
On Tue, Jul 2, 2013 at 10:46 PM, Peter Eisentraut pete...@gmx.net wrote:
On 7/2/13 12:20 AM, James Sewell wrote:
Hey All,
This patch request grew from this post (of mine) to pgsql-general:
http://www.postgresql.org/message-id/cabuevezouae-g1_oejagujjmem675dnystwybp4d_wz6om+...@mail.gmail.com
The patch adds another available LDAP option (ldapnochaseref) for
search+bind mode in the pg_hba.conf fil. If set to 1 (0 is default) then
it performs a ldap_set_option which disables chasing of any LDAP
references which are returned as part of the search LDIF.
This appears to be the same as the referrals option in pam_ldap
(http://linux.die.net/man/5/pam_ldap). So it seems legitimate.
For consistency, I would name the option ldapreferrals={0|1}. I prefer
avoiding double negatives.
Do you know of a standard way to represent this option in an LDAP URL,
perhaps as an extension?
--
--
The contents of this email are confidential and may be subject to legal or
professional privilege and copyright. No representation is made that this
email is free of viruses or other defects. If you have received this
communication in error, you may not copy or distribute any part of it or
otherwise disclose its contents to anyone. Please advise the sender of your
incorrect receipt of this correspondence.
image001.png
Re: [HACKERS] Review: query result history in psql
On Tue, Jul 2, 2013 at 8:16 PM, ian link i...@ilink.io wrote: We covered that earlier in the email thread, but it's the current name for the query history. I think we are going to change it to something a little more descriptive. I was thinking qh for short or query-history. I'm not sure if I'll be able to implement this feature any time soon, as I'm very busy at the moment and going for a business trip in few days. I can try implementing the feature, I might have some time. I'm kinda not all that convinced that this feature does anything that's actually useful. If you want to save your query results, you can just stick CREATE TEMP TABLE ans AS in front of your query. What does that not give you that this gives you? -- 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: query result history in psql
I'm kinda not all that convinced that this feature does anything that's actually useful. If you want to save your query results, you can just stick CREATE TEMP TABLE ans AS in front of your query. What does that not give you that this gives you? Convenience. It auto-increments with each new query, giving you a different temporary table for each query. Seems pretty helpful to me. On Tue, Jul 2, 2013 at 6:16 PM, Robert Haas robertmh...@gmail.com wrote: On Tue, Jul 2, 2013 at 8:16 PM, ian link i...@ilink.io wrote: We covered that earlier in the email thread, but it's the current name for the query history. I think we are going to change it to something a little more descriptive. I was thinking qh for short or query-history. I'm not sure if I'll be able to implement this feature any time soon, as I'm very busy at the moment and going for a business trip in few days. I can try implementing the feature, I might have some time. I'm kinda not all that convinced that this feature does anything that's actually useful. If you want to save your query results, you can just stick CREATE TEMP TABLE ans AS in front of your query. What does that not give you that this gives you? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: [HACKERS] Patch for removng unused targets
From: Alvaro Herrera [mailto:alvhe...@2ndquadrant.com] Etsuro Fujita escribió: From: Hitoshi Harada [mailto:umi.tan...@gmail.com] I tried several ways but I couldn't find big problems. Small typo: s/rejunk/resjunk/ Thank you for the review. Attached is an updated version of the patch. Thanks. I gave this a look, and made it some trivial adjustments. Attached is the edited version. I think this needs some more (succint) code comments: . why do we want to remove these entries . why can't we do it in the DISTINCT case . why don't we remove the cases we don't remove, within adjust_targetlist(). Thank you for the adjustments and comments! In addition to adding comments to the function, I've improved the code in the function a little bit. Please find attached an updated version of the patch. Sorry for the late response. (I was busy with another job lately...) Best regards, Etsuro Fujita unused-targets-20130703.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] Review: query result history in psql
2013/7/3 Robert Haas robertmh...@gmail.com: On Tue, Jul 2, 2013 at 8:16 PM, ian link i...@ilink.io wrote: We covered that earlier in the email thread, but it's the current name for the query history. I think we are going to change it to something a little more descriptive. I was thinking qh for short or query-history. I'm not sure if I'll be able to implement this feature any time soon, as I'm very busy at the moment and going for a business trip in few days. I can try implementing the feature, I might have some time. I'm kinda not all that convinced that this feature does anything that's actually useful. If you want to save your query results, you can just stick CREATE TEMP TABLE ans AS in front of your query. What does that not give you that this gives you? it solve lot of chars. I am thinking so idea is interesting. I can imagine, so it can works similar like our \g statement, but result will be stored to temp table and immediately showed. Regards Pavel -- 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] proposal: enable new error fields in plpgsql (9.4)
Hello 2013/7/2 Noah Misch n...@leadboat.com: On Fri, Jun 28, 2013 at 12:08:21PM -0400, Noah Misch wrote: On Fri, Jun 28, 2013 at 05:21:29PM +0200, Pavel Stehule wrote: 2013/6/28 Noah Misch n...@leadboat.com: Okay. I failed to note the first time through that while the patch uses the same option names for RAISE and GET STACKED DIAGNOSTICS, the existing option lists for those commands differ: --RAISE option----GET STACKED DIAGNOSTICS option-- ERRCODE RETURNED_SQLSTATE MESSAGE MESSAGE_TEXT DETAIL PG_EXCEPTION_DETAIL HINTPG_EXCEPTION_HINT CONTEXT PG_EXCEPTION_CONTEXT To be consistent with that pattern, I think we would use COLUMN, CONSTRAINT, TABLE, TYPE and SCHEMA as the new RAISE options. I understand to your motivation, but I am not sure. Minimally word TYPE is too general. I have not strong opinion in this area. maybe DATATYPE ?? I'm not positive either. DATATYPE rather than TYPE makes sense. Here's a revision bearing the discussed name changes and protocol documentation tweaks, along with some cosmetic adjustments. If this seems good to you, I will commit it. +1 Pavel -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [9.4 CF 1] The Commitfest Slacker List
Hackers, Clearly I ticked off a bunch of people by publishing the list. On the other hand, in the 5 days succeeding the post, more than a dozen additional people signed up to review patches, and we got some of the ready for committer patches cleared out -- something which nothing else I did, including dozens of private emails, general pleas to this mailing list, mails to the RRReviewers list, served to accomplish, in this or previous CFs. So, as an experiment, call it a mixed result. I would like to have some other way to motivate reviewers than public shame. I'd like to have some positive motivations for reviewers, such as public recognition by our project and respect from hackers, but I'm doubting that those are actually going to happen, given the feedback I've gotten on this list to the idea. I do think I succeeded in calling attention to the fact that this project has slid into a rut of letting a handful of people do 90% of the reviewing, resulting in CFs which last forever and some very frustrated major contributors. That part shouldn't be necessary again for some time, hopefully. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Reduce maximum error in tuples estimation after vacuum.
On Thursday, June 27, 2013 4:58 PM Amit Kapila wrote:
On Wednesday, June 26, 2013 7:40 AM Kyotaro HORIGUCHI wrote:
I've recovered from messing up.
snip
Please let me have a bit of time to diagnose this.
I was completely messed up and walking on the wrong way. I looked
into
the vacuum for UPDATEs, not DELETE's so it's quite resonable to have
such results.
The renewed test script attached shows the verbose output of vacuum
after the deletes. I had following output from it.
# I belive this runs for you..
| INFO: t: found 98 removable, 110 nonremovable row
| versions in 6308 out of 10829 pages
On such a case of partially-scanned, lazy_scan_heap() tries to
estimate
resulting num_tuples in vac_estimate_reltuples() assuming the
uniformity of tuple density, which failes for such a a strong
imbalance
made by bulk updates.
Do you find any differences between what you will have and the
following I had?
I could see the same output with your latest script, also I could
reproduce
the test if I run the test with individual sql statements.
One of the main point for reproducing individual test was to keep
autovacuum
= off.
I checked further that why I could not reproduce the issue with
autovacuum=on.
The reason is that it starts analyzer which changes the value for reltuples
in pg_class and after that the estimated and real values become same.
Kindly refer below code:
relation_needs_vacanalyze()
{
..
anltuples = tabentry-changes_since_analyze;
..
anlthresh = (float4) anl_base_thresh + anl_scale_factor * reltuples;
..
*doanalyze = (anltuples anlthresh);
}
Test Results
--
postgres=# drop table if exists t;
DROP TABLE
postgres=# create table t (a int, b int, c int, d int default 0, e int
default 0
, f int default 0);
CREATE TABLE
postgres=# insert into t (select a, (random() * 10)::int from
generate_serie
s((select count(*) from t) + 1, 100) a);
INSERT 0 100
postgres=# update t set b = b + 1 where a (select count(*) from t) * 0.7;
UPDATE 69
postgres=# vacuum t;
VACUUM
postgres=# delete from t where a (select count(*) from t) * 0.99;
DELETE 98
postgres=# vacuum t;
VACUUM
postgres=# select c.relpages, s.n_live_tup, c.reltuples, (select count(*)
from t
) as tuples, reltuples::float / (select count(*) from t) as ratio from
pg_stat_
user_tables s, pg_class c where s.relname = 't' and c.relname = 't';
relpages | n_live_tup | reltuples | tuples | ratio
--++---++--
6370 | 417600 |417600 | 10001 | 41.7558244175582
(1 row)
Here I waited for 1 minute (sufficient time so that analyzer should get
trigger if required).
Infact if you run Analyze t, that also would have served the purpose.
postgres=# select c.relpages, s.n_live_tup, c.reltuples, (select count(*)
from t
) as tuples, reltuples::float / (select count(*) from t) as ratio from
pg_stat_
user_tables s, pg_class c where s.relname = 't' and c.relname = 't';
relpages | n_live_tup | reltuples | tuples | ratio
--++---++---
6370 | 10001 | 10001 | 10001 | 1
(1 row)
Now if subsequent analyzer run corrects the estimate, don't you think that
it is sufficient for the problem reported?
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
