Thread: Re: [COMMITTERS] pgsql: Added the Skytools extended transaction ID module to contrib as

On Sun, Oct 07, 2007 at 11:32:19PM +0000, Jan Wieck wrote:
> Log Message:
> -----------
> Added the Skytools extended transaction ID module to contrib as discussed
> on CORE previously.
> 
> This module offers transaction ID's containing the original XID and the
> transaction epoch as a bigint value to the user level. It also provides
> a special txid_snapshot data type that contains an entire transactions
> visibility snapshot information, which is useful to determine if a
> particular txid was visible to a transaction or not.
> 
> The module has been tested by porting Slony-I from using its original
> xxid data type.
> 
> Jan

A couple of questions on this. I'm not objecting to the tech stuff itself,
but on procedure:

1) Why was this added without any discussion, or even notification, on
-hackers before it was done? Last I checked, -core claim not to deal with
such technicalities, but defer to -hackers (or other lists as needed). I certainly
trust -core to make the right call no these things, but it's not the
procedure that we claim to have. 

If that procedure is changing (I've been getting a sneaky feeling that 
it's tilting a bit in that direction before this one), that's fine, but it
should be communicated to the community so everybody knows how it works.


2) How can this go in *months* after feature freeze, and even after we
tagged and bundled beta1? This makes such discission even more important,
IMHO.

3) I thought the general agreement was to cut down on contrib, and move
things either to pgfoundry or to core. Why are we adding more? I'm sure
there's motivation for this as discussed on -core, but the rest of us would
like to know that as well... Like why we're not trying to make it a real
feature, if it's something that's important enough to break the rules as in
#2 above.


Or I could've missed the discussion on -hackers that actually took place -
in that case, just discard this message. but the only one I recall is
someone asking for pl/proxy to go in.


//Magnus

Magnus Hagander wrote:
>
> Or I could've missed the discussion on -hackers that actually took place -
> in that case, just discard this message. but the only one I recall is
> someone asking for pl/proxy to go in.
>
>
>
>   

There was some discussion (subject: Provide 8-byte transaction IDs to 
user level), but that itself happened after feature freeze and didn't 
seem too conclusive.

I share your other concerns. This process certainly seems to have been 
less than transparent.


cheers

andrew

Andrew Dunstan <andrew@dunslane.net> writes:
> I share your other concerns. This process certainly seems to have been 
> less than transparent.

FWIW, Jan asked -core about a week ago whether it'd be okay to add this
code post-beta, and we concluded it would be okay on the grounds that
(1) we've always been laxer about contrib than the core code,
(2) we'd already granted similar permission to Oleg and Teodor to add   some example tsearch dictionaries to contrib
post-beta.
But I was expecting some -hackers discussion and/or a proposed patch
on -patches next.  I agree that Jan skipped a step.
        regards, tom lane

On 10/8/07, Magnus Hagander <magnus@hagander.net> wrote:
> On Sun, Oct 07, 2007 at 11:32:19PM +0000, Jan Wieck wrote:
> > Log Message:
> > -----------
> > Added the Skytools extended transaction ID module to contrib as discussed
> > on CORE previously.

To explain the situation, the public discussion about the current
submission happened here:
http://lists.pgfoundry.org/pipermail/skytools-users/2007-September/000245.html

and here:
http://lists.slony.info/pipermail/slony1-hackers/2007-September/000057.html

And ofcourse, the original submission was at 2006-07 to _8.2_:
http://archives.postgresql.org/pgsql-patches/2006-07/msg00157.php

It was rejected then mostly on 3 reasons (from my POV):

- it was messy and contained unnecesary cruft.
- it was submitted to core not /contrib
- slony was not interested in it at that moment

Now as you can read from recent disussion we had, we found out
that it would be *really* *really* cool if it would appear
in 8.3...  Talk about last moment...

Because of the bad timing it would have been -core call anyway
whether it gets in or not so Jan asked -core directly.  That's
my explanation about what happened, obviously Jan and Tom have
their own opinion.

-- 
marko

Marko Kreen wrote:
> Now as you can read from recent disussion we had, we found out
> that it would be *really* *really* cool if it would appear
> in 8.3...  Talk about last moment...
>
>
>   

That discussion didn't happen on any list I read, so to me it just came 
as a bolt from the blue.

Surely there should at the very least have been a patch posted, core 
approval or not.

cheers

andrew

Marko Kreen wrote:
> On 10/8/07, Magnus Hagander <magnus@hagander.net> wrote:
>> On Sun, Oct 07, 2007 at 11:32:19PM +0000, Jan Wieck wrote:
>>> Log Message:
>>> -----------
>>> Added the Skytools extended transaction ID module to contrib as discussed
>>> on CORE previously.
> 
> To explain the situation, the public discussion about the current
> submission happened here:
> 
>  http://lists.pgfoundry.org/pipermail/skytools-users/2007-September/000245.html
> 
> and here:
> 
>  http://lists.slony.info/pipermail/slony1-hackers/2007-September/000057.html

Ok. That certainly explains it - it did sound weird to have that go in
without any public discussion at all - but none of those lists are
pgsql-hackers ;-)


> And ofcourse, the original submission was at 2006-07 to _8.2_:
> 
>  http://archives.postgresql.org/pgsql-patches/2006-07/msg00157.php

Ah. I only searched for this year, since I only considered submissions
for 8.3. But still, it wasn't AFAIK on any of the patch lists etc.


> It was rejected then mostly on 3 reasons (from my POV):
> 
> - it was messy and contained unnecesary cruft.
> - it was submitted to core not /contrib
> - slony was not interested in it at that moment
> 
> Now as you can read from recent disussion we had, we found out
> that it would be *really* *really* cool if it would appear
> in 8.3...  Talk about last moment...

Well, if it's really really cool to have, why do we put it in /contrib?
If it's that cool, it should be in core, no?

That's not just making comments, I really *do* think that it should be
in core if it's interesting enough to be added to contrib at this time.


> Because of the bad timing it would have been -core call anyway
> whether it gets in or not so Jan asked -core directly.  That's
> my explanation about what happened, obviously Jan and Tom have
> their own opinion.

Right. I can see your point, but it's my understanding that -hackers is
really the ones supposed to decide on this.


//Magnus

Magnus Hagander <magnus@hagander.net> writes:
> Marko Kreen wrote:
>> Because of the bad timing it would have been -core call anyway
>> whether it gets in or not so Jan asked -core directly.  That's
>> my explanation about what happened, obviously Jan and Tom have
>> their own opinion.

> Right. I can see your point, but it's my understanding that -hackers is
> really the ones supposed to decide on this.

It would ultimately have been core's decision, but the discussion should
have happened on -hackers.  There was no reason for it to be private.
        regards, tom lane

On 10/8/2007 1:34 PM, Tom Lane wrote:
> Magnus Hagander <magnus@hagander.net> writes:
>> Marko Kreen wrote:
>>> Because of the bad timing it would have been -core call anyway
>>> whether it gets in or not so Jan asked -core directly.  That's
>>> my explanation about what happened, obviously Jan and Tom have
>>> their own opinion.
> 
>> Right. I can see your point, but it's my understanding that -hackers is
>> really the ones supposed to decide on this.
> 
> It would ultimately have been core's decision, but the discussion should
> have happened on -hackers.  There was no reason for it to be private.

That blame certainly belongs to me and I apologize for jumping that and 
adding it to contrib without any -hackers discussion.

It is definitely a timing issue since I write this very email from JFK, 
boarding a flight to Hong Kong in less than an hour and will be mostly 
offline for the rest of the week.

I agree with the technical issue Tom brought up. Slony itself doesn't 
rely on strtoull() either and this slipped through. I will see that I 
fix that by using Slony's int64 scanning code. I can work on it during 
the flight and commit the fix when I arrive in the hotel.

To Magnus: It certainly would have been cool to have this in core, but 
two weeks ago we didn't know if we can get the code into shape for that 
before BETA (as it is right now I would say it still isn't). So we shot 
for the next best target, which was contrib, where post BETA changes 
aren't as critical.


Jan

-- 
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.                                  #
#================================================== JanWieck@Yahoo.com #

On 10/8/07, Magnus Hagander <magnus@hagander.net> wrote:
> Marko Kreen wrote:
> > On 10/8/07, Magnus Hagander <magnus@hagander.net> wrote:
> >> On Sun, Oct 07, 2007 at 11:32:19PM +0000, Jan Wieck wrote:
> >>> Log Message:
> >>> -----------
> >>> Added the Skytools extended transaction ID module to contrib as discussed
> >>> on CORE previously.
> >
> > To explain the situation, the public discussion about the current
> > submission happened here:
> >
> >  http://lists.pgfoundry.org/pipermail/skytools-users/2007-September/000245.html
> >
> > and here:
> >
> >  http://lists.slony.info/pipermail/slony1-hackers/2007-September/000057.html
>
> Ok. That certainly explains it - it did sound weird to have that go in
> without any public discussion at all - but none of those lists are
> pgsql-hackers ;-)

Yes, sorry about that.  Just the discussion started very
hypetetically, with us probing each other opinion, and there
was nothing to discuss with -hackers.

When we saw a concrete plan for the module, then it was too late
to do a regular -hackers submission, due to the beta timeline
we needed -core opinion immidiately.

Now, after -core gave a nod, then yes, the patch should have been
to -hackers with a notice that it is on fast-path.

[btw, this is me guessing Jan's thinking, but I would have
acted same way.]

> > Now as you can read from recent disussion we had, we found out
> > that it would be *really* *really* cool if it would appear
> > in 8.3...  Talk about last moment...
>
> Well, if it's really really cool to have, why do we put it in /contrib?
> If it's that cool, it should be in core, no?

Main cool thing came from fact that this is the last moment
Slony could do such big conversion of it's codebase.

> That's not just making comments, I really *do* think that it should be
> in core if it's interesting enough to be added to contrib at this time.

Yes, it is cool enough to be in core and I think that's the goal.

But asking for it to be accepted to core a week before beta and
couple months after feature freeze would be asking bit too much,
don't you think?  ;)

-- 
marko

Jan Wieck wrote:
> On 10/8/2007 1:34 PM, Tom Lane wrote:
>> Magnus Hagander <magnus@hagander.net> writes:
>>> Marko Kreen wrote:
>>>> Because of the bad timing it would have been -core call anyway
>>>> whether it gets in or not so Jan asked -core directly.  That's
>>>> my explanation about what happened, obviously Jan and Tom have
>>>> their own opinion.
>>
>>> Right. I can see your point, but it's my understanding that -hackers is
>>> really the ones supposed to decide on this.
>>
>> It would ultimately have been core's decision, but the discussion should
>> have happened on -hackers.  There was no reason for it to be private.
> 
> That blame certainly belongs to me and I apologize for jumping that and
> adding it to contrib without any -hackers discussion.
> 
> It is definitely a timing issue since I write this very email from JFK,
> boarding a flight to Hong Kong in less than an hour and will be mostly
> offline for the rest of the week.
> 
> I agree with the technical issue Tom brought up. Slony itself doesn't
> rely on strtoull() either and this slipped through. I will see that I
> fix that by using Slony's int64 scanning code. I can work on it during
> the flight and commit the fix when I arrive in the hotel.

Good. Win32 is pretty much dead right now, so we can't proceed with
things like testing the buildfarm with msvc/ecpg.


> To Magnus: It certainly would have been cool to have this in core, but
> two weeks ago we didn't know if we can get the code into shape for that
> before BETA (as it is right now I would say it still isn't). So we shot
> for the next best target, which was contrib, where post BETA changes
> aren't as critical.

Right. My thought is still that if it isn't good enough for core, it
shouldn't be in contrib. If it *is* good enough, and we want it, we
should accept that it came in long after freeze and put it in core
anyway. If it *isn't*, then it should be on pgfoundry and be moved into
core when it's ready - for 8.4 or so.

The whole contrib thing confuses a lot of users. Is it included, or
isn't it?  IMHO, that distinction need to be clear, and I thought we
were working (if not actively then at least passively) to "retire"
contrib, moving things either to core or to pgFoundry. Adding yet
another important feature that's "just in contrib" is making things
worse, not better.
IMHO, of course ;-)

//Magnus

Tom Lane wrote:
> Magnus Hagander <magnus@hagander.net> writes:
>> Marko Kreen wrote:
>>> Because of the bad timing it would have been -core call anyway
>>> whether it gets in or not so Jan asked -core directly.  That's
>>> my explanation about what happened, obviously Jan and Tom have
>>> their own opinion.
> 
>> Right. I can see your point, but it's my understanding that -hackers is
>> really the ones supposed to decide on this.
> 
> It would ultimately have been core's decision, but the discussion should
> have happened on -hackers.  There was no reason for it to be private.

Hmm. I thought that -core doesn't decide on things like these, they just
"vote" on -hackers and have no special powers (other than being very
respected community members that we all listen to, of course).

I seem to recall hearing all the time (most often from people on core,
but I'm certainly one of the people who relay that information further)
that core specifically *doesn't* decide on things like that (being
direct technical issues, or just the talk about the name-change that's
been flooding -advocacy), but that core are only there for "dealing with
companies that don't want to deal in public", and for making decisions
"if -hackers can't agree", security sensitive stuff, and things like that.

It may be that it just was like that before, and isn't anymore, and my
information is outdated. I don't mind, really, because I certainly trust
-core to make good decisions. But if that's so, then at least I have to
change what I tell people that ask...


//Magnus

"Magnus Hagander" <magnus@hagander.net> writes:

> The whole contrib thing confuses a lot of users. Is it included, or
> isn't it?  IMHO, that distinction need to be clear, and I thought we
> were working (if not actively then at least passively) to "retire"
> contrib, moving things either to core or to pgFoundry. Adding yet
> another important feature that's "just in contrib" is making things
> worse, not better.
> IMHO, of course ;-)

I disagree with this principle. I think contrib has a role to play and there
are modules which belong in contrib for now and forever. The key distinction
isn't code quality -- I think that's an effect rather than a cause. 

The key distinction is the intended audience. Contrib is for things which are
integral parts of the system and as such would be harder to maintain in
pgfoundry, but have a very limited audience, especially things that a typical
admin wouldn't necessarily want in his install for security or safety reasons.

So pageinspect, adminpack, pg_buffercache, pg_standby, etc, these are all
things which are tightly tied to the system. They often need to be modified
when making unrelated changes to the core and can't be maintained as a
separate add-on by a different group of maintainers. But they're not
appropriate to install by default because they have a limited audience.

Some of the modules in there are legacy modules which today would probably be
done as pgfoundry modules. The GIST data types, earthdistance, isn, etc. We
did actively prune out a lot of modules like that which were poorly maintained
and bitrotting. The ones which remain are in reasonably good shape and have
wide enough user-bases that moving them to pgfoundry would cause more upgrade
pain than it would help.

But that doesn't mean we're phasing contrib out entirely. The question remains
whether txid is more like pageinspect/pg_standby/etc or more like
earthdistance/isn. It does sound like the whole point of it is to provide an
interface to core that pgfoundry modules can use, so presumably it's dealing
with the nitty gritty stuff that pgfoundry modules would have trouble
maintaining. Also, we only want *one* official such module. I think pushing it
to pgfoundry doesn't make any sense.

Does it make sense to put it in core? it has a limited audience in that only
skype and slony users need it. On the other hand there's not much reason an
admin wouldn't want it installed I don't think.

What happens if we put it in core now and then the replication folk add more
to the interface and include things that not all admins would want installed?
And is the interface mature enough that users should be building applications
depending on this interface directly?

--  Gregory Stark EnterpriseDB          http://www.enterprisedb.com

On Monday 08 October 2007 16:29, Magnus Hagander wrote:
> The whole contrib thing confuses a lot of users. Is it included, or
> isn't it?  IMHO, that distinction need to be clear, and I thought we
> were working (if not actively then at least passively) to "retire"
> contrib, moving things either to core or to pgFoundry. Adding yet
> another important feature that's "just in contrib" is making things
> worse, not better.
> IMHO, of course ;-)
>

+1.  I felt the same way about pg_standby, which would have been far more 
accessible for 8.2 users had it lived on pg_foundry. 

-- 
Robert Treat
Build A Brighter LAMP :: Linux Apache {middleware} PostgreSQL

Tom Lane wrote:
> (1) we've always been laxer about contrib than the core code,

While that appears to be true, I think

(a) there is no technical reason allowing us to do that, and
(b) most people don't seem to like it.

-- 
Peter Eisentraut
http://developer.postgresql.org/~petere/

On Mon, 08 Oct 2007 17:38:48 -0400
Robert Treat <xzilla@users.sourceforge.net> wrote:

> On Monday 08 October 2007 16:29, Magnus Hagander wrote:
> > The whole contrib thing confuses a lot of users. Is it included, or
> > isn't it?  IMHO, that distinction need to be clear, and I thought we
> > were working (if not actively then at least passively) to "retire"
> > contrib, moving things either to core or to pgFoundry. Adding yet
> > another important feature that's "just in contrib" is making things
> > worse, not better.
> > IMHO, of course ;-)
> >
>
> +1.  I felt the same way about pg_standby, which would have been far
> more accessible for 8.2 users had it lived on pg_foundry.

Well pg_standby is certainly an excellent example :). We have had this
similar discussion before, this exact discussion is one of the reasons
Tsearch2 got pushed into core.

Contrib is just a dead zone for the user populous. Most people consider
it unsupported *stuff* with no particular purpose (regardless of the
real meaning).

If you look at contrib in other projects they are always user
contributed modules that are cool, but your mileage may vary.

Personally, I would like to see contrib completely removed.

Sincerely,

Joshua D. Drake


>


--
     === The PostgreSQL Company: Command Prompt, Inc. ===
Sales/Support: +1.503.667.4564   24x7/Emergency: +1.800.492.2240
PostgreSQL solutions since 1997  http://www.commandprompt.com/        UNIQUE NOT NULL
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
PostgreSQL Replication: http://www.commandprompt.com/products/

On Mon, 08 Oct 2007 22:32:55 +0200
Magnus Hagander <magnus@hagander.net> wrote:

> Tom Lane wrote:
> > Magnus Hagander <magnus@hagander.net> writes:
> >> Marko Kreen wrote:
> >>> Because of the bad timing it would have been -core call anyway
> >>> whether it gets in or not so Jan asked -core directly.  That's
> >>> my explanation about what happened, obviously Jan and Tom have
> >>> their own opinion.
> >
> >> Right. I can see your point, but it's my understanding that
> >> -hackers is really the ones supposed to decide on this.
> >
> > It would ultimately have been core's decision, but the discussion
> > should have happened on -hackers.  There was no reason for it to be
> > private.
>
> Hmm. I thought that -core doesn't decide on things like these, they
> just "vote" on -hackers and have no special powers (other than being
> very respected community members that we all listen to, of course).

