Re: [HACKERS] pg_cancel_backend by non-superuser

2011-12-12 Thread Greg Smith

On 12/11/2011 05:29 PM, Torello Querci wrote:

I will try to adjust the patch and submit for the next Commit Fest if
this is ok for you.
   


I don't think we'll need this, it will take a bit to explain why though.

First, thanks for returning this topic to discussion and keeping up with 
all the controversy around it.  You said back in February this was your 
first post here, and I doubt you expected that 10 months later this 
would still be active and argued over.  The fact that you're still here 
and everyone knows your name now is itself an accomplishment, many 
people just give up on their submission ideas under far less negative 
feedback.


I just took a long look at all three of the submissions in this area 
we've gotten.  The central idea that made yours different was making the 
database owner the person allowed to cancel things.  That hadn't been 
suggested as a cancellation requisite before that I know of, and this 
code may wander in that direction one day.  It's just a bit too much to 
accept right now.  You seem to need that specific feature for your 
environment.  If that's the case, you might want to develop something 
that works that way, but handles the concerns raised here.  The fact 
that it's not acceptable for a database owner to cancel a superuser 
query is the biggest objection, there were some others too.  Ultimately 
it may take a reworking of database permissions to really make this 
acceptable, which is a larger job than I think you were trying to get 
involved with.


Unfortunately, when I look at the new spec we have now, I don't see 
anything from what you did that we can re-use.  It's too specific to the 
owner-oriented idea.  The two other patches that have been submitted 
both are closer to what we've decided we want now.  What I'm going to do 
here is mark your submission "returned with feedback".


Rather than wait for something new from you, I'm going to review and 
rework the other two submissions.  That I can start on right now.  It's 
taken so long to reach this point that I don't want to wait much longer 
for another submission here, certainly not until over a month from now 
when the next CF starts.  We need to get the arguments around a new 
version started earlier than that.  Thanks for offering to work on this 
more, and I hope there's been something about this long wandering 
discussion that's been helpful to you.  As I said, you did at least make 
a good first impression, and that is worth something when it comes to 
this group.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us


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


Re: [HACKERS] JSON for PG 9.2

2011-12-12 Thread Simon Riggs
On Tue, Dec 13, 2011 at 5:22 AM, Tom Lane  wrote:
> Andrew Dunstan  writes:
>> On 12/12/2011 03:54 PM, Robert Haas wrote:
>>> There are way too many places that assume that the typmod can
>>> just be discarded.  I don't think that's going to fly, because
>>> =(text,text) probably has different semantics from =(json,json).
>
>> And certain places where they are not allowed at all, I think (unless I
>> am misremembering the early debates about enum types and output functions).
>
> Yeah.  The current system design assumes that typmod specifies a
> constraint of some sort.  It is not possible to use it to change the
> semantics of the datatype.  The most obvious way in which this is true
> is that selection of which operators and functions to apply to values
> does not consider typmod of the values.  This is not something we should
> lightly revisit.  We don't even have a handle on how to make domains
> behave differently from their underlying datatypes, and those *do* have
> their own type OIDs.  Injecting typmod into the algorithm seems like a
> disaster from here.

I'm glad I didn't think of doing that before then.

Can we agree some wording to put into the docs? Sounds like some clear
warnings are needed.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

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


Re: [HACKERS] JSON for PG 9.2

2011-12-12 Thread Jan Urbański
- Original message -
> On Dec 12, 2011, at 4:51 PM, Peter van Hardenberg wrote:
> 
> > Because we haven't heard from him in a while we've been using PL/V8 to
> > validate a JSON datatype simulated by a DOMAIN with a simple
> > acceptance function. (See below.) This is not ideally performant but
> > thanks to V8's JIT the JSON parser is actually reasonably good.
> 
> Note that Claes Jakobsson has been working on a JSON data type using the
> Jansson JSON library.
> 
>     http://pgxn.org/dist/pg-json/

We recently needed to store/valisate JSON data and be able to do some trivial 
extraction of values from it and went with pg-json.

The great benefit of having JSON as an extension type is being able to use an 
external library (Jansson is very small, MIT licensed, looks really well 
written and has been actively maintaied for years) and not being tied to a 
yearly release cycle.

Postgres jumps through a lot of hoops to be extensible and I think a JSON type 
is a kind of thing that fits the bill of an extension perfectly.

Cheers,
Jan

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


Re: [HACKERS] pgsql_fdw, FDW for PostgreSQL server

2011-12-12 Thread Tom Lane
Shigeru Hanada  writes:
> (2011/12/12 22:59), Robert Haas wrote:
>> ... I feel like we might need a system here that
>> allows for more explicit user control about what to push down vs. not,
>> rather than assuming we'll be able to figure it out behind the scenes.

> Agreed.  How about to add a per-column boolean FDW option, say
> "pushdown", to pgsql_fdw?  Users can tell pgsql_fdw that the column can
> be pushed down safely by setting this option to true.

[ itch... ] That doesn't seem like the right level of granularity.
ISTM the problem is with whether specific operators have the same
meaning at the far end as they do locally.  If you try to attach the
flag to columns, you have to promise that *every* operator on that
column means what it does locally, which is likely to not be the
case ever if you look hard enough.  Plus, having to set the flag on
each individual column of the same datatype seems pretty tedious.

I don't have a better idea to offer at the moment though.  Trying
to attach such a property to operators seems impossibly messy too.
If it weren't for the collations issue, I might think that labeling
datatypes as being compatible would be a workable approximation.

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] JSON for PG 9.2

2011-12-12 Thread Merlin Moncure
On Mon, Dec 12, 2011 at 7:37 PM, Daniel Farina  wrote:
> On Mon, Dec 12, 2011 at 4:51 PM, Peter van Hardenberg  wrote:>
>> PL/V8 is fast, it's sandboxed, and while it doesn't provide GIN or
>> GIST operators out of the box, maybe those could be motivated by its
>> inclusion.
>
> I also feel that a big problem with JSON as a data type is that there
> is not a powerful, common navigation method.  JSON path is basically
> pretty obscure by comparison to XPath.  As a result, the common
> approach to navigation in a JSON structure is basically "write
> procedures".  And that is only perfectly supported by a full-blown
> interpreter.

This.  For me, postgres xml extensions is 'a whole bunch of extra
stuff that comes with the xpath function'.  How you get data into and
out of json is much more interesting than how the type is set up
internally or how it's parsed.

There must be some way to avoid iterative set up and tear down of json
objects (maybe as a cast?) -- postgres arrays of composites can set up
data in a way that feels very much like json in it's construction.
One big reason why people might go to server side json is to try and
avoid tedious marshaling of data between client and server.  The xpath
function has had its warts, but it offers very tight coupling between
your database and your documents.  In the case of json, I think you
can go even further.

merlin

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


Re: [HACKERS] JSON for PG 9.2

2011-12-12 Thread Peter Eisentraut
On mån, 2011-12-12 at 16:51 -0800, Peter van Hardenberg wrote:
> Because we haven't heard from him in a while we've been using PL/V8 to
> validate a JSON datatype simulated by a DOMAIN with a simple
> acceptance function. (See below.) This is not ideally performant but
> thanks to V8's JIT the JSON parser is actually reasonably good.
> 
> I think releasing something simple and non-performant with reasonable
> semantics would be the best next step. If it were up to me, I'd
> probably even try to just land PL/V8 as PL/JavaScript for 9.2 if the
> crash bugs and deal breakers can be sifted out.

You don't need a new PL to do that.  The existing PLs can also parse
JSON.  So that's not nearly enough of a reason to consider adding this
new PL.



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


Re: [HACKERS] JSON for PG 9.2

2011-12-12 Thread Tom Lane
Andrew Dunstan  writes:
> On 12/12/2011 03:54 PM, Robert Haas wrote:
>> There are way too many places that assume that the typmod can
>> just be discarded.  I don't think that's going to fly, because
>> =(text,text) probably has different semantics from =(json,json).

> And certain places where they are not allowed at all, I think (unless I 
> am misremembering the early debates about enum types and output functions).

Yeah.  The current system design assumes that typmod specifies a
constraint of some sort.  It is not possible to use it to change the
semantics of the datatype.  The most obvious way in which this is true
is that selection of which operators and functions to apply to values
does not consider typmod of the values.  This is not something we should
lightly revisit.  We don't even have a handle on how to make domains
behave differently from their underlying datatypes, and those *do* have
their own type OIDs.  Injecting typmod into the algorithm seems like a
disaster from here.

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] Arithmetic operators for macaddr type

2011-12-12 Thread Brendan Jurd
On 12 December 2011 15:59, Pavel Stehule  wrote:
> 2011/12/12 Brendan Jurd :
>> I just bumped into a situation where I wanted to do a little macaddr
>> arithmetic in postgres.  I note that the inet type has support for
>> bitwise AND, OR and NOT, as well as subtraction, but macaddr has none
>> of the above.
>
> +1
>

Here is a patch for $SUBJECT.  I merely added support for ~, & and |
operators for the macaddr type.  The patch itself is rather trivial,
and includes regression tests and a doc update.

For the documentation, I did think about adding a new table for the
macaddr operators, but in the end decided it would probably be an
overkill.  If others think a table would be better, I'm happy to
revise it.

I also considered adding a function which would return the numeric
value of the MAC as a bigint, but figured I might save that for a
separate patch.

Cheers,
BJ
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***
*** 8300,8306  CREATE TYPE rainbow AS ENUM ('red', 'orange', 'yellow', 'green', 'blue', 'purple
 
  The macaddr type also supports the standard relational
  operators (>, <=, etc.) for
! lexicographical ordering.
 
  

--- 8300,8308 
 
  The macaddr type also supports the standard relational
  operators (>, <=, etc.) for
! lexicographical ordering, and the bitwise arithmetic operators
! (~, & and |)
! for NOT, AND and OR.
 
  

*** a/src/backend/utils/adt/mac.c
--- b/src/backend/utils/adt/mac.c
***
*** 242,247  hashmacaddr(PG_FUNCTION_ARGS)
--- 242,300 
  }
  
  /*
+  * Arithmetic functions: bitwise NOT, AND, OR.
+  */
+ Datum
+ macaddr_not(PG_FUNCTION_ARGS)
+ {
+ 	macaddr	   *addr = PG_GETARG_MACADDR_P(0);
+ 	macaddr	   *result;
+ 
+ 	result = (macaddr *) palloc(sizeof(macaddr));
+ 	result->a = ~addr->a;
+ 	result->b = ~addr->b;
+ 	result->c = ~addr->c;
+ 	result->d = ~addr->d;
+ 	result->e = ~addr->e;
+ 	result->f = ~addr->f;
+ 	PG_RETURN_MACADDR_P(result);
+ }
+ 
+ Datum
+ macaddr_and(PG_FUNCTION_ARGS)
+ {
+ 	macaddr	   *addr1 = PG_GETARG_MACADDR_P(0);
+ 	macaddr	   *addr2 = PG_GETARG_MACADDR_P(1);
+ 	macaddr	   *result;
+ 
+ 	result = (macaddr *) palloc(sizeof(macaddr));
+ 	result->a = addr1->a & addr2->a;
+ 	result->b = addr1->b & addr2->b;
+ 	result->c = addr1->c & addr2->c;
+ 	result->d = addr1->d & addr2->d;
+ 	result->e = addr1->e & addr2->e;
+ 	result->f = addr1->f & addr2->f;
+ 	PG_RETURN_MACADDR_P(result);
+ }
+ 
+ Datum
+ macaddr_or(PG_FUNCTION_ARGS)
+ {
+ 	macaddr	   *addr1 = PG_GETARG_MACADDR_P(0);
+ 	macaddr	   *addr2 = PG_GETARG_MACADDR_P(1);
+ 	macaddr	   *result;
+ 
+ 	result = (macaddr *) palloc(sizeof(macaddr));
+ 	result->a = addr1->a | addr2->a;
+ 	result->b = addr1->b | addr2->b;
+ 	result->c = addr1->c | addr2->c;
+ 	result->d = addr1->d | addr2->d;
+ 	result->e = addr1->e | addr2->e;
+ 	result->f = addr1->f | addr2->f;
+ 	PG_RETURN_MACADDR_P(result);
+ }
+ 
+ /*
   *	Truncation function to allow comparing mac manufacturers.
   *	From suggestion by Alex Pilosov 
   */
*** a/src/include/catalog/pg_operator.h
--- b/src/include/catalog/pg_operator.h
***
*** 1116,1121  DESCR("greater than");
--- 1116,1128 
  DATA(insert OID = 1225 (  ">="	   PGNSP PGUID b f f 829 829	 16 1223 1222 macaddr_ge scalargtsel scalargtjoinsel ));
  DESCR("greater than or equal");
  
+ DATA(insert OID = 3141 (  "~"	   PGNSP PGUID l f f	  0 829 829 0 0 macaddr_not - - ));
+ DESCR("bitwise not");
+ DATA(insert OID = 3142 (  "&"	   PGNSP PGUID b f f	829 829 829 0 0 macaddr_and - - ));
+ DESCR("bitwise and");
+ DATA(insert OID = 3143 (  "|"	   PGNSP PGUID b f f	829 829 829 0 0 macaddr_or - - ));
+ DESCR("bitwise or");
+ 
  /* INET type (these also support CIDR via implicit cast) */
  DATA(insert OID = 1201 (  "="	   PGNSP PGUID b t t 869 869	 16 1201 1202 network_eq eqsel eqjoinsel ));
  DESCR("equal");
