Re: [HACKERS] [PATCH] Windows x64 [repost]
Tatsuo, Ok. Your suggestion is very helpfull. In general Tsutomu will wait for feedbacks come in, probably until Jan 15th. BTW, is there anyone who wishes the patches get in 8.5? Apparently Tstutomu, Magnus and I are counted in the group:-) But I'd like to know how other people are interested in the patches. I am very interested. A 64bit-Windows-Version would give a boost perception-wise (I know the technical arguments about usefullness or not, but perception is different and often quite important) Harald -- GHUM Harald Massa persuadere et programmare Harald Armin Massa Spielberger Straße 49 70435 Stuttgart 0173/9409607 no fx, no carrier pigeon - %s is too gigantic of an industry to bend to the whims of reality -- 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] EXPLAIN BUFFERS
Takahiro Itagaki escreveu: Sure, I should have merge all of the comments. Patch attached. Thanks for your effort. Looks sane to me. - Updated the output format as follows. I think this format is the most similar to existing lines. (actual by ANALYZE and Filter:). If people object to it, we can always change it later. IMHO, we could remove those options completely because we can use EXPLAIN BUFFERS and DTrace probes instead of them. +1. But we need to propose some replacement options. -- Euler Taveira de Oliveira http://www.timbira.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] Windows x64 [repost]
On Wed, Dec 9, 2009 at 8:31 AM, Massa, Harald Armin c...@ghum.de wrote: Tatsuo, Ok. Your suggestion is very helpfull. In general Tsutomu will wait for feedbacks come in, probably until Jan 15th. BTW, is there anyone who wishes the patches get in 8.5? Apparently Tstutomu, Magnus and I are counted in the group:-) But I'd like to know how other people are interested in the patches. I am very interested. A 64bit-Windows-Version would give a boost perception-wise I'm also very interested - despite the fact it'll cause me a boatload of work to produce a new set of installers for this architecture! -- Dave Page EnterpriseDB UK: 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] Streaming replication, some small issues
On Wed, Dec 9, 2009 at 10:51 AM, Fujii Masao masao.fu...@gmail.com wrote: On Wed, Dec 9, 2009 at 10:12 AM, Tom Lane t...@sss.pgh.pa.us wrote: Fujii Masao masao.fu...@gmail.com writes: Thought? Am I missing something? This seems terribly overdesigned. Just emit a warning when you see the unlogged op record and have done. Sounds quite simple. OK, I'll do so. Here is the patch: - Write an XLOG UNLOGGED record in WAL if WAL-logging is skipped for only the reason that WAL archiving is not enabled and such record has not been written yet. - Cause archive recovery to end if an XLOG UNLOGGED record is found during it. I add this patch to the CommitFest 2010-01. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center *** a/src/backend/access/heap/heapam.c --- b/src/backend/access/heap/heapam.c *** *** 1972,1977 heap_insert(Relation relation, HeapTuple tup, CommandId cid, --- 1972,1984 PageSetTLI(page, ThisTimeLineID); } + /* + * Write an XLOG UNLOGGED record if WAL-logging is skipped for the reason + * that WAL archiving is not enabled. + */ + if (options HEAP_INSERT_SKIP_WAL !relation-rd_istemp) + XLogSkipLogging(); + END_CRIT_SECTION(); UnlockReleaseBuffer(buffer); *** a/src/backend/access/nbtree/nbtsort.c --- b/src/backend/access/nbtree/nbtsort.c *** *** 215,220 _bt_leafbuild(BTSpool *btspool, BTSpool *btspool2) --- 215,227 */ wstate.btws_use_wal = XLogArchivingActive() !wstate.index-rd_istemp; + /* + * Write an XLOG UNLOGGED record if WAL-logging is skipped for the reason + * that WAL archiving is not enabled. + */ + if (!XLogArchivingActive() !wstate.index-rd_istemp) + XLogSkipLogging(); + /* reserve the metapage */ wstate.btws_pages_alloced = BTREE_METAPAGE + 1; wstate.btws_pages_written = 0; *** a/src/backend/access/transam/xlog.c --- b/src/backend/access/transam/xlog.c *** *** 550,555 XLogInsert(RmgrId rmid, uint8 info, XLogRecData *rdata) --- 550,556 bool updrqst; bool doPageWrites; bool isLogSwitch = (rmid == RM_XLOG_ID info == XLOG_SWITCH); + bool isLogUnlogged = (rmid == RM_XLOG_ID info == XLOG_UNLOGGED); /* cross-check on whether we should be here or not */ if (!XLogInsertAllowed()) *** *** 699,707 begin:; * error checking in ReadRecord. This means that all callers of * XLogInsert must supply at least some not-in-a-buffer data. However, we * make an exception for XLOG SWITCH records because we don't want them to ! * ever cross a segment boundary. */ ! if (len == 0 !isLogSwitch) elog(PANIC, invalid xlog record length %u, len); START_CRIT_SECTION(); --- 700,708 * error checking in ReadRecord. This means that all callers of * XLogInsert must supply at least some not-in-a-buffer data. However, we * make an exception for XLOG SWITCH records because we don't want them to ! * ever cross a segment boundary. Also XLOG UNLOGGED records are exception. */ ! if (len == 0 !isLogSwitch !isLogUnlogged) elog(PANIC, invalid xlog record length %u, len); START_CRIT_SECTION(); *** *** 3551,3558 ReadRecord(XLogRecPtr *RecPtr, int emode) got_record:; /* ! * xl_len == 0 is bad data for everything except XLOG SWITCH, where it is ! * required. */ if (record-xl_rmid == RM_XLOG_ID record-xl_info == XLOG_SWITCH) { --- 3552,3559 got_record:; /* ! * xl_len == 0 is bad data for everything except XLOG SWITCH and XLOG UNLOGGED, ! * where it is required. */ if (record-xl_rmid == RM_XLOG_ID record-xl_info == XLOG_SWITCH) { *** *** 3564,3569 got_record:; --- 3565,3580 goto next_record_is_invalid; } } + else if (record-xl_rmid == RM_XLOG_ID record-xl_info == XLOG_UNLOGGED) + { + if (record-xl_len != 0) + { + ereport(emode, + (errmsg(invalid xlog unlogged operation record at %X/%X, + RecPtr-xlogid, RecPtr-xrecoff))); + goto next_record_is_invalid; + } + } else if (record-xl_len == 0) { ereport(emode, *** *** 3759,3764 got_record:; --- 3770,3788 */ readOff = XLogSegSize - XLOG_BLCKSZ; } + + /* + * Special processing if it's an XLOG UNLOGGED record and we are doing + * an archive recovery. + */ + if (record-xl_rmid == RM_XLOG_ID record-xl_info == XLOG_UNLOGGED + InArchiveRecovery) + { + ereport(emode, + (errmsg(unlogged operation record is found at %X/%X, + RecPtr-xlogid, RecPtr-xrecoff))); + goto next_record_is_invalid; + } return (XLogRecord *) buffer; next_record_is_invalid:; *** *** 6998,7003 RequestXLogSwitch(void) --- 7022,7057 } /* + * Write an XLOG UNLOGGED record. + */ + void + XLogSkipLogging(void) + { + XLogRecData rdata; + static bool skipped = false; + + /* + * If an XLOG UNLOGGED
Re: [HACKERS] What happened to pl/proxy and FDW?
Josh Berkus wrote: I thought the idea was that we were going to add PL/proxy to 8.5, with support for the foriegn data wrapper syntax? What happened to that? Using SQL/MED for defining pl/proxy clusters is still in the TODO list. I hope to do something about it during the next few weeks. However, I doubt if adding SQL/MED support would be enough for including it in 8.5 contrib. regards, Martin -- 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] Adding support for SE-Linux security
On Wed, Dec 9, 2009 at 1:44 AM, Magnus Hagander mag...@hagander.net wrote: 2009/12/9 Bruce Momjian br...@momjian.us: I frankly think the patch should be thought of as the SE-Linux-specific directory files, which KaiGai can maintain, and the other parts, which I think I can handle. I think that's a horribly bad idea. Me, too. The ECPG comparison is apt, except that this code is far more deeply integrated into core. The idea that the SE-Linux directory files can be maintained separately from the other parts does not seem realistic to me. The problems that are going to occur here are things like: somebody wants to rearrange some part of the permissions checking for some reason. So they move a bunch of code around and break SE-PostgreSQL. Someone has to review that patch and understand the danger it causes. That's going to require understanding both the SE-PostgreSQL-specific files and the other parts, and the relationship between the two of them. ...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] EXPLAIN BUFFERS
On Wed, Dec 9, 2009 at 12:36 AM, Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp wrote: Note that the patch also removes buffer counters from log_statement_stats, but we only have brief descriptions about the options. Our documentation say nothing about buffer counters, so I didn't modify those lines in sgml. http://developer.postgresql.org/pgdocs/postgres/runtime-config-statistics.html#RUNTIME-CONFIG-STATISTICS-MONITOR I'm not sure whether this is a good idea or not. Let me read the patch. I'm not sure an EXPLAIN option is really an adequate substitute for log_statement_stats - the latter will let you get stats for all of your queries automatically, I believe, and might still be useful as a quick and dirty tool. IMHO, we could remove those options completely because we can use EXPLAIN BUFFERS and DTrace probes instead of them. We certainly should NOT count on dtrace as a substitute for anything. It's not available on Windows, or all other platforms either. =# EXPLAIN (BUFFERS, ANALYZE) SELECT * FROM pgbench_accounts a, pgbench_branches b WHERE a.bid = b.bid AND abalance 0 ORDER BY abalance; QUERY PLAN -- Sort (cost=2891.03..2891.04 rows=1 width=461) (actual time=22.494..22.494 rows=0 loops=1) Sort Key: a.abalance Sort Method: quicksort Memory: 25kB Blocks: (shared hit=96 read=1544 written=0) (local hit=0 read=0 written=0) (temp read=0 written=0) - Nested Loop (cost=0.00..2891.02 rows=1 width=461) (actual time=22.488..22.488 rows=0 loops=1) Join Filter: (a.bid = b.bid) Blocks: (shared hit=96 read=1544 written=0) (local hit=0 read=0 written=0) (temp read=0 written=0) - Seq Scan on pgbench_accounts a (cost=0.00..2890.00 rows=1 width=97) (actual time=22.486..22.486 rows=0 loops=1) Filter: (abalance 0) Blocks: (shared hit=96 read=1544 written=0) (local hit=0 read=0 written=0) (temp read=0 written=0) - Seq Scan on pgbench_branches b (cost=0.00..1.01 rows=1 width=364) (never executed) Blocks: (shared hit=0 read=0 written=0) (local hit=0 read=0 written=0) (temp read=0 written=0) Total runtime: 22.546 ms (13 rows) I still think this is a bad format. Instead of putting ( and ) around each phrase, can't we just separate them with a , or ;? The filter uses parentheses in a mathematical way, for grouping related items. Not all filters are surrounded by parentheses (consider a filter like WHERE x, x being a boolean column) and some will have multiple sets, if there are ANDs and ORs in there. ...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] WAL format
On Mon, Dec 7, 2009 at 8:48 PM, Tom Lane t...@sss.pgh.pa.us wrote: Alvaro Herrera alvhe...@commandprompt.com writes: Heikki Linnakangas wrote: - at the end of WAL segment, when there's not enough space to write the next WAL record, always write an XLOG SWITCH record to fill the rest of the segment. What happens if a record is larger than a WAL segment? For example, what if I insert a 16 MB+ datum into a varlena field? That case doesn't pose a problem --- the datum would be toasted into individual tuples that are certainly no larger than a page. However we do have cases where a WAL record can get arbitrarily large; in particular a commit record with many subtransactions and/or many disk files to delete. These cases do get exercised in the field too --- I can recall at least one related bug report. Sounds like a reason to make the format simpler... If we raise the maximum segment size is there a point where we would be in a reasonable range to impose maximum sizes for these lists? 32MB? 64MB? It's not like there isn't a limit now -- we'll just throw an out of memory error when replaying the recovery if it doesn't fit in memory. What if we push the work of handling these lists up to the recovery manager instead of xlog.c? So commit records would send a record saying when xid commits the following subtransactions commit as well and it could send multiple such records. The recovery manager is responsible when it sees such records to remember the list somewhere and append the new values if it's already seen the list, possibly even spilling to disk and reload it when it sees the corresponding commit. -- greg -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] 答复: questions about concurrency control in Postgresql
It's a two step process. An update marks the tuple locked. Another transaction which comes along and wants to lock the tuple waits on the transaction marked on the tuple. When the first transaction commits or aborts then the second transaction can proceed and lock the tuple itself. I agree with it. The reason we need both locks is because the first transaction cannot go around the whole database finding every tuple it ever locked to unlock it, firstly that could be a very large list and secondly there would be no way to do that atomically. You mean that 2PL is hard to realize actually, I agree too. But it doesn't mean tuple lock is necessary. Tuple locks and all user-visible locks are indeed held until the end of the transaction. I don't agree with it, for I see unlocktuple(...) in heap_update(...). --Huang Xiaocheng --Database Information System Lab, Nankai University __ Information from ESET NOD32 Antivirus, version of virus signature database 4671 (20091208) __ The message was checked by ESET NOD32 Antivirus. http://www.eset.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] pg_ctl init extension
Greg Smith píše v út 08. 12. 2009 v 22:44 -0500: Zdenek Kotala wrote: thanks for your useful comments. I attached new doc patch version. I removed example changes and add link to create database cluster (I hope) everywhere. Please, look on it and let me know if there is still something what should be changed. That looks much better. There's only one bit that sticks out oddly now: + Note: The commandinitdb/command might be invoked by + commandpg_ctl initdb/command and commandinitdb/command cannot be in + default path on a productnamePostgreSQL/productname installations. What is that supposed to mean exactly? Ahh, It is somethink what I want to do, but it is not ready yet in this patch, but I already documented it. Idea is to install initdb and postgres into libexecdir and packager can select if libexecdir will be equal bindir or not. The paragraph should be removed at this moment. Shell I send modified patch or does committer remove it before commit? thanks Zdenek -- 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: [ADMIN] recovery is stuck when children are not processing SIGQUIT from previous crash
[moved to -hackers] On tor, 2009-11-12 at 09:35 -0500, Tom Lane wrote: Peter Eisentraut pete...@gmx.net writes: strace on the backend processes all showed them waiting at futex(0x7f1ee5e21c90, FUTEX_WAIT_PRIVATE, 2, NULL Notably, the first argument was the same for all of them. Looks like a race condition or lockup in the syslog code. Hm, why are there two signal handler calls in the stack? The only thing I can think of is that we sent SIGQUIT twice. That's probably bad --- is there any obvious path through the postmaster that would do that? The other thought is that quickdie should block signals before starting to do anything. Right. This would actually already work because a signal is blocked while its handler runs, except that we start quickdie() with PG_SETMASK(BlockSig); which blocks everything except SIGQUIT. That should probably be fixed in any case. -- 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] What happened to pl/proxy and FDW?
Martin Pihlak escribió: Josh Berkus wrote: I thought the idea was that we were going to add PL/proxy to 8.5, with support for the foriegn data wrapper syntax? What happened to that? Using SQL/MED for defining pl/proxy clusters is still in the TODO list. I hope to do something about it during the next few weeks. However, I doubt if adding SQL/MED support would be enough for including it in 8.5 contrib. regards, Martin I have two question about SQL/MED, It not enough to include this on the core? Peter doesn´t work on it ? Which is the development state of SQL/MED? Regards -- - TIP 4: No hagas 'kill -9' a postmaster Ing. Marcos Luís Ortíz Valmaseda PostgreSQL System DBA Centro de Tecnologías de Almacenamiento y Análisis de Datos (CENTALAD)(centa...@uci.cu) Universidad de las Ciencias Informáticas(http://www.uci.cu) La Habana, Cuba Linux User # 418229 http://www.postgresql-es.org http://www.postgresql.org http://www.planetpostgresql.org http://www.freebsd.org/es -- 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] pg_ctl init extension
On Dec 9, 2009, at 8:32 AM, Zdenek Kotala zdenek.kot...@sun.com wrote: Greg Smith píše v út 08. 12. 2009 v 22:44 -0500: Zdenek Kotala wrote: thanks for your useful comments. I attached new doc patch version. I removed example changes and add link to create database cluster (I hope) everywhere. Please, look on it and let me know if there is still something what should be changed. That looks much better. There's only one bit that sticks out oddly now: + Note: The commandinitdb/command might be invoked by + commandpg_ctl initdb/command and commandinitdb/command cannot be in + default path on a productnamePostgreSQL/productname installations. What is that supposed to mean exactly? Ahh, It is somethink what I want to do, but it is not ready yet in this patch, but I already documented it. Idea is to install initdb and postgres into libexecdir and packager can select if libexecdir will be equal bindir or not. The paragraph should be removed at this moment. Shell I send modified patch or does committer remove it before commit? I think Peter claimed this one but as far as I am concerned, I would always rather have an updated patch. ...Robert -- 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: [ADMIN] recovery is stuck when children are not processing SIGQUIT from previous crash
[moved to -hackers] On tor, 2009-11-12 at 22:37 +0200, Marko Kreen wrote: On 11/12/09, Tom Lane t...@sss.pgh.pa.us wrote: Marko Kreen mark...@gmail.com writes: You talked about blocking in quickdie(), but you'd need to block in elog(). I'm not really particularly worried about that case. By that logic, we could not use quickdie at all, because any part of the system might be doing something that wouldn't survive being interrupted. Not really - we'd need to care only about parts that quickdie() (or any other signal handler) wants to use. Which basically means elog() only. OK, full elog() is a beast, but why would SIGQUIT handler need full elog()? How about we export minimal log-writing function and make that signal-safe - that is, drop message if already active. This will excange potential crash/deadlock with lost msg which seems slightly better behaviour. Yeah, on reflection, calling elog in the SIGQUIT handler is just waiting for trouble. The call could block for any number of reasons, because there is a boatload of functionality that comes with a logging call. In the overall scheme of things, you don't really lose much if you just delete the call altogether, because in the event that it's called the postmaster will already have logged that it is going to kill all children. Or there ought to be some kind of alarm that would abort the thing if it takes too long. -- 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] dtrace probes for memory manager
Bernd Helmle píše v út 08. 12. 2009 v 22:06 +0100: --On 8. Dezember 2009 15:51:52 -0500 Greg Smith g...@2ndquadrant.com wrote: Try this instead, which will give you a test where checkpoints have a minimal impact, but lots of memory will be thrown around: pgbench -i -s 10 db pgbench -S -c 10 -T 600 db Thanks for the input, will try I modified probes to reduce overhead. Prototype patch is attached. Main point is to remove mcxt_alloc probe and keep only aset_alloc. I did also some testing with interesting results. At first I prepare special C store function (attached) which do only allocation and deallocation and I measured how long it takes: On 32bit the memory allocation is slow down 8.4% and on 64bit it is only 4.6%. Good to mention that I call palloc and pfree but in standard behavior pfree is not much used and memory is freed when context is destroyed. It means that we should think about 4.2% and 2.3% instead. But in normal situation database does also other thing and palloc is only one part of code path. It is why I run second test and use sun studio profiling tools (collect/analyzer) to determine how much CPU ticks cost the probes during pg_bench run. And results are much better. AllocSet alloc function takes about 4-5% and probes assembler code takes 0.1-0.2% on 64bit. I did not test 32bit but my expectation is that it should be about 0.3-0.4%. Zdenek diff -r 68b8827f4738 src/backend/utils/mmgr/aset.c --- a/src/backend/utils/mmgr/aset.c Fri Nov 13 11:17:04 2009 + +++ b/src/backend/utils/mmgr/aset.c Wed Dec 09 14:41:34 2009 +0100 @@ -64,6 +64,7 @@ #include postgres.h +#include pg_trace.h #include utils/memutils.h /* Define this to detail debug alloc information */ @@ -463,6 +464,8 @@ AssertArg(AllocSetIsValid(set)); + TRACE_POSTGRESQL_ASET_RESET(context); + /* Nothing to do if no pallocs since startup or last reset */ if (set-isReset) return; @@ -495,6 +498,8 @@ #endif block-freeptr = datastart; block-next = NULL; + TRACE_POSTGRESQL_ASET_BLOCK_RESET(context, block, +block-endptr - ((char *) block)); } else { @@ -503,6 +508,8 @@ /* Wipe freed memory for debugging purposes */ memset(block, 0x7F, block-freeptr - ((char *) block)); #endif + TRACE_POSTGRESQL_ASET_BLOCK_FREE(context, block, +block-endptr - ((char *) block)); free(block); } block = next; @@ -529,6 +536,7 @@ AllocBlock block = set-blocks; AssertArg(AllocSetIsValid(set)); + TRACE_POSTGRESQL_ASET_DELETE(context); #ifdef MEMORY_CONTEXT_CHECKING /* Check for corruption and leaks before freeing */ @@ -548,6 +556,8 @@ /* Wipe freed memory for debugging purposes */ memset(block, 0x7F, block-freeptr - ((char *) block)); #endif + TRACE_POSTGRESQL_ASET_BLOCK_FREE(context, block, + block-endptr - ((char *) block)); free(block); block = next; } @@ -570,6 +580,7 @@ AssertArg(AllocSetIsValid(set)); + TRACE_POSTGRESQL_ASET_ALLOC(context, size); /* * If requested size exceeds maximum for chunks, allocate an entire block * for this request. @@ -623,6 +634,7 @@ set-isReset = false; AllocAllocInfo(set, chunk); + TRACE_POSTGRESQL_ASET_BLOCK_NEW(context, block, blksize); return AllocChunkGetPointer(chunk); } @@ -771,6 +783,7 @@ errdetail(Failed on request of size %lu., (unsigned long) size))); } + TRACE_POSTGRESQL_ASET_BLOCK_NEW(context, block, blksize); block-aset = set; block-freeptr = ((char *) block) + ALLOC_BLOCKHDRSZ; @@ -830,6 +843,7 @@ AllocChunk chunk = AllocPointerGetChunk(pointer); AllocFreeInfo(set, chunk); + TRACE_POSTGRESQL_ASET_FREE(context, pointer); #ifdef MEMORY_CONTEXT_CHECKING /* Test for someone scribbling on unused space in chunk */ @@ -866,6 +880,9 @@ set-blocks = block-next; else prevblock-next = block-next; + + TRACE_POSTGRESQL_ASET_BLOCK_FREE(context, block, + block-endptr - ((char *) block)); #ifdef CLOBBER_FREED_MEMORY /* Wipe freed memory for debugging purposes */ memset(block, 0x7F, block-freeptr - ((char *) block)); @@ -905,6 +922,8 @@ AllocChunk chunk = AllocPointerGetChunk(pointer); Size oldsize = chunk-size; + TRACE_POSTGRESQL_ASET_REALLOC(context, pointer, size, oldsize); + #ifdef MEMORY_CONTEXT_CHECKING /* Test for someone scribbling on unused space in chunk */ if (chunk-requested_size oldsize) @@ -948,6 +967,7 @@ */ AllocBlock block = set-blocks; AllocBlock prevblock = NULL; + AllocBlock newblock; Size chksize; Size blksize; @@ -967,7 +987,11 @@ /* Do the realloc */ chksize = MAXALIGN(size); blksize = chksize + ALLOC_BLOCKHDRSZ + ALLOC_CHUNKHDRSZ; - block = (AllocBlock) realloc(block, blksize); + newblock = (AllocBlock) realloc(block, blksize); + + TRACE_POSTGRESQL_ASET_BLOCK_REALLOC(context, block, newblock, blksize); + block = newblock; + if (block == NULL) { MemoryContextStats(TopMemoryContext); diff -r 68b8827f4738
Re: [HACKERS] What happened to pl/proxy and FDW?
On ons, 2009-12-09 at 08:56 -0500, Ing. Marcos Ortiz Valmaseda wrote: Which is the development state of SQL/MED? That depends on what features of SQL/MED you are interested in. As you could read upthread, PL/Proxy support is being worked on. Dblink supports it in 8.4. The next step might be foreign table support. I recall that there was a patch proposed for that a couple of commitfests ago, but that did not go further. -- 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] pg_ctl init extension
Robert Haas píše v st 09. 12. 2009 v 08:56 -0500: On Dec 9, 2009, at 8:32 AM, Zdenek Kotala zdenek.kot...@sun.com wrote: snip Ahh, It is somethink what I want to do, but it is not ready yet in this patch, but I already documented it. Idea is to install initdb and postgres into libexecdir and packager can select if libexecdir will be equal bindir or not. The paragraph should be removed at this moment. Shell I send modified patch or does committer remove it before commit? I think Peter claimed this one but as far as I am concerned, I would always rather have an updated patch. OK, here it is. Thanks Zdenek diff -r ab32ed8164e7 doc/src/sgml/config.sgml --- a/doc/src/sgml/config.sgml Mon Dec 07 15:10:09 2009 +0100 +++ b/doc/src/sgml/config.sgml Wed Dec 09 15:17:54 2009 +0100 @@ -54,9 +54,10 @@ para One way to set these parameters is to edit the file filenamepostgresql.conf/indextermprimarypostgresql.conf//, -which is normally kept in the data directory. (applicationinitdb/ -installs a default copy there.) An example of what this file might look -like is: +which is normally kept in the data directory. +(link linkend=creating-clusterdatabase cluster initialization/link +process installs a default copy there.) +An example of what this file might look like is: programlisting # This is a comment log_connections = yes @@ -365,8 +366,8 @@ Determines the maximum number of concurrent connections to the database server. The default is typically 100 connections, but might be less if your kernel settings will not support it (as -determined during applicationinitdb/). This parameter can -only be set at server start. +determined during link linkend=creating-clusterdatabase cluster +initialization/link). This parameter can only be set at server start. /para para @@ -747,7 +748,8 @@ Sets the amount of memory the database server uses for shared memory buffers. The default is typically 32 megabytes (literal32MB/), but might be less if your kernel settings will -not support it (as determined during applicationinitdb/). +not support it (as determined during link linkend=creating-clusterdatabase +cluster initialization)/link. This setting must be at least 128 kilobytes. (Non-default values of symbolBLCKSZ/symbol change the minimum.) However, settings significantly higher than the minimum are usually needed @@ -4267,10 +4269,10 @@ keywords literalUS/, literalNonEuro/, and literalNonEuropean/ are synonyms for literalMDY/. See xref linkend=datatype-datetime for more information. The -built-in default is literalISO, MDY/, but -applicationinitdb/application will initialize the -configuration file with a setting that corresponds to the -behavior of the chosen varnamelc_time/varname locale. +built-in default is literalISO, MDY/, but +link linkend=creating-clusterdatabase cluster initialization/link +will initialize the configuration file with a setting that corresponds +to the behavior of the chosen varnamelc_time/varname locale. /para /listitem /varlistentry @@ -4476,9 +4478,9 @@ specifying the configuration. See xref linkend=textsearch for further information. The built-in default is literalpg_catalog.simple/, but -applicationinitdb/application will initialize the -configuration file with a setting that corresponds to the -chosen varnamelc_ctype/varname locale, if a configuration +link linkend=creating-clusterdatabase cluster initialization/link +will initialize the configuration file with a setting that corresponds +to the chosen varnamelc_ctype/varname locale, if a configuration matching that locale can be identified. /para /listitem @@ -5240,8 +5242,9 @@ listitem para Allows modification of the structure of system tables. -This is used by commandinitdb/command. -This parameter can only be set at server start. +This is used by link linkend=creating-clusterdatabase cluster +initialization/link process. This parameter can only be set at server +start. /para /listitem /varlistentry diff -r ab32ed8164e7 doc/src/sgml/manage-ag.sgml --- a/doc/src/sgml/manage-ag.sgml Mon Dec 07 15:10:09 2009 +0100 +++ b/doc/src/sgml/manage-ag.sgml Wed Dec 09 15:17:54 2009 +0100 @@ -107,10 +107,9 @@ Since you need to be connected to the database server in order to execute the commandCREATE DATABASE/command command, the question remains how the emphasisfirst/ database at any given - site can be created. The first database is always created by the - commandinitdb/ command when the data storage area is -
Re: [HACKERS] [ADMIN] recovery is stuck when children are not processing SIGQUIT from previous crash
Peter Eisentraut pete...@gmx.net writes: Yeah, on reflection, calling elog in the SIGQUIT handler is just waiting for trouble. The call could block for any number of reasons, because there is a boatload of functionality that comes with a logging call. In the overall scheme of things, you don't really lose much if you just delete the call altogether, because in the event that it's called the postmaster will already have logged that it is going to kill all children. Or there ought to be some kind of alarm that would abort the thing if it takes too long. Well, the point of that call is not to log the event. The point is to tell the client why it's losing its connection. Admittedly there are assorted corner cases where that would fail, but it works well enough often enough that I don't want to just delete the attempt. 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] XLogInsert
Jaime Casanova jcasa...@systemguards.com.ec writes: i haven't made any performance tests but it should gain something :), maybe someone can make those tests? The argument for changing this at all is only that it will improve performance, so I'd like some independent confirmation that it does. 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] Streaming replication and non-blocking I/O
Fujii Masao masao.fu...@gmail.com writes: On Wed, Dec 9, 2009 at 3:58 PM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: But if everyone is happy with just relying on the OS buffer to not fill up, let's just drop it. The OS buffer is expected to be able to store a large number of XLogRecPtr messages, because its size is small. So it's also OK to just drop it. It certainly seems to be something we could improve later, when and if evidence emerges that it's a real-world problem. For now, simple is beautiful. 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] Windows x64 [repost]
Tatsuo, Ok. Your suggestion is very helpfull. In general Tsutomu will wait for feedbacks come in, probably until Jan 15th. BTW, is there anyone who wishes the patches get in 8.5? Apparently Tstutomu, Magnus and I are counted in the group:-) But I'd like to know how other people are interested in the patches. I am very interested. A 64bit-Windows-Version would give a boost perception-wise (I know the technical arguments about usefullness or not, but perception is different and often quite important) Totally agreed. -- Tatsuo Ishii SRA OSS, Inc. Japan -- 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] Windows x64 [repost]
BTW, is there anyone who wishes the patches get in 8.5? Apparently Tstutomu, Magnus and I are counted in the group:-) But I'd like to know how other people are interested in the patches. I am very interested. A 64bit-Windows-Version would give a boost perception-wise I'm also very interested - despite the fact it'll cause me a boatload of work to produce a new set of installers for this architecture! Sorry for this:-) -- Tatsuo Ishii SRA OSS, Inc. Japan -- 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] Build sizes vs docs
2009/12/7 Tom Lane t...@sss.pgh.pa.us: Magnus Hagander mag...@hagander.net writes: Came cross this when updating the cvs fix. We declare size requirements as: Also check that you have sufficient disk space. You will need about 65 MB for the source tree during compilation and about MB for the installation directory. An empty database cluster takes about 25 MB; databases take about five times the amount of space that a flat text file with the same data would take. If you are going to run the regression tests you will temporarily need up to an extra 90 MB. Use the commanddf/command command to check free disk My source *without* compile is 82 Mb, and with a build in it (linux i686) is 110Mb during compilations, rather than 65. An empty cluster takes about 33Mb. The regression database takes about 132Mb. (this is 8.4) Should we fix these numbers, or just remove them? They're clearly platform dependent, but perhaps there's still a point in including them - mainly as hints? Maybe round them off to an order of magnitude. I think it's useful to have some idea of the size requirements, even if they change over time. It wouldn't be a bad idea to say as of 8.4 or some such, too. Hmm, I don't like that, it'll just make things look old :-) For now I've applied a patch to update the values with what it is now. Perhaps we should just add it to the release checklist to verify that they are reasonably correct? Shouldn't take too long, and it's not likely it'll ever change in a minor release - only major releases are interesting. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Windows x64 [repost]
2009/12/9 Tatsuo Ishii is...@postgresql.org: Ok. Your suggestion is very helpfull. In general Tsutomu will wait for feedbacks come in, probably until Jan 15th. Of course there's also no rule that you couldn't review these sooner - that might help get the ball rolling! Of course I did before he publishes the patches. (I and he are working for the same company). However I'm not a Windows programmer by no means. So my suggestion was mainly for designs... :-) As a reference for the future, please let us know when you have done this before the patch is submitted. I think it's not very common that just because you are in the same company, you have reviewed it. For example, I highly doubt that Heikki reviews all the patches Bruce post, or the other way around :-) And it's very useful to know that one set of eyes have been on it already. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Build sizes vs docs
Magnus Hagander mag...@hagander.net writes: Perhaps we should just add it to the release checklist to verify that they are reasonably correct? Go for it. Now that you mention it, there are some memory-usage tables in the documentation about shared memory configuration that also have a pretty short half-life, and should be rechecked. 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] Build sizes vs docs
2009/12/9 Tom Lane t...@sss.pgh.pa.us: Magnus Hagander mag...@hagander.net writes: Perhaps we should just add it to the release checklist to verify that they are reasonably correct? Go for it. Now that you mention it, there are some memory-usage tables in the documentation about shared memory configuration that also have a pretty short half-life, and should be rechecked. Done. They're up-to-date per 8.3, btw. Are there changes in 8.4 that should be put in? If so, it would be good to include it before the next minor release, so the proper data goes up on the website. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Build sizes vs docs
Magnus Hagander wrote: 2009/12/9 Tom Lane t...@sss.pgh.pa.us: Magnus Hagander mag...@hagander.net writes: Perhaps we should just add it to the release checklist to verify that they are reasonably correct? Go for it. Now that you mention it, there are some memory-usage tables in the documentation about shared memory configuration that also have a pretty short half-life, and should be rechecked. Done. They're up-to-date per 8.3, btw. Are there changes in 8.4 that should be put in? If so, it would be good to include it before the next minor release, so the proper data goes up on the website. At the very least we don't have the FSM bits anymore ... -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- 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] Clearing global statistics
On sön, 2009-12-06 at 19:50 -0500, Greg Smith wrote: The fact that you're asking the question this way suggests to me I've named this completely wrong. pg_stat_reset_global only resets the bits global to all databases. It doesn't touch any of the database-specific things that pg_stat_reset can handle right now. At the moment, the only global information is what's in pg_stat_bgwriter: buffer statistics and checkpoint stats. I'm thinking that I should rename this new function to pg_stat_reset_bgwriter so it's obvious how limited its target is. Using either global or cluster for the name is just going to leave people thinking it acts across a much larger area than it does. The term shared is used elsewhere to describe the, well, shared catalogs. -- 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] Need a mentor, and a project.
On mån, 2009-12-07 at 09:53 +0100, Albe Laurenz wrote: I would start with the TODO list: http://wiki.postgresql.org/wiki/Todo These are things for which there is a consensus that it would be a good idea to implement them. The Todo list is not a list of things for which such a consensus exists. The Todo list is in general a list of things that someone thought should be considered at some point. But unless the item is linked to a mailing list thread that already shows a consensus about the feature, you need to start with a discussion about a plan. So don't submit a project plan to your university or boss based on I will work on item X because it's on the Todo list without taking ample time to discuss things here first. -- 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] tsearch parser inefficiency if text includes urls or emails - new version
On Tuesday 08 December 2009 17:15:36 Kevin Grittner wrote: Andres Freund and...@anarazel.de wrote: Could you show your testcase? OK. I was going to try to check other platforms first, and package up the information better, but here goes. I created 1 lines with random IP-based URLs for a test. The first few lines are: create table t1 (c1 int not null primary key, c2 text); insert into t1 values (2, 'http://255.102.51.212/*/quick/brown/fox?jumpsover*lazydog.html http://204.56.222.143/*/quick/brown/fox?jumpsover*lazydog.html http://138.183.168.227/*/quick/brown/fox?jumpsover*lazydog.html I think you see no real benefit, because your strings are rather short - the documents I scanned when noticing the issue where rather long. If your strings are short, they and the copy will fit into cpu cache anyway, so copying them around/converting them to some other string format is not that expensive compared to the rest of the work done. Also after each copying step for large strings the complete cache is filled with unrelated information (namely the end of the string). So every charwise access will need to wait for a memory access. A rather extreme/contrived example: postgres=# SELECT 1 FROM to_tsvector(array_to_string(ARRAY(SELECT 'and...@anarazel.de http://www.postgresql.org/'::text FROM generate_series(1, 1) g(i)), ' - ')); ?column? -- 1 (1 row) Time: 3.740 ms postgres=# SELECT 1 FROM to_tsvector(array_to_string(ARRAY(SELECT 'and...@anarazel.de http://www.postgresql.org/'::text FROM generate_series(1, 1000) g(i)), ' - ')); ?column? -- 1 (1 row) Time: 115.027 ms postgres=# SELECT 1 FROM to_tsvector(array_to_string(ARRAY(SELECT 'and...@anarazel.de http://www.postgresql.org/'::text FROM generate_series(1, 1) g(i)), ' - ')); ?column? -- 1 (1 row) Time: 24355.339 ms postgres=# SELECT 1 FROM to_tsvector(array_to_string(ARRAY(SELECT 'and...@anarazel.de http://www.postgresql.org/'::text FROM generate_series(1, 2) g(i)), ' - ')); ?column? -- 1 (1 row) Time: 47276.739 ms One easily can see the quadratic complexity here. The quadratic complexity lies in the length/amount of emails/urls of the strings, not in the number of to_tsvector calls! After the patch: postgres=# SELECT 1 FROM to_tsvector(array_to_string(ARRAY(SELECT 'and...@anarazel.de http://www.postgresql.org/'::text FROM generate_series(1, 2) g(i)), ' - ')); ?column? -- 1 (1 row) Time: 168.384 ms I could not reproduce the slowdown you mentioned: Without patch: postgres=# SELECT to_tsvector('and...@anarazel.de http://www.postgresql.org/'||g.i::text) FROM generate_series(1, 10) g(i) ORDER BY g.i LIMIT 1; to_tsvector --- '/1':4 'and...@anarazel.de':1 'www.postgresql.org':3 'www.postgresql.org/1':2 (1 row) Time: 1109.833 ms With patch: postgres=# SELECT to_tsvector('and...@anarazel.de http://www.postgresql.org/'||g.i::text) FROM generate_series(1, 10) g(i) ORDER BY g.i LIMIT 1; to_tsvector --- '/1':4 'and...@anarazel.de':1 'www.postgresql.org':3 'www.postgresql.org/1':2 (1 row) Time: 1036.689 ms So on the hardware I tried its even a little bit faster for small strings (Older Xeon32bit, Core2 Duo, 64bit, Nehalem based Xeon 64bit). I could not reproduce any difference with strings not involving urls or emails: Without patch: postgres=# SELECT to_tsvector('live hard, love fast, die young'||g.i::text) FROM generate_series(1, 10) g(i) ORDER BY g.i LIMIT 1; to_tsvector 'die':5 'fast':4 'hard':2 'live':1 'love':3 'young1':6 (1 row) Time: 988.426 ms With patch: postgres=# SELECT to_tsvector('live hard, love fast, die young'||g.i::text) FROM generate_series(1, 10) g(i) ORDER BY g.i LIMIT 1; to_tsvector 'die':5 'fast':4 'hard':2 'live':1 'love':3 'young1':6 (1 row) Time: 975.339 ms So at least in my testing I do see no danger in the patch ;-) Andres PS: I averaged all the time result over multiple runs where it was relevant. -- 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
Tom Lane wrote: Jaime Casanova jcasa...@systemguards.com.ec writes: i haven't made any performance tests but it should gain something :), maybe someone can make those tests? The argument for changing this at all is only that it will improve performance, so I'd like some independent confirmation that it does. I've done a little review of this myself, and I'm not quite happy with how this patch was delivered to us. The bar for committing something that touches the WAL is really high--it needs to be a unquestionable win to touch that code. The justification of the patch makes the overall code a bit cleaner is a hard sell on something that's hard to debug (triggering bad WAL situations at will isn't easy) and critical to the system. If there's a clear performance improvement, that helps justify why it's worth working on. Here's the original performance justification: Using the only XLogInsert-bound test case I know of, parallel COPY into a skinny, unindexed table, using 8 parallel copies on a 4 x dual-core x86_64 and with fsync turned off (to approxiamately simulate SSD, which I do not have), I get a speed improvement of 2-4% with the patch over unpatched head. That makes sense, and using this spec I could probably come up with the test program to reproduce this. But I'm getting tired of doing that. It's hard enough to reproduce performance changes when someone gives the exact configuration and test program they used. If we're working with a verbal spec for how to reproduce the issues, forget about it--that's more than we can expect a reviewer to handle, and the odds of that whole thing ending well are low. Jeff: before we do anything else with your patch, I'd like to see a script of some sort that runs the test you describe above, everything changed in the postgresql.conf from the defaults, and the resulting raw numbers that come from it on your system that prove an improvement there--not just a summary of the change. That's really mandatory for a performance patch. If any reviewer who's interested can't just run something and get a report suggesting whether the patch helped or harmed results in five minutes, unless we really, really want your patch it's just going to stall at that point. And unfortunately, in the case of something that touches the WAL path, we really don't want to change anything unless there's a quite good reason to do so. I've also realized that Patch LWlocks instrumentation at http://archives.postgresql.org/message-id/op.uz8sfkxycke...@soyouz should have been evaluated as its own patch altogether. I think that the test program you're suggesting also proves its utility though, so for now I'll keep them roped together. Sorry this ended up so late in this CommitFest, just a series of unexpected stuff rippled down to you. On the bright side, had you submitted this before the whole organized CF process started, you could have waited months longer to get the same feedback. -- Greg Smith2ndQuadrant Baltimore, MD PostgreSQL Training, Services and Support g...@2ndquadrant.com www.2ndQuadrant.com
Re: [HACKERS] Need a mentor, and a project.
Peter Eisentraut wrote: But unless the item is linked to a mailing list thread that already shows a consensus about the feature, you need to start with a discussion about a plan. And realistically, even if the item is so linked, someone new to the project still shouldn't just plow away on it without asking for confirmation first anyway. There are many things on the TODO list that everyone would like to see fixed, the problem is well defined and unambiguous, but the way the solution needs to be structured is much harder than is obvious. As a simplest example, we regularly have people show up with patches where the solution was just add threading to the back-end here... which might seem completely reasonable to someone new--but it will never get committed. -- 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
[HACKERS] thread safety on clients
Hi, I compiled current HEAD and trying to use pgbench, i initialized a test database this way: bin/pgbench -i -F80 -s100 test and then run with this options: bin/pgbench -c 50 -j 5 -l -t 20 test and get this crash: starting vacuum...end. TRAP: FailedAssertion(!((data - start) == data_size), File: heaptuple.c, Line: 255) Client 0 aborted in state 8. Probably the backend died while processing. LOG: server process (PID 30713) was terminated by signal 6: Aborted TRAP: FailedAssertion(!((data - start) == data_size), File: heaptuple.c, Line: 255) Client 8 aborted in state 8. Probably the backend died while processing. LOG: terminating any other active server processes WARNING: terminating connection because of crash of another server process DETAIL: The postmaster has commanded this server process to roll back the current transaction and exit, because another server process exited abnormally and possibly corrupted shared memory. if i remove the -j option then it runs without a problem -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] explain output infelicity in psql
I have just noticed while checking the EXPLAIN YAML patch that the non-text explain formats are output as a single line with embedded line feeds, while the text format is delivered as a set of text records, one per line. The practical effect of this is that psql decorates the non-text format output with continuation characters: andrew=# explain select count(*) from pg_class where relname ~ 'pg_'; QUERY PLAN Aggregate (cost=9.67..9.68 rows=1 width=0) - Seq Scan on pg_class (cost=0.00..9.16 rows=204 width=0) Filter: (relname ~ 'pg_'::text) (3 rows) Time: 5.813 ms andrew=# explain (format yaml) select count(*) from pg_class where relname ~ 'pg_'; QUERY PLAN - - Plan: + Node Type: Aggregate + Strategy: Plain+ Startup Cost: 9.67 + Total Cost: 9.68 + Plan Rows: 1 + Plan Width: 0 + Plans: + - Node Type: Seq Scan+ Parent Relationship: Outer + Relation Name: pg_class+ Alias: pg_class+ Startup Cost: 0.00 + Total Cost: 9.16 + Plan Rows: 204 + Plan Width: 0 + Filter: (relname ~ 'pg_'::text) (1 row) Those + chars at the end of the line are ugly, to say the least, and they make the supposedly machine-readable formats not so machine readable if anyone wanted to cp the output into a parser. (I'm mildly surprised this hasn't been noticed before). Maybe we need to teach psql not to do this formatting for EXPLAIN output? 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] explain output infelicity in psql
On Wed, Dec 9, 2009 at 2:37 PM, Andrew Dunstan and...@dunslane.net wrote: I have just noticed while checking the EXPLAIN YAML patch that the non-text explain formats are output as a single line with embedded line feeds, while the text format is delivered as a set of text records, one per line. The practical effect of this is that psql decorates the non-text format output with continuation characters: andrew=# explain select count(*) from pg_class where relname ~ 'pg_'; QUERY PLAN Aggregate (cost=9.67..9.68 rows=1 width=0) - Seq Scan on pg_class (cost=0.00..9.16 rows=204 width=0) Filter: (relname ~ 'pg_'::text) (3 rows) Time: 5.813 ms andrew=# explain (format yaml) select count(*) from pg_class where relname ~ 'pg_'; QUERY PLAN - - Plan: + Node Type: Aggregate + Strategy: Plain + Startup Cost: 9.67 + Total Cost: 9.68 + Plan Rows: 1 + Plan Width: 0 + Plans: + - Node Type: Seq Scan + Parent Relationship: Outer + Relation Name: pg_class + Alias: pg_class + Startup Cost: 0.00 + Total Cost: 9.16 + Plan Rows: 204 + Plan Width: 0 + Filter: (relname ~ 'pg_'::text) (1 row) Those + chars at the end of the line are ugly, to say the least, and they make the supposedly machine-readable formats not so machine readable if anyone wanted to cp the output into a parser. (I'm mildly surprised this hasn't been noticed before). Maybe we need to teach psql not to do this formatting for EXPLAIN output? Oh, dear. I think that line continuation syntax was recently added - subsequent to the machine-readable EXPLAIN patch. The reason why it's coded to emit everything as a single row is because that will be most convenient for programs that are sucking down this data programatically. Otherwise, they'll have to concatenate all the lines that are returned. And in fact for XML format, it's even worse: the data is returned as type xml, but that obviously won't fly if we return each line as a separate tuple. On first blush, I'm inclined to suggest that the addition of + signs to mark continuation lines is a misfeature. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Has anyone used CLANG yet?
This is a C front end for the LLVM compiler... I noticed that it entered Debian/Unstable today: http://packages.debian.org/sid/main/clang I thought it would be interesting to see if PostgreSQL compiles with this, as an alternative compiler that should presumably become more and more available on Linux et al. (And I suppose that the randomly selected .sig is supremely apropos!) configure blows up here at the following: conftest.c:75:28: error: invalid token after top level declarator extern unsigned int PASCAL accept (unsigned int, void *, void *); I suspect there's something about PASCAL that's a problem, as clang is nominally supposed to be a C compiler ;-). I haven't looked deeper, so haven't the remotest idea how deep the issue lies. At any rate, I should poke at this further soon, but if it seems interesting to others, well, CLANG is now an easy install on some number of systems! -- let name=cbbrowne and tld=gmail.com in name ^ @ ^ tld;; http://linuxdatabases.info/info/postgresql.html The problem with the cutting edge is that someone has to bleed. -- Zalman Stern -- 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] Has anyone used CLANG yet?
Chris Browne wrote: I suspect there's something about PASCAL that's a problem, as clang is nominally supposed to be a C compiler ;-). Pascal refers to a way of different way of pushing things onto the stack when calling things; there's Pascal order and c order when you call a function, each approach has its good and bad sides. There's work in progress to support different calling conventions including Pascal order for LLVM at http://nondot.org/sabre/LLVMNotes/CustomCallingConventions.txt . At this point, supporting different call conventions is supported in LLVM 1.5: http://llvm.org/releases/1.5/docs/LangRef.html#callingconv but it doesn't look like the syntax to support the Pascal one has made it in there yet. Probably requires a fairly small patch to LLVM now that the main infrastructure is available. Don't know if it's feasible to rip all the Pascal convention code out PostgreSQL, that's the other approach. -- 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] Has anyone used CLANG yet?
On Dec 9, 2009, at 4:23 PM, Chris Browne wrote: This is a C front end for the LLVM compiler... I noticed that it entered Debian/Unstable today: http://packages.debian.org/sid/main/clang I thought it would be interesting to see if PostgreSQL compiles with this, as an alternative compiler that should presumably become more and more available on Linux et al. (And I suppose that the randomly selected .sig is supremely apropos!) configure blows up here at the following: conftest.c:75:28: error: invalid token after top level declarator extern unsigned int PASCAL accept (unsigned int, void *, void *); I suspect there's something about PASCAL that's a problem, as clang is nominally supposed to be a C compiler ;-). I haven't looked deeper, so haven't the remotest idea how deep the issue lies. At any rate, I should poke at this further soon, but if it seems interesting to others, well, CLANG is now an easy install on some number of systems! Clang works for me on MacOS 10.6.2: /Developer/usr/bin/clang --version clang version 1.0.1 (http://llvm.org/svn/llvm-project/cfe/tags/Apple/clang-24 exported) Target: x86_64-apple-darwin10 CC=/Developer/usr/bin/clang ./configure --prefix=/Users/agentm/pgsql841/ make -j 8 /Users/agentm/pgsql841/initdb -E UTF8 ../data ./pg_ctl -D ../data/ start server starting RD07:bin agentm$ LOG: database system was shut down at 2009-12-09 17:01:51 EST LOG: database system is ready to accept connections LOG: autovacuum launcher started /Users/agentm/pgsql841/psql postgres psql (8.4.1) Type help for help. postgres=# select 1; ?column? -- 1 (1 row) I do see lots of warnings regarding unsupported compiler flags: clang: warning: argument unused during compilation: '-no-cpp-precomp' clang: warning: argument unused during compilation: '-O2' clang: warning: argument unused during compilation: '-Wall' clang: warning: argument unused during compilation: '-Wmissing-prototypes' clang: warning: argument unused during compilation: '-Wpointer-arith' clang: warning: argument unused during compilation: '-Wdeclaration-after-statement' clang: warning: argument unused during compilation: '-Wendif-labels' clang: warning: argument unused during compilation: '-fno-strict-aliasing' clang: warning: argument unused during compilation: '-fwrapv' and some code-based warnings: print.c:1105:24: warning: field width should have type 'int', but argument has type 'unsigned int' [-Wformat] fprintf(fout, %-s%*s, hlineptr[line_count].ptr, ^ pl_exec.c:3529:6: warning: expression result unused [-Wunused-value] ItemPointerSetInvalid((tmptup.t_self)); ^~~ ../../../../src/include/storage/itemptr.h:134:2: note: instantiated from: BlockIdSet(((pointer)-ip_blkid), InvalidBlockNumber), \ ^ ../../../../src/include/storage/block.h:86:2: note: instantiated from: AssertMacro(PointerIsValid(blockId)), \ ^ ../../../../src/include/postgres.h:675:39: note: instantiated from: #define AssertMacro(condition) ((void)true) ^ ../../../../src/include/c.h:185:15: note: instantiated from: #define true((bool) 1) You are probably running configure with gcc, no? FYI: with clang: time make (not -j 8) real1m46.511s user1m26.295s sys 0m14.639s with gcc: time make real2m41.934s user2m20.778s sys 0m17.441s du -h pgsql841gcc/bin/* 52Kpgsql841gcc/bin/clusterdb 52Kpgsql841gcc/bin/createdb 60Kpgsql841gcc/bin/createlang 52Kpgsql841gcc/bin/createuser 52Kpgsql841gcc/bin/dropdb 60Kpgsql841gcc/bin/droplang 52Kpgsql841gcc/bin/dropuser 616Kpgsql841gcc/bin/ecpg 72Kpgsql841gcc/bin/initdb 32Kpgsql841gcc/bin/pg_config 28Kpgsql841gcc/bin/pg_controldata 36Kpgsql841gcc/bin/pg_ctl 280Kpgsql841gcc/bin/pg_dump 68Kpgsql841gcc/bin/pg_dumpall 36Kpgsql841gcc/bin/pg_resetxlog 128Kpgsql841gcc/bin/pg_restore 4.6Mpgsql841gcc/bin/postgres 4.0Kpgsql841gcc/bin/postmaster 340Kpgsql841gcc/bin/psql 52Kpgsql841gcc/bin/reindexdb 32Kpgsql841gcc/bin/vacuumdb du -h pgsql841/bin/* (clang build) 52Kpgsql841/bin/clusterdb 52Kpgsql841/bin/createdb 60Kpgsql841/bin/createlang 52Kpgsql841/bin/createuser 48Kpgsql841/bin/dropdb 60Kpgsql841/bin/droplang 48Kpgsql841/bin/dropuser 612Kpgsql841/bin/ecpg 72Kpgsql841/bin/initdb 28Kpgsql841/bin/pg_config 28Kpgsql841/bin/pg_controldata 36Kpgsql841/bin/pg_ctl 272Kpgsql841/bin/pg_dump 68Kpgsql841/bin/pg_dumpall 36Kpgsql841/bin/pg_resetxlog 124Kpgsql841/bin/pg_restore 4.5Mpgsql841/bin/postgres 4.0Kpgsql841/bin/postmaster 344Kpgsql841/bin/psql 52Kpgsql841/bin/reindexdb 32Kpgsql841/bin/vacuumdb Cheers, M
Re: [HACKERS] Adding support for SE-Linux security
Robert Haas wrote: On Wed, Dec 9, 2009 at 1:44 AM, Magnus Hagander mag...@hagander.net wrote: 2009/12/9 Bruce Momjian br...@momjian.us: I frankly think the patch should be thought of as the SE-Linux-specific directory files, which KaiGai can maintain, and the other parts, which I think I can handle. I think that's a horribly bad idea. Me, too. The ECPG comparison is apt, except that this code is far more deeply integrated into core. The idea that the SE-Linux directory files can be maintained separately from the other parts does not seem realistic to me. The problems that are going to occur here are things like: somebody wants to rearrange some part of the permissions checking for some reason. So they move a bunch of code around and break SE-PostgreSQL. Someone has to review that patch and understand the danger it causes. That's going to require understanding both the SE-PostgreSQL-specific files and the other parts, and the relationship between the two of them. We did something similar for Win32 because it was the only way to do it. We don't have the luxury of educating our developers on SE-Linux API for a while --- there is the ideal world, and there is reality. What this means is that SE-Linux would break when permissions changes happen, and the SE-Linux folks will have to come in and clean things up later. If you want to avoid all good reasons for this features and are looking for reasons why this patch is a bad idea, I am sure you can find them. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] explain output infelicity in psql
Robert Haas wrote: On Wed, Dec 9, 2009 at 2:37 PM, Andrew Dunstan and...@dunslane.net wrote: I have just noticed while checking the EXPLAIN YAML patch that the non-text explain formats are output as a single line with embedded line feeds, while the text format is delivered as a set of text records, one per line. The practical effect of this is that psql decorates the non-text format output with continuation characters: [snip] Those + chars at the end of the line are ugly, to say the least, and they make the supposedly machine-readable formats not so machine readable if anyone wanted to cp the output into a parser. (I'm mildly surprised this hasn't been noticed before). Maybe we need to teach psql not to do this formatting for EXPLAIN output? Oh, dear. I think that line continuation syntax was recently added - subsequent to the machine-readable EXPLAIN patch. The reason why it's coded to emit everything as a single row is because that will be most convenient for programs that are sucking down this data programatically. Otherwise, they'll have to concatenate all the lines that are returned. And in fact for XML format, it's even worse: the data is returned as type xml, but that obviously won't fly if we return each line as a separate tuple. On first blush, I'm inclined to suggest that the addition of + signs to mark continuation lines is a misfeature. I certainly agree we don't want to break up the non-text formats. A simple if ugly hack would make psql use old-ascii print style (which doesn't use these contionuation chars) if the first attribute in the resultset was named 'QUERY PLAN' If someone has a better suggestion I'm all ears. 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] [PATCH] Windows x64 [repost]
As a reference for the future, please let us know when you have done this before the patch is submitted. I think it's not very common that just because you are in the same company, you have reviewed it. For example, I highly doubt that Heikki reviews all the patches Bruce post, or the other way around :-) And it's very useful to know that one set of eyes have been on it already. Ok, next time I will do that. As I said before, I'm not a good Windows programmer at all and hesitated to say that Trust me, I have reviewd his patches:-) -- Tatsuo Ishii SRA OSS, Inc. Japan -- 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] executor and slru dtrace probes
On Dec 8, 2009, at 5:10 AM, Zdenek Kotala wrote: Dne 8.12.09 00:27, Bernd Helmle napsal(a): --On 13. November 2009 23:29:41 +0100 Zdenek Kotala zdenek.kot...@sun.com wrote: t contains two DTrace probe groups. One is related to monitoring SLRU and second is about executor nodes. I merged it with the head. Original end of mail thread is here: http://archives.postgresql.org/pgsql-hackers/2009-04/msg00148.php I've started to review this. It seems to me the attached patch wasn't adjusted or discussed again to address Tom's complaints? At least the executor probes contained here hold still the same issues mentioned by Tom in the discussion linked here. I did not make any change. I only revival patch and merge it with head. I think that SLRU probes are OK and acceptable. Tom's issues with executor probes are still there and I expect discussion about them. IIRC Theo uses these probes in production. If you think that it is better I could split patch into two separate patches and both can be reviewed separately. I suppose I see it as a simple thing. The probes have no performance impact when they are not instrumented. I've used them on rare occasion to understand which exec nodes are causing which disk accesses. Seemed pretty darn useful at the time. There is (of course) some performance overhead when they are enabled, but that is intentionally performed by the operator, so it seems like a non-issue. Now, there was some indication that there was a better place to probe that would be more comprehensive -- that should be addressed. -- Theo Schlossnagle Esoteric Curio -- http://lethargy.org/ OmniTI Computer Consulting, Inc. -- http://omniti.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] Adding support for SE-Linux security
Bruce Momjian wrote: Robert Haas wrote: On Wed, Dec 9, 2009 at 1:44 AM, Magnus Hagander mag...@hagander.net wrote: 2009/12/9 Bruce Momjian br...@momjian.us: I frankly think the patch should be thought of as the SE-Linux-specific directory files, which KaiGai can maintain, and the other parts, which I think I can handle. I think that's a horribly bad idea. Me, too. The ECPG comparison is apt, except that this code is far more deeply integrated into core. The idea that the SE-Linux directory files can be maintained separately from the other parts does not seem realistic to me. The problems that are going to occur here are things like: somebody wants to rearrange some part of the permissions checking for some reason. So they move a bunch of code around and break SE-PostgreSQL. Someone has to review that patch and understand the danger it causes. That's going to require understanding both the SE-PostgreSQL-specific files and the other parts, and the relationship between the two of them. We did something similar for Win32 because it was the only way to do it. We don't have the luxury of educating our developers on SE-Linux API for a while --- there is the ideal world, and there is reality. What this means is that SE-Linux would break when permissions changes happen, and the SE-Linux folks will have to come in and clean things up later. If you want to avoid all good reasons for this features and are looking for reasons why this patch is a bad idea, I am sure you can find them. Right, I (and my employer) offers development and maintenance resource for the feature. If I'll be busy in future days, it means I'm devotedly working on this feature. When we need to change permission mechanism in the future, we can provide our efforts not to break them. -- OSS Platform Development Division, NEC KaiGai Kohei kai...@ak.jp.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] unprivileged user
The other day I returned idly to thinking about some work I did a few years ago on creating a totally unprivileged user, i.e. one with not even public permissions. The work I did then involved hacking the pg_catalog, information_schema and public schemas and their contents. Unfortunately, it doesn't survive a dump and restore, so it's really quite fragile. But it occurred to me that this could be more easily and robustly accomplished if we create a role attribute of NOPUBLIC or some such. Thoughts? 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] Adding support for SE-Linux security
On Wed, Dec 9, 2009 at 5:38 PM, Bruce Momjian br...@momjian.us wrote: If you want to avoid all good reasons for this features and are looking for reasons why this patch is a bad idea, I am sure you can find them. You seem to be suggesting that our reactions are pure obstructionism, or that they have an ulterior motive. ...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] bug: fuzzystrmatch levenshtein is wrong
On Tue, Dec 8, 2009 at 9:45 PM, Robert Haas robertmh...@gmail.com wrote: On Tue, Dec 8, 2009 at 9:08 PM, Bruce Momjian br...@momjian.us wrote: Robert Haas wrote: patch attached. I cannot get this patch to apply for anything. All 4 hunks fail, both on HEAD and on the the pre-8.4-pgindent version of fuzzystrmatch.c. Either I'm doing something wrong here, or there's something wrong with this patch file. The author converted tabs to spaces --- there is not a single tab in the diff file. Ah. I knew there had to be a reason. I'm attaching my version of this patch. Barring objections, I am going to apply this to HEAD and backpatch to 8.4, where this feature (and the associated bug) were introduced. Done. Yeah, my first commit! However, it appears that someone needs to tell the pgsql-committers list that rh...@postgresql.org is allowed to post there. ...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] ProcessUtility_hook
Why does this patch #ifdef out the _PG_fini code in pg_stat_statements? Where you check for INSERT, UPDATE, and DELETE return codes in pgss_ProcessUtility, I think this deserves a comment explaining that these could occur as a result of EXECUTE. It wasn't obvious to me, anyway. It seems to me that the current hook placement does not address this complaint 1. The placement of the hook. Why is it three lines down in ProcessUtility? It's probably reasonable to have the Assert first, but I don't see why the hook function should have the ability to editorialize on the behavior of everything about ProcessUtility *except* the read-only-xact check. ...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] ProcessUtility_hook
Robert Haas robertmh...@gmail.com wrote: Why does this patch #ifdef out the _PG_fini code in pg_stat_statements? That's because _PG_fini is never called in current releases. We could remove it completely, but I'd like to leave it for future releases where _PG_fini callback is re-enabled. Or, removing #ifdef (enabling _PG_fini function) is also harmless. Where you check for INSERT, UPDATE, and DELETE return codes in pgss_ProcessUtility, I think this deserves a comment explaining that these could occur as a result of EXECUTE. It wasn't obvious to me, anyway. Like this? /* * Parse command tag to retrieve the number of affected rows. * COPY command returns COPY tag. EXECUTE command might return INSERT, * UPDATE, or DELETE tags, but we cannot retrieve the number of rows * for SELECT. We assume other commands always return 0 row. */ It seems to me that the current hook placement does not address this complaint I don't see why the hook function should have the ability to editorialize on the behavior of everything about ProcessUtility *except* the read-only-xact check. I moved the initialization code of completionTag as the comment, but check_xact_readonly() should be called before the hook. Am I missing something? Regards, --- Takahiro Itagaki NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ProcessUtility_hook
On Wed, Dec 9, 2009 at 9:33 PM, Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp wrote: Robert Haas robertmh...@gmail.com wrote: Why does this patch #ifdef out the _PG_fini code in pg_stat_statements? That's because _PG_fini is never called in current releases. We could remove it completely, but I'd like to leave it for future releases where _PG_fini callback is re-enabled. Or, removing #ifdef (enabling _PG_fini function) is also harmless. I guess my vote is for leaving it alone, but I might not know what I'm talking about. :-) Where you check for INSERT, UPDATE, and DELETE return codes in pgss_ProcessUtility, I think this deserves a comment explaining that these could occur as a result of EXECUTE. It wasn't obvious to me, anyway. Like this? /* * Parse command tag to retrieve the number of affected rows. * COPY command returns COPY tag. EXECUTE command might return INSERT, * UPDATE, or DELETE tags, but we cannot retrieve the number of rows * for SELECT. We assume other commands always return 0 row. */ I'm confused by the we cannot retrieve the number of rows for SELECT part. Can you clarify that? It seems to me that the current hook placement does not address this complaint I don't see why the hook function should have the ability to editorialize on the behavior of everything about ProcessUtility *except* the read-only-xact check. I moved the initialization code of completionTag as the comment, but check_xact_readonly() should be called before the hook. Am I missing something? Beats me. I interpreted Tom's remark to be saying the hook call should come first, but I'm not sure which way is actually better. ...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] bug: fuzzystrmatch levenshtein is wrong
On Wed, Dec 9, 2009 at 9:00 PM, Robert Haas robertmh...@gmail.com wrote: On Tue, Dec 8, 2009 at 9:45 PM, Robert Haas robertmh...@gmail.com wrote: On Tue, Dec 8, 2009 at 9:08 PM, Bruce Momjian br...@momjian.us wrote: Robert Haas wrote: patch attached. I cannot get this patch to apply for anything. All 4 hunks fail, both on HEAD and on the the pre-8.4-pgindent version of fuzzystrmatch.c. Either I'm doing something wrong here, or there's something wrong with this patch file. The author converted tabs to spaces --- there is not a single tab in the diff file. Ah. I knew there had to be a reason. I'm attaching my version of this patch. Barring objections, I am going to apply this to HEAD and backpatch to 8.4, where this feature (and the associated bug) were introduced. Done. Yeah, my first commit! However, it appears that someone needs to tell the pgsql-committers list that rh...@postgresql.org is allowed to post there. Uh-oh. Is it bad that I did this between the time Tom updated the release notes and the time Marc stamped 8.4.2? *ducks and runs for cover* ...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] ProcessUtility_hook
Robert Haas robertmh...@gmail.com wrote: Like this? /* * Parse command tag to retrieve the number of affected rows. * COPY command returns COPY tag. EXECUTE command might return INSERT, * UPDATE, or DELETE tags, but we cannot retrieve the number of rows * for SELECT. We assume other commands always return 0 row. */ I'm confused by the we cannot retrieve the number of rows for SELECT part. Can you clarify that? Ah, I meant the SELECT was EXECUTE of SELECT. If I use internal structure names, the explanation will be: EXECUTE command returns INSERT, UPDATE, DELETE, or SELECT tags. We can retrieve the number of rows from INSERT, UPDATE, and DELETE tags, but cannot from SELECT tag because the tag doesn't contain row numbers and also EState-es_processed is unavailable for EXECUTE commands. Regards, --- Takahiro Itagaki NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ProcessUtility_hook
On Wed, Dec 9, 2009 at 10:14 PM, Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp wrote: I'm confused by the we cannot retrieve the number of rows for SELECT part. Can you clarify that? Ah, I meant the SELECT was EXECUTE of SELECT. If I use internal structure names, the explanation will be: EXECUTE command returns INSERT, UPDATE, DELETE, or SELECT tags. We can retrieve the number of rows from INSERT, UPDATE, and DELETE tags, but cannot from SELECT tag because the tag doesn't contain row numbers and also EState-es_processed is unavailable for EXECUTE commands. OK, that makes sense. It might read a little better this way: The EXECUTE command returns INSERT, UPDATE, DELETE, or SELECT tags. We can retrieve the number of rows from INSERT, UPDATE, and DELETE tags, but the SELECT doesn't contain row numbers. We also can't get it from EState-es_processed, because that is unavailable for EXECUTE commands. That seems like a rather unfortunate limitation though... ...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] Adding support for SE-Linux security
Robert Haas wrote: On Wed, Dec 9, 2009 at 5:38 PM, Bruce Momjian br...@momjian.us wrote: If you want to avoid all good reasons for this features and are looking for reasons why this patch is a bad idea, I am sure you can find them. You seem to be suggesting that our reactions are pure obstructionism, or that they have an ulterior motive. I am merely stating that this is the same as the Win32 port, and that there are many reasons to believe the SE-PostgreSQL patch will cause all sorts of problems --- this is not a surprise. I am giving a realistic analysis of the patch --- if people want to say that thinking of it as two separate patches that have to be maintained separately is a terrible idea, I have no reply except to say that realistically that is the only possible direction I see for this feature in the short term. Few Postgres people modifying the permissions system are going to understand how to modify SE-Linux support routines to match their changes. I got a similar reaction when I wanted to do the Win32 port, and the reasons not to do it were similar to the ones I am hearing now. Finally the agreement was that I could attempt the Win32 port as long as I didn't destabilize the rest of the code --- not exactly a resounding endorsement. Looking back I think everyone is glad we did the port, but at the time there wasn't much support. I got the same reaction to pg_migrator. I am having trouble figuring out when I should heed community concerns, and when the concerns are merely because the task is hard/messy/difficult. Frankly, we don't analyze hard/messy/difficult tasks very well. Now, I am not saying that the SE-PostgreSQL patch should be pursued, but I am saying that we shouldn't avoid it for these reasons, because sometimes hard/messy/difficult is necessary to accomplish dramatic software advances. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] unprivileged user
Andrew Dunstan and...@dunslane.net writes: The other day I returned idly to thinking about some work I did a few years ago on creating a totally unprivileged user, i.e. one with not even public permissions. And the point would be what exactly? 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] Largeobject Access Controls (r2460)
Hi, I'm reviewing LO-AC patch. KaiGai Kohei kai...@ak.jp.nec.com wrote: Nothing are changed in other codes, including something corresponding to in-place upgrading. I'm waiting for suggestion. I have a question about the behavior -- the patch adds ownership management of large objects. Non-privileged users cannot read, write, or drop othere's LOs. But they can read the contents of large object when they read pg_catalog.pg_largeobject directly. Even if the patch is applied, we still allow SELECT * FROM pg_largeobject ...right? This issue might be solved by the core SE-PgSQL patch, but what should we do fo now? Other changes in the patch seem to be reasonable. GRANT/REVOKE ON LARGE OBJECT number might be hard to use if used alone, but we can use the commands as dynamic SQLs in DO statements if we want to grant or revoke privileges in bulk. SELECT oid FROM pg_largeobject_metadata is used in some places instead of SELECT DISTINCT loid FROM pg_largeobject. They return the same result, but the former will be faster because we don't use DISTINCT. pg_dump will be slightly accelerated by the new query. Regards, --- Takahiro Itagaki NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] random() in multi-threaded pgbench
While testing the pgbench setshell command patch with -j option, I found all threads use the same sequence of random value. At first, I think we need to call srandom() in each thread, but the manual says we should use random_r() instead of random() on multi-threaded programs. http://www.kernel.org/doc/man-pages/online/pages/man3/random_r.3.html Should we replace random() to random_r()? Some configure test might be needed. Regards, --- Takahiro Itagaki NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [patch] pg_ctl init extension
On ons, 2009-12-09 at 15:18 +0100, Zdenek Kotala wrote: Robert Haas píše v st 09. 12. 2009 v 08:56 -0500: On Dec 9, 2009, at 8:32 AM, Zdenek Kotala zdenek.kot...@sun.com wrote: snip Ahh, It is somethink what I want to do, but it is not ready yet in this patch, but I already documented it. Idea is to install initdb and postgres into libexecdir and packager can select if libexecdir will be equal bindir or not. The paragraph should be removed at this moment. Shell I send modified patch or does committer remove it before commit? I think Peter claimed this one but as far as I am concerned, I would always rather have an updated patch. OK, here it is. Committed with some adjustments. -- 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] Streaming replication and non-blocking I/O
On Thu, Dec 10, 2009 at 12:00 AM, Tom Lane t...@sss.pgh.pa.us wrote: The OS buffer is expected to be able to store a large number of XLogRecPtr messages, because its size is small. So it's also OK to just drop it. It certainly seems to be something we could improve later, when and if evidence emerges that it's a real-world problem. For now, simple is beautiful. I just dropped the backend libpq changes related to non-blocking I/O. git://git.postgresql.org/git/users/fujii/postgres.git branch: replication Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] bug: fuzzystrmatch levenshtein is wrong
On Wed, 2009-12-09 at 21:00 -0500, Robert Haas wrote: Done. Yeah, my first commit! :) I think you forgot to update release notes for 8.4.2 :( -- Devrim GÜNDÜZ, RHCE Command Prompt - http://www.CommandPrompt.com devrim~gunduz.org, devrim~PostgreSQL.org, devrim.gunduz~linux.org.tr http://www.gunduz.org Twitter: http://twitter.com/devrimgunduz signature.asc Description: This is a digitally signed message part
Re: [HACKERS] Largeobject Access Controls (r2460)
Takahiro Itagaki wrote: Hi, I'm reviewing LO-AC patch. KaiGai Kohei kai...@ak.jp.nec.com wrote: Nothing are changed in other codes, including something corresponding to in-place upgrading. I'm waiting for suggestion. I have a question about the behavior -- the patch adds ownership management of large objects. Non-privileged users cannot read, write, or drop othere's LOs. But they can read the contents of large object when they read pg_catalog.pg_largeobject directly. Even if the patch is applied, we still allow SELECT * FROM pg_largeobject ...right? This issue might be solved by the core SE-PgSQL patch, but what should we do fo now? Oops, I forgot to fix it. It is a misconfiguration on database initialization, and not related issue with SE-PgSQL feature. It can be solved with revoking any privileges from anybody in the initdb phase. So, we should inject the following statement for setup_privileges(). REVOKE ALL ON pg_largeobject FROM PUBLIC; In the default PG model, database superuser is an exception in access controls, so he can bypass the checks eventually. Here is no difference, even if he can see pg_largeobject. For unprivileged users, this configuration restricts the way to access large object into lo_*() functions, so we can acquire all their accesses and apply permission checks comprehensively. When database superuser tries to consider something malicious, such as exposing any large object to public, we have to give up anything. Other changes in the patch seem to be reasonable. GRANT/REVOKE ON LARGE OBJECT number might be hard to use if used alone, but we can use the commands as dynamic SQLs in DO statements if we want to grant or revoke privileges in bulk. We already have COMMENT ON LARGE OBJECT number IS comment; statement :-) SELECT oid FROM pg_largeobject_metadata is used in some places instead of SELECT DISTINCT loid FROM pg_largeobject. They return the same result, but the former will be faster because we don't use DISTINCT. pg_dump will be slightly accelerated by the new query. Regards, --- Takahiro Itagaki NTT Open Source Software Center -- OSS Platform Development Division, NEC KaiGai Kohei kai...@ak.jp.nec.com *** base/src/bin/initdb/initdb.c 2009-11-21 05:52:12.0 +0900 --- blob/src/bin/initdb/initdb.c 2009-12-13 06:33:55.0 +0900 *** setup_privileges(void) *** 1783,1788 --- 1783,1789 WHERE relkind IN ('r', 'v', 'S') AND relacl IS NULL;\n, GRANT USAGE ON SCHEMA pg_catalog TO PUBLIC;\n, GRANT CREATE, USAGE ON SCHEMA public TO PUBLIC;\n, + REVOKE ALL ON pg_largeobject FROM PUBLIC;\n, NULL }; -- 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] explain output infelicity in psql
2009/12/9 Robert Haas robertmh...@gmail.com: On Wed, Dec 9, 2009 at 2:37 PM, Andrew Dunstan and...@dunslane.net wrote: I have just noticed while checking the EXPLAIN YAML patch that the non-text explain formats are output as a single line with embedded line feeds, while the text format is delivered as a set of text records, one per line. The practical effect of this is that psql decorates the non-text format output with continuation characters: andrew=# explain select count(*) from pg_class where relname ~ 'pg_'; QUERY PLAN Aggregate (cost=9.67..9.68 rows=1 width=0) - Seq Scan on pg_class (cost=0.00..9.16 rows=204 width=0) Filter: (relname ~ 'pg_'::text) (3 rows) Time: 5.813 ms andrew=# explain (format yaml) select count(*) from pg_class where relname ~ 'pg_'; QUERY PLAN - - Plan: + Node Type: Aggregate + Strategy: Plain + Startup Cost: 9.67 + Total Cost: 9.68 + Plan Rows: 1 + Plan Width: 0 + Plans: + - Node Type: Seq Scan + Parent Relationship: Outer + Relation Name: pg_class + Alias: pg_class + Startup Cost: 0.00 + Total Cost: 9.16 + Plan Rows: 204 + Plan Width: 0 + Filter: (relname ~ 'pg_'::text) (1 row) Those + chars at the end of the line are ugly, to say the least, and they make the supposedly machine-readable formats not so machine readable if anyone wanted to cp the output into a parser. (I'm mildly surprised this hasn't been noticed before). Maybe we need to teach psql not to do this formatting for EXPLAIN output? Oh, dear. I think that line continuation syntax was recently added - subsequent to the machine-readable EXPLAIN patch. The reason why it's coded to emit everything as a single row is because that will be most convenient for programs that are sucking down this data programatically. Otherwise, they'll have to concatenate all the lines that are returned. And in fact for XML format, it's even worse: the data is returned as type xml, but that obviously won't fly if we return each line as a separate tuple. On first blush, I'm inclined to suggest that the addition of + signs to mark continuation lines is a misfeature. +1 Pavel ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers