Re: [HACKERS] Initial review of xslt with no limits patch

[David E. Wheeler]

On Aug 6, 2010, at 10:49 PM, Pavel Stehule wrote:

>> Huh? You can select into an array:
> 
> and pg doesn't handle 2D arrays well - can't to use ARRAY(subselect)
> constructor for 2D arrays

Right.

>> try=# select ARRAY(SELECT ARRAY[k,v] FROM foo);
>> ERROR:  could not find array type for datatype text[]
> 
> try SELECT ARRAY(SELECT row(k,v) FROM foo)

Yeah, but those aren't nested arrays., They're…well, they're ordered pairs. ;-P

> sure, but it isn't relevant here - the problem is buildin output
> functions for datatypes. For example - true is different formated in
> PostgresSQL and different formated in xml or JSON. Date values are
> differently formated in JSON and XML. So if you would to correctly
> format some date type value and if your interface is only text - then
> you have to cast value back to binary and format it again. More - if
> you have a information about original data type, you can use a corect
> format. So if you use a only text parameters, then you lost a
> significant information (when some parameter are not text). For
> example, if I have only text interface for some hypothetical JSON API,
> then I am not able to show a boolean value correctly - because it
> doesn't use a quoting - and it is string and isn't number.

Point. FWIW, though, this is already an issue for non-SQL functions. PL/Perl, 
for example, gets all arguments cast to text, AFAICT:

try=# create or replace function try(bool) returns text language plperl AS 
'shift';
CREATE FUNCTION
Time: 121.403 ms
try=# select try(true);
 try 
-
 t
(1 row)

I wish this wasn't so.

> There is some other issue - PLpgSQL can't to work well with untyped
> collections. But it isn't problem for C custom functions, and there
> are not any reason why we can't to support polymorphic collections
> (+/- because these collection cannot be accessed from PLpgSQL
> directly).

I completely agree with you here. I'd love to be able to support RECORD 
arguments to non-C functions.

>> I agree that it's not as sugary as pairs would be. But I admit to having no 
>> problem with
>> 
>>  SELECT foo(ARRAY[ ['foo', 'bar'], ['baz', 'yow']]);
>> 
>> But maybe I'm biased, since there's a lot of that sort of syntax in pgTAP..
>> 
> 
> Yes, when you are a author of code, you know what you are wrote. But
> when you have do some review? Then an reviewer have to look on
> definition of foo, and he has to verify, if you are use a parameters
> well. For two params I don't see on first view what system you used -
> [[key,key],[value,value]] or [[key,value],[key, value]]. More you have
> to use a nested data structure - what is less readable then variadic
> parameters. And - in pg - you are lost information about original data
> types.

Valid points. I agree that it would be nicer to use RECORDs:

SELECT foo( row('foo', 1), row('bar', true));

Certainly much clearer. But given that we've gone round and round on allowing 
non-C functions to use ROWs and gotten nowhere, I don't know that we'll get any 
further now. But can you not create a C function that allows a signature of 
VARIADIC RECORD?

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] Initial review of xslt with no limits patch

[Pavel Stehule]

2010/8/7 David E. Wheeler :
> On Aug 6, 2010, at 10:15 PM, Pavel Stehule wrote:
>
>>> This is not exactly without precedent, either: our built-in xpath()
>>> function appears to use precisely this approach for its namespace-list
>>> argument.
>>
>> it's one variant, but isn't perfect
>>
>> a) it expects so key and value are literals
>
> Huh? You can select into an array:

and pg doesn't handle 2D arrays well - can't to use ARRAY(subselect)
constructor for 2D arrays

>
> try=# create table foo(k text, v text);
> CREATE TABLE
> try=# insert into foo values ('foo', 'bar'), ('baz', 'yow');
> INSERT 0 2
> try=# select ARRAY[k,v] FROM foo;
>   array
> ---
>  {foo,bar}
>  {baz,yow}
> (2 rows)
>
> try=# select ARRAY(SELECT ARRAY[k,v] FROM foo);
> ERROR:  could not find array type for datatype text[]

try SELECT ARRAY(SELECT row(k,v) FROM foo)

>
> Okay, so nested arrays are harder.
>
>> b) it expects so all values has same types - what is usually what we
>> need, but not necessary
>
> The XML interface converts them all to text anyway, no?
>

sure, but it isn't relevant here - the problem is buildin output
functions for datatypes. For example - true is different formated in
PostgresSQL and different formated in xml or JSON. Date values are
differently formated in JSON and XML. So if you would to correctly
format some date type value and if your interface is only text - then
you have to cast value back to binary and format it again. More - if
you have a information about original data type, you can use a corect
format. So if you use a only text parameters, then you lost a
significant information (when some parameter are not text). For
example, if I have only text interface for some hypothetical JSON API,
then I am not able to show a boolean value correctly - because it
doesn't use a quoting - and it is string and isn't number.

There is some other issue - PLpgSQL can't to work well with untyped
collections. But it isn't problem for C custom functions, and there
are not any reason why we can't to support polymorphic collections
(+/- because these collection cannot be accessed from PLpgSQL
directly).


>> c) isn't too readable - I am sorry so I am repeating - it is same
>> reason, why people will prefer a VARIADIC function before function
>> with array - but I can accept, so this is my view of world
>
> I agree that it's not as sugary as pairs would be. But I admit to having no 
> problem with
>
>  SELECT foo(ARRAY[ ['foo', 'bar'], ['baz', 'yow']]);
>
> But maybe I'm biased, since there's a lot of that sort of syntax in pgTAP.
>

Yes, when you are a author of code, you know what you are wrote. But
when you have do some review? Then an reviewer have to look on
definition of foo, and he has to verify, if you are use a parameters
well. For two params I don't see on first view what system you used -
[[key,key],[value,value]] or [[key,value],[key, value]]. More you have
to use a nested data structure - what is less readable then variadic
parameters. And - in pg - you are lost information about original data
types.

Regards

Pavel

> 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] Initial review of xslt with no limits patch

[Pavel Stehule]

2010/8/7 David E. Wheeler :
> On Aug 6, 2010, at 9:48 PM, Pavel Stehule wrote:
>
>>> That's exactly what my solution does. The array solution doesn't. Whether 
>>> it's appropriate to use a custom composite type, however, is an open 
>>> question.
>>
>> no it doesn't - in your design there are no different notation for key
>> and for value. Next this design block  a '->'. Because it's based on
>> polymorphic operator. But it can be a one variant - where you would to
>> put together expr with expr. And you can't do more from user space
>> now. But if you have a build in operator for (sqlidentifier, any) with
>> early processing - like "AS" in xml_attributies, we can do it. The
>> using of this operator can be limited only on function parameter
>> context.
>
> I'm sorry, I still don't follow. It creates an ordered pair, with one value 
> being named "key" and the other one "val". And when you use the ~> operator, 
> the lhs is the key and the rhs if the value.
>
>> Postgres has a array of rows (Inside C or plperlu can be transofmed to
>> real hash simply). It just miss a user friendly notation for using it.
>
> I think Tom's right, frankly: An array of arrays will be the best solution 
> for your interface. Sure someone could include more than two items in each 
> nested array, or fewer than 2, but if there are more you ignore them, and if 
> there are fewer you treat them as NULLs.

it can be a just plain text - it isn't about functionality, it is
about readability, verbosity and stored procedures developer's comfort
- and some consistency.

Pavel

>
> 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] Initial review of xslt with no limits patch

[David E. Wheeler]

On Aug 6, 2010, at 10:15 PM, Pavel Stehule wrote:

>> This is not exactly without precedent, either: our built-in xpath()
>> function appears to use precisely this approach for its namespace-list
>> argument.
> 
> it's one variant, but isn't perfect
> 
> a) it expects so key and value are literals

Huh? You can select into an array:

try=# create table foo(k text, v text);
CREATE TABLE
try=# insert into foo values ('foo', 'bar'), ('baz', 'yow');
INSERT 0 2
try=# select ARRAY[k,v] FROM foo;
   array   
---
 {foo,bar}
 {baz,yow}
(2 rows)

try=# select ARRAY(SELECT ARRAY[k,v] FROM foo);
ERROR:  could not find array type for datatype text[]

Okay, so nested arrays are harder.

> b) it expects so all values has same types - what is usually what we
> need, but not necessary

The XML interface converts them all to text anyway, no?

> c) isn't too readable - I am sorry so I am repeating - it is same
> reason, why people will prefer a VARIADIC function before function
> with array - but I can accept, so this is my view of world

I agree that it's not as sugary as pairs would be. But I admit to having no 
problem with

  SELECT foo(ARRAY[ ['foo', 'bar'], ['baz', 'yow']]);

But maybe I'm biased, since there's a lot of that sort of syntax in pgTAP.

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] Initial review of xslt with no limits patch

[David E. Wheeler]

On Aug 6, 2010, at 9:59 PM, Tom Lane wrote:

> It's not immediately clear to me what an ordered-pair type would get you
> that you don't get with 2-element arrays.

Just syntactic sugar, really. And control over how many items you have (a 
bounded pair rather than an unlimited element array).

> A couple of quick experiments suggest that 2-D arrays might be the thing
> to use.  They're easy to construct:
> 
> regression=# select array[[1,2],[3,4]];
> array 
> ---
> {{1,2},{3,4}}
> (1 row)
> 
> and you can build them dynamically at need:
> 
> regression=# select array[[1,2],[3,4]] || array[5,6];
>  ?column?   
> -
> {{1,2},{3,4},{5,6}}
> (1 row)
> 
> This is not exactly without precedent, either: our built-in xpath()
> function appears to use precisely this approach for its namespace-list
> argument.

Agreed.

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] Initial review of xslt with no limits patch

[Pavel Stehule]

2010/8/7 Tom Lane :
> "David E. Wheeler"  writes:
>> I think that some sort of variadic pairs would be useful for this. But since 
>> there is no core "ordered pair" data type, I don't think you're going to get 
>> too far.
>
> It's not immediately clear to me what an ordered-pair type would get you
> that you don't get with 2-element arrays.
>
> A couple of quick experiments suggest that 2-D arrays might be the thing
> to use.  They're easy to construct:
>
> regression=# select array[[1,2],[3,4]];
>     array
> ---
>  {{1,2},{3,4}}
> (1 row)
>
> and you can build them dynamically at need:
>
> regression=# select array[[1,2],[3,4]] || array[5,6];
>      ?column?
> -
>  {{1,2},{3,4},{5,6}}
> (1 row)
>
> This is not exactly without precedent, either: our built-in xpath()
> function appears to use precisely this approach for its namespace-list
> argument.

it's one variant, but isn't perfect

a) it expects so key and value are literals
b) it expects so all values has same types - what is usually what we
need, but not necessary
c) isn't too readable - I am sorry so I am repeating - it is same
reason, why people will prefer a VARIADIC function before function
with array - but I can accept, so this is my view of world

Regards

Pavel Stehule

>
>                        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] Initial review of xslt with no limits patch

[David E. Wheeler]

On Aug 6, 2010, at 9:48 PM, Pavel Stehule wrote:

>> That's exactly what my solution does. The array solution doesn't. Whether 
>> it's appropriate to use a custom composite type, however, is an open 
>> question.
> 
> no it doesn't - in your design there are no different notation for key
> and for value. Next this design block  a '->'. Because it's based on
> polymorphic operator. But it can be a one variant - where you would to
> put together expr with expr. And you can't do more from user space
> now. But if you have a build in operator for (sqlidentifier, any) with
> early processing - like "AS" in xml_attributies, we can do it. The
> using of this operator can be limited only on function parameter
> context.

I'm sorry, I still don't follow. It creates an ordered pair, with one value 
being named "key" and the other one "val". And when you use the ~> operator, 
the lhs is the key and the rhs if the value.

> Postgres has a array of rows (Inside C or plperlu can be transofmed to
> real hash simply). It just miss a user friendly notation for using it.

I think Tom's right, frankly: An array of arrays will be the best solution for 
your interface. Sure someone could include more than two items in each nested 
array, or fewer than 2, but if there are more you ignore them, and if there are 
fewer you treat them as NULLs.

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] Initial review of xslt with no limits patch

[Tom Lane]

"David E. Wheeler"  writes:
> I think that some sort of variadic pairs would be useful for this. But since 
> there is no core "ordered pair" data type, I don't think you're going to get 
> too far.

It's not immediately clear to me what an ordered-pair type would get you
that you don't get with 2-element arrays.

A couple of quick experiments suggest that 2-D arrays might be the thing
to use.  They're easy to construct:

regression=# select array[[1,2],[3,4]];
 array 
---
 {{1,2},{3,4}}
(1 row)

and you can build them dynamically at need:

regression=# select array[[1,2],[3,4]] || array[5,6];
  ?column?   
-
 {{1,2},{3,4},{5,6}}
(1 row)

This is not exactly without precedent, either: our built-in xpath()
function appears to use precisely this approach for its namespace-list
argument.

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] Initial review of xslt with no limits patch

[Pavel Stehule]

2010/8/7 David E. Wheeler :
> On Aug 6, 2010, at 8:49 PM, Pavel Stehule wrote:
>
>>> Sorry, not following you here
>>
>> I would to difference a key and value in notation.
>
> That's exactly what my solution does. The array solution doesn't. Whether 
> it's appropriate to use a custom composite type, however, is an open question.

no it doesn't - in your design there are no different notation for key
and for value. Next this design block  a '->'. Because it's based on
polymorphic operator. But it can be a one variant - where you would to
put together expr with expr. And you can't do more from user space
now. But if you have a build in operator for (sqlidentifier, any) with
early processing - like "AS" in xml_attributies, we can do it. The
using of this operator can be limited only on function parameter
context.

>
>>> Pavel doesn't understand "no" ;-)
>>
>> you are don't writing a stored procedures like me - so maybe you are
>> doesn't understand a my motivation. :). I have to try it. You are
>> rejected almost of all my proposals - named parameters, variadic
>> functions, enhancing of RAISE STATEMENT - and now its in core. But it
>> was a battle :).
>
> This is how most stuff gets in: you fight Tom to exhaustion. It's a slog, but 
> usually the resulting implementation is better than it would otherwise have 
> been.
>
>> Try to write a XML-RPC support for PostgreSQL, and
>> try to thinking on programmer comfort, please. I am sure so our
>> support for stored procedures or external procedures are not complete
>> - it is limited by BISON possibilities, and because BISON isn't
>> extensible parser, I am searching other ways. If I can enhance a
>> syntax from external module, I don't talk.
>
> I think that some sort of variadic pairs would be useful for this. But since 
> there is no core "ordered pair" data type, I don't think you're going to get 
> too far.

Postgres has a array of rows (Inside C or plperlu can be transofmed to
real hash simply). It just miss a user friendly notation for using it.

Regards

Pavel
>
> 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] Surprising dead_tuple_count from pgstattuple

[Gordon Shannon]

Robert Haas wrote:
> 
> My thought would be "is autovacuum running in the background in
> between these commands?".
> 

That's a good thought, but no, autovacuum_vacuum_scale_factor is set to 0.2,
meaning that over 1 million dead tuples are necessary for autovacuum. 
Besides, if autovacuum had run, I think the pg_stat_user_tables.n_dead_tup
would have reset to zero, as it did after my manual vacuum.

Regarding HOT prune, I never did any updates, so I think there couldn't be
any HOT tuples.  Or does HOT prune do more than that?
-- 
View this message in context: 
http://postgresql.1045698.n5.nabble.com/Surprising-dead-tuple-count-from-pgstattuple-tp2266955p2267263.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

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

Re: [HACKERS] Initial review of xslt with no limits patch

[David E. Wheeler]

On Aug 6, 2010, at 8:49 PM, Pavel Stehule wrote:

>> Sorry, not following you here
> 
> I would to difference a key and value in notation.

That's exactly what my solution does. The array solution doesn't. Whether it's 
appropriate to use a custom composite type, however, is an open question.

>> Pavel doesn't understand "no" ;-)
> 
> you are don't writing a stored procedures like me - so maybe you are
> doesn't understand a my motivation. :). I have to try it. You are
> rejected almost of all my proposals - named parameters, variadic
> functions, enhancing of RAISE STATEMENT - and now its in core. But it
> was a battle :).

This is how most stuff gets in: you fight Tom to exhaustion. It's a slog, but 
usually the resulting implementation is better than it would otherwise have 
been.

> Try to write a XML-RPC support for PostgreSQL, and
> try to thinking on programmer comfort, please. I am sure so our
> support for stored procedures or external procedures are not complete
> - it is limited by BISON possibilities, and because BISON isn't
> extensible parser, I am searching other ways. If I can enhance a
> syntax from external module, I don't talk.

I think that some sort of variadic pairs would be useful for this. But since 
there is no core "ordered pair" data type, I don't think you're going to get 
too far.

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] GROUPING SETS revisited

[Pavel Stehule]