*** a/src/include/catalog/pg_proc.h
--- b/src/include/catalog/pg_proc.h
***
*** 2037,2042  DATA(insert OID = 834 (  macaddr_ge			PGNSP PGUID 12 1 0 0 0 f f f t f i 2 0 16
--- 2037,2045 
  DATA(insert OID = 835 (  macaddr_ne			PGNSP PGUID 12 1 0 0 0 f f f t f i 2 0 16 "829 829" _null_ _null_ _null_ _null_	macaddr_ne _null_ _null_ _null_ ));
  DATA(insert OID = 836 (  macaddr_cmp		PGNSP PGUID 12 1 0 0 0 f f f t f i 2 0 23 "829 829" _null_ _null_ _null_ _null_	macaddr_cmp _null_ _null_ _null_ ));
  DESCR("less-equal-greater");
+ DATA(insert OID = 3138 (  macaddr_not		PGNSP PGUID 12 1 0 0 0 f f f t f i 1 0 829 "829" _null_ _null_ _null_ _null_	macaddr_not _null_ _null_ _null_ ));
+ DATA(insert OID = 3139 (  macaddr_and		PGNSP PGUID 12 1 0 0 0 f f f t f i 2 0 829 "829 829" _null_ _null_ _null_ _null_	macaddr_and _null_ _null_ _null_ ));
+ DATA(insert OID = 3140 (  macaddr_or		PGNSP PGUID 12 1 0 0 0 f f f t f i 2 0 829 "829 829" _null_ _null_ _null_ _null_	macaddr_or _null_ _null_ _null_ ));
  
  /* for inet type support */
  DATA(insert OID = 910 (  inet_in			PGNSP

Re: [HACKERS] JSON for PG 9.2

2011-12-12 Thread Alvaro Herrera

Excerpts from Daniel Farina's message of lun dic 12 22:37:13 -0300 2011:

> * It'd be nice to pass intermediate in-memory representations rather
> than calling JSON.parse all the time, similar to OPAQUE except sound
> (so bogus pointers cannot be passed).  Basically, an "ephemeral type".
>  It could save a lot of when composing operators.  I've needed this
> for other projects, but for much the same reason.

I remember there was the idea of doing something like this for regexes
-- have a specialized data type that saves the trouble of parsing it.
I imagine this is pretty much the same.

Nobody got around to doing anything about it though.  

-- 
Á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] patch : Allow toast tables to be moved to a different tablespace

2011-12-12 Thread Jaime Casanova
On Mon, Dec 12, 2011 at 10:54 AM, Julien Tachoires  wrote:
> Hi,
>
> 2011/12/10 Jaime Casanova :
>> On Mon, Nov 28, 2011 at 1:32 PM, Julien Tachoires  wrote:
>>
 2) after CLUSTER the index of the toast table gets moved to the same
 tablespace as the main table
>>>
>>
>> there is still a variant of this one, i created 3 tablespaces (datos_tblspc):
>>
>> """
>> create table t1 (
>>     i serial primary key,
>>     t text
>> ) tablespace datos_tblspc;
>>
>> ALTER TABLE t1 SET TOAST TABLESPACE pg_default;
>> CLUSTER t1 USING t1_pkey;
>> """
>
> I am not able to reproduce this case, could you show me exactly how to
> reproduce it ?
>

just as that...
- create a table in a certain tablespace (diferent from pg_default),
the toast table will be in the same tablespace,
- then change the tablespace to pg_default and
- then cluster the table...
the toast table will be again in the same tablespace as the main table


-- 
Jaime Casanova         www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación

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


Re: [HACKERS] JSON for PG 9.2

2011-12-12 Thread Andrew Dunstan



On 12/12/2011 08:46 PM, Daniel Farina wrote:

On Mon, Dec 12, 2011 at 5:36 PM, Andrew Dunstan  wrote:

The trouble with using JSON.parse() as a validator is that it's probably
doing way too much work. PLV8 is cool, and I keep trying to get enough time
to work on it more, but I don't think it's a substitute for a JSON type with
a purpose built validator and some native operations. I think these efforts
can continue in parallel.

Hmm. Maybe?  While I'm sure things could be faster, we've had results
that are fast enough to be usable even with constant reparsing.  Here
are some microbenchmarks I did some time ago where I tried to find the
overhead of calling JSON.parse and doing some really simple stuff in
V8 that I thought would maximize the amount of constant-time overhead:

https://gist.github.com/1150804

On my workstation, one core was able to do 130,000 JSON.parses + other
stuff necessary to create an index per second.  One could maybe try to
improve the speed and memory footprint on large documents by having
validators that don't actually build the V8 representation and
possibly defining a space of operators that are known to build, by
induction, valid JSON without rechecks.

But in the end, I think there's already a class of problem where the
performance plv8 provides is already quite sufficient, and provides a
much more complete and familiar approach to the problem of how people
choose to navigate, project, and manipulate JSON documents.

I also haven't tried this for larger documents, as I was trying to get
a sense of how much time was spent in a few primitive operations, and
not testing performance with regard to document length.



Yes, I didn't mean to say it's not fast. For many cases I agree it is 
probably fast enough.


But in the end PLV8 is likely to remain an addon - nice as it can be I 
doubt the core team will want to add another PL to the core code, 
especially one written in C++. If we want a JSON type built in, which 
many people seem to want, we'll need to do it without the support of 
PLV8, I think.


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] pgsql_fdw, FDW for PostgreSQL server

2011-12-12 Thread Shigeru Hanada
(2011/12/12 22:59), Robert Haas wrote:
> It does seem like this might not be enough information for the FDW to
> make good decisions about pushdown.  Even supposing the server on the
> other hand is also PostgreSQL, the collation names might not match
> (if, say, one is running Windows, and the other, Linux).  And even if
> they do, there is no guarantee that two collations with the same name
> have the same behavior on two different machines; they probably
> should, but who knows?  And if we're using an FDW to talk to some
> other database server, the problem is much worse; it's not clear that
> we'll even begin to be able to guess whether the remote side has
> compatible semantics.  I feel like we might need a system here that
> allows for more explicit user control about what to push down vs. not,
> rather than assuming we'll be able to figure it out behind the scenes.

Agreed.  How about to add a per-column boolean FDW option, say
"pushdown", to pgsql_fdw?  Users can tell pgsql_fdw that the column can
be pushed down safely by setting this option to true.  IMO default
should be false (no push down).

In most cases, columns with numeric/time-related types are safe to be
pushed down because they are free from collations issue, so users would
want to set to true.  OTOH, columns with string types would need some
considerations.  Once users have ensured that the column has compatible
semantics, they can set "pushdown=true" for efficiency.

If a condition contains any columns with pushdown=false, that condition
should NOT be pushed down.

This idea is only for pgsql_fdw now, but it can be used for other FDWs
which support push-down, and it would be also useful for ORDER BY
push-down support in future, which is apparently contains collation issue.

Regards,
-- 
Shigeru Hanada

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


Re: [HACKERS] JSON for PG 9.2

2011-12-12 Thread Daniel Farina
On Mon, Dec 12, 2011 at 5:36 PM, Andrew Dunstan  wrote:
> The trouble with using JSON.parse() as a validator is that it's probably
> doing way too much work. PLV8 is cool, and I keep trying to get enough time
> to work on it more, but I don't think it's a substitute for a JSON type with
> a purpose built validator and some native operations. I think these efforts
> can continue in parallel.

Hmm. Maybe?  While I'm sure things could be faster, we've had results
that are fast enough to be usable even with constant reparsing.  Here
are some microbenchmarks I did some time ago where I tried to find the
overhead of calling JSON.parse and doing some really simple stuff in
V8 that I thought would maximize the amount of constant-time overhead:

https://gist.github.com/1150804

On my workstation, one core was able to do 130,000 JSON.parses + other
stuff necessary to create an index per second.  One could maybe try to
improve the speed and memory footprint on large documents by having
validators that don't actually build the V8 representation and
possibly defining a space of operators that are known to build, by
induction, valid JSON without rechecks.

But in the end, I think there's already a class of problem where the
performance plv8 provides is already quite sufficient, and provides a
much more complete and familiar approach to the problem of how people
choose to navigate, project, and manipulate JSON documents.

I also haven't tried this for larger documents, as I was trying to get
a sense of how much time was spent in a few primitive operations, and
not testing performance with regard to document length.

-- 
fdr

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


Re: [HACKERS] JSON for PG 9.2

2011-12-12 Thread Daniel Farina
On Mon, Dec 12, 2011 at 4:51 PM, Peter van Hardenberg  wrote:>
> PL/V8 is fast, it's sandboxed, and while it doesn't provide GIN or
> GIST operators out of the box, maybe those could be motivated by its
> inclusion.

I also feel that a big problem with JSON as a data type is that there
is not a powerful, common navigation method.  JSON path is basically
pretty obscure by comparison to XPath.  As a result, the common
approach to navigation in a JSON structure is basically "write
procedures".  And that is only perfectly supported by a full-blown
interpreter.

So that's why I'm personally more inclined to lend my attention to
embedding JavaScript entirely.  Not to say there aren't areas ripe for
improvement:

* It'd be nice to pass intermediate in-memory representations rather
than calling JSON.parse all the time, similar to OPAQUE except sound
(so bogus pointers cannot be passed).  Basically, an "ephemeral type".
 It could save a lot of when composing operators.  I've needed this
for other projects, but for much the same reason.

* It'd be nice to be able to safely define indexes in a trusted
language somehow, writing the penalty and split functions, et al.
Right now it's my recollection that defining GiST operators in a naive
port to Javascript would give you the power to return garbage that is
not merely wrong, but could also crash Postgres if it uses a bogus
indexes.  Ready and willing to be corrected*

* Some kind of partial toasting of large datums (I think Simon Riggs
briefly glossed over such an idea when we were talking about this
general use case)

But nothing I can quickly identify in the Postgres as-is is opposed to
any of these improvements at a design level, so they can be chipped
off into incremental work in the future.

-- 
fdr

* Madness, you say? http://bellard.org/jslinux/, if your browser is
new enough.  The relevant spec:
https://www.khronos.org/registry/typedarray/specs/latest/

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


Re: [HACKERS] JSON for PG 9.2

2011-12-12 Thread Andrew Dunstan



On 12/12/2011 07:51 PM, Peter van Hardenberg wrote:

We reached out to Joseph to see if we could help sponsor the project,
but never really heard back from him.

Because we haven't heard from him in a while we've been using PL/V8 to
validate a JSON datatype simulated by a DOMAIN with a simple
acceptance function. (See below.) This is not ideally performant but
thanks to V8's JIT the JSON parser is actually reasonably good.

I think releasing something simple and non-performant with reasonable
semantics would be the best next step. If it were up to me, I'd
probably even try to just land PL/V8 as PL/JavaScript for 9.2 if the
crash bugs and deal breakers can be sifted out.

PL/V8 is fast, it's sandboxed, and while it doesn't provide GIN or
GIST operators out of the box, maybe those could be motivated by its
inclusion.

Andrew, you've been down in the guts here, what do you think?


The trouble with using JSON.parse() as a validator is that it's probably 
doing way too much work. PLV8 is cool, and I keep trying to get enough 
time to work on it more, but I don't think it's a substitute for a JSON 
type with a purpose built validator and some native operations. I think 
these efforts can continue in parallel.


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] JSON for PG 9.2

2011-12-12 Thread David E. Wheeler
On Dec 12, 2011, at 4:51 PM, Peter van Hardenberg wrote:

> Because we haven't heard from him in a while we've been using PL/V8 to
> validate a JSON datatype simulated by a DOMAIN with a simple
> acceptance function. (See below.) This is not ideally performant but
> thanks to V8's JIT the JSON parser is actually reasonably good.
> 
> I think releasing something simple and non-performant with reasonable
> semantics would be the best next step. If it were up to me, I'd
> probably even try to just land PL/V8 as PL/JavaScript for 9.2 if the
> crash bugs and deal breakers can be sifted out.

Note that Claes Jakobsson has been working on a JSON data type using the 
Jansson JSON library.

  http://pgxn.org/dist/pg-json/

I’ve submitted a pull request renaming it to jansson-json (though the data type 
is still "json"):

  https://github.com/theory/pg-json/tree/pgxn

Anyway, it seems like a decent start to an extensible type implemented entirely 
as an extension. Claes tells me he plans to add index support soonish, so it 
could get to be pretty robust before long.

Just another stab at the problem to alert folks to.

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] WIP: URI connection string support for libpq

2011-12-12 Thread David E. Wheeler
On Dec 12, 2011, at 3:55 PM, Peter van Hardenberg wrote:

> I'd like to make the controversial proposal that the URL prefix should
> be "postgres:" instead of "postgresql:". Postgres is a widely accepted
> nickname for the project, and is eminently more pronounceable. Once
> the url is established it will be essentially impossible to change
> later, but right now only a nearly insurmountable mailing list thread
> prevents it.

What happened to SexQL?

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] JSON for PG 9.2

2011-12-12 Thread Peter van Hardenberg
We reached out to Joseph to see if we could help sponsor the project,
but never really heard back from him.

Because we haven't heard from him in a while we've been using PL/V8 to
validate a JSON datatype simulated by a DOMAIN with a simple
acceptance function. (See below.) This is not ideally performant but
thanks to V8's JIT the JSON parser is actually reasonably good.

I think releasing something simple and non-performant with reasonable
semantics would be the best next step. If it were up to me, I'd
probably even try to just land PL/V8 as PL/JavaScript for 9.2 if the
crash bugs and deal breakers can be sifted out.

PL/V8 is fast, it's sandboxed, and while it doesn't provide GIN or
GIST operators out of the box, maybe those could be motivated by its
inclusion.

Andrew, you've been down in the guts here, what do you think?

-pvh

(Code sample.)

create or replace function valid_json(json text)
returns bool as $$
  try { JSON.parse(json); return true }
  catch(e) { return false}
$$ LANGUAGE plv8 IMMUTABLE STRICT;

select valid_json('{"key": "value"}'), valid_json('lol');
valid_json | t
valid_json | f
Time: 0.283 ms

create domain json
  as text check(valid_json(VALUE));
create table jsononly(data json);

insert into jsononly values 'lol';
ERROR:  syntax error at or near "'lol'"
LINE 1: insert into jsononly values 'lol';

insert into jsononly
  values ('{"ok": true}');
INSERT 0 1

-p

On Mon, Dec 12, 2011 at 1:34 PM, Josh Berkus  wrote:
> Bruce,
>
> I thought that Joseph Adams was still working on this, sponsored by
> Heroku.  Joseph?
>
>
> --
> Josh Berkus
> PostgreSQL Experts Inc.
> http://pgexperts.com
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers



-- 
Peter van Hardenberg
San Francisco, California
"Everything was beautiful, and nothing hurt." -- Kurt Vonnegut

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


Re: [HACKERS] Is anybody actually using XLR_BKP_REMOVABLE?

2011-12-12 Thread Tom Lane
I wrote:
> Simon Riggs  writes:
>> I'll volunteer. Assume you can reuse the flag and I will patch afterwards.

> Thanks for the offer, but after thinking about it a bit more I realized
> that this change is quite trivial, so I just went ahead and did it along
> with the change in XLR_MAX_BKP_BLOCKS.  This seems better since both
> related changes are in one commit, and we can't forget to do it.

BTW, just for the archives' sake: some digging in the git history showed
that my memory was faulty about the pre-2007 limit of XLR_MAX_BKP_BLOCKS
having been 4.  It was originally 2, and then in 2003 we increased it to
3 (cf commit 799bc58dc7ed9899facfc8302040749cb0a9af2f).  So at the time
the last xl_info bit got taken over for XLR_BKP_REMOVABLE, it had in
fact been unused, and that probably explains why we didn't think harder
about whether there would be a less expensive way to do it.

I still think allowing 4 pages per WAL entry is a good thing, though,
and so am not inclined to withdraw the proposal.  But perhaps it would
be worth explaining why this is necessary for SP-GiST.  The case where
it comes up is trying to split a list of leaf-page tuples when we need
to add another entry to the list but there's no room on the page.
SP-GiST doesn't allow such lists to cross pages (which I think is a
reasonable restriction, both to avoid excess seeks and because the list
links can thereby be 2 bytes not 6).  So what it has to do here is
insert an upper-page tuple ("inner tuple" in the patch's jargon) to
describe the set of leaf page tuples that have now been split into two
or more lists.  This requires touching:
1. The leaf page currently holding the list to be modified.
2. Another leaf page that has enough free space for the overrun.
3. An inner page (inner pages and leaf pages are disjoint in SP-GiST)
   where there's enough room to put the new inner tuple.
4. The inner page holding the inner tuple that is the parent of the
   leaf-page list; we have to update its downlink to point to the
   new inner tuple instead of the leaf list.

The code will try to put the new inner tuple on the same page as the
original parent tuple, but if there's no room there, there's no way to
get around the fact that there are four different pages involved here.

If somebody held a gun to my head and said "do it with only three",
what I'd try to do is make the update of the parent tuple into a
separately logged WAL action.  However this is not trivial, or at least
it's not trivial to recover during replay if the database crashes after
logging the first action --- there is not enough information on-disk to
figure out what needs to be done.  Also, I believe we've been trying
to get rid of that sort of recovery-time cleanup requirement, because
of hot standby.  So on the whole I think extending the XLogInsert
machinery to allow 4 backed-up pages is the best solution.

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] WIP: URI connection string support for libpq

2011-12-12 Thread Peter van Hardenberg
On Mon, Dec 12, 2011 at 2:06 PM, Alexander Shulgin
 wrote:
>
>
>  psql -d postgresql://user@pw:host:port/dbname?param1=value1¶m2=value2...
>

I'd like to make the controversial proposal that the URL prefix should
be "postgres:" instead of "postgresql:". Postgres is a widely accepted
nickname for the project, and is eminently more pronounceable. Once
the url is established it will be essentially impossible to change
later, but right now only a nearly insurmountable mailing list thread
prevents it.

Excluding references to the "postgresql.org" domain, there are already
5x as many references in the source code to "postgres" (2583 lines)
than to "postgresql" (539 lines). Taking into account that the name of
the binary and the usual Unix user are already postgres, having one
less place which would eventually need changing seems like a good plan
overall.

Here is, for those who have understandably blocked this argument from
their memory, a link to the existing wiki document on the pros and
cons of the two names:
http://wiki.postgresql.org/wiki/Postgres

Over at Heroku decided to side with Tom's assessment that "arguably,
the 1996 decision to call it PostgreSQL instead of reverting to plain
Postgres was the single worst mistake this project ever made." (And we
at Heroku have also frustratingly split our references and
occasionally used the "SQL" form.)

Although I do not have the stomach to push for a full renaming blitz,
I felt I must at least make a case for not making the situation any
worse.

My apologies in advance for re-opening this can of worms.

Best regards,
-pvh

PS: It is not in any way shape or form relevant to my argument, nor do
I claim that anyone else should care, but in the spirit of full
disclosure, and depending on how you count, we currently have
somewhere between 250,000 and 500,000 URLs which begin with
postgres:// in our care.

--
Peter van Hardenberg
San Francisco, California
"Everything was beautiful, and nothing hurt." -- Kurt Vonnegut

-- 
Sent 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] pg_last_xact_insert_timestamp

2011-12-12 Thread Greg Smith

On 12/12/2011 08:45 AM, Robert Haas wrote:
But I'm skeptical that anything that we only update once per 
checkpoint cycle will help much in

calculating an accurate lag value.


I'm sure there is no upper bound on how much WAL lag you can build up 
between commit/abort records either; they can be far less frequent than 
checkpoints.  All it takes is a multi-hour COPY with no other commits to 
completely hose lag measured by that advance, and that is not an unusual 
situation at all.  Overnight daily ETL or reporting MV-ish roll-ups, 
scheduled specifically for when no one is normally at the office, are 
the first thing that spring to mind.


Anyway, I wasn't suggesting checkpoints as anything other than a worst 
case behavior.  We can always thump out more frequent updates to reduce 
lag, and in what I expect to the most common case the WAL send/receive 
stuff will usually do much better.  I see the XID vs. WAL position UI 
issues as being fundamentally unsolvable, which really bothers me.  I'd 
have preferred to run screaming away from this thread if it hadn't.



It also strikes me that anything that is based on augmenting the 
walsender/walreceiver protocol leaves
anyone who is using WAL shipping out in the cold.  I'm not clear from
the comments you or Simon have made how important you think that use
case still is.
   


There's a number of reasons why we might want more timestamps streamed 
into the WAL; this might be one.  We'd just need one to pop out one as 
part of the archive_timeout switch to in theory make it possible for 
these people to be happy.  I think Simon was hoping to avoid WAL 
timestamps, I wouldn't bet too much on that myself.  The obvious 
implementation problem here is that the logical place to put the 
timestamps is right at the end of the WAL file, just before it's closed 
for archiving.  But that position isn't known until you've at least 
started processing it, which you clearly are not doing fast enough if 
lag exists.


