Re: [HACKERS] Make TOAST_TUPLES_PER_PAGE configurable per table.

2010-02-01 Thread Jesper Krogh
Tom Lane wrote:
> Jesper Krogh  writes:
>> This patch enables users to set TOAST_TUPLES_PER_PAGE with
>> ALTER TABLE  SET (tuples_per_page = X); .. currently with 1 <= X
>> <= 32;
> 
> It's not clear to me that fiddling with that is useful unless the toast
> tuple size also changes; and unfortunately changing that is much harder,
> because it's wired into the addressing of toast data.  See also these
> notes:
> 
>  * XXX while these can be modified without initdb, some thought needs to be
>  * given to needs_toast_table() in toasting.c before unleashing random
>  * changes.  Also see LOBLKSIZE in large_object.h, which can *not* be
>  * changed without initdb.

I can see that needs_toast_table() might need some changes since it also
uses TUPLE_TOAST_THRESHOLD, and might benefit from being aware of a
toast table is triggered.

There might be more benefits with changes the toast tuple size (I dont
have enought insight to see that), but even without it I can get a
speedup of x10 on a "simple test" and permanently get the system to used
the caching for "more commonly used data" than these attributes that are
rarely used.

Ultimately I would like an infinite amount of configurabillity since I
have tables that only consists of simple values were 50% is really
rarely used and 50% is very often used. But just changing the
TOAST_TUPLE_PER_PAGE as above can easily increase my "tuple-density"
from 6/page to 40-60/page, which translates directly into:
* Less data to read when accessing the tuples.
* Less data to cache that is rarely used.

Where as on the the table with simple values I might at best be able to
double the tuple-density.

But yes it isn't a silverbullet, it requires knowledge of the access
patterns of the data.

What kind of arguments/tests/benchmarks is required to push for the
usefulness of "fiddling" with this parameter?

Realworld database in our environment has:
12M rows sitting with an average text length of ~2KB directly
"toastable" set is: 5GB which is really rarely used, but the webapp is
doing random reads for the presense/counts of these rows.
another table has ~700M rows sitting of a size of 135GB where around
120GB is of the "really rarely used type". (but takes time to compute so
it makes sense "wasting dead disk" on them).

So based on the benchmark provided in email I think that it can
significantly change the ration of cache hit/misses for the application.
(which has 64GB of dedicated memory).

Jesper
-- 
Jesper

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] New VACUUM FULL crashes on temp relations

2010-02-01 Thread Takahiro Itagaki

Takahiro Itagaki  wrote:

> > TRAP: FailedAssertion("!(typeNamespace == typ->typnamespace)", File:
> > "pg_type.c", Line: 658)

It comes from swapping toast relations:
  DEBUG:  typeNamespace mismatch: 99 (pg_toast) vs. 16386 (pg_toast_temp_2)

AFAICS, the assertion is broken, but the code is correct. We just need to
adjust the expression in the assertion.
Patch attached, including new regression tests for clustering a temp table.

> I see the same assertion failure on 8.4.2, too.

I'll go for backpatcting if the attached fix is correct. Comments welcome.

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



cluster_assert_20100202.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Largeobject Access Controls (r2460)

2010-02-01 Thread KaiGai Kohei
>>> The --schema-only with large objects might be unnatural, but the
>>> --data-only with properties of large objects are also unnatural.
>>> Which behavior is more unnatural?
>>
>> I think large object metadata is a kind of row-based access controls.
>> How do we dump and restore ACLs per rows when we support them for
>> normal tables? We should treat LO metadata as same as row-based ACLs.
>> In my opinion, I'd like to treat them as part of data (not of schema).
> 
> OK, I'll update the patch according to the behavior you suggested.
> | I'd prefer to keep the existing behavior:
> |   * default or data-only : dump all attributes and data of blobs
> |   * schema-only  : don't dump any blobs
> | and have independent options to control blob dumps:
> |   * -b, --blobs: dump all blobs even if schema-only
> |   * -B, --no-blobs : [NEW] don't dump any blobs even if default or data-only

I found out it needs special treatments to dump comments and access
privileges of blobs. See dumpACL() and dumpComment()

| static void
| dumpACL(Archive *fout, CatalogId objCatId, DumpId objDumpId,
| const char *type, const char *name, const char *subname,
| const char *tag, const char *nspname, const char *owner,
| const char *acls)
| {
| PQExpBuffer sql;
|
| /* Do nothing if ACL dump is not enabled */
| if (dataOnly || aclsSkip)
| return;
| :

| static void
| dumpComment(Archive *fout, const char *target,
| const char *namespace, const char *owner,
| CatalogId catalogId, int subid, DumpId dumpId)
| {
| CommentItem *comments;
| int ncomments;
|
| /* Comments are SCHEMA not data */
| if (dataOnly)
| return;
| :

In addition, _printTocEntry() is not called with acl_pass = true,
when --data-only is given.

I again wonder whether we are on the right direction.

Originally, the reason why we decide to use per blob toc entry was
that "BLOB ACLS" entry needs a few exceptional treatments in the code.
But, if we deal with "BLOB ITEM" entry as data contents, it will also
need additional exceptional treatments.

Indeed, even if we have row-level ACLs, it will be dumped in data section
without separating them into properties and data contents because of the
restriction on implementation, not a data modeling reason.

Many of empty large objects may not be sane situation, but it is suitable
for the existing manner in pg_dump/pg_restore, at least.

Thanks,
-- 
OSS Platform Development Division, NEC
KaiGai Kohei 

-- 
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: [COMMITTERS] pgsql: Write a WAL record whenever we perform an operation without

2010-02-01 Thread Fujii Masao
On Mon, Feb 1, 2010 at 7:40 PM, Heikki Linnakangas
 wrote:
> So you get those messages when the table is *not* a temporary table. I
> can see now what Fujii was trying to say. His patch seems Ok, though
> perhaps it would be better to move the responsibility of calling
> XLogReportUnloggedStatement() to the callers of heap_sync(). When I put
> it in heap_sync(), I didn't take into account that it's sometimes called
> just to flush buffers from buffer cache, not to fsync() non-WAL-logged
> operations.

As you said, I moved the responsibility of calling XLogReportUnloggedStatement()
to the callers of heap_sync(). Here is the patch.

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
***
*** 5074,5089  heap2_desc(StringInfo buf, uint8 xl_info, char *rec)
  void
  heap_sync(Relation rel)
  {
- 	char reason[NAMEDATALEN + 30];
- 
  	/* temp tables never need fsync */
  	if (rel->rd_istemp)
  		return;
  
- 	snprintf(reason, sizeof(reason), "heap inserts on \"%s\"",
- 			 RelationGetRelationName(rel));
- 	XLogReportUnloggedStatement(reason);
- 
  	/* main heap */
  	FlushRelationBuffers(rel);
  	/* FlushRelationBuffers will have opened rd_smgr */
--- 5074,5083 
*** a/src/backend/access/heap/rewriteheap.c
--- b/src/backend/access/heap/rewriteheap.c
***
*** 246,252  begin_heap_rewrite(Relation new_heap, TransactionId oldest_xmin,
   * state and any other resources are freed.
   */
  void
! end_heap_rewrite(RewriteState state)
  {
  	HASH_SEQ_STATUS seq_status;
  	UnresolvedTup unresolved;
--- 246,252 
   * state and any other resources are freed.
   */
  void
! end_heap_rewrite(RewriteState state, bool use_wal)
  {
  	HASH_SEQ_STATUS seq_status;
  	UnresolvedTup unresolved;
***
*** 278,283  end_heap_rewrite(RewriteState state)
--- 278,292 
     (char *) state->rs_buffer, true);
  	}
  
+ 	/* Write an XLOG UNLOGGED record if WAL-logging was skipped */
+ 	if (!use_wal && !state->rs_new_rel->rd_istemp)
+ 	{
+ 		char reason[NAMEDATALEN + 30];
+ 		snprintf(reason, sizeof(reason), "heap rewrite on \"%s\"",
+  RelationGetRelationName(state->rs_new_rel));
+ 		XLogReportUnloggedStatement(reason);
+ 	}
+ 
  	/*
  	 * If the rel isn't temp, must fsync before commit.  We use heap_sync to
  	 * ensure that the toast table gets fsync'd too.
*** a/src/backend/commands/cluster.c
--- b/src/backend/commands/cluster.c
***
*** 998,1004  copy_heap_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex,
  		heap_endscan(heapScan);
  
  	/* Write out any remaining tuples, and fsync if needed */
! 	end_heap_rewrite(rwstate);
  
  	pfree(values);
  	pfree(isnull);
--- 998,1004 
  		heap_endscan(heapScan);
  
  	/* Write out any remaining tuples, and fsync if needed */
! 	end_heap_rewrite(rwstate, use_wal);
  
  	pfree(values);
  	pfree(isnull);
*** a/src/backend/commands/copy.c
--- b/src/backend/commands/copy.c
***
*** 2225,2231  CopyFrom(CopyState cstate)
--- 2225,2237 
  	 * indexes since those use WAL anyway)
  	 */
  	if (hi_options & HEAP_INSERT_SKIP_WAL)
+ 	{
+ 		char reason[NAMEDATALEN + 30];
+ 		snprintf(reason, sizeof(reason), "COPY FROM on \"%s\"",
+  RelationGetRelationName(cstate->rel));
+ 		XLogReportUnloggedStatement(reason);
  		heap_sync(cstate->rel);
+ 	}
  }
  
  
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
***
*** 3297,3303  ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
--- 3297,3309 
  
  		/* If we skipped writing WAL, then we need to sync the heap. */
  		if (hi_options & HEAP_INSERT_SKIP_WAL)
+ 		{
+ 			char reason[NAMEDATALEN + 30];
+ 			snprintf(reason, sizeof(reason), "table rewrite on \"%s\"",
+ 	 RelationGetRelationName(newrel));
+ 			XLogReportUnloggedStatement(reason);
  			heap_sync(newrel);
+ 		}
  
  		heap_close(newrel, NoLock);
  	}
*** a/src/backend/executor/execMain.c
--- b/src/backend/executor/execMain.c
***
*** 2240,2246  CloseIntoRel(QueryDesc *queryDesc)
--- 2240,2252 
  
  		/* If we skipped using WAL, must heap_sync before commit */
  		if (myState->hi_options & HEAP_INSERT_SKIP_WAL)
+ 		{
+ 			char reason[NAMEDATALEN + 30];
+ 			snprintf(reason, sizeof(reason), "SELECT INTO on \"%s\"",
+ 	 RelationGetRelationName(myState->rel));
+ 			XLogReportUnloggedStatement(reason);
  			heap_sync(myState->rel);
+ 		}
  
  		/* close rel, but keep lock until commit */
  		heap_close(myState->rel, NoLock);
*** a/src/include/access/rewriteheap.h
--- b/src/include/access/rewriteheap.h
***
*** 22,28  typedef struct RewriteStateData *RewriteState;
  extern RewriteState begin_heap_rewrite(Relation NewHeap,
     TransactionId OldestXmin, TransactionId FreezeXid,
     bool use_wal);
! extern void end_heap_rewrite(RewriteState state);
  extern void rewrite_heap_tuple(Rewri

Re: [HACKERS] PITR - Bug or feature?

2010-02-01 Thread Fujii Masao
Hi,

On Mon, Feb 1, 2010 at 7:33 PM, Rafael Martinez
 wrote:
> The PITR backup history file named
> pg_xlog/00010038.0020.backup included this information:
> 
> START WAL LOCATION: 38/20 (file 00010038)
> STOP WAL LOCATION: 38/88 (file 00010038)
> CHECKPOINT LOCATION: 38/20
> START TIME: 2010-02-01 07:20:05 CET
> LABEL:
> /usit/dbpg-research/pg_bck/PITR_data/PITRBASE-dbpg-research_v8.3.9_j10_2010-02-01_072001
> STOP TIME: 2010-02-01 07:22:42 CET
> 
>
> As you can see, the second number in START/STOP and CHECKPOINT LOCATION
> has a length of 2. This second number usually has a length of 8.
>
> I have checked the logs from our last 5000 PITR jobs and the format of
> these values has always been "/<8 digits number>"
>
> We have been using the "/<8 digits>" ID returned by the
> pg_start_backup() function to find out the PITR backup history file we
> have to wait for in the archive directory, before we can delete old WAL
> files that are not needed anymore. This task gets more complicated if we
> cannot trust to get a consistent format from pg_start_backup().
>
> These are some thoughts that may help to debug this issue:
>
> * The postgresql version with this 'problem' is 8.3.9
> * The active WAL ID when we started and stopped PITR ends with '00'
> * We have not seen this change of format in 8.3.9 when the WAL ID does
>  not ends with '00'
> * We have had WAL files ending with '00' with versions < 8.3.9 and the
>  format used have been the expected ("/<8 digits>").
>
> Any thoughts about this? Is this a bug or a 'feature'?

This is not a bug. Since pg_start_backup() uses "%X/%X" (not "%08X/%08X")
as the format of WAL location, the length of the second number of the WAL
location could be less than 8.

Instead of calculating the name of the backup history file for yourself,
how about using pg_xlogfile_name() or pg_xlogfile_name_offset()? Those
functions convert the WAL location regardless of a format into the file name.
http://www.postgresql.org/docs/8.3/static/functions-admin.html#FUNCTIONS-ADMIN-BACKUP-TABLE

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?] strange behavior in ALTER TABLE ... RENAME TO on inherited columns

2010-02-01 Thread KaiGai Kohei
(2010/02/02 11:44), Robert Haas wrote:
> 2010/2/1 KaiGai Kohei:
>> (2010/02/02 11:31), Robert Haas wrote:
>>> 2010/2/1 KaiGai Kohei:
 (2010/02/02 11:09), Tom Lane wrote:
> KaiGai Kohei  writes:
>> The attached one also clean up ATPrepAddColumn() and ATExecAddColumn() 
>> code,
>> not only ATPrepAlterColumnType(), according to what I mentioned above.
>
> What exactly do you claim is wrong with the ADD COLUMN case?

 ADD COLUMN case works correctly, but it takes unnecessary loops,
 because the find_all_inheritors() didn't provide the value to be
 set on the new pg_attribute.attinhcount.

 I'm saying it can be rewritten in more graceful manner using the
 new expected_parents argument.
>>>
>>> The subject line of this thread is getting less and less appropriate
>>> to the content thereof.
>>>
>>> I am not in favor of doing anything for 9.0 that is not a bug fix.
>>
>> Are you talking about ATPrepAddColumn() only? Or both of ATPrepAddColumn()
>> and ATPrepAlterColumnType()?
>>
>> My motivation to clean up ATPrepAddColumn() is less than the bugfix.
> 
> I'm making a general statement - if something is BROKEN (like the
> rename case we just dealt with), we should look at fixing it.  If it's
> just something that could be cleaned up or done more nicely, we should
> leave it alone for now.

OK, Please forget the second patch.

The former patch (pgsql-fix-inherit-attype.1.patch) just fixes the matter
in ALTER COLUMN TYPE case. Do you think it is a reasonable change for
the 9.0 release?