That was my understanding as well.

I quote from Tom Lane on August 30th of this year:

I think you're overstating the amount of power the core committee has.
Core sets release schedules, but beyond that has no more power than any
other committer. The pool of committers is quite a bit bigger than core.

In practice, of course, core has quite a lot of political power because
folk are often willing to follow our lead. But we'd lose that real
quick if we tried to lead in the wrong direction.

Source:

http://people.planetpostgresql.org/xzilla/index.php?/archives/317-PostgreSQL-is-Not-a-Democracy.html

Sincerely,

Joshua D. Drake

--
     === The PostgreSQL Company: Command Prompt, Inc. ===
Sales/Support: +1.503.667.4564   24x7/Emergency: +1.800.492.2240
PostgreSQL solutions since 1997  http://www.commandprompt.com/        UNIQUE NOT NULL
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
PostgreSQL Replication: http://www.commandprompt.com/products/

Magnus Hagander <magnus@hagander.net> writes:
> Right. My thought is still that if it isn't good enough for core, it
> shouldn't be in contrib. If it *is* good enough, and we want it, we
> should accept that it came in long after freeze and put it in core
> anyway. If it *isn't*, then it should be on pgfoundry and be moved into
> core when it's ready - for 8.4 or so.

The long and the short of it was that the patch wasn't ready.  To put it
in core for 8.3, we'd have either had to delay the beta yet more, or
force initdb post-beta1, neither of which would have flown.

> The whole contrib thing confuses a lot of users.

To me, contrib exists mostly as a forcing function to ensure that we
keep the extension-module system working.  If we got rid of it entirely,
as some here propose, we'd be more likely to break things that we'd not
find out about until much later (like when some pgfoundry project tried
to update their code, which almost certainly wouldn't be till the next
beta cycle).

Contrib also has a role to play as a repository of code examples that
people can crib from when developing new extension modules.  I would
not want to claim that it's all "best practice" code --- a lot of it
definitely isn't --- but it stands a lot better chance of representing
current good practice if it's maintained with the core code than if it's
out on pgfoundry.  On pgfoundry, it won't get included in the global-
search-and-replace patches that we do so many of, and it'll most likely
accumulate a lot of cruft from trying to be compatible with multiple
core releases.

So I have no interest in trying to "retire" contrib.  I think there's
room for debate about exactly which modules to include, of course.
        regards, tom lane

A Dimarts 09 Octubre 2007, Joshua D. Drake va escriure:
>
> Contrib is just a dead zone for the user populous. Most people consider
> it unsupported *stuff* with no particular purpose (regardless of the
> real meaning).
>

I think no visible documentation is the reason for this misconception and 8.3 
will provide contrib documentation together with core docs. Let's see what 
happens then...


-- 
Albert Cervera i Areny
http://www.NaN-tic.com

Tom Lane wrote:
> So I have no interest in trying to "retire" contrib.  I think there's
> room for debate about exactly which modules to include, of course.
>
>     
>   

I dont' think there's much call for retiring it. I think there is 
interest in renaming it, as "contrib" is a wholly misleading name, and 
rearranging the modules somewhat, and above all in documenting it properly.

One of the original goals of the buildfarm was to lessen the extent to 
which contrib was a second class citizen by making sure it was tested on 
the same schedule as the rest of Postgres.

cheers

andrew

> On Sun, Oct 07, 2007 at 11:32:19PM +0000, Jan Wieck wrote:
> > Log Message:
> > -----------
> > Added the Skytools extended transaction ID module to contrib as discussed
> > on CORE previously.
> > 
> > This module offers transaction ID's containing the original XID and the
> > transaction epoch as a bigint value to the user level. It also provides
> > a special txid_snapshot data type that contains an entire transactions
> > visibility snapshot information, which is useful to determine if a
> > particular txid was visible to a transaction or not.
> > 
> > The module has been tested by porting Slony-I from using its original
> > xxid data type.
> > 
> > Jan
> 
> A couple of questions on this. I'm not objecting to the tech stuff itself,
> but on procedure:
> 
> 1) Why was this added without any discussion, or even notification, on
> -hackers before it was done? Last I checked, -core claim not to deal with
> such technicalities, but defer to -hackers (or other lists as needed). I certainly
> trust -core to make the right call no these things, but it's not the
> procedure that we claim to have. 
> 
> If that procedure is changing (I've been getting a sneaky feeling that 
> it's tilting a bit in that direction before this one), that's fine, but it
> should be communicated to the community so everybody knows how it works.
> 
> 
> 2) How can this go in *months* after feature freeze, and even after we
> tagged and bundled beta1? This makes such discission even more important,
> IMHO.
> 
> 3) I thought the general agreement was to cut down on contrib, and move
> things either to pgfoundry or to core. Why are we adding more? I'm sure
> there's motivation for this as discussed on -core, but the rest of us would
> like to know that as well... Like why we're not trying to make it a real
> feature, if it's something that's important enough to break the rules as in
> #2 above.
> 
> 
> Or I could've missed the discussion on -hackers that actually took place -
> in that case, just discard this message. but the only one I recall is
> someone asking for pl/proxy to go in.

More question. Who is in charge of updating HISTORY? I see no commit
messages for this.
--
Tatsuo Ishii
SRA OSS, Inc. Japan

Tatsuo Ishii wrote:

> More question. Who is in charge of updating HISTORY? I see no commit
> messages for this.

It is a generated file.

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

> > More question. Who is in charge of updating HISTORY? I see no commit
> > messages for this.
> 
> It is a generated file.

I mean release.sgml.
--
Tatsuo Ishii
SRA OSS, Inc. Japan

Tatsuo Ishii wrote:
> > > More question. Who is in charge of updating HISTORY? I see no commit
> > > messages for this.
> > 
> > It is a generated file.
> 
> I mean release.sgml.

Tom and others made commits to this and we will update it again before
final.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://postgres.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

> Tatsuo Ishii wrote:
> > > > More question. Who is in charge of updating HISTORY? I see no commit
> > > > messages for this.
> > > 
> > > It is a generated file.
> > 
> > I mean release.sgml.
> 
> Tom and others made commits to this and we will update it again before
> final.

Did it? I see nothing for txid in relesase.sgml.
--
Tatsuo Ishii
SRA OSS, Inc. Japan

Tatsuo Ishii wrote:
> > Tatsuo Ishii wrote:
> > > > > More question. Who is in charge of updating HISTORY? I see no commit
> > > > > messages for this.
> > > > 
> > > > It is a generated file.
> > > 
> > > I mean release.sgml.
> > 
> > Tom and others made commits to this and we will update it again before
> > final.
> 
> Did it? I see nothing for txid in relesase.sgml.

Right.  release.sgml will be updated in batches as we near final
release.  We don't update for individual commits.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://postgres.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

Jan Wieck wrote:
> On 10/8/2007 1:34 PM, Tom Lane wrote:
> > Magnus Hagander <magnus@hagander.net> writes:
> >> Marko Kreen wrote:
> >>> Because of the bad timing it would have been -core call anyway
> >>> whether it gets in or not so Jan asked -core directly.  That's
> >>> my explanation about what happened, obviously Jan and Tom have
> >>> their own opinion.
> > 
> >> Right. I can see your point, but it's my understanding that -hackers is
> >> really the ones supposed to decide on this.
> > 
> > It would ultimately have been core's decision, but the discussion should
> > have happened on -hackers.  There was no reason for it to be private.
> 
> That blame certainly belongs to me and I apologize for jumping that and 
> adding it to contrib without any -hackers discussion.
> 
> It is definitely a timing issue since I write this very email from JFK, 
> boarding a flight to Hong Kong in less than an hour and will be mostly 
> offline for the rest of the week.

I don't see how timing has anything to do with this.  You could have
added it between beta1 and beta2 after sufficient hackers discussion. 
Doing it the way you did with no warning, right before beta, and then
leaving is the worse of all times.  I am surprised we are not backing
out the patch and requiring that the patch go through the formal review
process.

This is not the first time you have had trouble with patches.  There was
an issue with your patch of February, 2007:
http://archives.postgresql.org/pgsql-hackers/2007-02/msg00385.php

(In summary, you had to be coaxed to explain your patch to the
community.)  Basically, I am not sure you understand the process that
has to be followed, or feel you are somehow immune from following it.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://postgres.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce@momjian.us> writes:
> I don't see how timing has anything to do with this.  You could have
> added it between beta1 and beta2 after sufficient hackers discussion. 

Uh, it *was* after beta1.
        regards, tom lane

On Mon, 2007-10-08 at 17:38 -0400, Robert Treat wrote:
> On Monday 08 October 2007 16:29, Magnus Hagander wrote:
> > The whole contrib thing confuses a lot of users. Is it included, or
> > isn't it?  IMHO, that distinction need to be clear, and I thought we
> > were working (if not actively then at least passively) to "retire"
> > contrib, moving things either to core or to pgFoundry. Adding yet
> > another important feature that's "just in contrib" is making things
> > worse, not better.
> > IMHO, of course ;-)
> >
> 
> +1.  I felt the same way about pg_standby, which would have been far more 
> accessible for 8.2 users had it lived on pg_foundry. 

I think we should move a version of pg_standby to pg_foundry anyway,
specifically to support 8.2 users. 

--  Simon Riggs 2ndQuadrant  http://www.2ndQuadrant.com

On Tue, Oct 09, 2007 at 08:20:45AM +0100, Simon Riggs wrote:
> On Mon, 2007-10-08 at 17:38 -0400, Robert Treat wrote:
> > On Monday 08 October 2007 16:29, Magnus Hagander wrote:
> > > The whole contrib thing confuses a lot of users. Is it included, or
> > > isn't it?  IMHO, that distinction need to be clear, and I thought we
> > > were working (if not actively then at least passively) to "retire"
> > > contrib, moving things either to core or to pgFoundry. Adding yet
> > > another important feature that's "just in contrib" is making things
> > > worse, not better.
> > > IMHO, of course ;-)
> > >
> > 
> > +1.  I felt the same way about pg_standby, which would have been far more 
> > accessible for 8.2 users had it lived on pg_foundry. 
> 
> I think we should move a version of pg_standby to pg_foundry anyway,
> specifically to support 8.2 users. 

Are you saying you want two versions of pg_standby, one in contrbi and one
on pgfoundry, or are you saying we should take the one in contrib away?

//Magnus

> > Did it? I see nothing for txid in relesase.sgml.
> 
> Right.  release.sgml will be updated in batches as we near final
> release.  We don't update for individual commits.

Ok. I will explain about txid for local users myself.
--
Tatsuo Ishii
SRA OSS, Inc. Japan

On Tue, 2007-10-09 at 10:58 +0200, Magnus Hagander wrote:
> On Tue, Oct 09, 2007 at 08:20:45AM +0100, Simon Riggs wrote:
> > On Mon, 2007-10-08 at 17:38 -0400, Robert Treat wrote:
> > > On Monday 08 October 2007 16:29, Magnus Hagander wrote:
> > > > The whole contrib thing confuses a lot of users. Is it included, or
> > > > isn't it?  IMHO, that distinction need to be clear, and I thought we
> > > > were working (if not actively then at least passively) to "retire"
> > > > contrib, moving things either to core or to pgFoundry. Adding yet
> > > > another important feature that's "just in contrib" is making things
> > > > worse, not better.
> > > > IMHO, of course ;-)
> > > >
> > > 
> > > +1.  I felt the same way about pg_standby, which would have been far more 
> > > accessible for 8.2 users had it lived on pg_foundry. 
> > 
> > I think we should move a version of pg_standby to pg_foundry anyway,
> > specifically to support 8.2 users. 
> 
> Are you saying you want two versions of pg_standby, one in contrbi and one
> on pgfoundry, or are you saying we should take the one in contrib away?

I would prefer that we backported pg_standby into 8.2 contrib, so the
solution is where people need it to be. If not...

--  Simon Riggs 2ndQuadrant  http://www.2ndQuadrant.com

Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > I don't see how timing has anything to do with this.  You could have
> > added it between beta1 and beta2 after sufficient hackers discussion. 
> 
> Uh, it *was* after beta1.

Oh, so it didn't hold up beta1 --- that's good.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://postgres.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
> Tom Lane wrote:
>   
>> Bruce Momjian <bruce@momjian.us> writes:
>>     
>>> I don't see how timing has anything to do with this.  You could have
>>> added it between beta1 and beta2 after sufficient hackers discussion. 
>>>       
>> Uh, it *was* after beta1.
>>     
>
> Oh, so it didn't hold up beta1 --- that's good.
>
>   

No it's not.

Can somebody please explain to me what beta means if you can commit new 
stuff after it has been declared?

cheers

andrew

Andrew Dunstan wrote:
> 
> 
> Bruce Momjian wrote:
>> Tom Lane wrote:
>>  
>>> Bruce Momjian <bruce@momjian.us> writes:
>>>    
>>>> I don't see how timing has anything to do with this.  You could have
>>>> added it between beta1 and beta2 after sufficient hackers
>>>> discussion.       
>>> Uh, it *was* after beta1.
>>>     
>>
>> Oh, so it didn't hold up beta1 --- that's good.
>>
>>   
> 
> No it's not.
> 
> Can somebody please explain to me what beta means if you can commit new
> stuff after it has been declared?

Yeah, I'd like to know that as well. And specifically what kind of stuffit is that's ok...

If nothing else, that'll be good to know for packagers. Due to the
addition of this module we'll have to make code changes that aren't just
bugfixes to the win32 installer for example, which is also in feature
freeze...

//Magnus

Andrew Dunstan wrote:
> 
> 
> Bruce Momjian wrote:
> > Tom Lane wrote:
> >   
> >> Bruce Momjian <bruce@momjian.us> writes:
> >>     
> >>> I don't see how timing has anything to do with this.  You could have
> >>> added it between beta1 and beta2 after sufficient hackers discussion. 
> >>>       
> >> Uh, it *was* after beta1.
> >>     
> >
> > Oh, so it didn't hold up beta1 --- that's good.
> >
> >   
> 
> No it's not.
> 
> Can somebody please explain to me what beta means if you can commit new 
> stuff after it has been declared?

We allow /contrib to be more lax about beta changes.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://postgres.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
> Andrew Dunstan wrote:
>   
>>
>> Can somebody please explain to me what beta means if you can commit new 
>> stuff after it has been declared?
>>     
>
> We allow /contrib to be more lax about beta changes.
>
>   

I think we should be looking long and hard at that. I can't see any 
justification for it at all.


cheers

andrew

Bruce Momjian wrote:
> Andrew Dunstan wrote:
>>
>> Bruce Momjian wrote:
>>> Tom Lane wrote:
>>>   
>>>> Bruce Momjian <bruce@momjian.us> writes:
>>>>     
>>>>> I don't see how timing has anything to do with this.  You could have
>>>>> added it between beta1 and beta2 after sufficient hackers discussion. 
>>>>>       
>>>> Uh, it *was* after beta1.
>>>>     
>>> Oh, so it didn't hold up beta1 --- that's good.
>>>
>>>   
>> No it's not.
>>
>> Can somebody please explain to me what beta means if you can commit new 
>> stuff after it has been declared?
> 
> We allow /contrib to be more lax about beta changes.

the postgresql ecosystem is growing and there is a lot of people like
packagers that will be a quite irritated if we keep randomly adding
completely new code and modules during BETA.


Stefan

Bruce Momjian wrote:
>> Can somebody please explain to me what beta means if you can commit new 
>> stuff after it has been declared?
> 
> We allow /contrib to be more lax about beta changes.

Why? When people were complaining about not being able to use TSearch
because their ISPs wouldn't install contrib modules we couldn't
understand why they would think that way. If we are going to be less
stringent about /contrib, maybe they were right to cautious.

/D

Dave Page wrote:
> Bruce Momjian wrote:
> >> Can somebody please explain to me what beta means if you can commit new 
> >> stuff after it has been declared?
> > 
> > We allow /contrib to be more lax about beta changes.
> 
> Why? When people were complaining about not being able to use TSearch
> because their ISPs wouldn't install contrib modules we couldn't
> understand why they would think that way. If we are going to be less
> stringent about /contrib, maybe they were right to cautious.

The idea is /contrib isn't installed by default and it isn't tied into
the core code, and can be tested easier because it is stand-alone.  We
can rethink that logic but that has been the guide in the past.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://postgres.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

On 10/9/2007 1:06 AM, Bruce Momjian wrote:
> Jan Wieck wrote:
>> On 10/8/2007 1:34 PM, Tom Lane wrote:
>> > Magnus Hagander <magnus@hagander.net> writes:
>> >> Marko Kreen wrote:
>> >>> Because of the bad timing it would have been -core call anyway
>> >>> whether it gets in or not so Jan asked -core directly.  That's
>> >>> my explanation about what happened, obviously Jan and Tom have
>> >>> their own opinion.
>> > 
>> >> Right. I can see your point, but it's my understanding that -hackers is
>> >> really the ones supposed to decide on this.
>> > 
>> > It would ultimately have been core's decision, but the discussion should
>> > have happened on -hackers.  There was no reason for it to be private.
>> 
>> That blame certainly belongs to me and I apologize for jumping that and 
>> adding it to contrib without any -hackers discussion.
>> 
>> It is definitely a timing issue since I write this very email from JFK, 
>> boarding a flight to Hong Kong in less than an hour and will be mostly 
>> offline for the rest of the week.
> 
> I don't see how timing has anything to do with this.  You could have
> added it between beta1 and beta2 after sufficient hackers discussion. 
> Doing it the way you did with no warning, right before beta, and then
> leaving is the worse of all times.  I am surprised we are not backing
> out the patch and requiring that the patch go through the formal review
> process.
> 
> This is not the first time you have had trouble with patches.  There was
> an issue with your patch of February, 2007:
> 
>     http://archives.postgresql.org/pgsql-hackers/2007-02/msg00385.php

That email might contain the keyword COMMIT, but it doesn't have to do 
with anything I committed to CVS. The trigger changes you are referring 
to have been discussed and a patch for discussion was presented here:
    http://archives.postgresql.org/pgsql-hackers/2007-02/msg00146.php


> (In summary, you had to be coaxed to explain your patch to the
> community.)  Basically, I am not sure you understand the process that
> has to be followed, or feel you are somehow immune from following it.

I don't see how you leap from the above example to that conclusion.


Jan

-- 
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.                                  #
#================================================== JanWieck@Yahoo.com #