As far as who's still important here, two observations.  Note that the 
pg_last_xact_insert_timestamp approach can fail to satisfy WAL shipping 
people who are going to a separate network, where it's impractical to 
connect to both servers with libpq.  I have some customers who like 
putting a one-way WAL wall (sorry) between production and the standby 
server, with the log shipping being the only route between them; that's 
one reason why they might still be doing this instead of using 
streaming.  There's really no good way to make these people happy and 
provide time lag monitoring inside the database.


I was actually the last person I recall who suggested some extra 
monitoring mainly aimed at WAL shipping environments:  
http://archives.postgresql.org/pgsql-hackers/2010-01/msg01522.php  Had 
some pg_standby changes I was also working on back then, almost two 
years ago.  I never circled back to any of it due to having zero demand 
since 9.0 shipped, the requests I had been regularly getting about this 
all dried up.  While I'm all for keeping new features working for 
everyone when it doesn't hold progress back, it's not unreasonable to 
recognize we can't support every monitoring option through all of the 
weird ways WAL files can move around.  pg_stat_replication isn't very 
helpful for 9.0+ WAL shippers either, yet they still go on doing their 
thing.


In the other direction, people who will immediately adopt the latest 
hotness, cascading is a whole new layer of use case concerns on top of 
the ones considered so far.  Now you're talking two layers of 
connections users have to navigate though to compute master->cascaded 
standby lag.  Cascade the WALSender timestamps instead, which seems 
pretty simple to do, and then people can just ask their local standby.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us


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


[HACKERS] WIP: URI connection string support for libpq

2011-12-12 Thread Alexander Shulgin
Hello Hackers,

Attached is a work-in-progress patch for URI connection string syntax support 
in libpq.  The recent discussion (also pointing to the original one) is here: 

  http://archives.postgresql.org/message-id/132180-sup-1235@moon

The patch adds support for the following syntax in psql, by adding special 
handling of dbname parameter, when it starts with "postgresql://", e.g:

  psql -d postgresql://user@pw:host:port/dbname?param1=value1¶m2=value2...

Virtually every component of the above syntax is optional, ultimately allowing 
for, e.g:

  psql -d postgresql:///

to specify local connection via Unix socket, with default port, user name, 
dbname, etc.

URI percent-encoding is handled, in particular, allowing to include special 
symbols in the embedded password string, or to specify non-standard Unix socket 
location, like the following:

  psql -d postgresql://%2Fvar%2Fpgsql%2Ftmp/mydb

The patch applies cleanly against the master branch and compiles w/o errors or 
warnings.  No tests were broken by this patch on my box, as far as I can tell.

The patch follows design initially proposed and tries to address feedback 
gathered from the recent discussion.  Special provision was made to improve 
compatibility with JDBC's connection URIs, by treating "ssl=true" parameter as 
equivalent of "sslmode=require".

The patch intentionally omits documentation changes, to focus on the desired 
behavior and new code design.

I've put reasonable effort into testing the new code by feeding various 
parameters to "psql -d".  However, if there's a facility for writing formal 
regression tests against psql, I'd be happy to use that.

I'm also adding this to the next open CommitFest: 2012-01.

--
Regards,
Alex


libpq-uri-v3.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] includeifexists in configuration file

2011-12-12 Thread Andrew Dunstan



On 12/12/2011 02:24 PM, Greg Smith wrote:

On 11/16/2011 10:19 AM, Robert Haas wrote:

I haven't read the code yet, but just to get the bikeshedding started,
I think it might be better to call this include_if_exists rather than
running it together as one word.


What's going on, it's like this bikeshed just disappeared.  I should 
figure out how that happened so I can replicate it.


This naming style change sounds fine to me, and I just adopted it for 
the updated configuration directory patch.  That patch now rearranges 
the documentation this feature modifies.  This is a pretty trivial 
feature I'm not real concerned about getting a review for.  I'll 
update this with the name change and appropriate rebased patch once 
some decision has been made about that one; will just bounce this 
forward to January if it's still here when the current CF starts 
closing in earnest.





I have briefly looked at the code (but not tried to apply or build it), 
and modulo the naming issue it looks OK to me.


Unless there is some other issue let's just get it applied. It looks 
like almost a no-brainer to me.


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] pg_restore --no-post-data and --post-data-only

2011-12-12 Thread Andrew Dunstan



On 12/08/2011 09:18 PM, Joachim Wieland wrote:

On Tue, Nov 15, 2011 at 6:14 PM, Andrew Dunstan  wrote:

Updated version with pg_restore included is attached.

The patch applies with some fuzz by now but compiles without errors or warnings.

The feature just works, it is not adding a lot of new code, basically
it parses the given options and then skips over steps depending on the
selected section.

I verified the equivalence of -a and -s to the respective sections in
the different archive formats and no surprise here either, they were
equivalent except for the header (which has a timestamp).

If you ask pg_restore to restore a section out of an archive which
doesn't have this section, there is no error and the command just
succeeds. This is what I expected and I think it's the right thing to
do but maybe others think that
there should be a warning.

In pg_restore, pre-data cannot be run in parallel, it would only run
serially, data and post-data can run in parallel, though. This is also
what I had expected but it might be worth to add a note about this to
the documentation.



This is true now of parallel restore, and is by design (see debates from 
the time.)




What I didn't like about the implementation was the two set_section()
functions, I'd prefer them to move to a file that is shared between
pg_dump and pg_restore and become one function...


Done



Minor issues:

{"section", required_argument, NULL, 5} in pg_dump.c is not in the alphabetical
order of the options.

  ./pg_restore --section=foobar
pg_restore: unknown section name "foobar")

Note the trailing ')', it's coming from a _(...) confusion

Some of the lines in the patch have trailing spaces and in the
documentation part tabs and spaces are mixed.

int skip used as bool skip in dumpDumpableObject()





Should all be fixed. Revised patch attached.

cheers

andrew
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index f6f33de..b16b429 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -116,9 +116,7 @@ PostgreSQL documentation

 

-This option is only meaningful for the plain-text format.  For
-the archive formats, you can specify the option when you
-call pg_restore.
+This option is equivalent to specifying --section=data.

   
  
@@ -404,10 +402,30 @@ PostgreSQL documentation

 Dump only the object definitions (schema), not data.

+   
+This option is equivalent to specifying 
+--section=pre-data --section=post-data.
+   
   
  
 
  
+   --section=sectionname
+   
+ 
+   Only dump the named section. The name can be one of pre-data, data 
+   and post-data. 
+   This option can be specified more than once. The default is to dump all sections.
+ 
+ 
+   Post-data items consist of definitions of indexes, triggers, rules 
+   and constraints other than check constraints. 
+   Pre-data items consist of all other data definition items.
+ 
+   
+ 
+
+ 
   -S username
   --superuser=username
   
diff --git a/doc/src/sgml/ref/pg_restore.sgml b/doc/src/sgml/ref/pg_restore.sgml
index be11d17..a28faf8 100644
--- a/doc/src/sgml/ref/pg_restore.sgml
+++ b/doc/src/sgml/ref/pg_restore.sgml
@@ -93,6 +93,9 @@

 Restore only the data, not the schema (data definitions).

+   
+This option is equivalent to specifying --section=data.
+   
   
  
 
@@ -359,6 +362,10 @@
 (Do not confuse this with the --schema option, which
 uses the word schema in a different meaning.)

+   
+This option is equivalent to specifying 
+--section=pre-data --section=post-data.
+   
   
  
 
@@ -505,6 +512,22 @@
  
 
  
+   --section=sectionname
+   
+ 
+   Only restore the named section. The name can be one of pre-data, data 
+   and post-data. 
+   This option can be specified more than once. The default is to restore all sections.
+ 
+ 
+   Post-data items consist of definitions of indexes, triggers, rules 
+   and constraints other than check constraints. 
+   Pre-data items consist of all other data definition items.
+ 
+   
+ 
+
+ 
   --use-set-session-authorization
   

diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index 5dab967..659ec06 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -17,6 +17,7 @@
 #include 
 
 #include "dumputils.h"
+#include "pg_backup.h"
 
 #include "parser/keywords.h"
 
@@ -1262,3 +1263,32 @@ exit_horribly(const char *modulename, const char *fmt,...)
 
 	exit(1);
 }
