Re: [HACKERS] Range types

2009-12-14 Thread Takahiro Itagaki

Scott Bailey arta...@comcast.net wrote:

 CREATE TYPE periodtz AS RANGE (timestamptz, '1 microsecond'::interval);

What does the second argument mean? Is it a default interval?

 So basically I have a pg_range table to store the base typeid, a text 
 field for the granule value and the granule typeid.

As another approach, what about storing typeid in typmod?
(Oid can be assumed to be stored in int32.)

For example,
CREATE TABLE tbl ( r range(timestamp) );
SELECT '[ 2.0, 3.0 )'::range(float);

There might be some overhead to store typeid for each range instance,
but the typmod approach does not require additinal catalogs and syntax
changes. It can be possible even on 8.4.

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



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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Peter Eisentraut
On sön, 2009-12-13 at 19:20 +, Simon Riggs wrote:
 Barring resolving a few points and subject to even more testing, this
 is the version I expect to commit to CVS on Wednesday.

To clarify: Did you pick Wednesday so that it gets included before the
end of the commit fest (and thus into alpha3) or so that it gets into
CVS as early as possible after the commit fest (and thus not into
alpha3)?


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


[HACKERS] Crash on pg_ctl initdb without options

2009-12-14 Thread Takahiro Itagaki
pg_ctl initdb crashes if no options are passed.
Do we need additional if-null test for pgdata_opt just like post_opts?

$ pg_ctl initdb
sh: -c: line 0: syntax error near unexpected token `null'
sh: -c: line 0: `/usr/local/pgsql/bin/initdb (null)'
pg_ctl: database system initialization failed


Index: src/bin/pg_ctl/pg_ctl.c
===
--- src/bin/pg_ctl/pg_ctl.c (head)
+++ src/bin/pg_ctl/pg_ctl.c (fix)
@@ -656,6 +656,9 @@
if (exec_path == NULL)
exec_path = find_other_exec_or_die(argv0, initdb, initdb 
(PostgreSQL)  PG_VERSION \n);
 
+   if (pgdata_opt == NULL)
+   pgdata_opt = ;
+
if (post_opts == NULL)
post_opts = ;
 

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


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


Re: [HACKERS] Winflex

2009-12-14 Thread Dave Page
On Sun, Dec 13, 2009 at 11:29 AM, Magnus Hagander mag...@hagander.net wrote:
 On Sun, Dec 13, 2009 at 11:36, Dave Page dp...@pgadmin.org wrote:
 On Sun, Dec 13, 2009 at 5:42 AM, Andrew Dunstan and...@dunslane.net wrote:

 Yes. I spent a few cents and a few hours wrestling with it. AFAICT your are
 hosed on 64bit Windows. I can't get flex built and Cygwin is behaving very
 oddly. There are indications that the problem could be fairly deep - see
 http://www.mail-archive.com/cyg...@cygwin.com/msg102463.html

 What Linda describes there is all normal behaviour for a 32 bit app on
 64 bit Windows. Windows is providing a virtual 32 bit environment,
 where for the most part the 32 bit app doesn't realise it's running on
 64 bit. Unfortunately there are always things that look a bit odd due
 to this, but normally I've found that the 32bit code runs fine, it
 just looks odd from Explorer or 64 bit apps because of the
 folder/registry redirection that happens behind the scenes.

 Yeah, none of that should have an effect on a tool like flex, though...

Thats my point.

 The other possible option that I hesitate to suggest is Windows
 Services for Unix or SUA as I think it's now called.

 You mean we should post flex to that? Or have you found someone who has 
 already?

No idea if someone has done it already. I'm just throwing it out there
as an idea possibly worthy of further investigation.


-- 
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com

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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Simon Riggs
On Mon, 2009-12-14 at 10:11 +0200, Peter Eisentraut wrote:
 On sön, 2009-12-13 at 19:20 +, Simon Riggs wrote:
  Barring resolving a few points and subject to even more testing, this
  is the version I expect to commit to CVS on Wednesday.
 
 To clarify: Did you pick Wednesday so that it gets included before the
 end of the commit fest (and thus into alpha3) or so that it gets into
 CVS as early as possible after the commit fest (and thus not into
 alpha3)?

Wednesday because that seemed a good delay to allow review. Josh and I
had discussed the value of getting patch into Alpha3, so that was my
wish and aim.

I'm not aware of any particular date for end of commitfest, though
possibly you are about to update me on that?

(Perhaps we really need a single Project Management page that lists all
the dates that have been agreed, so that everybody can go there and be
clear. Commitfest start and end dates, beta dates, de-support dates etc.
BTW, it is absolutely brilliant that we have these now.)

-- 
 Simon Riggs   www.2ndQuadrant.com


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


Re: [HACKERS] Row-Level Security

2009-12-14 Thread KaiGai Kohei
One more issue I found.

What row-level policy should be applied on inherited tables?

If inconsistent policy is applied on the parent and child table,
we can see different result set, although a part of result set
in the parent table come from the child table.

My idea is to copy row-level policies to inherited tables from the
parent table. We can additional row-level policy on the inherited
tables, but all the condition is chained by AND, so here is no
inconsistency.
Even if the inherited table has multiple parents, all the row-level
policies shall be applied, so here is no inconsistency.
(Needless to say, child table have same columns, so we can apply
same row-level policies.)

Thanks,
-- 
OSS Platform Development Division, NEC
KaiGai Kohei kai...@ak.jp.nec.com

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


Re: [HACKERS] Range types

2009-12-14 Thread tomas
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On Sun, Dec 13, 2009 at 11:49:53PM -0800, Scott Bailey wrote:
 I had proposed a temporal contrib module earlier and you wanted to see 
 support for many range types not just timestamptz [...]

 So basically I have an anyrange pseudo type with the functions prev, next, 
 last, etc defined. So instead of hard coding range types, we would allow 
 the user to define their own range types. Basically if we are able to 
 determine the previous and next values of the base types we'd be able to 
 define a range type. I'm envisioning in a manner much like defining an enum 
 type.

 CREATE TYPE periodtz AS RANGE (timestamptz, '1 microsecond'::interval);
 CREATE TYPE numrange AS RANGE (numeric(8,2));
 -- determine granularity from typmod
 CREATE TYPE floatrange AS RANGE (float, '0.1'::float);

I might be asking the same as Itagaki is (see below) but... are you just
envisioning ranges on 'discrete' types? What about ranges on floats or
(arbitrary length) strings, where there is no prev/next? Too difficult?
(mind you: I don't know exactly what I'm talking about, but in would be
definitely useful).

On Mon, Dec 14, 2009 at 05:10:24PM +0900, Takahiro Itagaki wrote:
 
 Scott Bailey arta...@comcast.net wrote:
 
  CREATE TYPE periodtz AS RANGE (timestamptz, '1 microsecond'::interval);
 
 What does the second argument mean? Is it a default interval?

I think it's the granularity. That defines how to find the 'next' point
in time. As I understood it, Scott envisions ranges only for discrete
types, i.e. those advancing in well-defined steps. Note that (a) I might
be wrong and (b) there might be a very good reason for doing it this way.

  So basically I have a pg_range table to store the base typeid, a text 
  field for the granule value and the granule typeid.
 
 As another approach, what about storing typeid in typmod?
 (Oid can be assumed to be stored in int32.)
 
 For example,
 CREATE TABLE tbl ( r range(timestamp) );
 SELECT '[ 2.0, 3.0 )'::range(float);
 
 There might be some overhead to store typeid for each range instance,
 but the typmod approach does not require additinal catalogs and syntax
 changes. It can be possible even on 8.4.

This looks more natural to me too.

Regards
- -- tomás
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFLJgAZBcgs9XrR2kYRAljHAJwJjYV6fHz4qPSY6sXROYZ6pKIlGQCeO4X1
eszUJopVGqcPkXbiHdQOVrs=
=IYQ0
-END PGP SIGNATURE-

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


Re: [HACKERS] ECPG patch 2, SQLDA support

2009-12-14 Thread Boszormenyi Zoltan
Hi,

Jaime Casanova írta:
 On Mon, Nov 16, 2009 at 5:59 AM, Boszormenyi Zoltan z...@cybertec.at wrote:
   
 New version: rebased to current CVS.

 

 This one no longer applies to HEAD, could you update it please?
   

Will post a new version soon, Michael asked to turn it
into an ECPG native feature instead of compat-mode-only.

Best regards,
Zoltán Böszörményi

-- 
Bible has answers for everything. Proof:
But let your communication be, Yea, yea; Nay, nay: for whatsoever is more
than these cometh of evil. (Matthew 5:37) - basics of digital technology.
May your kingdom come - superficial description of plate tectonics

--
Zoltán Böszörményi
Cybertec Schönig  Schönig GmbH
http://www.postgresql.at/


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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Peter Eisentraut
On mån, 2009-12-14 at 08:54 +, Simon Riggs wrote:
 Wednesday because that seemed a good delay to allow review. Josh and I
 had discussed the value of getting patch into Alpha3, so that was my
 wish and aim.
 
 I'm not aware of any particular date for end of commitfest, though
 possibly you are about to update me on that?

Commit fests for 8.5 have usually run from the 15th to the 15th of next
month, but the CF manager may choose to vary that.

FWIW, the alpha release manager may also vary the release timeline of
alpha3. ;-)

 (Perhaps we really need a single Project Management page that lists all
 the dates that have been agreed, so that everybody can go there and be
 clear. Commitfest start and end dates, beta dates, de-support dates etc.
 BTW, it is absolutely brilliant that we have these now.)

We had
http://wiki.postgresql.org/wiki/PostgreSQL_8.4_Development_Plan, but I
don't think we ever actually agreed on a schedule for 8.5.



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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Heikki Linnakangas
Simon Riggs wrote:
 Enclose latest version of Hot Standby. This is the basic patch, with
 no must-fix issues and no known bugs. Further additions will follow
 after commit, primarily around usability, which will include additional
 control functions for use in testing. Various thoughts discussed here
 http://wiki.postgresql.org/wiki/Hot_Standby_TODO

I still consider it highly important to be able to start standby from a
shutdown checkpoint. If you remove it, at the very least put it back on
the TODO.

But as it is, StandbyRecoverPreparedTransactions() is dead code, and the
changes to PrescanPreparedTransactions() are not needed either.

 Patch now includes a set of regression tests that can be run against a
 standby server using make standbycheck

Nice!

 (requires setup, see src/test/regress/standby_schedule). 

Any chance of automating that?

 Complete with full docs and comments.
 
 Barring resolving a few points and subject to even more testing, this is
 the version I expect to commit to CVS on Wednesday. I would appreciate
 further objective testing before commit, if possible.

* Please remove any spurious whitespace.  git diff --color makes them
stand out like a sore thumb, in red. (pgindent will fix them but always
better to fix them before committing, IMO).

* vacuum_defer_cleanup_age is very hard to tune. You'll need an estimate
on your transaction rate to begin with. Do we really want this setting
in its current form? Does it make sense as PGC_USERSET, as if one
backend uses a lower setting than others, that's the one that really
determines when transactions are killed in the standby? I think this
should be discussed and implemented as a separate patch.

* Are you planning to remove the recovery_connections setting before
release? The documentation makes it sound like it's a temporary hack
that we're not really sure is needed at all. That's not very comforting.

* You removed this comment from KnownAssignedXidsInit:

-   /*
-* XXX: We should check that we don't exceed maxKnownAssignedXids.
-* Even though the hash table might hold a few more entries than that,
-* we use fixed-size arrays of that size elsewhere and expected all
-* entries in the hash table to fit.
-*/

but AFAICS you didn't address the issue. It's referring to the 'xids'
array in TransactionIdIsInProgress(), which KnownAssignedXidsGet() fills
in without checking that it fits.

* LockAcquireExtended needs a function comment. Or at least something
explaining what report_memory_error does. And perhaps the argument
should be named reportMemoryError to be in line with the other arguments.

* We tend to not add remarks about authors in code (comments in standby.c).

* This optimization in GetConflictingVirtualXIDs():

 +   /*
 +* If we don't know the TransactionId that created the conflict, set
 +* it to latestCompletedXid which is the latest possible value.
 +*/
 +   if (!TransactionIdIsValid(limitXmin))
 +   limitXmin = ShmemVariableCache-latestCompletedXid;
 +

needs a lot more explanation. It took me a very long time to figure out
why using latest completed xid is safe.

* Are you going to leave the trace_recovery GUC in?

* Can you merge with CVS HEAD, please? There's some merge conflicts.

 Last remaining points
 
 * NonTransactionalInvalidation logging has been removed following
 review, but AFAICS that means VACUUM FULL doesn't work correctly on
 catalog tables, which regrettably will be the only ones still standing
 even after we apply VFI patch. Did I misunderstand the original intent?
 Was it just buggy somehow? Or is this hoping VF goes completely, which
 seems unlikely in this release. Just noticed this, decided better to get
 stuff out there now.

I removed it in the hope that VF is gone before beta.

 * Are we OK with using hash indexes in standby plans, even when we know
 the indexes are stale and could return incorrect results?

It doesn't seem any more wrong than using hash indexes right after
recovery, crash recovery or otherwise. It's certainly broken, but I
don't see much value in a partial fix. The bottom line is that hash
indexes should be WAL-logged.

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

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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Magnus Hagander
On Mon, Dec 14, 2009 at 10:54, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 * Please remove any spurious whitespace.  git diff --color makes them
 stand out like a sore thumb, in red. (pgindent will fix them but always
 better to fix them before committing, IMO).

+1 in general, not particularly for this patch (haven't checked that
in this patch).

Actually, how about we add that to the page at
http://wiki.postgresql.org/wiki/Submitting_a_Patch?


-- 
 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] Range types

2009-12-14 Thread Robert Haas
On Mon, Dec 14, 2009 at 4:06 AM,  to...@tuxteam.de wrote:
 As another approach, what about storing typeid in typmod?
 (Oid can be assumed to be stored in int32.)

 For example,
     CREATE TABLE tbl ( r range(timestamp) );
     SELECT '[ 2.0, 3.0 )'::range(float);

 There might be some overhead to store typeid for each range instance,
 but the typmod approach does not require additinal catalogs and syntax
 changes. It can be possible even on 8.4.

 This looks more natural to me too.

It 's very different than the way we've traditionally used typmod,
though, which Tom described pretty well here:

http://archives.postgresql.org/pgsql-hackers/2009-11/msg01183.php

For example, function signatures ignore typmod, so you'll be able to
write a function that takes a range, but you won't know what kind of
range you're getting.  Pavel proposed changing that, but the problem
is that while you might want to discriminate on the basis of what sort
of range you're getting, you probably DON'T want to discriminate on
the length of the character string being passed in with a varchar
argument, or the number of decimal places in a numeric.

So I think this is going to be awkward.

Also, typid is unsigned and typmod is signed.  Again, awkward.  Maybe
with a big enough crowbar you can make it work, but it seems like it
won't be pretty...

...Robert

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


[HACKERS] Re: Hot Standby, deferred conflict resolution for cleanup records (v2)