Magnus Hagander wrote:
> Andrew Dunstan wrote:
>>
>> Bruce Momjian wrote:
>>> Tom Lane wrote:
>>>  
>>>> Bruce Momjian <bruce@momjian.us> writes:
>>>>    
>>>>> I don't see how timing has anything to do with this.  You could have
>>>>> added it between beta1 and beta2 after sufficient hackers
>>>>> discussion.       
>>>> Uh, it *was* after beta1.
>>>>     
>>> Oh, so it didn't hold up beta1 --- that's good.
>>>
>>>   
>> No it's not.
>>
>> Can somebody please explain to me what beta means if you can commit new
>> stuff after it has been declared?
> 
> Yeah, I'd like to know that as well. And specifically what kind of stuff
>  it is that's ok...

I hate to say this - but this "adding completely new steff after or
immediatly before beta" business really scares the hell out of me and
somewhat starts to resemble the mysql way of adding new features at will
and even during stable release trains ...
There is no point in having any kind of feature freeze or even a
PatchStatus Page if we keep adding stuff (as useful as it might be) that
late in the cycle ...


Stefan

Bruce Momjian wrote:
> Dave Page wrote:
>> Bruce Momjian wrote:
>>>> Can somebody please explain to me what beta means if you can commit new 
>>>> stuff after it has been declared?
>>> We allow /contrib to be more lax about beta changes.
>> Why? When people were complaining about not being able to use TSearch
>> because their ISPs wouldn't install contrib modules we couldn't
>> understand why they would think that way. If we are going to be less
>> stringent about /contrib, maybe they were right to cautious.
> 
> The idea is /contrib isn't installed by default and it isn't tied into
> the core code, and can be tested easier because it is stand-alone.  We
> can rethink that logic but that has been the guide in the past.

I think you just outlined a whole lot of arguments for pgfoundry, and
not for contrib.

//Magnus

Bruce Momjian wrote:
> Dave Page wrote:
>> Bruce Momjian wrote:
>>>> Can somebody please explain to me what beta means if you can commit new 
>>>> stuff after it has been declared?
>>> We allow /contrib to be more lax about beta changes.
>> Why? When people were complaining about not being able to use TSearch
>> because their ISPs wouldn't install contrib modules we couldn't
>> understand why they would think that way. If we are going to be less
>> stringent about /contrib, maybe they were right to cautious.
> 
> The idea is /contrib isn't installed by default and it isn't tied into
> the core code, and can be tested easier because it is stand-alone.  We
> can rethink that logic but that has been the guide in the past.

Yes, I think we should if this is the result. It's one thing keeping
modules seperate for ease of testing, but it's another to become less
rigourous about how that code is maintained, developed and tested
compared to the core code.

/D

On Tue, Oct 09, 2007 at 01:06:11AM -0400, Bruce Momjian wrote:
> Jan Wieck wrote:
> > On 10/8/2007 1:34 PM, Tom Lane wrote:
> > > Magnus Hagander <magnus@hagander.net> writes:
> > >> Marko Kreen wrote:
> > >>> Because of the bad timing it would have been -core call anyway
> > >>> whether it gets in or not so Jan asked -core directly.  That's
> > >>> my explanation about what happened, obviously Jan and Tom have
> > >>> their own opinion.
> > > 
> > >> Right. I can see your point, but it's my understanding that
> > >> -hackers is really the ones supposed to decide on this.
> > > 
> > > It would ultimately have been core's decision, but the
> > > discussion should have happened on -hackers.  There was no
> > > reason for it to be private.
> > 
> > That blame certainly belongs to me and I apologize for jumping
> > that and adding it to contrib without any -hackers discussion.
> > 
> > It is definitely a timing issue since I write this very email from
> > JFK, boarding a flight to Hong Kong in less than an hour and will
> > be mostly offline for the rest of the week.
> 
> I don't see how timing has anything to do with this.  You could have
> added it between beta1 and beta2 after sufficient hackers
> discussion.  Doing it the way you did with no warning, right before
> beta, and then leaving is the worse of all times.  I am surprised we
> are not backing out the patch and requiring that the patch go
> through the formal review process.

The feature is great, and it should stay on pgfoundry or other
suitable place until the 8.4 development cycle.  I believe it would
make a great addition to the core product, but only in 8.4.

Cheers,
David.
-- 
David Fetter <david@fetter.org> http://fetter.org/
phone: +1 415 235 3778        AIM: dfetter666                             Skype: davidfetter

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

On Oct 9, 2007, at 0:06 , Bruce Momjian wrote:

> I am surprised we are not backing
> out the patch and requiring that the patch go through the formal  
> review
> process.

I have no opinion as to the patch itself (other than the fact that  
it's a not bug fix), but I think this patch should be reverted  
because it's (a) after feature freeze, (b) had no discussion on  
hackers (or patches), (c) is not a bug fix. IMO rules can be bent but  
there should always at least be discussion before a new feature is  
committed after feature freeze and definitely after beta. Otherwise,  
the rule appears to be if you can get it in somehow, it's in.

Again, I have no opinion regarding the patch itself, and these issues  
are regardless of who commits or submits. Personally, I regard Jan as  
a helpful guy and a solid coder who has contributed a lot to  
PostgreSQL in the past and I'm sure will contribute even more in the  
future.

Michael Glaesemann
grzm seespotcode net

On Tue, 9 Oct 2007 18:35:52 -0500
Michael Glaesemann <grzm@seespotcode.net> wrote:

>
> On Oct 9, 2007, at 0:06 , Bruce Momjian wrote:
>
> > I am surprised we are not backing
> > out the patch and requiring that the patch go through the formal
> > review
> > process.
>
> I have no opinion as to the patch itself (other than the fact that
> it's a not bug fix), but I think this patch should be reverted
> because it's (a) after feature freeze, (b) had no discussion on
> hackers (or patches), (c) is not a bug fix. IMO rules can be bent
> but there should always at least be discussion before a new feature
> is committed after feature freeze and definitely after beta.
> Otherwise, the rule appears to be if you can get it in somehow, it's
> in.

I think this almost says it all. My particular gripe about this whole
thing is that there are other features that are not too intrusive (or
appear so anyway) that are easily more useful that are not being
considered at all. Namely,
http://archives.postgresql.org/pgsql-patches/2007-10/msg00087.php . It
makes the whole process seem tilted and subjective.

IMO, the patch is reverted, and submitted for 8.4 or pgfoundry.

Sincerely,

Joshua D. Drake


--
     === The PostgreSQL Company: Command Prompt, Inc. ===
Sales/Support: +1.503.667.4564   24x7/Emergency: +1.800.492.2240
PostgreSQL solutions since 1997  http://www.commandprompt.com/        UNIQUE NOT NULL
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
PostgreSQL Replication: http://www.commandprompt.com/products/

Hi,

On Tue, 2007-10-09 at 16:50 -0700, Joshua D. Drake wrote:
> IMO, the patch is reverted, and submitted for 8.4 or pgfoundry.

That means another delay for improving PostgreSQL replication.

I think we are all pretty sure that Jan knows what he is doing -- he has
involved in replication issues for years, and as a long-time core
member, I'm sure that he knows the rules -- but I think this patch will
speed up works on replication.

(Will all respect to pginstaller team, I'm *think* it won't take much
time to add txid to installer, at least compared to the time that we
spent discussing this issue.)

You know, txid was discussed in Slony-I + Skytools lists for a
reasonably long time, and Tom also commented in that thread. I agree
that we broke the policy this time, but this does not mean the end of
the world.

What we should to do is to prevent such things happening in the future,
rather than reverting this patch and delaying replication issues.

Regards,
--
Devrim GÜNDÜZ
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Managed Services, Shared and Dedicated Hosting
Co-Authors: plPHP, ODBCng - http://www.commandprompt.com/

On Tue, 2007-10-09 at 16:50 -0700, Joshua D. Drake wrote:
> I think this almost says it all. My particular gripe about this whole
> thing is that there are other features that are not too intrusive (or
> appear so anyway) that are easily more useful that are not being 
> considered at all. Namely,
> http://archives.postgresql.org/pgsql-patches/2007-10/msg00087.php .

That is NOT a good example. That patch is a first-cut of a non-trivial
optimizer feature that was submitted just before beta1 shipped, by
someone who hasn't modified the optimizer before. Jan's patch was a
contrib module that has been already developed by the Skype folks, and
it goes without saying that Jan has contributed to Postgres extensively.

> It makes the whole process seem tilted and subjective.

There are some fairly obvious, objective reasons why txid differs from
the inline-SQL-SRF patch.

That said, I agree that the process should have been followed in this
case.

-Neil

On Tue, 09 Oct 2007 17:04:41 -0700
Devrim GÜNDÜZ <devrim@CommandPrompt.com> wrote:

> Hi,
>
> On Tue, 2007-10-09 at 16:50 -0700, Joshua D. Drake wrote:
> > IMO, the patch is reverted, and submitted for 8.4 or pgfoundry.
>
> That means another delay for improving PostgreSQL replication.

No. It can go on pgFoundry. Remember this is a contrib module, not
something that is directly integrated into core. We loose nothing by
having it on pgfoundry.

>
> I think we are all pretty sure that Jan knows what he is doing -- he
> has involved in replication issues for years, and as a long-time core
> member, I'm sure that he knows the rules -- but I think this patch
> will speed up works on replication.
>

That isn't really the point. This has nothing to do with Jan or Peter
or Dave. It has to do with procedure. Procedures should be followed and
followed equally for all. That didn't happen in this case.

> (Will all respect to pginstaller team, I'm *think* it won't take much
> time to add txid to installer, at least compared to the time that we
> spent discussing this issue.)

With respect, you don't know. My understanding of the pginstaller
project is that it is a fairly heft undertaking. It isn't as simple on
Linux as just calling to the OS level dependencies because library
versions etc..

>
> You know, txid was discussed in Slony-I + Skytools lists for a
> reasonably long time, and Tom also commented in that thread. I agree
> that we broke the policy this time, but this does not mean the end of
> the world.

This is irrelevant. It should have happen on -hackers.

>
> What we should to do is to prevent such things happening in the
> future, rather than reverting this patch and delaying replication
> issues.
>

There is no delay in what is being proposed.

Sincerely,

Joshua D. Drake

> Regards,


--
     === The PostgreSQL Company: Command Prompt, Inc. ===
Sales/Support: +1.503.667.4564   24x7/Emergency: +1.800.492.2240
PostgreSQL solutions since 1997  http://www.commandprompt.com/        UNIQUE NOT NULL
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
PostgreSQL Replication: http://www.commandprompt.com/products/

On Tue, 09 Oct 2007 17:07:29 -0700
Neil Conway <neilc@samurai.com> wrote:

> On Tue, 2007-10-09 at 16:50 -0700, Joshua D. Drake wrote:
> > I think this almost says it all. My particular gripe about this
> > whole thing is that there are other features that are not too
> > intrusive (or appear so anyway) that are easily more useful that
> > are not being considered at all. Namely,
> > http://archives.postgresql.org/pgsql-patches/2007-10/msg00087.php .
>
> That is NOT a good example. That patch is a first-cut of a non-trivial
> optimizer feature that was submitted just before beta1 shipped, by
> someone who hasn't modified the optimizer before.

That is certainly a fair assertion and perhaps my point wasn't as
clear as I wanted it to be. I in no way expect that we can or should
have the inline-SQL-SRF patch for 8.3.

I do however feel that the process should be equal for all and as
the process wasn't followed it sets a bad precedent.

> Jan's patch was a
> contrib module that has been already developed by the Skype folks, and
> it goes without saying that Jan has contributed to Postgres
> extensively.
>

Then there is no reason for it to be in contrib is there? It can be on
pgfoundry.

> That said, I agree that the process should have been followed in this
> case.

Yep.

Sincerely,

Joshua D. Drake



>
> -Neil
>
>
>
> ---------------------------(end of
> broadcast)--------------------------- TIP 6: explain analyze is your
> friend
>


--
     === The PostgreSQL Company: Command Prompt, Inc. ===
Sales/Support: +1.503.667.4564   24x7/Emergency: +1.800.492.2240
PostgreSQL solutions since 1997  http://www.commandprompt.com/        UNIQUE NOT NULL
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
PostgreSQL Replication: http://www.commandprompt.com/products/

Devrim GÜNDÜZ wrote:
> What we should to do is to prevent such things happening in the future,
> rather than reverting this patch and delaying replication issues.
>
>
>   

The next time the same sort of argument will be made. The way to prevent 
it in future is not to allow it now.

One of the things that people seem to fail to appreciate is that it 
really looks very bad for us when an insider does this. The message we 
send to non-insiders is just dreadful. If we want to encourage people to 
participate in development then we should all play by the same rules. 
One of the things I have found most attractive about the PostgreSQL 
community, quite apart from the technical excellence of our product, is 
the community's openness, fairness and egalitarianism. And I know I'm 
not alone in that. We should guard those qualities jealously.

cheers

andrew

Andrew Dunstan wrote:
> 
> 
> Devrim G?ND?Z wrote:
> > What we should to do is to prevent such things happening in the future,
> > rather than reverting this patch and delaying replication issues.
> >
> >
> >   
> 
> The next time the same sort of argument will be made. The way to prevent 
> it in future is not to allow it now.
> 
> One of the things that people seem to fail to appreciate is that it 
> really looks very bad for us when an insider does this. The message we 
> send to non-insiders is just dreadful. If we want to encourage people to 
> participate in development then we should all play by the same rules. 
> One of the things I have found most attractive about the PostgreSQL 
> community, quite apart from the technical excellence of our product, is 
> the community's openness, fairness and egalitarianism. And I know I'm 
> not alone in that. We should guard those qualities jealously.

Andrew is right that the appearance here is the biggest problem, and I
already emailed that to Jan privately.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://postgres.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

Ühel kenal päeval, T, 2007-10-09 kell 22:36, kirjutas Stefan
Kaltenbrunner:
> Bruce Momjian wrote:
> > Andrew Dunstan wrote:
> >>
> >> Bruce Momjian wrote:
> >>> Tom Lane wrote:
> >>>   
> >>>> Bruce Momjian <bruce@momjian.us> writes:
> >>>>     
> >>>>> I don't see how timing has anything to do with this.  You could have
> >>>>> added it between beta1 and beta2 after sufficient hackers discussion. 
> >>>>>       
> >>>> Uh, it *was* after beta1.
> >>>>     
> >>> Oh, so it didn't hold up beta1 --- that's good.
> >>>
> >>>   
> >> No it's not.
> >>
> >> Can somebody please explain to me what beta means if you can commit new 
> >> stuff after it has been declared?
> > 
> > We allow /contrib to be more lax about beta changes.
> 
> the postgresql ecosystem is growing and there is a lot of people like
> packagers that will be a quite irritated if we keep randomly adding
> completely new code and modules during BETA.

Should packagers be concerned with /contrib at all ?

As noted before /contrib is a technical way of ensuring that something
gets updated together with core, not a recommendation to include it in a
"package".

Arguably, a good packager should make its own decisions about what to
include in his/her/its packages, which may include stuff from pgfoundry
and also may omit stuff from /contrib .

----------------
Hannu

On Wed, 10 Oct 2007 06:25:49 +0300
Hannu Krosing <hannu@skype.net> wrote:

> As noted before /contrib is a technical way of ensuring that something
> gets updated together with core, not a recommendation to include it
> in a "package".
>
> Arguably, a good packager should make its own decisions about what to
> include in his/her/its packages, which may include stuff from
> pgfoundry and also may omit stuff from /contrib .
>

In a perfect world sure, in the real world... it gets packaged and
tested as postgresql-contrib :)

Joshua D. Drake


> ----------------
> Hannu
>
>
> ---------------------------(end of
> broadcast)--------------------------- TIP 9: In versions below 8.0,
> the planner will ignore your desire to choose an index scan if your
> joining column's datatypes do not match
>


--
     === The PostgreSQL Company: Command Prompt, Inc. ===
Sales/Support: +1.503.667.4564   24x7/Emergency: +1.800.492.2240
PostgreSQL solutions since 1997  http://www.commandprompt.com/        UNIQUE NOT NULL
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
PostgreSQL Replication: http://www.commandprompt.com/products/

Hi,

On Wed, 2007-10-10 at 06:25 +0300, Hannu Krosing wrote:
> Should packagers be concerned with /contrib at all ?

IMHO, yes. We (PGDG RPMs) ship contrib modules with a separate RPM, in
case someone wants to use it.

> Arguably, a good packager should make its own decisions about what to
> include in his/her/its packages, which may include stuff from
> pgfoundry and also may omit stuff from /contrib .

It is not packager's decision to add/remove software from PostgreSQL
code. A packager should be responsible to ship the software which is the
(almost) the same as PostgreSQL Core (of course we add some patches, but
they do not change the functionality, etc).

...for example, I built many pgfoundry modules as seperate packages (and
they will be available with a yum repository next week or so)

http://developer.postgresql.org/~devrim/rpms/other

Regards
--
Devrim GÜNDÜZ
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Managed Services, Shared and Dedicated Hosting
Co-Authors: plPHP, ODBCng - http://www.commandprompt.com/

Devrim GÜNDÜZ wrote:
> Hi,
> 
> On Tue, 2007-10-09 at 16:50 -0700, Joshua D. Drake wrote:
>> IMO, the patch is reverted, and submitted for 8.4 or pgfoundry.
> 
> You know, txid was discussed in Slony-I + Skytools lists for a
> reasonably long time, and Tom also commented in that thread. I agree
> that we broke the policy this time, but this does not mean the end of
> the world.

If it has been discussed and planned for so long then it should have 
been considered for inclusion earlier, not just slipped under the radar. 
Even if at feature freeze it wasn't ready it could have been discussed 
whether it could be added after feature freeze if it reached an 
acceptable standard.

If Slony or Skytools need this for a new feature in their x.y release 
then it can be a patch that is included with their release or be a 
prerequisite for their version x.y and detailed in their install steps.

Then they can discuss getting the change accepted into core or contrib 
for the next pg release.


just my .02c


-- 

Shane Ambler
pgSQL@Sheeky.Biz

Get Sheeky @ http://Sheeky.Biz

Simon Riggs wrote:

> I would prefer that we backported pg_standby into 8.2 contrib, so the
> solution is where people need it to be. If not...
> 
Don't know about the policy to put things in already-released-version
but if it's not the case, we could at least put the code somewhere in
the ftp.postgresql.org. IMHO pgfoundry project will confuse people.


--  Euler Taveira de Oliveira http://www.timbira.com/

Hi,

On Tue, 2007-10-09 at 17:10 -0700, Joshua D. Drake wrote:
> > (Will all respect to pginstaller team, I'm *think* it won't take
> much
> > time to add txid to installer, at least compared to the time that we
> > spent discussing this issue.)
>
> With respect, you don't know. My understanding of the pginstaller
> project is that it is a fairly heft undertaking. It isn't as simple on
> Linux as just calling to the OS level dependencies because library
> versions etc..

To avoid any problem/misunderstanding: I do never ever underestimate any
packaging work. I fully respect what Dave, Magnus, Hiroshi and others do
for pginstaller -- and it is the work that I personally cannot do. I
just tried to compare the amount of work from RPM perspective.

Regards,
--
Devrim GÜNDÜZ
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Managed Services, Shared and Dedicated Hosting
Co-Authors: plPHP, ODBCng - http://www.commandprompt.com/

On Wed, 2007-10-10 at 01:14 -0300, Euler Taveira de Oliveira wrote:
> Simon Riggs wrote:
> 
> > I would prefer that we backported pg_standby into 8.2 contrib, so the
> > solution is where people need it to be. If not...
> > 
> Don't know about the policy to put things in already-released-version
> but if it's not the case, we could at least put the code somewhere in
> the ftp.postgresql.org. IMHO pgfoundry project will confuse people.

Both: ftp and pgfoundry.

--  Simon Riggs 2ndQuadrant  http://www.2ndQuadrant.com

Devrim GÜNDÜZ wrote:
> (Will all respect to pginstaller team, I'm *think* it won't take much
> time to add txid to installer, at least compared to the time that we
> spent discussing this issue.)

Time is not the issue.

/D

On 10/10/07, Joshua D. Drake <jd@commandprompt.com> wrote:
> On Tue, 9 Oct 2007 18:35:52 -0500
> Michael Glaesemann <grzm@seespotcode.net> wrote:
> > On Oct 9, 2007, at 0:06 , Bruce Momjian wrote:
> > > I am surprised we are not backing
> > > out the patch and requiring that the patch go through the formal
> > > review
> > > process.
> >
> > I have no opinion as to the patch itself (other than the fact that
> > it's a not bug fix), but I think this patch should be reverted
> > because it's (a) after feature freeze, (b) had no discussion on
> > hackers (or patches), (c) is not a bug fix. IMO rules can be bent
> > but there should always at least be discussion before a new feature
> > is committed after feature freeze and definitely after beta.
> > Otherwise, the rule appears to be if you can get it in somehow, it's
> > in.
>
> I think this almost says it all. My particular gripe about this whole
> thing is that there are other features that are not too intrusive (or
> appear so anyway) that are easily more useful that are not being
> considered at all. Namely,
> http://archives.postgresql.org/pgsql-patches/2007-10/msg00087.php . It
> makes the whole process seem tilted and subjective.
>
> IMO, the patch is reverted, and submitted for 8.4 or pgfoundry.

Yes, reverting is an option, but please, do that at least with
an understanding what actually happened.  Current discussion
seems to give picture that Jan committed some private piece of
code without consulting anybody which was not the case.

It was actually my patch that was reviewed by 2 senior PostgreSQL
developers: Jan and Tom, then later committed by Jan.  I don't
think the fact that Jan was an interested party by being Slony
developer invalidates his status as PostgreSQL developer.

Obviously that does not make skipping -hackers less mistake,
but there was no evil from anybody and the "process" for such
exceptional case was _mostly_ followed.

Now the skipping -hackers part - that was also my mistake,
I should have Cc-d the design and code review discussion here
also.  I just saw the contrib-acceptance as minor question,
the main issue was whether Slony was prepared to such a major
rewrite of its core parts on such short notice, so I wanted
to sync with them first.

Also I think several people are annoyed by the "Jan asked permission
from -core" part of the process.  But I think if you replace the
-core with "release manager" it will become more understandable.
The fact is there are only few people responsible for releases and
non-technical decisions need to be made by them.  And yes, it should
have been accompanied by technical review in -hackers.

-- 
marko

On 10/10/2007 12:05 AM, Shane Ambler wrote:
> Devrim GÜNDÜZ wrote:
>> Hi,
>> 
>> On Tue, 2007-10-09 at 16:50 -0700, Joshua D. Drake wrote:
>>> IMO, the patch is reverted, and submitted for 8.4 or pgfoundry.
>> 
>> You know, txid was discussed in Slony-I + Skytools lists for a
>> reasonably long time, and Tom also commented in that thread. I agree
>> that we broke the policy this time, but this does not mean the end of
>> the world.
> 
> If it has been discussed and planned for so long then it should have 
> been considered for inclusion earlier, not just slipped under the radar. 
> Even if at feature freeze it wasn't ready it could have been discussed 
> whether it could be added after feature freeze if it reached an 
> acceptable standard.
> 
> If Slony or Skytools need this for a new feature in their x.y release 
> then it can be a patch that is included with their release or be a 
> prerequisite for their version x.y and detailed in their install steps.

There was no intend to "slip it in under the radar". Discussion had 
happened and I failed to realize at the time the code actually looked 
good that the discussion had happened somewhere other than -hackers.

I can certainly take a good amount of flak, especially considering that 
there was a fault on my side. But what is going on right now here is 
getting annoying.

Also Slony doesn't need this module. We can certainly wait until we are 
forced by other reasons to bump Slony to version 3.0, declare 3.0 
incompatible with 8.3 and switch to using txid then. Slony can continue 
using its own xxid data type until then. No problem here.


Jan

-- 
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.                                  #
#================================================== JanWieck@Yahoo.com #

On Wed, 2007-10-10 at 11:50 +0300, Marko Kreen wrote:
> On 10/10/07, Joshua D. Drake <jd@commandprompt.com> wrote:

> > IMO, the patch is reverted, and submitted for 8.4 or pgfoundry.
> 
> Yes, reverting is an option

Reverting is only an option if we need to solve a technical problem. If
there is no problem then why would we revert it? The only argument I've
seen for reverting the patch is that it broke a process. 

It's hard enough for Developers to get code in without a team of
Anti-Developers trying to take it back out again. Perhaps we should have
an anti-credits section in the release notes to allow all those who've
managed to get work removed get full credit for their anti-efforts. :-)

--  Simon Riggs 2ndQuadrant  http://www.2ndQuadrant.com

On Wed, Oct 10, 2007 at 11:50:12AM +0300, Marko Kreen wrote:
> On 10/10/07, Joshua D. Drake <jd@commandprompt.com> wrote:
> > On Tue, 9 Oct 2007 18:35:52 -0500
> > Michael Glaesemann <grzm@seespotcode.net> wrote:
> > > On Oct 9, 2007, at 0:06 , Bruce Momjian wrote:
> > > > I am surprised we are not backing
> > > > out the patch and requiring that the patch go through the formal
> > > > review
> > > > process.
> > >
> > > I have no opinion as to the patch itself (other than the fact that
> > > it's a not bug fix), but I think this patch should be reverted
> > > because it's (a) after feature freeze, (b) had no discussion on
> > > hackers (or patches), (c) is not a bug fix. IMO rules can be bent
> > > but there should always at least be discussion before a new feature
> > > is committed after feature freeze and definitely after beta.
> > > Otherwise, the rule appears to be if you can get it in somehow, it's
> > > in.
> >
> > I think this almost says it all. My particular gripe about this whole
> > thing is that there are other features that are not too intrusive (or
> > appear so anyway) that are easily more useful that are not being
> > considered at all. Namely,
> > http://archives.postgresql.org/pgsql-patches/2007-10/msg00087.php . It
> > makes the whole process seem tilted and subjective.
> >
> > IMO, the patch is reverted, and submitted for 8.4 or pgfoundry.
> 
> Yes, reverting is an option, but please, do that at least with
> an understanding what actually happened.  Current discussion
> seems to give picture that Jan committed some private piece of
> code without consulting anybody which was not the case.

At least I am fully aware that it's not a private piece of code. And in
general, I trust Jan (and of course Tom as well) to take a patch from
elsewhere and put it in.

My objections are twofold:

1) We don't add things after beta. I can live with adding it during feature
freeze since it's contrib, and reviewed by these people, but I think it's
horrible to do it after we've shipped beta1.

2) I get the strong feeling that it's going into contrib only because it
missed feature freeze. If it hadn't missed feature freeze, it wuold be in
the backend and not contrib. If the plan is that it lives in contrib
forever, that argument falls. But if the plan is to migrate it into the
backend for 8.4, then I strongly object to using contrib just as a way to
"get it in even though we're feature-frozen".