+
+/*
+ * Set the bitmask in dumpSections according to the first argument.
+ * dumpSections is initialised as DUMP_UNSECTION

Re: [HACKERS] JSON for PG 9.2

2011-12-12 Thread Josh Berkus
Bruce,

I thought that Joseph Adams was still working on this, sponsored by
Heroku.  Joseph?


-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

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


Re: [HACKERS] JSON for PG 9.2

2011-12-12 Thread Andrew Dunstan



On 12/12/2011 03:54 PM, Robert Haas wrote:

On Mon, Dec 12, 2011 at 3:38 PM, Simon Riggs  wrote:

Rather than fuss with specific data formats, why not implement
something a little more useful?

At present we can have typmods passed as a cstring, so it should be
possible to add typmods onto the TEXT data type.

e.g. TEXT('JSON'), TEXT('JSONB')



[...]

   There are way too many places that assume that the typmod can
just be discarded.  I don't think that's going to fly, because
=(text,text) probably has different semantics from =(json,json).



And certain places where they are not allowed at all, I think (unless I 
am misremembering the early debates about enum types and output functions).


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] Is anybody actually using XLR_BKP_REMOVABLE?

2011-12-12 Thread Tom Lane
Simon Riggs  writes:
> On Mon, Dec 12, 2011 at 3:42 PM, Tom Lane  wrote:
>> It occurs to me also that we could just move the flag from
>> per-WAL-record info bytes to per-page or even per-segment WAL headers.
>> Because we now force a segment switch when starting a backup, the
>> flag would be seen turned-on soon enough to prevent problems.
>> Finding out that it's off again after the end of a backup might be
>> a little delayed, but the only cost is failure to compress a few
>> compressible records.
>> 
>> I'm not volunteering to do the above, unless someone steps forward
>> to say that there's active use of this flag, but either one of these
>> solutions seems more tenable than using up an info-byte bit.

> I'll volunteer. Assume you can reuse the flag and I will patch afterwards.

Thanks for the offer, but after thinking about it a bit more I realized
that this change is quite trivial, so I just went ahead and did it along
with the change in XLR_MAX_BKP_BLOCKS.  This seems better since both
related changes are in one commit, and we can't forget to do it.

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] JSON for PG 9.2

2011-12-12 Thread David E. Wheeler
On Dec 12, 2011, at 12:54 PM, Robert Haas wrote:

> I don't think that's going to fly, because
> =(text,text) probably has different semantics from =(json,json).

No question:

david=# select '{"foo": 1, "bar": 2}'::json = '{"bar": 2, "foo": 1}'::json;
 ?column? 
--
 t
(1 row)

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] JSON for PG 9.2

2011-12-12 Thread Peter Eisentraut
On mån, 2011-12-12 at 21:08 +, Simon Riggs wrote:
> On Mon, Dec 12, 2011 at 8:54 PM, Robert Haas  wrote:
> 
> > There are way too many places that assume that the typmod can
> > just be discarded.
> 
> If true, that probably ought to be documented cos it sounds fairly important.
> 
> Where and when is it true?

Function arguments and return values, for example.



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


Re: [HACKERS] JSON for PG 9.2

2011-12-12 Thread Robert Haas
On Mon, Dec 12, 2011 at 4:08 PM, Simon Riggs  wrote:
> On Mon, Dec 12, 2011 at 8:54 PM, Robert Haas  wrote:
>> There are way too many places that assume that the typmod can
>> just be discarded.
>
> If true, that probably ought to be documented cos it sounds fairly important.
>
> Where and when is it true?

I'm not going to go compile an exhaustive list, since that would take
a week and I don't have any particular desire to invest that much time
in it, but just to take a couple of simple examples:

rhaas=# create or replace function wuzzle(numeric(5,2)) returns int as
$$select 1$$ language sql;
CREATE FUNCTION
rhaas=# \df wuzzle
 List of functions
 Schema |  Name  | Result data type | Argument data types |  Type
++--+-+
 public | wuzzle | numeric  | | normal
 public | wuzzle | integer  | numeric | normal
(2 rows)

rhaas=# select pg_typeof(1.23::numeric(5,2));
 pg_typeof
---
 numeric
(1 row)

There are a very large number of others.  Possibly grepping for places
where we do getBaseType() rather than getBaseTypeAndTypmod() would be
a way to find some of them.

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

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


Re: [HACKERS] JSON for PG 9.2

2011-12-12 Thread Simon Riggs
On Mon, Dec 12, 2011 at 8:54 PM, Robert Haas  wrote:

> There are way too many places that assume that the typmod can
> just be discarded.

If true, that probably ought to be documented cos it sounds fairly important.

Where and when is it true?

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

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


Re: [HACKERS] [PATCH] Caching for stable expressions with constant arguments v3

2011-12-12 Thread Tom Lane
Marti Raudsepp  writes:
> The 'struct Expr' type could have another attribute that stores
> whether its sub-expressions contain stable/volatile functions, and
> whether it only contains of constant arguments. This attribute would
> be filled in for every Expr by eval_const_expressions.

Hmm.  I'm a bit hesitant to burn the extra space that would be required
for that.  On the other hand it is a relatively clean solution to
postponing the whether-to-actually-cache decisions until runtime.
But on the third hand, I don't know whether this is really much cleaner
than always injecting CacheExpr and having some runtime flag that
prevents it from caching.  The argument that CacheExpr could confuse
equal() checks applies just as much to this, unless you make some ad-hoc
decision that equal() should ignore these flag bits.

> This would decouple expression caching from the planner entirely;
> CacheExpr wouldn't exist anymore. AFAICT this would also let us get
> rid of contain_mutable_functions_walker() and possibly others.

Yeah, you could imagine getting rid of tree walks for both mutability
and returns-set tests, and maybe other things too --- once you've paid
the price of an expression attributes flag word, there's room for quite
a few bits in there.  Don't know offhand if that's attractive enough to
tip the scales.

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] includeifexists in configuration file

2011-12-12 Thread Ross Reedstrom
On Mon, Dec 12, 2011 at 02:24:53PM -0500, Greg Smith wrote:
> On 11/16/2011 10:19 AM, Robert Haas wrote:
> >I haven't read the code yet, but just to get the bikeshedding started,
> >I think it might be better to call this include_if_exists rather than
> >running it together as one word.
> 
> What's going on, it's like this bikeshed just disappeared.  I should
> figure out how that happened so I can replicate it.

Must be that special "camo" paint.


Ross
Woohoo! Caught up from my beginning of Oct. trip backlog, just in time for 
Christmas!
-- 
Ross Reedstrom, Ph.D. reeds...@rice.edu
Systems Engineer & Admin, Research Scientistphone: 713-348-6166
Connexions  http://cnx.orgfax: 713-348-3665
Rice University MS-375, Houston, TX 77005
GPG Key fingerprint = F023 82C8 9B0E 2CC6 0D8E  F888 D3AE 810E 88F0 BEDE

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


Re: [HACKERS] foreign key locks, 2nd attempt

2011-12-12 Thread Alvaro Herrera

Excerpts from Alvaro Herrera's message of lun dic 12 17:20:39 -0300 2011:

> I found that this is caused by mxid_to_string being leaked all over the
> place :-(  I "fixed" it by making the returned string be a static that's
> malloced and then freed on the next call.  There's still virtsize growth
> (not sure it's a legitimate leak) with that, but it's much smaller.

this fixes the remaining leaks.  AFAICS it now grows to a certain point
and it's fixed size after that.  I was able to share-lock a 10M rows
table with a 30MB RSS process.

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 49d3369..7069950 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -3936,6 +3936,8 @@ l3:
keep_xmax = xwait;
keep_xmax_multi = true;
}
+
+   pfree(members);
}
}
else if (infomask & HEAP_XMAX_KEYSHR_LOCK)
@@ -4693,6 +4695,9 @@ GetMultiXactIdHintBits(MultiXactId multi)
if (!has_update)
bits |= HEAP_XMAX_IS_NOT_UPDATE;
 
+   if (nmembers > 0)
+   pfree(members);
+
return bits;
 }
 
@@ -4743,6 +4748,8 @@ HeapTupleGetUpdateXid(HeapTupleHeader tuple)
break;
 #endif
}
+
+   pfree(members);
}
 
return update_xact;

-- 
Á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] JSON for PG 9.2

2011-12-12 Thread Robert Haas
On Mon, Dec 12, 2011 at 3:38 PM, Simon Riggs  wrote:
> Rather than fuss with specific data formats, why not implement
> something a little more useful?
>
> At present we can have typmods passed as a cstring, so it should be
> possible to add typmods onto the TEXT data type.
>
> e.g. TEXT('JSON'), TEXT('JSONB')
>
> We then invent a new catalog table called pg_text_format which has
> oid PRIMARY KEY
> textformatname UNIQUE
> textformatvalidationproc
> textformatstorageproc
>
> The typmod must reduce to a single integer, so we just store the
> integer. If no typmod, we store 0, so we have a fastpath for normal
> TEXT datatypes.
>
> This would then allow people to have variations of the TEXT type that
> supports conversions, casts, indexing etc without additional fuss and
> without everything else outside the database breaking because it
> doesn't know that datatype name.
>
> We could then support JSON (both kinds), YAML, etc
> as well as providing a way to add validation into the datatype itself.
> We can replace citext with TEXT('CASE_INSENSITIVE')
>
> Think of this as using the object-relational capabilities of Postgres
> to extend the TEXT data type.

Well, it's arguable that text-format JSON and YAML and our existing
XML datatype ought to share some structure with text, but
binary-format JSON is a different animal altogether; you might as well
propose having text('int8').  In any case, I doubt that trying to make
the typmod provide subclassing behavior is going to work out very
well.  There are way too many places that assume that the typmod can
just be discarded.  I don't think that's going to fly, because
=(text,text) probably has different semantics from =(json,json).

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

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


Re: [HACKERS] [PATCH] Caching for stable expressions with constant arguments v3

2011-12-12 Thread Marti Raudsepp
On Sun, Dec 4, 2011 at 22:53, Heikki Linnakangas
 wrote:
> I wonder if it would be better to add the CacheExpr nodes to the tree as a
> separate pass, instead of shoehorning it into eval_const_expressions? I
> think would be more readable that way, even though a separate pass would be
> more expensive. And there are callers of eval_const_expressions() that have
> no use for the caching, like process_implied_equality().

I had an idea how to implement this approach with minimal performance
impact. It might well be a dumb idea, but I wanted to get it out
there.

The 'struct Expr' type could have another attribute that stores
whether its sub-expressions contain stable/volatile functions, and
whether it only contains of constant arguments. This attribute would
be filled in for every Expr by eval_const_expressions.

Based on this information, ExecInitExpr (which is pretty simple for
now) could decide where to inject CacheExprState nodes. It's easier to
make that decision there, since by that stage the cachability of the
parent node with each argument is already known. (Currently I have to
stick all cachable arguments in a temporary list until I've determined
whether all arguments are cachable, or just some of them are)

This would decouple expression caching from the planner entirely;
CacheExpr wouldn't exist anymore. AFAICT this would also let us get
rid of contain_mutable_functions_walker() and possibly others.

Regards,
Marti

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


Re: [HACKERS] JSON for PG 9.2

2011-12-12 Thread Simon Riggs
On Mon, Dec 12, 2011 at 7:58 PM, Robert Haas  wrote:
> On Mon, Dec 5, 2011 at 3:12 PM, Bruce Momjian  wrote:
>> Where are we with adding JSON for Postgres 9.2?  We got bogged down in
>> the data representation last time we discussed this.
>
> We're waiting for you to send a patch that resolves all
> previously-raised issues.  :-)
>
> In all seriousness, I think the right long-term answer here is to have
> two data types - one that simply validates JSON and stores it as text,
> and the other of which uses some binary encoding.  The first would be
> similar to our existing xml datatype and would be suitable for cases
> when all or nearly all of your storage and retrieval operations will
> be full-column operations, and the json types is basically just
> providing validation.  The second would be optimized for pulling out
> (or, perhaps, replacing) pieces of arrays or hashes, but would have
> additional serialization/deserialization overhead when working with
> the entire value.  As far as I can see, these could be implemented
> independently of each other and in either order, but no one seems to
> have yet found the round tuits.

Rather than fuss with specific data formats, why not implement
something a little more useful?

At present we can have typmods passed as a cstring, so it should be
possible to add typmods onto the TEXT data type.

e.g. TEXT('JSON'), TEXT('JSONB')

We then invent a new catalog table called pg_text_format which has
oid PRIMARY KEY
textformatname UNIQUE
textformatvalidationproc
textformatstorageproc

The typmod must reduce to a single integer, so we just store the
integer. If no typmod, we store 0, so we have a fastpath for normal
TEXT datatypes.

This would then allow people to have variations of the TEXT type that
supports conversions, casts, indexing etc without additional fuss and
without everything else outside the database breaking because it
doesn't know that datatype name.

We could then support JSON (both kinds), YAML, etc
as well as providing a way to add validation into the datatype itself.
We can replace citext with TEXT('CASE_INSENSITIVE')

Think of this as using the object-relational capabilities of Postgres
to extend the TEXT data type.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

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


Re: [HACKERS] Command Triggers

2011-12-12 Thread Dimitri Fontaine
Robert Haas  writes:
> It seems to me (and it may seem differently to other people) that what
> most people who want to trap DDL events really want to do is either

  [ detailed analysis, mostly right on spot ]

Yeah, I'm proposing a rather crude tool.  I think it's still solving
real problems we have now, and that no other tool nor design is solving
for us.

So for me the real questions are:

 - do we want the feature in that form?
 - are we avoiding to paint ourselves into a corner?

I think both answers are positive, because I want the feature bad enough
to have spent time working on it, and it's able to solve two problems in
one stone already.  Also, the only way to have that feature in an
extension implementing ProcessUtility_hook is duplicating what I've been
doing, minus grammar support (just because you can't).

Also that's not the kind of efforts that either slony or londiste will
put into their own project, the amount of work wouldn't be justified for
this only return, as history is telling us.  (there's a game changer
here though, which is the idea of doing “command triggers” as opposed to
“ddl triggers” or even “catalog triggers”, thanks Jan)

Then, we can expand the trigger function signature down the road and
keep the current one as a compatibility support.  For example we could
add a cascading boolean argument and decide whether or not to call
the trigger function when cascading based upon the trigger's procedure
signature.

So I believe it's somewhat coarse or crude, still useful enough, and not
painting us into a corner.
 
> using a security definer function as a DDL instead-of trigger is an
> interesting one, but does that leave newly created objects owned by
> the wrong user?  Or what if you want user A to be able to do some but
> not all of the things user B can do?  Drilling an optimally-sized hole
> through the existing permissions system is unfortunately not very
> simple.

You can still use ALTER OBJECT … OWNER TO …; from within the security
definer function to allow the setup you're interested into.  I guess
that you can still have some conditional on current_user from the code
too, but I didn't check that we have user id and effective user id both
available from the SQL level (yet).

All in all, I understand your reticence here, but I'm not sharing it,

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et 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] foreign key locks, 2nd attempt

2011-12-12 Thread Alvaro Herrera

Noah,

Many thanks for this review.  I'm going through items on it; definitely
there are serious issues here, as well as minor things that also need
fixing.  Thanks for all the detail.

I'll post an updated patch shortly (probably not today though); in the
meantime, this bit:

Excerpts from Noah Misch's message of dom dic 04 09:20:27 -0300 2011:

> Second, I tried a SELECT FOR SHARE on a table of 1M tuples; this might incur
> some cost due to the now-guaranteed use of pg_multixact for FOR SHARE.  See
> attached fklock-test-forshare.sql.  The median run slowed by 7% under the
> patch, albeit with a rather brief benchmark run.  Both master and patched
> PostgreSQL seemed to exhibit a statement-scope memory leak in this test case:
> to lock 1M rows, backend-private memory grew by about 500M.  When trying 10M
> rows, I cancelled the query after 1.2 GiB of consumption.  This limited the
> duration of a convenient test run.

I found that this is caused by mxid_to_string being leaked all over the
place :-(  I "fixed" it by making the returned string be a static that's
malloced and then freed on the next call.  There's still virtsize growth
(not sure it's a legitimate leak) with that, but it's much smaller.
This being a debugging aid, I don't think there's any need to backpatch
this.  

diff --git a/src/backend/access/transam/multixact.c 
b/src/backend/access/transam/multixact.c
index ddf76b3..c45bd36 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -1305,9 +1305,14 @@ mxstatus_to_string(MultiXactStatus status)
 static char *
 mxid_to_string(MultiXactId multi, int nmembers, MultiXactMember *members)
 {
-   char   *str = palloc(15 * (nmembers + 1) + 4);
+   static char*str = NULL;
int i;
 
+   if (str != NULL)
+   free(str);
+
+   str = malloc(15 * (nmembers + 1) + 4);
+
snprintf(str, 47, "%u %d[%u (%s)", multi, nmembers, members[0].xid,
 mxstatus_to_string(members[0].status));
 

-- 
Á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] static or dynamic libpgport

2011-12-12 Thread Andrew Dunstan



On 12/12/2011 02:59 PM, Tom Lane wrote:

Peter Eisentraut  writes:

On lör, 2011-12-10 at 20:26 -0500, Tom Lane wrote:

Right now, libpq laboriously rebuilds all the .o files it needs from
src/port/ so as to get them with -fpic.  It would be nice if we could
clean that up while we're doing this.  It might be all right to always
build the client-side version of libpgport with -fpic, though I'd be sad
if that leaked into the server-side build.

So would we continue to build the client binaries (psql, pg_dump, etc.)
against the static libpgport.a, thus keeping it "invisible" there, or
would we dynamically link them, thus creating a new dependency.

I think that if possible we should avoid creating a new dependency for
either the client binaries or libpq.so itself; what I suggest above
is only a simplification of the build process for libpq.  If we create
a new dependency we risk packagers breaking things by forgetting to
include it.




OK, I'll work on this basis. The downside is that we'll be building it 
but not using it, but I can see the advantages.


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] psql output locations

2011-12-12 Thread Peter Eisentraut
On mån, 2011-12-12 at 14:47 +0100, Magnus Hagander wrote:
> We're either pretty inconsistent with our output in psql, or I'm not
> completely understanding it.. I was trying to implement a switch that
> would let me put all the output in the query output channel controlled
> by \o, and not just the output of the query itself. Because that would
> make it possible to control it from inside the script. Now, this made
> me notice:
> 
> * There are 102 calls to psql_error(), 42 direct uses of
> fprintf(stderr), and 7 uses of fputs(stderr). And there is also
> write_stderr() used in the cancel handler. Is there actually a point
> behind all those direct uses, or should they really be psql_error()
> calls?

Some of this could probably be more more uniform.  But I don't see how
this related to your question.  All the output goes uniformly to stderr,
which is what matters.

> * There are a number of things that are always written to stdout, that
> there is no way to redirect. In some cases it's interactive prompts -
> makes sense - but also for example the output of \timing goes to
> stdout always. Is there some specific logic behind what/when this
> should be done?

Everything that is not an error goes to stdout, no?  Except the query
output, if you change it.

Maybe the way to do what you want is to invent a new setting that
temporarily changes stdout.



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


Re: [HACKERS] static or dynamic libpgport

2011-12-12 Thread Tom Lane
Peter Eisentraut  writes:
> On lör, 2011-12-10 at 20:26 -0500, Tom Lane wrote:
>> Right now, libpq laboriously rebuilds all the .o files it needs from
>> src/port/ so as to get them with -fpic.  It would be nice if we could
>> clean that up while we're doing this.  It might be all right to always
>> build the client-side version of libpgport with -fpic, though I'd be sad
>> if that leaked into the server-side build.

> So would we continue to build the client binaries (psql, pg_dump, etc.)
> against the static libpgport.a, thus keeping it "invisible" there, or
> would we dynamically link them, thus creating a new dependency.

I think that if possible we should avoid creating a new dependency for
either the client binaries or libpq.so itself; what I suggest above
is only a simplification of the build process for libpq.  If we create
a new dependency we risk packagers breaking things by forgetting to
include it.

The Fedora/RHEL rule against static libraries is meant to prevent
situations where changes in a library would require rebuilding other
packages to get the fixes in place.  If we had to make a quick security
fix in libpq, for example, it would suck if dozens of other packages had
to be rebuilt to propagate the change everywhere.  However, I don't think
that concern applies to programs that are in the same source package as
the library --- they'd get rebuilt anyway.  So I see nothing wrong with
continuing to statically link these .o files into files belonging to the
postgresql package.  It's just that I can't export them in a .a file for
*other* source packages to use.

(Whether a security issue in libpgport is really likely to happen is not
a question that this policy concerns itself with ...)

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] JSON for PG 9.2

2011-12-12 Thread Robert Haas
On Mon, Dec 5, 2011 at 3:12 PM, Bruce Momjian  wrote:
> Where are we with adding JSON for Postgres 9.2?  We got bogged down in
> the data representation last time we discussed this.

We're waiting for you to send a patch that resolves all
previously-raised issues.  :-)

In all seriousness, I think the right long-term answer here is to have
two data types - one that simply validates JSON and stores it as text,
and the other of which uses some binary encoding.  The first would be
similar to our existing xml datatype and would be suitable for cases
when all or nearly all of your storage and retrieval operations will
be full-column operations, and the json types is basically just
providing validation.  The second would be optimized for pulling out
(or, perhaps, replacing) pieces of arrays or hashes, but would have
additional serialization/deserialization overhead when working with
the entire value.  As far as I can see, these could be implemented
independently of each other and in either order, but no one seems to
have yet found the round tuits.

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

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


Re: [HACKERS] static or dynamic libpgport

2011-12-12 Thread Peter Eisentraut
On fre, 2011-12-09 at 11:13 -0500, Andrew Dunstan wrote:
> Is there any good reason why we shouldn't build and install a dynamic 
> libpgport.so?

Just note, if you do this, you need to carefully manage API, ABI,
soname, symbol list, and all that.  Every time you tweak configure's
decision about when to include a replacement function, you need to
change the library version.  Every time you remove a function, you need
to change the soname.  Every backpatched portability fix has the
potential to escalate to a full shared library versioning dance.
Downstream packagers will be delighted, especially if this requires
changing the package name every three minor releases.

To see what this can lead to in the extreme, check the dependencies that
bind has on its internal libraries:

bind9 depends: libbind9-60, libdns69, libisc62, libisccc60, libisccfg62, 
liblwres60



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


Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-12-12 Thread Kevin Grittner
Yeb Havinga  wrote:
 
> Forgot to copy regression output to expected - attached v7 fixes
> that.
 
This version addresses all of my concerns.  It applies cleanly and
compiles without warning against current HEAD and performs as
advertised.  I'm marking it Ready for Committer.
 
-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] static or dynamic libpgport

2011-12-12 Thread Peter Eisentraut
On lör, 2011-12-10 at 20:26 -0500, Tom Lane wrote:
> > The other 
> > thing is we'd need to turn on flags that make the object suitable for a 
> > dynamic library (e.g. -fpic).
> 
> Right now, libpq laboriously rebuilds all the .o files it needs from
> src/port/ so as to get them with -fpic.  It would be nice if we could
> clean that up while we're doing this.  It might be all right to always
> build the client-side version of libpgport with -fpic, though I'd be sad
> if that leaked into the server-side build.

So would we continue to build the client binaries (psql, pg_dump, etc.)
against the static libpgport.a, thus keeping it "invisible" there, or
would we dynamically link them, thus creating a new dependency.



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


Re: [HACKERS] Is anybody actually using XLR_BKP_REMOVABLE?

2011-12-12 Thread Simon Riggs
On Mon, Dec 12, 2011 at 5:51 PM, Tom Lane  wrote:
> Jesper Krogh  writes:
>>> So: is there actually any such compression program out there?
>
>> Perhaps http://pglesslog.projects.postgresql.org/
>
> Hah ... the search facilities on pgfoundry really do leave something to
> be desired :-(
>
> So I guess we should try to preserve the functionality.  I think the
> move-the-flag-to-the-segment-header idea is probably the best.

Yes, that's the best idea.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

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


Re: [HACKERS] pg_dump --exclude-table-data

2011-12-12 Thread Andrew Dunstan



On 12/08/2011 11:36 AM, Andrew Dunstan wrote:



On 12/08/2011 11:13 AM, Robert Haas wrote:

Ah, hmm.  Well, maybe it's fine the way that you have it.  But I'd be
tempted to at least add a sentence to the sgml documentation for each
option referring to the other, e.g. "To dump only schema for all
tables in the database, see --schema-only." and "To exclude table data
for only a subset of tables in the database, see
--exclude-table-data.", or something along those lines.



Sure, no problem with that.




Revised patch attached.

cheers

andrew
*** a/doc/src/sgml/ref/pg_dump.sgml
--- b/doc/src/sgml/ref/pg_dump.sgml
***
*** 404,409  PostgreSQL documentation
--- 404,413 
 
  Dump only the object definitions (schema), not data.
 
+
+ To exclude table data for only a subset of tables in the database, 
+ see --exclude-table-data.
+

   
  
***
*** 612,617  PostgreSQL documentation
--- 616,639 
   
  
   
+   --exclude-table-data=table
+   
+
+ Do not dump data for any tables matching the table pattern.  The pattern is
+ interpreted according to the same rules as for -t.
+ --exclude-table-data can be given more than once to 
+ exclude tables matching any of several patterns. This option is
+ useful when you need the definition of a particular table even
+ though you do not need the data in it.
+
+
+ To exclude data for all tables in the database, see --schema-only.
+
+   
+  
+ 
+  
--inserts

 
*** a/src/bin/pg_dump/pg_dump.c
--- b/src/bin/pg_dump/pg_dump.c
***
*** 115,120  static SimpleStringList table_include_patterns = {NULL, NULL};
--- 115,122 
  static SimpleOidList table_include_oids = {NULL, NULL};
  static SimpleStringList table_exclude_patterns = {NULL, NULL};
  static SimpleOidList table_exclude_oids = {NULL, NULL};
+ static SimpleStringList tabledata_exclude_patterns = {NULL, NULL};
+ static SimpleOidList tabledata_exclude_oids = {NULL, NULL};
  
  /* default, if no "inclusion" switches appear, is to dump everything */
  static bool include_everything = true;