Thanks,
-- 
OSS Platform Development Division, NEC
KaiGai Kohei 

-- 
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] Package namespace and Safe init cleanup for plperl UPDATE 3 [PATCH]

2010-02-01 Thread Alex Hunsaker
On Mon, Feb 1, 2010 at 03:58, Tim Bunce  wrote:
> On Sat, Jan 30, 2010 at 06:38:59PM -0700, Alex Hunsaker wrote:
>
>> plc_safe_ok.pl seems to loose its CVS $PostgreSQL$ keyword.
>Probably a slip-up when I merged the changes from HEAD up through my
>chain of branches.

Can you send an updated patch?  I think Andrew will probably fix it up
anyway but better safe than sorry.

>> That being said *im* ok with it.  Its useful from a debug standpoint.
>
> Yes. And, as I mentioned previously, I expect people like myself, David
> Wheeler, and others to experiment with the undocumented functionality
> and define and document a good API to it for the 9.1 release.

Huh, I missed that.

> I'd much rather get this change in than shoot for a larger change that
> doesn't get committed due to long-running discussions.  (Which seems
> more likely as Andrew's going to be less available for the rest of the
> commitfest.)

Plus its hard to get people to agree on anything GUCy (my new favorite
pun) thats not well thought out and tested.

Anyway yes I agree, but I thought I should at least raise it for
discussion. You'll notice the patch has been marked "Ready for
Commiter" this whole time. =)

-- 
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?] strange behavior in ALTER TABLE ... RENAME TO on inherited columns

2010-02-01 Thread Robert Haas
2010/2/1 KaiGai Kohei :
> (2010/02/02 11:31), Robert Haas wrote:
>> 2010/2/1 KaiGai Kohei:
>>> (2010/02/02 11:09), Tom Lane wrote:
 KaiGai Kohei    writes:
> The attached one also clean up ATPrepAddColumn() and ATExecAddColumn() 
> code,
> not only ATPrepAlterColumnType(), according to what I mentioned above.

 What exactly do you claim is wrong with the ADD COLUMN case?
>>>
>>> ADD COLUMN case works correctly, but it takes unnecessary loops,
>>> because the find_all_inheritors() didn't provide the value to be
>>> set on the new pg_attribute.attinhcount.
>>>
>>> I'm saying it can be rewritten in more graceful manner using the
>>> new expected_parents argument.
>>
>> The subject line of this thread is getting less and less appropriate
>> to the content thereof.
>>
>> I am not in favor of doing anything for 9.0 that is not a bug fix.
>
> Are you talking about ATPrepAddColumn() only? Or both of ATPrepAddColumn()
> and ATPrepAlterColumnType()?
>
> My motivation to clean up ATPrepAddColumn() is less than the bugfix.

I'm making a general statement - if something is BROKEN (like the
rename case we just dealt with), we should look at fixing it.  If it's
just something that could be cleaned up or done more nicely, we should
leave it alone for now.

...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?] strange behavior in ALTER TABLE ... RENAME TO on inherited columns

2010-02-01 Thread KaiGai Kohei
(2010/02/02 11:31), Robert Haas wrote:
> 2010/2/1 KaiGai Kohei:
>> (2010/02/02 11:09), Tom Lane wrote:
>>> KaiGai Koheiwrites:
 The attached one also clean up ATPrepAddColumn() and ATExecAddColumn() 
 code,
 not only ATPrepAlterColumnType(), according to what I mentioned above.
>>>
>>> What exactly do you claim is wrong with the ADD COLUMN case?
>>
>> ADD COLUMN case works correctly, but it takes unnecessary loops,
>> because the find_all_inheritors() didn't provide the value to be
>> set on the new pg_attribute.attinhcount.
>>
>> I'm saying it can be rewritten in more graceful manner using the
>> new expected_parents argument.
> 
> The subject line of this thread is getting less and less appropriate
> to the content thereof.
> 
> I am not in favor of doing anything for 9.0 that is not a bug fix.

Are you talking about ATPrepAddColumn() only? Or both of ATPrepAddColumn()
and ATPrepAlterColumnType()?

My motivation to clean up ATPrepAddColumn() is less than the bugfix.

Thanks,
-- 
OSS Platform Development Division, NEC
KaiGai Kohei 

-- 
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?] strange behavior in ALTER TABLE ... RENAME TO on inherited columns

2010-02-01 Thread Tom Lane
KaiGai Kohei  writes:
> (2010/02/02 11:09), Tom Lane wrote:
>> KaiGai Kohei  writes:
>>> The attached one also clean up ATPrepAddColumn() and ATExecAddColumn() code,
>>> not only ATPrepAlterColumnType(), according to what I mentioned above.
>> 
>> What exactly do you claim is wrong with the ADD COLUMN case?

> ADD COLUMN case works correctly, but it takes unnecessary loops,
> because the find_all_inheritors() didn't provide the value to be
> set on the new pg_attribute.attinhcount.

> I'm saying it can be rewritten in more graceful manner using the
> new expected_parents argument.

I tend to think that if it ain't broke don't fix it; the odds of
actually breaking it are too high.  I don't really find the new coding
more graceful, anyway ...

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] [BUG?] strange behavior in ALTER TABLE ... RENAME TO on inherited columns

2010-02-01 Thread Robert Haas
2010/2/1 KaiGai Kohei :
> (2010/02/02 11:09), Tom Lane wrote:
>> KaiGai Kohei  writes:
>>> The attached one also clean up ATPrepAddColumn() and ATExecAddColumn() code,
>>> not only ATPrepAlterColumnType(), according to what I mentioned above.
>>
>> What exactly do you claim is wrong with the ADD COLUMN case?
>
> ADD COLUMN case works correctly, but it takes unnecessary loops,
> because the find_all_inheritors() didn't provide the value to be
> set on the new pg_attribute.attinhcount.
>
> I'm saying it can be rewritten in more graceful manner using the
> new expected_parents argument.

The subject line of this thread is getting less and less appropriate
to the content thereof.

I am not in favor of doing anything for 9.0 that is not a bug fix.

...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?] strange behavior in ALTER TABLE ... RENAME TO on inherited columns

2010-02-01 Thread KaiGai Kohei
(2010/02/02 11:09), Tom Lane wrote:
> KaiGai Kohei  writes:
>> The attached one also clean up ATPrepAddColumn() and ATExecAddColumn() code,
>> not only ATPrepAlterColumnType(), according to what I mentioned above.
> 
> What exactly do you claim is wrong with the ADD COLUMN case?

ADD COLUMN case works correctly, but it takes unnecessary loops,
because the find_all_inheritors() didn't provide the value to be
set on the new pg_attribute.attinhcount.

I'm saying it can be rewritten in more graceful manner using the
new expected_parents argument.

Thanks,
-- 
OSS Platform Development Division, NEC
KaiGai Kohei 

-- 
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?] strange behavior in ALTER TABLE ... RENAME TO on inherited columns

2010-02-01 Thread Tom Lane
KaiGai Kohei  writes:
> The attached one also clean up ATPrepAddColumn() and ATExecAddColumn() code,
> not only ATPrepAlterColumnType(), according to what I mentioned above.

What exactly do you claim is wrong with the ADD COLUMN case?

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] contrib\xml2 package's xpath_table function in PostgreSQL

2010-02-01 Thread M Z
I am very interested in doing it. However I am new to postgresql. Could you
and anyone here please give me some hint, which way I should, which part of
code I should focus to fix it?

Thanks,
M Z

On Mon, Feb 1, 2010 at 1:23 PM, Robert Haas  wrote:

> On Mon, Feb 1, 2010 at 1:20 PM, M Z  wrote:
> > Is there a way to fix it so that those functions are usable in 8.4
> without
> > crashing the server?
>
> Nobody seems to be interested enough to figure that out and submit a
> patch to fix it.  If someone does, I think it would have a good chance
> of being accepted.
>
> ...Robert
>


Re: [HACKERS] Largeobject Access Controls (r2460)

2010-02-01 Thread KaiGai Kohei
(2010/02/02 9:33), Takahiro Itagaki wrote:
> 
> KaiGai Kohei  wrote:
> 
>>> Can we remove such path and raise an error instead?
>>> Also, even if we support the older servers in the routine,
>>> the new bytea format will be another problem anyway.
>>
>> OK, I'll fix it.
> 
> I think we might need to discuss about explicit version checks in pg_restore.
> It is not related with large objects, but with pg_restore's capability.
> We've not supported to restore a dump to older servers, but we don't have any
> version checks, right? Should we do the checks at the beginning of restoring?
> If we do so, LO patch could be more simplified.

I agree it is a good idea.

>> The --schema-only with large objects might be unnatural, but the
>> --data-only with properties of large objects are also unnatural.
>> Which behavior is more unnatural?
> 
> I think large object metadata is a kind of row-based access controls.
> How do we dump and restore ACLs per rows when we support them for
> normal tables? We should treat LO metadata as same as row-based ACLs.
> In my opinion, I'd like to treat them as part of data (not of schema).

OK, I'll update the patch according to the behavior you suggested.
| I'd prefer to keep the existing behavior:
|   * default or data-only : dump all attributes and data of blobs
|   * schema-only  : don't dump any blobs
| and have independent options to control blob dumps:
|   * -b, --blobs: dump all blobs even if schema-only
|   * -B, --no-blobs : [NEW] don't dump any blobs even if default or data-only

Please wait for a while. Thanks,
-- 
OSS Platform Development Division, NEC
KaiGai Kohei 

-- 
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?] strange behavior in ALTER TABLE ... RENAME TO on inherited columns

2010-02-01 Thread KaiGai Kohei
(2010/02/02 9:48), KaiGai Kohei wrote:
>>> Also, the ALTER COLUMN TYPE case should be also fixed in the 9.1 release
>>> (or 9.0.1?).
>>
>> If the fix is something we could commit for 9.0.1, then we ought to do
>> it now before 9.0 is released.  If you want to submit a follow-on
>> patch to address ALTER COLUMN TYPE once this is committed, then please
>> do so.
> 
> The attached patch also fixes ALTER COLUMN TYPE case.
> 
> It replaced the 'recursing' argument in ATPrepCmd() by 'expected_parents',
> and it is delivered to ATPrepAlterColumnType(). The logic to forbid altering
> the column type is same as renameatt().
> ATSimpleRecursion() is also modified to use forboth() to call ATPrepCmd()
> recursively.
> 
> One concern is at ATOneLevelRecursion() which is called by ATPrepAddColumn()
> only, and it also calls ATPrepCmd() for the direct children.
> Right now, I set 1 on the 'expected_parents'. However, IMO, here is no reason
> why we cannot rewrite the ATPrepAddColumn() using ATSimpleRecursion().
> 
> Eventually, ATExecAddColumn() shall be invoked several times for same column,
> if the inheritance tree has diamond-inheritance structure. And, it increments
> pg_attribute.attinhcount except for the first invocation.
> If we store the 'expected_parents' on the ColumnDef->inhcount, we don't need
> to call the ATExecAddColumn() more than once in a single ALTER TABLE command.
> 
> Any comments? And, when should we do it? 9.0? 9.1?

The attached one also clean up ATPrepAddColumn() and ATExecAddColumn() code,
not only ATPrepAlterColumnType(), according to what I mentioned above.

There are two regression test fails, because it does not call ATExecAddColumn()
twice or more in diamond-inheritance cases, so it does not notice merging
definitions of columns.

If we should go on right now, I'll add and fix regression tests, and submit
a formal patch again. If not, I'll work it later.

Thanks,
-- 
OSS Platform Development Division, NEC
KaiGai Kohei 


pgsql-fix-inherit-attype.2.patch
Description: application/octect-stream

-- 
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] remove contrib/xml2

2010-02-01 Thread Robert Haas
On Mon, Feb 1, 2010 at 5:23 PM, Andrew Dunstan  wrote:
> Robert Haas wrote:
>> (2) add a very, very large warning that this will crash if you do
>> almost anything with it.
>
> I think that's an exaggeration. Certain people are known to be using it
> quite successfully.

Hmm.  Well, all I know is that the first thing I tried crashed the server.

CREATE TABLE xpath_test (id integer NOT NULL, t xml);
INSERT INTO xpath_test VALUES (1, '1');
SELECT * FROM xpath_table('id', 't', 'xpath_test', '/doc/int', 'true')
as t(id int4);

It doesn't crash if you change the type of t from xml to text; instead
you get a warning about some sort of memory allocation problem.

DROP TABLE xpath_test;
CREATE TABLE xpath_test (id integer NOT NULL, t text);
INSERT INTO xpath_test VALUES (1, '1');
SELECT * FROM xpath_table('id', 't', 'xpath_test', '/doc/int', 'true')
as t(id int4);

yields:

WARNING:  problem in alloc set ExprContext: bogus aset link in block
0x14645e0, chunk 0x14648b8

And then there's this (see also bug #5285):

DELETE FROM xpath_test;
INSERT INTO xpath_test VALUES (1, '');
SELECT * FROM xpath_table('id', 't', 'xpath_test',
'/rowlist/row/@a|/rowlist/row/@b', 'true') as t(id int4, a text, b
text);

which yields an answer that is, at least, extremely surprising, if not
flat-out wrong:

 id | a |  b
+---+--
  1 | 1 | oops
  1 | 2 |
(2 rows)

Bugs #4953 and #5079 can also be reproduced in CVS HEAD.  Both crash the server.

...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?] strange behavior in ALTER TABLE ... RENAME TO on inherited columns

2010-02-01 Thread KaiGai Kohei
(2010/02/02 3:01), Robert Haas wrote:
> 2010/1/31 KaiGai Kohei:
>> The attached patch modified find_all_inheritors() to return the list of
>> expected inhcount, if List * pointer is given. And, it focuses on only
>> the bugs in renameatt() case.
> 
> I have cleaned up and simplified this patch.  Attached is the version
> I intend to commit.  Changes:
> 
> - make find_all_inheritors return the list of OIDs, as previously,
> instead of using an out parameter
> - undo some useless variable renamings
> - undo some useless whitespace changes
> - rework comments
> - fix regression tests to avoid using the same alias twice in the same query
> - add an ORDER BY clause to the regression tests so that they pass on my 
> machine
> - improve the names of some of the new variables
> - instead of adding an additional argument to renameatt(), just
> replace the existing 'bool recursing' with 'int expected_parents'.
> This allows merging the two versions of the "cannot rename inherited
> column" message together, which seems like a reasonably good idea to
> me, though if someone has a better idea that's fine.  I didn't think
> the one additional word added to the message provided enough clarity
> to make it worth creating another translatable string.

Thanks for your checks.

>> Also, the ALTER COLUMN TYPE case should be also fixed in the 9.1 release
>> (or 9.0.1?).
> 
> If the fix is something we could commit for 9.0.1, then we ought to do
> it now before 9.0 is released.  If you want to submit a follow-on
> patch to address ALTER COLUMN TYPE once this is committed, then please
> do so.

The attached patch also fixes ALTER COLUMN TYPE case.

It replaced the 'recursing' argument in ATPrepCmd() by 'expected_parents',
and it is delivered to ATPrepAlterColumnType(). The logic to forbid altering
the column type is same as renameatt().
ATSimpleRecursion() is also modified to use forboth() to call ATPrepCmd()
recursively.