> It was actually my patch that was reviewed by 2 senior PostgreSQL
> developers: Jan and Tom, then later committed by Jan.  I don't
> think the fact that Jan was an interested party by being Slony
> developer invalidates his status as PostgreSQL developer.

Certainly not. 

> Obviously that does not make skipping -hackers less mistake,
> but there was no evil from anybody and the "process" for such
> exceptional case was _mostly_ followed.

I don't think anybody thinks there were "evil intentions" behind it. I
certainly don't think Jan would've committed it if he expected people to
dislike it technically. 

All objections have been procedural, AFICS.


> Now the skipping -hackers part - that was also my mistake,
> I should have Cc-d the design and code review discussion here
> also.  I just saw the contrib-acceptance as minor question,
> the main issue was whether Slony was prepared to such a major
> rewrite of its core parts on such short notice, so I wanted
> to sync with them first.
> 
> Also I think several people are annoyed by the "Jan asked permission
> from -core" part of the process.  But I think if you replace the
> -core with "release manager" it will become more understandable.
> The fact is there are only few people responsible for releases and
> non-technical decisions need to be made by them.  And yes, it should
> have been accompanied by technical review in -hackers.

AFAIK, our "release manager" is -hackers concensus. We don't *have* a
release manager as such. The closest thing you'd get in that case is the
-packagers list which is for those building the binary packages, and they
were not consulted...

But again, I don't see any issues with the technical side of things. It's
procedural, and placement (is contrib really the right place for it).

//Magnus

Magnus Hagander wrote:
> On Wed, Oct 10, 2007 at 11:50:12AM +0300, Marko Kreen wrote:
>> On 10/10/07, Joshua D. Drake <jd@commandprompt.com> wrote:
>>> On Tue, 9 Oct 2007 18:35:52 -0500
>>> Michael Glaesemann <grzm@seespotcode.net> wrote:
>>>> On Oct 9, 2007, at 0:06 , Bruce Momjian wrote:
>>>>> I am surprised we are not backing
>>>>> out the patch and requiring that the patch go through the formal
>>>>> review
>>>>> process.
>>>> I have no opinion as to the patch itself (other than the fact that
>>>> it's a not bug fix), but I think this patch should be reverted
>>>> because it's (a) after feature freeze, (b) had no discussion on
>>>> hackers (or patches), (c) is not a bug fix. IMO rules can be bent
>>>> but there should always at least be discussion before a new feature
>>>> is committed after feature freeze and definitely after beta.
>>>> Otherwise, the rule appears to be if you can get it in somehow, it's
>>>> in.
>>> I think this almost says it all. My particular gripe about this whole
>>> thing is that there are other features that are not too intrusive (or
>>> appear so anyway) that are easily more useful that are not being
>>> considered at all. Namely,
>>> http://archives.postgresql.org/pgsql-patches/2007-10/msg00087.php . It
>>> makes the whole process seem tilted and subjective.
>>>
>>> IMO, the patch is reverted, and submitted for 8.4 or pgfoundry.
>> Yes, reverting is an option, but please, do that at least with
>> an understanding what actually happened.  Current discussion
>> seems to give picture that Jan committed some private piece of
>> code without consulting anybody which was not the case.
> 
> At least I am fully aware that it's not a private piece of code. And in
> general, I trust Jan (and of course Tom as well) to take a patch from
> elsewhere and put it in.
> 
> My objections are twofold:
> 
> 1) We don't add things after beta. I can live with adding it during feature
> freeze since it's contrib, and reviewed by these people, but I think it's
> horrible to do it after we've shipped beta1.

yeah that is exactly the point - if we do have a feature freeze we 
should hold to it. if we are in BETA we should not add any new code.

> 
> 2) I get the strong feeling that it's going into contrib only because it
> missed feature freeze. If it hadn't missed feature freeze, it wuold be in
> the backend and not contrib. If the plan is that it lives in contrib
> forever, that argument falls. But if the plan is to migrate it into the
> backend for 8.4, then I strongly object to using contrib just as a way to
> "get it in even though we're feature-frozen".

yeah I agree that code like this should be either in core or somewhere 
else (either pgfoundry or even shipped as part of the replication 
solutions mentioned which is basically something slony did for ages with 
the xxid stuff). Just pushing it now into contrib results in people 
wanting to use one of those solution having to deal with 3 kinds of 
packages:

1. postgresql
2. postgresql-contrib
3. skytools/slony/...

instead of just two which does not strike me as much of an improvement.


Stefan

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1



- --On Wednesday, October 10, 2007 07:09:20 +0100 Simon Riggs 
<simon@2ndquadrant.com> wrote:

> On Wed, 2007-10-10 at 01:14 -0300, Euler Taveira de Oliveira wrote:
>> Simon Riggs wrote:
>>
>> > I would prefer that we backported pg_standby into 8.2 contrib, so the
>> > solution is where people need it to be. If not...
>> >
>> Don't know about the policy to put things in already-released-version
>> but if it's not the case, we could at least put the code somewhere in
>> the ftp.postgresql.org. IMHO pgfoundry project will confuse people.
>
> Both: ftp and pgfoundry.

ftp://ftp.postgresql.org/pub/projects/{gborg,pgFoundry}

