Re: [HACKERS] Hot Standby and cancelling idle queries
On Thu, 2009-11-26 at 08:30 +0200, Heikki Linnakangas wrote: I suspect you missed the context of this change. It's about the code in tablespc.c, to kill all backends that might have a temporary file in a tablespace that's being dropped. It's not about tuple visibility but temporary files. Got you now. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ecpg.addons
On Thu, Nov 26, 2009 at 11:14:54AM -0500, Tom Lane wrote: Michael Meskes mes...@postgresql.org writes: I wrote a hackish little script that checks if all rules in ecpg.addons are indeed used to create the precompiler's parser to catch renamed symbol and similar changes. Now I wonder whether this should be added to the build process. If it is, the buildfarm will catch every such change, not just the ones breaking the regression tests. However, this might also make the buildfarm become red if only a very minor and unimportant change is not syncronized. Any comment? The point of the buildfarm is to catch mistakes ... Okay, committed. As you guys know I don't really speak perl, so feel free to go in and fix whatever stupid perl mistake I made. 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 ICQ: 179140304, AIM/Yahoo/Skype: michaelmeskes, Jabber: mes...@jabber.org VfL Borussia! Forca Barca! 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] [PATCH 4/4] Add tests to dblink covering use of COPY TO FUNCTION
On Tue, 2009-11-24 at 22:13 -0800, Jeff Davis wrote: My disagreement with the row-by-row approach is more semantics than performance. COPY translates records to bytes and vice-versa, and your original patch maintains those semantics. The bytes - records conversion is a costly one. Anything we can do to avoid that in either direction will be worth it. I would regard performance as being part/most of the reason to support this. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unknown libpq service entries ignored
Peter Eisentraut pete...@gmx.net writes: + printfPQExpBuffer(errorMessage, + libpq_gettext(ERROR: service \%s\ not found\n), service); Please make the message consistent with the rest of libpq. AFAICS none of the other messages in that file use an ERROR: prefix. 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] KNNGiST for knn-search (WIP)
Planner (find_usable_indexes function, actually) could push pathkey expression into restriction clauses of index. I'm not fully satisfied with this piece of code, but, may be, someone has a better idea. I though about adding separate indexorderquals in IndexPath structure... Done, IndexScan and IndexPath have separate field to store order clauses. That's allowed to improve explain output: # EXPLAIN (COSTS OFF) SELECT * FROM point_tbl WHERE f1 @ '(-10,-10),(10,10)':: box ORDER BY f1 - '0,1'; QUERY PLAN Index Scan using gpointind on point_tbl Index Cond: (f1 @ '(10,10),(-10,-10)'::box) Sort Cond: (f1 - '(0,1)'::point) (3 rows) We are waiting feedback to choose a way of planner support of knn-search. Still TODO: - cost of index scan - Sort condition should not be ANDed in explain output - current patch remove support of IndexScanDesc-kill_prior_tuple, it's needed to restore support if it will not create too big overhead - documentation -- Teodor Sigaev E-mail: teo...@sigaev.ru WWW: http://www.sigaev.ru/ builtin_knngist-0.4.1.gz Description: Unix tar archive -- 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] KNNGiST for knn-search (WIP)
Teodor Sigaev teo...@sigaev.ru writes: Planner (find_usable_indexes function, actually) could push pathkey expression into restriction clauses of index. I'm not fully satisfied with this piece of code, but, may be, someone has a better idea. I though about adding separate indexorderquals in IndexPath structure... Done, IndexScan and IndexPath have separate field to store order clauses. Why? Isn't that redundant with the pathkey structures? We generate enough paths during a complex planning problem that I'm not in favor of adding unnecessary structures to them. 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] Hot Standby remaining issues
On Wed, 2009-11-25 at 13:00 +0200, Heikki Linnakangas wrote: I've put up a wiki page with the issues I see with the patch as it stands. They're roughly categorized by seriousness. http://wiki.postgresql.org/wiki/Hot_Standby_TODO New issues can and probably will still pop up, let's add them to the list as they're found so that we know what still needs to be done. You had a list of work items at the hot standby main page, but I believe it's badly out-of-date. Please move any still relevant items to the above list, if any. 4 changes on TODO included here, including all must-fix issues. This is a combined patch. Will push changes to git also, so each commit is visible separately. Lots of wiki comments added. -- Simon Riggs www.2ndQuadrant.com diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml index 75aca23..5af05fe 100644 --- a/doc/src/sgml/backup.sgml +++ b/doc/src/sgml/backup.sgml @@ -1985,7 +1985,7 @@ if (!triggered) /listitem listitem para - LOCK TABLE, though only when explicitly IN ACCESS SHARE MODE + LOCK TABLE, though only when explicitly in one of these modes: ACCESS SHARE, ROW SHARE or ROW EXCLUSIVE. /para /listitem listitem @@ -2033,7 +2033,7 @@ if (!triggered) listitem para LOCK TABLE, in short default form, since it requests ACCESS EXCLUSIVE MODE. - LOCK TABLE that explicitly requests a lock other than ACCESS SHARE MODE. + LOCK TABLE that explicitly requests a mode higher than ROW EXCLUSIVE MODE. /para /listitem listitem @@ -2110,10 +2110,10 @@ if (!triggered) para In recovery, transactions will not be permitted to take any table lock - higher than AccessShareLock. In addition, transactions may never assign + higher than RowExclusiveLock. In addition, transactions may never assign a TransactionId and may never write WAL. Any LOCK TABLE command that runs on the standby and requests a specific - lock type other than AccessShareLock will be rejected. + lock mode higher than ROW EXCLUSIVE MODE will be rejected. /para para @@ -2207,15 +2207,16 @@ if (!triggered) para We have a number of choices for resolving query conflicts. The default - is that we wait and hope the query completes. The server will wait automatically until the lag between - primary and standby is at most max_standby_delay seconds. Once that grace - period expires, we take one of the following actions: + is that we wait and hope the query completes. The server will wait + automatically until the lag between primary and standby is at most + max_standby_delay seconds. Once that grace period expires, we take one + of the following actions: itemizedlist listitem para - If the conflict is caused by a lock, we cancel the standby transaction - immediately, even if it is idle-in-transaction. + If the conflict is caused by a lock, we cancel the conflicting standby + transaction immediately, even if it is idle-in-transaction. /para /listitem listitem @@ -2224,7 +2225,9 @@ if (!triggered) that a conflict has occurred and that it must cancel itself to avoid the risk that it silently fails to read relevant data because that data has been removed. (This is very similar to the much feared - error message snapshot too old). + error message snapshot too old). Some cleanup records only cause + conflict with older queries, though some types of cleanup record + affect all queries. /para para @@ -2364,6 +2367,9 @@ LOG: database system is ready to accept read only connections para As a result, you cannot create additional indexes that exist solely on the standby, nor can statistics that exist solely on the standby. + If these administrator commands are needed they should be executed + on the primary so that the changes will propagate through to the + standby. /para para @@ -2373,7 +2379,8 @@ LOG: database system is ready to accept read only connections managed by the Startup process that owns all AccessExclusiveLocks held by transactions being replayed by recovery. pg_stat_activity does not show an entry for the Startup process, nor do recovering transactions - show as active. + show as active. Note that Startup process does not acquire locks to + make database changes and thus no locks are shown in pg_locks. /para para diff --git a/src/backend/access/nbtree/nbtpage.c b/src/backend/access/nbtree/nbtpage.c index 364756d..05e1c5e 100644 --- a/src/backend/access/nbtree/nbtpage.c +++ b/src/backend/access/nbtree/nbtpage.c @@ -659,7 +659,10 @@ _bt_page_recyclable(Page page) * order when replaying the effects of a VACUUM, just as we do for the * original VACUUM itself. lastBlockVacuumed allows us to tell whether an * intermediate range of blocks has had no changes at all by VACUUM, - * and so must be scanned anyway during replay. + * and so must be scanned anyway during replay. We always
Re: [HACKERS] KNNGiST for knn-search (WIP)
Done, IndexScan and IndexPath have separate field to store order clauses. Why? Isn't that redundant with the pathkey structures? We generate enough paths during a complex planning problem that I'm not in favor of adding unnecessary structures to them. I found two ways to add support of knn-seaech to planner - 0.4 version: add sorting clauses to restrictclauses in find_usable_indexes, and there is two issues: - find_usable_indexes could not be used to find indexes for index and bitmap scans at once, because sorting clauses will be used in bitmap scan. Full scan of index with knn-ordering on large index is much more expensive. - implied/refused predicate machinery is teached to ignore sorting clauses, but it's not so obvious: it should ignore operation returning non-boolean values - 0.4.1 version: pull sort clauses separately and merge them with regular clauses at create_indexscan_plan function. That's solves problems above Do you suggest to construct that clauses from pathkey representation? And append constructed clauses to indexquals in create_indexscan_plan? -- Teodor Sigaev E-mail: teo...@sigaev.ru WWW: http://www.sigaev.ru/ -- 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] KNNGiST for knn-search (WIP)
Teodor Sigaev teo...@sigaev.ru writes: Do you suggest to construct that clauses from pathkey representation? And append constructed clauses to indexquals in create_indexscan_plan? I'd probably have to read the patch to make any intelligent suggestions ... and right now I'm busy with patches that are already in the commitfest. But usually it's a good idea to postpone work to createplan time if you can. 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] Writeable CTE patch
Marko Tiikkaja marko.tiikk...@cs.helsinki.fi writes:
Tom Lane wrote:
since OIDs in user tables have been deprecated for several versions
now, I'm thinking that maybe the case doesn't arise often enough to
justify keeping such a wart in the executor.
Under the circumstances I'd lean towards this option.
I've been fooling around with this further and have gotten as far as
the attached patch. It passes regression tests but suffers from an
additional performance loss: the physical-tlist optimization is disabled
when scanning a relation having OIDs. (That is, we'll always use
ExecProject even if the scan is SELECT * FROM ) I think this loss
is worth worrying about since it would apply to queries on system
catalogs, even if the database has no OIDs in user tables. The trick
is to make the knowledge of the required hasoid state available at
ExecAssignResultType time, so that the plan node's result tupdesc is
constructed correctly.
What seems like the best bet is to merge ExecAssignResultTypeFromTL
and ExecAssignScanProjectionInfo into a single function that should
be used by scan node types. It'll do the determination of whether
a physical-tlist optimization is possible, and then set up both the
output tupdesc and the projection info accordingly. This will make the
patch diff a good bit longer but not much more interesting, so I'm
sending it along at this stage.
I think this is worth doing since it cleans up one of the grottier
parts of executor initialization. The whole thing around
ExecContextForcesOids was never pretty, and it's been the source of
more than one bug if memory serves.
Comments?
regards, tom lane
Index: src/backend/commands/copy.c
===
RCS file: /cvsroot/pgsql/src/backend/commands/copy.c,v
retrieving revision 1.318
diff -c -r1.318 copy.c
*** src/backend/commands/copy.c 20 Nov 2009 20:38:10 - 1.318
--- src/backend/commands/copy.c 27 Nov 2009 17:21:55 -
***
*** 1811,1817
estate-es_result_relations = resultRelInfo;
estate-es_num_result_relations = 1;
- estate-es_result_relation_info = resultRelInfo;
/* Set up a tuple slot too */
slot = ExecInitExtraTupleSlot(estate);
--- 1811,1816
***
*** 2165,2171
heap_insert(cstate-rel, tuple, mycid, hi_options, bistate);
if (resultRelInfo-ri_NumIndices 0)
! recheckIndexes = ExecInsertIndexTuples(slot, (tuple-t_self),
estate, false);
/* AFTER ROW INSERT Triggers */
--- 2164,2171
heap_insert(cstate-rel, tuple, mycid, hi_options, bistate);
if (resultRelInfo-ri_NumIndices 0)
! recheckIndexes = ExecInsertIndexTuples(resultRelInfo,
! slot, (tuple-t_self),
estate, false);
/* AFTER ROW INSERT Triggers */
Index: src/backend/commands/tablecmds.c
===
RCS file: /cvsroot/pgsql/src/backend/commands/tablecmds.c,v
retrieving revision 1.306
diff -c -r1.306 tablecmds.c
*** src/backend/commands/tablecmds.c 20 Nov 2009 20:38:10 - 1.306
--- src/backend/commands/tablecmds.c 27 Nov 2009 17:21:55 -
***
*** 941,947
resultRelInfo = resultRelInfos;
foreach(cell, rels)
{
- estate-es_result_relation_info = resultRelInfo;
ExecBSTruncateTriggers(estate, resultRelInfo);
resultRelInfo++;
}
--- 941,946
***
*** 1009,1015
resultRelInfo = resultRelInfos;
foreach(cell, rels)
{
- estate-es_result_relation_info = resultRelInfo;
ExecASTruncateTriggers(estate, resultRelInfo);
resultRelInfo++;
}
--- 1008,1013
Index: src/backend/commands/vacuum.c
===
RCS file: /cvsroot/pgsql/src/backend/commands/vacuum.c,v
retrieving revision 1.396
diff -c -r1.396 vacuum.c
*** src/backend/commands/vacuum.c 16 Nov 2009 21:32:06 - 1.396
--- src/backend/commands/vacuum.c 27 Nov 2009 17:21:55 -
***
*** 181,187
ec-estate-es_result_relations = ec-resultRelInfo;
ec-estate-es_num_result_relations = 1;
- ec-estate-es_result_relation_info = ec-resultRelInfo;
/* Set up a tuple slot too */
ec-slot = MakeSingleTupleTableSlot(tupdesc);
--- 181,186
***
*** 3099,3105
if (ec-resultRelInfo-ri_NumIndices 0)
{
ExecStoreTuple(newtup, ec-slot, InvalidBuffer, false);
! ExecInsertIndexTuples(ec-slot, (newtup.t_self), ec-estate, true);
ResetPerTupleExprContext(ec-estate);
}
}
--- 3098,3105
if (ec-resultRelInfo-ri_NumIndices 0)
{
ExecStoreTuple(newtup, ec-slot, InvalidBuffer, false);
! ExecInsertIndexTuples(ec-resultRelInfo, ec-slot, (newtup.t_self),
! ec-estate, true);
ResetPerTupleExprContext(ec-estate);
}
}
***
*** 3225,3231
if (ec-resultRelInfo-ri_NumIndices 0)
{
ExecStoreTuple(newtup, ec-slot,
Re: [HACKERS] Writeable CTE patch
I wrote: I think this is worth doing since it cleans up one of the grottier parts of executor initialization. The whole thing around ExecContextForcesOids was never pretty, and it's been the source of more than one bug if memory serves. On further review there's a really serious stumbling block here. Consider INSERT INTO t1 SELECT * FROM t2 UNION ALL SELECT * FROM t3 where the three tables all have the same user columns but t2 has OIDs and t3 not (or vice versa). Without ExecContextForcesOids or something very much like it, both scan nodes will think they can return physical tuples. The output of the Append node will therefore contain some tuples with OIDs and some without. Append itself can't fix that since it doesn't project. In many queries this would not matter --- but if we are inserting them directly into t1 without any further filtering, it does matter. I can imagine various ways around this, but it's not clear that any of them are much less grotty than the code is now. In any case this was just a marginal code cleanup idea and it doesn't seem worth spending so much time on right now. I'm going to go back to plan A: drop the es_result_relation_info changes from the patch. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH 4/4] Add tests to dblink covering use of COPY TO FUNCTION
On Fri, 2009-11-27 at 13:39 +, Simon Riggs wrote: On Tue, 2009-11-24 at 22:13 -0800, Jeff Davis wrote: My disagreement with the row-by-row approach is more semantics than performance. COPY translates records to bytes and vice-versa, and your original patch maintains those semantics. The bytes - records conversion is a costly one. Anything we can do to avoid that in either direction will be worth it. I would regard performance as being part/most of the reason to support this. Right. I was responding to an idea that copy support sending records from a table to a function, or from a function to a table, which is something that INSERT/SELECT can already do. Our use case is a table to a remote table, so it would go something like: 1. COPY TO WITH BINARY on local node 2. stream output bytes from #1 to remote node 3. COPY FROM WITH BINARY on remote node The only faster mechanism that I could imagine is sending the records themselves, which would be machine-dependent. 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
[HACKERS] OpenSSL key renegotiation with patched openssl
Recently openssl has been patched to not renegotiate keys. http://www.links.org/?p=780 After a certain amount of data has gone through a postgresql connection the server will attempt to switch session keys. What is the workaround (if any ) to avoid this in postgresql ? Dave -- 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] OpenSSL key renegotiation with patched openssl
Dave Cramer p...@fastcrypt.com writes: Recently openssl has been patched to not renegotiate keys. http://www.links.org/?p=780 After a certain amount of data has gone through a postgresql connection the server will attempt to switch session keys. What is the workaround (if any ) to avoid this in postgresql ? Install the updated openssl library. Why are you bugging us about an openssl patch? 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] OpenSSL key renegotiation with patched openssl
Tom Lane wrote: Dave Cramer p...@fastcrypt.com writes: Recently openssl has been patched to not renegotiate keys. http://www.links.org/?p=780 After a certain amount of data has gone through a postgresql connection the server will attempt to switch session keys. What is the workaround (if any ) to avoid this in postgresql ? Install the updated openssl library. Why are you bugging us about an openssl patch? regards, tom lane After applying the updated openssl library slony dies, presumably because the server requests a new session key Dave -- 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] OpenSSL key renegotiation with patched openssl
Dave Cramer davecra...@gmail.com writes: Tom Lane wrote: Install the updated openssl library. Why are you bugging us about an openssl patch? After applying the updated openssl library slony dies, presumably because the server requests a new session key The discussion I saw suggested that you need such a patch at both ends. 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] OpenSSL key renegotiation with patched openssl
Tom Lane wrote: Dave Cramer davecra...@gmail.com writes: Tom Lane wrote: Install the updated openssl library. Why are you bugging us about an openssl patch? After applying the updated openssl library slony dies, presumably because the server requests a new session key The discussion I saw suggested that you need such a patch at both ends. and likely requires a restart of both postgresql and slony afterwards... Stefan -- 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] OpenSSL key renegotiation with patched openssl
Stefan Kaltenbrunner ste...@kaltenbrunner.cc writes: Tom Lane wrote: The discussion I saw suggested that you need such a patch at both ends. and likely requires a restart of both postgresql and slony afterwards... Actually, after looking through the available info about this: https://svn.resiprocate.org/rep/ietf-drafts/ekr/draft-rescorla-tls-renegotiate.txt I think my comment above is wrong. It is useful to patch the *server*-side library to reject a renegotiation request. Applying that patch on the client side, however, is useless and simply breaks things. 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] unknown libpq service entries ignored
On fre, 2009-11-27 at 10:22 -0500, Tom Lane wrote: Peter Eisentraut pete...@gmx.net writes: + printfPQExpBuffer(errorMessage, + libpq_gettext(ERROR: service \%s\ not found\n), service); Please make the message consistent with the rest of libpq. AFAICS none of the other messages in that file use an ERROR: prefix. Well, that style is used four times in that same function, but no where else in the file. So the existing uses should probably also be cleaned up. -- 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] Re: [COMMITTERS] pgsql: Rewrite GEQO's gimme_tree function so that it always finds a
Robert Haas robertmh...@gmail.com writes: I guess it's somewhat unsurprising that you can make the planner get into trouble with the above settings - we've been over that ground before. But it might be possibly interesting that such a simple schema is capable of generating so many paths. It's not so much so-many-paths as so-many-join-relations that's killing it. I put some instrumentation into join_search_one_level to count the number of joinrels it was creating, and got this before getting bored: join_search_one_level(2): 36 total, 36 by clause, 0 by clauseless, 0 bushy, 0 lastditch; paths/rel min 1 max 4 avg 3.06 join_search_one_level(3): 192 total, 384 by clause, 0 by clauseless, 0 bushy, 0 lastditch; paths/rel min 1 max 6 avg 4.70 join_search_one_level(4): 865 total, 2373 by clause, 0 by clauseless, 222 bushy, 0 lastditch; paths/rel min 1 max 8 avg 6.20 join_search_one_level(5): 3719 total, 12637 by clause, 0 by clauseless, 2239 bushy, 0 lastditch; paths/rel min 2 max 10 avg 7.81 join_search_one_level(6): 15387 total, 62373 by clause, 0 by clauseless, 14562 bushy, 0 lastditch; paths/rel min 3 max 12 avg 9.43 join_search_one_level(7): 59857 total, 283371 by clause, 0 by clauseless, 75771 bushy, 0 lastditch; paths/rel min 4 max 15 avg 10.92 So the number of paths per relation seems to be staying manageable, but the number of join relations is going through the roof. On the other hand, you've got 37 base relations here, and (37 choose 7) is 10295472, so the planner is doing reasonably well at pruning the search tree. Anyway, the immediate problem is that list_concat_unique_ptr is a bad way of forming the lists of relations with a certain number of members. It strikes me that that's easily fixable given that we know when we are constructing a new RelOptInfo how many members it has. We could add the rel to the right list at that time, instead of waiting until we get back up to join_search_one_level. It implies making the joinrels[] array part of the planner's global state instead of local to make_one_rel and join_search_one_level, but for the sorts of list lengths we're seeing here it seems clearly worthwhile. Maybe I'll go try that while I'm sitting here digesting leftover turkey. 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] New VACUUM FULL
Review comments:
* I attached some documentation suggestions.
* The patch should be merged with CVS HEAD. The changes required are
minor; but notice that there is a minor style difference in the assert
in vacuum().
* vacuumdb should reject -i without -f
* The replace or inplace option is a magical default, because VACUUM
FULL defaults to replace for user tables and inplace for system
tables. I tried to make that more clear in my documentation suggestions.
* VACUUM (FULL REPLACE) pg_class should be rejected, not silently
turned into VACUUM (FULL INPLACE) pg_class.
* There are some windows line endings in the patch, which should be
removed.
* In vacuumdb, the code for handling INPLACE is a little ugly. Perhaps
it should be changed to always use your new options syntax? That might
be more code, but it would be a little more readable.
Regards,
Jeff Davis
*** a/doc/src/sgml/ref/vacuum.sgml
--- b/doc/src/sgml/ref/vacuum.sgml
***
*** 22,29 PostgreSQL documentation
refsynopsisdiv
synopsis
VACUUM [ ( { FULL [ INPLACE | REPLACE ] | FREEZE | VERBOSE | ANALYZE } [, ...] ) ] [ replaceable class=PARAMETERtable/replaceable [ (replaceable class=PARAMETERcolumn/replaceable [, ...] ) ] ]
! VACUUM [ FULL [ INPLACE | REPLACE ] ] [ FREEZE ] [ VERBOSE ] [ replaceable class=PARAMETERtable/replaceable ]
! VACUUM [ FULL [ INPLACE | REPLACE ] ] [ FREEZE ] [ VERBOSE ] ANALYZE [ replaceable class=PARAMETERtable/replaceable [ (replaceable class=PARAMETERcolumn/replaceable [, ...] ) ] ]
/synopsis
/refsynopsisdiv
--- 22,29
refsynopsisdiv
synopsis
VACUUM [ ( { FULL [ INPLACE | REPLACE ] | FREEZE | VERBOSE | ANALYZE } [, ...] ) ] [ replaceable class=PARAMETERtable/replaceable [ (replaceable class=PARAMETERcolumn/replaceable [, ...] ) ] ]
! VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ replaceable class=PARAMETERtable/replaceable ]
! VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] ANALYZE [ replaceable class=PARAMETERtable/replaceable [ (replaceable class=PARAMETERcolumn/replaceable [, ...] ) ] ]
/synopsis
/refsynopsisdiv
***
*** 85,101 VACUUM [ FULL [ INPLACE | REPLACE ] ] [ FREEZE ] [ VERBOSE ] ANALYZE [ replacea
para
Selects quotefull/quote vacuum, which can reclaim more
space, but takes much longer and exclusively locks the table.
- literalINPLACE/literal option consumes less disk space but takes
- longer time, and literalREPLACE/literal option vice versa.
- The default is literalREPLACE/literal.
/para
para
! commandVACUUM (FULL INPLACE)/command behaves as a traditional
! commandVACUUM FULL/command command. It moves tuples in the table
! from the tail to the head of files. On the other hand,
! commandVACUUM (FULL REPLACE)/command behaves as a
! commandCLUSTER/command command without any sorting.
! It moves all tuples into a new table and swap the old and the new tables.
/para
/listitem
/varlistentry
--- 85,107
para
Selects quotefull/quote vacuum, which can reclaim more
space, but takes much longer and exclusively locks the table.
/para
para
! If literalREPLACE/literal is specified, the entire table and
! all indexes are rewritten. This method requires extra disk space
! in which to write the new data, and is generally most efficient
! when a significant amount of space needs to be reclaimed from
! within the table. This method is the default for user tables,
! but it cannot be used on system catalogs.
! /para
! para
! If literalINPLACE/literal is specified, the table and
! indexes are modified in place to reclaim space. This method may
! require less disk space than literalFULL REPLACE/literal for
! the table data, but the indexes will grow which may counteract
! that benefit. literalFULL INPLACE/literal is generally
! slower, and should only be used for system catalogs (for which
! it is the default).
/para
/listitem
/varlistentry
--
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] Re: [COMMITTERS] pgsql: Rewrite GEQO's gimme_tree function so that it always finds a
On Fri, Nov 27, 2009 at 3:05 PM, Robert Haas robertmh...@gmail.com wrote: On Mon, Nov 9, 2009 at 1:42 PM, Robert Haas robertmh...@gmail.com wrote: On Mon, Nov 9, 2009 at 1:10 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Mon, Nov 9, 2009 at 10:57 AM, Tom Lane t...@sss.pgh.pa.us wrote: Too bad you don't have debug symbols ... it'd be interesting to see how long that list is. I stopped it a couple of times. Lengths of list1, list2 respectively: 8876, 20 14649, 18 15334, 10 17148, 18 18173, 18 Yowza. 18000 distinct paths for one relation? Could we see the test case? Well, the test case isn't simple, and I'm not sure that my employer would be pleased if I posted it to a public mailing list. The general thrust of it is that [...] Test case attached. Load this into an empty database and then: set geqo to off; set join_collapse_limit to 100; set from_collapse_limit to 100; select * from foo_view order by name; I guess it's somewhat unsurprising that you can make the planner get into trouble with the above settings - we've been over that ground before. But it might be possibly interesting that such a simple schema is capable of generating so many paths. This breaks 40,000 without finishing. Hey, wait a minute. Unless I'm missing something, the items being accumulated here are not paths (as Tom said upthread and I naively believed), but RelOptInfos. It looks like we create a RelOptInfo for every legal join order, so this is going to happen on any large-ish query where the joins can be reordered relatively freely. I don't think there's any easy fix for this. When the joins can be reordered relatively freely, the number of legal join orders is roughly n!. It might be possible to reduce the overhead associated with a single one of those join orderings (which consists of a RelOptInfo, some supporting data structures, and however many paths) but given the explosive growth of n! that won't buy very much at all, and it won't make the lists shorter in any case. Playing around with this a bit, I was easily able to get 2-second planing times on 15 table join, 6-second planning times on a 16 table join and 30-second planning times on a 17 table join. This makes it hard to support raising the GEQO threshold, as most recently suggested by Greg Sabino Mullane here (and previously by me on an earlier thread): http://archives.postgresql.org/pgsql-hackers/2009-11/msg01106.php What sucks about the current geqo_threshold mechanism is that the number of tables is a fairly coarse predictor of the number of legal join orders. On some queries it is close to n! - on others it is far less. It would be nice if we had a way to either (1) approximate the number of legal join orders before we start planning the query, and base the GEQO decision on that number rather than on the number of tables, or (2) always start with the standard (exhaustive) planner, and switch to some kind of approximation algorithm (GEQO or otherwise) if we find that to be impractical. But neither of these seems at all straightforward. ...Robert -- 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] Re: [COMMITTERS] pgsql: Rewrite GEQO's gimme_tree function so that it always finds a
On Sat, Nov 28, 2009 at 12:04 AM, Tom Lane t...@sss.pgh.pa.us wrote: It's not so much so-many-paths as so-many-join-relations that's killing it. I put some instrumentation into join_search_one_level to count the number of joinrels it was creating, and got this before getting bored: This is pretty off-topic, but if we had some upper bound on the cost of the complete plan, we could discard pieces of the plan that already cost more. One way to get the upper bound is to generate the plan in depth-first fashion, instead of the current breadth-first. Instead of bottom-up dynamic programming, employ memoization. The doubt I have is that this could show to not be a win because to discard a sub-plan we would have to consider the startup cost, not the total cost, and therefore we would be discarding not enough to make it worthwile. But I thought I`d mention it anyway, in case someone has a better idea :) Greetings Marcin Mańk -- 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] Re: [COMMITTERS] pgsql: Rewrite GEQO's gimme_tree function so that it always finds a
Robert Haas robertmh...@gmail.com writes: On Mon, Nov 9, 2009 at 1:10 PM, Tom Lane t...@sss.pgh.pa.us wrote: Yowza. 18000 distinct paths for one relation? Could we see the test case? Hey, wait a minute. Unless I'm missing something, the items being accumulated here are not paths (as Tom said upthread and I naively believed), but RelOptInfos. Yeah, I failed to think carefully about that. It looks like we create a RelOptInfo for every legal join order, so this is going to happen on any large-ish query where the joins can be reordered relatively freely. Actually, it's more like a RelOptInfo for every combination of base rels that could be formed along any legal join path (where legal includes the heuristics that avoid clauseless joins when possible). So the worst case is 2^n for n base rels, but usually considerably fewer. Nonetheless, even a small fraction of 2^37 is Too Many. I don't think there's any easy fix for this. Nope :-(. I was able to get rid of the specific O(N^2) behavior that you were running into, but that just pushes the problem out a bit. FWIW, a profile of CVS HEAD running on this query looks like samples %symbol name 208189 15.9398 compare_fuzzy_path_costs 1162608.9014 add_path 97509 7.4657 AllocSetAlloc 78354 5.9991 make_canonical_pathkey 69027 5.2850 generate_join_implied_equalities 65153 4.9884 cost_nestloop 59689 4.5700 bms_is_subset 49176 3.7651 add_paths_to_joinrel 39720 3.0411 bms_overlap 32562 2.4931 cost_mergejoin 30101 2.3047 pathkeys_useful_for_merging 26409 2.0220 AllocSetFree 24459 1.8727 MemoryContextAllocZeroAligned 23247 1.7799 hash_search_with_hash_value 16018 1.2264 check_list_invariants which is at least unsurprising. However, it eats memory fast enough to start swapping after level 7 or so, so there is no way that the exhaustive planner is ever going to finish this problem. Playing around with this a bit, I was easily able to get 2-second planing times on 15 table join, 6-second planning times on a 16 table join and 30-second planning times on a 17 table join. This makes it hard to support raising the GEQO threshold, as most recently suggested by Greg Sabino Mullane here (and previously by me on an earlier thread): Yeah. I think GEQO or other approximate methods are the only hope for very large join problems. We could however hope to get to the point of being able to raise the collapse limits, rather than keeping them below the geqo_threshold by default. In principle that should result in better plans ... regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH 4/4] Add tests to dblink covering use of COPY TO FUNCTION
Jeff Davis wrote: All I mean is that the second argument to COPY should produce/consume bytes and not records. I'm not discussing the internal implementation at all, only semantics. In other words, STDIN is not a source of records, it's a source of bytes; and likewise for STDOUT. In the context of the read case, I'm not as sure it's so black and white. While the current situation does map better to a function that produces a stream of bytes, that's not necessarily the optimal approach for all situations. It's easy to imagine a function intended for accelerating bulk loading that is internally going to produce a stream of already processed records. A good example would be a function that is actually reading from another database system for the purpose of converting its data into PostgreSQL. If those were then loaded by a fairly direct path, that would happen at a much higher rate than if one had to convert those back into a stream of bytes with delimiters and then re-parse. I think there's a very valid use-case for both approaches. Maybe it just turns into an option, so you can get a faster loading path record at a time or just produce a stream characters, depending on what your data source maps to better. Something like this: COPY target FROM FUNCTION foo() WITH RECORDS; COPY target FROM FUNCTION foo() WITH BYTES; Would seem to cover both situations. I'd think that the WITH BYTES situation would just do some basic parsing and then pass the result through the same basic code path as WITH RECORDS, so having both available shouldn't increase the size of the implementation that much. -- Greg Smith2ndQuadrant Baltimore, MD PostgreSQL Training, Services and Support g...@2ndquadrant.com www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: [COMMITTERS] pgsql: Rewrite GEQO's gimme_tree function so that it always finds a
marcin mank marcin.m...@gmail.com writes: This is pretty off-topic, but if we had some upper bound on the cost of the complete plan, we could discard pieces of the plan that already cost more. One way to get the upper bound is to generate the plan in depth-first fashion, instead of the current breadth-first. Instead of bottom-up dynamic programming, employ memoization. Well, the breadth-first approach also allows elimination of large parts of the search space. It's not immediately obvious to me that the above would be better. You should be able to try it if you want though --- these days, there's even a hook to allow replacement of the join search strategy at runtime. 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