2010/8/7 Joshua Tolley :
> On Thu, Aug 05, 2010 at 04:46:51PM +0200, Pavel Stehule wrote:
>> I am sending a updated version.
>
> I've been looking at the changes to gram.y, and noted the comment under 
> func_expr
> where you added CUBE and ROLLUP definitions. It says that CUBE can't be a
> reserved keyword because it's already used in the cube contrib module. But
> then the changes to kwlist.h include this:
>

I am little bit confused now - it's bad comment - and I have to verify
it. What I remember, we cannot to use a two parser's rules, because it
going to a conflict. So there have to be used a trick with a moving to
decision to transform stage, where we have a context info. I have to
recheck a minimal level - probably it can't be a RESERVED_KEYWORD.
Because then we can't to create a function "cube".

> + PG_KEYWORD("cube", CUBE, RESERVED_KEYWORD)
> ...
> + PG_KEYWORD("rollup", ROLLUP, RESERVED_KEYWORD)
>
> ...and CUBE and ROLLUP are added in gram.y under the reserved_keyword list. I
> realize things like CURRENT_TIME, that also have special entries in the
> func_expr grammar, are also reserved keywords, but this all seems at odds with
> the comment. What am I missing? Is the comment simply pointing out that the
> designation of CUBE and ROLLUP as reserved keywords will have to change at
> some point, but it hasn't been implemented yet (or no one has figured out how
> to do it)?
>
> --
> Joshua Tolley / eggyknap
> End Point Corporation
> http://www.endpoint.com
>
> -BEGIN PGP SIGNATURE-
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iEYEARECAAYFAkxcjSIACgkQRiRfCGf1UMPpCwCcCHBh/1NiLykIcVYgPyfbIegF
> xq0AoID75rCPiW8yf29OSkaJVza1FQt5
> =PcLs
> -END PGP SIGNATURE-
>
>

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

Re: [HACKERS] Initial review of xslt with no limits patch

[Pavel Stehule]

2010/8/7 Tom Lane :
> "David E. Wheeler"  writes:
>> On Aug 6, 2010, at 2:12 PM, Pavel Stehule wrote:
>>> so there is only small step to proposed feature
>>> SELECT foo(this := 'that', "1" := 4)
>
>> Sorry, not following you here

I would to difference a key and value in notation.

>
> Pavel doesn't understand "no" ;-)
>

you are don't writing a stored procedures like me - so maybe you are
doesn't understand a my motivation. :). I have to try it. You are
rejected almost of all my proposals - named parameters, variadic
functions, enhancing of RAISE STATEMENT - and now its in core. But it
was a battle :). Try to write a XML-RPC support for PostgreSQL, and
try to thinking on programmer comfort, please. I am sure so our
support for stored procedures or external procedures are not complete
- it is limited by BISON possibilities, and because BISON isn't
extensible parser, I am searching other ways. If I can enhance a
syntax from external module, I don't talk.

Regards

Pavel




Regards

Pavel

>                        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] refactoring comment.c

[KaiGai Kohei]

(2010/08/07 0:02), Robert Haas wrote:

At PGCon, we discussed the possibility that a minimal SE-PostgreSQL
implementation would need little more than a hook in
ExecCheckRTPerms() [which we've since added] and a security label
facility [for which KaiGai has submitted a patch].  I actually sat
down to write the security label patch myself while we were in Ottawa,
but quickly ran into difficulties: while the hook we have now can't do
anything useful with objects other than relations, it's pretty clear
from previous discussions on this topic that the demand for labels on
other kinds of objects is not going to go away.  Rather than adding
additional syntax to every object type in the system (some of which
don't even have ALTER commands at present), I suggested basing the
syntax on the existing COMMENT syntax.  After some discussion[1], we
seem to have settled on the following:

SECURITY LABEL [ FOR  ] ONIS '';

At present, there are some difficulties with generalizing this syntax
to other object types.  As I found out when I initially set out to
write this patch, it'd basically require duplicating all of comment.c,
which is an unpleasant prospect, because that file is big and crufty;
it has a large amount of internal duplication.  Furthermore, the
existing locking mechanism that we're using for comments is known to
be inadequate[2].  Dropping a comment while someone else is in the
midst of commenting on it leaves orphaned comments lying around in
pg_(sh)description that could later be inherited by a new object.
That's a minor nuisance for comments and would be nice to fix, but is
obviously a far larger problem for security labels, where even a small
chance of randomly mislabeling an object is no good.

So I wrote a patch.  The attached patch factors out all of the code in
comment.c that is responsible for translating parser representations
into a new file parser/parse_object.c, leaving just the
comment-specific stuff in commands/comment.c.  It also adds
appropriate locking, so that concurrent COMMENT/DROP scenarios don't
leave behind leftovers.  It's a fairly large patch, but the changes
are extremely localized: comment.c gets a lot smaller, and
parse_object.c gets bigger by a slightly smaller amount.

Any comments?  (ha ha ha...)

[1] http://archives.postgresql.org/pgsql-hackers/2010-07/msg01328.php
[2] http://archives.postgresql.org/pgsql-hackers/2010-07/msg00351.php



Thanks for your efforts.
I believe the get_object_address() enables to implement security
label features on various kind of database objects.

I tried to look at the patch. Most part is fine, but I found out
two issues.

On the object_exists(), when we verify existence of a large object,
it needs to scan pg_largeobject_metadata, instead of pg_largeobject.
When we implement pg_largeobject_metadata catalog, we decided to set
LargeObjectRelationId on object.classId due to the backend compatibility.

|/*
| * For object types that have a relevant syscache, we use it; for
| * everything else, we'll have to do an index-scan.  This switch
| * sets either the cache to be used for the syscache lookup, or the
| * index to be used for the index scan.
| */
|switch (address.classId)
|{
|case RelationRelationId:
|cache = RELOID;
|break;
|  :
|case LargeObjectRelationId:
|indexoid = LargeObjectMetadataOidIndexId;
|break;
|  :
| }
|
|/* Found a syscache? */
|if (cache != -1)
|return SearchSysCacheExists1(cache, 
ObjectIdGetDatum(address.objectId));
|
|/* No syscache, so examine the table directly. */
|Assert(OidIsValid(indexoid));
|ScanKeyInit(&skey[0], ObjectIdAttributeNumber, BTEqualStrategyNumber,
|F_OIDEQ, ObjectIdGetDatum(address.objectId));
|rel = heap_open(address.classId, AccessShareLock);
 ^^^ <- It tries to open pg_largeobject
|sd = systable_beginscan(rel, indexoid, true, SnapshotNow, 1, skey);
|found = HeapTupleIsValid(systable_getnext(sd));
|systable_endscan(sd);
|heap_close(rel, AccessShareLock);
|return found;
| }


Although no caller invokes get_object_address() with lockmode = NoLock,
isn't it necessary to skip locking if NoLock was given.

|/*
| * If we're dealing with a relation or attribute, then the relation is
| * already locked.  If we're dealing with any other type of object, we need
| * to lock it and then verify that it still exists.
| */
|if (address.classId != RelationRelationId)
|{
|if (IsSharedRelation(address.classId))
|LockSharedObject(address.classId, address.objectId, 0, lockmode);
|else
|LockDatabaseObject(address.classId, address.objectId, 0, lockmode);
|/* Did it go away while we were waiting for the lock? */
|if (!object_exists(address))
|elog(ERROR, "cache lookup failed for class %u object %u subobj %d",

Re: [HACKERS] Functional dependencies and GROUP BY

[Tom Lane]

Peter Eisentraut  writes:
> Next version.  Changed dependencies to pg_constraint, removed handling
> of unique constraints for now, and made some enhancements so that views
> track dependencies on constraints even in subqueries.  Should be close
> to final now. :-)

I've committed this with some revisions, notably:

The view.c changes were fundamentally wrong.  The right place to
extract dependencies from a parsetree is in dependency.c, specifically
find_expr_references_walker.  The way you were doing it meant that
dependencies on constraints would only be noticed for views, not for
other cases such as stored plans.

I rewrote the catalog search to look only at pg_constraint, not using
pg_index at all.  I think this will be easier to extend to the case of
looking for UNIQUE + NOT NULL, whenever we get around to doing that.
I also moved the search into catalog/pg_constraint.c, because it didn't
seem to belong in parse_agg (as hinted by the large number of #include
additions you had to make to put it there).

I put in a bit of caching logic to prevent repeating the search for
multiple Vars of the same relation.  Tests or no tests, I can't believe
that's going to be cheap enough that we want to repeat it over and
over...

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] Surprising dead_tuple_count from pgstattuple

[Robert Haas]

On Fri, Aug 6, 2010 at 9:11 PM, Itagaki Takahiro
 wrote:
> 2010/8/7 Gordon Shannon :
>> 1. I delete 10,000 rows.
>> pgstattuple.dead_tuple_count -> 1
>>
>> 2. I delete 15,000 more rows.
>> pgstattuple.dead_tuple_count -> 15000 ??
>>
>> pgstattuple now appears to count the earlier 10K deleted tuples as no longer
>> dead, but free space.
>
> I think it's expected behavior that comes from HOT page reclaim.
> The second DELETE not only deleted rows but also removed physical
> tuples that were deleted in 1. Missing dead rows were pruned by HOT.

What would have triggered a HOT prune at any point in this operation?
And why would it have reclaimed all of the deleted rows?

My thought would be "is autovacuum running in the background in
between these commands?".

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

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

Re: [HACKERS] Surprising dead_tuple_count from pgstattuple

[Itagaki Takahiro]

2010/8/7 Gordon Shannon :
> 1. I delete 10,000 rows.
> pgstattuple.dead_tuple_count -> 1
>
> 2. I delete 15,000 more rows.
> pgstattuple.dead_tuple_count -> 15000 ??
>
> pgstattuple now appears to count the earlier 10K deleted tuples as no longer
> dead, but free space.

I think it's expected behavior that comes from HOT page reclaim.
The second DELETE not only deleted rows but also removed physical
tuples that were deleted in 1. Missing dead rows were pruned by HOT.

-- 
Itagaki Takahiro

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

Re: [HACKERS] [JDBC] Trouble with COPY IN

[James William Pye]

On Aug 6, 2010, at 4:31 PM, Kris Jurka wrote:
> 

I think there's a snag in the patch:

postgres=# COPY data FROM '/Users/jwp/DATA.bcopy' WITH BINARY;
ERROR:  row field count is -1, expected 1
CONTEXT:  COPY data, line 4

Probably a quick/small fix away, I imagine.


But, I was able to trigger the new ERROR with py-postgresql:

>>> import postgresql as pg
>>> db=pg.open('localhost/postgres')
>>> q=db.prepare('copy data FROM STDIN WITH BINARY')
>>> from itertools import chain
>>> import sys
>>> db.pq.tracer = sys.stderr.write
>>> q.load_rows(chain(open('/Users/jwp/DATA.bcopy', 'rb'), (b'EXTRA',)))
↑ B(25): b'B\x00\x00\x00\x18\x00py:0x1268b30\x00\x00\x00\x00\x00\x00\x00'
↑ E(10): b'E\x00\x00\x00\t\x00\x00\x00\x00\x01'
↑ S(5): b'S\x00\x00\x00\x04'
↓ b'2'(0): b''
↓ b'G'(5): b'\x01\x00\x01\x00\x01'
↑__(7): b'PGCOPY\n'
↑__(3): b'\xff\r\n'
↑__(41): 
b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x04\x00\x00\x00\x01\x00\x01\x00\x00\x00\x04\x00\x00\x00\x02\x00\x01\x00\x00\x00\x04\x00\x00\x00\x03\xff\xff'
↑__(5): b'EXTRA'
↑ c(5): b'c\x00\x00\x00\x04'
↑ S(5): b'S\x00\x00\x00\x04'
↓ b'E'(95): b'SERROR\x00C22P04\x00Mreceived copy data after EOF marker\x00WCOPY 
data, line 4\x00Fcopy.c\x00L2081\x00RCopyFrom\x00\x00'
↓ b'Z'(1): b'I'
Traceback (most recent call last):
  File "", line 1, in 
  
  File 
"/Library/Frameworks/Python.framework/Versions/3.1/lib/python3.1/site-packages/postgresql/driver/pq3.py",
 line 462, in raise_server_error
raise server_error
postgresql.exceptions.BadCopyError: received copy data after EOF marker
  CODE: 22P04
  LOCATION: File 'copy.c', line 2081, in CopyFrom from SERVER
  CONTEXT: COPY data, line 4
STATEMENT: [prepared]
  sql_parameter_types: []
  statement_id: py:0x1268b30
  string: copy data FROM STDIN WITH BINARY
CONNECTION: [idle]
  client_address: ::1/128
  client_port: 63922
  version:
PostgreSQL 9.1devel on x86_64-apple-darwin10.4.0, compiled by GCC 
i686-apple-darwin10-gcc-4.2.1 (GCC) 4.2.1 (Apple Inc. build 5664), 64-bit
CONNECTOR: [Host] pq://jwp:*...@localhost:5432/postgres
  category: None
DRIVER: postgresql.driver.pq3.Driver
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] patch (for 9.1) string functions

[Itagaki Takahiro]

2010/7/26 Robert Haas :
> Come to think of it, have we checked that the behavior of LEFT, RIGHT,
> REVERSE, etc. is the same on other DBs, especially as far as nulls,
> empty strings, too-large or negative subscripts, etc is concerned?  Is
> CONCAT('foo', NULL) => 'foo' really the behavior that everyone else
> implements here?

I made a discussion page in wiki for the compatibility issue.
http://wiki.postgresql.org/wiki/String_Functions_and_Operators_Compatibility

Please fill empty cells and fix wrong descriptions.
  * concat() is not compatible between MySQL and Oracle/DB2. Which do we buy?
  * How do other databases behave in left() and right() with negative lengths?
  * Are there any databases that has similar features with format() or
sprintf() ?


> And why does CONCAT() take a variadic "ANY"
> argument?  Shouldn't that be variadic TEXT?

I think we have no other choice but to use VARIADIC "any" for variadic
functions.
We have all combinations of argument types for || operator, (text, text),
(text, any), (any, text), but we cannot use such codes for variadic functions
-- they have no limits of argument numbers. And in the case, the functions
should be STABLE because they convert arguments to text in it with typout
functions that might be STABLE.


IMHO, I'd repeat, syntax for format() is a bad choice because it cannot
concatenate multiple arguments without separator, though RAISE also uses it.
%s format in sprintf() or {n} syntax in C#'s String.Format() seems to be
a better design.

-- 
Itagaki Takahiro

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

Re: [HACKERS] [JDBC] Trouble with COPY IN

[Kris Jurka]

On Wed, 28 Jul 2010, James William Pye wrote:

Not directly regarding your patch, but while the discussion is in the 
general area. I think it would be wise to throw an error when non-empty 
CopyData messages are received after CopyData(EOF). Chances are that the 
user is making a mistake and should be notified of it.




As this is also the direction that Tom Lane indicated we should go, here 
is a patch which errors out after receiving any more copy data past the 
EOF marker.  This also fixes the protocol problem I previously brought up 
because the act of checking to see if there is any more data does ensure 
that if there isn't any more data in the current buffer, that we wait for 
the client to provide CopyDone/Fail.


Kris Jurka*** a/src/backend/commands/copy.c
--- b/src/backend/commands/copy.c
***
*** 2058,2069  CopyFrom(CopyState cstate)
int16   fld_count;
ListCell   *cur;
  
!   if (!CopyGetInt16(cstate, &fld_count) ||
!   fld_count == -1)
{
done = true;
break;
}
  
if (fld_count != attr_count)
ereport(ERROR,
--- 2058,2090 
int16   fld_count;
ListCell   *cur;
  
!   if (!CopyGetInt16(cstate, &fld_count))
{
done = true;
break;
}
+   
+   if (fld_count == -1)
+   {
+   /*
+* Reached EOF.  In protocol version 3, we must 
wait for
+* the protocol end of copy (CopyDone/Fail).  
If we
+* receive any more copy data after EOF, 
complain.
+*/
+   if (cstate->copy_dest == COPY_NEW_FE)
+   {
+   int8 unused;
+   if (CopyGetData(cstate, &unused, 
sizeof(unused), sizeof(unused)))
+   {
+   ereport(ERROR,
+   
(errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
+
errmsg("received copy data after EOF marker")));
+   } else {
+   done = true;
+   break;
+   }
+   }
+   }
  
if (fld_count != attr_count)
ereport(ERROR,

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

Re: [HACKERS] Update hstore % Doc

[Robert Haas]

On Fri, Aug 6, 2010 at 6:49 PM, David E. Wheeler
 wrote:
> On Aug 6, 2010, at 3:38 PM, Tom Lane wrote:
>
>>> We definitely need to document the `text % text` constructor
>>
>> BTW, there isn't any % constructor anymore --- we agreed to provide
>> only the hstore(text, text) constructor.
>
> Oh, I must've been looking at an older checkout, then. Never mind.

And also - it was never the constructor.  It was briefly the slice operator.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

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

Re: [HACKERS] Update hstore % Doc

[David E. Wheeler]

On Aug 6, 2010, at 3:38 PM, Tom Lane wrote:

>> We definitely need to document the `text % text` constructor
> 
> BTW, there isn't any % constructor anymore --- we agreed to provide
> only the hstore(text, text) constructor.

Oh, I must've been looking at an older checkout, then. Never mind.

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] Initial review of xslt with no limits patch

[David Fetter]

On Fri, Aug 06, 2010 at 11:48:58PM +0300, Peter Eisentraut wrote:
> On fre, 2010-08-06 at 21:31 +0200, Pavel Stehule wrote:
> > It must not be a function. Just I missing any tool that helps with
> > complex structured data. This proposed kind functions has one
> > advantage - there isn't necessary any change in parser. Yes, I can
> > use a pair of arrays, I can use a one array with seq name, value,
> > I can use a custom parser. But nothing from these offers a comfort
> > or readability for example a Perl's hash tables.
> 
> Maybe you should just use PL/XSLT. :-)