***
*** 324,329  main(int argc, char **argv)
--- 326,332 
  		{"column-inserts", no_argument, &column_inserts, 1},
  		{"disable-dollar-quoting", no_argument, &disable_dollar_quoting, 1},
  		{"disable-triggers", no_argument, &disable_triggers, 1},
+ 		{"exclude-table-data", required_argument, NULL, 4},
  		{"inserts", no_argument, &dump_inserts, 1},
  		{"lock-wait-timeout", required_argument, NULL, 2},
  		{"no-tablespaces", no_argument, &outputNoTablespaces, 1},
***
*** 487,492  main(int argc, char **argv)
--- 490,499 
  use_role = optarg;
  break;
  
+ 			case 4:			/* exclude table(s) data */
+ simple_string_list_append(&tabledata_exclude_patterns, optarg);
+ break;
+ 
  			default:
  fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
  exit(1);
***
*** 715,720  main(int argc, char **argv)
--- 722,731 
  	}
  	expand_table_name_patterns(&table_exclude_patterns,
  			   &table_exclude_oids);
+ 
+ 	expand_table_name_patterns(&tabledata_exclude_patterns,
+ 			   &tabledata_exclude_oids);
+ 
  	/* non-matching exclusion patterns aren't an error */
  
  	/*
***
*** 854,859  help(const char *progname)
--- 865,871 
  	printf(_("  --column-insertsdump data as INSERT commands with column names\n"));
  	printf(_("  --disable-dollar-quotingdisable dollar quoting, use SQL standard quoting\n"));
  	printf(_("  --disable-triggers  disable triggers during data-only restore\n"));
+ 	printf(_("  --exclude-table-data=TABLE  do NOT dump data for the named table(s)\n"));
  	printf(_("  --inserts   dump data as INSERT commands, rather than COPY\n"));
  	printf(_("  --no-security-labelsdo not dump security label assignments\n"));
  	printf(_("  --no-tablespacesdo not dump tablespace assignments\n"));
***
*** 1087,1092  selectDumpableTable(TableInfo *tbinfo)
--- 1099,1113 
  		simple_oid_list_member(&table_exclude_oids,
  			   tbinfo->dobj.catId.oid))
  		tbinfo->dobj.dump = false;
+ 
+ 	/* If table is to be dumped, check that the data is not excluded */
+ 	if (tbinfo->dobj.dump && !
+ 		simple_oid_list_member(&tabledata_exclude_oids,
+ 			   tbinfo->dobj.catId.oid))
+ 		tbinfo->dobj.dumpdata = true;
+ 	else
+ 		tbinfo->dobj.dumpdata = false;
+ 		
  }
  
  /*
***
*** 1518,1523  dumpTableData(Archive *fout, TableDataInfo *tdinfo)
--- 1539,1548 
  	DataDumperPtr dumpFn;
  	char	   *copyStmt;
  
+ 	/* don't do anything if the data isn't wanted */
+ 	if (!tbinfo->dobj.dumpdata)
+ 		return;
+ 
  	if (!dump_inserts)
  	{
  		/* Dump/restore using COPY */
*** a/src/b

Re: [HACKERS] includeifexists in configuration file

2011-12-12 Thread Greg Smith

On 11/16/2011 10:19 AM, Robert Haas wrote:

I haven't read the code yet, but just to get the bikeshedding started,
I think it might be better to call this include_if_exists rather than
running it together as one word.
   


What's going on, it's like this bikeshed just disappeared.  I should 
figure out how that happened so I can replicate it.


This naming style change sounds fine to me, and I just adopted it for 
the updated configuration directory patch.  That patch now rearranges 
the documentation this feature modifies.  This is a pretty trivial 
feature I'm not real concerned about getting a review for.  I'll update 
this with the name change and appropriate rebased patch once some 
decision has been made about that one; will just bounce this forward to 
January if it's still here when the current CF starts closing in earnest.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us


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


Re: [HACKERS] Command Triggers

2011-12-12 Thread Robert Haas
On Mon, Dec 12, 2011 at 11:49 AM, Andres Freund  wrote:
>> I haven't yet thought about your specific proposal here in enough to
>> have a fully-formed opinion, but I am a little nervous that this may
>> turn out to be one of those cases where the obvious API ends up
>> working less well than might have been supposed.
> What are your thoughts about a "not-obvious api"?

It seems to me (and it may seem differently to other people) that what
most people who want to trap DDL events really want to do is either
(a) get control at the permissions-checking stage so that they can
override the system's default access control policy, either to allow
something that would normally be denied (as in Dimitri's sudo example)
or deny something that would normally be allowed (as with sepgsql, or
the example in the documentation for Dimitri's patch showing how to
enforce relation naming conventions) or else (b) perform some
additional steps after completing the operation (e.g. audit it,
replicate it, apply a security label to it).  Much of the
selinux-related work that KaiGai Kohei and I have been doing over the
last couple of years has revolved around where to put those hooks and
what information to pass to them.

For example, we added some post-creation hooks where the system
basically says "we just created a  and its OID is
".  I'm not going to present this as a model of
usability (because it isn't).  However, I think the underlying model
is worth considering.  It's not a command trigger, because it doesn't
care what command the user executed that led to the object getting
created.  It's more like an event system - whenever a table gets
created (no matter how exactly that happens), you get a callback.  I
think that's probably closer to what people want for the
additional-steps-after-completing-the-operation use case.  Typically,
you're not really interested in what the user typed: you want to know
what the system decided to do about it.

The "get control at the permissions checking stage" use case is a bit
harder.  We've added some hooks for that as well, but really only for
DML thus far, and I'm not convinced we've got the right design even
there.  Part of the complexity here is that there are actually a lot
of different permissions rules depending on exactly what operation you
are performing - you might think of REINDEX TABLE, ALTER TABLE ..
ALTER COLUMN .. RENAME and DROP TABLE as requiring the same
permissions (i.e. ownership) but in fact they're all slightly
different.  It would be nice if the permissions checking machinery
were better separated from the code that actually does the work, so
that you could rip and replace just that part.  Dimitri's idea of
using a security definer function as a DDL instead-of trigger is an
interesting one, but does that leave newly created objects owned by
the wrong user?  Or what if you want user A to be able to do some but
not all of the things user B can do?  Drilling an optimally-sized hole
through the existing permissions system is unfortunately not very
simple.

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

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


Re: [HACKERS] IDLE in transaction introspection

2011-12-12 Thread Magnus Hagander
On Wed, Dec 7, 2011 at 17:45, Scott Mead  wrote:
>
> On Tue, Dec 6, 2011 at 6:38 AM, Magnus Hagander  wrote:
>>
>> On Sat, Nov 19, 2011 at 02:55, Scott Mead  wrote:
>> >
>> > On Thu, Nov 17, 2011 at 11:58 AM, Scott Mead  wrote:
>> >>
>> >> On Wed, Nov 16, 2011 at 4:09 PM, Scott Mead  wrote:
>> >>>
>> >>> On Tue, Nov 15, 2011 at 1:18 PM, Robert Treat  wrote:
>> 
>>  On Tue, Nov 15, 2011 at 12:00 PM, Greg Smith 
>>  wrote:
>>  > On 11/15/2011 09:44 AM, Scott Mead wrote:
>>  >>
>>  >> Fell off the map last week, here's v2 that:
>>  >>  * RUNNING => active
>>  >>  * all states from CAPS to lower case
>>  >>
>>  >
>>  > This looks like what I was hoping someone would add here now.
>>  >  Patch
>>  > looks
>>  > good, only issue I noticed was a spaces instead of a tab goof where
>>  > you set
>>  > beentry_>st_state at line 2419 in src/backend/postmaster/pgstat.c
>>  >
>>  > Missing pieces:
>>  >
>>  > -There is one regression test that uses pg_stat_activity that is
>>  > broken now.
>>  > -The documentation should list the new field and all of the states
>>  > it
>>  > might
>>  > include.  That's a serious doc update from the minimal info
>>  > available
>>  > right
>>  > now.
>> >
>> >
>> > V3 Attached:
>> >
>> >   * Updates Documentation
>> >     -- Adds a separate table to cover each column of pg_stat_activity
>>
>> I like that a lot - we should've done taht years ago :-)
>>
>> For consistency, either both datname and usename should refer to their
>> respective oid, or none of them.
>>
>>
>> application_name  should probably have a link to the libpq
>> documentation for said parameter.
>>
>> "field is not set" should probably be "field is NULL"
>>
>
> Thanks for pointing these out.
>
>>
>> "Boolean, if the query is waiting because of a block / lock" reads
>> really strange to me. "Boolean indicating if" would be a good start,
>> but I'm not sure what you're trying to say with "block / lock" at all?
>
>
> Yeah, me neither.  What about:
>    "Boolean indicating if a query is in a wait state due to a conflict with
> another backend."

Maybe "Boolean indicating if a backend is currently waiting on a lock"?


>> The use of single quotes in the descriptions, things like "This is the
>> 'state' of" seems wrong. If we should use anything, it should be
>> double quotes, but why do we need double quotes around that? And the
>> reference to "think time" is just strange, IMO.
>>
> Yeah, that's a 'Scottism' (see, I used it again :-).  I'll clean that up

:-)


>> In some other cases, the single quotes should probably be replaced
>> with .
>>
>> NB: should probably be .
>>
>
> I am actually looking for some helping in describing the "
> function call" state.  Any takers?

Not sure how detailed you have to be - since it's a fairly uncommon
thing, you can probalby get away with something as simple as
"executing a fast-path function" or something like that?

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

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


Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

2011-12-12 Thread Jeff Davis
On Fri, 2011-12-02 at 15:48 +0400, Alexander Korotkov wrote:
> Rebased with head.
> 
Thank you. I have attached a patch that's mostly just cleanup to this
one.

Comments:

* You use the term "ordinal range" quite a lot, which I haven't heard
before. Is that a mathematical term, or do you mean something more like
"ordinary"?

* There's a lot of code for range_gist_penalty. Rather than having
special cases for all combinations of properties in the new an original,
is it possible to use something a little simpler? Maybe just start the
penalty at zero, and add something for each property of the predicate
range that must be changed. The penalties added might vary, e.g., if the
original range has an infinite lower bound, changing it to have an
infinite upper bound might be a higher penalty.

* It looks like LIMIT_RATIO is not always considered. Should it be?

* You defined get/set_range_contain_empty, but didn't use them. I think
this was a merge error, but I removed them. So now there are no changes
in rangetypes.c.

Regards,
Jeff Davis


range_gist_cleanup.patch.gz
Description: GNU Zip compressed 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] Configuration include directory

2011-12-12 Thread Greg Smith

On 12/12/2011 01:34 PM, Greg Smith wrote:
You can see a snapshot of the new doc page I built at 
http://http://www.westnet.com/~gsmith/config-setting.html


One minute past send note on brain fade:  this section

include '00shared.conf'
include '01memory.conf'
include '02server.conf'


Was a pasto-o that was supposed to just be a list of files:

00shared.conf
01memory.conf
02server.conf

If that's the only thing I'm called on to change, I'll happily update patch and 
doc sample to reflect it.  It's a trivial and obvious fix to the documentation 
source.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us


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


Re: [HACKERS] Configuration include directory

2011-12-12 Thread Greg Smith
Attached is an update to my earlier patch.  This clears my own bug, 
usability concerns, and implementation ideas list on this one.


There's full documentation on this now, including some suggested ways 
all these include features might be used.  Since there's so much 
controversy around the way I would like to see things organized by 
default, instead of kicking that off again I just outline a couple of 
ways people might do things, showing both the flexibility and some good 
practices I've seen that people can adopt--or not.  Possibilities rather 
than policy.  The include-related section got big enough that it was 
really disruptive, so I moved it to a new sub-section.  I like the flow 
of this better, it slightly slimmed down the too big "Setting 
Parameters" section.  You can see a snapshot of the new doc page I built 
at http://http://www.westnet.com/~gsmith/config-setting.html


Here's a partial demo highlighting the updated ignoring logic (which I 
tested more extensively with some debugging messages about its 
decisions, then removed those).  Both .02test.conf and .conf appear, but 
neither are processed:


$ tail -n 1 $PGDATA/postgresql.conf
include_dir='conf.d'
$ ls -a $PGDATA/conf.d
.  ..  00test.conf  01test.conf  .02test.conf  .conf
$ cat $PGDATA/conf.d/00test.conf
work_mem = 2MB
$ cat $PGDATA/conf.d/01test.conf
work_mem = 3MB
$ cat $PGDATA/conf.d/.conf
checkpoint_segments=10
$ psql -x -c "select name,setting,sourcefile from pg_settings where 
name='work_mem'"

-[ RECORD 1 ]---
name   | work_mem
setting| 3072
sourcefile | /home/gsmith/pgwork/data/confdir/conf.d/01test.conf
$ psql -x -c "select name,setting,sourcefile from pg_settings where 
name='checkpoint_segments'"

-[ RECORD 1 ]---
name   | checkpoint_segments
setting| 3
sourcefile |

In addition to adding the docs, I changed these major things relative to 
the version that was reviewed:


-The configuration directory name is now considered relative to the 
directory that the including file is located in.  That's the same logic 
ParseConfigFile used to convert relative to absolute paths, and as Noah 
suggested it nicely eliminates several of the concerns I had about what 
I'd submitted before.  I was concerned before about creating a packaging 
problem, now I'm not.  Relocate postgresql.conf, and the include 
directory base goes with it, regardless of the method you used to do 
so.  The shared logic for this has been refactored into a new 
AbsoluteConfigLocation function that both ParseConfigFile and this new 
ParseConfigDirectory call.  That's an improvement to the readability of 
both functions too.


-With that change, all of the hackery around exporting configdir goes 
away too.  Which means that the code works as expected during SIGHUP 
reloads too.  I love it when a plan comes together.


-Hidden files (starts with ".") are now skipped.  This also eliminates 
the concern about whether ".conf" is considered a valid name for a file; 
it is clearly not.


-Per renaming suggestion from Robert to make my other submission use 
include_if_exists, I've made this one include_dir instead of includedir.


There's some new error handling:

-If the configuration directory does not exist at all, throw an error.  
It was quietly accepted before.  I'm not sure why Magnus had coded it 
that way originally, and I just passed that through without considering 
a change.  This still quietly accepts a directory that exists but has no 
files in it.  I consider that a reasonable behavior, but I wouldn't 
reject the idea of giving a warning when that happens, if someone feels 
that's appropriate here.


-When a file that was in the directory goes away before it is checked 
with stat, consider that an error too.


There are some internal changes that should eliminate the main concerns 
about Windows compatibility in particular:


-File name joining uses join_path_components, eliminating all sorts of bugs
-Use pstrdup() instead of strdup when building the list of files in the 
directory

-Switch from using guc_realloc to [re]palloc for that temporary file list.
-Eliminated now unneeded export of guc_malloc and guc_realloc
-Moved ParseConfigDirectory prototype to the right place
-Don't bother trying to free individual bits of memory now that it's all 
in the same context.  Saves some lines of code, and I do not miss the 
asserts I am no longer triggering.


The only review suggestion I failed to incorporate was this one from Noah:

> > > + if (strcmp(de->d_name + strlen(de->d_name) - 5, 
".conf") != 0)

> > + continue;
> That may as well be memcmp().

While true, his idea bothers both my abstraction and unexpected bug 
concern sides for reasons I can't really justify.  I seem to have 
received too many past beatings toward using the string variants of all 
these functions whenever operating on things that are clearly strings.  
I'll punt this one toward whoever

Re: [HACKERS] static or dynamic libpgport

2011-12-12 Thread Andrew Dunstan



On 12/10/2011 08:26 PM, Tom Lane wrote:



The other
thing is we'd need to turn on flags that make the object suitable for a
dynamic library (e.g. -fpic).

Right now, libpq laboriously rebuilds all the .o files it needs from
src/port/ so as to get them with -fpic.  It would be nice if we could
clean that up while we're doing this.  It might be all right to always
build the client-side version of libpgport with -fpic, though I'd be sad
if that leaked into the server-side build.



Here's a small diff that seems to build things the right way. No leakage 
of -fpic into the server side code. Still a deal of work to do, but it's 
a start.


Would we want to link our own non-backend executables against the shared 
lib? That would almost certainly break the buildfarm for Windows builds, 
as it only currently copies the libpq DLL into the bin directory. That's 
no reason on its own not to do it, of course, and there are only a 
couple of owners other than me anyway, so it would be easy to fix.


How do you want to proceed for libpq (and the ecpg library cases that do 
the same thing)? Just link in the object files directly?


cheers

andrew

*** Makefile2011-12-03 17:21:59.944509111 -0500
--- GNUmakefile2011-12-12 12:39:43.176260505 -0500
***
*** 37,47 
  # foo_srv.o and foo.o are both built from foo.c, but only foo.o has 