One concern is at ATOneLevelRecursion() which is called by ATPrepAddColumn()
only, and it also calls ATPrepCmd() for the direct children.
Right now, I set 1 on the 'expected_parents'. However, IMO, here is no reason
why we cannot rewrite the ATPrepAddColumn() using ATSimpleRecursion().

Eventually, ATExecAddColumn() shall be invoked several times for same column,
if the inheritance tree has diamond-inheritance structure. And, it increments
pg_attribute.attinhcount except for the first invocation.
If we store the 'expected_parents' on the ColumnDef->inhcount, we don't need
to call the ATExecAddColumn() more than once in a single ALTER TABLE command.

Any comments? And, when should we do it? 9.0? 9.1?

Thanks,
-- 
OSS Platform Development Division, NEC
KaiGai Kohei 


pgsql-fix-inherit-attype.1.patch
Description: application/octect-stream

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] New VACUUM FULL crashes on temp relations

2010-02-01 Thread Takahiro Itagaki

Simon Riggs  wrote:

> TRAP: FailedAssertion("!(typeNamespace == typ->typnamespace)", File:
> "pg_type.c", Line: 658)
> 
> Test case attached, repeated, consistent failure on CVS HEAD.

I see the same assertion failure on 8.4.2, too.
I'll investigating it...

-- minimum reproducible pattern
drop table if exists footemp;
create temp table footemp (col1 serial, col2 text);
create index footemp_col1_idx on footemp (col1);
cluster footemp using footemp_col1_idx;

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] Largeobject Access Controls (r2460)

2010-02-01 Thread Takahiro Itagaki

KaiGai Kohei  wrote:

> > Can we remove such path and raise an error instead?
> > Also, even if we support the older servers in the routine,
> > the new bytea format will be another problem anyway.
> 
> OK, I'll fix it.

I think we might need to discuss about explicit version checks in pg_restore.
It is not related with large objects, but with pg_restore's capability.
We've not supported to restore a dump to older servers, but we don't have any
version checks, right? Should we do the checks at the beginning of restoring?
If we do so, LO patch could be more simplified.


> The --schema-only with large objects might be unnatural, but the
> --data-only with properties of large objects are also unnatural.
> Which behavior is more unnatural?

I think large object metadata is a kind of row-based access controls.
How do we dump and restore ACLs per rows when we support them for
normal tables? We should treat LO metadata as same as row-based ACLs.
In my opinion, I'd like to treat them as part of data (not of schema).

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] Make TOAST_TUPLES_PER_PAGE configurable per table.

2010-02-01 Thread Tom Lane
Jesper Krogh  writes:
> This patch enables users to set TOAST_TUPLES_PER_PAGE with
> ALTER TABLE  SET (tuples_per_page = X); .. currently with 1 <= X
> <= 32;

It's not clear to me that fiddling with that is useful unless the toast
tuple size also changes; and unfortunately changing that is much harder,
because it's wired into the addressing of toast data.  See also these
notes:

 * XXX while these can be modified without initdb, some thought needs to be
 * given to needs_toast_table() in toasting.c before unleashing random
 * changes.  Also see LOBLKSIZE in large_object.h, which can *not* be
 * changed without initdb.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Database-Role settings behaviour and docs mismatch

2010-02-01 Thread Tom Lane
Simon Riggs  writes:
> On Mon, 2010-02-01 at 20:11 -0300, Alvaro Herrera wrote:
>> It'd probably be worth changing the order of the ApplySetting calls so
>> that it doesn't look suspicious.

> Just a comment would be enough I think

Yeah.  Changing the order would mean that we'd do extra work applying
and then removing conflicting settings.  But the general principle here
is that GUC settings coming from different places are resolved by
source priority, not order of execution.

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] development setup and libdir

2010-02-01 Thread Ivan Sergio Borgonovo
On Sun, 31 Jan 2010 22:24:40 +
Mark Cave-Ayland  wrote:

> Ivan Sergio Borgonovo wrote:
> 
> > Of course I can write a script that can workaround this.
> > It seems that the only thing missing is that pgxs 8.3 used to
> > prefix .so with lib and then rename them at install time, but
> > pgxs 8.4 build them directly without prefix.
> > I'm just speculating this is the issue and it is the only one I
> > still have to solve... but... what's going to happen next
> > release? Wouldn't it be better if make install could install
> > stuff where I ask so I could put modules in different places
> > *even* for the same installation of postgres?

> FWIW the soon-to-be-released PostGIS 1.5 has an "out of place" 
> regression testing feature that allows PostGIS to be built using
> PGXS and regression tested without putting anything in the
> PostgreSQL installation directory itself.

Thanks I'll give a look as soon as possible, now I'll try to
concentrate on C development.

I used something surely simpler.
I wrote this small script and it actually does what I need.

#!/bin/bash
export USE_PGXS=1; make

MODULE_big=$(sed -ne '/MODULE_big/s/^MODULE_big[ \t]*=[ \t]*\([^
\t]*\)/\1/gp' Makefile)

so=$(ls -1 *"$MODULE_big"*.so)

sed -e 's#\$libdir[^'"'"']*#'`pwd -P`'/'$so'#g' $MODULE_big.sql >
$MODULE_big.l.sql

sed -e 's#\$libdir[^'"'"']*#'`pwd -P`'/'$so'#g'
uninstall_$MODULE_big.sql > uninstall_$MODULE_big.l.sql psql

psql test < $MODULE_big.l.sql

/* some more stuff to test functions */

And finally I have my first function working. Without other
functions the extension isn't that useful yet but I think I'll be
able to build something useful.

Thanks for the help. Especially to RhodiumToad and klando on
#postgresql

I'll try to write some documentation shortly.

-- 
Ivan Sergio Borgonovo
http://www.webthatworks.it


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Database-Role settings behaviour and docs mismatch

2010-02-01 Thread Simon Riggs
On Mon, 2010-02-01 at 20:11 -0300, Alvaro Herrera wrote:

> It'd probably be worth changing the order of the ApplySetting calls so
> that it doesn't look suspicious.

Just a comment would be enough I think on ApplySetting to make it clear
that it really means ApplySettingIfNotAlreadySet()

-- 
 Simon Riggs   www.2ndQuadrant.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Database-Role settings behaviour and docs mismatch

2010-02-01 Thread Alvaro Herrera
Simon Riggs wrote:

> Whereas in process_settings() the sequence is this
> 
> ApplySetting(databaseid, roleid, relsetting, PGC_S_DATABASE_USER);
> ApplySetting(InvalidOid, roleid, relsetting, PGC_S_USER);
> ApplySetting(databaseid, InvalidOid, relsetting, PGC_S_DATABASE);
> 
> which looks to me like database-role specific settings are overridden by
> both user and database specific ones, in contrast to how the docs
> describe this.

Yeah, except that set_config_option contains this bit:

/*
 * Ignore attempted set if overridden by previously processed setting.
 * However, if changeVal is false then plow ahead anyway since we are
 * trying to find out if the value is potentially good, not actually use
 * it. Also keep going if makeDefault is true, since we may want to set
 * the reset/stacked values even if we can't set the variable itself.
 */
if (record->source > source)
{
if (changeVal && !makeDefault)
{
elog(DEBUG3, "\"%s\": setting ignored because previous source is 
higher priority",
 name);
return true;
}
changeVal = false;
}


> Not that bothered, but seems like the docs provide more useful behaviour
> and the code less useful.

It'd probably be worth changing the order of the ApplySetting calls so
that it doesn't look suspicious.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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] plpython3

2010-02-01 Thread James William Pye
On Feb 1, 2010, at 1:23 PM, Nathan Boley wrote:

>> I think it would be great for you to review it... I doubt that will
>> cause it to get committed for 9.0, but my doubt is no reason for you
>> to hold off reviewing it.
> 
> I assumed so, but the pretense of a chance will probably help to motivate me 
> :-)
> 
> I'll have something by Thursday, and then 'Returned with Feedback'
> will at least be factual.

I haven't updated the plpython3 branch in a while, so you may want to hit the 
github repo with the PGXS build: http://github.com/jwp/pg-python

...Should probably get the updated docs published too, but they are available 
in src/documentation as ReST files. If you have sphinx installed, running `make 
html` in the root project directory should build them into src/sphinx/html.
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] Database-Role settings behaviour and docs mismatch

2010-02-01 Thread Simon Riggs

In the docs it says
"It is also possible to tie a session default to a specific database
rather than to a role; see ALTER DATABASE. If there is a conflict,
database-role-specific settings override role-specific ones, which in
turn override database-specific ones."

Whereas in process_settings() the sequence is this

ApplySetting(databaseid, roleid, relsetting, PGC_S_DATABASE_USER);
ApplySetting(InvalidOid, roleid, relsetting, PGC_S_USER);
ApplySetting(databaseid, InvalidOid, relsetting, PGC_S_DATABASE);

which looks to me like database-role specific settings are overridden by
both user and database specific ones, in contrast to how the docs
describe this.

Am I confused, or is this a problem?

Not that bothered, but seems like the docs provide more useful behaviour
and the code less useful.

-- 
 Simon Riggs   www.2ndQuadrant.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] remove contrib/xml2

2010-02-01 Thread Andrew Dunstan



Robert Haas wrote:

(2) add a very, very large warning that this will crash if you do
almost anything with it.


  


I think that's an exaggeration. Certain people are known to be using it 
quite successfully.


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] plpython3

2010-02-01 Thread Robert Haas
On Mon, Feb 1, 2010 at 4:30 PM, Joshua D. Drake  wrote:
> On Mon, 2010-02-01 at 16:13 -0500, Bruce Momjian wrote:
>> Alvaro Herrera wrote:
>> > Peter Eisentraut escribi?:
>> > > On m?n, 2010-02-01 at 12:01 -0800, Nathan Boley wrote:
>> > > > I code nearly exclusively in python and C, but I have
>> > > > often found pl/python to be very unwieldy.  For this reason I often
>> > > > use pl/perl or pl/pgsql for problems that, outside of postgres, I
>> > > > would always use python.
>> > >
>> > > I find that curious, because much of the criticism about the current
>> > > PL/Python can be traced back to the fact that the implementation used to
>> > > be an exact copy of PL/Perl.
>> >
>> > Perhaps the problem is that PL/Perl used to be unwieldy back when
>> > PL/Python was created.  PL/Perl has definitely seen a lot more activity.
>>
>> I would love to know why PL/Python can't be incrementally improved like
>> the rest of our code.
>
> It has been. That is exactly what PeterE has been doing.
>
> However, if you look at this whole thread, you will see the James has a
> very different view of the implementation. One that at least appears to
> be more advanced and "pythonic" than our version.

I don't know if the native typing stuff is "more advanced" than our
current code or not; that's kind of fuzzy terminology if you think
about it.  It is, however, a lot different than what we do in the
existing PL/python, or, to the best of my knowledge, any of the other
PLs with, perhaps, the exception of PL/pgsql.  So conceivably someone
could submit a PL/perlNG, a PL/lolcodeNG, etc. taking a similar
approach.  It's worth thinking about how we feel about that.

...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] plpython3

2010-02-01 Thread Josh Berkus
On 2/1/10 1:39 PM, Joshua D. Drake wrote:
> On Mon, 2010-02-01 at 16:31 -0500, Bruce Momjian wrote:
>>  
 I would love to know why PL/Python can't be incrementally improved like
 the rest of our code.
>>> It has been. That is exactly what PeterE has been doing.
>>>
>>> However, if you look at this whole thread, you will see the James has a
>>> very different view of the implementation. One that at least appears to
>>> be more advanced and "pythonic" than our version.
>> More "pythonic" in its internal processing or in its user interface?
> 
> User interface and also internal processing (see the types discussion).

Yeah, from the sound of it, we should put this in pgfoundry (or
elsewhere) and have people try it out for 9.0.  If the python folks love
it, we can consider adding it to core, and then we can have the argument
about whether to depreciate the older version.

--Josh Berkus

-- 
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] plpython3

2010-02-01 Thread James William Pye
On Feb 1, 2010, at 2:13 PM, Bruce Momjian wrote:
> I would love to know why PL/Python can't be incrementally improved like
> the rest of our code.

AFAICT, there are two primary, perhaps identifying, parts to a PL extension: 
code management (compilation, execution, etc) and type I/O (conversion in most 
PLs). (well, aside from the language itself =)

My proposed extension chooses a different design for both of those parts.

It didn't make sense to try and incrementally change PL/Python because I would 
have been rewriting the whole thing anyways. Not to mention breaking user code 
in the process for the mentioned parts--thus the Python 3 target.

Hope this clears things up.
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plpython3

2010-02-01 Thread Joshua D. Drake
On Mon, 2010-02-01 at 16:31 -0500, Bruce Momjian wrote:
>  
> > > I would love to know why PL/Python can't be incrementally improved like
> > > the rest of our code.
> > 
> > It has been. That is exactly what PeterE has been doing.
> > 
> > However, if you look at this whole thread, you will see the James has a
> > very different view of the implementation. One that at least appears to
> > be more advanced and "pythonic" than our version.
> 
> More "pythonic" in its internal processing or in its user interface?

User interface and also internal processing (see the types discussion).

Joshua D. Drake




-- 
PostgreSQL.org Major Contributor
Command Prompt, Inc: http://www.commandprompt.com/ - 503.667.4564
Consulting, Training, Support, Custom Development, Engineering
Respect is earned, not gained through arbitrary and repetitive use or Mr. or 
Sir.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] Make TOAST_TUPLES_PER_PAGE configurable per table.

2010-02-01 Thread Jesper Krogh
Hi

This is my first attempt to hack PostgreSQL (even C actually), so bear
over with obvious mistakes done.

I've had a wish to be able to teach Postgres a bit more about how to
store its data on disk. Our systems is a typical web-based system where
all access more or less can be devided into 2 categories:
"List view" .. which is overview, counts, aggregates on simple values
with 50..200 rows per page and
"details views" which is more or less all data from a single rows
combined with aggregates of relations and similar.

Bases on this knowledge I know that there is a significant amount of
data stored "inline" in tuples and being read of disk for the listing
that is "never needed". At the moment it'll try to compress an get below
pagesize/4 ~ 2KB/tuple before it gets out to TOASTING the large tables.

Looking at the current implementation it seems to "do the right thing"
since the "large, variable length" attributes are the "most likely" to
not be shown on listing pages anyway, but it is not aggressive enough
(in my view for all common web-things), so this patch tries to make
TOAST_TUPLES_PER_PAGE per table configurable (the desired tuple-density
on the main storage).

This patch enables users to set TOAST_TUPLES_PER_PAGE with

ALTER TABLE  SET (tuples_per_page = X); .. currently with 1 <= X
<= 32;