When's that going into the tree?

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

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

Re: [HACKERS] Update hstore % Doc

[Tom Lane]

"David E. Wheeler"  writes:
> We definitely need to document the `text % text` constructor

BTW, there isn't any % constructor anymore --- we agreed to provide
only the hstore(text, text) constructor.

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] Update hstore % Doc

[Tom Lane]

"David E. Wheeler"  writes:
> On Aug 6, 2010, at 3:16 PM, Tom Lane wrote:
>> It looks to me like you are changing the examples of the I/O
>> representation ... which did NOT change.

> Hrm? The first few examples at the top? I find them confusing because there 
> are no single quotes around them, so they look like the use of the deprecated 
> => operator (especially the first two). Just look at: 

Yeah, but there's a sentence in front of them that says specifically
that these are examples of the text representation, not pieces of SQL.

> And I hate to say it, but % is awful.

Yeah, I know, but you can't have =>.  Unless you can persuade the SQL
committee to back off their syntax choice for parameters.  (:= would
have been a lot better ...)

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] GROUPING SETS revisited

[Joshua Tolley]

On Thu, Aug 05, 2010 at 04:46:51PM +0200, Pavel Stehule wrote:
> I am sending a updated version.

I've been looking at the changes to gram.y, and noted the comment under 
func_expr
where you added CUBE and ROLLUP definitions. It says that CUBE can't be a
reserved keyword because it's already used in the cube contrib module. But
then the changes to kwlist.h include this:

+ PG_KEYWORD("cube", CUBE, RESERVED_KEYWORD)
...
+ PG_KEYWORD("rollup", ROLLUP, RESERVED_KEYWORD)

...and CUBE and ROLLUP are added in gram.y under the reserved_keyword list. I
realize things like CURRENT_TIME, that also have special entries in the
func_expr grammar, are also reserved keywords, but this all seems at odds with
the comment. What am I missing? Is the comment simply pointing out that the
designation of CUBE and ROLLUP as reserved keywords will have to change at
some point, but it hasn't been implemented yet (or no one has figured out how
to do it)?

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com


signature.asc
Description: Digital signature

Re: [HACKERS] Update hstore % Doc

[David E. Wheeler]

On Aug 6, 2010, at 3:16 PM, Tom Lane wrote:

>> I noticed that the hstore docs still document the => operator instead
>> of %. This patch changes that.
> 
> It looks to me like you are changing the examples of the I/O
> representation ... which did NOT change.

Hrm? The first few examples at the top? I find them confusing because there are 
no single quotes around them, so they look like the use of the deprecated => 
operator (especially the first two). Just look at: 

 http://developer.postgresql.org/pgdocs/postgres/hstore.html

Maybe that's standard in the docs, but it seems weird to me. It's a minor 
point, though.

We definitely need to document the `text % text` constructor rather than the 
deprecated `text => text` constructor.

And I hate to say it, but % is awful. Sorry, I know I'm probably opening a can 
of worms here, and I did finally push % after all the arguments, but coming 
back to it fresh it just looks bizarre to me. Maybe that ship has sailed, 
though, and I'm just being difficult.

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] Update hstore % Doc

[Tom Lane]

"David E. Wheeler"  writes:
> I noticed that the hstore docs still document the => operator instead
> of %. This patch changes that.

It looks to me like you are changing the examples of the I/O
representation ... which did NOT change.

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] Initial review of xslt with no limits patch

[Tom Lane]

"David E. Wheeler"  writes:
> On Aug 6, 2010, at 2:12 PM, Pavel Stehule wrote:
>> so there is only small step to proposed feature
>> SELECT foo(this := 'that', "1" := 4)

> Sorry, not following you here

Pavel doesn't understand "no" ;-)

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] Initial review of xslt with no limits patch

[Tom Lane]

Peter Eisentraut  writes:
> On fre, 2010-08-06 at 13:01 -0400, Tom Lane wrote:
>> 2. I'm not sure whether we ought to auto-single-quote the values.
>> If we don't, how hard is it for users to properly quote nonconstant
>> parameter values?  (Will quote_literal work, or are the quoting rules
>> different for libxslt?)  If we do, are we giving up functionality
>> someone cares about?

> Not every parameter is a string.

So I gather, but what else is there, and do we actually want to expose
the other alternatives in xslt_process()?

If we don't auto-quote, we need to provide some sort of quote_xslt()
function that will apply the appropriate quoting/escaping to deal
with an arbitrary string value.

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: xml_is_well_formed

[Robert Haas]

On Fri, Aug 6, 2010 at 4:52 PM, Peter Eisentraut  wrote:
> On fre, 2010-08-06 at 07:31 -0400, Robert Haas wrote:
>> > What about making the function sensitive to the XML OPTION, such
>> that:
>> >
>> > test=# SET xmloption TO DOCUMENT;
>> > SET
>> > text=# SELECT xml_is_well_formed('foo');
>> >
>> >  xml_is_well_formed
>> >  
>> >  f
>> >  (1 row)
>>
>> That will make using this function a huge hassle, won't it?  Functions
>> that do different things depending on GUC settings are usually
>> troublesome.  Having three functions would be more sensible if we need
>> all three behaviors, but I don't see why we do.
>
> Upthread you opined that this function should essentially indicate
> whether a cast to type xml would succeed, and observing the xmloption is
> an essential part of that.  I had assumed the original patch actually
> did that.

Oh.

I didn't realize the casting behavior was already dependent on the GUC.  Yuck.

Maybe we should following the GUC after all, then.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

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

Re: [HACKERS] gincostestimate

[Tom Lane]

=?UTF-8?B?SmFuIFVyYmHFhHNraQ==?=  writes:
> [ review of gincostestimate-0.19 ]

I went through this patch, re-synced with current HEAD, and did some
minor editorializing; a new version is attached.  (Caution: I have not
tested this beyond verifying that it still compiles.)

> Codewise I have one question: the patch changes a loop in 
> ginvacuumcleanup from
> for (blkno = GIN_ROOT_BLKNO + 1; blkno < npages; blkno++)
> to
> for (blkno = GIN_ROOT_BLKNO; blkno < npages; blkno++)
> why should it now go through all blocks?

I think this is correct.  Before, vacuum had nothing useful to do on the
root page so it just skipped it.  Now, it needs to count the root page
in the appropriate way in the stats it's gathering.  The previous coding
maybe could have used a comment, but this version is unsurprising.

> The patch has lots of statements like if ( GinPageIsLeaf(page) ), that 
> is with extra space between the outer parenthesis and the condition, 
> which AFAIK is not the project style. I guess pgindent fixes that, so 
> it's no big deal.

> There are lines with elog(NOTICE) that are #if 0, they probably should 
> either become elog(DEBUGX) or get removed.

I fixed the latter and cleaned up some of the formatting violations,
though not all.  I dunno if anyone feels like running pgindent on the
patch at the moment.

I think there are two big problems left before this patch can be
applied:

1. The use of rd_amcache is very questionable.  There's no provision for
updates executed by one session to get reflected into stats already
cached in another session.  You could fix that by forcing relcache
flushes whenever you update the stats, as btree does:

/* send out relcache inval for metapage change */
if (!InRecovery)
CacheInvalidateRelcache(rel);

However I think that's probably a Really Bad Idea, because it would
result in extremely frequent relcache flushes, and those are expensive.
(The reason this mechanism is good for btree is it only needs to update
its cache after a root page split, which is infrequent.)  My advice is
to drop the use of rd_amcache completely, and just have the code read
the metapage every time gincostestimate runs.  If you don't like that
then you're going to need to think hard about how often to update the
cache and what can drive that operation at the right times.

BTW, another problem that would need to be fixed if you keep this code
is that ginUpdateStatInPlace wants to force the new values into
rd_amcache, which (a) is pretty useless and (b) risks a PANIC on
palloc failure, because it's called from a critical section.

2. Some of the calculations in gincostestimate seem completely bogus.
In particular, this bit:

/* calc partial match: we don't need a search but an index scan */
*indexStartupCost += partialEntriesInQuals * numEntryPages / numEntries;

is surely wrong, because the number being added to indexStartupCost is
a pure ratio not scaled by any cost unit.  I don't understand what this
number is supposed to be, so it's not clear what cost variable ought to
be included.

This equation doesn't seem amazingly sane either:

/* cost to scan data pages for each matched entry */
pagesFetched = ceil((searchEntriesInQuals + partialEntriesInQuals) *
 numDataPages / numEntries);

This has pagesFetched *decreasing* as numEntries increases, which cannot
be right can it?

Also, right after that step, there's a bit of code that re-estimates
pagesFetched from selectivity and uses the larger value.  Fine, but why
are you only applying that idea here and not to the entry-pages
calculation done a few lines earlier?

regards, tom lane



binfclKHw9sIz.bin
Description: gincostestimate-0.20.gz

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

Re: [HACKERS] Initial review of xslt with no limits patch

[David E. Wheeler]

On Aug 6, 2010, at 2:12 PM, Pavel Stehule wrote:

>> SELECT foo('this' ~> 'that', 1 ~> 4);
>> 
>> Not bad, I think. I kind of like it. It reminds me how much I hate the % 
>> hstore construction operator, though (the new name for =>).
> 
> so there is only small step to proposed feature
> 
> SELECT foo(this := 'that', "1" := 4)
> 
> there is only one difference (but you cannot implement it now)
> * notation for key is same like for sql identifier - why: I would to
> clearly identify key and value. When I use a custom operator - like
> you did, it depends on implementation what is key, what is value. When
> you use a SQL identifier's notation for key, you can't to do a error

Sorry, not following you here…

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] MERGE Specification

[Peter Eisentraut]

On tor, 2010-08-05 at 16:35 +0100, Simon Riggs wrote:
> * DELETE is an extension to the standard, though supported by Oracle,
> DB2 and SQLServer and damn useful

-> SQL:2011


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

Re: [HACKERS] Initial review of xslt with no limits patch

[Pavel Stehule]

2010/8/6 David E. Wheeler :
> On Aug 6, 2010, at 1:49 PM, Pavel Stehule wrote:
>
>> yes it is one a possibility and probably best. The nice of this
>> variant can be two forms like current variadic does -  foo(.., a :=
>> 10, b := 10) or foo(.., variadic ARRAY[(a,10),(b,10)])
>
> I started fiddling and got as far as this:
>
>
> CREATE TYPE pair AS ( key text, val text );
>
> CREATE OR REPLACE FUNCTION pair(anyelement, anyelement) RETURNS pair
> LANGUAGE SQL AS $$
>    SELECT ROW($1, $2)::pair;
> $$;
>
> CREATE OR REPLACE FUNCTION pair(text, text) RETURNS pair
> LANGUAGE SQL AS $$
>    SELECT ROW($1, $2)::pair;
> $$;
>
> CREATE OPERATOR ~> (
>        LEFTARG = anyelement,
>        RIGHTARG = anyelement,
>        PROCEDURE = pair
> );
>
> CREATE OPERATOR ~> (
>        LEFTARG = text,
>        RIGHTARG = text,
>        PROCEDURE = pair
> );
>
> CREATE OR REPLACE FUNCTION foo(variadic pair[]) RETURNS SETOF text
> LANGUAGE SQL AS $$
> --    SELECT unnest($1)::text
>    SELECT $1[1].key
>    UNION  SELECT $1[1].val
>    UNION  SELECT $1[2].key
>    UNION  SELECT $1[2].val;
> $$;
>
> SELECT foo('this' ~> 'that', 1 ~> 4);
>
> Not bad, I think. I kind of like it. It reminds me how much I hate the % 
> hstore construction operator, though (the new name for =>).

so there is only small step to proposed feature

SELECT foo(this := 'that', "1" := 4)

there is only one difference (but you cannot implement it now)
* notation for key is same like for sql identifier - why: I would to
clearly identify key and value. When I use a custom operator - like
you did, it depends on implementation what is key, what is value. When
you use a SQL identifier's notation for key, you can't to do a error

Regards

Pavel

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

[HACKERS] Update hstore % Doc

[David E. Wheeler]

Hackers,

I noticed that the hstore docs still document the => operator instead of %. 
This patch changes that. It also updates the first examples to us full SQL 
statements, because otherwise the use of => without surrounding single quotes 
was confusing.

Best,

David



hstoredoc.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] patch for contrib/isn

[Peter Eisentraut]

On ons, 2010-08-04 at 19:32 +0200, Jan Otto wrote:
> patch against HEAD is attached and validated against a lot of
> previously wrong and correct hyphenated isbn.

I think this module could use a regression test.


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

Re: [HACKERS] Functional dependencies and GROUP BY

[Peter Eisentraut]

On mån, 2010-07-26 at 10:46 -0600, Alex Hunsaker wrote:
> On Sat, Jul 24, 2010 at 06:23, Peter Eisentraut  wrote:
> 
> > Another open question I thought of was whether we should put the
> > dependency record on the pg_index row, or the pg_constraint row, or
> > perhaps the pg_class row.  Right now, it is using pg_index, because that
> > was easiest to code up, but I suspect that once we have not-null
> > constraints in pg_constraint, it will be more consistent to make all
> > dependencies go against pg_constraint rather than a mix of several
> > catalogs.
> 
> I think for primary keys pg_index is OK.  However for the not-null
> case we have to use pg_constraint... So given that we end up having to
> code that anyways, it seems like it will end up being
> cleaner/consistent to always use the pg_constraint row(s).  So +1 for
> using pg_constraint instead of pg_index from me.

Next version.  Changed dependencies to pg_constraint, removed handling
of unique constraints for now, and made some enhancements so that views
track dependencies on constraints even in subqueries.  Should be close
to final now. :-)
diff --git a/doc/src/sgml/queries.sgml b/doc/src/sgml/queries.sgml
index 416e599..a103229 100644
--- a/doc/src/sgml/queries.sgml
+++ b/doc/src/sgml/queries.sgml
@@ -886,10 +886,7 @@ SELECT product_id, p.name, (sum(s.units) * p.price) AS sales
 In this example, the columns product_id,
 p.name, and p.price must be
 in the GROUP BY clause since they are referenced in
-the query select list.  (Depending on how the products
-table is set up, name and price might be fully dependent on the
-product ID, so the additional groupings could theoretically be
-unnecessary, though this is not implemented.)  The column
+the query select list (but see below).  The column
 s.units does not have to be in the GROUP
 BY list since it is only used in an aggregate expression
 (sum(...)), which represents the sales
@@ -898,6 +895,18 @@ SELECT product_id, p.name, (sum(s.units) * p.price) AS sales

 

+If the products table is set up so that,
+say, product_id is the primary key, then it
+would be enough to group by product_id in the
+above example, since name and price would
+be functionally
+dependentfunctional
+dependency on the product ID, and so there
+would be no ambiguity about which name and price value to return
+for each product ID group.
+   
+
+   
 In strict SQL, GROUP BY can only group by columns of
 the source table but PostgreSQL extends
 this to also allow GROUP BY to group by columns in the
diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml
index 74021e8..8436d85 100644
--- a/doc/src/sgml/ref/select.sgml
+++ b/doc/src/sgml/ref/select.sgml
@@ -520,9 +520,12 @@ GROUP BY expression [, ...]
 produces a single value computed across all the selected rows).
 When GROUP BY is present, it is not valid for
 the SELECT list expressions to refer to
-ungrouped columns except within aggregate functions, since there
-would be more than one possible value to return for an ungrouped
-column.
+ungrouped columns except within aggregate functions or if the
+ungrouped column is functionally dependent on the grouped columns,
+since there would otherwise be more than one possible value to
+return for an ungrouped column.  A functional dependency exists if
+the grouped columns (or a subset thereof) are the primary key of
+the table containing the ungrouped column.

   
 
diff --git a/src/backend/commands/view.c b/src/backend/commands/view.c
index 67c90a2..455fc6e 100644
--- a/src/backend/commands/view.c
+++ b/src/backend/commands/view.c
@@ -16,7 +16,9 @@
 
 #include "access/heapam.h"
 #include "access/xact.h"
+#include "catalog/dependency.h"
 #include "catalog/namespace.h"
+#include "catalog/pg_constraint.h"
 #include "commands/defrem.h"
 #include "commands/tablecmds.h"
 #include "commands/view.h"