2009-12-14 Thread Simon Riggs
On Mon, 2009-12-14 at 04:57 +, Greg Stark wrote:
 On Sat, Dec 12, 2009 at 3:06 PM, Simon Riggs si...@2ndquadrant.com wrote:
  Anyone acquiring a lock on a table should check the latestRemovedXid for
  the table and abort if their xmin is too old. This prevents new lockers
  from accessing a cleaned relation immediately after we decide to abort
  anyone looking at that table. (Anyone queuing for the existing locks
  would be caught by this).
 
 I fear given HOT pruning that this could mean no query can even get
 started against a busy table. It seems like you would have to start
 your transaction several times until you manage to get a lock on the
 busy table soon enough after taking the snapshot to not have missed
 any cleanups in the table. 

The proposal improves this situation. Right now we would cancel all
queries, not just the ones looking at the busy table.

 Or have I missed something that protects
 against that?

At your suggestion, I previously added a feature, described in docs:

It is also possible to set vacuum_defer_cleanup_age on the primary to
defer the cleanup of records by autovacuum, vacuum and HOT. This may
allow more time for queries to execute before they are cancelled on the
standby, without the need for setting a high max_standby_delay.

vacuum_defer_cleanup_age delays globalxmin by a fixed number of xids.

That is fairly crude and so the proposal here is to add a finer-grained
conflict resolution, please read on.

 The bigger problem with this is that I don't see any way to tune this
 to have a safe replica. In the current system you can set
 standby_max_delay to 0 or -1 or whatever to completely disable killing
 off valid queries on the replica. In this setup you're going ahead
 with cleanup records which may or may not be safe and then have no
 recourse if they turn out to conflict.

Attempting a full analysis...

An example of current and proposed behaviours, using tables A, B and C
T0: An AccessExclusiveLock is applied to B
T1: Q1 takes snapshot, takes lock on A and begins query 
T2: Q2 takes snapshot, queues for lock on B behind AccessExclusiveLock
T3: Cleanup on table C is handled that will conflict with both snapshots
T4: Q3 takes snapshot, takes lock on C and begins query (if possible)
T5: Cleanup on table C is handled that will conflict with Q3

Current: At T3, current conflict resolution will wait for
max_standby_delay and then cancel Q1 and Q2. Q3 can begin processing
immediately because the snapshot it takes will always be same or later
than the xmin that generated the cleanup at T3 (*). At T5, Q3 will be
quickly cancelled because all the standby delay was used up at T3 and
there is none left to spend on delaying for Q3.
(*) is obviously a key assumption.

Proposal1: Conflict resolution will not wait at T3 at all and Q1 and Q2
will continue towards completion. At T5, Q3 will be cancelled without
much delay, as explained for current.

Proposal1 seems better than current situation.

Taking up your point about timing delays, if the sequence of actions is
T6: Q4 takes snapshot
T7: commit of transaction that advances xmin
T8: Cleanup on table C handled without delay
T9: Q4 takes lock on C and cancels
then yes, Q4 is cancelled without a delay. There is a possible race
condition that would allow this, but in the vast majority of read
committed queries this would be a small window, since T7, T8 are seldom
adjacent in WAL, whereas T6, T9 are typically very fast.  If the race
does occur then the effect is not incorrect query results, just a query
cancelled earlier than we would ideally like it to have been.

A slight modification to the proposal would be to check for conflicts
based upon the snapshot first, wait, then check for lock conflicts
before cancelling, rather than the other way around. That closes the
timing window you've pointed out, at least as far as max_standby_delay.
Call that Proposal2.

The first example would then result like this:
Proposal2: Conflict resolution will wait at T3; Q1 and Q2 will continue
towards completion because there is no lock conflict. At T5, Q3 will be
cancelled without much delay, as explained for current. So almost
identical outcome in the typical case, but Proposal 2 doesn't cancel
queries early in some cases.

In summary:
* Current: Q1, Q2 and Q3 are cancelled.
* Proposal1: Q1, Q2 continue until completion. Q3 is cancelled, with
roughly same delay as in current proposal.
* Proposal2: Q1, Q2 continue until completion. Q3 is cancelled, with
roughly same delay as in current proposal, though consistent handling in
all, not just typical cases.

So Proposal2 wins, though problems still possible.

AFAICS the faster a table generates cleanup records, the shorter the
window of opportunity for queries to run against it without conflict on
Hot Standby. The worst case is a single row table being updated
constantly by short transactions, which will generate a rapid stream of
cleanup from WAL. The only simple solution in that case is to pause the

Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Simon Riggs
Thanks for the further review, much appreciated.

On Mon, 2009-12-14 at 11:54 +0200, Heikki Linnakangas wrote:
 Simon Riggs wrote:
  Enclose latest version of Hot Standby. This is the basic patch, with
  no must-fix issues and no known bugs. Further additions will follow
  after commit, primarily around usability, which will include additional
  control functions for use in testing. Various thoughts discussed here
  http://wiki.postgresql.org/wiki/Hot_Standby_TODO
 
 I still consider it highly important to be able to start standby from a
 shutdown checkpoint. If you remove it, at the very least put it back on
 the TODO.

Happy to put it back on TODO, but I'm not likely to do this without a
good reason. IMHO your arguments as to why that is useful were not
convincing, especially when it introduces further complications and
requirements for tests.

 But as it is, StandbyRecoverPreparedTransactions() is dead code, and the
 changes to PrescanPreparedTransactions() are not needed either.
 
  Patch now includes a set of regression tests that can be run against a
  standby server using make standbycheck
 
 Nice!
 
  (requires setup, see src/test/regress/standby_schedule). 
 
 Any chance of automating that?

Future, yes. 

I view standbycheck as similar to installcheck - it requires some setup
before it can run, so allows you to test an existing server. I see the
same need here.

  Complete with full docs and comments.
  
  Barring resolving a few points and subject to even more testing, this is
  the version I expect to commit to CVS on Wednesday. I would appreciate
  further objective testing before commit, if possible.
 
 * Please remove any spurious whitespace.  git diff --color makes them
 stand out like a sore thumb, in red. (pgindent will fix them but always
 better to fix them before committing, IMO).

What is your definition of spurious whitespace? I removed all
additions/deletions of individual lines. git diff colours many things
red, and it seems like a waste of time to spend hours fiddling with
spaces manually if there is a utility that does this anyway.

 * vacuum_defer_cleanup_age is very hard to tune. You'll need an estimate
 on your transaction rate to begin with. Do we really want this setting
 in its current form? Does it make sense as PGC_USERSET, as if one
 backend uses a lower setting than others, that's the one that really
 determines when transactions are killed in the standby? I think this
 should be discussed and implemented as a separate patch.

All the vacuum_*_age parameters have this characteristic, yet they
exist.

I would like to provide simple features for conflict avoidance in the
first alpha release. If we find that nobody found it useful then it can
be removed easily enough.

It's a USERSET now, but it could also be other things. This patch isn't
the end of discussion, for many people it will be the start.

 * Are you planning to remove the recovery_connections setting before
 release? The documentation makes it sound like it's a temporary hack
 that we're not really sure is needed at all. That's not very comforting.

It has been requested and I agree, so its there. Saying it might be
removed in future is no more than we do elsewhere and AFAIK we all hope
it will be. Not sure why that is or isn't comforting.

 * You removed this comment from KnownAssignedXidsInit:
 
 -   /*
 -* XXX: We should check that we don't exceed maxKnownAssignedXids.
 -* Even though the hash table might hold a few more entries than that,
 -* we use fixed-size arrays of that size elsewhere and expected all
 -* entries in the hash table to fit.
 -*/
 
 but AFAICS you didn't address the issue. It's referring to the 'xids'
 array in TransactionIdIsInProgress(), which KnownAssignedXidsGet() fills
 in without checking that it fits.

I have ensured that they are always the same size, by definition, so no
need to check.

 * LockAcquireExtended needs a function comment. Or at least something
 explaining what report_memory_error does. And perhaps the argument
 should be named reportMemoryError to be in line with the other arguments.

OK

 * We tend to not add remarks about authors in code (comments in standby.c).

OK

 * This optimization in GetConflictingVirtualXIDs():
 
  +   /*
  +* If we don't know the TransactionId that created the conflict, set
  +* it to latestCompletedXid which is the latest possible value.
  +*/
  +   if (!TransactionIdIsValid(limitXmin))
  +   limitXmin = ShmemVariableCache-latestCompletedXid;
  +
 
 needs a lot more explanation. It took me a very long time to figure out
 why using latest completed xid is safe.

OK. Took me a long time as well.

 * Are you going to leave the trace_recovery GUC in?

For now, at least. I have a later proposal around that to follow.

 * Can you merge with CVS HEAD, please? There's some merge conflicts.

Huh? I did. And tested patch against a CVS checkout before submitting.
Can you explain further?

  Last remaining points
 

Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Simon Riggs
On Mon, 2009-12-14 at 11:09 +0100, Magnus Hagander wrote:
 On Mon, Dec 14, 2009 at 10:54, Heikki Linnakangas
 heikki.linnakan...@enterprisedb.com wrote:
  * Please remove any spurious whitespace.  git diff --color makes them
  stand out like a sore thumb, in red. (pgindent will fix them but always
  better to fix them before committing, IMO).
 
 +1 in general, not particularly for this patch (haven't checked that
 in this patch).
 
 Actually, how about we add that to the page at
 http://wiki.postgresql.org/wiki/Submitting_a_Patch?

If we can define spurious whitespace it would help decide whether
there is any action to take, and when.

-- 
 Simon Riggs   www.2ndQuadrant.com


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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Fujii Masao
On Mon, Dec 14, 2009 at 8:07 PM, Simon Riggs si...@2ndquadrant.com wrote:
 * Please remove any spurious whitespace.  git diff --color makes them
 stand out like a sore thumb, in red. (pgindent will fix them but always
 better to fix them before committing, IMO).

 What is your definition of spurious whitespace? I removed all
 additions/deletions of individual lines. git diff colours many things
 red, and it seems like a waste of time to spend hours fiddling with
 spaces manually if there is a utility that does this anyway.

I guess it's a trailing whitespace. How about using git diff --check
instead of --color?

Regards,

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

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


Re: [HACKERS] Row-Level Security

2009-12-14 Thread Robert Haas
2009/12/14 KaiGai Kohei kai...@ak.jp.nec.com:
 Robert Haas wrote:
 One point. MAC is mandatory, so the table owner should not be able to
 control whether row-level checks are applied, or not.
 So, I used a special purpose system column to represent security label.
 It is generated for each tables, and no additional storage consumption
 when MAC feature is disabled.
 My current feeling is that a special-purpose system column is not the
 best approach.  I don't see what we gain by doing it that way.  Even
 in an SE-PostgreSQL environment, row-level security might not be
 desired on every table - after all, we've been told that SE-PostgreSQL
 is useful without any row-level security AT ALL, so it's not hard to
 think there could be environments where only some tables need to
 protected.  So I think we want to have a way to turn it on and off on
 a per-table basis.

 Of course, as you point out, we have to make sure that anyone who
 tries to turn RLS on or off for a particular table is authorized to
 perform that operation.  But that's a separate problem which is I
 don't think has much to do with row-level security.
 Yes, it is a separate problem not to be concluded at the moment.
 (Perhaps, it depends on security model. In DAC, per-table basis is 
 preferable.)

 Even for MAC, it might be desirable to turn it off on codes tables or
 the like, to minimize the performance hit.  But we can defer this
 question to another day.

 Yes, I provide sepgsql_row_level guc in my local branch to turn on/off
 its row-level controls. It allows to reduce performance penalty related
 to RLS and reduce storage consumption for security labels. (It requires
 additional sizeof(Oid) bytes for each tuples.)
 The point is this guc option is configurable from the only administrator
 who can edit $PGDATA/postgresql.conf.

 But it is an implementation detail not to be concluded at the moment.

Well, that would be a global switch, not per table.

 If we set up database cluster without any label-based MAC, all the tuple
 shall not have any security label. If the security label is stored within
 regular column, we have to modify schema for any tables at first.
 If system column provides a security label of tuple, we can dynamically
 generate an appropriate security label. In SELinux case, it assumes any
 unlabeled objects performs as if it has a pseudo security label:
  system_u:object_r:unlabeled_t:s0

 Needless to say, we need to assign appropriate security labels for
 meaningful access controls later, but it does not require any schema
 changes, even if we repeat to turn on/off the label-based MAC feature.

 When label-based MAC feature is disabled, this system column can return
 a pseudo value such as NULL or empty string.

 I think you are wrong about all of this.  To add security labels to
 existing tuples, you're going to need to rewrite the table, period.
 Whether you're adding a column in the process or just populating the
 contents of a previous-omitted column doesn't seem particularly
 relevant.  Similarly you can insert a pseudo security label when the
 column is missing just as well as you can when it's present but
 unpopulated.

 For system catalogs, we cannot touch its schema with a light heart,
 even if active enhanced security provider is switched or turned on/off.
 If we define a common system column for all the label-based MAC,
 it can be available for both of user tables and system catalogs
 without any table-rewrite process.

 But it is an implementation detail not to be concluded at the moment.

Err... well, as I said upthread: None of this addresses the issue of
doing RLS on system catalogs, which seems like a much harder problem,
possibly one that we should just ignore for the first phase of this
project.  So yeah, I agree: it won't work for system catalogs.

[snip]

 * Foreign Key constraint(2)

 FK is implemented as a trigger which internally uses SELECT/UPDATE/DELETE.
 If associated tuples are filtered out, it breaks reference integrity.
 So, we have to apply special care. In SE-PgSQL case, it raises an error
 instead of filtering during FK checks. And, row-level security hook is
 called at the last for each tuples, unlike normal cases.
 Perfecting referential integrity here seems like a pretty tough
 problem, but it's likely not necessary to solve it in order to get an
 implementation of row-level security that is useful for some purposes.
 Is the approach in SE-PgSQL suitable for the issue?
 It can prevent to update/delete tuple referenced by invisible tuples.

 We have two modes in row-level security.
 The first is filtering-mode. It applies security policy function prior
 to any other user given conditions, and filters out violated tuples from
 the result set.
 The second is aborting-mode. It is only used by internal stuff which does
 not provide any malicious function in the condition. It applies security
 policy function next to all the WHERE clause, and raises an error if the
 query tries to refer 

Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Robert Haas
On Mon, Dec 14, 2009 at 6:11 AM, Simon Riggs si...@2ndquadrant.com wrote:
 On Mon, 2009-12-14 at 11:09 +0100, Magnus Hagander wrote:
 On Mon, Dec 14, 2009 at 10:54, Heikki Linnakangas
 heikki.linnakan...@enterprisedb.com wrote:
  * Please remove any spurious whitespace.  git diff --color makes them
  stand out like a sore thumb, in red. (pgindent will fix them but always
  better to fix them before committing, IMO).

 +1 in general, not particularly for this patch (haven't checked that
 in this patch).

 Actually, how about we add that to the page at
 http://wiki.postgresql.org/wiki/Submitting_a_Patch?

 If we can define spurious whitespace it would help decide whether
 there is any action to take, and when.

git defines it as either (1) extra whitespace at the end of a line or
(2) an initial indent that uses spaces followed by tabs (typically
something like space-tab, where tab alone would have produced the same
result).  git diff --check master tends to be useful here.