ftstest=# create table testtext8(id SERIAL,col text) with
(tuples_per_page=8);
NOTICE:  CREATE TABLE will create implicit sequence "testtext8_id_seq"
for serial column "testtext8.id"
CREATE TABLE
ftstest=# create table testtext2(id SERIAL,col text) with
(tuples_per_page=2);
NOTICE:  CREATE TABLE will create implicit sequence "testtext2_id_seq"
for serial column "testtext2.id"
CREATE TABLE
ftstest=# insert into testtext8(col) (select (select
array_to_string(array_agg(chr((random()*95+30)::integer)),'') from
generate_series(1,3000)) as testtext from generate_series(1,5));
INSERT 0 5
ftstest=# insert into testtext2(col) (select (select
array_to_string(array_agg(chr((random()*95+30)::integer)),'') from
generate_series(1,3000)) as testtext from generate_series(1,5));
INSERT 0 5
ftstest=# \timing
### Here i stop PG and echo 3 > /proc/sys/vm/drop_caches
ftstest=# select count(id) from testtext2;
FATAL:  terminating connection due to administrator command
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Succeeded.
ftstest=# select count(id) from testtext2;
 count
---
 5
(1 row)

Time: 4613.044 ms
ftstest=# select count(id) from testtext8;
 count
---
 5
(1 row)

Time: 318.743 ms

This obviously comes with a drawback if I actually "Need" the data.

ftstest=# select max(length(col)) from testtext2;
 max
--
 3000
(1 row)

Time: 1445.016 ms
ftstest=# select max(length(col)) from testtext8;
 max
--
 3000
(1 row)

Time: 4184.994 ms

relation |size
-+
 pg_toast.pg_toast_1450869   | 195 MB
 public.testtext2| 195 MB
 public.testtext8| 2552 kB


No documentation on the patch. I'll do that a bit later.

Generally speaking.. if you have some knowledge about the access
patterns of your data then this patch can enable you to teach postgresql
to take advantage of that. In my situation I would estimate that the
production set would be able to drop a couple of GB from main memory
(leaving room for more index-pages and such).


Thanks in advance.

-- 
Jesper Krogh
diff -rc ../postgresql-8.5alpha3.orig/src/backend/access/common/reloptions.c ./src/backend/access/common/reloptions.c
*** ../postgresql-8.5alpha3.orig/src/backend/access/common/reloptions.c	2009-08-27 19:18:44.0 +0200
--- ./src/backend/access/common/reloptions.c	2010-02-01 21:12:41.0 +0100
***
*** 15,20 
--- 15,21 
  
  #include "postgres.h"
  
+ #include "access/tuptoaster.h"
  #include "access/gist_private.h"
  #include "access/hash.h"
  #include "access/nbtree.h"
***
*** 157,162 
--- 158,170 
  			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
  		}, -1, 0, 20
  	},
+ 	{
+ 		{
+ 			"tuples_per_page",
+ 			"Desired number of tuples per page (worst-case)",
+ 			RELOPT_KIND_HEAP
+ 		},TOAST_TUPLES_PER_PAGE , 1,32
+ 	},
  	/* list terminator */
  	{{NULL}}
  };
***
*** 1074,1079 
--- 1082,1088 
  	int			numoptions;
  	static const relopt_parse_elt tab[] = {
  		{"fillfactor", RELOPT_TYPE_INT, offsetof(StdRdOptions, fillfactor)},
+ 		{"tuples_per_page", RELOPT_TYPE_INT, offsetof(StdRdOptions, tuples_per_page)},
  		{"autovacuum_enabled", RELOPT_TYPE_BOOL,
  		offsetof(StdRdOptions, autovacuum) +offsetof(AutoVacOpts, enabled)},
  		{"autovacuum_vacuum_threshold", RELOPT_TYPE_INT,
diff -rc ../postgresql-8.5alpha3.orig/src/backend/access/heap/tuptoaster.c ./src/backend/access/heap/tuptoaster.c
*** ../postgresq

Re: [HACKERS] plpython3

2010-02-01 Thread Bruce Momjian
Joshua D. Drake wrote:
> On Mon, 2010-02-01 at 16:13 -0500, Bruce Momjian wrote:
> > Alvaro Herrera wrote:
> > > Peter Eisentraut escribi?:
> > > > On m?n, 2010-02-01 at 12:01 -0800, Nathan Boley wrote:
> > > > > I code nearly exclusively in python and C, but I have
> > > > > often found pl/python to be very unwieldy.  For this reason I often
> > > > > use pl/perl or pl/pgsql for problems that, outside of postgres, I
> > > > > would always use python.
> > > > 
> > > > I find that curious, because much of the criticism about the current
> > > > PL/Python can be traced back to the fact that the implementation used to
> > > > be an exact copy of PL/Perl.
> > > 
> > > Perhaps the problem is that PL/Perl used to be unwieldy back when
> > > PL/Python was created.  PL/Perl has definitely seen a lot more activity.
> > 
> > I would love to know why PL/Python can't be incrementally improved like
> > the rest of our code.
> 
> It has been. That is exactly what PeterE has been doing.
> 
> However, if you look at this whole thread, you will see the James has a
> very different view of the implementation. One that at least appears to
> be more advanced and "pythonic" than our version.

More "pythonic" in its internal processing or in its user interface?

-- 
  Bruce Momjian  http://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] plpython3

2010-02-01 Thread Joshua D. Drake
On Mon, 2010-02-01 at 16:13 -0500, Bruce Momjian wrote:
> Alvaro Herrera wrote:
> > Peter Eisentraut escribi?:
> > > On m?n, 2010-02-01 at 12:01 -0800, Nathan Boley wrote:
> > > > I code nearly exclusively in python and C, but I have
> > > > often found pl/python to be very unwieldy.  For this reason I often
> > > > use pl/perl or pl/pgsql for problems that, outside of postgres, I
> > > > would always use python.
> > > 
> > > I find that curious, because much of the criticism about the current
> > > PL/Python can be traced back to the fact that the implementation used to
> > > be an exact copy of PL/Perl.
> > 
> > Perhaps the problem is that PL/Perl used to be unwieldy back when
> > PL/Python was created.  PL/Perl has definitely seen a lot more activity.
> 
> I would love to know why PL/Python can't be incrementally improved like
> the rest of our code.

It has been. That is exactly what PeterE has been doing.

However, if you look at this whole thread, you will see the James has a
very different view of the implementation. One that at least appears to
be more advanced and "pythonic" than our version.

Joshua D. Drake



-- 
PostgreSQL.org Major Contributor
Command Prompt, Inc: http://www.commandprompt.com/ - 503.667.4564
Consulting, Training, Support, Custom Development, Engineering
Respect is earned, not gained through arbitrary and repetitive use or Mr. or 
Sir.


-- 
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] plpython3

2010-02-01 Thread Bruce Momjian
Alvaro Herrera wrote:
> Peter Eisentraut escribi?:
> > On m?n, 2010-02-01 at 12:01 -0800, Nathan Boley wrote:
> > > I code nearly exclusively in python and C, but I have
> > > often found pl/python to be very unwieldy.  For this reason I often
> > > use pl/perl or pl/pgsql for problems that, outside of postgres, I
> > > would always use python.
> > 
> > I find that curious, because much of the criticism about the current
> > PL/Python can be traced back to the fact that the implementation used to
> > be an exact copy of PL/Perl.
> 
> Perhaps the problem is that PL/Perl used to be unwieldy back when
> PL/Python was created.  PL/Perl has definitely seen a lot more activity.

I would love to know why PL/Python can't be incrementally improved like
the rest of our code.

-- 
  Bruce Momjian  http://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] plpython3

2010-02-01 Thread Alvaro Herrera
Peter Eisentraut escribió:
> On mån, 2010-02-01 at 12:01 -0800, Nathan Boley wrote:
> > I code nearly exclusively in python and C, but I have
> > often found pl/python to be very unwieldy.  For this reason I often
> > use pl/perl or pl/pgsql for problems that, outside of postgres, I
> > would always use python.
> 
> I find that curious, because much of the criticism about the current
> PL/Python can be traced back to the fact that the implementation used to
> be an exact copy of PL/Perl.

Perhaps the problem is that PL/Perl used to be unwieldy back when
PL/Python was created.  PL/Perl has definitely seen a lot more activity.

-- 
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] plpython3

2010-02-01 Thread Joshua D. Drake
On Mon, 2010-02-01 at 22:35 +0200, Peter Eisentraut wrote:
> On mån, 2010-02-01 at 12:01 -0800, Nathan Boley wrote:
> > I code nearly exclusively in python and C, but I have
> > often found pl/python to be very unwieldy.  For this reason I often
> > use pl/perl or pl/pgsql for problems that, outside of postgres, I
> > would always use python.
> 
> I find that curious, because much of the criticism about the current
> PL/Python can be traced back to the fact that the implementation used to
> be an exact copy of PL/Perl.

Well my guess is, if you want to code python, you don't want to feel
like you are coding perl and thus you might as well just code perl?

Joshua D. Drake


> 


-- 
PostgreSQL.org Major Contributor
Command Prompt, Inc: http://www.commandprompt.com/ - 503.667.4564
Consulting, Training, Support, Custom Development, Engineering
Respect is earned, not gained through arbitrary and repetitive use or Mr. or 
Sir.


-- 
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] plpython3

2010-02-01 Thread Peter Eisentraut
On mån, 2010-02-01 at 12:01 -0800, Nathan Boley wrote:
> I code nearly exclusively in python and C, but I have
> often found pl/python to be very unwieldy.  For this reason I often
> use pl/perl or pl/pgsql for problems that, outside of postgres, I
> would always use python.

I find that curious, because much of the criticism about the current
PL/Python can be traced back to the fact that the implementation used to
be an exact copy of PL/Perl.


-- 
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] Provide rowcount for utility SELECTs

2010-02-01 Thread Robert Haas
2010/1/12 Boszormenyi Zoltan :
> Tom Lane írta:
>> Alvaro Herrera  writes:
>>
>>> But it would be broken in very obvious ways, no?  It's not like it would
>>> be silently broken and thus escape testing ...
>>>
>>
>> Well, if we wanted to adopt that approach, we should add the count to
>> *all* SELECT tags not just a small subset.  As the patch stands it
>> seems entirely possible that a breakage would escape immediate notice.
>>
>
> Can you give me an example that would return
> plain "SELECT" after my new patch? I added
> one more change to the patch, is it enough to return
> "SELECT N" in every case now?

I just tested this, so I can say definitely: no.  I hacked psql with
the attached patch, and if you just do a plain old SELECT * FROM
table, you get back only SELECT, not SELECT N.

...Robert
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 70b9310..5b37d3d 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -698,7 +698,6 @@ static bool
 PrintQueryResults(PGresult *results)
 {
 	bool		success = false;
-	const char *cmdstatus;
 
 	if (!results)
 		return false;
@@ -709,11 +708,7 @@ PrintQueryResults(PGresult *results)
 			/* print the data ... */
 			success = PrintQueryTuples(results);
 			/* if it's INSERT/UPDATE/DELETE RETURNING, also print status */
-			cmdstatus = PQcmdStatus(results);
-			if (strncmp(cmdstatus, "INSERT", 6) == 0 ||
-strncmp(cmdstatus, "UPDATE", 6) == 0 ||
-strncmp(cmdstatus, "DELETE", 6) == 0)
-PrintQueryStatus(results);
+			PrintQueryStatus(results);
 			break;
 
 		case PGRES_COMMAND_OK:

-- 
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] plpython3

2010-02-01 Thread Nathan Boley
> I think it would be great for you to review it... I doubt that will
> cause it to get committed for 9.0, but my doubt is no reason for you
> to hold off reviewing it.

I assumed so, but the pretense of a chance will probably help to motivate me :-)

I'll have something by Thursday, and then 'Returned with Feedback'
will at least be factual.

Best,
Nathan

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Hot Standby and VACUUM FULL

2010-02-01 Thread Tom Lane
I wrote:
> The design I sketched doesn't require such an assumption anyway.  Once
> the map file is written, the relocation is effective, commit or no.
> As long as we restrict relocations to maintenance operations such as
> VACUUM FULL, which have no transactionally significant results, this
> doesn't seem like a problem.  What we do need is that after a CLUSTER
> or V.F., which is going to relocate not only the rel but its indexes,
> the relocations of the rel and its indexes have to all "commit"
> atomically.  But saving up the transaction's map changes and applying
> them in one write takes care of that.

BTW, I noticed a couple of other issues that need to be dealt with to
make that safe.  During CLUSTER/V.F. we typically try to update the
relation's relfilenode, relpages, reltuples, relfrozenxid (in
setNewRelfilenode) as well as its toastrelid (in swap_relation_files).
These are regular transactional updates to the pg_class tuple that will
fail to commit if the outer transaction rolls back.  However:

* For a mapped relation, both the old and new relfilenode will be zero,
so it doesn't matter.

* Possibly losing the updates of relpages and reltuples is not critical.

* For relfrozenxid, we can simply force the new and old values to be the
same rather than hoping to advance the value, if we're dealing with a
mapped relation.  Or just let it be; I think that losing an advance
of relfrozenxid wouldn't be critical either.

* We can not change the toast rel OID of a shared catalog -- there's no
way to propagate that into the other copies of pg_class.  So we need to
rejigger the logic for heap rewriting a little bit.  Toast rel swapping
has to be handled by swapping their relfilenodes not their OIDs.  This
is no big deal as far as cluster.c itself is concerned, but the tricky
part is that when we write new toasted values into the new toast rel,
the TOAST pointers going into the new heap have to be written with the
original toast-table OID value not the one that the transient target
toast rel has got.  This is doable but it would uglify the TOAST API a
bit I think.  Another possibility is to treat the toast rel OID of a
catalog as something that can be supplied by the map file.  Not sure
which way to jump.

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] plpython3

2010-02-01 Thread Robert Haas
On Mon, Feb 1, 2010 at 3:01 PM, Nathan Boley  wrote:
>> On the basis of all of the foregoing, I don't think we can consider
>> this patch further for this CommitFest and will update
>> commitfest.postgresql.org accordingly.
>
> FWIW, I am very excited about this patch and would be happy to review
> it but have been very busy over the past month. If I can promise a
> review by Thursday morning could we keep it active? Hopefully, at the
> very least, I can provide some useful feedback and spawn some
> community interest.
>
> I am worried that there is a bit of a chicken and an egg problem with
> this patch. I code nearly exclusively in python and C, but I have
> often found pl/python to be very unwieldy.  For this reason I often
> use pl/perl or pl/pgsql for problems that, outside of postgres, I
> would always use python. From the documentation, this patch seems like
> an enormous step in the right direction.

I think it would be great for you to review it... I doubt that will
cause it to get committed for 9.0, but my doubt is no reason for you
to hold off reviewing it.

...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] plpython3

2010-02-01 Thread Joshua D. Drake
On Mon, 2010-02-01 at 12:01 -0800, Nathan Boley wrote:
> > On the basis of all of the foregoing, I don't think we can consider
> > this patch further for this CommitFest and will update
> > commitfest.postgresql.org accordingly.
> 
> FWIW, I am very excited about this patch and would be happy to review
> it but have been very busy over the past month. If I can promise a
> review by Thursday morning could we keep it active? Hopefully, at the
> very least, I can provide some useful feedback and spawn some
> community interest.
> 
> I am worried that there is a bit of a chicken and an egg problem with
> this patch. I code nearly exclusively in python and C, but I have
> often found pl/python to be very unwieldy.  For this reason I often
> use pl/perl or pl/pgsql for problems that, outside of postgres, I
> would always use python. From the documentation, this patch seems like
> an enormous step in the right direction.