@@ -390,6 +392,28 @@ UpdateRangeTableOfViewParse(Oid viewOid, Query *viewParse)
 }
 
 /*
+ * Walker to collect the constraintDeps fields of all Query nodes in a
+ * query tree into a single list.
+ */
+static bool
+collectConstraintDeps_walker(Node *node, void *context)
+{
+	if (node == NULL)
+		return false;
+	else if (IsA(node, Query))
+	{
+		Query *query = (Query *) node;
+		List **listp = (List **) context;
+
+		*listp = list_concat_unique_oid(*listp, query->constraintDeps);
+
+		return query_tree_walker(query, collectConstraintDeps_walker, context, 0);
+	}
+	else
+		return expression_tree_walker(node, collectConstraintDeps_walker, context);
+}
+
+/*
  * DefineView
  *		Execute a CREATE VIEW command.
  */
@@ -399,6 +423,7 @@ DefineView(ViewStmt *stmt, const char *queryString)
 	Query	   *viewParse;
 	Oid			viewOid;
 	RangeVar   *view;
+	List	   *allConstraintDeps;
 
 	/*
 	 * Run parse analysis to convert the raw parse tree to a Query.  Note this
@@ -479,6 +504,30 @@ Define

Re: [HACKERS] Initial review of xslt with no limits patch

[Robert Haas]

On Fri, Aug 6, 2010 at 1:46 PM, Pavel Stehule  wrote:
> I'll propose a new kind of functions (only position parameter's
> function). My idea is simple - for functions with this mark the mixed
> and named notation is blocked. But these functions can have a
> parameter names - and these names can be passed to function. So there
> is possible have a
>
> xslt_process function with current behave - third argument has not
> label, and new variadic version like
>
> xslt_process(..,.., param_name1 = 'v1', param_name2 = 'v2',
> param_name3 = 'v3', ...)
>
> an implementation of this functionality can be very simple, and we can
> use this for xslt_process in 9.1

Why wouldn't we just pass two text arrays to this function and be done
with it?  Custom syntax is all well and good when you're writing these
calls by hand, but it's not hard to imagine someone wanting to pass in
values stored somewhere else.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

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

Re: [HACKERS] Initial review of xslt with no limits patch

[David E. Wheeler]

On Aug 6, 2010, at 1:49 PM, Pavel Stehule wrote:

> yes it is one a possibility and probably best. The nice of this
> variant can be two forms like current variadic does -  foo(.., a :=
> 10, b := 10) or foo(.., variadic ARRAY[(a,10),(b,10)])

I started fiddling and got as far as this:


CREATE TYPE pair AS ( key text, val text );

CREATE OR REPLACE FUNCTION pair(anyelement, anyelement) RETURNS pair
LANGUAGE SQL AS $$
SELECT ROW($1, $2)::pair;
$$;

CREATE OR REPLACE FUNCTION pair(text, text) RETURNS pair
LANGUAGE SQL AS $$
SELECT ROW($1, $2)::pair;
$$;

CREATE OPERATOR ~> (
LEFTARG = anyelement,
RIGHTARG = anyelement,
PROCEDURE = pair
);

CREATE OPERATOR ~> (
LEFTARG = text,
RIGHTARG = text,
PROCEDURE = pair
);

CREATE OR REPLACE FUNCTION foo(variadic pair[]) RETURNS SETOF text
LANGUAGE SQL AS $$
--SELECT unnest($1)::text
SELECT $1[1].key
UNION  SELECT $1[1].val
UNION  SELECT $1[2].key
UNION  SELECT $1[2].val;
$$;

SELECT foo('this' ~> 'that', 1 ~> 4);

Not bad, I think. I kind of like it. It reminds me how much I hate the % hstore 
construction operator, though (the new name for =>).

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] review: xml_is_well_formed

[Peter Eisentraut]

On fre, 2010-08-06 at 14:43 +0100, Mike Fowler wrote:
> > Or perhaps it could return a string instead of a boolean: content,
> > document, or NULL if it's neither.
> >
> 
> I like the sound of that. In fact this helps workaround the IS
> DOCUMENT 
> and IS CONTENT limitations such that you can you can select only 
> content, only documents or both is you use IS NOT NULL.
> 
> Unless anyone sees a reason that this function needs to remain a
> boolean function, I'll rework the patch over the weekend. 

What is the actual use case for this function?  Is the above behavior
actually useful?

One reason to stick with boolean is backward compatibility.


-- 
Sent 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: xml_is_well_formed

[Peter Eisentraut]

On fre, 2010-08-06 at 07:31 -0400, Robert Haas wrote:
> > What about making the function sensitive to the XML OPTION, such
> that:
> >
> > test=# SET xmloption TO DOCUMENT;
> > SET
> > text=# SELECT xml_is_well_formed('foo');
> >
> >  xml_is_well_formed
> >  
> >  f
> >  (1 row)
> 
> That will make using this function a huge hassle, won't it?  Functions
> that do different things depending on GUC settings are usually
> troublesome.  Having three functions would be more sensible if we need
> all three behaviors, but I don't see why we do.

Upthread you opined that this function should essentially indicate
whether a cast to type xml would succeed, and observing the xmloption is
an essential part of that.  I had assumed the original patch actually
did that.


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

Re: [HACKERS] Initial review of xslt with no limits patch

[Pavel Stehule]

> yes it is one a possibility and probably best. The nice of this
> variant can be two forms like current variadic does -  foo(.., a :=
> 10, b := 10) or foo(.., variadic ARRAY[(a,10),(b,10)])
>
>
pardon foo(..., VARIADIC ARRAY[('a', 10), ('b' 10)])

regards

Pavel

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

Re: [HACKERS] Initial review of xslt with no limits patch

[Pavel Stehule]

2010/8/6 Peter Eisentraut :
> On fre, 2010-08-06 at 21:31 +0200, Pavel Stehule wrote:
>> It must not be a function. Just I missing any tool that helps with
>> complex structured data. This proposed kind functions has one
>> advantage - there isn't necessary any change in parser. Yes, I can use
>> a pair of arrays, I can use a one array with seq name, value, I can
>> use a custom parser. But nothing from these offers a comfort or
>> readability for example a Perl's hash tables.
>
> Maybe you should just use PL/XSLT. :-)
>
:)

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

Re: [HACKERS] Initial review of xslt with no limits patch

[Pavel Stehule]

2010/8/6 David E. Wheeler :
> On Aug 6, 2010, at 11:13 AM, Tom Lane wrote:
>
>> That would work too, although I think it might be a bit harder to use
>> than one alternating-name-and-value array, at least in some scenarios.
>> You'd have to be careful that you got the values in the same order in
>> both arrays, which'd be easy to botch.
>>
>> There might be other use-cases where two separate arrays are easier
>> to use, but I'm not seeing one offhand.
>
> Stuff like this makes me wish PostgreSQL had an ordered pair data type. Then 
> you'd just have a function with `variadic ordered pair` as the signature.
>

yes it is one a possibility and probably best. The nice of this
variant can be two forms like current variadic does -  foo(.., a :=
10, b := 10) or foo(.., variadic ARRAY[(a,10),(b,10)])



> I don't suppose anyone has implemented a data type like this…
>
> 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] Initial review of xslt with no limits patch

[Peter Eisentraut]

On fre, 2010-08-06 at 21:31 +0200, Pavel Stehule wrote:
> It must not be a function. Just I missing any tool that helps with
> complex structured data. This proposed kind functions has one
> advantage - there isn't necessary any change in parser. Yes, I can use
> a pair of arrays, I can use a one array with seq name, value, I can
> use a custom parser. But nothing from these offers a comfort or
> readability for example a Perl's hash tables.

Maybe you should just use PL/XSLT. :-)


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

Re: [HACKERS] Initial review of xslt with no limits patch

[Peter Eisentraut]

On fre, 2010-08-06 at 13:01 -0400, Tom Lane wrote:
> 2. I'm not sure whether we ought to auto-single-quote the values.
> If we don't, how hard is it for users to properly quote nonconstant
> parameter values?  (Will quote_literal work, or are the quoting rules
> different for libxslt?)  If we do, are we giving up functionality
> someone cares about?

Not every parameter is a string.

Compare xsltproc:

  --param PARAMNAME PARAMVALUE
   Pass a parameter of name PARAMNAME and value PARAMVALUE to the
   stylesheet. You may pass multiple name/value pairs up to a
   maximum of 32. If the value being passed is a string, you can use
   --stringparam instead, to avoid additional quote characters
   that appear in string expressions. Note: the XPath expression
   must be UTF-8 encoded.

  --stringparam PARAMNAME PARAMVALUE
   Pass a parameter of name PARAMNAME and value PARAMVALUE where
   PARAMVALUE is a string rather than a node identifier.  Note:
   The string must be UTF-8 encoded.



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

Re: [HACKERS] including backend ID in relpath of temp rels - updated patch

[Alvaro Herrera]

Excerpts from Robert Haas's message of vie ago 06 15:32:21 -0400 2010:

> > Perhaps run through pg_class after restart and flush anything marked
> > relistemp?
> 
> The trouble is that you have to bind to a database before you can run
> through pg_class, and the postmaster doesn't.  Of course, if it could
> attach to a database and then detach again, this might be feasible,
> although perhaps still a bit overly complex for the postmaster, but in
> any event it doesn't.

A simpler idea seems to run a process specifically to connect to the
database to scan pg_class there, and then die.  It sounds a tad
expensive though.

> I've been thinking about that, but it's a bit challenging to imagine
> how it could work.  It's not just the pg_class entries you have to
> think about, but also pg_attrdef, pg_attribute, pg_constraint,
> pg_description, pg_index, pg_rewrite, and pg_trigger.  An even
> stickier problem is that we have lots of places in the backend code
> that refer to objects by OID.  We'd either need to change all of that
> code (which seems like a non-starter) or somehow guarantee that the
> OIDs assigned to any given backend's private objects would be
> different from those assigned to any public object (which I also don't
> see how to do).

Maybe we could reserve one of the 32 bits of OID to indicate private-ness.

-- 
Álvaro Herrera 
The PostgreSQL Company - Command Prompt, Inc.
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] Cost of AtEOXact_Buffers in --enable-cassert

[Andres Freund]

On Friday 06 August 2010 20:23:15 Tom Lane wrote:
> Andres Freund  writes:
> > The most prohibitively expensive part is the AtEOXact_Buffers check of
> > running through all buffers and checking their pin count. And it makes
> > $app's regression tests take thrice their time...
> > 
> > Would somebody object agains putting those in an extra define so that
> > those can be disabled in pg_config_manual? Or even disable it by default
> > entirely...
> Not a chance for the latter; this is an important sanity check that
> catches real coding mistakes with some frequency.
Ok.

> I'd be willing to consider a "half assert" mode that turns off some of
> the most expensive checks, but AtEOXact_Buffers is hardly the only thing
> that ought to be in that list.  The CLOBBER_FREED_MEMORY and memory
> context checking stuff is pretty durn expensive too.
I personally have seen that catching way more bugs than the AtEOXact_Buffers 
check, but that might be because I have found mostly bugs in random c 
functions, not in pg stuff ;-)

I will wait a bit and wait for more suggestions about expensive checks and/or 
other comments and will provide a patch for such a mode.

Andres

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

Re: [HACKERS] Initial review of xslt with no limits patch

[David E. Wheeler]

On Aug 6, 2010, at 11:13 AM, Tom Lane wrote:

> That would work too, although I think it might be a bit harder to use
> than one alternating-name-and-value array, at least in some scenarios.
> You'd have to be careful that you got the values in the same order in
> both arrays, which'd be easy to botch.
> 
> There might be other use-cases where two separate arrays are easier
> to use, but I'm not seeing one offhand.

Stuff like this makes me wish PostgreSQL had an ordered pair data type. Then 
you'd just have a function with `variadic ordered pair` as the signature.

I don't suppose anyone has implemented a data type like this…

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: Review: Re: [PATCH] Re: [HACKERS] Adding xpath_exists function

[Peter Eisentraut]

On fre, 2010-08-06 at 09:04 +0100, Mike Fowler wrote:
> If the patch is to be committed, does it make sense for me to refine
> it such that it uses the new xpath internal function you extracted in
> the xmlexists patch?

Yes, you can probably shrink this patch down to about 20 lines.


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

Re: [HACKERS] [ADMIN] postgres 9.0 crash when bringing up hot standby

[Heikki Linnakangas]

On 06/08/10 17:31, Fujii Masao wrote:

On Fri, Aug 6, 2010 at 10:10 PM, Alanoly Andrews  wrote:

I’m testing “hot standby” using “streaming WAL records”. On trying to bring
(dbx) where
_alloc_initial_pthread(??) at 0x949567c
__pth_init(??) at 0x9493ba4
uload(??, ??, ??, ??, ??, ??, ??, ??) at 0x9fff0001954
load_64.load(??, ??, ??) at 0x904686c
loadAndInit() at 0x947bd7c
dlopen(??, ??) at 0x911cc4c
internal_load_library(libname =
"/apps/pg_9.0_b4/lib/postgresql/libpqwalreceiver.so"), line 234 in "dfmgr.c"
load_file(filename = "libpqwalreceiver", restricted = '\0'), line 156 in
"dfmgr.c"
WalReceiverMain(), line 248 in "walreceiver.c"
AuxiliaryProcessMain(argc = 2, argv = 0x0fffa8b8), line 428 in
"bootstrap.c"
StartChildProcess(type = WalReceiverProcess), line 4405 in "postmaster.c"
sigusr1_handler(postgres_signal_arg = 30), line 4227 in "postmaster.c"
__fd_select(??, ??, ??, ??, ??) at 0x911805c
postmaster.select(__fds = 5, __readlist = 0x0fffd0a8, __writelist =
(nil), __exceptlist = (nil), __timeout = 0x00c0), line 229 in
"time.h"
unnamed block in ServerLoop(), line 1391 in "postmaster.c"
unnamed block in ServerLoop(), line 1391 in "postmaster.c"
ServerLoop(), line 1391 in "postmaster.c"
PostmasterMain(argc = 1, argv = 0x0001102aa4b0), line 1092 in
"postmaster.c"
main(argc = 1, argv = 0x0001102aa4b0), line 188 in "main.c"

Any pointers on how to resolve the issue will be much appreciated.


So, loading libpqwalreceiver library crashes. It looks like it might be 
pthread-related. Perhaps something wrong with our makefiles, causing 
libpqwalreceiver to be built with wrong flags? Does contrib/dblink work? 
If you look at the build log, what is the command line used to compile 
libpqwalreceiver, and what is the command line used to build other 
libraries, like contrib/dblink?


--
  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] including backend ID in relpath of temp rels - updated patch

[Robert Haas]

On Fri, Aug 6, 2010 at 2:43 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Fri, Aug 6, 2010 at 2:07 PM, Tom Lane  wrote:
>>> Sure, it tops out somewhere, but 32K is way too close to configurations
>>> we know work well enough in the field (I've seen multiple reports of
>>> people using a couple thousand backends).
>
>> Well, I wouldn't expect anyone to use an exclusive lock for readers
>> without a good reason, but you still have n backends that each have to
>> read, presumably, about O(n) messages, so eventually that's going to
>> start to pinch.
>
> Sure, but I don't see much to be gained from multiple queues either.
> There are few (order of zero, in fact) cases where sinval messages
> are transmitted that aren't of potential interest to all backends.
> Maybe you could do something useful with a very large number of
> dynamically-defined queues (like one per relation) ... but managing that
> would probably swamp any savings.

Well, what I was thinking is that if you could guarantee that a
certain backend COULDN'T have a particular relfilenode open at the
smgr level, for example, then it needn't read the invalidation
messages for that relfilenode.  Precisely how to slice that up is
another matter.  For the present case, for instance, you could
creating one queue per backend.  In the normal course of events, each
backend would subscribe only to its own queue, but if one backend
wanted to drop a temporary relation belonging to some other backend,
it would temporarily subscribe to that backend's queue, do whatever it
needed to do, and then flush all the smgr references before
unsubscribing from the queue.  That's a bit silly because we doubtless
wouldn't invent such a complicated mechanism just for this case, but I
think it illustrates the kind of thing that one could do.  Or if you
wanted to optimize for the case of a large number of databases running
in a single cluster, perhaps you'd want one queue per database plus a
shared queue for the shared catalogs.  I don't know.  This is just pie
in the sky.

>> Do you think it's worth worrying about the reduction in the number of
>> possible SI message types?
>
> IIRC the number of message types is the number of catalog caches plus
> half a dozen or so.  We're a long way from exhausting even a 1-byte
> ID field; and we could play more games if we had to, since there would
> be a padding byte free in the message types that refer to a catalog
> cache.  IOW, 1-byte id doesn't bother me.

OK.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

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

Re: [HACKERS] including backend ID in relpath of temp rels - updated patch

[Robert Haas]

On Fri, Aug 6, 2010 at 3:00 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> This patch only directly addresses the issue of cleaning up the
>> storage, so there are still the catalog entries to worry about.  But
>> it doesn't seem impossible to think about building on this approach to
>> eventually handle that part of the problem better, too.  I haven't
>> thought too much about what that would look like, but I think it could
>> be done.
>
> Perhaps run through pg_class after restart and flush anything marked
> relistemp?

The trouble is that you have to bind to a database before you can run
through pg_class, and the postmaster doesn't.  Of course, if it could
attach to a database and then detach again, this might be feasible,
although perhaps still a bit overly complex for the postmaster, but in
any event it doesn't.  We seem to already have a mechanism in place
where a backend that creates a temprel nukes any pre-existing temprel
schema for its own backend, but of course that's not fool-proof.

> Although the ideal solution, probably, would be for temp
> tables to not have persistent catalog entries in the first place.

I've been thinking about that, but it's a bit challenging to imagine
how it could work.  It's not just the pg_class entries you have to
think about, but also pg_attrdef, pg_attribute, pg_constraint,
pg_description, pg_index, pg_rewrite, and pg_trigger.  An even
stickier problem is that we have lots of places in the backend code
that refer to objects by OID.  We'd either need to change all of that
code (which seems like a non-starter) or somehow guarantee that the
OIDs assigned to any given backend's private objects would be
different from those assigned to any public object (which I also don't
see how to do).

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

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

Re: [HACKERS] Initial review of xslt with no limits patch

[Pavel Stehule]

2010/8/6 Tom Lane :
> Pavel Stehule  writes:
>> For Tom: proposed syntax can be used generally - everywhere when you
>> are working with collection. It can be used for hash (hstore)
>> constructor for example. For me is more readable code like
>
>> select hstore(name := 'Tomas', surname := 'Novak')
>
> You've tried to sell us on that before, with few takers.  This proposed
> use-case impresses me even less than the previous ones, because callers
> of xslt_process seem quite likely to need to work with non-constant
> parameter names.
>
> In any case, given what we have at the moment for function overload
> resolution rules, I think it's a fundamentally bad idea to introduce
> a "wild card" function type that would necessarily conflict with
> practically every other possible function declaration.  So regardless
> of what use-cases you propose, I'm going to vote against that.

It must not be a function. Just I missing any tool that helps with
complex structured data. This proposed kind functions has one
advantage - there isn't necessary any change in parser. Yes, I can use
a pair of arrays, I can use a one array with seq name, value, I can
use a custom parser. But nothing from these offers a comfort or
readability for example a Perl's hash tables.

so if we have a integrated hash tables, then we can to write some like

CREATE FUNCTION xslt_process(..,.., text{})

select xslt_process(..,.., { par1 => val1, par2 => val3, .. } )

any simple method can significantly help for us, who write a lot of
complex stored procedures. It can be a big help. I am only
dissatisfied because it is "Perlism" - maybe I don't understand SQL
well, but my personal opinion about the most natural syntax for this
situation is some like SQL/XML - xmlattributes or named notation. SQL
isn't too much consistent too - it uses two semantically similar
syntax

foo(name=>value, ..) versus foo(value AS name, ..)

Next my idea was mix of named parameters and marked variadic parameter:

CREATE FUNCTION xslt_process(..,.., VARIADIC LABELED text[])

and then we can call SELECT xslt_process(..,.., par1 := var, ..)

This doesn't introduce a heavy new syntax - just use old in some
specific context. It is only my feeling, so this way is more SQL
natural than using a some hash data type. Maybe you don't think, so
stored procedures must not to have a this functionality. Everywhere
where you can to wrap a c library for SQL you have to meet this task -
often you have to pass a set of flags, collection of values. With
default parameter values situation is better then before, but its
still not ideal - for example SOAP interface or dblink to Oracle

SELECT exec(db, 'SELECT * FROM foo where c = :name', name => 'Pavel')

so this is my motivation - the possibility to write generic custom
functions. Sure - this is "redundant" to existing functionality. I can
write:

SELECT exec(db, 'SELECT * FROM ...', ARRAY['name'], ARRAY['Pavel']) --
Robert'syntax
or your syntax:
SELECT exec(db, 'SELECT * FROM ...', ARRAY['name','Pavel']) or

like current xml2 syntax:
SELECT exec(db, 'SELECT * FROM ...', 'name="Pavel")

But these versions are "too simple" if you understand me. It doesn't
do life of SQL stored procedure's programmer simper.

Regards

Pavel

p.s. sorry for offtopic

p.s. for me isn't important notation. Just I would to like more things
with custom functions withou parser modification.



>
>                        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] including backend ID in relpath of temp rels - updated patch

[Tom Lane]

David Fetter  writes:
> On Fri, Aug 06, 2010 at 03:00:35PM -0400, Tom Lane wrote:
>> Perhaps run through pg_class after restart and flush anything marked
>> relistemp?  Although the ideal solution, probably, would be for temp
>> tables to not have persistent catalog entries in the first place.

> For the upcoming global temp tables, which are visible in other
> sessions, how would this work?

Well, that's a totally different matter.  Those would certainly have
persistent entries, at least for the "master" version.  I don't think
anyone's really figured out what the best implementation looks like;
but maybe there would be per-backend "child" versions that could act
just like the current kind of temp table (and again would ideally not
have any persistent catalog entries).

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] including backend ID in relpath of temp rels - updated patch

[David Fetter]

On Fri, Aug 06, 2010 at 03:00:35PM -0400, Tom Lane wrote:
> Robert Haas  writes:
> > This patch only directly addresses the issue of cleaning up the
> > storage, so there are still the catalog entries to worry about.
> > But it doesn't seem impossible to think about building on this
> > approach to eventually handle that part of the problem better,
> > too.  I haven't thought too much about what that would look like,
> > but I think it could be done.
> 
> Perhaps run through pg_class after restart and flush anything marked
> relistemp?  Although the ideal solution, probably, would be for temp
> tables to not have persistent catalog entries in the first place.

For the upcoming global temp tables, which are visible in other
sessions, how would this work?

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

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

Re: [HACKERS] including backend ID in relpath of temp rels - updated patch

[Tom Lane]

Robert Haas  writes:
> This patch only directly addresses the issue of cleaning up the
> storage, so there are still the catalog entries to worry about.  But
> it doesn't seem impossible to think about building on this approach to
> eventually handle that part of the problem better, too.  I haven't
> thought too much about what that would look like, but I think it could
> be done.

Perhaps run through pg_class after restart and flush anything marked
relistemp?  Although the ideal solution, probably, would be for temp
tables to not have persistent catalog entries in the first place.

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] including backend ID in relpath of temp rels - updated patch

[Tom Lane]

Robert Haas  writes:
> On Fri, Aug 6, 2010 at 2:07 PM, Tom Lane  wrote:
>> Sure, it tops out somewhere, but 32K is way too close to configurations
>> we know work well enough in the field (I've seen multiple reports of
>> people using a couple thousand backends).

> Well, I wouldn't expect anyone to use an exclusive lock for readers
> without a good reason, but you still have n backends that each have to
> read, presumably, about O(n) messages, so eventually that's going to
> start to pinch.

Sure, but I don't see much to be gained from multiple queues either.
There are few (order of zero, in fact) cases where sinval messages
are transmitted that aren't of potential interest to all backends.
Maybe you could do something useful with a very large number of
dynamically-defined queues (like one per relation) ... but managing that
would probably swamp any savings.

> Do you think it's worth worrying about the reduction in the number of
> possible SI message types?

IIRC the number of message types is the number of catalog caches plus
half a dozen or so.  We're a long way from exhausting even a 1-byte
ID field; and we could play more games if we had to, since there would
be a padding byte free in the message types that refer to a catalog
cache.  IOW, 1-byte id doesn't bother me.

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] including backend ID in relpath of temp rels - updated patch

[Robert Haas]

On Fri, Aug 6, 2010 at 2:16 PM, Tom Lane  wrote:
> Jaime Casanova  writes:
>> On Fri, Aug 6, 2010 at 12:50 PM, Robert Haas  wrote:
>>> Just "DROP TABLE pg_temp_2.foo" or whatever and away you go.
>
>> wow! that's true... and certainly a bug...
>
> No, it's not a bug.  You'll find only superusers can do it.
>
>> we shouldn't allow any session to drop other session's temp tables, or
>> is there a reason for this misbehavior?
>
> It is intentional, though I'd be willing to give it up if we had more
> bulletproof crash-cleanup of temp tables --- which is one of the things
> this patch is supposed to result in.

This patch only directly addresses the issue of cleaning up the
storage, so there are still the catalog entries to worry about.  But
it doesn't seem impossible to think about building on this approach to
eventually handle that part of the problem better, too.  I haven't
thought too much about what that would look like, but I think it could
be done.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

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

Re: [HACKERS] Initial review of xslt with no limits patch

[Tom Lane]

Pavel Stehule  writes:
> For Tom: proposed syntax can be used generally - everywhere when you
> are working with collection. It can be used for hash (hstore)
> constructor for example. For me is more readable code like

> select hstore(name := 'Tomas', surname := 'Novak')

You've tried to sell us on that before, with few takers.  This proposed
use-case impresses me even less than the previous ones, because callers
of xslt_process seem quite likely to need to work with non-constant
parameter names.

In any case, given what we have at the moment for function overload
resolution rules, I think it's a fundamentally bad idea to introduce
a "wild card" function type that would necessarily conflict with
practically every other possible function declaration.  So regardless
of what use-cases you propose, I'm going to vote against that.

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

[HACKERS] Surprising dead_tuple_count from pgstattuple

[Gordon Shannon]

This is an expansion of the question I posed in this thread:

http://postgresql.1045698.n5.nabble.com/Need-help-understanding-vacuum-verbose-output-tp2265895p2266912.html

I am framing the question here in relation to pgstattuple.  Running 8.4.4 on
Centos.

I have a table T with 5,063,463 rows.   It was just restored from a backup,
and there is no other activity in this database.  I ran a vacuum.

pg_stat_user_tables.n_dead_tup (which is really
pg_stat_get_dead_tuples('T'::regclass::oid)) says 0
pgstattuple says dead_tuple_count=0, free_space=1,355,152

1. I delete 10,000 rows.

pg_stat_user_tables.n_live_tup -> 5053463
pg_stat_user_tables.n_dead_tup -> 1
pgstattuple.dead_tuple_count -> 1
pgstattuple.free_space -> 1355152

So far, so good. pgstattuple is counting the dead tuples, and not including
those tuples in the free space count.

2. I delete 15,000 more rows.

pg_stat_user_tables.n_live_tup -> 5038463
pg_stat_user_tables.n_dead_tup -> 25000
pgstattuple.dead_tuple_count -> 15000 ??
pgstattuple.free_space -> 1996904 ??

pgstattuple now appears to count the earlier 10K deleted tuples as no longer
dead, but free space.

3. I delete 50,000 more rows.

pg_stat_user_tables.n_live_tup -> 4988463
pg_stat_user_tables.n_dead_tup -> 75000
pgstattuple.dead_tuple_count -> 50022 ??
pgstattuple.free_space -> 2966628 ??

Same thing, pgstattuple appears to "see" only the most recent delete
transaction (but off by 22), and count the prior ones as free.

4. vacuum verbose

vacuum verbose t;
INFO:  vacuuming "public.t"
INFO:  scanned index "t_pkey" to remove 75000 row versions
DETAIL:  CPU 0.01s/0.38u sec elapsed 0.40 sec.
INFO:  "t": removed 75000 row versions in 637 pages
DETAIL:  CPU 0.00s/0.00u sec elapsed 0.00 sec.
INFO:  index "t_pkey" now contains 4988463 row versions in 13886 pages
DETAIL:  75000 index row versions were removed.
204 index pages have been deleted, 0 are currently reusable.
CPU 0.00s/0.00u sec elapsed 0.00 sec.
INFO:  "t": found 50022 removable, 3696 nonremovable row versions in 668 out
of 51958 pages
DETAIL:  0 dead row versions cannot be removed yet.
There were 0 unused item pointers.
0 pages are entirely empty.
CPU 0.01s/0.39u sec elapsed 0.40 sec.
VACUUM
Time: 482.771 ms

It seems relevant that vacuum reports the same incorrect number -- 50022 --
as part of its output.  That makes me think that pgstattuple may be using
similar logic to get its dead tuple count.
I wonder if the key to this is that pgstattuple uses
HeapTupleSatisfiesVisibility() to test for deadness.  If so, why would this
call return apparently false positives?  

I know that pgstattuple is meant to be used for debugging only. I have found
pgstatindex to be very helpful in identifying bloat in my indexes.  
Per Tom in the other thread, I now understand that the "found 50022
removable, 3696 nonremovable" line is referring to the subset of pages
that it scanned looking for dead tuples.

I keep coming back to this, though -- 50,022 seems to be just wrong, or
perhaps simply misleading -- i.e. way too low. 
It's present in the output of vacuum, and the output of pgstattuple.
I'd like to understand what meaning this number has, and, ideally, how I can
use to to detect things like bloat or fragmentation.

Thanks!

-- 
View this message in context: 
http://postgresql.1045698.n5.nabble.com/Surprising-dead-tuple-count-from-pgstattuple-tp2266955p2266955.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

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

Re: [HACKERS] including backend ID in relpath of temp rels - updated patch

[Robert Haas]

On Fri, Aug 6, 2010 at 2:07 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Fri, Aug 6, 2010 at 1:14 PM, Tom Lane  wrote:
>>> Really?  Surely that should be illegal during normal operation. We
>>> might be doing such during crash recovery, but we don't need to
>>> broadcast sinval messages then.
>
>> autovacuum.c does it when we start to worry about XID wraparound, but
>> you can also do it from any normal backend.  Just "DROP TABLE
>> pg_temp_2.foo" or whatever and away you go.
>
> Mph.  I'm still not convinced that the sinval message needs to carry
> backendid.

Hey, if you have an idea... I'd love to get rid of it, too, but I
don't see how to do it.

> But maybe the first-cut solution should just be to squeeze
> the id into the padding area.  You should be able to get up to 65535
> allowed backends, not 32k --- or perhaps use different message type IDs
> for local and global backendid, so that all 65536 bitpatterns are
> allowed for a non-global backendid.
>
>> Well, presumably we'd just represent it as a 1-byte field followed by
>> a 2-byte field, and do a bit of math.  But I don't really see the
>> point.  The whole architecture of a shared invalidation queue is
>> fundamentally non-scalable because it's a broadcast medium.
>
> Sure, it tops out somewhere, but 32K is way too close to configurations
> we know work well enough in the field (I've seen multiple reports of
> people using a couple thousand backends).  In any case, sinval readers
> don't block each other in the current implementation, so I'm really
> dubious that there's any inherent scalability limitation there.  I'll
> hold still for 64K but I think it might be better to go for 2^24.

Well, I wouldn't expect anyone to use an exclusive lock for readers
without a good reason, but you still have n backends that each have to
read, presumably, about O(n) messages, so eventually that's going to
start to pinch.

Do you think it's worth worrying about the reduction in the number of
possible SI message types?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

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

Re: [HACKERS] Cost of AtEOXact_Buffers in --enable-cassert

[Tom Lane]

Andres Freund  writes:
> The most prohibitively expensive part is the AtEOXact_Buffers check of 
> running 
> through all buffers and checking their pin count. And it makes $app's 
> regression tests take thrice their time...

> Would somebody object agains putting those in an extra define so that those 
> can 
> be disabled in pg_config_manual? Or even disable it by default entirely...

Not a chance for the latter; this is an important sanity check that
catches real coding mistakes with some frequency.

I'd be willing to consider a "half assert" mode that turns off some of
the most expensive checks, but AtEOXact_Buffers is hardly the only thing
that ought to be in that list.  The CLOBBER_FREED_MEMORY and memory
context checking stuff is pretty durn expensive too.

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] including backend ID in relpath of temp rels - updated patch

[Tom Lane]

Jaime Casanova  writes:
> On Fri, Aug 6, 2010 at 12:50 PM, Robert Haas  wrote:
>> Just "DROP TABLE pg_temp_2.foo" or whatever and away you go.

> wow! that's true... and certainly a bug...

No, it's not a bug.  You'll find only superusers can do it.

> we shouldn't allow any session to drop other session's temp tables, or
> is there a reason for this misbehavior?

It is intentional, though I'd be willing to give it up if we had more
bulletproof crash-cleanup of temp tables --- which is one of the things
this patch is supposed to result in.

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] Initial review of xslt with no limits patch

[Tom Lane]

Robert Haas  writes:
> Why wouldn't we just pass two text arrays to this function and be done
> with it?

That would work too, although I think it might be a bit harder to use
than one alternating-name-and-value array, at least in some scenarios.
You'd have to be careful that you got the values in the same order in
both arrays, which'd be easy to botch.

There might be other use-cases where two separate arrays are easier
to use, but I'm not seeing one offhand.

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] including backend ID in relpath of temp rels - updated patch

[Jaime Casanova]

On Fri, Aug 6, 2010 at 12:50 PM, Robert Haas  wrote:
>
> Just "DROP TABLE
> pg_temp_2.foo" or whatever and away you go.
>

wow! that's true... and certainly a bug...
we shouldn't allow any session to drop other session's temp tables, or
is there a reason for this misbehavior?

-- 
Jaime Casanova         www.2ndQuadrant.com
Soporte y capacitación de PostgreSQL

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

Re: [HACKERS] Initial review of xslt with no limits patch

[Pavel Stehule]

2010/8/6 Robert Haas :
> On Fri, Aug 6, 2010 at 1:46 PM, Pavel Stehule  wrote:
>> I'll propose a new kind of functions (only position parameter's
>> function). My idea is simple - for functions with this mark the mixed
>> and named notation is blocked. But these functions can have a
>> parameter names - and these names can be passed to function. So there
>> is possible have a
>>
>> xslt_process function with current behave - third argument has not
>> label, and new variadic version like
>>
>> xslt_process(..,.., param_name1 = 'v1', param_name2 = 'v2',
>> param_name3 = 'v3', ...)
>>
>> an implementation of this functionality can be very simple, and we can
>> use this for xslt_process in 9.1
>
> Why wouldn't we just pass two text arrays to this function and be done
> with it?  Custom syntax is all well and good when you're writing these
> calls by hand, but it's not hard to imagine someone wanting to pass in
> values stored somewhere else.

yes, it is possible too. And maybe is better then current
xslt_process. But it isn't too much readable and robust. You have to
calculate position of name and position of value. This is same in
other languages. You can use a dynamic parameters in PHP or Perl via
two arrays, but nobody use it. People use a hash syntax (param1=>val,
param2=>val). This proposal isn't only for xslt_process function. Why
hstore has a custom parser? It can use only a pair of arrays too.

For Tom: proposed syntax can be used generally - everywhere when you
are working with collection. It can be used for hash (hstore)
constructor for example. For me is more readable code like

select hstore(name := 'Tomas', surname := 'Novak')

than

select hstore('name=\'Tomas\', surname=\'Novak\'')

The main advance of this feature is simplicity of custom functions.
Its must not have a custom parser. So possible an using is hstore,
xslt_process

Regards

Pavel Stehule


>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise Postgres Company
>

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

Re: [HACKERS] including backend ID in relpath of temp rels - updated patch

[Tom Lane]

Robert Haas  writes:
> On Fri, Aug 6, 2010 at 1:14 PM, Tom Lane  wrote:
>> Really?  Surely that should be illegal during normal operation. We
>> might be doing such during crash recovery, but we don't need to
>> broadcast sinval messages then.

> autovacuum.c does it when we start to worry about XID wraparound, but
> you can also do it from any normal backend.  Just "DROP TABLE
> pg_temp_2.foo" or whatever and away you go.

Mph.  I'm still not convinced that the sinval message needs to carry
backendid.  But maybe the first-cut solution should just be to squeeze
the id into the padding area.  You should be able to get up to 65535
allowed backends, not 32k --- or perhaps use different message type IDs
for local and global backendid, so that all 65536 bitpatterns are
allowed for a non-global backendid.

> Well, presumably we'd just represent it as a 1-byte field followed by
> a 2-byte field, and do a bit of math.  But I don't really see the
> point.  The whole architecture of a shared invalidation queue is
> fundamentally non-scalable because it's a broadcast medium.

Sure, it tops out somewhere, but 32K is way too close to configurations
we know work well enough in the field (I've seen multiple reports of
people using a couple thousand backends).  In any case, sinval readers
don't block each other in the current implementation, so I'm really
dubious that there's any inherent scalability limitation there.  I'll
hold still for 64K but I think it might be better to go for 2^24.

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] Initial review of xslt with no limits patch

[Tom Lane]

Pavel Stehule  writes:
> 2010/8/6 Tom Lane :
>> I think there are issues here that we need to take a step back and think
>> about.  Right now, thanks to the lack of documentation, we can probably
>> assume there are approximately zero users of the xslt_process parameter
>> feature.  Once we document it that'll no longer be true.  So right now
>> would be the time to reflect on whether this is a specification we
>> actually like or believe is usable; it'll be too late to change it
>> later.

> I know about one important user from Czech Republic

Well, if there actually is anybody who's figured it out, we could easily
have a backwards-compatible mode.  Provide one variadic function that
acts as follows:
even number of variadic array elements -> they're names/values
one variadic array element -> parse it the old way
otherwise -> error

I wouldn't even bother with fixing the MAXPARAMS limitation for the
"old way" code, just make it work exactly as before.

> I'll propose a new kind of functions (only position parameter's
> function). My idea is simple - for functions with this mark the mixed
> and named notation is blocked.

We don't need random new function behaviors for this.  Anyway your
proposal doesn't work at all for non-constant parameter names.

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

[HACKERS] Cost of AtEOXact_Buffers in --enable-cassert

[Andres Freund]

Hi,

I do test (and even run) some stuff running with cassert enabled. For many 
things the reliability and performance is ok enough and its useful, especially 
if you have your own c functions and such.
Imho thats useful as it makes catching some bugs more likely...

The most prohibitively expensive part is the AtEOXact_Buffers check of running 
through all buffers and checking their pin count. And it makes $app's 
regression tests take thrice their time...

Would somebody object agains putting those in an extra define so that those can 
be disabled in pg_config_manual? Or even disable it by default entirely...

Andres

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

Re: [HACKERS] including backend ID in relpath of temp rels - updated patch

[Robert Haas]

On Fri, Aug 6, 2010 at 1:14 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Thu, Aug 5, 2010 at 7:05 PM, Tom Lane  wrote:
>>> One thing that I find rather distressing about this is the 25% bloat
>>> in sizeof(SharedInvalidationMessage).  Couldn't we avoid that?  Is it
>>> really necessary to *ever* send an SI message for a backend-local rel?
>
>> It can be dropped or unlinked by another backend, so, yes.
>
> Really?  Surely that should be illegal during normal operation. We
> might be doing such during crash recovery, but we don't need to
> broadcast sinval messages then.

autovacuum.c does it when we start to worry about XID wraparound, but
you can also do it from any normal backend.  Just "DROP TABLE
pg_temp_2.foo" or whatever and away you go.

> It might be sufficient to consider that there are "local" and "global"
> smgr inval messages, where the former never get out of the generating
> backend, so a bool is enough in the message struct.

It would be nice to be able to do it that way, but I don't believe
it's the case, per the above.

>> had was that if we could count on the backend ID to fit into an int16
>> we could fit it in to what's currently padding space.  That would
>> require rather dramatically lowering the maximum number of backends
>> (currently INT_MAX/4), but it's a little hard to imagine that we can
>> really support more than 32,767 simultaneous backends anyway.
>
> Yeah, that occurred to me too.  A further thought is that the id field
> could probably be reduced to 1 byte, leaving 3 for backendid, which
> would certainly be plenty.  However representing that in a portable
> struct declaration would be a bit painful I fear.

Well, presumably we'd just represent it as a 1-byte field followed by
a 2-byte field, and do a bit of math.  But I don't really see the
point.  The whole architecture of a shared invalidation queue is
fundamentally non-scalable because it's a broadcast medium.  If we
wanted to efficiently support even thousands of backends (let alone
tens or hundreds of thousands) I assume we would need to rearchitect
this completely with more fine-grained queues, and have backends
subscribe to the queues pertaining to the objects they want to access
before touching them.  Or maybe something else entirely. But I don't
think broadcasting to 30,000 backends is going to work for the same
reason that plugging 30,000 machines into an Ethernet *hub* doesn't
work.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

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

Re: [HACKERS] Initial review of xslt with no limits patch

[Pavel Stehule]

2010/8/6 Tom Lane :
> Andrew Dunstan  writes:
>> On 08/06/2010 12:15 PM, Tom Lane wrote:
>>> Some examination of
>>> http://www.xmlsoft.org/XSLT/tutorial/libxslttutorial.html
>>> suggests that the parameter values need to be single-quoted,
>>> and indeed when I change the last part of your example to
>>>
>>> 'n1=''v1'',n2=''v2'',n3=''v3'',n4=''v4'',n5=''v5'''::text);
>
>> Which would look a whole lot nicer with dollar quoting ;-)
>
> No doubt.  But one would assume that constant parameters aren't going
> to be the normal use-case, and dollar quoting isn't helpful for
> nonconstant text.
>
> I think there are issues here that we need to take a step back and think
> about.  Right now, thanks to the lack of documentation, we can probably
> assume there are approximately zero users of the xslt_process parameter
> feature.  Once we document it that'll no longer be true.  So right now
> would be the time to reflect on whether this is a specification we
> actually like or believe is usable; it'll be too late to change it
> later.
>

I know about one important user from Czech Republic


> There are two specific points bothering me now that I see how it works:
>
> 1. name = value pretty much sucks, especially with the 100% lack of any
> quoting convention for either equals or comma.  I concur with the
> thoughts upthread that turning this into a variadic function would be a
> more sensible solution.

I'll propose a new kind of functions (only position parameter's
function). My idea is simple - for functions with this mark the mixed
and named notation is blocked. But these functions can have a
parameter names - and these names can be passed to function. So there
is possible have a

xslt_process function with current behave - third argument has not
label, and new variadic version like

xslt_process(..,.., param_name1 = 'v1', param_name2 = 'v2',
param_name3 = 'v3', ...)

an implementation of this functionality can be very simple, and we can
use this for xslt_process in 9.1

Regards

Pavel Stehule


>
> 2. I'm not sure whether we ought to auto-single-quote the values.
> If we don't, how hard is it for users to properly quote nonconstant
> parameter values?  (Will quote_literal work, or are the quoting rules
> different for libxslt?)  If we do, are we giving up functionality
> someone cares about?
>
>                        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] including backend ID in relpath of temp rels - updated patch

[Tom Lane]

Robert Haas  writes:
> On Thu, Aug 5, 2010 at 7:05 PM, Tom Lane  wrote:
>> One thing that I find rather distressing about this is the 25% bloat
>> in sizeof(SharedInvalidationMessage).  Couldn't we avoid that?  Is it
>> really necessary to *ever* send an SI message for a backend-local rel?

> It can be dropped or unlinked by another backend, so, yes.

Really?  Surely that should be illegal during normal operation.  We
might be doing such during crash recovery, but we don't need to
broadcast sinval messages then.

It might be sufficient to consider that there are "local" and "global"
smgr inval messages, where the former never get out of the generating
backend, so a bool is enough in the message struct.

> had was that if we could count on the backend ID to fit into an int16
> we could fit it in to what's currently padding space.  That would
> require rather dramatically lowering the maximum number of backends
> (currently INT_MAX/4), but it's a little hard to imagine that we can
> really support more than 32,767 simultaneous backends anyway.

Yeah, that occurred to me too.  A further thought is that the id field
could probably be reduced to 1 byte, leaving 3 for backendid, which
would certainly be plenty.  However representing that in a portable
struct declaration would be a bit painful I fear.

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] Initial review of xslt with no limits patch

[Tom Lane]

Andrew Dunstan  writes:
> On 08/06/2010 12:15 PM, Tom Lane wrote:
>> Some examination of
>> http://www.xmlsoft.org/XSLT/tutorial/libxslttutorial.html
>> suggests that the parameter values need to be single-quoted,
>> and indeed when I change the last part of your example to
>> 
>> 'n1=''v1'',n2=''v2'',n3=''v3'',n4=''v4'',n5=''v5'''::text);

> Which would look a whole lot nicer with dollar quoting ;-)

No doubt.  But one would assume that constant parameters aren't going
to be the normal use-case, and dollar quoting isn't helpful for
nonconstant text.

I think there are issues here that we need to take a step back and think
about.  Right now, thanks to the lack of documentation, we can probably
assume there are approximately zero users of the xslt_process parameter
feature.  Once we document it that'll no longer be true.  So right now
would be the time to reflect on whether this is a specification we
actually like or believe is usable; it'll be too late to change it
later.

There are two specific points bothering me now that I see how it works:

1. name = value pretty much sucks, especially with the 100% lack of any
quoting convention for either equals or comma.  I concur with the
thoughts upthread that turning this into a variadic function would be a
more sensible solution.

2. I'm not sure whether we ought to auto-single-quote the values.
If we don't, how hard is it for users to properly quote nonconstant
parameter values?  (Will quote_literal work, or are the quoting rules
different for libxslt?)  If we do, are we giving up functionality
someone cares about?

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] including backend ID in relpath of temp rels - updated patch

[Robert Haas]

On Thu, Aug 5, 2010 at 7:05 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> [ BackendRelFileNode patch ]
>
> One thing that I find rather distressing about this is the 25% bloat
> in sizeof(SharedInvalidationMessage).  Couldn't we avoid that?  Is it
> really necessary to *ever* send an SI message for a backend-local rel?

It can be dropped or unlinked by another backend, so, yes.

> I agree that one needs to send relcache inval sometimes for temp rels,
> but I don't see why each backend couldn't interpret that as a flush
> on its own local version.

The problem is that receipt of the inval message results in a call to
smgrclosenode(), which has to look up the (Backend)RelFileNode in
SMgrRelationHash.  If the backend ID isn't present, we can't search
the hash table efficiently.  This is not only a problem for
smgrclosenode(), either; that hash is used in a bunch of places, so
changing the hash key isn't really an option.  One possibility would
be to have two separate shared-invalidation message types for smgr.
One would indicate that a non-temporary relation was flushed, so the
backend ID field is known to be -1 and we can search the hash quickly.
 The other would indicate that a temporary relation was flushed and
you'd have to scan the whole hash table to find and flush all matching
entries.  That still doesn't sound great, though.  Another thought I
had was that if we could count on the backend ID to fit into an int16
we could fit it in to what's currently padding space.  That would
require rather dramatically lowering the maximum number of backends
(currently INT_MAX/4), but it's a little hard to imagine that we can
really support more than 32,767 simultaneous backends anyway.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

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

Re: [HACKERS] Initial review of xslt with no limits patch

[Andrew Dunstan]

On 08/06/2010 12:15 PM, Tom Lane wrote:


Some examination of
http://www.xmlsoft.org/XSLT/tutorial/libxslttutorial.html
suggests that the parameter values need to be single-quoted,
and indeed when I change the last part of your example to

'n1=''v1'',n2=''v2'',n3=''v3'',n4=''v4'',n5=''v5'''::text);




Which would look a whole lot nicer with dollar quoting ;-)

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] Initial review of xslt with no limits patch

[Pavel Stehule]

Hello

attached updated patch with regression test



2010/8/6 Tom Lane :
> Mike Fowler  writes:
>> SELECT
>> xslt_process( ... , ... ,
>>              'n1=v1,n2=v2,n3=v3,n4=v4,n5=v5'::text)
>
> produces
>
>> 
>>    v1
>>    v2
>>    v3
>>    v4
>>    v5
>> 
>
>> Sadly I get the following in both versions:
>
>> 
>>    
>>    
>>    
>>    
>>    
>> 
>
> Some examination of
> http://www.xmlsoft.org/XSLT/tutorial/libxslttutorial.html
> suggests that the parameter values need to be single-quoted,
> and indeed when I change the last part of your example to
>
>        'n1=''v1'',n2=''v2'',n3=''v3'',n4=''v4'',n5=''v5'''::text);
>
> I get
>
>     xslt_process
> ---
>              +
>   v1+
>   v2+
>   v3+
>   v4+
>   v5+
>             +
>
> (1 row)
>
> So this seems to be a documentation problem more than a code problem.
>
> (It's a bit distressing to notice that the regression tests for the
> module fail to exercise 3-parameter xslt_process at all, though.)
>

??? I don't see it

Regards

Pavel Stehule

>                        regards, tom lane
>
*** ./contrib/xml2/expected/xml2.out.orig	2010-02-28 22:31:57.0 +0100
--- ./contrib/xml2/expected/xml2.out	2010-08-06 18:46:41.0 +0200
***
*** 145,147 
--- 145,215 
  Value');
  create index idx_xpath on t1 ( xpath_string
  ('/attributes/attribu...@name="attr_1"]/text()', xml_data::text));
+ SELECT xslt_process('cim30400'::text, $$http://www.w3.org/1999/XSL/Transform"; version="1.0">
+   
+   
+   
+   
+   
+   
+   
+   
+ 
+   
+ 
+   
+   
+ 
+   
+   
+ 
+   
+   
+ 
+   
+   
+ 
+   
+   
+ 
+   
+   
+ 
+   
+   
+ 
+   
+   
+ 
+   
+   
+ 
+   
+   
+ 
+   
+   
+ 
+   
+ 
+   
+ $$::text, 'n1="v1",n2="v2",n3="v3",n4="v4",n5="v5",n6="v6",n7="v7",n8="v8",n9="v9",n10="v10",n11="v11",n12="v12"'::text);
+   xslt_process  
+ 
+   +
+v1 +
+v2 +
+v3 +
+v4 +
+v5 +
+v6 +
+v7 +
+v8 +
+v9 +
+v10+
+v11+
+v12+
+  +
+  
+ (1 row)
+ 
*** ./contrib/xml2/sql/xml2.sql.orig	2010-08-06 18:30:00.0 +0200
--- ./contrib/xml2/sql/xml2.sql	2010-08-06 18:30:57.0 +0200
***
*** 80,82 
--- 80,132 
  
  create index idx_xpath on t1 ( xpath_string
  ('/attributes/attribu...@name="attr_1"]/text()', xml_data::text));
+ 
+ SELECT xslt_process('cim30400'::text, $$http://www.w3.org/1999/XSL/Transform"; version="1.0">
+   
+   
+   
+   
+   
+   
+   
+   
+ 
+   
+ 
+   
+   
+ 
+   
+   
+ 
+   
+   
+ 
+   
+   
+ 
+   
+   
+ 
+   
+   
+ 
+   
+   
+ 
+   
+   
+ 
+   
+   
+ 
+   
+   
+ 
+   
+   
+ 
+   
+ 
+   
+ $$::text, 'n1="v1",n2="v2",n3="v3",n4="v4",n5="v5",n6="v6",n7="v7",n8="v8",n9="v9",n10="v10",n11="v11",n12="v12"'::text);
*** ./contrib/xml2/xslt_proc.c.orig	2010-07-06 21:18:55.0 +0200
--- ./contrib/xml2/xslt_proc.c	2010-08-06 18:39:02.0 +0200
***
*** 41,49 
  extern void pgxml_parser_init(void);
  
  /* local defs */