...Robert

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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Simon Riggs
On Mon, 2009-12-14 at 06:21 -0500, Robert Haas wrote:
 On Mon, Dec 14, 2009 at 6:11 AM, Simon Riggs si...@2ndquadrant.com wrote:
  On Mon, 2009-12-14 at 11:09 +0100, Magnus Hagander wrote:
  On Mon, Dec 14, 2009 at 10:54, Heikki Linnakangas
  heikki.linnakan...@enterprisedb.com wrote:
   * Please remove any spurious whitespace.  git diff --color makes them
   stand out like a sore thumb, in red. (pgindent will fix them but always
   better to fix them before committing, IMO).
 
  +1 in general, not particularly for this patch (haven't checked that
  in this patch).
 
  Actually, how about we add that to the page at
  http://wiki.postgresql.org/wiki/Submitting_a_Patch?
 
  If we can define spurious whitespace it would help decide whether
  there is any action to take, and when.
 
 git defines it as either (1) extra whitespace at the end of a line or
 (2) an initial indent that uses spaces followed by tabs (typically
 something like space-tab, where tab alone would have produced the same
 result).  git diff --check master tends to be useful here.

(2) is a problem that has been discussed before on hackers, anything
like that should be changed.

Why is (1) important, and if it is important, why is it being mentioned
only now? Are we saying that all previous reviewers of my work (and
others') removed these without ever mentioning they had done so?

-- 
 Simon Riggs   www.2ndQuadrant.com


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


Re: [HACKERS] [PATCH] ACE Framework - Database, Schema

2009-12-14 Thread Robert Haas
2009/12/14 KaiGai Kohei kai...@ak.jp.nec.com:
 Robert Haas wrote:
 2009/12/13 KaiGai Kohei kai...@ak.jp.nec.com:
 Just to name a few really obvious problems (I only looked at the
 01-database patch):

 1. We have been talking for several days about the need to make the
 initial patch in this area strictly a code cleanup patch.  Is this
 cleaner than the code that it is replacing?  Is it even making an
 attempt to conform to that mandate?
 Even if it is unclear whether the current form is more clear than the
 current inlined pg_xxx_aclcheck() form, or not, it will obviously
 provide a set of common entry points for upcoming enhanced security
 providers.
 Eventually, it is more clear than enumeration of #ifdef ... #endif
 blocks for SELinux, Smack, Solaris-TX and others.

 Right, but it will also not get committed.  :-(

 The framework will be necessary to get them committed.
 Which is an egg, and which is a chicken? :-(

We've been around that path a few times, but that's not my point here.
 Doing the framework first makes a lot of sense; the problem is that
we just had a design discussion regarding that framework and you've
chosen to do something other than what was discussed.  Did you not
read that discussion?  Did you not understand it?

Unfortunately, what this project has turned into is a very long series
of patch submissions that all basically have the same problems.  The
code is messy.  It doesn't conform to project style.  It embeds
undiscussed design assumptions that the community does not endorse.
It has poorly factored interfaces that may serve SE-PostgreSQL
adequately for the moment but are likely to require constant
rejiggering as new problems arise.  It is filled with shims that don't
accomplish anything useful and other places where useful shims are
left out.

Now, it may be that even if you respond to all of the comments and fix
all of the issues that people are concerned about, the patch still
won't get committed.  As you know, Tom is very skeptical that the
user-base for this feature is large enough to warrant the work that it
will take.   But my point is that you seem to be very deliberately not
fixing those problems, and I don't see how we're going to make any
progress that way.  Many of the problems that I found in my review of
this patch were covered in design discussions that happened this week,
and yet the patch shows almost no evidence of those design
discussions.

Frankly, I find that kind of depressing.

...Robert

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


Re: [HACKERS] Range types

2009-12-14 Thread tomas
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On Mon, Dec 14, 2009 at 06:02:04AM -0500, Robert Haas wrote:
 On Mon, Dec 14, 2009 at 4:06 AM,  to...@tuxteam.de wrote:

[...]

  This looks more natural to me too.
 
 It 's very different than the way we've traditionally used typmod,
 though, which Tom described pretty well here:
 
 http://archives.postgresql.org/pgsql-hackers/2009-11/msg01183.php
 
 For example, function signatures ignore typmod, so you'll be able to
 write a function that takes a range, but you won't know what kind of
 range you're getting [...]

 Also, typid is unsigned and typmod is signed.  Again, awkward [...]

Ugh. I see. Thank you for this very insightful comment.

Regards
- -- tomás
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFLJiVuBcgs9XrR2kYRAp4ZAJsHjzYuVxwaeAUr1ogqRsZOecxdcwCeLfUv
8lZmeY6lb4r+57c6ZdB0J9M=
=0Ips
-END PGP SIGNATURE-

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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Robert Haas
On Mon, Dec 14, 2009 at 6:35 AM, Simon Riggs si...@2ndquadrant.com wrote:
 On Mon, 2009-12-14 at 06:21 -0500, Robert Haas wrote:
 On Mon, Dec 14, 2009 at 6:11 AM, Simon Riggs si...@2ndquadrant.com wrote:
  On Mon, 2009-12-14 at 11:09 +0100, Magnus Hagander wrote:
  On Mon, Dec 14, 2009 at 10:54, Heikki Linnakangas
  heikki.linnakan...@enterprisedb.com wrote:
   * Please remove any spurious whitespace.  git diff --color makes them
   stand out like a sore thumb, in red. (pgindent will fix them but always
   better to fix them before committing, IMO).
 
  +1 in general, not particularly for this patch (haven't checked that
  in this patch).
 
  Actually, how about we add that to the page at
  http://wiki.postgresql.org/wiki/Submitting_a_Patch?
 
  If we can define spurious whitespace it would help decide whether
  there is any action to take, and when.

 git defines it as either (1) extra whitespace at the end of a line or
 (2) an initial indent that uses spaces followed by tabs (typically
 something like space-tab, where tab alone would have produced the same
 result).  git diff --check master tends to be useful here.

 (2) is a problem that has been discussed before on hackers, anything
 like that should be changed.

 Why is (1) important, and if it is important, why is it being mentioned
 only now? Are we saying that all previous reviewers of my work (and
 others') removed these without ever mentioning they had done so?

pgident will remove such white spaces and create merge conflicts for
everyone working on those areas of the code.  I certainly mention this
in any review I do where it's applicable, and have been doing so for
some time.  I also will certainly fix it for any code I commit.  I
also mentioned it in the review that I did of Hot Standby.

...Robert

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


Re: [HACKERS] Row-Level Security

2009-12-14 Thread KaiGai Kohei

(2009/12/14 20:18), Robert Haas wrote:

* Foreign Key constraint(2)

FK is implemented as a trigger which internally uses SELECT/UPDATE/DELETE.
If associated tuples are filtered out, it breaks reference integrity.
So, we have to apply special care. In SE-PgSQL case, it raises an error
instead of filtering during FK checks. And, row-level security hook is
called at the last for each tuples, unlike normal cases.

Perfecting referential integrity here seems like a pretty tough
problem, but it's likely not necessary to solve it in order to get an
implementation of row-level security that is useful for some purposes.

Is the approach in SE-PgSQL suitable for the issue?
It can prevent to update/delete tuple referenced by invisible tuples.

We have two modes in row-level security.
The first is filtering-mode. It applies security policy function prior
to any other user given conditions, and filters out violated tuples from
the result set.
The second is aborting-mode. It is only used by internal stuff which does
not provide any malicious function in the condition. It applies security
policy function next to all the WHERE clause, and raises an error if the
query tries to refer violated tuples.


Hmm... the idea of having two modes doesn't sound right off the top of
my head.  But I think we have a long time before we need worry about
this.  We have neither SE-PostgreSQL nor RLS in core, nor are either
one anywhere close to being merged.  So worrying about how the two
will interact when we have both is putting the cart before the horse.
A lot can change between now and then.


IIRC, I've not gotten any opposition about this two-modes design.
Most of arguments about RLS were information leaks via covert-channels
which allows us to estimate an existence of invisible PK/FK.
But we don't define it as a problem to be resolved.


I know that was one of Tom's concerns.  Personally, my concerns are:

1. I want to implement row-level security in a way that is useful for
people who don't care about SE-PostgreSQL.  I think there are lots of
people who would be interested in that.  In fact, as Josh said, there
are probably MORE people who are interested in the constraint-based
approach than there are who want label-based security a la
SE-PostgreSQL.


I also agree it is a right direction. SELinux shall be one of them to
make access control decision in row-level.
In my current standpoint, we add general row-level security first, then
SELinux support provides its access control decision function. OK?


2. I want to implement row-level security in a way that is very
flexible and allows for a wide range of access control policies.  The
core row-level security mechanism should not care about or prejudge a
particular policy - it should just be a mechanism for enforcing
row-filtering.


Ditto,


3. I want to implement row-level security in a way that allows the
planner maximum flexibility in implementing the row filtering that is
needed in a particular case.  SE-PostgreSQL RLS presumes what is
essentially an additional join against the security table ID for every
table in the query - doing this in a way that allows joins to be
reordered or implemented in multiple ways (straight nestloop, nestloop
with inner indexscan, hash join) will drastically improve performance.
  The original implementation didn't actually implement it as a join,
but rather with special-case code that performed the security ID
lookups as part of the heap scan.  That's not going to work for any
kind of row-level security other than SE-PostgreSQL (so, see points 1
and 2) and it's also going to make the performance much worse than it
needs to be.  Granted, the performance is never going to be GOOD, but
we should try to at least make it not ATROCIOUS.


The reason why I put on the security hook in ExecScan() is to avoid the
problem that row-cost user defined function can be evaluated earlier
than row-level security policy. (I believed it was a well-known problem
at that time yet.) So, I didn't want to append it before optimization.

I also believe this matter should be resolved when we provide row-level
security stuff, because it is a security feature.

Thanks,
--
KaiGai Kohei kai...@kaigai.gr.jp

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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Magnus Hagander
On Mon, Dec 14, 2009 at 12:51, Robert Haas robertmh...@gmail.com wrote:
 On Mon, Dec 14, 2009 at 6:35 AM, Simon Riggs si...@2ndquadrant.com wrote:
 On Mon, 2009-12-14 at 06:21 -0500, Robert Haas wrote:
 On Mon, Dec 14, 2009 at 6:11 AM, Simon Riggs si...@2ndquadrant.com wrote:
  On Mon, 2009-12-14 at 11:09 +0100, Magnus Hagander wrote:
  On Mon, Dec 14, 2009 at 10:54, Heikki Linnakangas
  heikki.linnakan...@enterprisedb.com wrote:
   * Please remove any spurious whitespace.  git diff --color makes them
   stand out like a sore thumb, in red. (pgindent will fix them but always
   better to fix them before committing, IMO).
 
  +1 in general, not particularly for this patch (haven't checked that
  in this patch).
 
  Actually, how about we add that to the page at
  http://wiki.postgresql.org/wiki/Submitting_a_Patch?
 
  If we can define spurious whitespace it would help decide whether
  there is any action to take, and when.

 git defines it as either (1) extra whitespace at the end of a line or
 (2) an initial indent that uses spaces followed by tabs (typically
 something like space-tab, where tab alone would have produced the same
 result).  git diff --check master tends to be useful here.

 (2) is a problem that has been discussed before on hackers, anything
 like that should be changed.

 Why is (1) important, and if it is important, why is it being mentioned
 only now? Are we saying that all previous reviewers of my work (and
 others') removed these without ever mentioning they had done so?

 pgident will remove such white spaces and create merge conflicts for
 everyone working on those areas of the code.  I certainly mention this
 in any review I do where it's applicable, and have been doing so for
 some time.  I also will certainly fix it for any code I commit.  I
 also mentioned it in the review that I did of Hot Standby.

I also always do this when committing other peoples patches (which I
don't do as often as I should, but when I *do* do it..)


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

2009-12-14 Thread Andrew Dunstan


Dave Page wrote:
  

The other possible option that I hesitate to suggest is Windows
Services for Unix or SUA as I think it's now called.
  

You mean we should post flex to that? Or have you found someone who has already?



No idea if someone has done it already. I'm just throwing it out there
as an idea possibly worthy of further investigation.
  



Don't you need to have SUA installed to run a program built with SUA? Or 
are you suggesting we can build a distributable flex executable with SUA 
which would run standalone (as I did for 32bit Windows using Cygwin)? 
Requiring developers to install SUA would be quite a siginificant extra 
burden, ISTM.


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

2009-12-14 Thread Dave Page
On Mon, Dec 14, 2009 at 12:10 PM, Andrew Dunstan and...@dunslane.net wrote:

 Dave Page wrote:



 The other possible option that I hesitate to suggest is Windows
 Services for Unix or SUA as I think it's now called.


 You mean we should post flex to that? Or have you found someone who has
 already?


 No idea if someone has done it already. I'm just throwing it out there
 as an idea possibly worthy of further investigation.



 Don't you need to have SUA installed to run a program built with SUA? Or are
 you suggesting we can build a distributable flex executable with SUA which
 would run standalone (as I did for 32bit Windows using Cygwin)? Requiring
 developers to install SUA would be quite a siginificant extra burden, ISTM.

The latter. And I have no idea if it's actually an option - just
throwing it out there as an idea.


-- 
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com

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


Re: [HACKERS] [patch] executor and slru dtrace probes

2009-12-14 Thread Bernd Helmle



--On 10. Dezember 2009 16:49:50 +0100 Zdenek Kotala zdenek.kot...@sun.com 
wrote:



You need to determine which SLRU is used. Because SLRUs are initialized
during startup  pointer should be same in all backends. If I think more
about it. Maybe it could be goot to add probe also into SimpleLruInit to
catch name of SLRUs.


Hi Zdenek,

do you plan to work on this further? I was about to mark the SLRU probes as 
ready for committer...


--
Thanks

Bernd

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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Simon Riggs
On Mon, 2009-12-14 at 11:07 +, Simon Riggs wrote:
 Thanks for the further review, much appreciated.
 
 On Mon, 2009-12-14 at 11:54 +0200, Heikki Linnakangas wrote:
  Simon Riggs wrote:
   Enclose latest version of Hot Standby. 
  
  * LockAcquireExtended needs a function comment. Or at least something
  explaining what report_memory_error does. And perhaps the argument
  should be named reportMemoryError to be in line with the other arguments.
 
 OK

Done

  * We tend to not add remarks about authors in code (comments in standby.c).
 
 OK

Done

  * This optimization in GetConflictingVirtualXIDs():
  
   +   /*
   +* If we don't know the TransactionId that created the conflict, set
   +* it to latestCompletedXid which is the latest possible value.
   +*/
   +   if (!TransactionIdIsValid(limitXmin))
   +   limitXmin = ShmemVariableCache-latestCompletedXid;
   +
  
  needs a lot more explanation. It took me a very long time to figure out
  why using latest completed xid is safe.
 
 OK. Took me a long time as well.

Done

  * Can you merge with CVS HEAD, please? There's some merge conflicts.
 
 Huh? I did. And tested patch against a CVS checkout before submitting.
 Can you explain further?

Still not sure what conflicts you see or where they might come from...

I am now unable to push these changes to the shared repo. What is
happening?

-- 
 Simon Riggs   www.2ndQuadrant.com


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


Re: [HACKERS] [PATCH] ACE Framework - Database, Schema

2009-12-14 Thread KaiGai Kohei

(2009/12/14 20:48), Robert Haas wrote:

2009/12/14 KaiGai Koheikai...@ak.jp.nec.com:

Robert Haas wrote:

2009/12/13 KaiGai Koheikai...@ak.jp.nec.com:

Just to name a few really obvious problems (I only looked at the
01-database patch):

1. We have been talking for several days about the need to make the
initial patch in this area strictly a code cleanup patch.  Is this
cleaner than the code that it is replacing?  Is it even making an
attempt to conform to that mandate?

Even if it is unclear whether the current form is more clear than the
current inlined pg_xxx_aclcheck() form, or not, it will obviously
provide a set of common entry points for upcoming enhanced security
providers.
Eventually, it is more clear than enumeration of #ifdef ... #endif
blocks for SELinux, Smack, Solaris-TX and others.


Right, but it will also not get committed.  :-(


The framework will be necessary to get them committed.
Which is an egg, and which is a chicken? :-(


We've been around that path a few times, but that's not my point here.
  Doing the framework first makes a lot of sense; the problem is that
we just had a design discussion regarding that framework and you've
chosen to do something other than what was discussed.  Did you not
read that discussion?  Did you not understand it?


Please point out, if my understanding is incorrect from the discussion
in a few days.

* As a draft of the discussion, I have to split out the access control
  reworks patch in the 2nd CF per object classes.
* This framework supports only the default PG privileges at the moment.
* The way to host enhanced security providers are not decided.
  (Maybe #ifdef ... #endif block, Maybe function pointer)
* It is not decided how many security labels are assigned on a database
  object. (Maybe 1:1, Maybe 1:n)

I don't intend to go to something undecided, but, might understand
something incorrectly or not be able to follow the discussion enough.

Thanks,
--
KaiGai Kohei kai...@kaigai.gr.jp

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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Robert Haas
On Mon, Dec 14, 2009 at 7:22 AM, Simon Riggs si...@2ndquadrant.com wrote:
 I am now unable to push these changes to the shared repo. What is
 happening?

Perhaps you need to run cvs update on your local copy?

(I find this flavor the most useful: cvs -q update -d  YMMV.)

If that's not it, error message?

...Robert

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


Re: [HACKERS] [PATCH] ACE Framework - Database, Schema

2009-12-14 Thread Robert Haas
On Mon, Dec 14, 2009 at 7:30 AM, KaiGai Kohei kai...@kaigai.gr.jp wrote:
 (2009/12/14 20:48), Robert Haas wrote:

 2009/12/14 KaiGai Koheikai...@ak.jp.nec.com:

 Robert Haas wrote:

 2009/12/13 KaiGai Koheikai...@ak.jp.nec.com:

 Just to name a few really obvious problems (I only looked at the
 01-database patch):

 1. We have been talking for several days about the need to make the
 initial patch in this area strictly a code cleanup patch.  Is this
 cleaner than the code that it is replacing?  Is it even making an
 attempt to conform to that mandate?

 Even if it is unclear whether the current form is more clear than the
 current inlined pg_xxx_aclcheck() form, or not, it will obviously
 provide a set of common entry points for upcoming enhanced security
 providers.
 Eventually, it is more clear than enumeration of #ifdef ... #endif
 blocks for SELinux, Smack, Solaris-TX and others.

 Right, but it will also not get committed.  :-(

 The framework will be necessary to get them committed.
 Which is an egg, and which is a chicken? :-(

 We've been around that path a few times, but that's not my point here.
  Doing the framework first makes a lot of sense; the problem is that
 we just had a design discussion regarding that framework and you've
 chosen to do something other than what was discussed.  Did you not
 read that discussion?  Did you not understand it?

 Please point out, if my understanding is incorrect from the discussion
 in a few days.

 * As a draft of the discussion, I have to split out the access control
  reworks patch in the 2nd CF per object classes.
 * This framework supports only the default PG privileges at the moment.
 * The way to host enhanced security providers are not decided.
  (Maybe #ifdef ... #endif block, Maybe function pointer)
 * It is not decided how many security labels are assigned on a database
  object. (Maybe 1:1, Maybe 1:n)

 I don't intend to go to something undecided, but, might understand
 something incorrectly or not be able to follow the discussion enough.

Hmm...  all of those things are true, but it seems to leave quite a bit out.

...Robert

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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Simon Riggs
On Mon, 2009-12-14 at 06:51 -0500, Robert Haas wrote:
 On Mon, Dec 14, 2009 at 6:35 AM, Simon Riggs si...@2ndquadrant.com wrote:
  On Mon, 2009-12-14 at 06:21 -0500, Robert Haas wrote:
  On Mon, Dec 14, 2009 at 6:11 AM, Simon Riggs si...@2ndquadrant.com wrote:
   On Mon, 2009-12-14 at 11:09 +0100, Magnus Hagander wrote:
   On Mon, Dec 14, 2009 at 10:54, Heikki Linnakangas
   heikki.linnakan...@enterprisedb.com wrote:
* Please remove any spurious whitespace.  git diff --color makes 
them
stand out like a sore thumb, in red. (pgindent will fix them but 
always
better to fix them before committing, IMO).
  
   +1 in general, not particularly for this patch (haven't checked that
   in this patch).
  
   Actually, how about we add that to the page at
   http://wiki.postgresql.org/wiki/Submitting_a_Patch?
  
   If we can define spurious whitespace it would help decide whether
   there is any action to take, and when.
 
  git defines it as either (1) extra whitespace at the end of a line or
  (2) an initial indent that uses spaces followed by tabs (typically
  something like space-tab, where tab alone would have produced the same
  result).  git diff --check master tends to be useful here.
 
  (2) is a problem that has been discussed before on hackers, anything
  like that should be changed.
 
  Why is (1) important, and if it is important, why is it being mentioned
  only now? Are we saying that all previous reviewers of my work (and
  others') removed these without ever mentioning they had done so?
 
 pgident will remove such white spaces and create merge conflicts for
 everyone working on those areas of the code.  I certainly mention this
 in any review I do where it's applicable, and have been doing so for
 some time.  I also will certainly fix it for any code I commit.  I
 also mentioned it in the review that I did of Hot Standby.

I don't recall you mentioning that.

There are no changes in this patch that are purely whitespace changes.
Those were removed prior to patch submission. This is all about code I
am adding or changing. My additions may disrupt their patches, but not
because of the whitespace.

If we are going to run pgindent anyway, what is the point? Seems like we
need a tool to fix patches, not a tool to fix the code.

I've made some changes to the larger files.

-- 
 Simon Riggs   www.2ndQuadrant.com


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


Re: [HACKERS] [patch] executor and slru dtrace probes

2009-12-14 Thread Robert Haas
On Mon, Dec 14, 2009 at 7:19 AM, Bernd Helmle maili...@oopsware.de wrote:
 --On 10. Dezember 2009 16:49:50 +0100 Zdenek Kotala zdenek.kot...@sun.com
 wrote:

 You need to determine which SLRU is used. Because SLRUs are initialized
 during startup  pointer should be same in all backends. If I think more
 about it. Maybe it could be goot to add probe also into SimpleLruInit to
 catch name of SLRUs.

 Hi Zdenek,

 do you plan to work on this further? I was about to mark the SLRU probes as
 ready for committer...

Since the author has pretty much admitted he didn't fix any of the
issues that were raised by the last committer review, I'm a little
confused about why you're asking for another one.

...Robert

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


Re: [HACKERS] [PATCH] ACE Framework - Database, Schema

2009-12-14 Thread KaiGai Kohei

(2009/12/14 21:34), Robert Haas wrote:

On Mon, Dec 14, 2009 at 7:30 AM, KaiGai Koheikai...@kaigai.gr.jp  wrote:

(2009/12/14 20:48), Robert Haas wrote:


2009/12/14 KaiGai Koheikai...@ak.jp.nec.com:


Robert Haas wrote:


2009/12/13 KaiGai Koheikai...@ak.jp.nec.com:


Just to name a few really obvious problems (I only looked at the
01-database patch):

1. We have been talking for several days about the need to make the
initial patch in this area strictly a code cleanup patch.  Is this
cleaner than the code that it is replacing?  Is it even making an
attempt to conform to that mandate?


Even if it is unclear whether the current form is more clear than the
current inlined pg_xxx_aclcheck() form, or not, it will obviously
provide a set of common entry points for upcoming enhanced security
providers.
Eventually, it is more clear than enumeration of #ifdef ... #endif
blocks for SELinux, Smack, Solaris-TX and others.


Right, but it will also not get committed.  :-(


The framework will be necessary to get them committed.
Which is an egg, and which is a chicken? :-(


We've been around that path a few times, but that's not my point here.
  Doing the framework first makes a lot of sense; the problem is that
we just had a design discussion regarding that framework and you've
chosen to do something other than what was discussed.  Did you not
read that discussion?  Did you not understand it?


Please point out, if my understanding is incorrect from the discussion
in a few days.

* As a draft of the discussion, I have to split out the access control
  reworks patch in the 2nd CF per object classes.
* This framework supports only the default PG privileges at the moment.
* The way to host enhanced security providers are not decided.
  (Maybe #ifdef ... #endif block, Maybe function pointer)
* It is not decided how many security labels are assigned on a database
  object. (Maybe 1:1, Maybe 1:n)

I don't intend to go to something undecided, but, might understand
something incorrectly or not be able to follow the discussion enough.


Hmm...  all of those things are true, but it seems to leave quite a bit out.


Since I had to look many messages in a day, my concentration for each
messages might not be enough. I'll try to check it again.
At least, I don't intend to ignore our discussion.

--
KaiGai Kohei kai...@kaigai.gr.jp

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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Simon Riggs
On Mon, 2009-12-14 at 07:33 -0500, Robert Haas wrote:
 On Mon, Dec 14, 2009 at 7:22 AM, Simon Riggs si...@2ndquadrant.com wrote:
  I am now unable to push these changes to the shared repo. What is
  happening?
 
 Perhaps you need to run cvs update on your local copy?
 
 (I find this flavor the most useful: cvs -q update -d  YMMV.)
 
 If that's not it, error message?

It was a question for Heikki only, not a CVS issue.

git push ssh://g...@git.postgresql.org/users/heikki/postgres.git
hs-simon:hs-riggs
To ssh://g...@git.postgresql.org/users/heikki/postgres.git
 ! [rejected]hs-simon - hs-riggs (non-fast forward)
error: failed to push some refs to
'ssh://g...@git.postgresql.org/users/heikki/postgres.git'

-- 
 Simon Riggs   www.2ndQuadrant.com


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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Magnus Hagander
On Mon, Dec 14, 2009 at 13:47, Simon Riggs si...@2ndquadrant.com wrote:
 On Mon, 2009-12-14 at 07:33 -0500, Robert Haas wrote:
 On Mon, Dec 14, 2009 at 7:22 AM, Simon Riggs si...@2ndquadrant.com wrote:
  I am now unable to push these changes to the shared repo. What is
  happening?

 Perhaps you need to run cvs update on your local copy?

 (I find this flavor the most useful: cvs -q update -d  YMMV.)

 If that's not it, error message?

 It was a question for Heikki only, not a CVS issue.

 git push ssh://g...@git.postgresql.org/users/heikki/postgres.git
 hs-simon:hs-riggs
 To ssh://g...@git.postgresql.org/users/heikki/postgres.git
  ! [rejected]        hs-simon - hs-riggs (non-fast forward)
 error: failed to push some refs to
 'ssh://g...@git.postgresql.org/users/heikki/postgres.git'

Same issue can be it in git - did you do a git pull before? You may
need merging with what's on there, and for that to work you must pull
before you can push.

-- 
 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] Range types

2009-12-14 Thread Dimitri Fontaine
Scott Bailey arta...@comcast.net writes:

 So basically I have an anyrange pseudo type with the functions prev, next,
 last, etc defined. So instead of hard coding range types, we would allow the
 user to define their own range types. Basically if we are able to determine
 the previous and next values of the base types we'd be able to define a
 range type. I'm envisioning in a manner much like defining an enum
 type.

It's not clear how to define those functions for the prefix_range
datatype, where '123' represents any text begining with those chars and
'123[4-6]' any text begining with '123' then either 4, 5 or 6.

What's supposed to return SELECT next('123'::prefix_range); ?

Regards,
-- 
dim

PS: as I'm used to use that in the telephony context, the example
contain figures, but it's a text based type and given questions and
reports in pgsql-general, people do use it with text ranges too.

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


Re: [HACKERS] Range types

2009-12-14 Thread Martijn van Oosterhout
On Sun, Dec 13, 2009 at 11:49:53PM -0800, Scott Bailey wrote:
 So basically I have an anyrange pseudo type with the functions prev,  
 next, last, etc defined. So instead of hard coding range types, we would  
 allow the user to define their own range types. Basically if we are able  
 to determine the previous and next values of the base types we'd be able  
 to define a range type. I'm envisioning in a manner much like defining  
 an enum type.

I find it odd that you could define functions next() and prev() since
that assumes some kind of dicretisation which simply does not exist for
most types I can think of.

It would seem to me the real useful uses of ranges would be the
operations overlaps, disjoint, proceeds, follows, etc, which could all
be defined on any well-ordered type (wherever greater-than and
less-than are well defined). No need to discretise anything.

Do you have any actual usecase for a distretized range type for
timestamp?

Have a nice day,
-- 
Martijn van Oosterhout   klep...@svana.org   http://svana.org/kleptog/
 Please line up in a tree and maintain the heap invariant while 
 boarding. Thank you for flying nlogn airlines.


signature.asc
Description: Digital signature


Re: [HACKERS] Streaming replication and non-blocking I/O

2009-12-14 Thread Fujii Masao
On Sat, Dec 12, 2009 at 5:09 PM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 Could we change the API of PQgetXLogData to be more like PQgetCopyData?
 I'm thinking of removing the timeout argument, and instead looping with
 select/poll and PQconsumeInput in the caller. That probably means
 introducing a new state analogous to PGASYNC_COPY_IN. I haven't thought
 this fully through yet, but it seems like it would be good to have a
 consistent API.

On a related issue, so far I haven't considered about the way to output
the notice message at all :( In the current SR, it's always written to
stderr by the defaultNoticeProcessor by using fprintf, whether the
log_destination is specified or not. This is bizarre, and would need to
be fixed.

I'm going to set the new function calling ereport as the current notice
processor by using PQsetNoticeProcessor. But the problem is that only the
completed message like NOTICE: xxx is passed to such notice processor,
i.e., the error level itself is not passed.

So I wonder which error level should be used to output the notice message.
There are some approaches to address this;

1. Always use a specific level without regard to the actual one
2. Reverse-engineer the level from the complete message
3. Change some libpq functions so as to pass the error level to the notice
   processor

But nothing really stands out. Do you have another good idea?

Regards,

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

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


Re: [HACKERS] Adding support for SE-Linux security

2009-12-14 Thread Bruce Momjian
Stephen Frost wrote:
 * Bruce Momjian (br...@momjian.us) wrote:
  I am not replying to many of these emails so I don't appear to be
  brow-beating (forcing) the community into accepting this features.  I
  might be brow-beating the community, but I don't want to _appear_ to be
  brow-beating.  ;-)
 
 My apologies if I come across this way- I don't intend to...  But I'm

You are fine.  I was just saying that at a time I was one of the few
loud voices on this, and if this is going to happen, it will be because
we have a team that wants to do this, not because I am being loud.  I
see the team forming nicely.

 also very enthusiastic about this.  Also, it's become a much more
 personal issue for me due to this:
 
 http://csrc.nist.gov/news_events/documents/omb/draft-omb-fy2010-security-metrics.pdf
 
 OMB is now looking to include label-based security in their metrics.
 This directly impacts some of the PG-based systems I run.

Ah, very interesting, and good.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [HACKERS] Row-Level Security

2009-12-14 Thread Stephen Frost
KaiGai,

* KaiGai Kohei (kai...@kaigai.gr.jp) wrote:
 The reason why I put on the security hook in ExecScan() is to avoid the
 problem that row-cost user defined function can be evaluated earlier
 than row-level security policy. (I believed it was a well-known problem
 at that time yet.) So, I didn't want to append it before optimization.

This is a problem which needs to be addressed and fixed independently.

 I also believe this matter should be resolved when we provide row-level
 security stuff, because it is a security feature.

This issue should be fixed first, not as part of some large-scale patch.

If you have thoughts or ideas about how to address this problem as it
relates to views, I think you would find alot of people willing to
listen and to discuss it.  This must be independent of SELinux,
independent of row-level security, and isn't something based on any of
the patches which have been submitted so far.  None of them that I've
seen resolve this problem in a way that the community is willing to
accept.

Thanks,

Stephen


signature.asc
Description: Digital signature


[HACKERS] pgAdmin III: timestamp displayed in what time zone?

2009-12-14 Thread Fred Janon
Sorry if if's a double post, but I thought that it would be more likely I
would get an answer on the hackers list.

Thanks

Fred

-- --
From: Fred Janon fja...@gmail.com
Date: Mon, Dec 14, 2009 at 19:04
Subject: pgAdmin III: timestamp displayed in what time zone?
To: pgsql-gene...@postgresql.org


Hi,

I am using Postgres 8.3. I have a table defined like this:

===
-- Table: timeson

-- DROP TABLE timeson;

CREATE TABLE timeson
(
  id bigint NOT NULL,
  enddatetime timestamp without time zone NOT NULL,
  startdatetime timestamp without time zone NOT NULL,
  times_id bigint,
  CONSTRAINT timeson_pkey PRIMARY KEY (id),
  CONSTRAINT fkb1af5ba5890cf3da FOREIGN KEY (times_id)
  REFERENCES times (id) MATCH SIMPLE
  ON UPDATE NO ACTION ON DELETE NO ACTION
)
WITH (OIDS=FALSE);
ALTER TABLE timeson OWNER TO myfreo;

==
I populate the table with some data and use pgAdmin III 1.8.4 to view the
date View date first top100 rows. the question is: in what timezone are
the fields showed in pgAdmin? no timezone (as stored), the server time zone
or the time zone of the computer where pgAdmin runs?

Thanks

Fred


Re: [HACKERS] Crash on pg_ctl initdb without options

2009-12-14 Thread Peter Eisentraut
On mån, 2009-12-14 at 17:25 +0900, Takahiro Itagaki wrote:
 pg_ctl initdb crashes if no options are passed.
 Do we need additional if-null test for pgdata_opt just like post_opts?
 
 $ pg_ctl initdb
 sh: -c: line 0: syntax error near unexpected token `null'
 sh: -c: line 0: `/usr/local/pgsql/bin/initdb (null)'
 pg_ctl: database system initialization failed

Oops.  Please fix it. :)



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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Robert Haas
On Mon, Dec 14, 2009 at 7:42 AM, Simon Riggs si...@2ndquadrant.com wrote:
 There are no changes in this patch that are purely whitespace changes.
 Those were removed prior to patch submission. This is all about code I
 am adding or changing. My additions may disrupt their patches, but not
 because of the whitespace.

 If we are going to run pgindent anyway, what is the point? Seems like we
 need a tool to fix patches, not a tool to fix the code.

 I've made some changes to the larger files.

I'll try to explain this again because it is a little bit subtle.

Whenever someone commits ANY patch, there is a danger of disrupting
other outstanding patches that touch the same code.  That's basically
unavoidable, although certainly it's a reason to avoid superfluous
changes like whitespace adjustments or useless identifier renaming.

However, there's a second, completely independent issue, which is that
eventually we will run pgindent for 8.5, and when we do that, it's
going to reformat stuff, and that reformatting is going to break
things.  The amount of reformatting that it does (and therefore the
amount of breakage that it causes) is directly related to the extent
to which people have committed patches throughout the release cycle
that don't conform to the layout that pgident is going to want.  If
every patch perfectly matched the pgident style, then the pgindent run
would change nothing and we would all be VERY happy.  The more
deviations there are, the more stuff breaks.

So I agree with you: we need a tool to fix patches.  However, since we
haven't got one, we owe it to other contributors not to make the
problem any worse than necessary by adhering to the project's
formatting conventions as best we're able when committing things,
especially with regards to trivial things like trailing white-space
that git diff --check can easily find.  Actually, git apply has an
option to fix these simple types of problems, so it's possible to fix
it diffing the patch set against the master branch, applying it to a
separate copy of the master branch using git apply --whitespace=fix,
then diffing that against the original batch and applying the changes.
 Although that's usually overkill unless it's really bad.

There's some interesting discussion on whitespace more generally from
Tom Lane here:

http://archives.postgresql.org/pgsql-hackers/2009-08/msg01001.php

...Robert

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


Re: [HACKERS] Streaming replication and non-blocking I/O

2009-12-14 Thread Tom Lane
Fujii Masao masao.fu...@gmail.com writes:
 On Mon, Dec 14, 2009 at 11:38 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Do we need a new PQgetXLogData function at all?  Seems like you could
 shove the data through the COPY protocol and not have to touch libpq
 at all, rather than duplicating a nontrivial amount of code there.

 Yeah, I also think that all data (the WAL data itself, its LSN and
 the flag bits) which the PQgetXLogData handles could be shoved
 through the COPY protocol. But, outside libpq, it's somewhat messy
 to extract the LSN and the flag bits from the data buffer which
 PQgetCopyData returns, by using ntohs(). So I provided the new
 libpq function only for replication. That is, I didn't want to expose
 the low layer of network which libpq should handle.

I find that a completely unconvincing division of labor.  Who is to say
that the LSN is the only part of the data that needs special treatment?

The very, very large practical problem with this is that if you decide
to change the behavior at any time, the only way to be sure that the WAL
receiver is using the right libpq version is to perform a soname major
version bump.  The transformations done by libpq will essentially become
part of its ABI, and not a very visible part at that.

I am going to insist that no such logic be placed in libpq.  From a
packager's standpoint that's insanity.

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] EXPLAIN BUFFERS

2009-12-14 Thread Tom Lane
Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp writes:
 Tom Lane t...@sss.pgh.pa.us wrote:
 Pushing extra arguments around would create overhead of its own ...
 overhead that would be paid even when not using EXPLAIN at all.

 I cannot understand what you mean... The additional argument should
 not be a performance overhead because the code path is run only once
 per execution.

Hmm, maybe, but still: once you have two flags you're likely to need
more.  I concur with turning doInstrument into a bitmask as per Robert's
suggestion downthread.

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] thread safety on clients

2009-12-14 Thread Tom Lane
Greg Stark gsst...@mit.edu writes:
 On Fri, Dec 11, 2009 at 6:08 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 I'll commit a fix for that shortly, but this reminds me once again that
 the EvalPlanQual logic is desperately under-tested in our normal
 regression testing.  We really need some kind of testing infrastructure
 that would let us exercise concurrent-update cases a bit more than not
 at all.

 If i went back and cleaned up the parallel psql patch we would be able
 to write tests which tested basic concurrent functionality such as
 transactions blocking when they're supposed to. But that wouldn't
 catch something like this I don't think, not unless it got triggered
 pretty reliably by a simple evalplanqual recheck.

It would have been caught if anyone had tried an EvalPlanQual'd update on
a table with one of the unchanged columns being a non-null pass-by-ref
value.  Now it's certainly possible that a set of regression tests would
by chance fail to exercise that case --- but IMO the real problem right
now is we aren't even trying to exercise EvalPlanQual; we're actively
avoiding it because the current test infrastructure can't ensure
deterministic results if any such situation arises.

But my recollection of the parallel psql patch discussion is that it was
rejected because nobody felt comfortable with the API design.  Do we
have any better ideas in that department yet?

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] WAL Info messages

2009-12-14 Thread Tom Lane
Greg Stark gsst...@mit.edu writes:
 What happens on the slave when normal NOTIFYs are generated on the
 master? IIRC NOTIFYs are wal-logged so I imagine LISTEN on the slave
 would just work and a plain old NOTIFY/LISTEN would suffice for this
 use case?

Nothing happens: NOTIFYs are *not* WAL-logged, and it would be quite
against the intended use-case to have them be.

If you're hoping that processes on the slave could LISTEN for events
happening on the master, we'd have a bit of a conflict.  This might be
an example of a case where we need that are we doing HS flag that
Simon insists we don't need.

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] thread safety on clients

2009-12-14 Thread Alvaro Herrera
Tom Lane wrote:

 But my recollection of the parallel psql patch discussion is that it was
 rejected because nobody felt comfortable with the API design.  Do we
 have any better ideas in that department yet?

It wasn't rejected AFAICT.  A finalized API with which there was
(almost?) no dissent was posted by you, after a design/path from Greg
Stark.  The problem is that nobody stepped up to implementing that spec.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

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


Re: [HACKERS] Range types

2009-12-14 Thread Tom Lane
Scott Bailey arta...@comcast.net writes:
 So basically I have an anyrange pseudo type with the functions prev, 
 next, last, etc defined. So instead of hard coding range types, we would 
 allow the user to define their own range types. Basically if we are able 
 to determine the previous and next values of the base types we'd be able 
 to define a range type. I'm envisioning in a manner much like defining 
 an enum type.

I think array types, not enums, would be a better model.

The real question is how the heck granularity enters into it.  Why
should a range type require that?  I think you are mixing up two
concepts that would be better kept separate.

In particular, the granularity examples you give seem to assume that
the underlying datatype is exact not approximate --- which among other
things will mean that it fails to work for float timestamps.  Since
timestamps are supposedly the main use-case, that's pretty troubling.

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] Range types

2009-12-14 Thread Tom Lane
Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp writes:
 As another approach, what about storing typeid in typmod?
 (Oid can be assumed to be stored in int32.)

No, it cannot --- you'd be one bit short.  The general rule for typmod
is that all negative values are treated as unspecified.  Even if you
managed to find and change every place that assumed that, you'd still
have a failure for -1, which is a perfectly legal value of Oid.

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] Row-Level Security

2009-12-14 Thread Tom Lane
KaiGai Kohei kai...@ak.jp.nec.com writes:
 One more issue I found.
 What row-level policy should be applied on inherited tables?

Yup, that seems like an interesting problem.

 Even if the inherited table has multiple parents, all the row-level
 policies shall be applied, so here is no inconsistency.
 (Needless to say, child table have same columns, so we can apply
 same row-level policies.)

I don't think I believe either of those statements.

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] WAL Info messages

2009-12-14 Thread Simon Riggs
On Mon, 2009-12-14 at 09:51 -0500, Tom Lane wrote:
 This might be
 an example of a case where we need that are we doing HS flag that
 Simon insists we don't need.

Simon has put it there at your request, in the place you suggested.

Others did speak against it, not me.

-- 
 Simon Riggs   www.2ndQuadrant.com


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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Simon Riggs
On Mon, 2009-12-14 at 09:32 -0500, Robert Haas wrote:

 If every patch perfectly matched the pgident style, then the pgindent
 run would change nothing and we would all be VERY happy.

I've made all whitespace changes to the patch.

I understand the reason for acting as you suggest, but we either police
it, or we don't. If we don't police in all cases then I'm not personally
happy to be policed.

About 90% of the whitespace in the patch were in docs, README or test
output files. A great many of that type of file have numerous line
ending whitespace, not introduced by me, so it seems to me that this has
never been adequately policed in the way you say. If we really do care
about this issue then I expect people to look a little further than just
my patch.

Portability of patches across releases isn't a huge issue for me, nor
for the project, I think, but I am willing to commit to cleaning future
patches in this way.

-- 
 Simon Riggs   www.2ndQuadrant.com


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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Kevin Grittner
Simon Riggs si...@2ndquadrant.com wrote:
 
 If we are going to run pgindent anyway, what is the point?
 
Perhaps it would take less time to run this than to argue the point?:
 
sed -e 's/[ \t][ \t]*$//' -e 's/  *\t/\t/g' *
 
-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] Hot Standby, release candidate?

2009-12-14 Thread Simon Riggs
On Mon, 2009-12-14 at 13:56 +0100, Magnus Hagander wrote:

 Same issue can be it in git - did you do a git pull before? You may
 need merging with what's on there, and for that to work you must pull
 before you can push.

Found some merge conflicts and resolved them. I did fetch and merge at
different times, so that seems to be the source.

I've resolved the other git issues, so latest version on git now.

-- 
 Simon Riggs   www.2ndQuadrant.com


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


Re: [HACKERS] Range types

2009-12-14 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Mon, Dec 14, 2009 at 4:06 AM,  to...@tuxteam.de wrote:
 As another approach, what about storing typeid in typmod?
 (Oid can be assumed to be stored in int32.)

 It 's very different than the way we've traditionally used typmod,

Aside from the problems already pointed out, there's another: this
definition usurps the ability to attach a typmod to the range's
base type.  Again, you should be looking at arrays not enums for
a reference example.  The typmod of an array feeds through to its
element type.

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] WAL Info messages

2009-12-14 Thread Simon Riggs
On Mon, 2009-12-14 at 09:51 -0500, Tom Lane wrote:
 Greg Stark gsst...@mit.edu writes:
  What happens on the slave when normal NOTIFYs are generated on the
  master? IIRC NOTIFYs are wal-logged so I imagine LISTEN on the slave
  would just work and a plain old NOTIFY/LISTEN would suffice for this
  use case?
 
 Nothing happens: NOTIFYs are *not* WAL-logged, and it would be quite
 against the intended use-case to have them be.
 
 If you're hoping that processes on the slave could LISTEN for events
 happening on the master, we'd have a bit of a conflict.

What I've proposed is essentially a variant of NOTIFY on master, LISTEN
on slave, as Greg suggests. Almost identical if we have
NOTIFY-with-payload.

I definitely wouldn't presume that anybody using Hot Standby would
necessarily want NOTIFY to reach the standby, especially if there was an
overhead to doing so. If using NOTIFY is the favoured approach, I would
add a separate parameter for it and/or an explicit option on NOTIFY.

-- 
 Simon Riggs   www.2ndQuadrant.com


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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Mon, Dec 14, 2009 at 6:35 AM, Simon Riggs si...@2ndquadrant.com wrote:
 Why is (1) important, and if it is important, why is it being mentioned
 only now? Are we saying that all previous reviewers of my work (and
 others') removed these without ever mentioning they had done so?

 pgident will remove such white spaces and create merge conflicts for
 everyone working on those areas of the code.

What I try really hard to remove from committed patches is spurious
whitespace changes to pre-existing code.  Whether new code blocks
exactly match pgindent's rules is less of a concern, but changing
code you don't have to in a way that pgindent will undo later anyway
is just useless creation of potential conflicts.

The whole thing would be a lot easier if someone would put together an
easily-installable version of pgindent.  Bruce has posted the patches he
uses but I don't know what version of indent they're against...

regards, tom lane

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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Robert Haas
On Mon, Dec 14, 2009 at 10:08 AM, Simon Riggs si...@2ndquadrant.com wrote:
 On Mon, 2009-12-14 at 09:32 -0500, Robert Haas wrote:
 If every patch perfectly matched the pgident style, then the pgindent
 run would change nothing and we would all be VERY happy.

 I've made all whitespace changes to the patch.

 I understand the reason for acting as you suggest, but we either police
 it, or we don't. If we don't police in all cases then I'm not personally
 happy to be policed.

I am doing my best to police it, and certainly will police it for
anything that I review or commit.  Heikki was the one who originally
pointed it on this thread, Magnus gave a +1, and I am pretty sure Tom
tries to keep an eye out for it, too, so generally I would say it is
project practice.  Obviously I am not able to control the actions of
all the other committers, and it does appear that some crap has crept
in since the last pgindent run.  :-(

At any rate I don't think you're being singled out for special treatment.

 About 90% of the whitespace in the patch were in docs, README or test
 output files. A great many of that type of file have numerous line
 ending whitespace, not introduced by me, so it seems to me that this has
 never been adequately policed in the way you say. If we really do care
 about this issue then I expect people to look a little further than just
 my patch.

pgindent won't reindent the docs or the README, but git diff --check
picks up on trailing whitespace, so it may be that Tom (who doesn't
use git but does commit a lot of patches) is less finicky about
trailing whitespace in those places.  If we move to git across the
board, I expect this to get more standardized handling, but I think we
have a ways to go on that yet.

 Portability of patches across releases isn't a huge issue for me, nor
 for the project, I think, but I am willing to commit to cleaning future
 patches in this way.

It's a pretty significant issue for me personally, and anyone who is
maintaining a fork of the main source base that has to be merged when
the pgindent run hits.  It would be nice if someone wanted to build a
better mousetrap here, but so far no volunteers.

...Robert

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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Alvaro Herrera
Tom Lane escribió:

 The whole thing would be a lot easier if someone would put together an
 easily-installable version of pgindent.  Bruce has posted the patches he
 uses but I don't know what version of indent they're against...

And we're still unclear on the typedef list that's going to be used.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Heikki Linnakangas
Simon Riggs wrote:
 On Mon, 2009-12-14 at 11:09 +0100, Magnus Hagander wrote:
 On Mon, Dec 14, 2009 at 10:54, Heikki Linnakangas
 heikki.linnakan...@enterprisedb.com wrote:
 * Please remove any spurious whitespace.  git diff --color makes them
 stand out like a sore thumb, in red. (pgindent will fix them but always
 better to fix them before committing, IMO).
 +1 in general, not particularly for this patch (haven't checked that
 in this patch).

 Actually, how about we add that to the page at
 http://wiki.postgresql.org/wiki/Submitting_a_Patch?
 
 If we can define spurious whitespace it would help decide whether
 there is any action to take, and when.

There's two definitions that are useful:

- Anything that git diff --color shows up as glaring red. Important to
remove simply because the red whitespace is distracting when I review a
patch.

- Anything that pgindent would/will eventually fix. Because you might as
well fix them before committing and save the extra churn and potential
(although trivial) merge conflicts in people's outstanding patches when
pgindent is run. I don't run pgindent myself, so I wouldn't notice most
stuff, but I tend to fix things that I do notice.

 Why is (1) important, and if it is important, why is it being mentioned
 only now? Are we saying that all previous reviewers of my work (and
 others') removed these without ever mentioning they had done so?

I did it in one pass, Oct 15th according to git log.

I tend to silently fix whitespace issues like that in all patches I
commit. It's generally trivial enough to fix that it's easier to just
fix it myself than complain, explain, and look like a real nitpick.

I note that if it was easy to run pgindent yourself on a patch before
committing/submitting, we wouldn't need to have this discussion. I don't
know hard it is to get it working right, but I recall I tried once and
gave up. Any volunteers?

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

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


Re: [HACKERS] new CommitFest states

2009-12-14 Thread Kevin Grittner
Greg Smith g...@2ndquadrant.com wrote:
 
 I didn't really get any feedback on my proposal to add a new
 Discussing review state
 
It seems like a good idea to me; it better models the reality.
 
 http://wiki.postgresql.org/wiki/Running_a_CommitFest
 
It seems to me that a patch could move from Discussing review to
Needs review -- if the reviewer decided to discuss the approach
before continuing the review process and the discussion confirms the
approach as viable.
 
-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] Hot Standby, release candidate?

2009-12-14 Thread Simon Riggs
On Mon, 2009-12-14 at 09:14 -0600, Kevin Grittner wrote:
 Simon Riggs si...@2ndquadrant.com wrote:
  
  If we are going to run pgindent anyway, what is the point?
  
 Perhaps it would take less time to run this than to argue the point?:
  
 sed -e 's/[ \t][ \t]*$//' -e 's/  *\t/\t/g' *

Not certain that is exactly correct, plus it doesn't only work against a
current patch since there are already many examples of whitespace in CVS
already. I do appreciate your attempts at resolution and an easy
tool-based approach for the future, though I'll let someone else run
with it.

-- 
 Simon Riggs   www.2ndQuadrant.com


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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread David Fetter
On Mon, Dec 14, 2009 at 11:09:49AM +0100, Magnus Hagander wrote:
 On Mon, Dec 14, 2009 at 10:54, Heikki Linnakangas
 heikki.linnakan...@enterprisedb.com wrote:
  * Please remove any spurious whitespace.  git diff --color makes
  them stand out like a sore thumb, in red. (pgindent will fix them
  but always better to fix them before committing, IMO).
 
 +1 in general, not particularly for this patch (haven't checked that
 in this patch).
 
 Actually, how about we add that to the page at
 http://wiki.postgresql.org/wiki/Submitting_a_Patch?

Done.

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

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

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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Robert Haas
On Mon, Dec 14, 2009 at 10:49 AM, Simon Riggs si...@2ndquadrant.com wrote:
 On Mon, 2009-12-14 at 09:14 -0600, Kevin Grittner wrote:
 Simon Riggs si...@2ndquadrant.com wrote:

  If we are going to run pgindent anyway, what is the point?

 Perhaps it would take less time to run this than to argue the point?:

 sed -e 's/[ \t][ \t]*$//' -e 's/  *\t/\t/g' *

 Not certain that is exactly correct, plus it doesn't only work against a
 current patch since there are already many examples of whitespace in CVS
 already. I do appreciate your attempts at resolution and an easy
 tool-based approach for the future, though I'll let someone else run
 with it.

Yeah, that would actually be a disaster, because it would actually add
deltas to the patch in the short term.

Seems to me that we would be better off figuring out which exact ident
Bruce is running and checking the typedef list into CVS.

...Robert

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


Re: [HACKERS] Streaming replication and non-blocking I/O

2009-12-14 Thread Tom Lane
Fujii Masao masao.fu...@gmail.com writes:
 I'm going to set the new function calling ereport as the current notice
 processor by using PQsetNoticeProcessor. But the problem is that only the
 completed message like NOTICE: xxx is passed to such notice processor,
 i.e., the error level itself is not passed.

Use PQsetNoticeReceiver.  The other one is just there for backwards
compatibility.

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] WAL Info messages

2009-12-14 Thread Tom Lane
Simon Riggs si...@2ndquadrant.com writes:
 I definitely wouldn't presume that anybody using Hot Standby would
 necessarily want NOTIFY to reach the standby, especially if there was an
 overhead to doing so. If using NOTIFY is the favoured approach, I would
 add a separate parameter for it and/or an explicit option on NOTIFY.

Yeah, I had just come to the same conclusion: you'd want a separate
spigot handle on sending NOTIFY to WAL, regardless of any general flag
about HS.

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] Range types

2009-12-14 Thread Scott Bailey

Martijn van Oosterhout wrote:

On Sun, Dec 13, 2009 at 11:49:53PM -0800, Scott Bailey wrote:
So basically I have an anyrange pseudo type with the functions prev,  
next, last, etc defined. So instead of hard coding range types, we would  
allow the user to define their own range types. Basically if we are able  
to determine the previous and next values of the base types we'd be able  
to define a range type. I'm envisioning in a manner much like defining  
an enum type.


I find it odd that you could define functions next() and prev() since
that assumes some kind of dicretisation which simply does not exist for
most types I can think of.


Because intervals (mathematical not SQL) can be open or closed at each 
end point we need to know what the next an previous value would be at 
the specified granularity. And while you can do some operations without 
knowing this, there are many you can't. For instance you could not tell 
whether two [] or () ranges were adjacent, or be able to coalesce an 
array of ranges.



It would seem to me the real useful uses of ranges would be the
operations overlaps, disjoint, proceeds, follows, etc, which could all
be defined on any well-ordered type (wherever greater-than and
less-than are well defined). No need to discretise anything.

Do you have any actual usecase for a distretized range type for
timestamp?




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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Bruce Momjian
Tom Lane wrote:
 Robert Haas robertmh...@gmail.com writes:
  On Mon, Dec 14, 2009 at 6:35 AM, Simon Riggs si...@2ndquadrant.com wrote:
  Why is (1) important, and if it is important, why is it being mentioned
  only now? Are we saying that all previous reviewers of my work (and
  others') removed these without ever mentioning they had done so?
 
  pgident will remove such white spaces and create merge conflicts for
  everyone working on those areas of the code.
 
 What I try really hard to remove from committed patches is spurious
 whitespace changes to pre-existing code.  Whether new code blocks
 exactly match pgindent's rules is less of a concern, but changing
 code you don't have to in a way that pgindent will undo later anyway
 is just useless creation of potential conflicts.
 
 The whole thing would be a lot easier if someone would put together an
 easily-installable version of pgindent.  Bruce has posted the patches he
 uses but I don't know what version of indent they're against...

The entire indent tarball with patches is on our ftp site.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [HACKERS] Range types

2009-12-14 Thread Tom Lane
Scott Bailey arta...@comcast.net writes:
 Because intervals (mathematical not SQL) can be open or closed at each 
 end point we need to know what the next an previous value would be at 
 the specified granularity. And while you can do some operations without 
 knowing this, there are many you can't. For instance you could not tell 
 whether two [] or () ranges were adjacent, or be able to coalesce an 
 array of ranges.

This statement seems to me to demonstrate that you don't actually
understand the concept of open and closed ranges.  It has nothing
whatsoever to do with assuming that the data type is discrete;
these concepts are perfectly well defined for the reals, for example.
What it is about is whether the inclusion conditions are  bound
or = bound.

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] pgAdmin III: timestamp displayed in what time zone?

2009-12-14 Thread Greg Smith

Fred Janon wrote:
Sorry if if's a double post, but I thought that it would be more 
likely I would get an answer on the hackers list.
In that case just posting here would have been better than hitting 
both.  I usually ignore any request for help that is posted on more than 
one list just to draw attention to itself; I wouldn't be surprised that 
are other people who are similarly annoyed by the behavior and do 
likewise. 

In your case, both lists you picked were the wrong ones, since you're 
asking a pgAdmin specific question which isn't the direct topic of any 
of the pgsql-* lists; pgadmin-support was the right place for it:  
http://archives.postgresql.org/pgadmin-support/


--
Greg Smith2ndQuadrant   Baltimore, MD
PostgreSQL Training, Services and Support
g...@2ndquadrant.com  www.2ndQuadrant.com


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


Re: [HACKERS] new CommitFest states

2009-12-14 Thread Greg Smith

Kevin Grittner wrote:

http://wiki.postgresql.org/wiki/Running_a_CommitFest

 
It seems to me that a patch could move from Discussing review to

Needs review -- if the reviewer decided to discuss the approach
before continuing the review process and the discussion confirms the
approach as viable.
  
In that case, the patch would be in Needs review the whole time.  
Discussing review is intended to be a I'm done but not sure of the 
next step for this patch state the reviewer can use.  In the situation 
you described, the patch would never have left Needs review.  I just 
made that more clear by documenting that it's shorthand for discussing 
review results.


I also added a transition path for a similar situation though, where the 
discussion concludes the reviewer didn't do the right thing in the first 
place (even though they thought they did) and they return to reviewing 
after realizing what was missing.


--
Greg Smith2ndQuadrant   Baltimore, MD
PostgreSQL Training, Services and Support
g...@2ndquadrant.com  www.2ndQuadrant.com



Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Greg Smith

Heikki Linnakangas wrote:

I note that if it was easy to run pgindent yourself on a patch before
committing/submitting, we wouldn't need to have this discussion. I don't
know hard it is to get it working right, but I recall I tried once and
gave up.
  

What sort of problems did you run into?

--
Greg Smith2ndQuadrant   Baltimore, MD
PostgreSQL Training, Services and Support
g...@2ndquadrant.com  www.2ndQuadrant.com


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


Re: [HACKERS] new CommitFest states

2009-12-14 Thread Robert Haas
On Mon, Dec 14, 2009 at 12:01 PM, Greg Smith g...@2ndquadrant.com wrote:
 Kevin Grittner wrote:

 http://wiki.postgresql.org/wiki/Running_a_CommitFest



 It seems to me that a patch could move from Discussing review to
 Needs review -- if the reviewer decided to discuss the approach
 before continuing the review process and the discussion confirms the
 approach as viable.


 In that case, the patch would be in Needs review the whole time.
 Discussing review is intended to be a I'm done but not sure of the next
 step for this patch state the reviewer can use.  In the situation you
 described, the patch would never have left Needs review.  I just made that
 more clear by documenting that it's shorthand for discussing review
 results.

 I also added a transition path for a similar situation though, where the
 discussion concludes the reviewer didn't do the right thing in the first
 place (even though they thought they did) and they return to reviewing after
 realizing what was missing.

I don't think there should be a transition from Returned with Feedback
back to Waiting for review.  Granted we might allow that occasionally
as an exceptional case, but normally Returned with Feedback is a final
state.

(Also, Waiting for review is actually the wrong name for the state
it's trying to talk about.)

...Robert

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


Re: [HACKERS] new CommitFest states

2009-12-14 Thread Greg Smith

Robert Haas wrote:

I don't think there should be a transition from Returned with Feedback
back to Waiting for review.  Granted we might allow that occasionally
as an exceptional case, but normally Returned with Feedback is a final
state.
  
I did throw some disclaimers in the notes about this particular subject 
at the bottom of the table.  The main reason I put that in there is that 
sometimes a reviewer or even the CF manager (I did this myself once this 
time) will mark something Returned with feedback, thinking there's no 
way the issues pointed out can be addressed right now.  And then, a day 
or two later, in comes a patch that does just that; surprise!  Since it 
seems to happen anyway, and I'd prefer not to get in the position where 
people are screaming you threw me out with 'RWF' unfairly, I thought 
it was better to accept that possibility so long as the whole thing is 
tightly bounded as far as how much time the author has to do it.



(Also, Waiting for review is actually the wrong name for the state
it's trying to talk about.)
  

Uh, what are you talking about here?

--
Greg Smith2ndQuadrant   Baltimore, MD
PostgreSQL Training, Services and Support
g...@2ndquadrant.com  www.2ndQuadrant.com


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


Re: [HACKERS] new CommitFest states

2009-12-14 Thread Robert Haas
On Mon, Dec 14, 2009 at 12:38 PM, Greg Smith g...@2ndquadrant.com wrote:
 Robert Haas wrote:
 I don't think there should be a transition from Returned with Feedback
 back to Waiting for review.  Granted we might allow that occasionally
 as an exceptional case, but normally Returned with Feedback is a final
 state.

 I did throw some disclaimers in the notes about this particular subject at
 the bottom of the table.  The main reason I put that in there is that
 sometimes a reviewer or even the CF manager (I did this myself once this
 time) will mark something Returned with feedback, thinking there's no way
 the issues pointed out can be addressed right now.  And then, a day or two
 later, in comes a patch that does just that; surprise!  Since it seems to
 happen anyway, and I'd prefer not to get in the position where people are
 screaming you threw me out with 'RWF' unfairly, I thought it was better to
 accept that possibility so long as the whole thing is tightly bounded as far
 as how much time the author has to do it.

Hmm, I'm not aware of any actual cases of this.  I'm usually pretty
conservative about jumping to RWF unless there's been lag or we're
near the end of the CommitFest, so it doesn't come up.

 (Also, Waiting for review is actually the wrong name for the state
 it's trying to talk about.)

 Uh, what are you talking about here?

Well, we have Needs Review and Waiting on Author, but not Waiting for
Review.  I assume you mean Needs Review.

...Robert

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


Re: [HACKERS] Range types

2009-12-14 Thread Nathan Boley
 Because intervals (mathematical not SQL) can be open or closed at each
 end point we need to know what the next an previous value would be at
 the specified granularity. And while you can do some operations without
 knowing this, there are many you can't. For instance you could not tell
 whether two [] or () ranges were adjacent, or be able to coalesce an
 array of ranges.

 This statement seems to me to demonstrate that you don't actually
 understand the concept of open and closed ranges.  It has nothing
 whatsoever to do with assuming that the data type is discrete;
 these concepts are perfectly well defined for the reals, for example.
 What it is about is whether the inclusion conditions are  bound
 or = bound.

IMHO the first question is whether, for integers, [1,2] UNION [3,5]
should be equal to [1,5]. In math this is certainly true, and defining
'next' seems like a reasonable way to establish this in postgres.

The next question is whether, for floats, [1,3-FLT_EPSILON] UNION
[3,5] should be [1,5].

And the next question is whether, for numeric(6,2), [1,2.99] UNION
[3,5] should be [1,5].

FWIW, I would answer yes, no, yes to those three questions.

-Nathan

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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Greg Stark
On Mon, Dec 14, 2009 at 11:07 AM, Simon Riggs si...@2ndquadrant.com wrote:
 * vacuum_defer_cleanup_age is very hard to tune. You'll need an estimate
 on your transaction rate to begin with. Do we really want this setting
 in its current form? Does it make sense as PGC_USERSET, as if one
 backend uses a lower setting than others, that's the one that really
 determines when transactions are killed in the standby? I think this
 should be discussed and implemented as a separate patch.

 All the vacuum_*_age parameters have this characteristic, yet they
 exist.

I think it makes sense to have it be userset because someone could run
vacuum on one table with a different defer_cleanup_age for some
reason. I admit I'm having trouble coming up with a good use case.
Personally I'm fine with having parameters like this in alphas that
end up being renamed, or changed to different semantics, or even
removed by the time it's launched.


 It doesn't seem any more wrong than using hash indexes right after
 recovery, crash recovery or otherwise. It's certainly broken, but I
 don't see much value in a partial fix. The bottom line is that hash
 indexes should be WAL-logged.

 I know that's your thought; I'm just checking its everyone else's as
 well. We go to great lengths elsewhere in the patch to avoid queries
 returning incorrect results and there is a loss of capability as a
 result. I don't want Hash index users to view this as a feature. I don't
 feel too strongly, but it can be argued both ways, at least.

This goes back to your pluggable rmgr point. Someone could add a new
index method and get bogus results on their standby. And unlike hash
indexes where there's some hope of addressing the problem there's
nothing they can do to fix this.

It does seem like having a flag in the catalog to mark nonrecoverable
indexes and make them unavailable to query plans on the standby would
be worth its weight in code.

-- 
greg

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


Re: [HACKERS] Aggregate ORDER BY patch

2009-12-14 Thread Tom Lane
Andrew Gierth and...@tao11.riddles.org.uk writes:
 Updated version of the aggregate order by patch.

I'm starting to look at this now.  I find it rather bizarre to merge
both the actual arguments of an aggregate and the optional ORDER BY
expressions into a single targetlist.  It doesn't seem like that would
be an especially convenient representation to work with, and I would
also expect there to be a nonzero performance hit from the extra
TargetEntry expression nodes, even when the feature is not in use.
Why didn't you use separate lists?

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] Range types

2009-12-14 Thread Jeff Davis
On Mon, 2009-12-14 at 11:25 -0500, Tom Lane wrote:
 Scott Bailey arta...@comcast.net writes:
  Because intervals (mathematical not SQL) can be open or closed at each 
  end point we need to know what the next an previous value would be at 
  the specified granularity. And while you can do some operations without 
  knowing this, there are many you can't. For instance you could not tell 
  whether two [] or () ranges were adjacent, or be able to coalesce an 
  array of ranges.
 
 This statement seems to me to demonstrate that you don't actually
 understand the concept of open and closed ranges.  It has nothing
 whatsoever to do with assuming that the data type is discrete;
 these concepts are perfectly well defined for the reals, for example.
 What it is about is whether the inclusion conditions are  bound
 or = bound.

Of course you can still define the obvious contains and overlaps
operators for a continuous range. But there are specific differences in
the API between a discrete range and a continuous range, which is what
Scott was talking about.

1. With discrete ranges, you can change the displayed
inclusivity/exclusivity without changing the value. For instance in the
integer domain, [ 5, 10 ] is the same value as [ 5, 11 ). This works on
both input and output. It is not possible to change the display for
continuous ranges.

2. With discrete ranges, you can get the last point before the range,
the first point in the range, the last point in the range, and the first
point after the range. These are more than enough to describe the range
completely. For continuous ranges, those functions will fail depending
on the inclusivity/exclusivity of the range. Practically speaking, you
would want to have a different set of accessors: start_inclusive(),
start_point(), end_point(), and end_inclusive(). However, those
functions can't be usefully defined on a discrete range.

We can't choose the API for continuous ranges as the API for discrete
ranges as well. If we did, then we would think that [5, 10] and [5, 11)
were not equal, but they are. Similarly, we would think that [5, 10] and
[11, 12] were not adjacent, but they are.

So there are certainly some user-visible API differences between the
two, and I don't think those differences can be hidden.

Regards,
Jeff Davis




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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Heikki Linnakangas
Greg Smith wrote:
 Heikki Linnakangas wrote:
 I note that if it was easy to run pgindent yourself on a patch before
 committing/submitting, we wouldn't need to have this discussion. I don't
 know hard it is to get it working right, but I recall I tried once and
 gave up.
   
 What sort of problems did you run into?

I don't remember, unfortunately.

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

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


Re: [HACKERS] Range types

2009-12-14 Thread Jeff Davis
On Mon, 2009-12-14 at 09:58 -0500, Tom Lane wrote:
 In particular, the granularity examples you give seem to assume that
 the underlying datatype is exact not approximate --- which among other
 things will mean that it fails to work for float timestamps.  Since
 timestamps are supposedly the main use-case, that's pretty troubling.

Additionally, granularity for timestamps is not particularly useful when
you get to things like days and months which don't have a clean
algebra.

Is the granule there only to try to support continuous ranges? If so, I
don't think it's necessary if we expose the API differences I outlined
in another email in this thread. Also, that would mean that we don't
need a granule for float, because we can already treat it as discrete*.

Regards,
Jeff Davis

*: nextafter() allows you to increment or decrement a double (loosely
speaking), and according to the man page it's part of C99 and
POSIX.1-2001.


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


Re: [HACKERS] Range types

2009-12-14 Thread Tom Lane
Nathan Boley npbo...@gmail.com writes:
 This statement seems to me to demonstrate that you don't actually
 understand the concept of open and closed ranges.

 IMHO the first question is whether, for integers, [1,2] UNION [3,5]
 should be equal to [1,5]. In math this is certainly true, and defining
 'next' seems like a reasonable way to establish this in postgres.

Well, that's nice to have (not essential) for data types that actually
are discrete.  It's not a sufficient argument for creating a definition
that is flat out broken for non-discrete datatypes.

It might be worth pointing out here that the concept of an open interval
was only invented in the first place for dealing with a continuum.
If you could assume the underlying set is discrete, every open interval
could be replaced with a closed one, using the next or prior value as
the bound instead.  There would be no need for two kinds of interval.

If you are intending to support UNION on intervals, you are going to
need some more-general representation anyway.  (I trust you don't think
we will accept a datatype for which [1,2] UNION [3,5] works but
[1,2] UNION [4,5] throws an error.)  So whether the code can reduce the
result of a union to a single range or has to keep it as two ranges is
only an internal optimization issue anyhow, not something that should
drive an artificial (and infeasible) attempt to force every domain to be
discrete.

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] Range types

2009-12-14 Thread Jeff Davis
On Mon, 2009-12-14 at 10:00 -0800, Nathan Boley wrote:
 IMHO the first question is whether, for integers, [1,2] UNION [3,5]
 should be equal to [1,5]. In math this is certainly true, and defining
 'next' seems like a reasonable way to establish this in postgres.

[ you say yes ]

Agreed.

 The next question is whether, for floats, [1,3-FLT_EPSILON] UNION
 [3,5] should be [1,5].

[ you say no ]

I think this should be true, because all floats between 1 and 5 are
contained. I don't feel too strongly about this, so I would not complain
if floats were treated as continuous.

 And the next question is whether, for numeric(6,2), [1,2.99] UNION
 [3,5] should be [1,5].

[ you say yes ]

I almost agree. Unfortunately, typmod isn't really a part of the type,
it just affects input/output. So, we can't really use it that way -- as
Tom points out, typmod is not passed along to functions that take the
value.

But if it were a part of the type, then I would certainly agree.

Regards,
Jeff Davis


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


Re: [HACKERS] Range types

2009-12-14 Thread Alvaro Herrera
Jeff Davis wrote:

 So there are certainly some user-visible API differences between the
 two, and I don't think those differences can be hidden.

ISTM you are saying we should have two types of range types.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

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


Re: [HACKERS] Range types

2009-12-14 Thread Tom Lane
Jeff Davis pg...@j-davis.com writes:
 On Mon, 2009-12-14 at 11:25 -0500, Tom Lane wrote:
 This statement seems to me to demonstrate that you don't actually
 understand the concept of open and closed ranges.

 Of course you can still define the obvious contains and overlaps
 operators for a continuous range. But there are specific differences in
 the API between a discrete range and a continuous range, which is what
 Scott was talking about.

Well, if the intention is to invent two different kinds of ranges, with
different operators, for continuous and discrete data types, then fine.
But the original post suggested no such thing, and provided (unworkable)
examples suggesting that the intent was to force every type to be
treated as discrete whether that makes any sense or not.

The main question I would have is how to tell whether the underlying
type is really discrete.  If we allow people to define things like
range over float4 with 0.01 step, then somebody will try to do it
--- and file bugs when it doesn't work sanely.  I don't think there is
anything in our API for user-defined types that really tells you whether
it's an exact or approximate type.

(Also, stuff like strings simply doesn't have any sane concept of a
unique next or previous value.  I think the number of types that are
really discrete in this sense is very small, like maybe just ints and
enums.)

regards, tom lane

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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Heikki Linnakangas
Simon Riggs wrote:
 On Mon, 2009-12-14 at 11:54 +0200, Heikki Linnakangas wrote:
 Simon Riggs wrote:
 * Are you planning to remove the recovery_connections setting before
 release? The documentation makes it sound like it's a temporary hack
 that we're not really sure is needed at all. That's not very comforting.
 
 It has been requested and I agree, so its there. Saying it might be
 removed in future is no more than we do elsewhere and AFAIK we all hope
 it will be. Not sure why that is or isn't comforting.

Now that recovery_connections has a double-role, and does in the master
what the wal_standby_info used to do, the documentation probably should
be clarified that the whole parameter is not going to go away, just the
role in the master.

 * You removed this comment from KnownAssignedXidsInit:

 -   /*
 -* XXX: We should check that we don't exceed maxKnownAssignedXids.
 -* Even though the hash table might hold a few more entries than that,
 -* we use fixed-size arrays of that size elsewhere and expected all
 -* entries in the hash table to fit.
 -*/

 but AFAICS you didn't address the issue. It's referring to the 'xids'
 array in TransactionIdIsInProgress(), which KnownAssignedXidsGet() fills
 in without checking that it fits.
 
 I have ensured that they are always the same size, by definition, so no
 need to check.

How did you ensure that? The hash table has no hard size limit.

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

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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Simon Riggs
On Mon, 2009-12-14 at 18:02 +, Greg Stark wrote:
  It doesn't seem any more wrong than using hash indexes right after
  recovery, crash recovery or otherwise. It's certainly broken, but I
  don't see much value in a partial fix. The bottom line is that hash
  indexes should be WAL-logged.
 
  I know that's your thought; I'm just checking its everyone else's as
  well. We go to great lengths elsewhere in the patch to avoid queries
  returning incorrect results and there is a loss of capability as a
  result. I don't want Hash index users to view this as a feature. I
 don't
  feel too strongly, but it can be argued both ways, at least.
 
 This goes back to your pluggable rmgr point. Someone could add a new
 index method and get bogus results on their standby. And unlike hash
 indexes where there's some hope of addressing the problem there's
 nothing they can do to fix this.
 
 It does seem like having a flag in the catalog to mark nonrecoverable
 indexes and make them unavailable to query plans on the standby would
 be worth its weight in code.

pg_am.amalmostworks or perhaps pg_am.amhalffinished... :-)

I wouldn't wish to literally persist that situation, especially since if
we had it we couldn't update it during recovery. We need to allow
pluggable indexes in full, not just partially. I think we should extend
pg_am so that rmgr routines can be defined for them also, with dynamic
assignment of rmgrids, recorded in file so we can use them during
recovery.

What we also need is a mechanism to identify and mark indexes as corrupt
while they are being rebuilt, so recovery can complete without them and
then rebuild automatically when recovery finishes. And so they can be
skipped during hot standby.

-- 
 Simon Riggs   www.2ndQuadrant.com


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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Simon Riggs
On Sun, 2009-12-13 at 22:25 +, Simon Riggs wrote:

 * Which exact tables are we talking about: just pg_class and the shared
 catalogs? Everything else is in pg_class, so if we can find it we're OK?
 formrdesc() tells me the list of nailed relations is: pg_database,
 pg_class, pg_attribute, pg_proc, and pg_type. Are the nailed relations
 the ones we care about, or are they just a subset? 

Comments in cluster.c's check_index_is_clusterable() suggest that the
list of tables to which this applies is nailed relations *and* shared
relations, plus their indexes.

/*
 * Disallow clustering system relations.  This will definitely NOT work
 * for shared relations (we have no way to update pg_class rows in other
 * databases), nor for nailed-in-cache relations (the relfilenode values
 * for those are hardwired, see relcache.c).  It might work for other
 * system relations, but I ain't gonna risk it.
 */

So that means we need to handle 3 cases: nailed-local, nailed-shared and
non-nailed-shared.

I would presume we would not want to relax the restriction on CLUSTER
working on these tables, even if new VACUUM FULL does.

Anyway, not going to be done for Alpha3, but seems fairly doable now.

-- 
 Simon Riggs   www.2ndQuadrant.com


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


Re: [HACKERS] Python 3.1 support

2009-12-14 Thread Peter Eisentraut
I wrote:
 On tor, 2009-11-12 at 16:06 -0500, Tom Lane wrote:
  There was considerable debate earlier about whether we wanted to treat
  Python 3 as a separate PL so it could be available in parallel with
  plpython 2, because of the user-level coding incompatibilities.  It
  looks like this patch simply ignores that problem.  What is going to
  happen to plpython functions that depend on 2.x behavior?
 
 I have a proposal for how to handle this, and a prototype patch
 attached.  This follows essentially what the CPython distribution itself
 does, which will make this tolerably easy to follow for users.
 
 We install plpython as plpython2.so or plpython3.so, depending on the
 version used to build it.  Then, plpython.so is a symlink to
 plpython2.so.

So here is the potentially final patch for this, including the original
port of plpython.c itself, build system adjustments, and documentation.


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


Re: [HACKERS] Aggregate ORDER BY patch

2009-12-14 Thread Andrew Gierth
 Tom == Tom Lane t...@sss.pgh.pa.us writes:

  Updated version of the aggregate order by patch.

 Tom I'm starting to look at this now.  I find it rather bizarre to
 Tom merge both the actual arguments of an aggregate and the optional
 Tom ORDER BY expressions into a single targetlist.  It doesn't seem
 Tom like that would be an especially convenient representation to
 Tom work with,

It's extremely convenient, since you need the arguments and the
ordering expressions together in a slot in order to feed them to
tuplesort (in the general case where there is more than one
expression); you need a tupledesc to pass to tuplesort; and there are
existing functions to construct all of these things from the tlist.
Also, you want to merge the argument expressions and ordering
expressions where possible, and this is exactly what the existing
transformSortClause stuff expects.

 Tom and I would also expect there to be a nonzero performance hit
 Tom from the extra TargetEntry expression nodes, even when the
 Tom feature is not in use.

I tested for that and couldn't reliably detect any (certainly 2%
on count(i) on generated data, and under the circumstances I was
testing that's not necessarily outside the error margin).

-- 
Andrew (irc:RhodiumToad)

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


Re: [HACKERS] Streaming replication and non-blocking I/O

2009-12-14 Thread Heikki Linnakangas
Tom Lane wrote:
 The very, very large practical problem with this is that if you decide
 to change the behavior at any time, the only way to be sure that the WAL
 receiver is using the right libpq version is to perform a soname major
 version bump.  The transformations done by libpq will essentially become
 part of its ABI, and not a very visible part at that.

Not having to change the libpq API would certainly be a big advantage.

It's going to be a bit more complicated in walsender/walreceiver to work
with the libpq COPY API. We're going to need a WAL sending/receiving
protocol on top of it, defined in terms of rows and columns passed
through the COPY protocol.

One problem is the the standby is supposed to send back acknowledgments
to the master, telling it how far it has received/replayed the WAL. Is
there any way to send information back to the server, while a COPY OUT
is in progress? That's not absolutely necessary with asynchronous
replication, but will be with synchronous.

One idea is to stop/start the COPY between every batch of WAL records
sent, giving the client (= walreceiver) a chance to send messages back.
But that will lead to extra round trips.

BTW, something that's been bothering me a bit with this patch is that we
now have to link the backend with libpq. I don't see an immediate
problem with that, but I'm not a packager. Does anyone see a problem
with that?

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

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


Re: [HACKERS] Python 3.1 support

2009-12-14 Thread Robert Haas
On Mon, Dec 14, 2009 at 1:42 PM, Peter Eisentraut pete...@gmx.net wrote:
 I wrote:
 On tor, 2009-11-12 at 16:06 -0500, Tom Lane wrote:
  There was considerable debate earlier about whether we wanted to treat
  Python 3 as a separate PL so it could be available in parallel with
  plpython 2, because of the user-level coding incompatibilities.  It
  looks like this patch simply ignores that problem.  What is going to
  happen to plpython functions that depend on 2.x behavior?

 I have a proposal for how to handle this, and a prototype patch
 attached.  This follows essentially what the CPython distribution itself
 does, which will make this tolerably easy to follow for users.

 We install plpython as plpython2.so or plpython3.so, depending on the
 version used to build it.  Then, plpython.so is a symlink to
 plpython2.so.

 So here is the potentially final patch for this, including the original
 port of plpython.c itself, build system adjustments, and documentation.

I think you forgot to actually attach it...

...Robert

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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Tom Lane
Simon Riggs si...@2ndquadrant.com writes:
  * Disallow clustering system relations.  This will definitely NOT work
  * for shared relations (we have no way to update pg_class rows in other
  * databases), nor for nailed-in-cache relations (the relfilenode values
  * for those are hardwired, see relcache.c).  It might work for other
  * system relations, but I ain't gonna risk it.

 I would presume we would not want to relax the restriction on CLUSTER
 working on these tables, even if new VACUUM FULL does.

Why not?  If we solve the problem of allowing these relations to change
relfilenodes, then CLUSTER would work just fine on them.  Whether it's
particularly useful is not ours to decide I think.

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] Python 3.1 support

2009-12-14 Thread Peter Eisentraut
I wrote:
 On tor, 2009-11-12 at 16:06 -0500, Tom Lane wrote:
  There was considerable debate earlier about whether we wanted to treat
  Python 3 as a separate PL so it could be available in parallel with
  plpython 2, because of the user-level coding incompatibilities.  It
  looks like this patch simply ignores that problem.  What is going to
  happen to plpython functions that depend on 2.x behavior?
 
 I have a proposal for how to handle this, and a prototype patch
 attached.  This follows essentially what the CPython distribution itself
 does, which will make this tolerably easy to follow for users.
 
 We install plpython as plpython2.so or plpython3.so, depending on the
 version used to build it.  Then, plpython.so is a symlink to
 plpython2.so.

So here is the potentially final patch for this, including the original
port of plpython.c itself, build system adjustments, and documentation.

Really attached this time.
diff --git a/config/python.m4 b/config/python.m4
index 9160a2b..32fff43 100644
--- a/config/python.m4
+++ b/config/python.m4
@@ -30,10 +30,12 @@ else
 AC_MSG_ERROR([distutils module not found])
 fi
 AC_MSG_CHECKING([Python configuration directory])
+python_majorversion=`${PYTHON} -c import sys; print(sys.version[[0]])`
 python_version=`${PYTHON} -c import sys; print(sys.version[[:3]])`
 python_configdir=`${PYTHON} -c from distutils.sysconfig import get_python_lib as f; import os; print(os.path.join(f(plat_specific=1,standard_lib=1),'config'))`
 python_includespec=`${PYTHON} -c import distutils.sysconfig; print('-I'+distutils.sysconfig.get_python_inc())`
 
+AC_SUBST(python_majorversion)[]dnl
 AC_SUBST(python_version)[]dnl
 AC_SUBST(python_configdir)[]dnl
 AC_SUBST(python_includespec)[]dnl
diff --git a/configure b/configure
index 009a177..be51281 100755
--- a/configure
+++ b/configure
@@ -677,6 +677,7 @@ python_libdir
 python_includespec
 python_configdir
 python_version
+python_majorversion
 PYTHON
 perl_embed_ldflags
 perl_useshrplib
@@ -6964,6 +6965,7 @@ $as_echo $as_me: error: distutils module not found 2;}
 fi
 { $as_echo $as_me:$LINENO: checking Python configuration directory 5
 $as_echo_n checking Python configuration directory...  6; }
+python_majorversion=`${PYTHON} -c import sys; print(sys.version[0])`
 python_version=`${PYTHON} -c import sys; print(sys.version[:3])`
 python_configdir=`${PYTHON} -c from distutils.sysconfig import get_python_lib as f; import os; print(os.path.join(f(plat_specific=1,standard_lib=1),'config'))`
 python_includespec=`${PYTHON} -c import distutils.sysconfig; print('-I'+distutils.sysconfig.get_python_inc())`
diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
index 4746e68..f9221c9 100644
--- a/doc/src/sgml/installation.sgml
+++ b/doc/src/sgml/installation.sgml
@@ -195,8 +195,12 @@ su - postgres
  para
   To build the applicationPL/Python/ server programming
   language, you need a productnamePython/productname
-  installation with the header files and the applicationdistutils/application module.
-  The minimum required version is productnamePython/productname 2.2.
+  installation with the header files and
+  the applicationdistutils/application module.  The minimum
+  required version is productnamePython/productname
+  2.2.  productnamePython 3/productname is supported with
+  version 3.1 or later; but see xref linkend=plpython-python23
+  when using Python 3.
  /para
 
  para
diff --git a/doc/src/sgml/plpython.sgml b/doc/src/sgml/plpython.sgml
index 502a5bc..e6c998a 100644
--- a/doc/src/sgml/plpython.sgml
+++ b/doc/src/sgml/plpython.sgml
@@ -14,7 +14,8 @@
 
  para
   To install PL/Python in a particular database, use
-  literalcreatelang plpythonu replaceabledbname//literal.
+  literalcreatelang plpythonu replaceabledbname//literal (but
+  see also xref linkend=plpython-python23).
  /para
 
   tip
@@ -42,6 +43,112 @@
   /para
  /note
 
+ sect1 id=plpython-python23
+  titlePython 2 vs. Python 3/title
+
+  para
+   PL/Python supports both the Python 2 and Python 3 language
+   variants.  (The PostgreSQL installation instructions might contain
+   more precise information about the exact supported minor versions
+   of Python.)  Because the Python 2 and Python 3 language variants
+   are incompatible in some important aspects, the following naming
+   and transitioning scheme is used by PL/Python to avoid mixing them:
+
+   itemizedlist
+listitem
+ para
+  The PostgreSQL language named literalplpython2u/literal
+  implements PL/Python based on the Python 2 language variant.
+ /para
+/listitem
+
+listitem
+ para
+  The PostgreSQL language named literalplpython3u/literal
+  implements PL/Python based on the Python 3 language variant.
+ /para
+/listitem
+
+listitem
+ para
+  The language named literalplpythonu/literal implements
+  PL/Python based on the default Python language variant, which is
+  currently 

Re: [HACKERS] Range types

2009-12-14 Thread Scott Bailey

Tom Lane wrote:

Scott Bailey arta...@comcast.net writes:
Because intervals (mathematical not SQL) can be open or closed at each 
end point we need to know what the next an previous value would be at 
the specified granularity. And while you can do some operations without 
knowing this, there are many you can't. For instance you could not tell 
whether two [] or () ranges were adjacent, or be able to coalesce an 
array of ranges.


This statement seems to me to demonstrate that you don't actually
understand the concept of open and closed ranges.  It has nothing
whatsoever to do with assuming that the data type is discrete;
these concepts are perfectly well defined for the reals, for example.
What it is about is whether the inclusion conditions are  bound
or = bound.


I won't address how you draw your conclusions here. But I find it 
'interesting' that you assume that I don't know what I'm talking about 
rather than assume you don't fully understand what I'm talking about.


Anyhow. For any given range you may be 4 combinations of values. Either 
the first value included in the range '[' or the last value preceding 
the start of the range '('; and the last value included in the range ']' 
or the first value following the end of the range ')'. We aren't going 
to store all four data points so we need to normalize into the most 
common form, a half-open interval [) and store just those two values. 
The first value included in the range and the first value after the end 
of our range.


So lets say you are using a  numeric range to model the high and low 
values of stocks trading on a given day. Now imagine doing this with no 
concept of granularity. You will most likely be given a range [low, 
high] with inclusive end points. So how do you convert that to a 
closed-open interval so you can store it? Is 20.4201 the 
next value after 20.42? Probably not. You are going to want to define 
0.01 as the granularity for this (either type or column) so that 20.43 is.


Or again are the ranges [14.0, 22.0] and [22.1, 29.0] adjacent? Maybe, 
maybe not. There is no way to tell w/o knowing the granularity. Perhaps 
the granularity is 0.1 and there are a billion values that are 
not included. Or perhaps the granularity is 0.1 and the are adjacent.


Scott

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


Re: [HACKERS] Hot Standby, release candidate?

2009-12-14 Thread Simon Riggs
On Mon, 2009-12-14 at 20:32 +0200, Heikki Linnakangas wrote:

  * You removed this comment from KnownAssignedXidsInit:
 
  -   /*
  -* XXX: We should check that we don't exceed maxKnownAssignedXids.
  -* Even though the hash table might hold a few more entries than that,
  -* we use fixed-size arrays of that size elsewhere and expected all
  -* entries in the hash table to fit.
  -*/
 
  but AFAICS you didn't address the issue. It's referring to the 'xids'
  array in TransactionIdIsInProgress(), which KnownAssignedXidsGet() fills
  in without checking that it fits.
  
  I have ensured that they are always the same size, by definition, so no
  need to check.
 
 How did you ensure that? The hash table has no hard size limit.

The hash table is in shared memory and the entry size is fixed. My
understanding was that this meant the hash table was fixed in size and
could not grow beyond the allocation. If that assumption was wrong, then
yes we could get an error. Is it? Do you know from experience, or from
code comments?

Incidentally just picked up two much easier issues in that stretch of
code. Thanks for making me look again!

-- 
 Simon Riggs   www.2ndQuadrant.com


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


Re: [HACKERS] Streaming replication and non-blocking I/O

2009-12-14 Thread Tom Lane
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
 It's going to be a bit more complicated in walsender/walreceiver to work
 with the libpq COPY API. We're going to need a WAL sending/receiving
 protocol on top of it, defined in terms of rows and columns passed
 through the COPY protocol.

AFAIR, libpq knows essentially nothing of the data being passed through
COPY --- it just treats that as a byte stream.  I think you can define
any data format you want, it doesn't need to look exactly like a COPY
of a table would.  In fact it's probably a lot better if it DOESN'T
look like COPY data once it gets past libpq, so that you can check
that it is WAL and not COPY data.

 One problem is the the standby is supposed to send back acknowledgments
 to the master, telling it how far it has received/replayed the WAL. Is
 there any way to send information back to the server, while a COPY OUT
 is in progress? That's not absolutely necessary with asynchronous
 replication, but will be with synchronous.

Well, a real COPY would of course not stop to look for incoming
messages, but I don't think that's inherent in the protocol.  You
would likely need some libpq adjustments so it didn't throw error
when you tried that, but it would be a small and one-time adjustment.

 BTW, something that's been bothering me a bit with this patch is that we
 now have to link the backend with libpq. I don't see an immediate
 problem with that, but I'm not a packager. Does anyone see a problem
 with that?

Yeah, I have a problem with that.  What's the backend doing with libpq?
It's not receiving this data, it's sending 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] Hot Standby, release candidate?

2009-12-14 Thread Simon Riggs
On Mon, 2009-12-14 at 13:48 -0500, Tom Lane wrote:
 Simon Riggs si...@2ndquadrant.com writes:
   * Disallow clustering system relations.  This will definitely NOT work
   * for shared relations (we have no way to update pg_class rows in other
   * databases), nor for nailed-in-cache relations (the relfilenode values
   * for those are hardwired, see relcache.c).  It might work for other
   * system relations, but I ain't gonna risk it.
 
  I would presume we would not want to relax the restriction on CLUSTER
  working on these tables, even if new VACUUM FULL does.
 
 Why not?  If we solve the problem of allowing these relations to change
 relfilenodes, then CLUSTER would work just fine on them.  Whether it's
 particularly useful is not ours to decide I think.

I think you are probably right, but my wish to prove Schrodinger's Bug
does not exist is not high enough for me personally to open that box
this side of 8.6, especially when the previous code author saw it as a
risk worth documenting. 

-- 
 Simon Riggs   www.2ndQuadrant.com


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


  1   2   >