-DFRONTEND

  OBJS_SRV = $(OBJS:%.o=%_srv.o)

! all: libpgport.a libpgport_srv.a

! # libpgport is needed by some contrib
! install: all installdirs
! $(INSTALL_STLIB) libpgport.a '$(DESTDIR)$(libdir)/libpgport.a'

  installdirs:
  $(MKDIR_P) '$(DESTDIR)$(libdir)'
--- 37,52 
  # foo_srv.o and foo.o are both built from foo.c, but only foo.o has 
-DFRONTEND

  OBJS_SRV = $(OBJS:%.o=%_srv.o)

! NAME = pgport
! SO_MAJOR_VERSION= 1
! SO_MINOR_VERSION= 1

! include $(top_srcdir)/src/Makefile.shlib
!
! all: all-lib libpgport_srv.a
!
! # libpgport is needed by any exe built with pgxs
! install: all installdirs install-lib

  installdirs:
  $(MKDIR_P) '$(DESTDIR)$(libdir)'
***
*** 49,57 
  uninstall:
  rm -f '$(DESTDIR)$(libdir)/libpgport.a'

- libpgport.a: $(OBJS)
- $(AR) $(AROPT) $@ $^
-
  # thread.o needs PTHREAD_CFLAGS (but thread_srv.o does not)
  thread.o: thread.c
  $(CC) $(CFLAGS) $(CPPFLAGS) $(PTHREAD_CFLAGS) -c $<
--- 54,59 
***
*** 64,70 
  $(AR) $(AROPT) $@ $^

  %_srv.o: %.c
! $(CC) $(CFLAGS) $(subst -DFRONTEND,, $(CPPFLAGS)) -c $< -o $@

  $(OBJS_SRV): | submake-errcodes

--- 66,72 
  $(AR) $(AROPT) $@ $^

  %_srv.o: %.c
! $(CC) $(subst $(CFLAGS_SL),,$(CFLAGS)) $(subst -DFRONTEND,, 
$(CPPFLAGS)) -c $< -o $@


  $(OBJS_SRV): | submake-errcodes

***
*** 97,100 
  echo "#define MANDIR \"$(mandir)\"" >>$@

  clean distclean maintainer-clean:
! rm -f libpgport.a libpgport_srv.a $(OBJS) $(OBJS_SRV) 
pg_config_paths.h

--- 99,102 
  echo "#define MANDIR \"$(mandir)\"" >>$@

  clean distclean maintainer-clean:
! rm -f libpgport.so* libpgport.a libpgport_srv.a $(OBJS) 
$(OBJS_SRV) pg_config_paths.h



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


Re: [HACKERS] Is anybody actually using XLR_BKP_REMOVABLE?

2011-12-12 Thread Tom Lane
Jesper Krogh  writes:
>> So: is there actually any such compression program out there?

> Perhaps http://pglesslog.projects.postgresql.org/

Hah ... the search facilities on pgfoundry really do leave something to
be desired :-(

So I guess we should try to preserve the functionality.  I think the
move-the-flag-to-the-segment-header idea is probably the best.

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] Command Triggers

2011-12-12 Thread Dimitri Fontaine
Alvaro Herrera  writes:
> Yeah.  I remember mentioning the ability of CREATE SCHEMA to embed all
> sort of object creation commands in a single top-level command, and
> being handwaved away by Jan.  "Nobody knows about that", I was told.

In fact CREATE SCHEMA implementation will fire a process utility command
per element, see src/backend/commands/schemacmds.c:122

 * Execute each command contained in the CREATE SCHEMA.  Since the 
grammar
 * allows only utility commands in CREATE SCHEMA, there is no need to 
pass
 * them through parse_analyze() or the rewriter; we can just hand them
 * straight to ProcessUtility.

> However, if this is to work in a robust way, these things should not be
> ignored.  In particular, firing triggers on each top-level command _is_
> going to get some objects ignored; the people writing the triggers _are_
> going to forget to handle CREATE SCHEMA properly most of the time.

My current thinking is that the tool as proposed in the patch is useful
enough now already, and that we should cover its limitations in the
documentation.  And also be somehow verbose about the nodeToString()
output where to find almost all you need.

Down the road, we might decide that either what we've done is covering
enough needs (after all, we managed not to have the features for years)
or decide to have internal commands implementation (create index from
inside a create table statement, say) go through ProcessUtility.

Also note that currently, anyone motivated enough to write a
ProcessUtility hook (in C) will face the same problem here, the hook
will not get called for any event where the command trigger would not
get called either.

So while I understand your concerns here, I think they are rather
theoretical, as we can already add support for create table, alter
table, and drop table to slony, londiste and bucardo with the existing
patch, and also implement create extension whitelisting and allow
non-superuser to install selected C coded extensions.

FWIW, those are the two main use cases I'm after.

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et 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] [v9.2] Fix Leaky View Problem

2011-12-12 Thread Kohei KaiGai
The attached patches are cut-off version based on the latest Robert's
updates. The "v8.regtest" adds regression test cases on variable
leaky-view scenarios with/without security-barrier property.

The "v8.option-1" add checks around restriction_selectivity, and prevent
to invoke estimator function if Var node touches relation with security-
barrier attribute.

The "v8.option-2" add checks around examine_simple_variable, and
prevent to reference statistical data, if Var node tries to reference
relation with security-barrier attribute.
(And, it shall be marked as "leakproof")

I initially thought restriction_selectivity called by clause_selectivity is
the best point to add checks, however, I reconsidered it might not be
the origin of this problem.
As long as user-defined functions acquires control on selectivity
estimation of operators, same problems can be re-produced;
if someone tries to reference unrelated data within estimator.

This scenario is normally prevented, because only superuser can define
a function that can bypass permission checks to reference internal data
structures; using "untrusted" procedural-language.
If my conclusion is right, what we should fix up is built-in estimators side,
and we should enforce estimator function being "leakproof", even though
we still allow unprivileged users to define operators.

Thanks,

2011/12/11 Kohei KaiGai :
> 2011/12/10 Kohei KaiGai :
>> 2011/12/9 Robert Haas :
>>> I feel like there must be some logic in the planner somewhere that is
>>> "looking through" the subquery RTE and figuring out that safe_foo.a is
>>> really the same variable as foo.a, and which therefore feels entitled
>>> to apply !!'s selectivity estimator to foo.a's statistics.  If that's
>>> the case, it might be possible to handicap that logic so that when the
>>> security_barrier flag is set, it doesn't do that, and instead treats
>>> safe_foo.a as a black box.  That would, obviously, degrade the quality
>>> of complex plans involving security views, but I think we should focus
>>> on getting something that meets our security goals first and then try
>>> to improve performance later.
>>>
>> I tried to investigate the code around size-estimation, and it seems to
>> me here is two candidates to put this type of checks.
>>
>> The one is examine_simple_variable() that is used to pull a datum
>> from statistic information, but it locates on the code path restriction
>> estimator of operators; so user controlable, although it requires least
>> code changes just after if (rte->rtekind == RTE_SUBQUERY).
>> The other is clause_selectivity(). Its code path is not user controlable,
>> so we can apply necessary checks to prevent qualifier that reference
>> variable come from sub-query with security-barrier.
>>
>> In my sense, clause_selectivity() is better place to apply this type of
>> checks. But, on the other hand, it provides get_relation_stats_hook
>> to allow extensions to control references to statistic data.
>> So, I wonder whether the PG core assumes this routine covers
>> all the code path here?
>>
> The attached patch adds checks around invocation of selectivity
> estimator functions, and it changes behavior of the estimator, if the
> supplied operator tries to touch variables come from security-barrier
> relations.
> Then, it fixes the problem you mentioned.
>
>  postgres=# explain select * from safe_foo where a !! 0;
>                           QUERY PLAN
>  -
>   Subquery Scan on safe_foo  (cost=0.00..2.70 rows=3 width=4)
>     Filter: (safe_foo.a !! 0)
>     ->  Seq Scan on foo  (cost=0.00..1.14 rows=6 width=4)
>           Filter: (a > 5)
>  (4 rows)
>
> However, I'm still a bit skeptical on my patch, because it still allows
> to invoke estimator function when operator's argument does not
> touch values originated from security-barrier relation.
> In the case when oprrest or oprjoin are implemented without our
> regular convention (E.g, it anyway reference whole of statistical data),
> it will break this solution.
>
> Of course, it is an assumption that we cannot prevent any attack
> using binary modules, so we need to say "use it your own risk" if
> people tries to use extensional modules. And, we also need to
> keep the built-in code secure.
>
> Some of built-in estimator functions (such as eqsel) provides
> a feature that invokes operator function with arguments originated
> from pg_statistics table. It didn't harmless, however, we got understand
> that this logic can be used to break row-level security.
> So, I begin to consider the routines to be revised are some of built-in
> functions to be used for estimator functions; such as eqsel and ...
> These function eventually reference statistical data at examine_variable.
>
> It might be a better approach to add checks whether invocation of
> the supplied operator possibly leaks contents to be invisible.
> It seems to me the Idea of the attached patch depends on something
> in

Re: [HACKERS] Command Triggers

2011-12-12 Thread Andres Freund
On Monday, December 12, 2011 05:32:45 PM Robert Haas wrote:
> On Sun, Dec 11, 2011 at 1:55 PM, Dimitri Fontaine
> 
>  wrote:
> > Andres Freund  writes:
> >> Hm. I just noticed a relatively big hole in the patch: The handling of
> >> deletion of dependent objects currently is nonexistant because they
> >> don't go through ProcessUtility...
> > 
> > That's the reason why we're talking about “command triggers” rather than
> > “DDL triggers”.  We don't intend to fire the triggers at each DDL
> > operation happening on the server, but for each command.
> > 
> > This restriction still allows us to provide a very useful feature when
> > checked against the main use cases we target here. Those are auditing,
> > and replication (the replay will also CASCADEs), and a generic enough
> > SUDO facility (because the trigger function can well be SECURITY
> > DEFINER).
> 
> I haven't yet thought about your specific proposal here in enough to
> have a fully-formed opinion, but I am a little nervous that this may
> turn out to be one of those cases where the obvious API ends up
> working less well than might have been supposed. 
What are your thoughts about a "not-obvious api"?

> For example,
> consider the "auditing" use case, and suppose that the user wants to
> log a message (to a table, or some other separate logging facility)
> every time someone creates an index.  In order to do that, they're
> going to need to trap not only CREATE INDEX but also CREATE TABLE and
> ALTER CONSTRAINT, and in the latter two cases they're then going to
> have to then write some grotty logic to grovel through the statement
> or its node tree in order to figure out what indexes are being
> created.  Also, if they want to log the name of the new index in cases
> where the user doesn't specify it, they're going to have to duplicate
> the backend logic which generates the index name, which is going to be
> a pain in the ass and a recipe for future bugs (e.g. because we might
> change the algorithm at some point, or the trigger might see a
> different view of the world than the actual command, due to
> intervening DDL).
I thought about using transformed statements before passing them to the 
triggers to avoid exactly that issue. That would make stuff like CREATE TABLE 
blub (LIKE), constraints and such transparent. It would mean quite some 
additional effort though (because for several statements the transformation is 
not really separated).

> This is something that has come up a lot in the context of sepgsql:
> it's not really enough to know what the toplevel command is in some
> cases, because whether or not the operation is allowed depends on
> exactly what it's doing, which depends on things like which schema
> gets used to create the table, and which tablespaces get used to
> create the table and its indexes, and perhaps (given Peter's pending
> work) what types are involved.  You can deduce all of these things
> from the command text, but it requires duplicating nontrivial amounts
> of backend code, which is undesirable from a maintainability point of
> view.  I think the same issues are likely to arise in the context of
> auditing, as in the example above, and even for replication: if, for
> example, the index name that is chosen on the master happens to exist
> on the standby with a different definition, replaying the actual
> command text might succeed or fail depending on how the command is
> phrased.  Maybe it's OK for the slave to just choose a different name
> for that index, but now a subsequent DROP INDEX command that gets
> replayed across all servers might end up dropping logically unrelated
> indexes on different machines.
Well, you shouldn't grovel through the text - you do get passed the parsetree 
which should make accessing that far easier. I can imagine somebody writing 
some shiny layer which lets you grovel trough that with some xpath alike api.

> Now, on the other hand, the conceptual simplicity of this approach is
> very appealing, and I can certainly see people who would never
> consider writing a ProcessUtility_hook using this type of facility.
> However, I'm worried that it will be difficult to use as a foundation
> for robust, general-purpose tools.  Is that enough reason to reject
> the whole approach?  I'm not sure.
Thats where I am a bit unsure as well. I think the basic approach is sound but 
it might need some work in several areas (transformed statements, 
dependencies, ...). On the other hand: perfect is the enemy of good...

Having looked at it I don't think dependency handling is that much effort. 
Havent looked at separation of the transformation phase.

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] patch : Allow toast tables to be moved to a different tablespace

2011-12-12 Thread Robert Haas
On Sat, Dec 10, 2011 at 4:16 PM, Jaime Casanova  wrote:
>>> now, if we are now supporting this variants
>>> ALTER TABLE SET TABLE TABLESPACE
>>> ALTER TABLE SET TOAST TABLESPACE
>>>
>>> why not also support ALTER TABLE SET INDEX TABLESPACE which should
>>> have the same behaviour as ALTER INDEX SET TABLESPACE... just an idea,
>>> and of course not necessary for this patch
>
> any opinion about this? maybe i can make a patch for that if there is
> consensus that it could be good for symettry

I'm not really convinced we need it.  I think it would end up just
being a shorthand for ALTER INDEX .. SET TABLESPACE for each index.
Most tables don't have more than a handful of indexes, so it doesn't
seem like we'd be gaining much (compare GRANT ... ON ALL TABLES IN
SCHEMA, which could easily be a shorthand for hundreds or perhaps even
thousands of individual GRANT statements).

Also, it seems that we haven't really discussed much why moving the
TOAST table to a different tablespace from the main table might be
useful.  I'm not saying we shouldn't have it if it's good for
something, but what's the reason for wanting it?

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

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


Re: [HACKERS] Command Triggers

2011-12-12 Thread Alvaro Herrera

Excerpts from Robert Haas's message of lun dic 12 13:32:45 -0300 2011:
> On Sun, Dec 11, 2011 at 1:55 PM, Dimitri Fontaine
>  wrote:
> > Andres Freund  writes:
> >> Hm. I just noticed a relatively big hole in the patch: The handling of
> >> deletion of dependent objects currently is nonexistant because they don't 
> >> go
> >> through ProcessUtility...
> >
> > That's the reason why we're talking about “command triggers” rather than
> > “DDL triggers”.  We don't intend to fire the triggers at each DDL
> > operation happening on the server, but for each command.
> >
> > This restriction still allows us to provide a very useful feature when
> > checked against the main use cases we target here. Those are auditing,
> > and replication (the replay will also CASCADEs), and a generic enough
> > SUDO facility (because the trigger function can well be SECURITY
> > DEFINER).
> 
> I haven't yet thought about your specific proposal here in enough to
> have a fully-formed opinion, but I am a little nervous that this may
> turn out to be one of those cases where the obvious API ends up
> working less well than might have been supposed.  For example,
> consider the "auditing" use case, and suppose that the user wants to
> log a message (to a table, or some other separate logging facility)
> every time someone creates an index.  In order to do that, they're
> going to need to trap not only CREATE INDEX but also CREATE TABLE and
> ALTER CONSTRAINT,

Yeah.  I remember mentioning the ability of CREATE SCHEMA to embed all
sort of object creation commands in a single top-level command, and
being handwaved away by Jan.  "Nobody knows about that", I was told.
However, if this is to work in a robust way, these things should not be
ignored.  In particular, firing triggers on each top-level command _is_
going to get some objects ignored; the people writing the triggers _are_
going to forget to handle CREATE SCHEMA properly most of the time.

-- 
Á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] Command Triggers

2011-12-12 Thread Robert Haas
On Sun, Dec 11, 2011 at 1:55 PM, Dimitri Fontaine
 wrote:
> Andres Freund  writes:
>> Hm. I just noticed a relatively big hole in the patch: The handling of
>> deletion of dependent objects currently is nonexistant because they don't go
>> through ProcessUtility...
>
> That's the reason why we're talking about “command triggers” rather than
> “DDL triggers”.  We don't intend to fire the triggers at each DDL
> operation happening on the server, but for each command.
>
> This restriction still allows us to provide a very useful feature when
> checked against the main use cases we target here. Those are auditing,
> and replication (the replay will also CASCADEs), and a generic enough
> SUDO facility (because the trigger function can well be SECURITY
> DEFINER).