I am a +1

Joshua D. Drake


> 
> -Nathan
> 


-- 
PostgreSQL.org Major Contributor
Command Prompt, Inc: http://www.commandprompt.com/ - 503.667.4564
Consulting, Training, Support, Custom Development, Engineering
Respect is earned, not gained through arbitrary and repetitive use or Mr. or 
Sir.


-- 
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] plpython3

2010-02-01 Thread Nathan Boley
> On the basis of all of the foregoing, I don't think we can consider
> this patch further for this CommitFest and will update
> commitfest.postgresql.org accordingly.

FWIW, I am very excited about this patch and would be happy to review
it but have been very busy over the past month. If I can promise a
review by Thursday morning could we keep it active? Hopefully, at the
very least, I can provide some useful feedback and spawn some
community interest.

I am worried that there is a bit of a chicken and an egg problem with
this patch. I code nearly exclusively in python and C, but I have
often found pl/python to be very unwieldy.  For this reason I often
use pl/perl or pl/pgsql for problems that, outside of postgres, I
would always use python. From the documentation, this patch seems like
an enormous step in the right direction.

-Nathan

-- 
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?] strange behavior in ALTER TABLE ... RENAME TO on inherited columns

2010-02-01 Thread Robert Haas
On Mon, Feb 1, 2010 at 2:03 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> Oh, one other thing.  Should we backpatch this, or just apply to HEAD?
>
> Just HEAD imo.  Without any complaints from the field, I don't think
> it's worth taking any risks for.  It's not like multiple inheritance
> is heavily used ...

OK, done.

...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] Hot Standby: Relation-specific deferred conflict resolution

2010-02-01 Thread Simon Riggs
On Fri, 2010-01-29 at 15:01 +, Simon Riggs wrote:

> Putting it back takes time and
> given enough of that rare cloth, it will eventually be put back.

Looks like I'll have time to add the starts-at-shutdown-checkpoint item
back in after all.

I'd appreciate it if you could review the relation-specific conflict
patch, 'cos it's still important.

-- 
 Simon Riggs   www.2ndQuadrant.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plpython3

2010-02-01 Thread James William Pye
On Feb 1, 2010, at 11:29 AM, Joshua D. Drake wrote:

> On Mon, 2010-02-01 at 13:20 -0500, Robert Haas wrote:
> 
>> On the basis of all of the foregoing, I don't think we can consider
>> this patch further for this CommitFest and will update
>> commitfest.postgresql.org accordingly.  If the user community grows or
>> if one of the committers takes an interest in this down the road, I
>> think we could consider it for a future release.
>> 
> 
> I spoke with James offline about this as well. My understanding (correct
> me James) is that he is working on an implementation that can be
> installed via PGXS.

yep, mostly done: http://github.com/jwp/pg-python

The tests that can pass are passing on 8.3 and 8.4 now, save optimized Cursor 
returns in 8.3(the materialize preferred flag).

Also made some other improvements like getting rid of the ugly 
`__func__.stateful = True` in favor of a decorator, @Stateful. (Thanks to 
Harald for a push in that direction.)

Right now, I'm trying to trim some of the easy issues[1] and getting a project 
web page up. I expect to be able to make a release soon, and I'll follow-up to 
this thread when I do.

However, the lack of release files shouldn't stop anyone from trying it out. =)

Snapshot:
 http://github.com/jwp/pg-python/zipball/master


[1] http://github.com/jwp/pg-python/issues
-- 
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] plpython3

2010-02-01 Thread David E. Wheeler
On Feb 1, 2010, at 10:53 AM, Tom Lane wrote:

> The first thought that comes to mind is "plpythonng", following a
> tradition established by the tcl client rewrite among others ... but
> that double n doesn't read very well.

And without it, you have a thong. Who's going to wear that?

Best,

David

-- 
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?] strange behavior in ALTER TABLE ... RENAME TO on inherited columns

2010-02-01 Thread Tom Lane
Robert Haas  writes:
> Oh, one other thing.  Should we backpatch this, or just apply to HEAD?

Just HEAD imo.  Without any complaints from the field, I don't think
it's worth taking any risks for.  It's not like multiple inheritance
is heavily used ...

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] Pathological regexp match

2010-02-01 Thread Michael Glaesemann


On Jan 31, 2010, at 22:14 , Tom Lane wrote:


The Tcl folk accepted that patch, so I went ahead and applied it to
our code.  It would still be a good idea for us to do any testing we
can on it, though.


I applied the patch and ran both the test query I submitted as well as  
original problematic query that triggered the report, and it runs much  
faster. Thanks for the fix!


Michael Glaesemann
michael.glaesem...@myyearbook.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] plpython3

2010-02-01 Thread Robert Haas
On Mon, Feb 1, 2010 at 2:00 PM, Alvaro Herrera
 wrote:
> Tom Lane escribió:
>
>> The first thought that comes to mind is "plpythonng", following a
>> tradition established by the tcl client rewrite among others ... but
>> that double n doesn't read very well.
>
> plpythoNG perhaps?

ROFL.  It's a scantily-clad implementation of PL/py...

...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] plpython3

2010-02-01 Thread Alvaro Herrera
Tom Lane escribió:

> The first thought that comes to mind is "plpythonng", following a
> tradition established by the tcl client rewrite among others ... but
> that double n doesn't read very well.

plpythoNG perhaps?

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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?] strange behavior in ALTER TABLE ... RENAME TO on inherited columns

2010-02-01 Thread Robert Haas
On Mon, Feb 1, 2010 at 1:40 PM, Robert Haas  wrote:
>> Looks sane otherwise.
>
> Thanks for the review.

Oh, one other thing.  Should we backpatch this, or just apply to HEAD?

...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] plpython3

2010-02-01 Thread Tom Lane
Robert Haas  writes:
> To recap the votes I've seen on this thread and elsewhere:

> - JD is very enthusiastic about this patch
> - So is the OP
> - Josh Berkus and I are both dubious about having two in-core PL/pythons
> - Peter Eisentraut prefers the original implementation
> - Greg Smith thinks (if I'm not putting words into his mouth) that
> this might be worth considering, but not for 9.0

One other problem with accepting this to be parallel with the existing
plpython is that there's a name conflict: Peter's work to allow the
existing PL to use Python 3 has already claimed the name "plpython3".
Whether it's to be distributed in core or separately, I think something
needs to be done about that.

The first thought that comes to mind is "plpythonng", following a
tradition established by the tcl client rewrite among others ... but
that double n doesn't read very well.

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] mailing list archiver chewing patches

2010-02-01 Thread Magnus Hagander
2010/2/1 Robert Haas :
> On Mon, Feb 1, 2010 at 11:41 AM, Tom Lane  wrote:
>> Robert Haas  writes:
>>> On Mon, Feb 1, 2010 at 11:28 AM, Tom Lane  wrote:
        * A sends a message
        * B replies, cc'ing A and the list
        * B's reply to list is delayed by greylisting
        * A replies to B's reply (cc'ing list)
        * A's reply goes through immediately
        * B's reply shows up a bit later

 That happens pretty frequently IME.
>>
>>> Yeah - and sometimes the delay can be DAYS.
>>
>> Greylisting wouldn't explain a delay of more than an hour or so.
>> OTOH, if B's reply got held for moderation for some reason, then
>> yeah it could be days :-(.  But in that case the rest of the list
>> didn't see it in real-time either, so having it show up out of
>> "logical" sequence in the archive doesn't seem like a terrible
>> reflection of reality.  I'm just concerned about the threading not
>> being sensitive to skews on the order of a few minutes --- those
>> are extremely common.
>
> I not infrequently receive messages out of sequence by time periods
> well in excess of a few minutes.
>
> Don't know why, but I do.

Quite often, it's stuck in the moderation queue.

Not quite as often, but still fairly frequently, it's stuck somewhere
in the hub.org relaying/antispam blackbox.


-- 
 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] remove contrib/xml2

2010-02-01 Thread Robert Haas
On Mon, Feb 1, 2010 at 1:38 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> My feeling is that if it's as flakey and unreliable as it currently
>> is, we shouldn't ship it.  Removing it from CVS doesn't mean "you
>> can't use this any more"; this is open source.  It just means people
>> will have to go and get an old copy out of CVS and presumably in so
>> doing they will be aware that we've removed it for a reason.  We have
>> a well-deserved reputation for quality and I would like to see us
>> preserve that.
>
> [ shrug... ]  It is not any more flaky than it's been since it was put in.
> The people who have been depending on it presumably have use-patterns
> for which it doesn't fail, and we're not going to be doing them a
> service by ripping out functionality for which we can't offer a
> replacement.

Well, then we'd at least better update the documentation to (1) remove
the statement that this will be removed in 8.4 (since we didn't), and
(2) add a very, very large warning that this will crash if you do
almost anything with it.

...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?] strange behavior in ALTER TABLE ... RENAME TO on inherited columns

2010-02-01 Thread Robert Haas
On Mon, Feb 1, 2010 at 1:31 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> I have cleaned up and simplified this patch.  Attached is the version
>> I intend to commit.  Changes:
>
> Minor suggestions:
>
> I think the names like "rel_parents" would read better as
> "rel_numparents" etc.  As-is, the reader could be forgiven for expecting
> that this will be a list of parent relation OIDs or some such.

I thought about that but ended up being lazy and leaving it the way I
had it.  I'll go be un-lazy.

> The new loop added within find_all_inheritors could really do with an
> addition to the comments, along the line of "If a child is already
> seen, increment the corresponding numparents count".

OK.

> I don't trust the proposed "order by attrelid" business in the
> regression test --- once in a blue moon, that will fail because the
> OID counter wrapped around mid-test, and we'll get an unreproducible
> bug report.  I'd suggest order by attrelid::regclass::text.

Wow, didn't think of that.  Will change.

> Looks sane otherwise.

Thanks for the review.

...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] plpython3

2010-02-01 Thread Robert Haas
On Mon, Feb 1, 2010 at 1:29 PM, Joshua D. Drake  wrote:
> On Mon, 2010-02-01 at 13:20 -0500, Robert Haas wrote:
>> On the basis of all of the foregoing, I don't think we can consider
>> this patch further for this CommitFest and will update
>> commitfest.postgresql.org accordingly.  If the user community grows or
>> if one of the committers takes an interest in this down the road, I
>> think we could consider it for a future release.
>
> I spoke with James offline about this as well. My understanding (correct
> me James) is that he is working on an implementation that can be
> installed via PGXS.

Sounds good!

...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] remove contrib/xml2

2010-02-01 Thread Tom Lane
Robert Haas  writes:
> My feeling is that if it's as flakey and unreliable as it currently
> is, we shouldn't ship it.  Removing it from CVS doesn't mean "you
> can't use this any more"; this is open source.  It just means people
> will have to go and get an old copy out of CVS and presumably in so
> doing they will be aware that we've removed it for a reason.  We have
> a well-deserved reputation for quality and I would like to see us
> preserve that.

[ shrug... ]  It is not any more flaky than it's been since it was put in.
The people who have been depending on it presumably have use-patterns
for which it doesn't fail, and we're not going to be doing them a
service by ripping out functionality for which we can't offer a
replacement.

As for the "quality" argument, contrib modules are not guaranteed
to be of the same standard as the core code; anyone who thinks they are
should disabuse themselves of the notion by reading some of that code.

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] [BUG?] strange behavior in ALTER TABLE ... RENAME TO on inherited columns

2010-02-01 Thread Tom Lane
Robert Haas  writes:
> I have cleaned up and simplified this patch.  Attached is the version
> I intend to commit.  Changes:

Minor suggestions:

I think the names like "rel_parents" would read better as
"rel_numparents" etc.  As-is, the reader could be forgiven for expecting
that this will be a list of parent relation OIDs or some such.

The new loop added within find_all_inheritors could really do with an
addition to the comments, along the line of "If a child is already
seen, increment the corresponding numparents count".

I don't trust the proposed "order by attrelid" business in the
regression test --- once in a blue moon, that will fail because the
OID counter wrapped around mid-test, and we'll get an unreproducible
bug report.  I'd suggest order by attrelid::regclass::text.

Looks sane otherwise.

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] plpython3

2010-02-01 Thread Joshua D. Drake
On Mon, 2010-02-01 at 13:20 -0500, Robert Haas wrote:

> On the basis of all of the foregoing, I don't think we can consider
> this patch further for this CommitFest and will update
> commitfest.postgresql.org accordingly.  If the user community grows or
> if one of the committers takes an interest in this down the road, I
> think we could consider it for a future release.
> 

I spoke with James offline about this as well. My understanding (correct
me James) is that he is working on an implementation that can be
installed via PGXS.


Joshua D. Drake


> ...Robert
> 


-- 
PostgreSQL.org Major Contributor
Command Prompt, Inc: http://www.commandprompt.com/ - 503.667.4564
Consulting, Training, Support, Custom Development, Engineering
Respect is earned, not gained through arbitrary and repetitive use or Mr. or 
Sir.


-- 
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] remove contrib/xml2

2010-02-01 Thread Robert Haas
On Thu, Jan 28, 2010 at 5:41 PM, Tom Lane  wrote:
> Andrew Dunstan  writes:
>> Robert Haas wrote:
>>> I think we need to either (1) fix the bugs and update the
>>> documentation to remove the statement that this will be removed or (2)
>>> actually remove it.
>
>> I agree it's a mess but I don't think just abandoning the functionality
>> is a good idea.
>
> Yeah, we can't really remove it until we have a plausible substitute for
> the xpath_table functionality.  This is in the TODO list ...

My feeling is that if it's as flakey and unreliable as it currently
is, we shouldn't ship it.  Removing it from CVS doesn't mean "you
can't use this any more"; this is open source.  It just means people
will have to go and get an old copy out of CVS and presumably in so
doing they will be aware that we've removed it for a reason.  We have
a well-deserved reputation for quality and I would like to see us
preserve that.

Is anyone going to try to fix this for 9.0?

...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] contrib\xml2 package's xpath_table function in PostgreSQL

2010-02-01 Thread Robert Haas
On Mon, Feb 1, 2010 at 1:20 PM, M Z  wrote:
> Is there a way to fix it so that those functions are usable in 8.4 without
> crashing the server?

Nobody seems to be interested enough to figure that out and submit a
patch to fix it.  If someone does, I think it would have a good chance
of being accepted.

...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] plpython3