! static void parse_params(const char **params, text *paramstr);
  
! #define MAXPARAMS 20			/* must be even, see parse_params() */
  #endif   /* USE_LIBXSLT */
  
  
--- 41,50 
  extern void pgxml_parser_init(void);
  
  /* local defs */
! const char **parse_params(text *paramstr);
  
! #define INIT_PARAMS 20			/* must be even, see parse_params() */
! #define EXTEND_PARAMS 20		/* must be even, see parse_params() */
  #endif   /* USE_LIBXSLT */
  
  
***
*** 57,63 
  	text	   *doct = PG_GETARG_TEXT_P(0);
  	text	   *ssheet = PG_GETARG_TEXT_P(1);
  	text	   *paramstr;
! 	const char *params[MAXPARAMS + 1];	/* +1 for the terminator */
  	xsltStylesheetPtr stylesheet = NULL;
  	xmlDocPtr	doctree;
  	xmlDocPtr	restree;
--- 58,64 
  	text	   *doct = PG_GETARG_TEXT_P(0);
  	text	   *ssheet = PG_GETARG_TEXT_P(1);
  	text	   *paramstr;
! 	const char **params;
  	xsltStylesheetPtr stylesheet = NULL;
  	xmlDocPtr	doctree;
  	xmlDocPtr	restree;