- ----
Marc G. Fournier           Hub.Org Networking Services (http://www.hub.org)
Email . scrappy@hub.org                              MSN . scrappy@hub.org
Yahoo . yscrappy               Skype: hub.org        ICQ . 7615664
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.4 (FreeBSD)

iD8DBQFHDLxU4QvfyHIvDvMRAjkCAJ9nN3JxEV/drYMO7uMN2uH1phhJbwCgiCKN
etp2sBtJoBTtqoAVUgLSkzw=
=yAQh
-----END PGP SIGNATURE-----

On 10/10/07, Magnus Hagander <magnus@hagander.net> wrote:
> All objections have been procedural, AFICS.

Lets not talk about mistakes we made for a moment.

And I agree with the rest of the objections in general. But I'd
like to summarise why I still hope the exception can be made
even this late.

This is directly related to attitude to the first submission to 8.2:
"unless Slony uses it we are not interested".  Now is the only
moment which won't come again in several years that it's possible
to unify txid handling in Slony and Skytools and also make the
functionality available to broader public.

This due to the fact that Slony 2.0 which will be released with 8.3
will not support PostgreSQL version lower then 8.3.

Yes, we realized the opportunity too late and now it's question
if PostgreSQL is flexible enough to react to this.

Note that rejection now does not cause any big problems to either
Slony and Skytools, we will keep our internal versions of the module,
invisible to anybody else.

But the potential use of the module is huge - it's killer feature is
that it allows to implement high-performance multi-reader / multi-writer
queues inside database.  Well, I know this sounds unimpressive, queues
do not belong to standard toolbox when doing database developement.
And those who have tried to implement them, carry a "avoid at any cost" tag,
because thus far there has not been a way to implement robust and
well-perfoming queue inside general-purpose database.

Now txid can change that.  E.g. in Skype, it has become irreplaceable
tool for coordinating work between several databases.  Here we are
probably going overboard with usage of queues...

-- 
marko

On Wed, Oct 10, 2007 at 03:27:17PM +0300, Marko Kreen wrote:
> On 10/10/07, Magnus Hagander <magnus@hagander.net> wrote:
> > All objections have been procedural, AFICS.
> 
> Lets not talk about mistakes we made for a moment.
> 
> And I agree with the rest of the objections in general. But I'd
> like to summarise why I still hope the exception can be made
> even this late.
> 
> This is directly related to attitude to the first submission to 8.2:
> "unless Slony uses it we are not interested".  Now is the only
> moment which won't come again in several years that it's possible
> to unify txid handling in Slony and Skytools and also make the
> functionality available to broader public.
> 
> This due to the fact that Slony 2.0 which will be released with 8.3
> will not support PostgreSQL version lower then 8.3.
> 
> Yes, we realized the opportunity too late and now it's question
> if PostgreSQL is flexible enough to react to this.
> 
> Note that rejection now does not cause any big problems to either
> Slony and Skytools, we will keep our internal versions of the module,
> invisible to anybody else.
> 
> But the potential use of the module is huge - it's killer feature is
> that it allows to implement high-performance multi-reader / multi-writer
> queues inside database.  Well, I know this sounds unimpressive, queues
> do not belong to standard toolbox when doing database developement.
> And those who have tried to implement them, carry a "avoid at any cost" tag,
> because thus far there has not been a way to implement robust and
> well-perfoming queue inside general-purpose database.
> 
> Now txid can change that.  E.g. in Skype, it has become irreplaceable
> tool for coordinating work between several databases.  Here we are
> probably going overboard with usage of queues...

If it is this irreplacable killer feature, it should *not* be in contrib.
It should be in the core backend, and we should be discussing if we can
bend the rules for that. This is the proper forum for discussing that, so
let's bring that question to the table.

Our beta-1 is already fairly broken (the locale stuff on our most
downloaded platform), so perhaps we should pull that one back, put this
stuff in the backend, and try to get a beta2 out ASAP?


Putting it in contrib "just because we were too late to put it in the
backend, but it is reallyi really important for our users" just doesn't
make sense.

//Magnus

Magnus Hagander wrote:

> If it is this irreplacable killer feature, it should *not* be in contrib.
> It should be in the core backend, and we should be discussing if we can
> bend the rules for that. This is the proper forum for discussing that, so
> let's bring that question to the table.

+1 there, I don't think it should go into contrib just cause it was a 
late entry. It really seems to be a matter of whether it gets into 8.3 
or 8.4

> Our beta-1 is already fairly broken (the locale stuff on our most
> downloaded platform), so perhaps we should pull that one back, put this
> stuff in the backend, and try to get a beta2 out ASAP?
> 

The question there is how long will it take to reach a decision of where 
the patch belongs? (8.3 8.4 or contrib)

> Putting it in contrib "just because we were too late to put it in the
> backend, but it is reallyi really important for our users" just doesn't
> make sense.
> 
+1



-- 

Shane Ambler
pgSQL@Sheeky.Biz

Get Sheeky @ http://Sheeky.Biz

Marko Kreen wrote:
> On 10/10/07, Joshua D. Drake <jd@commandprompt.com> wrote:
> > On Tue, 9 Oct 2007 18:35:52 -0500
> > Michael Glaesemann <grzm@seespotcode.net> wrote:
> > > On Oct 9, 2007, at 0:06 , Bruce Momjian wrote:
> > > > I am surprised we are not backing
> > > > out the patch and requiring that the patch go through the formal
> > > > review
> > > > process.
> > >
> > > I have no opinion as to the patch itself (other than the fact that
> > > it's a not bug fix), but I think this patch should be reverted
> > > because it's (a) after feature freeze, (b) had no discussion on
> > > hackers (or patches), (c) is not a bug fix. IMO rules can be bent
> > > but there should always at least be discussion before a new feature
> > > is committed after feature freeze and definitely after beta.
> > > Otherwise, the rule appears to be if you can get it in somehow, it's
> > > in.
> >
> > I think this almost says it all. My particular gripe about this whole
> > thing is that there are other features that are not too intrusive (or
> > appear so anyway) that are easily more useful that are not being
> > considered at all. Namely,
> > http://archives.postgresql.org/pgsql-patches/2007-10/msg00087.php . It
> > makes the whole process seem tilted and subjective.
> >
> > IMO, the patch is reverted, and submitted for 8.4 or pgfoundry.
> 
> Yes, reverting is an option, but please, do that at least with
> an understanding what actually happened.  Current discussion
> seems to give picture that Jan committed some private piece of
> code without consulting anybody which was not the case.
> 
> It was actually my patch that was reviewed by 2 senior PostgreSQL
> developers: Jan and Tom, then later committed by Jan.  I don't
> think the fact that Jan was an interested party by being Slony
> developer invalidates his status as PostgreSQL developer.
> 
> Obviously that does not make skipping -hackers less mistake,
> but there was no evil from anybody and the "process" for such
> exceptional case was _mostly_ followed.
> 
> Now the skipping -hackers part - that was also my mistake,
> I should have Cc-d the design and code review discussion here
> also.  I just saw the contrib-acceptance as minor question,
> the main issue was whether Slony was prepared to such a major
> rewrite of its core parts on such short notice, so I wanted
> to sync with them first.
> 
> Also I think several people are annoyed by the "Jan asked permission
> from -core" part of the process.  But I think if you replace the
> -core with "release manager" it will become more understandable.
> The fact is there are only few people responsible for releases and
> non-technical decisions need to be made by them.  And yes, it should
> have been accompanied by technical review in -hackers.

I don't think this is accurate.  Jan talked to Tom, not all of core, and
Tom just gave general approval.  Tom still expected this to go through
the hackers review process.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://postgres.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

Agreed.  I think if we had followed procedure the code would have been
accepted post-beta1.

---------------------------------------------------------------------------

Marko Kreen wrote:
> On 10/10/07, Magnus Hagander <magnus@hagander.net> wrote:
> > All objections have been procedural, AFICS.
> 
> Lets not talk about mistakes we made for a moment.
> 
> And I agree with the rest of the objections in general. But I'd
> like to summarise why I still hope the exception can be made
> even this late.
> 
> This is directly related to attitude to the first submission to 8.2:
> "unless Slony uses it we are not interested".  Now is the only
> moment which won't come again in several years that it's possible
> to unify txid handling in Slony and Skytools and also make the
> functionality available to broader public.
> 
> This due to the fact that Slony 2.0 which will be released with 8.3
> will not support PostgreSQL version lower then 8.3.
> 
> Yes, we realized the opportunity too late and now it's question
> if PostgreSQL is flexible enough to react to this.
> 
> Note that rejection now does not cause any big problems to either
> Slony and Skytools, we will keep our internal versions of the module,
> invisible to anybody else.
> 
> But the potential use of the module is huge - it's killer feature is
> that it allows to implement high-performance multi-reader / multi-writer
> queues inside database.  Well, I know this sounds unimpressive, queues
> do not belong to standard toolbox when doing database developement.
> And those who have tried to implement them, carry a "avoid at any cost" tag,
> because thus far there has not been a way to implement robust and
> well-perfoming queue inside general-purpose database.
> 
> Now txid can change that.  E.g. in Skype, it has become irreplaceable
> tool for coordinating work between several databases.  Here we are
> probably going overboard with usage of queues...
> 
> -- 
> marko

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://postgres.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

On Wednesday 10 October 2007 02:09, Simon Riggs wrote:
> On Wed, 2007-10-10 at 01:14 -0300, Euler Taveira de Oliveira wrote:
> > Simon Riggs wrote:
> > > I would prefer that we backported pg_standby into 8.2 contrib, so the
> > > solution is where people need it to be. If not...
> >
> > Don't know about the policy to put things in already-released-version
> > but if it's not the case, we could at least put the code somewhere in
> > the ftp.postgresql.org. IMHO pgfoundry project will confuse people.
>
> Both: ftp and pgfoundry.

Putting it on pgfoundry would automatically put it in the ftp tree
(ftp://ftp.postgresql.org/pub/projects/pgFoundry).  If it was to go on
pgfoundry (which I'd recommend) I'd suggest removing it from 8.3 contrib
before we release (cause having it in both places is really going to cause
confusion)

--
Robert Treat
Build A Brighter LAMP :: Linux Apache {middleware} PostgreSQL

Magnus Hagander wrote:
> > Now txid can change that.  E.g. in Skype, it has become irreplaceable
> > tool for coordinating work between several databases.  Here we are
> > probably going overboard with usage of queues...
> 
> If it is this irreplacable killer feature, it should *not* be in contrib.
> It should be in the core backend, and we should be discussing if we can
> bend the rules for that. This is the proper forum for discussing that, so
> let's bring that question to the table.
> 
> Our beta-1 is already fairly broken (the locale stuff on our most
> downloaded platform), so perhaps we should pull that one back, put this
> stuff in the backend, and try to get a beta2 out ASAP?
> 
> 
> Putting it in contrib "just because we were too late to put it in the
> backend, but it is reallyi really important for our users" just doesn't
> make sense.

I also agree with this.  We have to pretend it isn't in /contrib now,
figure out where want it, then put it there (contrib, pgfoundry, core).

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://postgres.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

Magnus Hagander wrote:
> If it is this irreplacable killer feature, it should *not* be in contrib.
> It should be in the core backend, and we should be discussing if we can
> bend the rules for that. This is the proper forum for discussing that, so
> let's bring that question to the table.
>
> Our beta-1 is already fairly broken (the locale stuff on our most
> downloaded platform), so perhaps we should pull that one back, put this
> stuff in the backend, and try to get a beta2 out ASAP?
>
>
> Putting it in contrib "just because we were too late to put it in the
> backend, but it is reallyi really important for our users" just doesn't
> make sense.
>
>
>   

I agree with most of this. If it's really that important we should 
abandon beta1 in effect and open the tree for just this and as much of 
the extra tsearch samples as we care to take. But that's a decision that 
should be taken openly, after discussion on -hackers. I don't think I'm 
being arrogant when I say that if I was completely unaware of this 
proposal then a great many other people probably were too.

cheers

andrew

Robert Treat wrote:
> On Wednesday 10 October 2007 02:09, Simon Riggs wrote:
>   
>> On Wed, 2007-10-10 at 01:14 -0300, Euler Taveira de Oliveira wrote:
>>     
>>> Simon Riggs wrote:
>>>       
>>>> I would prefer that we backported pg_standby into 8.2 contrib, so the
>>>> solution is where people need it to be. If not...
>>>>         
>>> Don't know about the policy to put things in already-released-version
>>> but if it's not the case, we could at least put the code somewhere in
>>> the ftp.postgresql.org. IMHO pgfoundry project will confuse people.
>>>       
>> Both: ftp and pgfoundry.
>>     
>
> Putting it on pgfoundry would automatically put it in the ftp tree 
> (ftp://ftp.postgresql.org/pub/projects/pgFoundry).  If it was to go on 
> pgfoundry (which I'd recommend) I'd suggest removing it from 8.3 contrib 
> before we release (cause having it in both places is really going to cause 
> confusion)
>
>   

One of pgfoundry's explicit purposes is for backports of features. Given 
that we (rightly) don't backport new features in mainline releases, 
where else should they go? I don't buy the "confusing" argument. If 
necessary the author can plaster big red notices in a README on the 
pgfoundry release saying "don't use this past postgres version x"

cheers

andrew

Magnus Hagander <magnus@hagander.net> writes:
> On Wed, Oct 10, 2007 at 03:27:17PM +0300, Marko Kreen wrote:
>> Now txid can change that.  E.g. in Skype, it has become irreplaceable
>> tool for coordinating work between several databases.  Here we are
>> probably going overboard with usage of queues...

> If it is this irreplacable killer feature, it should *not* be in contrib.
> It should be in the core backend, and we should be discussing if we can
> bend the rules for that.

It may be a killer feature for Skytools and Slony, but even so that's a
small part of our userbase.  I see nothing wrong with having it in
contrib now with an eye to migrating to the core later, when and if we
see there's enough demand for that.  Another reason for that approach
is that once it's in core it will be very much harder to make any tweaks
to the API; and with the prospective uses being largely unwritten as
yet, it hardly seems unlikely we might not want some changes.

I think our two realistic options today are (1) leave the code where
it is, or (2) remove it.  While Jan clearly failed to follow the agreed
procedures, I'm not convinced the transgression was severe enough to
justify (2).
        regards, tom lane

Bruce Momjian <bruce@momjian.us> writes:
> Marko Kreen wrote:
>> Also I think several people are annoyed by the "Jan asked permission
>> from -core" part of the process.

> I don't think this is accurate.  Jan talked to Tom, not all of core, and
> Tom just gave general approval.  Tom still expected this to go through
> the hackers review process.

Well, my view of the discussion was that Jan asked core if any of us
would veto a late contrib addition.  I trust you'll agree that if anyone
on core were to vote against, it would not have gone in; and so it
seemed reasonable to me for Jan to check this before expending any
further effort on creating a patch.

I asked a couple questions and then said it sounded okay to me; I don't
recall now if anyone else commented, but this was definitely a
discussion on -core not personal email.

In any case, since that was in advance of seeing the code, I certainly
was expecting a -patches submission before commit ...
        regards, tom lane

Bruce Momjian <bruce@momjian.us> writes:
> I also agree with this.  We have to pretend it isn't in /contrib now,
> figure out where want it, then put it there (contrib, pgfoundry, core).

Putting it in core now would mean forcing a post-beta1 initdb, which
I don't think adequate cause has been shown for.

Possibly we should sit on the decision for awhile and see if any
initdb-forcing bugs are reported.  But for the moment I think only the
contrib or pgfoundry options are acceptable.
        regards, tom lane

On 10/10/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Magnus Hagander <magnus@hagander.net> writes:
> > On Wed, Oct 10, 2007 at 03:27:17PM +0300, Marko Kreen wrote:
> >> Now txid can change that.  E.g. in Skype, it has become irreplaceable
> >> tool for coordinating work between several databases.  Here we are
> >> probably going overboard with usage of queues...
>
> > If it is this irreplacable killer feature, it should *not* be in contrib.
> > It should be in the core backend, and we should be discussing if we can
> > bend the rules for that.
>
> It may be a killer feature for Skytools and Slony, but even so that's a
> small part of our userbase.  I see nothing wrong with having it in
> contrib now with an eye to migrating to the core later, when and if we
> see there's enough demand for that.  Another reason for that approach
> is that once it's in core it will be very much harder to make any tweaks
> to the API; and with the prospective uses being largely unwritten as
> yet, it hardly seems unlikely we might not want some changes.

Considering the core operations are now being in active use
some 6-7 years, I really fail to see how there can be anything
to tweak, unless you are speaking changing naming style.

IMHO the core operations are already as stable as PostgreSQL use
of MVCC, as the module just exports backend internal state...
Current set of functions is the minimal necessary to implement
queue operations, there is nothing more to remove.

We might want to add some helper functions for query generation,
but that does not affect core operation.

Another thing can can be done is more compact representation for
txid_snapshot type, but that also won't affect core operation.

I am very happy for txid being in contrib, I'm not arguing against
that, but the hint that txid module is something that just freshly
popped out of thin air is irking me.

> I think our two realistic options today are (1) leave the code where
> it is, or (2) remove it.  While Jan clearly failed to follow the agreed
> procedures, I'm not convinced the transgression was severe enough to
> justify (2).

Thanks for being understanding.

-- 
marko

On Wed, Oct 10, 2007 at 11:30:47AM -0400, Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > I also agree with this.  We have to pretend it isn't in /contrib now,
> > figure out where want it, then put it there (contrib, pgfoundry, core).
> 
> Putting it in core now would mean forcing a post-beta1 initdb, which
> I don't think adequate cause has been shown for.

Ok. In that case, my vote is pgfoundry (heh, I'm sure that's clear by now).
I don't think an adequate cause to break all our procedures to stick it in
core has been shown either.


> Possibly we should sit on the decision for awhile and see if any
> initdb-forcing bugs are reported.  But for the moment I think only the
> contrib or pgfoundry options are acceptable.

This sounds like a good fallback - if the option opens up, I really think
it should be put in the backend. (Assuming it's technically sound - I still
haven't checked the actual code, but I'm assuming it's Ok since Jan
approved it)

//Magnus

"Marko Kreen" <markokr@gmail.com> writes:
> IMHO the core operations are already as stable as PostgreSQL use
> of MVCC, as the module just exports backend internal state...

Well, it exports backend internal state that did not exist before 8.2
(ie, XID epoch).  So it doesn't seem all that set in stone to me.

> Another thing can can be done is more compact representation for
> txid_snapshot type, but that also won't affect core operation.

That's another thing that's likely to become very much harder to change
once it's in core.  People keep threatening to produce a working
in-place-upgrade process, and once that's reality the on-disk
representation of core types is going to be hard to change.
        regards, tom lane

On Wednesday 10 October 2007 10:57, Andrew Dunstan wrote:
> Robert Treat wrote:
> > On Wednesday 10 October 2007 02:09, Simon Riggs wrote:
> >> On Wed, 2007-10-10 at 01:14 -0300, Euler Taveira de Oliveira wrote:
> >>> Simon Riggs wrote:
> >>>> I would prefer that we backported pg_standby into 8.2 contrib, so the
> >>>> solution is where people need it to be. If not...
> >>>
> >>> Don't know about the policy to put things in already-released-version
> >>> but if it's not the case, we could at least put the code somewhere in
> >>> the ftp.postgresql.org. IMHO pgfoundry project will confuse people.
> >>
> >> Both: ftp and pgfoundry.
> >
> > Putting it on pgfoundry would automatically put it in the ftp tree
> > (ftp://ftp.postgresql.org/pub/projects/pgFoundry).  If it was to go on
> > pgfoundry (which I'd recommend) I'd suggest removing it from 8.3 contrib
> > before we release (cause having it in both places is really going to
> > cause confusion)
>
> One of pgfoundry's explicit purposes is for backports of features. 

I can't think of any contrib modules we've added that also required backwards 
comptible modules to be released on foundry at the same time.  ISTM that such 
a requirement would be an argument that such a thing doesn't belong in 
contrib at all. 

-- 
Robert Treat
Build A Brighter LAMP :: Linux Apache {middleware} PostgreSQL

On 10/10/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> "Marko Kreen" <markokr@gmail.com> writes:
> > IMHO the core operations are already as stable as PostgreSQL use
> > of MVCC, as the module just exports backend internal state...
>
> Well, it exports backend internal state that did not exist before 8.2
> (ie, XID epoch).  So it doesn't seem all that set in stone to me.

Ok.  Lets say the API concepts are 6-7 years old.  The epoch was
a only thing missing in API ca. 2 years ago (the 8byte txid was
written on 8.0), so now the API is complete.

> > Another thing can can be done is more compact representation for
> > txid_snapshot type, but that also won't affect core operation.
>
> That's another thing that's likely to become very much harder to change
> once it's in core.  People keep threatening to produce a working
> in-place-upgrade process, and once that's reality the on-disk
> representation of core types is going to be hard to change.

Good point.  But txid_snapshot happens to have couple of free
bits that can be used to create backwards compatibility, so I
don't think that could be a big problem.

Anyway, the in-place upgrade seems a month-two away couple of
years now :)

-- 
marko

Magnus Hagander <magnus@hagander.net> writes:
> (Assuming it's technically sound - I still haven't checked the actual
> code, but I'm assuming it's Ok since Jan approved it)

I hadn't looked at it either, but here are a few things that need
review:

* Why no binary I/O support for the new datatype?  We tend to expect
that for all core types.

* Why is txid_current_snapshot() excluding subtransaction XIDs?  That
might be all right for the current uses in Slony/Skytools, but it seems
darn close to a bug for any other use.

* Why is txid_current_snapshot() reading SerializableSnapshot rather
than an actually current snap as its name suggests?  This isn't just
misleading, this will fail completely when SerializableSnapshot
goes away, as seems likely to happen in 8.4 (and no, we won't keep it
just because txid might want it).
        regards, tom lane

On Wed, Oct 10, 2007 at 11:47:15AM -0400, Tom Lane wrote:
> "Marko Kreen" <markokr@gmail.com> writes:
> > IMHO the core operations are already as stable as PostgreSQL use
> > of MVCC, as the module just exports backend internal state...
> 
> Well, it exports backend internal state that did not exist before 8.2
> (ie, XID epoch).  So it doesn't seem all that set in stone to me.
> 
> > Another thing can can be done is more compact representation for
> > txid_snapshot type, but that also won't affect core operation.
> 
> That's another thing that's likely to become very much harder to change
> once it's in core.  People keep threatening to produce a working
> in-place-upgrade process, and once that's reality the on-disk
> representation of core types is going to be hard to change.

Well, if that is a concern, than it's an equally big concern to have it in
contrib. If people start using it, they're not going to care about us
saying "hey, it was in contrib, why did you use it", when we earlier said
"in order do use our whiz-bang stuff, you must install from contrib". We'll
have complaints that it's too hard to install, but we won't manage to
escape from the responsibility to keep it working.

//Magnus

Robert Treat <xzilla@users.sourceforge.net> writes:
> On Wednesday 10 October 2007 10:57, Andrew Dunstan wrote:
>> One of pgfoundry's explicit purposes is for backports of features. 

> I can't think of any contrib modules we've added that also required
> backwards comptible modules to be released on foundry at the same
> time.  ISTM that such a requirement would be an argument that such a
> thing doesn't belong in contrib at all.

AFAICT there isn't any market for a backport of txid.  Slony won't
depend on it before their next release, which will require PG >= 8.3
for other reasons.  Skytools already has an internal version in their
existing releases.  And the code won't work before PG 8.2 so any
backport couldn't go very far anyway.

So while Andrew's statement is true in general, I don't think
it's very relevant to a consideration of what to do with txid.
        regards, tom lane

"Magnus Hagander" <magnus@hagander.net> writes:

> On Wed, Oct 10, 2007 at 11:30:47AM -0400, Tom Lane wrote:
>> Bruce Momjian <bruce@momjian.us> writes:
>> > I also agree with this.  We have to pretend it isn't in /contrib now,
>> > figure out where want it, then put it there (contrib, pgfoundry, core).
>> 
>> Putting it in core now would mean forcing a post-beta1 initdb, which
>> I don't think adequate cause has been shown for.
>
> Ok. In that case, my vote is pgfoundry (heh, I'm sure that's clear by now).
> I don't think an adequate cause to break all our procedures to stick it in
> core has been shown either.

I just don't see the point in putting it in pgfoundry. It's already in
pgfoundry as part of Skytools. The whole point of having such a datatype is to
build common interface to abstract away the internals of the database. That
makes the pgfoundry modules (Skytools and Slony) easier to maintain
separately. Putting txid on pgfoundry just means splitting one pgfoundry
package into two. Moving code around doesn't make it any easier to maintain,
the portion that depends on the database internals will still depend on
database internals.

Putting it in core or contrib means that when we change the snapshot mechanics
in 8.4 the same developer will be able to fix the module at the same time (and
find out if his changes break it at the same time). Also different versions of
Postgres would include appropriate txid modules without having to maintain a
single version with a million ifdefs to cover multiple versions of Postgres.

--  Gregory Stark EnterpriseDB          http://www.enterprisedb.com

On Wed, 10 Oct 2007 11:04:53 -0400
Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Magnus Hagander <magnus@hagander.net> writes:
> > On Wed, Oct 10, 2007 at 03:27:17PM +0300, Marko Kreen wrote:
> >> Now txid can change that.  E.g. in Skype, it has become
> >> irreplaceable tool for coordinating work between several
> >> databases.  Here we are probably going overboard with usage of
> >> queues...
>
> > If it is this irreplacable killer feature, it should *not* be in
> > contrib. It should be in the core backend, and we should be
> > discussing if we can bend the rules for that.
>
> It may be a killer feature for Skytools and Slony, but even so that's
> a small part of our userbase.

This is a valid point. Skytools is cool. Slony is cool, but their user
bases compared to PostgreSQL proper are miniscule. We need to be
considering the potential issues (if there are any) that this can cause
for the larger community.

I can think of a couple of immediate ones, I believe Tom has touched on
some of these elsewhere in the thread.

1) Maintainability
2) The eventual (lord it is about 5 years late) upgrade in place feature
3) This possibly should be in core not contrib.

*If* it should be in core, it needs to push to 8.4. It is that simple.
Else we need to open the tree and start reviewing all those patches
that got left behind before at feature freeze because of possible open
issues.

If it doesn't need to be in core, in certainly has zero need to be in
contrib and can push to pgFoundry.

>
> I think our two realistic options today are (1) leave the code where
> it is, or (2) remove it.  While Jan clearly failed to follow the
> agreed procedures, I'm not convinced the transgression was severe
> enough to justify (2).

Well I would like to step back and remove the Jan element. Jan has
really been taking a beating here. The reality is, A "committer" made a
mistake. It doesn't matter who it was.

The code needs to be removed. I just don't see a way around it,
unless we are willing to go back and review other items. This patch has
not been peer reviewed, it's benefit has not been discussed on
-hackers, and in general it hasn't been vetted.

Sincerely,

Joshua D. Drake


--
     === The PostgreSQL Company: Command Prompt, Inc. ===
Sales/Support: +1.503.667.4564   24x7/Emergency: +1.800.492.2240
PostgreSQL solutions since 1997  http://www.commandprompt.com/        UNIQUE NOT NULL
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
PostgreSQL Replication: http://www.commandprompt.com/products/

On Wed, 10 Oct 2007 18:33:03 +0300
"Marko Kreen" <markokr@gmail.com> wrote:

> Considering the core operations are now being in active use
> some 6-7 years, I really fail to see how there can be anything
> to tweak, unless you are speaking changing naming style.

Well that is the problem right there isn't it? As someone who has
financed, shipped, developed and tested an integrated Replication
solution for *years*, this statement is obvious naivety.

Your code, may be the most blessed, pristine and bug free code *in your
environment* but your environment is hardly the only environment out
there and things *always* come up.

>
> IMHO the core operations are already as stable as PostgreSQL use
> of MVCC, as the module just exports backend internal state...
> Current set of functions is the minimal necessary to implement
> queue operations, there is nothing more to remove.

Having a hard time buying that. MVCC has the pleasure of being tested
everyday by hundreds of thousands of installations.

>
> We might want to add some helper functions for query generation,
> but that does not affect core operation.
>

But it does affect the inclusion argument.

> Another thing can can be done is more compact representation for
> txid_snapshot type, but that also won't affect core operation.
>

You are starting to bring up things in your own post that may need to
change before inclusion. This is *exactly* why the code should be
removed. It wasn't vetted on -hackers, and if it had been we may have
had a more complete piece of software.

> I am very happy for txid being in contrib, I'm not arguing against
> that, but the hint that txid module is something that just freshly
> popped out of thin air is irking me.

Certainly, I can understand this as you have had a long time to work
with, develop and mature the code. However it is just out of thin air.
It doesn't exist except for a very small part of the PostgreSQL world.

It may not be new to you, but it is certainly 100% new to many of the
long time contributors of this project.


> > I think our two realistic options today are (1) leave the code where
> > it is, or (2) remove it.  While Jan clearly failed to follow the
> > agreed procedures, I'm not convinced the transgression was severe
> > enough to justify (2).
>
> Thanks for being understanding.
>

We all try to be :) but I do feel it needs to be removed, pgFoundry is
the perfect place for this.

Sincerely,

Joshua D. Drake

--
     === The PostgreSQL Company: Command Prompt, Inc. ===
Sales/Support: +1.503.667.4564   24x7/Emergency: +1.800.492.2240
PostgreSQL solutions since 1997  http://www.commandprompt.com/        UNIQUE NOT NULL
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
PostgreSQL Replication: http://www.commandprompt.com/products/

On Wed, 10 Oct 2007 13:01:54 +0200
Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> wrote:

> yeah I agree that code like this should be either in core or
> somewhere else (either pgfoundry or even shipped as part of the
> replication solutions mentioned which is basically something slony
> did for ages with the xxid stuff). Just pushing it now into contrib
> results in people wanting to use one of those solution having to deal
> with 3 kinds of packages:
>
> 1. postgresql
> 2. postgresql-contrib
> 3. skytools/slony/...
>
> instead of just two which does not strike me as much of an
> improvement.

I am not sure I am buying this argument. Skytools/slony is not
PostgreSQL. Should we also include Greg's recent replication software?
Or perhaps pgCluster?

I can think of at least a dozen modules that would be cool to have in
contrib... plpgsm or orafce?

Yes this is just a "small" bit but Marko has already made suggestions
in his own thread of items that should possibly change.

This needs to be pulled out, put on pgFoundry or submitted for 8.4.

Sincerely,

Joshua D. Drake




--
     === The PostgreSQL Company: Command Prompt, Inc. ===
Sales/Support: +1.503.667.4564   24x7/Emergency: +1.800.492.2240
PostgreSQL solutions since 1997  http://www.commandprompt.com/        UNIQUE NOT NULL
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
PostgreSQL Replication: http://www.commandprompt.com/products/

On Wed, 10 Oct 2007, Robert Treat wrote:

>>> Simon Riggs wrote:
>>>> I would prefer that we backported pg_standby into 8.2 contrib, so the
>>>> solution is where people need it to be. If not...
>
> If it was to go on pgfoundry (which I'd recommend) I'd suggest removing 
> it from 8.3 contrib before we release (cause having it in both places is 
> really going to cause confusion)

There is a perception that code in contrib, while not essential, has at 
least been reviewed by core to some degree and therefore is relatively 
safe to install.  I've listened to people state that they're not 
installing code from some random pgfoundry package on the production 
system in a meeting, while contrib code was considered perfectly 
acceptable.

pg_standby has such a wide potential audience that I'd hate to see it 
viewed as second-class code in this fashion were it moved to pgfoundry and 
pulled out of the 8.3 contrib.  If the concern is to avoid confusion, I'd 
suggest clearly labeling the foundry project something like "pg_standby 
8.2 backport", so people know it's aimed at being stable when run against 
an 8.2 server; that would make it obvious it's not intended for 8.3 
systems as well.

Right now I've been suggesting people use the version that comes with 8.3, 
but I get the feeling not everyone is comfortable with that; having a 
release specifically labeled 8.2 compatible would be a help.  Like Simon, 
I'd _like_ to see it show up in the 8.2 contrib, but as that goes against 
the project's stable version policies I don't expect that to happen. 
Having a "backport" (which may be the same code) as a pgfoundry project, 
with the understanding that it comes with the base contrib in 8.3, would 
help clear some of the concerns about the quality of the code I've run 
into.  Pushing the entire thing onto pgfoundry will make it even harder to 
get certain type of corporate customers to use pg_standby, which is a 
clear step backwards as far as I'm concerned.

--
* Greg Smith gsmith@gregsmith.com http://www.gregsmith.com Baltimore, MD

"Joshua D. Drake" <jd@commandprompt.com> writes:
> If it doesn't need to be in core, in certainly has zero need to be in
> contrib and can push to pgFoundry.

One advantage of having it in contrib is buildfarm testing, as indeed we
already found out ... although it's true that *keeping* it there now
that it passes probably won't teach us too much more.

But I think the argument that was being made was mostly that the Slony
and Skytools projects would find it easier to depend on a contrib module
than on something that has to be fetched separately from pgfoundry.
Now they could work around that by including copies of the pgfoundry
project in their own distributions, but then they have a collision
problem if someone tries to install both together.  (I have no idea how
likely that is, though; it might not be a big problem in practice?)
        regards, tom lane

On 10/10/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Magnus Hagander <magnus@hagander.net> writes:
> > (Assuming it's technically sound - I still haven't checked the actual
> > code, but I'm assuming it's Ok since Jan approved it)
>
> I hadn't looked at it either, but here are a few things that need
> review:
>
> * Why no binary I/O support for the new datatype?  We tend to expect
> that for all core types.

As I said, the current module is minimal, my goal was to
have code where there is nothing to remove.

But for a data type that targets core, yes binary i/o support
should be added.

> * Why is txid_current_snapshot() excluding subtransaction XIDs?  That
> might be all right for the current uses in Slony/Skytools, but it seems
> darn close to a bug for any other use.

In queue usage the transaction that stores snapshots does not do
any other work on its own, so it does not matter, main requirement
is that txid_current()/txid_current_snapshot() be symmetric -
whatever the txid_current() outputs, the txid_current_snapshot() measures.

But I agree, supporting subtransactions makes the API more
universal.  And it wouldn't break Slony/PgQ current usage.

> * Why is txid_current_snapshot() reading SerializableSnapshot rather
> than an actually current snap as its name suggests?  This isn't just
> misleading, this will fail completely when SerializableSnapshot
> goes away, as seems likely to happen in 8.4 (and no, we won't keep it
> just because txid might want it).

If you say so.  This code is from original xxid module and
has worked thus far. :)  If the requirement mentioned above
is not broken, the code needs to be brought in line with
current backend coding standards.

Will look into the problems and send patch tomorrow,
today has been too tiring already...

-- 
marko

On Wed, 10 Oct 2007 18:01:34 +0100
Gregory Stark <stark@enterprisedb.com> wrote:

> "Magnus Hagander" <magnus@hagander.net> writes:
>
> > On Wed, Oct 10, 2007 at 11:30:47AM -0400, Tom Lane wrote:
> >> Bruce Momjian <bruce@momjian.us> writes:
> >> > I also agree with this.  We have to pretend it isn't in /contrib
> >> > now, figure out where want it, then put it there (contrib,
> >> > pgfoundry, core).
> I just don't see the point in putting it in pgfoundry. It's already in
> pgfoundry as part of Skytools.
>  The whole point of having such a
> datatype is to build common interface to abstract away the internals
> of the database. That makes the pgfoundry modules (Skytools and
> Slony) easier to maintain separately.

I missed the part that it is part of Skytools already but as counter
point, what makes sense at that point is for Skytools to remove it and
make it it's own module. That way Slony (which is not a pgfoundry
project) or anyone else that wants to make use of it can.

>
> Putting it in core or contrib means that when we change the snapshot
> mechanics in 8.4 the same developer will be able to fix the module at
> the same time (and find out if his changes break it at the same
> time).

Which is very cool, for *8.4* :)

Joshua D. Drake


--
     === The PostgreSQL Company: Command Prompt, Inc. ===
Sales/Support: +1.503.667.4564   24x7/Emergency: +1.800.492.2240
PostgreSQL solutions since 1997  http://www.commandprompt.com/        UNIQUE NOT NULL
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
PostgreSQL Replication: http://www.commandprompt.com/products/

Ühel kenal päeval, K, 2007-10-10 kell 12:18, kirjutas Tom Lane:
> Magnus Hagander <magnus@hagander.net> writes:
> > (Assuming it's technically sound - I still haven't checked the actual
> > code, but I'm assuming it's Ok since Jan approved it)
> 
> I hadn't looked at it either, but here are a few things that need
> review:
> 
> * Why no binary I/O support for the new datatype?  We tend to expect
> that for all core types.

should be easy to add, likely a copy-past from any other varlena type.

> * Why is txid_current_snapshot() excluding subtransaction XIDs?  That
> might be all right for the current uses in Slony/Skytools, but it seems
> darn close to a bug for any other use.

Just thinking aloud here :

I think that the reason has something to do with it being for stored
snapshots used by different transactions for determining info about
other committed transactions , and the stored snapshots and transaction
ids become visible from SQL level only after both are committed.

There may be cases where you want to use it from other places, say C
code for user-defined function dealing with visibility of other
transactions, but before adding in subtransactions and thus possibly
bloating the storage, we should first identify such case.

Most likely it is better to just use in-backend snapshots straight from
backend internals if you dont need to store them.

> * Why is txid_current_snapshot() reading SerializableSnapshot rather
> than an actually current snap as its name suggests? This isn't just
> misleading, this will fail completely when SerializableSnapshot
> goes away, as seems likely to happen in 8.4 (and no, we won't keep it
> just because txid might want it).

Why is SerializableSnapshot going away ? 

How will we do serialized isolation level in 8.4 then?

----------------
Hannu

Ühel kenal päeval, K, 2007-10-10 kell 11:06, kirjutas Joshua D. Drake:
> On Wed, 10 Oct 2007 18:01:34 +0100
> Gregory Stark <stark@enterprisedb.com> wrote:
> 
> > "Magnus Hagander" <magnus@hagander.net> writes:
> > 
> > > On Wed, Oct 10, 2007 at 11:30:47AM -0400, Tom Lane wrote:
> > >> Bruce Momjian <bruce@momjian.us> writes:
> > >> > I also agree with this.  We have to pretend it isn't in /contrib
> > >> > now, figure out where want it, then put it there (contrib,
> > >> > pgfoundry, core).
>  
> > I just don't see the point in putting it in pgfoundry. It's already in
> > pgfoundry as part of Skytools.
> >  The whole point of having such a
> > datatype is to build common interface to abstract away the internals
> > of the database. That makes the pgfoundry modules (Skytools and
> > Slony) easier to maintain separately.
> 
> I missed the part that it is part of Skytools already but as counter
> point, what makes sense at that point is for Skytools to remove it and
> make it it's own module.

Is'nt  this just what happened when it was moved to contrib ?

>  That way Slony (which is not a pgfoundry
> project) or anyone else that wants to make use of it can.
> 
> > 
> > Putting it in core or contrib means that when we change the snapshot
> > mechanics in 8.4 the same developer will be able to fix the module at
> > the same time (and find out if his changes break it at the same
> > time).
> 
> Which is very cool, for *8.4* :)
> 
> Joshua D. Drake
> 
> 