2010-02-01 Thread Robert Haas
On Sat, Jan 23, 2010 at 3:28 PM, James William Pye  wrote:
> On Jan 14, 2010, at 7:08 PM, Greg Smith wrote:
>> So more targeted examples like you're considering now would help.
>
> Here's the trigger example which should help reveal some of the advantages of 
> "native typing". This is a generic trigger that constructs and logs 
> manipulation statements for simple replication purposes.
>
> The original plpython version is located here:
>
>  http://ar.pycon.org/common/2009/talkdata/PyCon2009/020/plpython.txt
>  [You'll need to scroll down to the very bottom of that page.]
>
>
> There are three points in this example that need to be highlighted:
>
>  1. There is no need for a "mogrify" function (see original in the above 
> link).
>  2. Attributes/columns of the records (new/old) are extracted when referenced.
>  3. The comparisons in after_update uses the data type's actual inequality 
> operator.
>
> The first point is true because "native typing" gives the user direct access 
> to a given type's typoutput via ``str(ob)``. This makes constructing the PG 
> string representation of a given object *much* easier--quote_nullable, and 
> done. The original plpython example will need to be updated to compensate for 
> any changes in conversion: arrays will now need special handling and MD 
> arrays will not work at all. It also relies heavily on the Python object 
> representation matching PG's; where that fails, special cases need to be 
> implemented(composites, notably). All of that compensation performed in the 
> original version is unnecessary in the plpython3 version.
>
> The second point touches on the "efficiency" that was referenced in an 
> earlier message. No cycles are spent converting the contents of a container 
> object unless the user chooses to. Naturally, there is no advantage 
> performance-wise if you are always converting everything.
> I'd wager that with triggers, it's rare that everything needs to be converted.
>
> The third point reveals that Postgres.Object instances--a component of native 
> typing--use the data type's operator for inequality. It's not limited to 
> comparisons as all available Python operators are mapped to corresponding 
> operators in PG. For many or all primitives, there is no added value over 
> conversion. However, this provides a lot of convenience when working with 
> UDTs, datetime types, and geometric types.
>
> ...ISTM that the primary advantage of "native typing" is that we get to 
> define the Python interface to a given Postgres data type.

When I initially read the description of this patch, I had really no
idea what it was about, and I have to say that over the last several
weeks, the subsequent posts have increased my interest substantially,
and I think it could potentially have a future in core, or in contrib.
 The performance gains from native typing are particularly
interesting.

That having been said, we don't have a reviewer for this patch, and
more to the point, even if we did, I don't believe we have a COMMITTER
for this patch.  The only committer who has done any significant work
on the existing PL/python implementation for this release is Peter
Eisentraut, and he's made it pretty clear that he's not interested in
this reimplementation.  I personally do not know Python even as a
user, let alone as a hacker, so I'm poorly qualified to review it
myself.  Even were it otherwise, I know that if I were to commit this
I would to some degree be on the hook to maintain it forever, which is
more than I really want to take on.  I suspect other committers feel
similarly; or if not, they haven't chimed in on any of the relevant
threads to say so.

To recap the votes I've seen on this thread and elsewhere:

- JD is very enthusiastic about this patch
- So is the OP
- Josh Berkus and I are both dubious about having two in-core PL/pythons
- Peter Eisentraut prefers the original implementation
- Greg Smith thinks (if I'm not putting words into his mouth) that
this might be worth considering, but not for 9.0

On the basis of all of the foregoing, I don't think we can consider
this patch further for this CommitFest and will update
commitfest.postgresql.org accordingly.  If the user community grows or
if one of the committers takes an interest in this down the road, I
think we could consider it for a future release.

...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] contrib\xml2 package's xpath_table function in PostgreSQL

2010-02-01 Thread M Z
Is there a way to fix it so that those functions are usable in 8.4 without
crashing the server?

Thanks,
M Z

On Mon, Feb 1, 2010 at 10:50 AM, Robert Haas  wrote:

> The functions haven't actually been removed in 8.4, in spite of the
> deprecation notice.  But it's very easy to use them in a way that
> crashes the entire server, so you're playing with fire.  :-(
>
> ...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?] strange behavior in ALTER TABLE ... RENAME TO on inherited columns

2010-02-01 Thread Robert Haas
2010/1/31 KaiGai Kohei :
> The attached patch modified find_all_inheritors() to return the list of
> expected inhcount, if List * pointer is given. And, it focuses on only
> the bugs in renameatt() case.

I have cleaned up and simplified this patch.  Attached is the version
I intend to commit.  Changes:

- make find_all_inheritors return the list of OIDs, as previously,
instead of using an out parameter
- undo some useless variable renamings
- undo some useless whitespace changes
- rework comments
- fix regression tests to avoid using the same alias twice in the same query
- add an ORDER BY clause to the regression tests so that they pass on my machine
- improve the names of some of the new variables
- instead of adding an additional argument to renameatt(), just
replace the existing 'bool recursing' with 'int expected_parents'.
This allows merging the two versions of the "cannot rename inherited
column" message together, which seems like a reasonably good idea to
me, though if someone has a better idea that's fine.  I didn't think
the one additional word added to the message provided enough clarity
to make it worth creating another translatable string.

> Also, the ALTER COLUMN TYPE case should be also fixed in the 9.1 release
> (or 9.0.1?).

If the fix is something we could commit for 9.0.1, then we ought to do
it now before 9.0 is released.  If you want to submit a follow-on
patch to address ALTER COLUMN TYPE once this is committed, then please
do so.

...Robert
diff --git a/src/backend/catalog/pg_inherits.c b/src/backend/catalog/pg_inherits.c
index e256f6f..b53f44d 100644
--- a/src/backend/catalog/pg_inherits.c
+++ b/src/backend/catalog/pg_inherits.c
@@ -148,6 +148,9 @@ find_inheritance_children(Oid parentrelId, LOCKMODE lockmode)
  * find_all_inheritors -
  *		Returns a list of relation OIDs including the given rel plus
  *		all relations that inherit from it, directly or indirectly.
+ *		Optionally, it also returns the number of parents found for
+ *		each such relation within the inheritance tree rooted at the
+ *		given rel.
  *
  * The specified lock type is acquired on all child relations (but not on the
  * given rel; caller should already have locked it).  If lockmode is NoLock
@@ -155,9 +158,9 @@ find_inheritance_children(Oid parentrelId, LOCKMODE lockmode)
  * against possible DROPs of child relations.
  */
 List *
-find_all_inheritors(Oid parentrelId, LOCKMODE lockmode)
+find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, List **parents)
 {
-	List	   *rels_list;
+	List	   *rels_list, *rel_parents;
 	ListCell   *l;
 
 	/*
@@ -168,11 +171,13 @@ find_all_inheritors(Oid parentrelId, LOCKMODE lockmode)
 	 * doesn't fetch the next list element until the bottom of the loop.
 	 */
 	rels_list = list_make1_oid(parentrelId);
+	rel_parents = list_make1_int(0);
 
 	foreach(l, rels_list)
 	{
 		Oid			currentrel = lfirst_oid(l);
 		List	   *currentchildren;
+		ListCell   *lc;
 
 		/* Get the direct children of this rel */
 		currentchildren = find_inheritance_children(currentrel, lockmode);
@@ -184,9 +189,35 @@ find_all_inheritors(Oid parentrelId, LOCKMODE lockmode)
 		 * loop, though theoretically there can't be any cycles in the
 		 * inheritance graph anyway.)
 		 */
-		rels_list = list_concat_unique_oid(rels_list, currentchildren);
+		foreach(lc, currentchildren)
+		{
+			Oid		child_oid = lfirst_oid(lc);
+			bool	found = false;
+			ListCell   *lo;
+			ListCell   *li;
+
+			forboth(lo, rels_list, li, rel_parents)
+			{
+if (lfirst_oid(lo) == child_oid)
+{
+	lfirst_int(li)++;
+	found = true;
+	break;
+}
+			}
+
+			if (!found)
+			{
+rels_list = lappend_oid(rels_list, child_oid);
+rel_parents = lappend_int(rel_parents, 1);
+			}
+		}
 	}
 
+	if (parents)
+		*parents = rel_parents;
+	else
+		list_free(rel_parents);
 	return rels_list;
 }
 
diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c
index 8c81eaa..efbd088 100644
--- a/src/backend/commands/alter.c
+++ b/src/backend/commands/alter.c
@@ -126,7 +126,7 @@ ExecRenameStmt(RenameStmt *stmt)
   stmt->subname,		/* old att name */
   stmt->newname,		/* new att name */
   interpretInhOption(stmt->relation->inhOpt),	/* recursive? */