***
*** 69,79 
  	if (fcinfo->nargs == 3)
  	{
  		paramstr = PG_GETARG_TEXT_P(2);
! 		parse_params(params, paramstr);
  	}
  	else
  		/* No parameters */
  		params[0] = NULL;
  
  	/* Setup parser */
  	pgxml_parser_init();
--- 70,83 
  	if (fcinfo->nargs == 3)
  	{
  		paramstr = PG_GETARG_TEXT_P(2);
! 		params = parse_params(paramstr);
  	}
  	else
+ 	{
  		/* No parameters */
+ 		params = palloc(sizeof(char *));
  		params[0] = NULL;
+ 	}
  
  	/* Setup parser */
  	pgxml_parser_init();
***
*** 139,160 
  
  #ifdef USE_LIBXSLT
  
! static void
! parse_pa

Re: [HACKERS] refactoring comment.c

[Robert Haas]

On Fri, Aug 6, 2010 at 12:26 PM, Simon Riggs  wrote:
> I understand the concept and it seems like it might work. Not too keen
> on pretending a noun is a verb. That leads to erroring.
>
>  SECURITY LABEL? verb = CREATE, ADD, ...
>
> Can't objects have more than one label?
>
> How will you set default security labels on objects?
>
> Where do you define labels?
>
> Will there be a new privilege to define this? Presumably object owners
> would not be able to set that themselves, otherwise you could create an
> object, add a security label to it and then use it to see other things
> at that level.