On 10/10/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> "Joshua D. Drake" <jd@commandprompt.com> writes:
> > If it doesn't need to be in core, in certainly has zero need to be in
> > contrib and can push to pgFoundry.
>
> One advantage of having it in contrib is buildfarm testing, as indeed we
> already found out ... although it's true that *keeping* it there now
> that it passes probably won't teach us too much more.
>
> But I think the argument that was being made was mostly that the Slony
> and Skytools projects would find it easier to depend on a contrib module
> than on something that has to be fetched separately from pgfoundry.
> Now they could work around that by including copies of the pgfoundry
> project in their own distributions, but then they have a collision
> problem if someone tries to install both together.  (I have no idea how
> likely that is, though; it might not be a big problem in practice?)

Well, if it is kicked from /contrib now, one way we could handle
it is by shipping the same module inside both skytools/slony.

That has obvious conflict problems.

Unfortunately the problem has very easy fix - each one keeps
using it's current module.  Very easy, no work required.

But that also scratches the common API possibility.

-- 
marko

Ühel kenal päeval, K, 2007-10-10 kell 18:23, kirjutas Magnus Hagander:
> On Wed, Oct 10, 2007 at 11:47:15AM -0400, Tom Lane wrote:
> > "Marko Kreen" <markokr@gmail.com> writes:
> > > IMHO the core operations are already as stable as PostgreSQL use
> > > of MVCC, as the module just exports backend internal state...
> > 
> > Well, it exports backend internal state that did not exist before 8.2
> > (ie, XID epoch).  So it doesn't seem all that set in stone to me.
> > 
> > > Another thing can can be done is more compact representation for
> > > txid_snapshot type, but that also won't affect core operation.
> > 
> > That's another thing that's likely to become very much harder to change
> > once it's in core.  People keep threatening to produce a working
> > in-place-upgrade process, and once that's reality the on-disk
> > representation of core types is going to be hard to change.
> 
> Well, if that is a concern, than it's an equally big concern to have it in
> contrib. If people start using it, they're not going to care about us
> saying "hey, it was in contrib, why did you use it", when we earlier said
> "in order do use our whiz-bang stuff, you must install from contrib". We'll
> have complaints that it's too hard to install, but we won't manage to
> escape from the responsibility to keep it working.

I guess Marko will be able to take that responsibility, as he has been
doing for pg_crypto for years, an recently also pl/python to some
degree.

tsearch lived in contrib for quite long, evolving a lot and going
through a big morphing when it moved to core. I can't see anything
nearly as big happening to txid/snapshot types.

----------------
Hannu

"Joshua D. Drake" <jd@commandprompt.com> writes:
> Gregory Stark <stark@enterprisedb.com> wrote:
>> Putting it in core or contrib means that when we change the snapshot
>> mechanics in 8.4 the same developer will be able to fix the module at
>> the same time (and find out if his changes break it at the same
>> time).

> Which is very cool, for *8.4* :)

I think you missed one point: waiting for 8.4 is too late because of the
mechanics of the slony/skytools projects.  The reason this came up at
all is those projects' recognition that they had a narrow window to
standardize on a common bit of code.  Slony is breaking backward
compatibility for 8.3 in order to make use of the new backend
ENABLE/DISABLE REPLICA TRIGGER functionality.  They can fold in
dependence on an externally-supplied txid at the same time, but if they
miss doing so, they're hardly likely to break compatibility again
anytime in the near future.  So if there's no solution available for 8.3
then there's no point in doing anything at all.

This is not an argument why they cannot depend on a pgfoundry project
for 8.3 instead of a contrib module, but it is the reason why simply
waiting to propose the feature for 8.4 wasn't a viable alternative.