-  false);		/* recursing already? */
+  0);			/* expected inhcount */
 		break;
 	case OBJECT_TRIGGER:
 		renametrig(relid,
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 9062c15..c93b57f 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -1390,7 +1390,8 @@ acquire_inherited_sample_rows(Relation onerel, HeapTuple *rows, int targrows,
 	 * Find all members of inheritance set.  We only need AccessShareLock on
 	 * the children.
 	 */
-	tableOIDs = find_all_inheritors(RelationGetRelid(onerel), AccessShareLock);
+	tableOIDs =
+		find_all_inheritors(RelationGetRelid(onerel), AccessShareLock, NULL);
 
 	/*
 	 * Check that there's at least one descendant, else fa

[HACKERS] Re: [COMMITTERS] pgsql: Augment EXPLAIN output with more details on Hash nodes.

2010-02-01 Thread Robert Haas
On Mon, Feb 1, 2010 at 12:12 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Mon, Feb 1, 2010 at 11:53 AM, Tom Lane  wrote:
>>> This needs to be damped down a bit.  It should not print useless
>>> non-information in cases where the plan wasn't actually run.
>>> Please compare show_sort_info.
>
>> Eh?  When does it do that?
>
> Oh, I'm sorry, it's using hashtable existence to condition the whole
> output.  So my complaint is backwards.  I thought the intention was
> to print the estimated number of batches in all cases, and then the
> actual as well in EXPLAIN ANALYZE.
>
> BTW, I think "estimated" and "actual" would be less confusing
> terminology than "original".

I think (but am not 100% sure) that the number that is computed during
the plan phase is acually thrown away and recomputed during the
execution phase (grep for ExecChooseHashTableSize).  So potentially
there is:

A. the number of buckets and batches estimated during planning
B. the number of buckets and batches decided on at the beginning of execution
C. the number of batches we end up using as a result of work_mem overflow

Right now I'm just printing out B and C; we could add A as well, but I
think there are some changes needed to hold on to that information for
longer than we presently do.  At any rate, the terminology we settle
on should be able to accommodate potentially dumping out all of these
values.

IMO, it's not worth spending an enormous amount of time on this.  The
most important questions that I think people will want to answer are
(1) was this done as a multi-batch hash join?, (b) if so, did it start
out that way or did nbatch increase on the fly?, and (3) how close was
I to overflowing work_mem?  I'm happy to make improvements, but I
don't think we should get too crazy.

...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] Hot Standby and deadlock detection

2010-02-01 Thread Simon Riggs
On Mon, 2010-02-01 at 17:50 +0200, Heikki Linnakangas wrote:

> Umm, so why not run the deadlock check immediately in
> max_standby_delay=-1 case as well? Why is that case handled differently
> from max_standby_delay>0 case?

Done, tested, working.

Will commit tomorrow if no further questions or comments.

-- 
 Simon Riggs   www.2ndQuadrant.com
*** a/src/backend/storage/ipc/procsignal.c
--- b/src/backend/storage/ipc/procsignal.c
***
*** 272,277  procsignal_sigusr1_handler(SIGNAL_ARGS)
--- 272,280 
  	if (CheckProcSignal(PROCSIG_RECOVERY_CONFLICT_SNAPSHOT))
  		RecoveryConflictInterrupt(PROCSIG_RECOVERY_CONFLICT_SNAPSHOT);
  
+ 	if (CheckProcSignal(PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK))
+ 		RecoveryConflictInterrupt(PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK);
+ 
  	if (CheckProcSignal(PROCSIG_RECOVERY_CONFLICT_BUFFERPIN))
  		RecoveryConflictInterrupt(PROCSIG_RECOVERY_CONFLICT_BUFFERPIN);
  
*** a/src/backend/storage/ipc/standby.c
--- b/src/backend/storage/ipc/standby.c
***
*** 127,132  WaitExceedsMaxStandbyDelay(void)
--- 127,135 
  	long	delay_secs;
  	int		delay_usecs;
  
+ 	if (MaxStandbyDelay == -1)
+ 		return false;
+ 
  	/* Are we past max_standby_delay? */
  	TimestampDifference(GetLatestXLogTime(), GetCurrentTimestamp(),
  		&delay_secs, &delay_usecs);
***
*** 351,365  ResolveRecoveryConflictWithLock(Oid dbOid, Oid relOid)
   * they hold one of the buffer pins that is blocking Startup process. If so,
   * backends will take an appropriate error action, ERROR or FATAL.
   *
!  * A secondary purpose of this is to avoid deadlocks that might occur between
!  * the Startup process and lock waiters. Deadlocks occur because if queries
   * wait on a lock, that must be behind an AccessExclusiveLock, which can only
!  * be clared if the Startup process replays a transaction completion record.
!  * If Startup process is waiting then that is a deadlock. If we allowed a
!  * setting of max_standby_delay that meant "wait forever" we would then need
!  * special code to protect against deadlock. Such deadlocks are rare, so the
!  * code would be almost certainly buggy, so we avoid both long waits and
!  * deadlocks using the same mechanism.
   */
  void
  ResolveRecoveryConflictWithBufferPin(void)
--- 354,368 
   * they hold one of the buffer pins that is blocking Startup process. If so,
   * backends will take an appropriate error action, ERROR or FATAL.
   *
!  * We also check for deadlocks before we wait, though applications that cause
!  * these will be extremely rare.  Deadlocks occur because if queries
   * wait on a lock, that must be behind an AccessExclusiveLock, which can only
!  * be cleared if the Startup process replays a transaction completion record.
!  * If Startup process is also waiting then that is a deadlock. The deadlock
!  * can occur if the query is waiting and then the Startup sleeps, or if
!  * Startup is sleeping and the the query waits on a lock. We protect against
!  * only the former sequence here, the latter sequence is checked prior to
!  * the query sleeping, in CheckRecoveryConflictDeadlock().
   */
  void
  ResolveRecoveryConflictWithBufferPin(void)
***
*** 368,378  ResolveRecoveryConflictWithBufferPin(void)
  
  	Assert(InHotStandby);
  
- 	/*
- 	 * Signal immediately or set alarm for later.
- 	 */
  	if (MaxStandbyDelay == 0)
! 		SendRecoveryConflictWithBufferPin();
  	else
  	{
  		TimestampTz now;
--- 371,393 
  
  	Assert(InHotStandby);
  
  	if (MaxStandbyDelay == 0)
! 	{
! 		/*
! 		 * We don't want to wait, so just tell everybody holding the pin to 
! 		 * get out of town.
! 		 */
! 		SendRecoveryConflictWithBufferPin(PROCSIG_RECOVERY_CONFLICT_BUFFERPIN);
! 	}
! 	else if (MaxStandbyDelay == -1)
! 	{
! 		/*
! 		 * Send out a request to check for buffer pin deadlocks before we wait.
! 		 * This is fairly cheap, so no need to wait for deadlock timeout before
! 		 * trying to send it out.
! 		 */
! 		SendRecoveryConflictWithBufferPin(PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK);
! 	}
  	else
  	{
  		TimestampTz now;
***
*** 386,392  ResolveRecoveryConflictWithBufferPin(void)
  			&standby_delay_secs, &standby_delay_usecs);
  
  		if (standby_delay_secs >= MaxStandbyDelay)
! 			SendRecoveryConflictWithBufferPin();
  		else
  		{
  			TimestampTz fin_time;			/* Expected wake-up time by timer */
--- 401,412 
  			&standby_delay_secs, &standby_delay_usecs);
  
  		if (standby_delay_secs >= MaxStandbyDelay)
! 		{
! 			/*
! 			 * We're already behind, so clear a path as quickly as possible.
! 			 */
! 			SendRecoveryConflictWithBufferPin(PROCSIG_RECOVERY_CONFLICT_BUFFERPIN);
! 		}
  		else
  		{
  			TimestampTz fin_time;			/* Expected wake-up time by timer */
***
*** 394,399  ResolveRecoveryConflictWithBufferPin(void)
--- 414,426 
  			int		timer_delay_usecs = 0;
  
  			/*
+ 			 * Send out a request to check for buffer pin deadlocks b

Re: [HACKERS] Package namespace and Safe init cleanup for plperl UPDATE 3 [PATCH]

2010-02-01 Thread Tim Bunce
On Mon, Feb 01, 2010 at 10:46:10AM -0500, Robert Haas wrote:
> On Mon, Feb 1, 2010 at 5:58 AM, Tim Bunce  wrote:
> > p.s. If there is interest in defining a documented API (for DBAs to
> > control what gets loaded into Safe and shared with it) for *9.0*
> > then that could be worked on, once this pach is in, ready for the
> > next commitfest.
> 
> This is the last CommitFest for 9.0.  It's time to wind down
> development on this release and work on trying to get the release
> stabilized and out the door.
> 
> This isn't intended as a disparagement of the work you're doing; I've
> thought about using PL/perl in the past and decided against it exactly
> because of some of the issues you're now fixing.  But we're really out
> of time to get things done for 9.0.

Understood Robert. No problem. (You can't blame me for trying ;-)

Tim.

-- 
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] mailing list archiver chewing patches

2010-02-01 Thread Tom Lane
Robert Haas  writes:
> On Mon, Feb 1, 2010 at 11:28 AM, Tom Lane  wrote:
>>* A sends a message
>>* B replies, cc'ing A and the list
>>* B's reply to list is delayed by greylisting
>>* A replies to B's reply (cc'ing list)
>>* A's reply goes through immediately
>>* B's reply shows up a bit later
>> 
>> That happens pretty frequently IME.

> Yeah - and sometimes the delay can be DAYS.

Greylisting wouldn't explain a delay of more than an hour or so.
OTOH, if B's reply got held for moderation for some reason, then
yeah it could be days :-(.  But in that case the rest of the list
didn't see it in real-time either, so having it show up out of
"logical" sequence in the archive doesn't seem like a terrible
reflection of reality.  I'm just concerned about the threading not
being sensitive to skews on the order of a few minutes --- those
are extremely common.

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] mailing list archiver chewing patches

2010-02-01 Thread Robert Haas
On Mon, Feb 1, 2010 at 11:41 AM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Mon, Feb 1, 2010 at 11:28 AM, Tom Lane  wrote:
>>>        * A sends a message
>>>        * B replies, cc'ing A and the list
>>>        * B's reply to list is delayed by greylisting
>>>        * A replies to B's reply (cc'ing list)
>>>        * A's reply goes through immediately
>>>        * B's reply shows up a bit later
>>>
>>> That happens pretty frequently IME.
>
>> Yeah - and sometimes the delay can be DAYS.
>
> Greylisting wouldn't explain a delay of more than an hour or so.
> OTOH, if B's reply got held for moderation for some reason, then
> yeah it could be days :-(.  But in that case the rest of the list
> didn't see it in real-time either, so having it show up out of
> "logical" sequence in the archive doesn't seem like a terrible
> reflection of reality.  I'm just concerned about the threading not
> being sensitive to skews on the order of a few minutes --- those
> are extremely common.

I not infrequently receive messages out of sequence by time periods
well in excess of a few minutes.

Don't know why, but I do.

...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] mailing list archiver chewing patches

2010-02-01 Thread Robert Haas
On Mon, Feb 1, 2010 at 11:28 AM, Tom Lane  wrote:
> Matteo Beccati  writes:
>> My main concern is that we'd need to overcomplicate the thread detection
>> algorithm so that it better deals with delayed messages: as it currently
>> works, the replies to a missing message get linked to the
>> "grand-parent". Injecting the missing message afterwards will put it at
>> the same level as its replies. If it happens only once in a while I
>> guess we can live with it, but definitely not if it happens tens of
>> times a day.
>
> That's quite common unfortunately --- I think you're going to need to
> deal with the case.  Even getting a direct feed from the mail relays
> wouldn't avoid it completely: consider cases like
>
>        * A sends a message
>        * B replies, cc'ing A and the list
>        * B's reply to list is delayed by greylisting
>        * A replies to B's reply (cc'ing list)
>        * A's reply goes through immediately
>        * B's reply shows up a bit later
>
> That happens pretty frequently IME.

Yeah - and sometimes the delay can be DAYS.

...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] mailing list archiver chewing patches

2010-02-01 Thread Tom Lane
Matteo Beccati  writes:
> My main concern is that we'd need to overcomplicate the thread detection 
> algorithm so that it better deals with delayed messages: as it currently 
> works, the replies to a missing message get linked to the 
> "grand-parent". Injecting the missing message afterwards will put it at 
> the same level as its replies. If it happens only once in a while I 
> guess we can live with it, but definitely not if it happens tens of 
> times a day.

That's quite common unfortunately --- I think you're going to need to
deal with the case.  Even getting a direct feed from the mail relays
wouldn't avoid it completely: consider cases like

* A sends a message
* B replies, cc'ing A and the list
* B's reply to list is delayed by greylisting
* A replies to B's reply (cc'ing list)
* A's reply goes through immediately
* B's reply shows up a bit later

That happens pretty frequently IME.

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] Re: [COMMITTERS] pgsql: Define INADDR_NONE on Solaris when it's missing.

2010-02-01 Thread Tom Lane
Magnus Hagander  writes:
> Here's what I came up with. Works well on the platforms I've tried,
> but I haven't tried on a non-ipv6 capable one yet (need to find one..)

Hmm, well, I have an ipv6-ignorant HPUX box at hand.  I do not have a
radius server though.  Are you only concerned about whether it compiles,
or do you want actual testing?

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] Allow parentheses around the query expression that follows a WITH clause

2010-02-01 Thread Tom Lane
"IP"  writes:
> I've got a question related to new feature in 8.5alpha, i.e. "Allow 
> parentheses around the query expression that follows a WITH clause". What 
> does that mean in fact? I can't see any difference between 8.4.2 and 
> 8.5alpha3 on Solaris x86.

That entry should not be in the alpha release notes; it's a bug fix that
was in fact also applied in 8.4.x.  The people making up the alpha notes
have not been very careful about removing entries that were also
back-patched.

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] Re: [COMMITTERS] pgsql: Write a WAL record whenever we perform an operation without

2010-02-01 Thread Tom Lane
Fujii Masao  writes:
> On Mon, Feb 1, 2010 at 7:27 PM, Heikki Linnakangas
>  wrote:
>> Fujii Masao wrote:
>>> The cause of the problem seems to be the new heap created by
>>> rebuild_relation() and copy_heap_data(), i.e., new VACUUM FULL.
>>> Since it's not a temporary heap, its rd_istemp is off. OTOH
>>> it needs to be synced after physical copy from old heap.
>> 
>> Why does it need to be synced?
>> 
>> ISTM the bug is that rd_istemp is off at that point.

> Umm... ISTM that new heap needs to be synced before calling
> swap_relation_files(), but, in the now, I'm not sure whether
> it's really required or not. Sorry.

If the original table is temp, then none of this work needs to be
fsync'd.  Ever.  So ISTM that CLUSTER ought to be copying the istemp
bit.  Another reason to do that is to make sure that the new instance
of the table goes into the temp tablespace and not the regular one.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Hot Standby and deadlock detection

2010-02-01 Thread Simon Riggs
On Mon, 2010-02-01 at 17:50 +0200, Heikki Linnakangas wrote:
> Simon Riggs wrote:
> > On Mon, 2010-02-01 at 09:40 +0200, Heikki Linnakangas wrote:
> >> Simon Riggs wrote:
> >>> The way this would work is if Startup waits on a buffer pin we
> >>> immediately send out a request to all backends to cancel themselves if
> >>> they are holding the buffer pin required && waiting on a lock. We then
> >>> sleep until max_standby_delay. When max_standby_delay = -1 we only sleep
> >>> until deadlock timeout and then check (on the Startup process).
> >> Should wake up to check for deadlocks after deadlock_timeout also when
> >> max_standby_delay > deadlock_timeout. max_standby_delay could be hours -
> >> we want to detect a deadlock sooner than that.
> > 
> > The patch does detect deadlocks sooner that that - "immediately", as
> > described above.
> 
> Umm, so why not run the deadlock check immediately in
> max_standby_delay=-1 case as well? Why is that case handled differently
> from max_standby_delay>0 case?

Cos the code to do that is easy.

I'll do the deadlock check immediately and make it even easier.

-- 
 Simon Riggs   www.2ndQuadrant.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Hot Standby and VACUUM FULL

2010-02-01 Thread Simon Riggs
On Mon, 2010-02-01 at 10:27 -0500, Tom Lane wrote:
> Simon Riggs  writes:
> > On Mon, 2010-02-01 at 10:06 -0500, Tom Lane wrote:
> >> the assumption that the file is less than one disk block,
> >> it should be just as atomic as pg_control updates are.
> 
> > IIRC there were 173 relations affected by this. 4 bytes each we would
> > have more than 512 bytes.
> 
> Where in the world did you get that number?
> 
> There are currently 29 shared relations (counting indexes), and 13
> nailed local relations, which would go into a different map file.
> I'm not sure if the set of local catalogs requiring the map treatment
> is exactly the same as what's presently nailed, but that's probably
> a good approximation.

I was suggesting that we only do shared and nailed relations. Sounds
like you agree.

-- 
 Simon Riggs   www.2ndQuadrant.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] contrib\xml2 package's xpath_table function in PostgreSQL

2010-02-01 Thread Robert Haas
On Sun, Jan 31, 2010 at 4:00 PM, HX Zheng  wrote:
> We have a huge system based on PostgreSQL with xml2 functions. From the
> PostgreSQL 8.4 documentation F.38.1. Deprecation notice, it looks like those
> functions are removed. However, our solution is very huge, and heavily
> depends on them.

The functions haven't actually been removed in 8.4, in spite of the
deprecation notice.  But it's very easy to use them in a way that
crashes the entire server, so you're playing with fire.  :-(

...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] Hot Standby and deadlock detection

2010-02-01 Thread Heikki Linnakangas
Simon Riggs wrote:
> On Mon, 2010-02-01 at 09:40 +0200, Heikki Linnakangas wrote:
>> Simon Riggs wrote:
>>> The way this would work is if Startup waits on a buffer pin we
>>> immediately send out a request to all backends to cancel themselves if
>>> they are holding the buffer pin required && waiting on a lock. We then
>>> sleep until max_standby_delay. When max_standby_delay = -1 we only sleep
>>> until deadlock timeout and then check (on the Startup process).
>> Should wake up to check for deadlocks after deadlock_timeout also when
>> max_standby_delay > deadlock_timeout. max_standby_delay could be hours -
>> we want to detect a deadlock sooner than that.
> 
> The patch does detect deadlocks sooner that that - "immediately", as
> described above.

Umm, so why not run the deadlock check immediately in
max_standby_delay=-1 case as well? Why is that case handled differently
from max_standby_delay>0 case?

-- 
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Package namespace and Safe init cleanup for plperl UPDATE 3 [PATCH]

2010-02-01 Thread Robert Haas
On Mon, Feb 1, 2010 at 5:58 AM, Tim Bunce  wrote:
> p.s. If there is interest in defining a documented API (for DBAs to
> control what gets loaded into Safe and shared with it) for *9.0*
> then that could be worked on, once this pach is in, ready for the
> next commitfest.

This is the last CommitFest for 9.0.  It's time to wind down
development on this release and work on trying to get the release
stabilized and out the door.

This isn't intended as a disparagement of the work you're doing; I've
thought about using PL/perl in the past and decided against it exactly
because of some of the issues you're now fixing.  But we're really out
of time to get things done for 9.0.

...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] Hot Standby and VACUUM FULL

2010-02-01 Thread Tom Lane
Heikki Linnakangas  writes:
> Tom Lane wrote:
>> The other problem with what you sketch is that it'd require holding the
>> mapfile write lock across commit, because we still have to have strict
>> serialization of updates.

> Why is the strict serialization of updates needed? To avoid overwriting
> the file with stale contents in a race condition?

Exactly.

> I was thinking that we only store the modified part in the WAL record.
> Right after writing commit record, take the lock, read() the map file,
> modify it in memory, write() it back, and release lock.

> That means that there's no full images of the file in WAL records, which
> makes me slightly uncomfortable from a disaster recovery point-of-view,

Yeah, me too, which is another advantage of using a separate WAL entry.

> That doesn't solve the problem I was trying to solve, which is that if
> the map file is rewritten, but the transaction later aborts, the map
> file update has hit the disk already. That's why I wanted to stash it
> into the commit record.

The design I sketched doesn't require such an assumption anyway.  Once
the map file is written, the relocation is effective, commit or no.
As long as we restrict relocations to maintenance operations such as
VACUUM FULL, which have no transactionally significant results, this
doesn't seem like a problem.  What we do need is that after a CLUSTER
or V.F., which is going to relocate not only the rel but its indexes,
the relocations of the rel and its indexes have to all "commit"
atomically.  But saving up the transaction's map changes and applying
them in one write takes care of that.

(What I believe this means is that pg_class and its indexes have to all
be mapped, but I'm thinking right now that no other non-shared catalogs
need the treatment.)

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] Review: listagg aggregate

2010-02-01 Thread Robert Haas
On Sun, Jan 31, 2010 at 10:29 PM, Takahiro Itagaki
 wrote:
> Applied with some editorialization. Please check the docs are good enough.

Looks pretty good.  I have committed a couple very minor adjustments.

...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] Hot Standby and VACUUM FULL

2010-02-01 Thread Heikki Linnakangas
Tom Lane wrote:
> That seems too fragile to me, as I don't find it a stretch at all to
> think that writing the map file might fail --- just think Windows
> antivirus code :-(.  Now, once we have written the WAL record for
> the mapfile change, we can't really afford a failure in my approach
> either.  But I think a rename() after successfully creating/writing/
> fsync'ing a temp file is a whole lot safer than writing from a standing
> start.

My gut feeling is exactly opposite. Creating and renaming a file
involves operations (and permissions) on the directory, while
overwriting a small file is just a simple write(). Especially if you
open() the file before doing anything irreversible.

> The other problem with what you sketch is that it'd require holding the
> mapfile write lock across commit, because we still have to have strict
> serialization of updates.

Why is the strict serialization of updates needed? To avoid overwriting
the file with stale contents in a race condition?

I was thinking that we only store the modified part in the WAL record.
Right after writing commit record, take the lock, read() the map file,
modify it in memory, write() it back, and release lock.

That means that there's no full images of the file in WAL records, which
makes me slightly uncomfortable from a disaster recovery point-of-view,
but we could keep a backup copy of the file in the data directory or
something if that's too scary otherwise.

> Maybe we should forget the
> rename() trick and overwrite the map file in place.  I still think it
> needs to be a separate WAL record though.  I'm thinking
> 
>   * obtain lock
>   * open file for read/write
>   * read current contents
>   * construct modified contents
>   * write and sync WAL record
>   * write back file through already-opened descriptor
>   * fsync
>   * release lock
> 
> Not totally clear if this is more or less safe than the rename method;
> but given the assumption that the file is less than one disk block,
> it should be just as atomic as pg_control updates are.

That doesn't solve the problem I was trying to solve, which is that if
the map file is rewritten, but the transaction later aborts, the map
file update has hit the disk already. That's why I wanted to stash it
into the commit record.

-- 
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Hot Standby and VACUUM FULL

2010-02-01 Thread Tom Lane
Simon Riggs  writes:
> On Mon, 2010-02-01 at 10:06 -0500, Tom Lane wrote:
>> the assumption that the file is less than one disk block,
>> it should be just as atomic as pg_control updates are.

> IIRC there were 173 relations affected by this. 4 bytes each we would
> have more than 512 bytes.

Where in the world did you get that number?

There are currently 29 shared relations (counting indexes), and 13
nailed local relations, which would go into a different map file.
I'm not sure if the set of local catalogs requiring the map treatment
is exactly the same as what's presently nailed, but that's probably
a good approximation.

At 8 bytes each (OID + relfilenode), we could fit 64 map entries in
a standard disk sector --- let's say 63 so there's room for a header.
So, barring more-than-doubling of the number of shared catalogs,
there's not going to be a problem.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Hot Standby and VACUUM FULL

2010-02-01 Thread Simon Riggs
On Mon, 2010-02-01 at 10:06 -0500, Tom Lane wrote:

> the assumption that the file is less than one disk block,
> it should be just as atomic as pg_control updates are.

IIRC there were 173 relations affected by this. 4 bytes each we would
have more than 512 bytes.

ISTM you need to treat some of those system relations just as normal
relations rather than give everybody a map entry.

-- 
 Simon Riggs   www.2ndQuadrant.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Deadlock in vacuum (check fails)

2010-02-01 Thread Tom Lane
Greg Stark  writes:
> Does this create a problem combined with the plan to allow the new
> VACUUM FULL to rewrite system tables? From my brief scan it sounds
> like the only reason there's no race condition here is that previously
> the oid of system tables couldn't change out from under
> load_critical_index.

Their OIDs will never change, so I don't see an issue.  Locks are taken
on OIDs not relfilenodes, so a relocation doesn't break locking.  If
it did we'd already have a problem with relocating user tables.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Hot Standby and VACUUM FULL

2010-02-01 Thread Tom Lane
Greg Stark  writes:
> On Mon, Feb 1, 2010 at 8:54 AM, Simon Riggs  wrote:
>>> Disallow catalog relocation inside subtransactions, to avoid having
>>> to handle subxact abort effects on the local-map-changes state.
>>> This could be implemented if desired, but doesn't seem worth it
>>> at least in first pass.
>> 
>> Agreed, not needed for emergency maintenance actions like this.

> Note that this would mean it will never work if you have psql's
> ROLLBACK_ON_ERROR set.

VACUUM has always failed in such a case, so I don't see this as a
showstopper.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Hot Standby and VACUUM FULL

2010-02-01 Thread Tom Lane
Heikki Linnakangas  writes:
> Tom Lane wrote:
>> Once the updated map file is moved into place, the relocation is effectively
>> committed even if we subsequently abort the transaction.  We can make that
>> window pretty narrow but not remove it completely.

> We could include the instructions to update the map file in the commit
> record, instead of introducing a new record type, and update the map
> file only *after* writing the commit record. The map file doesn't grow,
> so we can be pretty confident that updating it doesn't fail (failure
> would lead to PANIC).

> I'm assuming the map file is fixed size, with a fixed location for each
> relation, so that we can just overwrite the old file without the
> create+rename dance, and not worry about torn-pages.

That seems too fragile to me, as I don't find it a stretch at all to
think that writing the map file might fail --- just think Windows
antivirus code :-(.  Now, once we have written the WAL record for
the mapfile change, we can't really afford a failure in my approach
either.  But I think a rename() after successfully creating/writing/
fsync'ing a temp file is a whole lot safer than writing from a standing
start.

The other problem with what you sketch is that it'd require holding the
mapfile write lock across commit, because we still have to have strict
serialization of updates.

[ thinks for awhile ... ]  OTOH, overwrite-in-place is what we've always
used for pg_control updates, and I don't recall ever seeing a report of
a problem that could be traced to that.  Maybe we should forget the
rename() trick and overwrite the map file in place.  I still think it
needs to be a separate WAL record though.  I'm thinking

* obtain lock
* open file for read/write
* read current contents
* construct modified contents
* write and sync WAL record
* write back file through already-opened descriptor
* fsync
* release lock

Not totally clear if this is more or less safe than the rename method;
but given the assumption that the file is less than one disk block,
it should be just as atomic as pg_control updates are.

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] Deadlock in vacuum (check fails)

2010-02-01 Thread Greg Stark
On Wed, Jan 13, 2010 at 7:27 PM, Tom Lane  wrote:
> So basically what this boils down to is that load_critical_index is
> locking an index before locking its underlying relation, which generally
> speaking is against our coding rules.  The relcache code has been like
> that since 8.2, so I'm a bit surprised that we have not seen this
> reported before.  It could only happen when the relcache init file is
> missing, which isn't the normal state, but it's not exactly unusual
> either.
>
> Probably the appropriate fix is to make load_critical_index get
> AccessShare lock on the underlying catalog not only the index it's
> after.  This would slow things down a tad, but since it's not in the
> normal startup path I don't think that matters.
>
> Should we back-patch this?  The bug appears to be real back to 8.2,
> but if we've not noticed before, I'm not sure how important it is.

Does this create a problem combined with the plan to allow the new
VACUUM FULL to rewrite system tables? From my brief scan it sounds
like the only reason there's no race condition here is that previously
the oid of system tables couldn't change out from under
load_critical_index. If instead it's possible for them to would it be
possible for it to try to load this index, find the oid of the
underlying table, then go to lock the underlying table only to find it
had been vacuumed out from under it and no longer exists?

-- 
greg

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] mailing list archiver chewing patches

2010-02-01 Thread Magnus Hagander
2010/2/1 Matteo Beccati :
> On 01/02/2010 15:03, Magnus Hagander wrote:
>>
>> 2010/2/1 Matteo Beccati:
>>>
>>> My main concern is that we'd need to overcomplicate the thread detection 
>>> algorithm so that it better deals with delayed messages: as it currently 
>>> works, the replies to a missing message get linked to the "grand-parent". 
>>> Injecting the missing message afterwards will put it at the same level as 
>>> its replies. If it happens only once in a while I guess we can live with 
>>> it, but definitely not if it happens tens of times a day.
>>
>> That can potentially be a problem.
>>
>> Consider the case where message A it sent. Mesasge B is a response to
>> A, and message C is a response to B. Now assume B is held for
>> moderation (because the poser is not on the list, or because it trips
>> some other thing), then message C will definitely arrive before
>> message B. Is that going to cause problems with this method?
>>
>> Another case where the same thing will happen is if message delivery
>> of B gets for example graylisted, or is just slow from sender B, but
>> gets quickly delivered to the author of message A (because of a direct
>> CC). In this case, the author of message A may respond to it (making
>> message D),and this will again arrive before message B because author
>> A is not graylisted.
>>
>> So the system definitely needs to deal with out-of-order delivery.
>
> Hmm, it looks like I didn't factor in direct CCs when thinking about 
> potential problems with the simplified algorithm. Thanks for raising that.

That is a very common scenario. And even without that, email taking
different time to get delivered to majordomo is not at all uncomoon.


> I'll be out of town for a few days, but I will see what I can do when I get 
> back.

No rush.

-- 
 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] mailing list archiver chewing patches

2010-02-01 Thread Matteo Beccati

On 01/02/2010 15:03, Magnus Hagander wrote:

2010/2/1 Matteo Beccati:

My main concern is that we'd need to overcomplicate the thread detection algorithm so 
that it better deals with delayed messages: as it currently works, the replies to a 
missing message get linked to the "grand-parent". Injecting the missing message 
afterwards will put it at the same level as its replies. If it happens only once in a 
while I guess we can live with it, but definitely not if it happens tens of times a day.


That can potentially be a problem.

Consider the case where message A it sent. Mesasge B is a response to
A, and message C is a response to B. Now assume B is held for
moderation (because the poser is not on the list, or because it trips
some other thing), then message C will definitely arrive before
message B. Is that going to cause problems with this method?

Another case where the same thing will happen is if message delivery
of B gets for example graylisted, or is just slow from sender B, but
gets quickly delivered to the author of message A (because of a direct
CC). In this case, the author of message A may respond to it (making
message D),and this will again arrive before message B because author
A is not graylisted.

So the system definitely needs to deal with out-of-order delivery.


Hmm, it looks like I didn't factor in direct CCs when thinking about 
potential problems with the simplified algorithm. Thanks for raising that.


I'll be out of town for a few days, but I will see what I can do when I 
get back.



Cheers
--
Matteo Beccati

Development & Consulting - http://www.beccati.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] mailing list archiver chewing patches

2010-02-01 Thread Magnus Hagander
2010/2/1 Matteo Beccati :
> On 01/02/2010 10:26, Magnus Hagander wrote:
>>
>> Does the MBOX importer support incremental loading? Because majordomo
>> spits out MBOX files for us already.
>
> Unfortunately the aoximport shell command doesn't support incremental loading.
>
>> One option could be to use SMTP with a subscription as the primary way
>> (and we could set up a dedicated relaying from the mailserver for this
>> of course, so it's not subject to graylisting or anything like that),
>> and then daily or so load the MBOX files to cover anything that was
>> lost?
>
> I guess we could write a script that parses the mbox and adds whatever is 
> missing, as long as we keep it as a last resort if we can't make the primary 
> delivery a fail proof.
>
> My main concern is that we'd need to overcomplicate the thread detection 
> algorithm so that it better deals with delayed messages: as it currently 
> works, the replies to a missing message get linked to the "grand-parent". 
> Injecting the missing message afterwards will put it at the same level as its 
> replies. If it happens only once in a while I guess we can live with it, but 
> definitely not if it happens tens of times a day.

That can potentially be a problem.

Consider the case where message A it sent. Mesasge B is a response to
A, and message C is a response to B. Now assume B is held for
moderation (because the poser is not on the list, or because it trips
some other thing), then message C will definitely arrive before
message B. Is that going to cause problems with this method?

Another case where the same thing will happen is if message delivery
of B gets for example graylisted, or is just slow from sender B, but
gets quickly delivered to the author of message A (because of a direct
CC). In this case, the author of message A may respond to it (making
message D),and this will again arrive before message B because author
A is not graylisted.

So the system definitely needs to deal with out-of-order delivery.

-- 
 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] Review: listagg aggregate

2010-02-01 Thread Thom Brown
On 1 February 2010 13:40, Pavel Stehule  wrote:

> 2010/2/1 Thom Brown :
> > I noticed that the regression test results don't include the following
> case:
> >
> > select string_agg(a) from (values(null),(null)) g(a);
> >
> > There is one similar where a delimiter is provided.
> >
> > Which leads me to ask for clarification, would it be safe to assume that
> > string_agg can never itself return null?
>
> if all values are null, then result is null.
>
> Pavel
>
>
>
Ah, I was looking at the expected results, and couldn't see a NULL outcome,
but then these aren't indicated in such results anyway.

Thom


Re: [HACKERS] Review: listagg aggregate

2010-02-01 Thread Pavel Stehule
2010/2/1 Thom Brown :
> I noticed that the regression test results don't include the following case:
>
> select string_agg(a) from (values(null),(null)) g(a);
>
> There is one similar where a delimiter is provided.
>
> Which leads me to ask for clarification, would it be safe to assume that
> string_agg can never itself return null?

if all values are null, then result is null.

Pavel

>
> Thom
>
>

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] mailing list archiver chewing patches

2010-02-01 Thread Matteo Beccati

On 01/02/2010 10:26, Magnus Hagander wrote:

Does the MBOX importer support incremental loading? Because majordomo
spits out MBOX files for us already.


Unfortunately the aoximport shell command doesn't support incremental 
loading.



One option could be to use SMTP with a subscription as the primary way
(and we could set up a dedicated relaying from the mailserver for this
of course, so it's not subject to graylisting or anything like that),
and then daily or so load the MBOX files to cover anything that was
lost?


I guess we could write a script that parses the mbox and adds whatever 
is missing, as long as we keep it as a last resort if we can't make the 
primary delivery a fail proof.


My main concern is that we'd need to overcomplicate the thread detection 
algorithm so that it better deals with delayed messages: as it currently 
works, the replies to a missing message get linked to the 
"grand-parent". Injecting the missing message afterwards will put it at 
the same level as its replies. If it happens only once in a while I 
guess we can live with it, but definitely not if it happens tens of 
times a day.



Cheers
--
Matteo Beccati

Development & Consulting - http://www.beccati.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] Re: [COMMITTERS] pgsql: Define INADDR_NONE on Solaris when it's missing.

2010-02-01 Thread Magnus Hagander
2010/1/28 Magnus Hagander :
> On Thu, Jan 28, 2010 at 21:16, Tom Lane  wrote:
>> Magnus Hagander  writes:
>>> On Thu, Jan 28, 2010 at 17:16, Tom Lane  wrote:
 However, now that I know the real issue is you're using inet_addr, I
 would like to know why you're not using inet_aton instead; or even
 better, something that also copes with IPv6.
>>
>>> "Path of least resistance?"
>>
>>> Which method would you suggest?
>>
>> I haven't actually read the RADIUS patch, but generally we rely on
>> pg_getaddrinfo_all to interpret strings representing IP addresses.
>> Is there a reason not to use that?
>
> I don't think so. I'll look it over.

Here's what I came up with. Works well on the platforms I've tried,
but I haven't tried on a non-ipv6 capable one yet (need to find one..)
I'll also remove the defines from solaris.h when applying it.


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


radius_addr.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] odd output in initdb

2010-02-01 Thread Andrew Dunstan



Magnus Hagander wrote:

Did those two members produce consistent issues? Can you give them a
couple of kicks to get enough buidls out of them to figure out if this
solved the problem?

  


Not very easily. I am currently travelling. It was happening fairly 
consistently on red_bat and seems to have stopped now.


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


[HACKERS] Allow parentheses around the query expression that follows a WITH clause

2010-02-01 Thread IP
I've got a question related to new feature in 8.5alpha, i.e. "Allow parentheses 
around the query expression that follows a WITH clause". What does that mean in 
fact? I can't see any difference between 8.4.2 and 8.5alpha3 on Solaris x86.

  1   2   >