Uh, these are all good questions, but I think they'd be more
appropriate on the thread about the security label patch, to which I
linked in my previous email.  Many of them have already been discussed
there.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

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

Re: [HACKERS] refactoring comment.c

[Simon Riggs]

On Fri, 2010-08-06 at 11:02 -0400, Robert Haas wrote:
> At PGCon, we discussed the possibility that a minimal SE-PostgreSQL
> implementation would need little more than a hook in
> ExecCheckRTPerms() [which we've since added] and a security label
> facility [for which KaiGai has submitted a patch].  I actually sat
> down to write the security label patch myself while we were in Ottawa,
> but quickly ran into difficulties: while the hook we have now can't do
> anything useful with objects other than relations, it's pretty clear
> from previous discussions on this topic that the demand for labels on
> other kinds of objects is not going to go away.  Rather than adding
> additional syntax to every object type in the system (some of which
> don't even have ALTER commands at present), I suggested basing the
> syntax on the existing COMMENT syntax.  After some discussion[1], we
> seem to have settled on the following:
> 
> SECURITY LABEL [ FOR  ] ON   IS 
> '';

I understand the concept and it seems like it might work. Not too keen
on pretending a noun is a verb. That leads to erroring.

 SECURITY LABEL? verb = CREATE, ADD, ...

Can't objects have more than one label?

How will you set default security labels on objects?

Where do you define labels?

Will there be a new privilege to define this? Presumably object owners
would not be able to set that themselves, otherwise you could create an
object, add a security label to it and then use it to see other things
at that level.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Development, 24x7 Support, Training and Services


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

Re: [HACKERS] Initial review of xslt with no limits patch

[Tom Lane]

Mike Fowler  writes:
> SELECT 
> xslt_process( ... , ... ,
>  'n1=v1,n2=v2,n3=v3,n4=v4,n5=v5'::text)

produces

> 
>v1
>v2
>v3
>v4
>v5
> 

> Sadly I get the following in both versions:

> 
>
>
>
>
>
> 

Some examination of
http://www.xmlsoft.org/XSLT/tutorial/libxslttutorial.html
suggests that the parameter values need to be single-quoted,
and indeed when I change the last part of your example to

'n1=''v1'',n2=''v2'',n3=''v3'',n4=''v4'',n5=''v5'''::text);

I get

 xslt_process  
---
 +
   v1+
   v2+
   v3+
   v4+
   v5+
+
 
(1 row)

So this seems to be a documentation problem more than a code problem.

(It's a bit distressing to notice that the regression tests for the
module fail to exercise 3-parameter xslt_process at all, though.)

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] refactoring comment.c

[Robert Haas]

On Fri, Aug 6, 2010 at 11:36 AM, Alvaro Herrera
 wrote:
> Excerpts from Robert Haas's message of vie ago 06 11:02:58 -0400 2010:
>
>> Any comments?  (ha ha ha...)
>
> Interesting idea.  The patch looks fine on a quick once-over.

Thanks for taking a look.

> Two small
> things: this comment
>
> +    /*
> +     * Databases, tablespaces, and roles are cluster-wide objects, so any
> +     * comments on those objects are record in the shared pg_shdescription
> +     * catalog.  Comments on all other objects are recorded in 
> pg_description.
> +     */
>
> says "record" where it should say "recorded".

Thanks, good eye.

> Also, it strikes me that perhaps the ObjectAddress struct definition
> should be moved to the new header file; seems a more natural place for
> it (but then, it seems like a lot of files will need to include the new
> header, so perhaps that should be left to a subsequent patch).

I thought about that, but erred on the side of being conservative and
didn't move it.  I like the idea, though.

> Thirdly, is it just me or just could replace a lot of code in DropFoo
> functions with this new auxiliary code?  Seems it's just missing
> "missing_ok" support ...

I am not sure how much code it would save you at the level of the
individual DropFoo() functions; I'd have to look through them more
carefully.  But now that you mention it, what about getting rid of all
of the individual parse nodes for drop statements?  Right now we have:

DropTableSpaceStmt
DropFdwStmt
DropForeignServerStmt
DropUserMappingStmt
DropPLangStmt
DropRoleStmt
DropStmt (table, sequence, view, index, domain, conversion, schema,
text search {parser, dictionary, template, configuration}
DropPropertyStmt (rules and triggers)
DropdbStmt (capitalized differently, just for fun)
DropCastStmt
RemoveFuncStmt (so you can't find it by grepping for Drop!)
RemoveOpClassStmt
RemoveOpFamilyStmt

It seems like we could probably unify all of these into a single
DropStmt, following the same pattern as CommentStmt, although I think
perhaps that should be a follow-on patch rather than doing it as part
of this one.  GRANT also has some code to translate object names to
OIDs, which I thought might be able to use this machinery as well,
though I haven't really checked whether it makes sense.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

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

Re: [HACKERS] refactoring comment.c

[Alvaro Herrera]

Excerpts from Robert Haas's message of vie ago 06 11:02:58 -0400 2010:

> Any comments?  (ha ha ha...)

Interesting idea.  The patch looks fine on a quick once-over.  Two small
things: this comment

+/*
+ * Databases, tablespaces, and roles are cluster-wide objects, so any
+ * comments on those objects are record in the shared pg_shdescription
+ * catalog.  Comments on all other objects are recorded in pg_description.
+ */

says "record" where it should say "recorded".

Also, it strikes me that perhaps the ObjectAddress struct definition
should be moved to the new header file; seems a more natural place for
it (but then, it seems like a lot of files will need to include the new
header, so perhaps that should be left to a subsequent patch).

Thirdly, is it just me or just could replace a lot of code in DropFoo
functions with this new auxiliary code?  Seems it's just missing
"missing_ok" support ...

-- 
Álvaro Herrera 
The PostgreSQL Company - Command Prompt, Inc.
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] [ADMIN] postgres 9.0 crash when bringing up hot standby

[Alanoly Andrews]

Thanks. Yes, the LOAD command does work, on another database cluster on the 
same AIX machine.

-Original Message-
From: Fujii Masao [mailto:masao.fu...@gmail.com] 
Sent: Friday, August 06, 2010 10:31 AM
To: Alanoly Andrews
Cc: pgsql-ad...@postgresql.org; PostgreSQL-development
Subject: Re: [ADMIN] postgres 9.0 crash when bringing up hot standby

On Fri, Aug 6, 2010 at 10:10 PM, Alanoly Andrews  wrote:
> I'm testing "hot standby" using "streaming WAL records". On trying to bring
> up the hot standby, I see the following error in the log:

Thanks for the report!

> LOG:  database system was interrupted; last known up at 2010-08-05 14:46:36
> LOG:  entering standby mode
> LOG:  restored log file "00010007" from archive
> LOG:  redo starts at 0/720
> LOG:  consistent recovery state reached at 0/800
> LOG:  database system is ready to accept read only connections
> cp: /pgarclog/pg1/00010008: A file or directory in the path
> name does not exist.
> LOG:  WAL receiver process (PID 1073206) was terminated by signal 11
> LOG:  terminating any other active server processes
>
> There is a core dump. The debugger indicates the crash sequence as follows:
>
> (dbx) where
> _alloc_initial_pthread(??) at 0x949567c
> __pth_init(??) at 0x9493ba4
> uload(??, ??, ??, ??, ??, ??, ??, ??) at 0x9fff0001954
> load_64.load(??, ??, ??) at 0x904686c
> loadAndInit() at 0x947bd7c
> dlopen(??, ??) at 0x911cc4c
> internal_load_library(libname =
> "/apps/pg_9.0_b4/lib/postgresql/libpqwalreceiver.so"), line 234 in "dfmgr.c"
> load_file(filename = "libpqwalreceiver", restricted = '\0'), line 156 in
> "dfmgr.c"
> WalReceiverMain(), line 248 in "walreceiver.c"
> AuxiliaryProcessMain(argc = 2, argv = 0x0fffa8b8), line 428 in
> "bootstrap.c"
> StartChildProcess(type = WalReceiverProcess), line 4405 in "postmaster.c"
> sigusr1_handler(postgres_signal_arg = 30), line 4227 in "postmaster.c"
> __fd_select(??, ??, ??, ??, ??) at 0x911805c
> postmaster.select(__fds = 5, __readlist = 0x0fffd0a8, __writelist =
> (nil), __exceptlist = (nil), __timeout = 0x00c0), line 229 in
> "time.h"
> unnamed block in ServerLoop(), line 1391 in "postmaster.c"
> unnamed block in ServerLoop(), line 1391 in "postmaster.c"
> ServerLoop(), line 1391 in "postmaster.c"
> PostmasterMain(argc = 1, argv = 0x0001102aa4b0), line 1092 in
> "postmaster.c"
> main(argc = 1, argv = 0x0001102aa4b0), line 188 in "main.c"
>
> Any pointers on how to resolve the issue will be much appreciated.

Sorry, I have no idea what's wrong :(

Is the simple LOAD command successful on your AIX?

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

This e-mail may be privileged and/or confidential, and the sender does not 
waive any related rights and obligations. Any distribution, use or copying of 
this e-mail or the information it contains by other than an intended recipient 
is unauthorized. If you received this e-mail in error, please advise me (by 
return e-mail or otherwise) immediately.
 
Ce courriel est confidentiel et protégé. L'expéditeur ne renonce pas aux droits 
et obligations qui s'y rapportent. Toute diffusion, utilisation ou copie de ce 
message ou des renseignements qu'il contient par une personne autre que le 
(les) destinataire(s) désigné(s) est interdite. Si vous recevez ce courriel par 
erreur, veuillez m'en aviser immédiatement, par retour de courriel ou par un 
autre moyen.



-- 
Sent 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: Row-level Locks & SERIALIZABLE transactions, postgres vs. Oracle

[Florian Pflug]

On Aug3, 2010, at 00:43 , Florian Pflug wrote:
> Sounds good. That'll also give me some time to test the RI trigger
> infrastructure now that I've removed the crosscheck code.

Ok, I've found some time do run some additional tests.

I've created a small test suite to compare the behaviour of native (cascading) 
FK constraints to an PLPGSQL implementation. There is a dedicated child table 
(native_child respectively plpgsql_child) for each of the two FK 
implementation. Into both child rows for random parents are inserted, creating 
the parent if it does not already exists. Concurrently, random parent rows are 
deleted.

The number of successful inserts into native_child respectively plpgsql_child 
and the number of successfull deletes are counted, as well as the number of 
transactions failed due to either a serialization failure or a race condition 
during the insert (unique_violation or foreign_key_violation).

To verify that things behave as expected, the test suite reports a histogram of 
the number of parents over the child count after the test completes. The 
expected distribution is output alongside that histogram, assuming that the 
number of parents with N children follows an exponential distribution.

I've pushed the test suite to
 g...@github.com:fgp/fk_concurrency.git
and put a summary of the results of my test runs on
 http://wiki.github.com/fgp/fk_concurrency/results.

The results look good. There is no significant change in the behaviour of FKs 
between HEAD and HEAD+serializable_row_locks, and no significant difference 
between the native and plpgsql implementation. As expected, the plpgsql 
implementation fails on an unpatched HEAD, aborting with the error "database 
inconsistent" pretty quickly.

I'd be interesting to run the plpgsql implementation with the SHARE locking 
removed against HEAD+true_serializability to see if that changes the number of 
serialization_failures that occur. I'll do that, but I'll have to wait a bit 
for another spare time window to open.

best regards,
Florian Pflug


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

Re: [HACKERS] default of max_stack_depth

[Tom Lane]

Fujii Masao  writes:
> On Fri, Aug 6, 2010 at 11:02 PM, Tom Lane  wrote:
>> The initial value needs to be small until we have been able to probe
>> rlimit and figure out what is safe.

> Thanks! How about adding the comment about that as follows?

I added this:


/*
 * We use the hopefully-safely-small value of 100kB as the compiled-in
 * default for max_stack_depth.  InitializeGUCOptions will increase it if
 * possible, depending on the actual platform-specific stack limit.
 */

although I don't entirely see the point.  We are certainly not going to
comment every variable whose compiled-in default gets changed by later
processing.

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] CommitFest 2010-07 week three progress report

[Kevin Grittner]

"Kevin Grittner"  wrote:
 
> With only ten days to go, in order to leave time for committers to
> do their thing, we need to be wrapping up the remaining patches. 
> I think we look pretty good.  Of the remaining six patches, two
> are Work in Progress, so are not expected to go to a committer;
> three involve a committer, so I figure they can decide when and if
> it's time to return or move them, which just leaves one which is
> down to tweaking docs.
 
How embarrassing -- I miscounted.  It appears I counted Peter's WiP
patch in two categories and missed counting the "unlimited
parameters for xslt_process" in the above paragraph.  (An omission
which jumped out at me when reading this morning's posts.)  Mike
Fowler's latest post says "neither the existing code or the patched
code appear able to evaluate the parameters."  Is it time to mark
this "Returned with Feedback" and hope for a new patch in the next
CF?
 
-Kevin

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