I haven't yet thought about your specific proposal here in enough to
have a fully-formed opinion, but I am a little nervous that this may
turn out to be one of those cases where the obvious API ends up
working less well than might have been supposed.  For example,
consider the "auditing" use case, and suppose that the user wants to
log a message (to a table, or some other separate logging facility)
every time someone creates an index.  In order to do that, they're
going to need to trap not only CREATE INDEX but also CREATE TABLE and
ALTER CONSTRAINT, and in the latter two cases they're then going to
have to then write some grotty logic to grovel through the statement
or its node tree in order to figure out what indexes are being
created.  Also, if they want to log the name of the new index in cases
where the user doesn't specify it, they're going to have to duplicate
the backend logic which generates the index name, which is going to be
a pain in the ass and a recipe for future bugs (e.g. because we might
change the algorithm at some point, or the trigger might see a
different view of the world than the actual command, due to
intervening DDL).

This is something that has come up a lot in the context of sepgsql:
it's not really enough to know what the toplevel command is in some
cases, because whether or not the operation is allowed depends on
exactly what it's doing, which depends on things like which schema
gets used to create the table, and which tablespaces get used to
create the table and its indexes, and perhaps (given Peter's pending
work) what types are involved.  You can deduce all of these things
from the command text, but it requires duplicating nontrivial amounts
of backend code, which is undesirable from a maintainability point of
view.  I think the same issues are likely to arise in the context of
auditing, as in the example above, and even for replication: if, for
example, the index name that is chosen on the master happens to exist
on the standby with a different definition, replaying the actual
command text might succeed or fail depending on how the command is
phrased.  Maybe it's OK for the slave to just choose a different name
for that index, but now a subsequent DROP INDEX command that gets
replayed across all servers might end up dropping logically unrelated
indexes on different machines.

Now, on the other hand, the conceptual simplicity of this approach is
very appealing, and I can certainly see people who would never
consider writing a ProcessUtility_hook using this type of facility.
However, I'm worried that it will be difficult to use as a foundation
for robust, general-purpose tools.  Is that enough reason to reject
the whole approach?  I'm not sure.

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

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


Re: [HACKERS] Is anybody actually using XLR_BKP_REMOVABLE?

2011-12-12 Thread Jesper Krogh
> 
> 
> So: is there actually any such compression program out there?
> Would anybody really cry if this flag went away?

Perhaps http://pglesslog.projects.postgresql.org/

Jesper



Re: [HACKERS] Is anybody actually using XLR_BKP_REMOVABLE?

2011-12-12 Thread Simon Riggs
On Mon, Dec 12, 2011 at 3:42 PM, Tom Lane  wrote:
> Simon Riggs  writes:
>> On Mon, Dec 12, 2011 at 3:17 PM, Tom Lane  wrote:
>>> Furthermore, what the XLR_BKP_REMOVABLE bit actually reports is just
>>> whether a backup operation is in progress, and I think we have now (or
>>> easily could) add reporting records to the WAL stream that tell when a
>>> backup starts or stops.  So external compression would still be possible
>>> if it kept a bit more state around.
>>>
>>> So: is there actually any such compression program out there?
>>> Would anybody really cry if this flag went away?
>
>> Yes, WAL records could be invented to mark the boundaries, so yes,
>> IMHO it is OK to make that flag go away.
>
> It occurs to me also that we could just move the flag from
> per-WAL-record info bytes to per-page or even per-segment WAL headers.
> Because we now force a segment switch when starting a backup, the
> flag would be seen turned-on soon enough to prevent problems.
> Finding out that it's off again after the end of a backup might be
> a little delayed, but the only cost is failure to compress a few
> compressible records.
>
> I'm not volunteering to do the above, unless someone steps forward
> to say that there's active use of this flag, but either one of these
> solutions seems more tenable than using up an info-byte bit.

I'll volunteer. Assume you can reuse the flag and I will patch afterwards.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

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


Re: [HACKERS] review: CHECK FUNCTION statement

2011-12-12 Thread Pavel Stehule
hello

2011/12/12 Albe Laurenz :
> Pavel Stehule wrote:
>> there is merged patch
>
> Works fine, except that there are still missing const qualifiers
> in copyfuncs.c and equalfuncs.c that lead to compiler warnings.
>
> One thing I forgot to mention:
> I thought there was a consensus to add a WITH() or OPTIONS() clause
> to pass options to the checker function:
> http://archives.postgresql.org/message-id/12568.1322669...@sss.pgh.pa.us
>
> I think this should be there so that the API does not have to be
> changed in the future.
>

there is just one question - how propagate options to check functions

I am thinking about third parameter - probably text array

??
Regards

Pavel

> Yours,
> Laurenz Albe

-- 
Sent 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 : Allow toast tables to be moved to a different tablespace

2011-12-12 Thread Julien Tachoires
Hi,

2011/12/10 Jaime Casanova :
> On Mon, Nov 28, 2011 at 1:32 PM, Julien Tachoires  wrote:
>
>>> 2) after CLUSTER the index of the toast table gets moved to the same
>>> tablespace as the main table
>>
>
> there is still a variant of this one, i created 3 tablespaces (datos_tblspc):
>
> """
> create table t1 (
>     i serial primary key,
>     t text
> ) tablespace datos_tblspc;
>
> ALTER TABLE t1 SET TOAST TABLESPACE pg_default;
> CLUSTER t1 USING t1_pkey;
> """

I am not able to reproduce this case, could you show me exactly how to
reproduce it ?

>
>>>
>>> now, if we are now supporting this variants
>>> ALTER TABLE SET TABLE TABLESPACE
>>> ALTER TABLE SET TOAST TABLESPACE
>>>
>>> why not also support ALTER TABLE SET INDEX TABLESPACE which should
>>> have the same behaviour as ALTER INDEX SET TABLESPACE... just an idea,
>>> and of course not necessary for this patch
>>>
>
> any opinion about this? maybe i can make a patch for that if there is
> consensus that it could be good for symettry

Thanks,

-- 
Sent 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: CHECK FUNCTION statement

2011-12-12 Thread Albe Laurenz
Pavel Stehule wrote:
> there is merged patch

Works fine, except that there are still missing const qualifiers
in copyfuncs.c and equalfuncs.c that lead to compiler warnings.

One thing I forgot to mention:
I thought there was a consensus to add a WITH() or OPTIONS() clause
to pass options to the checker function:
http://archives.postgresql.org/message-id/12568.1322669...@sss.pgh.pa.us

I think this should be there so that the API does not have to be
changed in the future.

Yours,
Laurenz Albe

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


Re: [HACKERS] Is anybody actually using XLR_BKP_REMOVABLE?

2011-12-12 Thread Tom Lane
Simon Riggs  writes:
> On Mon, Dec 12, 2011 at 3:17 PM, Tom Lane  wrote:
>> Furthermore, what the XLR_BKP_REMOVABLE bit actually reports is just
>> whether a backup operation is in progress, and I think we have now (or
>> easily could) add reporting records to the WAL stream that tell when a
>> backup starts or stops.  So external compression would still be possible
>> if it kept a bit more state around.
>> 
>> So: is there actually any such compression program out there?
>> Would anybody really cry if this flag went away?

> Yes, WAL records could be invented to mark the boundaries, so yes,
> IMHO it is OK to make that flag go away.

It occurs to me also that we could just move the flag from
per-WAL-record info bytes to per-page or even per-segment WAL headers.
Because we now force a segment switch when starting a backup, the
flag would be seen turned-on soon enough to prevent problems.
Finding out that it's off again after the end of a backup might be
a little delayed, but the only cost is failure to compress a few
compressible records.

I'm not volunteering to do the above, unless someone steps forward
to say that there's active use of this flag, but either one of these
solutions seems more tenable than using up an info-byte bit.

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] Is anybody actually using XLR_BKP_REMOVABLE?

2011-12-12 Thread Simon Riggs
On Mon, Dec 12, 2011 at 3:17 PM, Tom Lane  wrote:

> Furthermore, what the XLR_BKP_REMOVABLE bit actually reports is just
> whether a backup operation is in progress, and I think we have now (or
> easily could) add reporting records to the WAL stream that tell when a
> backup starts or stops.  So external compression would still be possible
> if it kept a bit more state around.
>
> So: is there actually any such compression program out there?
> Would anybody really cry if this flag went away?

Yes, WAL records could be invented to mark the boundaries, so yes,
IMHO it is OK to make that flag go away.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

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


Re: [HACKERS] [PATCH] PostgreSQL fails to build with 32bit MinGW-w64

2011-12-12 Thread Lars Kanis
Am Montag, 12. Dezember 2011, 10:19:46 schrieb Andrew Dunstan:
> 
> On 12/12/2011 09:54 AM, Lars Kanis wrote:
> > Am Freitag, 9. Dezember 2011, 15:31:17 schrieb Andrew Dunstan:
> >> Yeah, fair enough. I'll work on that.
> > Many thanks for reviewing, tweaking and commiting the patch!
> > One thing I wonder about, is this snippet. Is the define really needed now?
> >
> >   * The Mingw64 headers choke if this is already defined - they
> >   * define it themselves.
> >   */
> > #if !defined(__MINGW64_VERSION_MAJOR) || defined(WIN32_ONLY_COMPILER)
> > #define _WINSOCKAPI_
> > #endif
> > #include
> >
> >
> >
> 
> As previously discussed, unless you can prove it's not needed I don't 
> want to remove it, on the ""if it ain't broke don't fix it" principle. I 
> believe it is needed for at least some older compilers (specifically 
> some of those used by buildfarm animals narwhal, frogmouth, mastodon, 
> hamerkop and currawong), and it doesn't appear to be hurting anything. 
> As you can see above it's been disabled for all Mingw-w64 compilers.

Ok. Thanks for clarification.

Kind regards,
Lars


-- 
Sent 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] PostgreSQL fails to build with 32bit MinGW-w64

2011-12-12 Thread Andrew Dunstan



On 12/12/2011 09:54 AM, Lars Kanis wrote:

Am Freitag, 9. Dezember 2011, 15:31:17 schrieb Andrew Dunstan:

Yeah, fair enough. I'll work on that.

Many thanks for reviewing, tweaking and commiting the patch!
One thing I wonder about, is this snippet. Is the define really needed now?

  * The Mingw64 headers choke if this is already defined - they
  * define it themselves.
  */
#if !defined(__MINGW64_VERSION_MAJOR) || defined(WIN32_ONLY_COMPILER)
#define _WINSOCKAPI_
#endif
#include





As previously discussed, unless you can prove it's not needed I don't 
want to remove it, on the ""if it ain't broke don't fix it" principle. I 
believe it is needed for at least some older compilers (specifically 
some of those used by buildfarm animals narwhal, frogmouth, mastodon, 
hamerkop and currawong), and it doesn't appear to be hurting anything. 
As you can see above it's been disabled for all Mingw-w64 compilers.


If it's really bugging people we can try disabling it and see if any of 
those break, but honestly we have far uglier things that we carry for 
legacy reasons :-)


cheers

andrew


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


[HACKERS] Is anybody actually using XLR_BKP_REMOVABLE?

2011-12-12 Thread Tom Lane
Back in 2007 (commit a8d539f12498de52453c8113892cbf48cc62478d), we
reduced the maximum number of backup blocks per WAL record from 4 to 3,
in order to permit addition of an XLR_BKP_REMOVABLE flag bit that
purports to show whether it's safe to suppress full-page-image backup
blocks in an external WAL-filtering program.  I'm having a problem with
this now because I don't see any way to get SP-GiST's leaf page
splitting operation to touch less than four pages.  So I'd like to
propose reverting that decision and again allowing a WAL record to touch
as many as four pages.

As the above commit message points out, compression of this sort could
only be done externally if that external program knew the complete
details of every single type of WAL record, so that it could figure out
whether any data needed to be extracted from the full-page images and
reinserted in the WAL record.  I'm not convinced that anybody has
written such a thing or will be able to maintain it into the future,
as we feel free to whack around the contents of WAL on a regular basis.
The commit message claims that such a program would be posted and
maintained on pgfoundry, but I couldn't find any trace of it (not that
pgfoundry's search tools are very good, but neither "compress", "xlog"
or "wal" produces any hits on such a thing).  In any case I think the
modern theory about this is you should get a filesystem that prevents
torn-page writes, and then you can just turn off full_page_writes.

Furthermore, what the XLR_BKP_REMOVABLE bit actually reports is just
whether a backup operation is in progress, and I think we have now (or
easily could) add reporting records to the WAL stream that tell when a
backup starts or stops.  So external compression would still be possible
if it kept a bit more state around.

So: is there actually any such compression program out there?
Would anybody really cry if this flag went away?

regards, tom lane

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


Re: [HACKERS] [PATCH] PostgreSQL fails to build with 32bit MinGW-w64

2011-12-12 Thread Andrew Dunstan



On 12/12/2011 06:43 AM, Mark Cave-Ayland wrote:

configuration, it seems to me that it would be fine to commit a patch
that made everything work, but for the compiler bug. We could refrain
from stating that we officially support that configuration until the
compiler bug is fixed, or even document the existence of the bug. We
can't be responsible for other people's broken code, but I don't
necessarily see that as a reason not to commit a prerequisite patch.
Otherwise, as you say, there's a chicken-and-egg problem, and who does
that help?




Yeah, fair enough. I'll work on that.

If we're talking about adding support for a previously-unsupported

Definitely do this (and then file a bug report with the project). I've 
worked with both Kai and NightStrike from the MingW-W64 project to fix 
previous bugs, and as long as they can build the offending source 
themselves then they are very helpful and quick to respond.






Done and done (see 
).


cheers

andrew

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


Re: [HACKERS] [PATCH] PostgreSQL fails to build with 32bit MinGW-w64

2011-12-12 Thread Lars Kanis
Am Freitag, 9. Dezember 2011, 15:31:17 schrieb Andrew Dunstan:
> Yeah, fair enough. I'll work on that.

Many thanks for reviewing, tweaking and commiting the patch!
One thing I wonder about, is this snippet. Is the define really needed now?

 * The Mingw64 headers choke if this is already defined - they
 * define it themselves.
 */
#if !defined(__MINGW64_VERSION_MAJOR) || defined(WIN32_ONLY_COMPILER)
#define _WINSOCKAPI_
#endif
#include 


Kind regards,
Lars Kanis


-- 
Sent 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] pg_last_xact_insert_timestamp

2011-12-12 Thread Robert Haas
On Mon, Dec 12, 2011 at 9:51 AM, Simon Riggs  wrote:
> On Mon, Dec 12, 2011 at 2:47 PM, Robert Haas  wrote:
>> On Mon, Dec 12, 2011 at 9:24 AM, Simon Riggs  wrote:
>>> On Mon, Dec 12, 2011 at 1:45 PM, Robert Haas  wrote:
 It also strikes me that anything
 that is based on augmenting the walsender/walreceiver protocol leaves
 anyone who is using WAL shipping out in the cold.  I'm not clear from
 the comments you or Simon have made how important you think that use
 case still is.
>>>
>>> archive_timeout > 0 works just fine at generating files even when
>>> quiet, or if it does not, it is a bug.
>>>
>>> So I don't understand your comments, please explain.
>>
>> If the standby has restore_command set but not primary_conninfo, then
>> it will never make a direct connection to the master.  So anything
>> that's based on extending that protocol won't get used in that case.
>
> Got that, but now explain the reason for saying such people are "out
> in the cold".

By that I only meant that if we add a lag-monitoring feature that
relies on the streaming replication protocol, then people who are not
using the streaming replication replication protocol will be unable to
monitor lag (or at least, the will be unable to do it any better than
they can do it today).

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

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


Re: [HACKERS] [REVIEW] pg_last_xact_insert_timestamp

2011-12-12 Thread Simon Riggs
On Mon, Dec 12, 2011 at 2:47 PM, Robert Haas  wrote:
> On Mon, Dec 12, 2011 at 9:24 AM, Simon Riggs  wrote:
>> On Mon, Dec 12, 2011 at 1:45 PM, Robert Haas  wrote:
>>> It also strikes me that anything
>>> that is based on augmenting the walsender/walreceiver protocol leaves
>>> anyone who is using WAL shipping out in the cold.  I'm not clear from
>>> the comments you or Simon have made how important you think that use
>>> case still is.
>>
>> archive_timeout > 0 works just fine at generating files even when
>> quiet, or if it does not, it is a bug.
>>
>> So I don't understand your comments, please explain.
>
> If the standby has restore_command set but not primary_conninfo, then
> it will never make a direct connection to the master.  So anything
> that's based on extending that protocol won't get used in that case.

Got that, but now explain the reason for saying such people are "out
in the cold".

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

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


Re: [HACKERS] [REVIEW] pg_last_xact_insert_timestamp

2011-12-12 Thread Robert Haas
On Mon, Dec 12, 2011 at 9:24 AM, Simon Riggs  wrote:
> On Mon, Dec 12, 2011 at 1:45 PM, Robert Haas  wrote:
>> It also strikes me that anything
>> that is based on augmenting the walsender/walreceiver protocol leaves
>> anyone who is using WAL shipping out in the cold.  I'm not clear from
>> the comments you or Simon have made how important you think that use
>> case still is.
>
> archive_timeout > 0 works just fine at generating files even when
> quiet, or if it does not, it is a bug.
>
> So I don't understand your comments, please explain.

If the standby has restore_command set but not primary_conninfo, then
it will never make a direct connection to the master.  So anything
that's based on extending that protocol won't get used in that case.

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

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


Re: [HACKERS] [REVIEW] pg_last_xact_insert_timestamp