I had been thinking that the choice between pgfoundry and contrib
was technically neutral and only a matter of policy, but Greg's
argument does raise a valid technical point: if the code is in contrib
then it *will* track any redesign of the snapshot maintenance code,
whereas if it's in pgfoundry then it won't.  That could perhaps be
addressed by merging it into 8.4 before anyone does any snapshot fixing,
but our track record on causing such things to happen in a particular
sequence isn't great ...
        regards, tom lane

Hannu Krosing <hannu@skype.net> writes:
> Ühel kenal päeval, K, 2007-10-10 kell 12:18, kirjutas Tom Lane:
>> * Why is txid_current_snapshot() reading SerializableSnapshot rather
>> than an actually current snap as its name suggests?

> Why is SerializableSnapshot going away ? 
> How will we do serialized isolation level in 8.4 then?

If we are in a serializable transaction, we'll keep that snap around
(though probably not stored exactly where it is now).  In a Read
Committed transaction we should discard snaps that are no longer going
to be used by any subsequent query; this will allow intratransaction
advancement of xmin with ensuing benefits for VACUUM etc.  (This has
been discussed repeatedly, though I'm too lazy to go searching the
archives at the moment.)

The proposed behavior of txid_current_snapshot would defeat any
possibility of such an optimization, because we'd have to keep around
the xact's oldest snapshot on the off chance that txid_current_snapshot
would be called later in the xact.

I think txid_current_snapshot should read ActiveSnapshot.  If the user
wants to get a beginning-of-xact rather than beginning-of-statement
snapshot from it, he should be required to call it in a serializable
transaction.
        regards, tom lane

Tom Lane wrote:
> The proposed behavior of txid_current_snapshot would defeat any possibility
> of such an optimization, because we'd have to keep around the xact's oldest
> snapshot on the off chance that txid_current_snapshot would be called later
> in the xact.
> 
> I think txid_current_snapshot should read ActiveSnapshot.  If the user wants
> to get a beginning-of-xact rather than beginning-of-statement snapshot from
> it, he should be required to call it in a serializable transaction.

Hm... does txid require that the snapshot it uses a valid in the sense that
its xmin follows OldestXmin? If not, we could keep the snapshot around for txid,
but still update our published xmin - which seems to be the main reason we care
about getting rid of old snapshots at all.

greetings, Florian Pflug

Hello,

Well this certainly turned into something bigger than I thought it ever
would. The questions that come into play with this whole thread are
larger than just, "the process wasn't followed, what do we do?"

We obviously don't want to make life difficult for our sibling projects
such as Slony and Skytools. On the other hand, we certainly want
processes followed because it leads to better overall quality control.
It also leads to fairness within community as a whole.

There are quite a few contributors that are upset that this whole
process went down the way that it did. I would say they are likely in
the majority versus the people that just want to leave it alone and
move on.

As I see it, we really only have a couple of choices:

Allow it as contrib which isn't really a good solution because,
Tom and others have piped up that this may need to be in core. If it is
to go into core we have two choices, back off beta and
review some of those last patches that were kicked or... push it to 8.4.
 Why? Because people are already talking about changes to this
patch such as supporting binary I/O and helper functions that don't
exist in the current piece of code.
 That means it is not complete. Which means we might as well look at
Concurrent psql, Table function support, bitmap scan changes, and GIT
as well.
 Those patches were submitted long ago and tried to go through the
correct process and have been pushed to 8.4. Now, some of them may need
more work than is warranted (I can't actually speak to that) but
certainly a couple of them are worth a second look.

Another option, is to push the contrib module to pgfoundry. There is
zero loss here to PostgreSQL that I can see, in the current state of the
patch. It just means that the two projects supporting this patch
(Skytools and Slony) will have to cooperate. They have already proven
they can do this.

Of course this doesn't solve the should be in core issue but the should
be in core came about *after*.

Either way, I think everyone has had there say. Can we just make a
decision. Personally I think we should just push to pgfoundry and call
it good.

Sincerely,

Joshua D. Drake



--
     === The PostgreSQL Company: Command Prompt, Inc. ===
Sales/Support: +1.503.667.4564   24x7/Emergency: +1.800.492.2240
PostgreSQL solutions since 1997  http://www.commandprompt.com/        UNIQUE NOT NULL
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
PostgreSQL Replication: http://www.commandprompt.com/products/

Tom Lane wrote:
> Robert Treat <xzilla@users.sourceforge.net> writes:
>   
>> On Wednesday 10 October 2007 10:57, Andrew Dunstan wrote:
>>     
>>> One of pgfoundry's explicit purposes is for backports of features. 
>>>       
>
>   
>> I can't think of any contrib modules we've added that also required
>> backwards comptible modules to be released on foundry at the same
>> time.  ISTM that such a requirement would be an argument that such a
>> thing doesn't belong in contrib at all.
>>     
>
> AFAICT there isn't any market for a backport of txid.  Slony won't
> depend on it before their next release, which will require PG >= 8.3
> for other reasons.  Skytools already has an internal version in their
> existing releases.  And the code won't work before PG 8.2 so any
> backport couldn't go very far anyway.
>
> So while Andrew's statement is true in general, I don't think
> it's very relevant to a consideration of what to do with txid.
>
>             
>   

The context of this quote was referring to pg_standby, not txid.

We wouldn't be having this discussion at all if we had not had a 
horribly long period beween feature freeze and beta. We'd be way past 
the stage where anyone would consider adding something to contrib or 
anywhere else. The only cure I can see for that is that we need much 
more stringent criteria for what is a candidate to make the cut. I know 
I committed things that really weren't ready when I got hold of them, 
and required a lot of work to get them anything like ready. Arguably 
that didn't matter because they weren't on the critical path, but I 
think all projects need to be handled equitably. I'm sure Tom faced the 
same problem I did ten times over.

cheers

andrew

On Oct 10, 2007, at 13:30 , Tom Lane wrote:

> That could perhaps be
> addressed by merging it into 8.4 before anyone does any snapshot
> fixing,
> but our track record on causing such things to happen in a particular
> sequence isn't great ...

Granted, everyone's focused on the 8.3 branch right now, but with the
enthusiasm of those who want txid, I can't help but think there'd be
a patch ready and waiting the day 8_3_STABLE is tagged. And there's
no reason not to have something submitted to -patches right now
(unless it's not ready)—there are patches in the patch queue that
didn't make it in before feature freeze.

Michael Glaesemann
grzm seespotcode net

"Marko Kreen" <markokr@gmail.com> writes:
> On 10/10/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> * Why is txid_current_snapshot() excluding subtransaction XIDs?  That
>> might be all right for the current uses in Slony/Skytools, but it seems
>> darn close to a bug for any other use.
> ...
> But I agree, supporting subtransactions makes the API more
> universal.  And it wouldn't break Slony/PgQ current usage.

After looking at this more closely, I think txid_current_snapshot is
okay as is, but is_visible_txid is probably buggy: the latter should be
folding subtransaction IDs to top-transaction IDs, no?  If not, why not?
I hope the answer is "no" because otherwise the code will be at huge risk
from truncation of pg_subtrans, but it's not apparent why this behavior
is okay.
        regards, tom lane

On 10/10/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> "Marko Kreen" <markokr@gmail.com> writes:
> > On 10/10/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> * Why is txid_current_snapshot() excluding subtransaction XIDs?  That
> >> might be all right for the current uses in Slony/Skytools, but it seems
> >> darn close to a bug for any other use.
> > ...
> > But I agree, supporting subtransactions makes the API more
> > universal.  And it wouldn't break Slony/PgQ current usage.
>
> After looking at this more closely, I think txid_current_snapshot is
> okay as is, but is_visible_txid is probably buggy: the latter should be
> folding subtransaction IDs to top-transaction IDs, no?  If not, why not?
> I hope the answer is "no" because otherwise the code will be at huge risk
> from truncation of pg_subtrans, but it's not apparent why this behavior
> is okay.

Could you describe bit more?  The is_visible_txid() works
on data returned by txid_current_snapshot()?  How can there
be any subtrans id's if txid_current_snapshot() wont return
them?

The basic idea is - only txid_current() and txid_current_snapshot()
communicate with backend, rest of functions work on data
returned by them.

-- 
marko

"Joshua D. Drake" <jd@commandprompt.com> writes:

> There are quite a few contributors that are upset that this whole
> process went down the way that it did. I would say they are likely in
> the majority versus the people that just want to leave it alone and
> move on.

>   That means it is not complete. Which means we might as well look at
> Concurrent psql, Table function support, bitmap scan changes, and GIT
> as well.

That's just nonsense. We need to fix our other problems too and that means
getting substantive feedback to the authors of those patches so they can
complete the work. But that has no bearing whatsoever on the current
situation.

> Another option, is to push the contrib module to pgfoundry. There is
> zero loss here to PostgreSQL that I can see, in the current state of the
> patch. 

You keep saying this, do you have any justification for it?

I've explained why I think this code belongs in Postgres and not pgfoundry,
did you have any counter-argument?

And the complaints Tom brought up are mostly precisely the kind of interface
issues that actually argue well for it being in contrib. It serves its current
purpose well but future users might need binary i/o or subxid support and so
on. Until the interface is very stable being in contrib makes perfect sense.

--  Gregory Stark EnterpriseDB          http://www.enterprisedb.com

On Wed, 10 Oct 2007 21:02:30 +0100
Gregory Stark <stark@enterprisedb.com> wrote:

>
> "Joshua D. Drake" <jd@commandprompt.com> writes:
>
> > There are quite a few contributors that are upset that this whole
> > process went down the way that it did. I would say they are likely
> > in the majority versus the people that just want to leave it alone
> > and move on.
>
> >   That means it is not complete. Which means we might as well look
> > at Concurrent psql, Table function support, bitmap scan changes,
> > and GIT as well.
>
> That's just nonsense. We need to fix our other problems too and that
> means getting substantive feedback to the authors of those patches so
> they can complete the work. But that has no bearing whatsoever on the
> current situation.

You seem to be diverting my point. We can not provide preferential
treatment. Those patches are out there and have been out there for some
time. They followed the rules. Frankly, they deserve to be fully
reviewed and have the opportunity to be put in core *before* this
contrib patch.

Especially since this patch has already been marked as *not complete*.
There is already discussion happening on the patch and the changes that
need to be made.

>
> > Another option, is to push the contrib module to pgfoundry. There is
> > zero loss here to PostgreSQL that I can see, in the current state
> > of the patch.
>
> You keep saying this, do you have any justification for it?

I believe if you read my posts I have made plenty of justification. The
simplest of course being, process wasn't followed.

>
> I've explained why I think this code belongs in Postgres and not
> pgfoundry, did you have any counter-argument?

I believe I have mentioned that there is an argument for it to be in
PostgreSQL.

>
> And the complaints Tom brought up are mostly precisely the kind of
> interface issues that actually argue well for it being in contrib.

Nothing that is in contrib can not also be maintained just as well with
pgFoundry. It just may take more proactiveness in the process.

> It
> serves its current purpose well but future users might need binary
> i/o or subxid support and so on. Until the interface is very stable
> being in contrib makes perfect sense.
>

I would state that until the interface is very stable pgfoundry also
makes perfect sense.

I am getting the impression that you think that I don't *want* this
module. I do, but I do not want it at the sacrifice of other modules
and code authors who did the job the right way.

I understand Tom's point about if we push to 8.4 that could cause
problems for Slony and Skytools. I certainly don't want to cause
problems for some very cool projects. I do however don't see those
problems existing if it was in pgFoundry.

Or are we saying that the only way to provide quality sofware to
PostgreSQL is if it is either in core or contrib? I do not believe you
are saying that.

Sincerely,

Joshua D. Drake






--
     === The PostgreSQL Company: Command Prompt, Inc. ===
Sales/Support: +1.503.667.4564   24x7/Emergency: +1.800.492.2240
PostgreSQL solutions since 1997  http://www.commandprompt.com/        UNIQUE NOT NULL
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
PostgreSQL Replication: http://www.commandprompt.com/products/

Looking at the discussion, I think we should just keep it in /contrib. 
The code is tightly tied to our backend transaction system so there is
logic to have it in /contrib rather than pgfoundry.  I do think we
should just move it into core for 8.4 though.

---------------------------------------------------------------------------

Joshua D. Drake wrote:
-- Start of PGP signed section.
> On Wed, 10 Oct 2007 21:02:30 +0100
> Gregory Stark <stark@enterprisedb.com> wrote:
> 
> > 
> > "Joshua D. Drake" <jd@commandprompt.com> writes:
> > 
> > > There are quite a few contributors that are upset that this whole
> > > process went down the way that it did. I would say they are likely
> > > in the majority versus the people that just want to leave it alone
> > > and move on.
> > 
> > >   That means it is not complete. Which means we might as well look
> > > at Concurrent psql, Table function support, bitmap scan changes,
> > > and GIT as well.
> > 
> > That's just nonsense. We need to fix our other problems too and that
> > means getting substantive feedback to the authors of those patches so
> > they can complete the work. But that has no bearing whatsoever on the
> > current situation.
> 
> You seem to be diverting my point. We can not provide preferential
> treatment. Those patches are out there and have been out there for some
> time. They followed the rules. Frankly, they deserve to be fully
> reviewed and have the opportunity to be put in core *before* this
> contrib patch.
> 
> Especially since this patch has already been marked as *not complete*.
> There is already discussion happening on the patch and the changes that
> need to be made.
> 
> > 
> > > Another option, is to push the contrib module to pgfoundry. There is
> > > zero loss here to PostgreSQL that I can see, in the current state
> > > of the patch. 
> > 
> > You keep saying this, do you have any justification for it?
> 
> I believe if you read my posts I have made plenty of justification. The
> simplest of course being, process wasn't followed.
> 
> > 
> > I've explained why I think this code belongs in Postgres and not
> > pgfoundry, did you have any counter-argument?
> 
> I believe I have mentioned that there is an argument for it to be in
> PostgreSQL. 
> 
> > 
> > And the complaints Tom brought up are mostly precisely the kind of
> > interface issues that actually argue well for it being in contrib.
> 
> Nothing that is in contrib can not also be maintained just as well with
> pgFoundry. It just may take more proactiveness in the process.
> 
> > It
> > serves its current purpose well but future users might need binary
> > i/o or subxid support and so on. Until the interface is very stable
> > being in contrib makes perfect sense.
> > 
> 
> I would state that until the interface is very stable pgfoundry also
> makes perfect sense.
> 
> I am getting the impression that you think that I don't *want* this
> module. I do, but I do not want it at the sacrifice of other modules
> and code authors who did the job the right way.
> 
> I understand Tom's point about if we push to 8.4 that could cause
> problems for Slony and Skytools. I certainly don't want to cause
> problems for some very cool projects. I do however don't see those
> problems existing if it was in pgFoundry.
> 
> Or are we saying that the only way to provide quality sofware to
> PostgreSQL is if it is either in core or contrib? I do not believe you
> are saying that.
> 
> Sincerely,
> 
> Joshua D. Drake
> 
> 
> 
> 
> 
> 
> -- 
> 
>       === The PostgreSQL Company: Command Prompt, Inc. ===
> Sales/Support: +1.503.667.4564   24x7/Emergency: +1.800.492.2240
> PostgreSQL solutions since 1997  http://www.commandprompt.com/
>             UNIQUE NOT NULL
> Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
> PostgreSQL Replication: http://www.commandprompt.com/products/
> 
-- End of PGP section, PGP failed!

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://postgres.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

On Wed, 10 Oct 2007 17:10:17 -0400 (EDT)
Bruce Momjian <bruce@momjian.us> wrote:

>
> Looking at the discussion, I think we should just keep it
> in /contrib. The code is tightly tied to our backend transaction
> system so there is logic to have it in /contrib rather than
> pgfoundry.  I do think we should just move it into core for 8.4
> though.

In the interest of killing the problem. I agree. Everyone has had ample
opportunity for their opinions and frankly there isn't a good solution,
so let's take the one presented.

It's committed. It is in 8.3.

Sincerely,

Joshua D. Drake
--
     === The PostgreSQL Company: Command Prompt, Inc. ===
Sales/Support: +1.503.667.4564   24x7/Emergency: +1.800.492.2240
PostgreSQL solutions since 1997  http://www.commandprompt.com/        UNIQUE NOT NULL
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
PostgreSQL Replication: http://www.commandprompt.com/products/

"Marko Kreen" <markokr@gmail.com> writes:
> Could you describe bit more?  The is_visible_txid() works
> on data returned by txid_current_snapshot()?  How can there
> be any subtrans id's if txid_current_snapshot() wont return
> them?

Ah, I see: txid_current() never reports a subxact ID so there's no need to
consider them elsewhere in txids either.  OK, but this desperately needs
to be documented.

BTW, I notice that use of txid_current will force assignment of an XID
in a transaction that might not otherwise have one.  Does this matter,
or is the expectation that it's only going to be used in transactions
that are making DB modifications anyway?
        regards, tom lane

Florian Pflug <fgp.phlo.org@gmail.com> writes:
> Tom Lane wrote:
>> I think txid_current_snapshot should read ActiveSnapshot.  If the user wants
>> to get a beginning-of-xact rather than beginning-of-statement snapshot from
>> it, he should be required to call it in a serializable transaction.

> Hm... does txid require that the snapshot it uses a valid in the sense that
> its xmin follows OldestXmin? If not, we could keep the snapshot around for txid,
> but still update our published xmin - which seems to be the main reason we care
> about getting rid of old snapshots at all.

Why should we complicate the main code like that for txid?  I have not
heard any argument why the function should be examining
SerializableSnapshot instead of the current transaction snapshot.
        regards, tom lane

Andrew Dunstan <andrew@dunslane.net> writes:
> We wouldn't be having this discussion at all if we had not had a 
> horribly long period beween feature freeze and beta.

I'm not sure about that.  The bottom line to me is that we are doing a
favor to the Slony and Skytools projects, who figured out *after* our
feature freeze that they could use some common code but it would be
too late if it wasn't available in 8.3.  Even had the freeze period
been only a couple weeks, that could have happened --- it was just the
sort of unfortunate timing that the universe likes to play on humans ;-)

There was some complaining in this thread that we were showing
favoritism to the txid patch.  I'll plead guilty instead to favoritism
to the Slony and Skytools projects --- the patch would certainly have
gotten bounced without the support of those projects.  I don't have a
good grasp on the importance of Skytools, but I do know about Slony,
and I don't have a problem with cutting them a break on a small,
low-risk patch, even if it is a feature addition.  The patches that got
passed over for 8.3 were neither small nor low-risk; in fact, the reason
the process dragged out so long was exactly that we busted our butts
to get in everything that had any chance at all of getting in.
        regards, tom lane

Ühel kenal päeval, K, 2007-10-10 kell 10:10, kirjutas Joshua D. Drake:
> On Wed, 10 Oct 2007 18:33:03 +0300
> "Marko Kreen" <markokr@gmail.com> wrote:
> 
> > Considering the core operations are now being in active use
> > some 6-7 years, I really fail to see how there can be anything
> > to tweak, unless you are speaking changing naming style.
> 
> Well that is the problem right there isn't it? As someone who has
> financed, shipped, developed and tested an integrated Replication
> solution for *years*, 

AH, now I see it , and I think I understand your concerns better ;)