Re: [HACKERS] [ADMIN] postgres 9.0 crash when bringing up hot standby

[Fujii Masao]

On Fri, Aug 6, 2010 at 10:10 PM, Alanoly Andrews  wrote:
> I’m testing “hot standby” using “streaming WAL records”. On trying to bring
> up the hot standby, I see the following error in the log:

Thanks for the report!

> LOG:  database system was interrupted; last known up at 2010-08-05 14:46:36
> LOG:  entering standby mode
> LOG:  restored log file "00010007" from archive
> LOG:  redo starts at 0/720
> LOG:  consistent recovery state reached at 0/800
> LOG:  database system is ready to accept read only connections
> cp: /pgarclog/pg1/00010008: A file or directory in the path
> name does not exist.
> LOG:  WAL receiver process (PID 1073206) was terminated by signal 11
> LOG:  terminating any other active server processes
>
> There is a core dump. The debugger indicates the crash sequence as follows:
>
> (dbx) where
> _alloc_initial_pthread(??) at 0x949567c
> __pth_init(??) at 0x9493ba4
> uload(??, ??, ??, ??, ??, ??, ??, ??) at 0x9fff0001954
> load_64.load(??, ??, ??) at 0x904686c
> loadAndInit() at 0x947bd7c
> dlopen(??, ??) at 0x911cc4c
> internal_load_library(libname =
> "/apps/pg_9.0_b4/lib/postgresql/libpqwalreceiver.so"), line 234 in "dfmgr.c"
> load_file(filename = "libpqwalreceiver", restricted = '\0'), line 156 in
> "dfmgr.c"
> WalReceiverMain(), line 248 in "walreceiver.c"
> AuxiliaryProcessMain(argc = 2, argv = 0x0fffa8b8), line 428 in
> "bootstrap.c"
> StartChildProcess(type = WalReceiverProcess), line 4405 in "postmaster.c"
> sigusr1_handler(postgres_signal_arg = 30), line 4227 in "postmaster.c"
> __fd_select(??, ??, ??, ??, ??) at 0x911805c
> postmaster.select(__fds = 5, __readlist = 0x0fffd0a8, __writelist =
> (nil), __exceptlist = (nil), __timeout = 0x00c0), line 229 in
> "time.h"
> unnamed block in ServerLoop(), line 1391 in "postmaster.c"
> unnamed block in ServerLoop(), line 1391 in "postmaster.c"
> ServerLoop(), line 1391 in "postmaster.c"
> PostmasterMain(argc = 1, argv = 0x0001102aa4b0), line 1092 in
> "postmaster.c"
> main(argc = 1, argv = 0x0001102aa4b0), line 188 in "main.c"
>
> Any pointers on how to resolve the issue will be much appreciated.

Sorry, I have no idea what's wrong :(

Is the simple LOAD command successful on your AIX?

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] default of max_stack_depth

[Fujii Masao]

On Fri, Aug 6, 2010 at 11:02 PM, Tom Lane  wrote:
> Fujii Masao  writes:
>> The document says that the max_stack_depth is 2MB by default.
>> OTOH, in the source code, the variable max_stack_depth is
>> initialized with 100 (kB), and guc.c also uses 100 as the
>> default. Why?
>
> The initial value needs to be small until we have been able to probe
> rlimit and figure out what is safe.

Thanks! How about adding the comment about that as follows?

*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***
*** 1520,1525  static struct config_int ConfigureNamesInt[] =
--- 1520,1531 
16384, 1024, MAX_KILOBYTES, NULL, NULL
},

+   /*
+* Note: the real default of max_stack_depth is calculated in
+* InitializeGUCOptions(). We use 100 as the sufficiently small
+* initial value until we have been able to probe rlimit and
+* figure out what is safe.
+*/
{
{"max_stack_depth", PGC_SUSET, RESOURCES_MEM,
gettext_noop("Sets the maximum stack depth, in
kilobytes."),

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] Initial review of xslt with no limits patch

[Mike Fowler]

On 06/08/10 15:08, Andrew Dunstan wrote:



On 08/06/2010 02:29 AM, Pavel Stehule wrote:

2010/8/6 David Fetter:

On Fri, Aug 06, 2010 at 05:57:37AM +0200, Pavel Stehule wrote:

2010/8/6 Andrew Dunstan:

On 08/05/2010 06:56 PM, Mike Fowler wrote:

SELECT
xslt_process('cim30400'::text, 


$$http://www.w3.org/1999/XSL/Transform";
version="1.0">



[snip]

$$::text, 'n1=v1,n2=v2,n3=v3,n4=v4,n5=v5'::text)
I haven't been paying attention to this, so sorry if this has been 
discussed
before, but it just caught my eye. Why are we passing these params 
as a

comma-separated list rather than as an array or as a variadic list of
params? This looks rather ugly. What if you want to have a param that
includes a comma?

There is probably problem in pairs - label = value. Can be nice, if we
can use a variadic functions for this, but I am afraid, ...

using a variadic function isn't too much nice now

some xslt_process(xmlsrc, 'n1=v1','n2=v2','n3=v3'

This sounds like the perfect case for pulling hstore into core code. :)

I afraid so integration of hstore can break and block work on real
hash support. I would to have hash tables in core, but with usual
features and usual syntax - like Perl or PHP



Can we just keep this discussion within reasonable bounds? The issue 
is not hstore or other hashes, but how to do the param list for xslt 
sanely given what we have now. A variadic list will be much nicer than 
what is currently proposed.


cheers

andrew


+1

Variadic seems the most sensible to me anyways.

However the more urgent problem is, pending someone spotting an error in 
my ways, neither the existing code or the patched code appear able to 
evaluate the parameters. Note than in my example I supplied a default 
value for the fifth parameter and not even that value is appearing in 
the outputted XML.


Regards,

--
Mike Fowler
Registered Linux user: 379787


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

Re: [HACKERS] Initial review of xslt with no limits patch

[Andrew Dunstan]

On 08/06/2010 02:29 AM, Pavel Stehule wrote:

2010/8/6 David Fetter:

On Fri, Aug 06, 2010 at 05:57:37AM +0200, Pavel Stehule wrote:

2010/8/6 Andrew Dunstan:

On 08/05/2010 06:56 PM, Mike Fowler wrote:

SELECT
xslt_process('cim30400'::text,
$$http://www.w3.org/1999/XSL/Transform";
version="1.0">



[snip]

$$::text, 'n1=v1,n2=v2,n3=v3,n4=v4,n5=v5'::text)

I haven't been paying attention to this, so sorry if this has been discussed
before, but it just caught my eye. Why are we passing these params as a
comma-separated list rather than as an array or as a variadic list of
params? This looks rather ugly. What if you want to have a param that
includes a comma?

There is probably problem in pairs - label = value. Can be nice, if we
can use a variadic functions for this, but I am afraid, ...

using a variadic function isn't too much nice now

some xslt_process(xmlsrc, 'n1=v1','n2=v2','n3=v3'

This sounds like the perfect case for pulling hstore into core code. :)

I afraid so integration of hstore can break and block work on real
hash support. I would to have hash tables in core, but with usual
features and usual syntax - like Perl or PHP



Can we just keep this discussion within reasonable bounds? The issue is 
not hstore or other hashes, but how to do the param list for xslt sanely 
given what we have now. A variadic list will be much nicer than what is 
currently proposed.


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] default of max_stack_depth

[Tom Lane]

Fujii Masao  writes:
> The document says that the max_stack_depth is 2MB by default.
> OTOH, in the source code, the variable max_stack_depth is
> initialized with 100 (kB), and guc.c also uses 100 as the
> default. Why?

The initial value needs to be small until we have been able to probe
rlimit and figure out what is safe.

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: xml_is_well_formed

[Mike Fowler]

On 06/08/10 12:31, Robert Haas wrote:


Maybe there should be

xml_is_well_formed()
xml_is_well_formed_document()
xml_is_well_formed_content()

I agree that consistency with SQL/XML is desirable, but for someone
coming from the outside, the unqualified claim that 'foo' is well-formed
XML might sound suspicious.
   

What about making the function sensitive to the XML OPTION, such that:

test=# SET xmloption TO DOCUMENT;
SET
text=# SELECT xml_is_well_formed('foo');

  xml_is_well_formed
  
  f
  (1 row)
 

That will make using this function a huge hassle, won't it?  Functions
that do different things depending on GUC settings are usually
troublesome.  Having three functions would be more sensible if we need
all three behaviors, but I don't see why we do.

Or perhaps it could return a string instead of a boolean: content,
document, or NULL if it's neither.
   


I like the sound of that. In fact this helps workaround the IS DOCUMENT 
and IS CONTENT limitations such that you can you can select only 
content, only documents or both is you use IS NOT NULL.


Unless anyone sees a reason that this function needs to remain a boolean 
function, I'll rework the patch over the weekend.


Regards,

--
Mike Fowler
Registered Linux user: 379787


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

[HACKERS] default of max_stack_depth

[Fujii Masao]

Hi,

The document says that the max_stack_depth is 2MB by default.
OTOH, in the source code, the variable max_stack_depth is
initialized with 100 (kB), and guc.c also uses 100 as the
default. Why?

This seems confusing to me though I know that
InitializeGUCOptions() sets max_stack_depth to 2MB.

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] review: xml_is_well_formed

[Robert Haas]

On Fri, Aug 6, 2010 at 4:28 AM, Mike Fowler  wrote:
> On 03/08/10 16:15, Peter Eisentraut wrote:
>>
>> On lör, 2010-07-31 at 13:40 -0400, Robert Haas wrote:

 Well-formedness should probably only allow XML documents.
>>>
>>> I think the point of this function is to determine whether a cast to
>>> xml will throw an error.  The behavior should probably match exactly
>>> whatever test would be applied there.
>>
>> Maybe there should be
>>
>> xml_is_well_formed()
>> xml_is_well_formed_document()
>> xml_is_well_formed_content()
>>
>> I agree that consistency with SQL/XML is desirable, but for someone
>> coming from the outside, the unqualified claim that 'foo' is well-formed
>> XML might sound suspicious.
>
> What about making the function sensitive to the XML OPTION, such that:
>
> test=# SET xmloption TO DOCUMENT;
> SET
> text=# SELECT xml_is_well_formed('foo');
>
>  xml_is_well_formed
>  
>  f
>  (1 row)

That will make using this function a huge hassle, won't it?  Functions
that do different things depending on GUC settings are usually
troublesome.  Having three functions would be more sensible if we need
all three behaviors, but I don't see why we do.

Or perhaps it could return a string instead of a boolean: content,
document, or NULL if it's neither.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

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

Re: [HACKERS] MERGE Specification

[Simon Riggs]

On Fri, 2010-08-06 at 16:26 +0800, Boxuan Zhai wrote:

> So, we need to add both DO NOTHING and RAISE ERROR actions in the
> MERGE command now !? What will RAISE ERROR do?

Let's get the rest of it working first. This would be a later extension,
though I think an important one for our developers.

> To stop the whole MERGE command OR, just throw an error notice for the
> row and move on.

As you say, it would be even better to be able to report errors in some
way and move onto next row. NOTICE is not the way though.

Maybe one of the actions would be to EXECUTE a procedure, so we can call
an error logging function.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Development, 24x7 Support, Training and Services


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

Re: [HACKERS] review: xml_is_well_formed

[Mike Fowler]

On 03/08/10 16:15, Peter Eisentraut wrote:

On lör, 2010-07-31 at 13:40 -0400, Robert Haas wrote:

Well-formedness should probably only allow XML documents.


I think the point of this function is to determine whether a cast to
xml will throw an error.  The behavior should probably match exactly
whatever test would be applied there.


Maybe there should be

xml_is_well_formed()
xml_is_well_formed_document()
xml_is_well_formed_content()

I agree that consistency with SQL/XML is desirable, but for someone
coming from the outside, the unqualified claim that 'foo' is well-formed
XML might sound suspicious.


What about making the function sensitive to the XML OPTION, such that:

test=# SET xmloption TO DOCUMENT;
SET
text=# SELECT xml_is_well_formed('foo');

  xml_is_well_formed
 
  f
 (1 row)

test=# SET xmloption TO CONTENT;
SET
text=# SELECT xml_is_well_formed('foo');

  xml_is_well_formed
 
  t
 (1 row)

with the inverse for DOCUMENTS? To me this makes the most sense as it makes the 
function behave much more like the other xml functions.

--
Mike Fowler
Registered Linux user: 379787


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

Re: [HACKERS] MERGE Specification

[Boxuan Zhai]

On Fri, Aug 6, 2010 at 3:53 PM, Simon Riggs  wrote:

> On Fri, 2010-08-06 at 10:28 +0300, Heikki Linnakangas wrote:
> > On 06/08/10 10:12, Simon Riggs wrote:
> > > So DO NOTHING is the default and implies silently ignoring rows. RAISE
> > > ERROR is the opposite.
> > >
> > > Coding for those seems very easy, its just a question of "should we do
> > > it?". DB2 has it; SQL:2008 does not. But then SQL:2008 followed the DB2
> > > introduction of AND clauses, and SQL:2011 has so far followed the DB2
> > > introduction of DELETE action also.
> >
> > I see neither DO NOTHING or RAISE ERROR in the documentation of DB2,
> > Oracle, or MSSQL server.
>
> Agreed, Oracle and MSSQL server does not have these.
>
> However, DB2 very clearly does have these features
>
> * SIGNAL which raises an error and can be used in place of any action,
> at any point in sequence of WHEN clauses. DB2 already supports SIGNAL as
> part of SQL/PSM, which we do not, so RAISE ERROR was the nearest
> equivalent command for PostgreSQL.
>
> * ELSE IGNORE which does same thing as DO NOTHING, except it must always
> be last statement in a sequence of WHEN clauses. DO NOTHING is already a
> phrase with exactly this meaning in PostgreSQL, so I suggest that.
>
>
So, we need to add both DO NOTHING and RAISE ERROR actions in the MERGE
command now !? What will RAISE ERROR do? To stop the whole MERGE command OR,
just throw an error notice for the row and move on.



>  --
>  Simon Riggs   www.2ndQuadrant.com 
>  PostgreSQL Development, 24x7 Support, Training and Services
>
>

Re: [HACKERS] MERGE Specification

[Boxuan Zhai]

On Fri, Aug 6, 2010 at 3:41 PM, Simon Riggs  wrote:

> On Fri, 2010-08-06 at 10:28 +0300, Heikki Linnakangas wrote:
>
> > > SQL:2011 makes no mention of how MERGE should react to statement level
> > > triggers. MERGE is not a trigger action even. Given considerable
> > > confusion in this area, IMHO we should just say the MERGE does not call
> > > statement triggers at all, of any kind.
> >
> > IMO the UPDATE/DELETE/INSERT actions should fire the respective
> > statement level triggers, but the MERGE itself should not.
>
> When, and how?
>
> If an UPDATE is mentioned 5 times, do we call the trigger 5 times?


My current process for BEFOR / AFTER STATEMENT trigger on MERGE is to fire
the triggers for all action types that appears in the command, unless it is
replaced by a INSTEAD rule. But the triggers for one action type will be
fired only once. That means you will get both UPDATE and INSERT triggers be
activated for only once if you are executing a MERGE command with 5 UPDATEs
and 10 INSERTs.


> What happens if none of the UPDATEs are ever executed?
>
> The triggers (I mean triggers for statement) will be fired anyway even the
UPDATE action matches no tuple. This is not for MERGE only. If you update a
table with the command
UPDATE foo SET ... WHERE false;
It will also fire the STATEMENT triggers of UPDATE type on foo (I think so).


And, even not been asked, I want to say that, in current implementation of
MERGE,  the row level triggers are fired by the actions that take the
tuples. If one tuple is caught by an UPDATE action, then the UPDATE row
trigger will be fired on this tuple. If it is handled by INSERT action, then
the INSRET row triggers are on.

Hope you agree with my designs.


> Best explain exactly what you mean.
>
> --
>  Simon Riggs   www.2ndQuadrant.com 
>  PostgreSQL Development, 24x7 Support, Training and Services
>
>

Re: Review: Re: [PATCH] Re: [HACKERS] Adding xpath_exists function

[Mike Fowler]

On 06/08/10 05:38, Peter Eisentraut wrote:

On tis, 2010-07-27 at 16:33 -0700, David Fetter wrote:
   

* Do we already have it?

 Not really.  There are kludges to accomplish these things, but
 they're available mostly in the sense that a general-purpose
 language allows you to write code to do anything a Turing machine
 can do.
 

I think this has been obsoleted by the xmlexists patch


In many ways yes. The only surviving difference is that xpath_exists has 
support for namespaces and xmlexists does not as the grammar expects 
namespaces to be handled in the xquery. So if people expect namespace 
support to be useful that having both functions is useful until I (or 
someone who works faster than me) get xquery going.


If the patch is to be committed, does it make sense for me to refine it 
such that it uses the new xpath internal function you extracted in the 
xmlexists patch?


Regards,

--
Mike Fowler
Registered Linux user: 379787


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