2011-12-12 Thread Simon Riggs
On Mon, Dec 12, 2011 at 1:45 PM, Robert Haas  wrote:

> It also strikes me that anything
> that is based on augmenting the walsender/walreceiver protocol leaves
> anyone who is using WAL shipping out in the cold.  I'm not clear from
> the comments you or Simon have made how important you think that use
> case still is.

archive_timeout > 0 works just fine at generating files even when
quiet, or if it does not, it is a bug.

So I don't understand your comments, please explain.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

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


Re: [HACKERS] pgsql_fdw, FDW for PostgreSQL server

2011-12-12 Thread Robert Haas
On Wed, Dec 7, 2011 at 2:34 AM, Shigeru Hanada  wrote:
> Sorry for delayed response.
>
> 2011/11/29 Albe Laurenz :
>> I think that this is not always safe even from PostgreSQL to PostgreSQL.
>> If two databases have different collation, "<" on strings will behave
>> differently.
>
> Indeed.  I think that only the owner of foreign table can keep collation
> consistent between foreign and local, like data type of column.

+1.

> We need to
> support per-column-collation on foreign tables too, or should deny pushing
> down condition which is collation-sensitive...

It seems that we already do:

rhaas=# create foreign table ft1 (a text collate "de_DE") server s1;
CREATE FOREIGN TABLE

It does seem like this might not be enough information for the FDW to
make good decisions about pushdown.  Even supposing the server on the
other hand is also PostgreSQL, the collation names might not match
(if, say, one is running Windows, and the other, Linux).  And even if
they do, there is no guarantee that two collations with the same name
have the same behavior on two different machines; they probably
should, but who knows?  And if we're using an FDW to talk to some
other database server, the problem is much worse; it's not clear that
we'll even begin to be able to guess whether the remote side has
compatible semantics.  I feel like we might need a system here that
allows for more explicit user control about what to push down vs. not,
rather than assuming we'll be able to figure it out behind the scenes.

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

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


[HACKERS] psql output locations

2011-12-12 Thread Magnus Hagander
We're either pretty inconsistent with our output in psql, or I'm not
completely understanding it.. I was trying to implement a switch that
would let me put all the output in the query output channel controlled
by \o, and not just the output of the query itself. Because that would
make it possible to control it from inside the script. Now, this made
me notice:

* There are 102 calls to psql_error(), 42 direct uses of
fprintf(stderr), and 7 uses of fputs(stderr). And there is also
write_stderr() used in the cancel handler. Is there actually a point
behind all those direct uses, or should they really be psql_error()
calls?

* There are a number of things that are always written to stdout, that
there is no way to redirect. In some cases it's interactive prompts -
makes sense - but also for example the output of \timing goes to
stdout always. Is there some specific logic behind what/when this
should be done?

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

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


Re: [HACKERS] [REVIEW] pg_last_xact_insert_timestamp

2011-12-12 Thread Robert Haas
On Sat, Dec 10, 2011 at 7:29 AM, Greg Smith  wrote:
> -It adds overhead at every commit, even for people who aren't using it.
>  Probably not enough to matter, but it's yet another thing going through the
> often maligned as too heavy pgstat system, often.

The bit about the pgstat system being too heavy is a red herring; this
could equally well be stored in the PGPROC.  So, the overhead is
basically one additional store to shared memory per commit, and we can
arrange for that store to involve a cache line that has to be touched
anyway (by ProcArrayEndTransaction).  I suppose you can make the
argument that that still isn't free, but there aren't very many places
in the code where we worry about effects this small.  Operations like
AcceptInvalidationMessages() or LockRelationOid() happen more often
than transaction commit, and yet we were happy for years with a system
where AcceptInvalidationMessages() did three spinlock
acquire-and-release cycles that were unnecessary in 99% of cases.
That cost was vastly greater than what we're talking about here, and
it affected an operation that is more frequent than this one.

> [ usability complaints ]

Fair enough.

> I'm normally a fan of building the simplest thing that will do something
> useful, and this patch succeeds there.  But the best data to collect needs
> to have a timestamp that keeps moving forward in a way that correlates
> reasonably well to the system WAL load.  Using the transaction ID doesn't do
> that.  Simon did some hand waving around sending a timestamp every
> checkpoint.  That would allow the standby to compute its own lag, limit
> overhead to something much lighter than per-transaction, and better track
> write volume.  There could still be a bigger than normal discontinuity after
> server restart, if the server was down a while, but at least there wouldn't
> ever be a point where the value was returned by the master but was NULL.
>
> But as Simon mentioned in passing, it will bloat the WAL streams for
> everyone.

But, as with this proposal, the overhead seems largely irrelevant; the
question is whether it actually solves the problem.  Unless I am
misunderstanding, we are talking about 4 bytes of WAL per checkpoint
cycle, which strikes me as even less worth worrying about than one
store to shared memory per transaction commit.  So, personally, I see
no reason to fret about the overhead.  But I'm skeptical that anything
that we only update once per checkpoint cycle will help much in
calculating an accurate lag value.  It also strikes me that anything
that is based on augmenting the walsender/walreceiver protocol leaves
anyone who is using WAL shipping out in the cold.  I'm not clear from
the comments you or Simon have made how important you think that use
case still is.

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

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


[HACKERS] Why create tuplestore for each fetch?

2011-12-12 Thread 高增琦
Hi,

I am reading code about cursor and fetch ...
Here is a test:

create table t (a int);
insert into t values (1),(3),(5),(7),(9);
insert into t select a+1 from t;
begin;
declare c cursor for select * from t order by a;
fetch 3 in c;
fetch 3 in c;
fetch 3 in c;

In func "PortalRun", FillPortalStore(portal, isTopLevel) will create a
tuplestore for each query...
Why create tuplestore for each fetch?

-- 
GaoZengqi
pgf...@gmail.com
zengqi...@gmail.com


Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters

2011-12-12 Thread Yeb Havinga

On 2011-12-11 16:26, Yeb Havinga wrote:

On 2011-12-06 17:58, Kevin Grittner wrote:

Kevin Grittner  wrote:

Yeb Havinga  wrote:



I personally tend to believe it doesn't even need to be an error.
There is no technical reason not to allow it. All the user needs
to do is make sure that the combination of named parameters and
the positional ones together are complete and not overlapping.



If there are no objections, I suggest that Yeb implement the mixed
notation for cursor parameters.


Hearing no objections -- Yeb, are you OK with doing this, and do you
feel this is doable for this CF?


Attach is v6 of the patch, allowing mixed mode and with new test cases 
in the regression tests. One difference with calling functions 
remains: it is allowed to place positional arguments after named 
parameters. Including that would add code, but nothing would be gained.


Forgot to copy regression output to expected - attached v7 fixes that.

-- Yeb

diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
new file mode 100644
index f33cef5..ee2e3a3
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
*** OPEN curs1 FOR EXECUTE 'SELECT * FROM '
*** 2823,2833 
 
   
  
! 
   Opening a Bound Cursor
  
  
! OPEN bound_cursorvar  ( argument_values ) ;
  
  
   
--- 2823,2833 
 
   
  
! 
   Opening a Bound Cursor
  
  
!  OPEN bound_cursorvar  (  argname :=  argument_value , ... ) ;
  
  
   
*** OPEN bound_cursorvar
  
   
+   Cursors may be opened using either positional
+   or named notation.  Similar to calling
+   functions, described in , it
+   is also allowed to mix positional and named notation.  In positional
+   notation, all arguments are specified in order.  In named notation,
+   each argument's name is specified using := to
+   separate it from the argument expression.
+  
+ 
+  
Examples (these use the cursor declaration examples above):
  
  OPEN curs2;
  OPEN curs3(42);
+ OPEN curs3(key := 42);
  
   
  
*** COMMIT;
*** 3169,3186 
  
  
   <

Re: [HACKERS] [REVIEW] pg_last_xact_insert_timestamp

2011-12-12 Thread Simon Riggs
On Sat, Dec 10, 2011 at 12:29 PM, Greg Smith  wrote:

> "We can send regular special messages from WALSender to WALReceiver that do
> not form part of the WAL stream, so we don't bulk
> up WAL archives. (i.e. don't use "w" messages)."
>
> Here's my understanding of how this would work.

Let me explain a little more and provide a very partial patch.

We define a new replication protocol message 'k' which sends a
keepalive from primary to standby when there is no WAL to send. The
message does not form part of the WAL stream so does not bloat WAL
files, nor cause them to fill when unattended.

Keepalives contain current end of WAL and a current timestamp.

Keepalive processing is all done on the standby and there is no
overhead on a primary which does not use replication. There is a
slight overhead on primary for keepalives but this happens only when
there are no writes. On the standby we already update shared state
when we receive some data, so not much else to do there.

When the standby has applied up to the end of WAL the replication
delay is receipt time - send time of keepalive.

When standby receives a data packet it records WAL ptr and time. As
standby applies each chunk it removes the record for each data packet
and sets the last applied timestamp.

If standby falls behind the number of data packet records will build
up, so we begin to keep record every 2 packets, then every 4 packets
etc. So the further the standby falls behind the less accurately we
record the replication delay - though the accuracy remains
proportional to the delay.

To complete the patch I need to
* send the keepalive messages when no WAL outstanding
* receive the messages
* store timestamp info for data and keepalives
* progressively filter the messages if we get too many

I will be working on this patch some more this week.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index d6332e5..71c40cc 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -1467,6 +1467,54 @@ The commands accepted in walsender mode are:
   
   
   
+  Primary keepalive message (B)
+  
+  
+  
+  
+  
+  
+  Byte1('k')
+  
+  
+  
+  Identifies the message as a sender keepalive.
+  
+  
+  
+  
+  
+  Byte8
+  
+  
+  
+  The current end of WAL on the server, given in
+  XLogRecPtr format.
+  
+  
+  
+  
+  
+  Byte8
+  
+  
+  
+  The server's system clock at the time of transmission,
+  given in TimestampTz format.
+  
+  
+  
+  
+  
+  
+  
+  
+ 
+
+ 
+  
+  
+  
   Standby status update (F)
   
   
diff --git a/src/include/replication/walprotocol.h b/src/include/replication/walprotocol.h
index 656c8fc..1c73d35 100644
--- a/src/include/replication/walprotocol.h
+++ b/src/include/replication/walprotocol.h
@@ -40,6 +40,21 @@ typedef struct
 } WalDataMessageHeader;
 
 /*
+ * Keepalive message from primary (message type 'k'). (lowercase k)
+ * This is wrapped within a CopyData message at the FE/BE protocol level.
+ *
+ * Note that the data length is not specified here.
+ */
+typedef struct
+{
+	/* Current end of WAL on the sender */
+	XLogRecPtr	walEnd;
+
+	/* Sender's system clock at the time of transmission */
+	TimestampTz sendTime;
+} PrimaryKeepaliveMessage;
+
+/*
  * Reply message from standby (message type 'r').  This is wrapped within
  * a CopyData message at the FE/BE protocol level.
  *

-- 
Sent 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] PostgreSQL fails to build with 32bit MinGW-w64

2011-12-12 Thread Mark Cave-Ayland

On -10/01/37 20:59, Andrew Dunstan wrote:


This is apparently an optimization bug in the compiler. If I turn
optimization off (CFLAGS=-O0) it goes away. Ick.

So at the moment I'm a bit blocked. I can't really file a bug because
the
compiler can't currently be used to build postgres, I don't have time to
construct a self-contained test case, and I don't want to commit
changes to
enable the compiler until the issue is solved.

If we're talking about adding support for a previously-unsupported
configuration, it seems to me that it would be fine to commit a patch
that made everything work, but for the compiler bug. We could refrain
from stating that we officially support that configuration until the
compiler bug is fixed, or even document the existence of the bug. We
can't be responsible for other people's broken code, but I don't
necessarily see that as a reason not to commit a prerequisite patch.
Otherwise, as you say, there's a chicken-and-egg problem, and who does
that help?




Yeah, fair enough. I'll work on that.


Definitely do this (and then file a bug report with the project). I've 
worked with both Kai and NightStrike from the MingW-W64 project to fix 
previous bugs, and as long as they can build the offending source 
themselves then they are very helpful and quick to respond.



ATB,

Mark.

--
Mark Cave-Ayland - Senior Technical Architect
PostgreSQL - PostGIS
Sirius Corporation plc - control through freedom
http://www.siriusit.co.uk
t: +44 870 608 0063

Sirius Labs: http://www.siriusit.co.uk/labs

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


Re: [HACKERS] WIP: Collecting statistics on CSV file data

2011-12-12 Thread Shigeru Hanada
(2011/12/09 21:16), Etsuro Fujita wrote:
> I updated the patch.  Please find attached a patch.

I've examined v5 patch, and got reasonable EXPLAIN results which reflect
collected statistics!  As increasing STATISTICS option, estimated rows
become better.  Please see attached stats_*.txt for what I
tested.

  stats_none.txt  : before ANALYZE
  stats_100.txt   : SET STATISTICS = 100 for all columns, and ANALYZE
  stats_1.txt : SET STATISTICS = 1 for all columns, and ANALYZE

I think that this patch become ready for committer after some
minor corrections:

Couldn't set n_distinct
===
I couldn't set n_distinct to columns of foreign tables.  With some
research, I noticed that ATSimplePermissions should accept
ATT_FOREIGN_TABLE too for that case.  In addition, regression tests for
ALTER FOREIGN TABLE should be added to detect this kind of problem.

Showing stats target

We can see stats target of ordinary tables with \d+, but it is not
available for foreign tables.  I think "Stats target" column should be
added even though output of \d+ for foreign tables become wider.  One
possible idea is to remove useless "Storage" column instead, but views
have that column even though their source could come from foreign tables.

Please see attached patch for these two items.

Comments of FdwRoutine
==
Mention about optional handler is obsolete.  We should clearly say
AnalyzeForeignTable is optional (can be set to NULL) and rest are
required.  IMO separating them with comment would help FDW authors to
understand requirements, e.g.:

typedef struct FdwRoutine
{
NodeTag type;

/*
 * These Handlers are required to execute simple scan on a foreign
 * table.  If any of them was set to NULL, scan on a foreign table
 * managed by such FDW would fail.
 */
PlanForeignScan_function PlanForeignScan;
ExplainForeignScan_function ExplainForeignScan;
BeginForeignScan_function BeginForeignScan;
IterateForeignScan_function IterateForeignScan;
ReScanForeignScan_function ReScanForeignScan;
EndForeignScan_function EndForeignScan;

/*
 * Handlers below are optional.  You can set any of them to
 * NULL to tell PostgreSQL that the FDW doesn't have the
 * capability.
 */
AnalyzeForeignTable_function AnalyzeForeignTable;
} FdwRoutine;

Code formatting
===
Some code lines go past 80 columns.

Message style
=
The terms 'cannot support option "n_distinct"...' used in
ATPrepSetOptions seems little unusual in PostgreSQL.  Should we say
'cannot set "n_distinct_inherited" for foreign tables' for that case?

Typo

Typo "spcify" is in document of analyze.

Regards,
-- 
Shigeru Hanada

postgres=# explain select * from csv_accounts where aid = 1;
   QUERY PLAN
-
 Foreign Scan on csv_accounts  (cost=0.00..44262.88 rows=1874 width=100)
   Filter: (aid = 1)
   Foreign File: /home/hanada/DB-fujita/CSV/pgbench_accounts.csv
   Foreign File Size: 47963070
(4 rows)

postgres=# explain select * from csv_accounts where aid < 1000;
QUERY PLAN
---
 Foreign Scan on csv_accounts  (cost=0.00..44262.88 rows=124904 width=100)
   Filter: (aid < 1000)
   Foreign File: /home/hanada/DB-fujita/CSV/pgbench_accounts.csv
   Foreign File Size: 47963070
(4 rows)

postgres=# explain select * from csv_accounts where aid > 1000;
QUERY PLAN
---
 Foreign Scan on csv_accounts  (cost=0.00..44262.88 rows=124904 width=100)
   Filter: (aid > 1000)
   Foreign File: /home/hanada/DB-fujita/CSV/pgbench_accounts.csv
   Foreign File Size: 47963070
(4 rows)

postgres=# explain select * from csv_accounts where bid = 1;
   QUERY PLAN
-
 Foreign Scan on csv_accounts  (cost=0.00..44262.88 rows=1874 width=100)
   Filter: (bid = 1)
   Foreign File: /home/hanada/DB-fujita/CSV/pgbench_accounts.csv
   Foreign File Size: 47963070
(4 rows)

postgres=# explain select * from csv_accounts where bid = 2;
   QUERY PLAN
-
 Foreign Scan on csv_accounts  (cost=0.00..44262.88 rows=1874 width=100)
   Filter: (bid = 2)
   Foreign File: /home/hanada/DB-fujita/CSV/pgbench_accounts.csv
   Foreign File Size: 47963070
(4 rows)

postgres=# explain select * from csv_accounts where bid = 4;
QUERY PLAN
--
 Foreign Scan on csv_accounts  (cost=0.00..57105.00 rows=102117 width=97)
   Filter: (bid = 4)
   Foreign File: /home/hanada/DB-fujita/CSV/pgbench_accounts.csv