> this statement is obvious naivety.

Then you should not feel threatened by including this in contrib 

> Your code, may be the most blessed, pristine and bug free code *in your
> environment* but your environment is hardly the only environment out
> there and things *always* come up. 

Meaning there will still be market for tried and tested commercially
supported wal-log based replication solutions. 

btw, can you publicly discuss how CommandPrompts  WAL-based replication
works ? 

Does it also also need something similar to snapshot type to do
replication effectively (or else you have to "quess", which portion of
WAL is safe to commit and in what order) or does it somehow come out of
WAL log naturally ? 

And how do you map WAL records back to tables/indexes/sequences in slave
databases in an effective manner ?

> > IMHO the core operations are already as stable as PostgreSQL use
> > of MVCC, as the module just exports backend internal state...
> > Current set of functions is the minimal necessary to implement
> > queue operations, there is nothing more to remove.
> 
> Having a hard time buying that. MVCC has the pleasure of being tested
> everyday by hundreds of thousands of installations.

that's what he is telling you - this code just exposes to userspace,
what has been tested everyday by hundreds of thousands of installations.

> > We might want to add some helper functions for query generation,
> > but that does not affect core operation.
> > 
> 
> But it does affect the inclusion argument.

Probably makes it stronger, as we dont want different users to add
slightly different versions of these. And hopefully these will be
discussed on -hackers before final design is committed :)

> > Another thing can can be done is more compact representation for
> > txid_snapshot type, but that also won't affect core operation.
> >
> 
> You are starting to bring up things in your own post that may need to
> change before inclusion. This is *exactly* why the code should be
> removed. It wasn't vetted on -hackers, and if it had been we may have
> had a more complete piece of software.

None of these needs to change before inclusion, as they don't change
interfaces. they are either plans for future enchancemants in
implementation (you cant argue that nothing can get into contrib unless
all conceivable future enchancements are already done), or possible new
interfaces to be added in future if needed.

> > I am very happy for txid being in contrib, I'm not arguing against
> > that, but the hint that txid module is something that just freshly
> > popped out of thin air is irking me.
> 
> Certainly, I can understand this as you have had a long time to work
> with, develop and mature the code. However it is just out of thin air.
> It doesn't exist except for a very small part of the PostgreSQL world.
> 
> It may not be new to you, but it is certainly 100% new to many of the
> long time contributors of this project. 
> 
> > > I think our two realistic options today are (1) leave the code where
> > > it is, or (2) remove it.  While Jan clearly failed to follow the
> > > agreed procedures, I'm not convinced the transgression was severe
> > > enough to justify (2).
> > 
> > Thanks for being understanding.
> > 
> 
> We all try to be :) but I do feel it needs to be removed, pgFoundry is
> the perfect place for this.

We feel that it is may be a generally enabling technology, and hope that
if it is being included in postgreSQL baseline distribution it will
sprout new uses beyound high-performanse replication and queueing, as it
lowers the barriers for making new creative uses of postgreSQL's MVCC
mechanisms by people not very familiar with minute details of
postgreSQL's inner workings.

--------------------
Hannu

Ühel kenal päeval, K, 2007-10-10 kell 17:17, kirjutas Tom Lane:
> "Marko Kreen" <markokr@gmail.com> writes:
> > Could you describe bit more?  The is_visible_txid() works
> > on data returned by txid_current_snapshot()?  How can there
> > be any subtrans id's if txid_current_snapshot() wont return
> > them?
> 
> Ah, I see: txid_current() never reports a subxact ID so there's no need to
> consider them elsewhere in txids either.  OK, but this desperately needs
> to be documented.
> 
> BTW, I notice that use of txid_current will force assignment of an XID
> in a transaction that might not otherwise have one.  Does this matter,
> or is the expectation that it's only going to be used in transactions
> that are making DB modifications anyway?

yes, the common use in Skytools (and now in Slony) is in
insert/update/delete triggers to add current txid to log records.

>             regards, tom lane
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 7: You can help support the PostgreSQL project by donating at
> 
>                 http://www.postgresql.org/about/donate

On Thu, 11 Oct 2007 01:43:16 +0300
Hannu Krosing <hannu@skype.net> wrote:

> AH, now I see it , and I think I understand your concerns better ;)
>
> > this statement is obvious naivety.
>
> Then you should not feel threatened by including this in contrib
>

Please do not mistake my concerns for having anything to do with
replicator. They don't. My concerns were purely about following correct
process within the community.

>
> btw, can you publicly discuss how CommandPrompts  WAL-based
> replication works ?

It's my company, if course I am ;)... but not in this thread. If you
are interested feel free to email me directly or start a new thread.

Joshua D. Drake



--
     === The PostgreSQL Company: Command Prompt, Inc. ===
Sales/Support: +1.503.667.4564   24x7/Emergency: +1.800.492.2240
PostgreSQL solutions since 1997  http://www.commandprompt.com/        UNIQUE NOT NULL
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
PostgreSQL Replication: http://www.commandprompt.com/products/

Tom Lane wrote:
> Florian Pflug <fgp.phlo.org@gmail.com> writes:
>> Tom Lane wrote:
>>> I think txid_current_snapshot should read ActiveSnapshot.  If the user 
>>> wants to get a beginning-of-xact rather than beginning-of-statement 
>>> snapshot from it, he should be required to call it in a serializable 
>>> transaction.
> 
>> Hm... does txid require that the snapshot it uses a valid in the sense that
>>  its xmin follows OldestXmin? If not, we could keep the snapshot around for
>>  txid, but still update our published xmin - which seems to be the main 
>> reason we care about getting rid of old snapshots at all.
> 
> Why should we complicate the main code like that for txid?  I have not heard 
> any argument why the function should be examining SerializableSnapshot 
> instead of the current transaction snapshot.

I wouldn't know. I just wanted to say that even if it needs to examine
SerializableSnapshot, that won't clash with the xmin optimizations planned for
8.4, as long as the snapshot won't be used for actual queries.

greetings, Florian Pflug

I don't undrestand why the txid stuff is in 8.3beta(this is an unsual
case right?), but if we decide to keep it, please consider updating
release.sgml. Bruce explained me that release.sgml will not be updated
until the official release, but this is the unusual case and we need to
break the rule, I think.
--
Tatsuo Ishii
SRA OSS, Inc. Japan

> Looking at the discussion, I think we should just keep it in /contrib. 
> The code is tightly tied to our backend transaction system so there is
> logic to have it in /contrib rather than pgfoundry.  I do think we
> should just move it into core for 8.4 though.
> 
> ---------------------------------------------------------------------------
> 
> Joshua D. Drake wrote:
> -- Start of PGP signed section.
> > On Wed, 10 Oct 2007 21:02:30 +0100
> > Gregory Stark <stark@enterprisedb.com> wrote:
> > 
> > > 
> > > "Joshua D. Drake" <jd@commandprompt.com> writes:
> > > 
> > > > There are quite a few contributors that are upset that this whole
> > > > process went down the way that it did. I would say they are likely
> > > > in the majority versus the people that just want to leave it alone
> > > > and move on.
> > > 
> > > >   That means it is not complete. Which means we might as well look
> > > > at Concurrent psql, Table function support, bitmap scan changes,
> > > > and GIT as well.
> > > 
> > > That's just nonsense. We need to fix our other problems too and that
> > > means getting substantive feedback to the authors of those patches so
> > > they can complete the work. But that has no bearing whatsoever on the
> > > current situation.
> > 
> > You seem to be diverting my point. We can not provide preferential
> > treatment. Those patches are out there and have been out there for some
> > time. They followed the rules. Frankly, they deserve to be fully
> > reviewed and have the opportunity to be put in core *before* this
> > contrib patch.
> > 
> > Especially since this patch has already been marked as *not complete*.
> > There is already discussion happening on the patch and the changes that
> > need to be made.
> > 
> > > 
> > > > Another option, is to push the contrib module to pgfoundry. There is
> > > > zero loss here to PostgreSQL that I can see, in the current state
> > > > of the patch. 
> > > 
> > > You keep saying this, do you have any justification for it?
> > 
> > I believe if you read my posts I have made plenty of justification. The
> > simplest of course being, process wasn't followed.
> > 
> > > 
> > > I've explained why I think this code belongs in Postgres and not
> > > pgfoundry, did you have any counter-argument?
> > 
> > I believe I have mentioned that there is an argument for it to be in
> > PostgreSQL. 
> > 
> > > 
> > > And the complaints Tom brought up are mostly precisely the kind of
> > > interface issues that actually argue well for it being in contrib.
> > 
> > Nothing that is in contrib can not also be maintained just as well with
> > pgFoundry. It just may take more proactiveness in the process.
> > 
> > > It
> > > serves its current purpose well but future users might need binary
> > > i/o or subxid support and so on. Until the interface is very stable
> > > being in contrib makes perfect sense.
> > > 
> > 
> > I would state that until the interface is very stable pgfoundry also
> > makes perfect sense.
> > 
> > I am getting the impression that you think that I don't *want* this
> > module. I do, but I do not want it at the sacrifice of other modules
> > and code authors who did the job the right way.
> > 
> > I understand Tom's point about if we push to 8.4 that could cause
> > problems for Slony and Skytools. I certainly don't want to cause
> > problems for some very cool projects. I do however don't see those
> > problems existing if it was in pgFoundry.
> > 
> > Or are we saying that the only way to provide quality sofware to
> > PostgreSQL is if it is either in core or contrib? I do not believe you
> > are saying that.
> > 
> > Sincerely,
> > 
> > Joshua D. Drake
> > 
> > 
> > 
> > 
> > 
> > 
> > -- 
> > 
> >       === The PostgreSQL Company: Command Prompt, Inc. ===
> > Sales/Support: +1.503.667.4564   24x7/Emergency: +1.800.492.2240
> > PostgreSQL solutions since 1997  http://www.commandprompt.com/
> >             UNIQUE NOT NULL
> > Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
> > PostgreSQL Replication: http://www.commandprompt.com/products/
> > 
> -- End of PGP section, PGP failed!
> 
> -- 
>   Bruce Momjian  <bruce@momjian.us>        http://momjian.us
>   EnterpriseDB                             http://postgres.enterprisedb.com
> 
>   + If your life is a hard drive, Christ can be your backup. +
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 5: don't forget to increase your free space map settings

Tatsuo Ishii wrote:
> I don't undrestand why the txid stuff is in 8.3beta(this is an unsual
> case right?), but if we decide to keep it, please consider updating
> release.sgml. Bruce explained me that release.sgml will not be updated
> until the official release, but this is the unusual case and we need to
> break the rule, I think.

release.sgml is updated before every beta.

---------------------------------------------------------------------------


> --
> Tatsuo Ishii
> SRA OSS, Inc. Japan
> 
> > Looking at the discussion, I think we should just keep it in /contrib. 
> > The code is tightly tied to our backend transaction system so there is
> > logic to have it in /contrib rather than pgfoundry.  I do think we
> > should just move it into core for 8.4 though.
> > 
> > ---------------------------------------------------------------------------
> > 
> > Joshua D. Drake wrote:
> > -- Start of PGP signed section.
> > > On Wed, 10 Oct 2007 21:02:30 +0100
> > > Gregory Stark <stark@enterprisedb.com> wrote:
> > > 
> > > > 
> > > > "Joshua D. Drake" <jd@commandprompt.com> writes:
> > > > 
> > > > > There are quite a few contributors that are upset that this whole
> > > > > process went down the way that it did. I would say they are likely
> > > > > in the majority versus the people that just want to leave it alone
> > > > > and move on.
> > > > 
> > > > >   That means it is not complete. Which means we might as well look
> > > > > at Concurrent psql, Table function support, bitmap scan changes,
> > > > > and GIT as well.
> > > > 
> > > > That's just nonsense. We need to fix our other problems too and that
> > > > means getting substantive feedback to the authors of those patches so
> > > > they can complete the work. But that has no bearing whatsoever on the
> > > > current situation.
> > > 
> > > You seem to be diverting my point. We can not provide preferential
> > > treatment. Those patches are out there and have been out there for some
> > > time. They followed the rules. Frankly, they deserve to be fully
> > > reviewed and have the opportunity to be put in core *before* this
> > > contrib patch.
> > > 
> > > Especially since this patch has already been marked as *not complete*.
> > > There is already discussion happening on the patch and the changes that
> > > need to be made.
> > > 
> > > > 
> > > > > Another option, is to push the contrib module to pgfoundry. There is
> > > > > zero loss here to PostgreSQL that I can see, in the current state
> > > > > of the patch. 
> > > > 
> > > > You keep saying this, do you have any justification for it?
> > > 
> > > I believe if you read my posts I have made plenty of justification. The
> > > simplest of course being, process wasn't followed.
> > > 
> > > > 
> > > > I've explained why I think this code belongs in Postgres and not
> > > > pgfoundry, did you have any counter-argument?
> > > 
> > > I believe I have mentioned that there is an argument for it to be in
> > > PostgreSQL. 
> > > 
> > > > 
> > > > And the complaints Tom brought up are mostly precisely the kind of
> > > > interface issues that actually argue well for it being in contrib.
> > > 
> > > Nothing that is in contrib can not also be maintained just as well with
> > > pgFoundry. It just may take more proactiveness in the process.
> > > 
> > > > It
> > > > serves its current purpose well but future users might need binary
> > > > i/o or subxid support and so on. Until the interface is very stable
> > > > being in contrib makes perfect sense.
> > > > 
> > > 
> > > I would state that until the interface is very stable pgfoundry also
> > > makes perfect sense.
> > > 
> > > I am getting the impression that you think that I don't *want* this
> > > module. I do, but I do not want it at the sacrifice of other modules
> > > and code authors who did the job the right way.
> > > 
> > > I understand Tom's point about if we push to 8.4 that could cause
> > > problems for Slony and Skytools. I certainly don't want to cause
> > > problems for some very cool projects. I do however don't see those
> > > problems existing if it was in pgFoundry.
> > > 
> > > Or are we saying that the only way to provide quality sofware to
> > > PostgreSQL is if it is either in core or contrib? I do not believe you
> > > are saying that.
> > > 
> > > Sincerely,
> > > 
> > > Joshua D. Drake
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > -- 
> > > 
> > >       === The PostgreSQL Company: Command Prompt, Inc. ===
> > > Sales/Support: +1.503.667.4564   24x7/Emergency: +1.800.492.2240
> > > PostgreSQL solutions since 1997  http://www.commandprompt.com/
> > >             UNIQUE NOT NULL
> > > Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
> > > PostgreSQL Replication: http://www.commandprompt.com/products/
> > > 
> > -- End of PGP section, PGP failed!
> > 
> > -- 
> >   Bruce Momjian  <bruce@momjian.us>        http://momjian.us
> >   EnterpriseDB                             http://postgres.enterprisedb.com
> > 
> >   + If your life is a hard drive, Christ can be your backup. +
> > 
> > ---------------------------(end of broadcast)---------------------------
> > TIP 5: don't forget to increase your free space map settings

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://postgres.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

On 10/11/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> "Marko Kreen" <markokr@gmail.com> writes:
> > Could you describe bit more?  The is_visible_txid() works
> > on data returned by txid_current_snapshot()?  How can there
> > be any subtrans id's if txid_current_snapshot() wont return
> > them?
>
> Ah, I see: txid_current() never reports a subxact ID so there's no need to
> consider them elsewhere in txids either.  OK, but this desperately needs
> to be documented.

Will do.

> BTW, I notice that use of txid_current will force assignment of an XID
> in a transaction that might not otherwise have one.  Does this matter,
> or is the expectation that it's only going to be used in transactions
> that are making DB modifications anyway?

Yes, the behaviour is fine - it is meant to be used in transactions
that do modifications.  Even if not, the lazy xid assignment should
stay internal optimization detail of backend and should not be
exposed to users that clearly.

-- 
marko

On 10/10/07, Marko Kreen <markokr@gmail.com> wrote:
> On 10/10/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > * Why is txid_current_snapshot() excluding subtransaction XIDs?  That
> > might be all right for the current uses in Slony/Skytools, but it seems
> > darn close to a bug for any other use.
>
> In queue usage the transaction that stores snapshots does not do
> any other work on its own, so it does not matter, main requirement
> is that txid_current()/txid_current_snapshot() be symmetric -
> whatever the txid_current() outputs, the txid_current_snapshot() measures.
>
> But I agree, supporting subtransactions makes the API more
> universal.  And it wouldn't break Slony/PgQ current usage.

I thought about it with a clear head, and am now on optinion
that the subtransactions should be left out from current API.

I really fail to imagine a scenario where it would be useful.

The module main use comes from the scenario where txid_current(),
txid_current_snapshot() and reader of them are all different
transactions.  Main guarantee that the APi makes is that as
soon you can see a inserted snapshot in table, you can also
see all inserted events in different table.

There does not seem to be any use of them intra-transaction.

Adding them currently in would be just premature bloat.

We can do it always later, when someone presents a case for
them.  Then we can also decide whether it should be added
to current API or have parallel API besides.  It should not
break Slony/Skytools either way.

-- 
marko

On 10/10/07, Marko Kreen <markokr@gmail.com> wrote:
> On 10/10/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > * Why is txid_current_snapshot() excluding subtransaction XIDs?  That
> > might be all right for the current uses in Slony/Skytools, but it seems
> > darn close to a bug for any other use.
>
> In queue usage the transaction that stores snapshots does not do
> any other work on its own, so it does not matter, main requirement
> is that txid_current()/txid_current_snapshot() be symmetric -
> whatever the txid_current() outputs, the txid_current_snapshot() measures.
>
> But I agree, supporting subtransactions makes the API more
> universal.  And it wouldn't break Slony/PgQ current usage.

I thought about it with a clear head, and am now on optinion
that the subtransactions should be left out from current API.

I really fail to imagine a scenario where it would be useful.

The module main use comes from the scenario where txid_current(),
txid_current_snapshot() and reader of them are all different
transactions.  Main guarantee that the APi makes is that as
soon you can see a inserted snapshot in table, you can also
see all inserted events in different table.

There does not seem to be any use of them intra-transaction.

Adding them currently in would be just premature bloat.

We can do it always later, when someone presents a case for
them.  Then we can also decide whether it should be added
to current API or have parallel API besides.  It should not
break Slony/Skytools either way.

-- 
marko

---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend