Bruce Momjian <bruce@momjian.us> writes:
> ... The GUC comment/default patch had tons of
> emails, but no other committers got involved to review or commit the
> patch. Peter, who knows GUC well, looked at it, but said he didn't
> review it enough.

Peter has made it pretty clear that he didn't care for the
refactorization aspect of that patch.

> I just spent 1/2 hour fixing the multi-value UPDATE
> patch for the code drift caused by UPDATE/RETURNING. The patch is a
> simple grammar macro. Any coder could have taken that, reviewed it, and
> applied it, but no one did.

Perhaps that's because nobody but you wanted it to go in.

Some amount of the issue here is that people won't work on patches they
don't approve of; that's certainly the case for me. I have more than
enough to do working on patches I do think should go in, and I get tired
of having to repeatedly object to the same bad patch. Do you remember
Sturgeon's Law? It applies to patches too.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > ... The GUC comment/default patch had tons of
> > emails, but no other committers got involved to review or commit the
> > patch. Peter, who knows GUC well, looked at it, but said he didn't
> > review it enough.
>
> Peter has made it pretty clear that he didn't care for the
> refactorization aspect of that patch.

Peter asked why it was done, a good answer was given, and Peter did not
reply.

> > I just spent 1/2 hour fixing the multi-value UPDATE
> > patch for the code drift caused by UPDATE/RETURNING. The patch is a
> > simple grammar macro. Any coder could have taken that, reviewed it, and
> > applied it, but no one did.
>
> Perhaps that's because nobody but you wanted it to go in.

We got tons of people who wanted that.

> Some amount of the issue here is that people won't work on patches they
> don't approve of; that's certainly the case for me. I have more than
> enough to do working on patches I do think should go in, and I get tired
> of having to repeatedly object to the same bad patch. Do you remember
> Sturgeon's Law? It applies to patches too.

Sure, you have to want the patch to be in to be motivated to work on it.
I think I am more willing to work with imperfection.

--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.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

>>> I just spent 1/2 hour fixing the multi-value UPDATE
>>> patch for the code drift caused by UPDATE/RETURNING. The patch is a
>>> simple grammar macro. Any coder could have taken that, reviewed it, and
>>> applied it, but no one did.
>> Perhaps that's because nobody but you wanted it to go in.
>
> We got tons of people who wanted that.

To further this, I didn't even know it was an issue. If it was only half
an hour and it needed to be done, why wasn't it put out there?

Thanks goes to Bruce for fixing it but I didn't know it was an issue, I
have 5 C developers, if any of them could have done it -- then some
communication is needed and I would have allocated someone to it.

I am sure that is the case with others as well.

I am not saying it is anyone's responsibility to speak up, but I can say
had I known some help was needed (especially something so trivial) I
would have gladly donated some time.

Sincerely,

Joshua D. Drake




--

=== The PostgreSQL Company: Command Prompt, Inc. ===
Sales/Support: +1.503.667.4564 || 24x7/Emergency: +1.800.492.2240
Providing the most comprehensive PostgreSQL solutions since 1997
http://www.commandprompt.com/



---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

Bruce Momjian <bruce@momjian.us> writes:
> Tom Lane wrote:
>> Peter has made it pretty clear that he didn't care for the
>> refactorization aspect of that patch.

> Peter asked why it was done, a good answer was given, and Peter did not
> reply.

Au contraire, he's reiterated since then that he didn't like it.

>> Perhaps that's because nobody but you wanted it to go in.

> We got tons of people who wanted that.

But no committers, else it would've got done.

There was some remark upthread about reviewers getting discouraged
because their comments seem to fall on deaf ears. I know exactly
what is meant by that. I'm getting tired of arguing with you about
bad patches, because it's obvious that you put no weight on my
objections --- and it looks like Peter has got the same problem.
How is it that you are willing to apply submissions from newbie
developers over the objections of core developers?

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > Tom Lane wrote:
> >> Peter has made it pretty clear that he didn't care for the
> >> refactorization aspect of that patch.
>
> > Peter asked why it was done, a good answer was given, and Peter did not
> > reply.
>
> Au contraire, he's reiterated since then that he didn't like it.

The thread order was: patch, Peter comments, submitter gives reasons,
patch put in the queue, Peter comments again, I reply that the change is
not just "refactoring" but is needed based on submitters comments, and
no reply from Peter:

http://archives.postgresql.org/pgsql...8/msg00334.php

Without a reply from Peter, I have to assume the patch is valid.

> >> Perhaps that's because nobody but you wanted it to go in.
>
> > We got tons of people who wanted that.
>
> But no committers, else it would've got done.

True that it was of more interest to non-committers than committers,
probably because it is the committers who are going to have to remove it
when full SQL functionality is added. :-}

> There was some remark upthread about reviewers getting discouraged
> because their comments seem to fall on deaf ears. I know exactly
> what is meant by that. I'm getting tired of arguing with you about
> bad patches, because it's obvious that you put no weight on my
> objections --- and it looks like Peter has got the same problem.
> How is it that you are willing to apply submissions from newbie
> developers over the objections of core developers?

Well, as I see it, core developers have no special weight in votes,
except by their ability to influence the community by their posted
opinions. If we want core people to have a larger weight in making
decisions, I think the community should decide that. I have never heard
that stated by anyone in this group.

I think the UPDATE ... SET (val, ...) patch is illustrative. Some
committers didn't want it because it didn't add new functionality, and
because it has to be remove later when full functionality is added. But
a lot of users wanted it because it implemented part of the spec, and
because it will make porting easier. Based on how many users wanted it,
and because the patch is very localized (gram.y), I thought it should be
in 8.2. The Win32 port is an extreme case of this dichotomy ---
headache for developers, boon for the user community.

We all started as newbie patch submitters (some better than others, of
course). I think as leaders if we don't like a patch for some reason,
we have to give feedback on how the submitter can fix it, or
community-agreed reasons why the patch will never be accepted. Another
example is the FETCH/MOVE int64 patch. If there is a performance
concern, should we just ask the submitters to do testing. You mentioned
there is a way to do MOVE >2gig already, but did not supply the method.
I can't figure out what it is, and I am sure others don't know either.
If we can do it, that should be stated so we can document it and all
understand why FETCH/MOVE int64 isn't a good addition.

Basically, the people who have been around longer have to spend time to
educate patch submitters to improve. If we don't, we don't grow our
pool of experienced developers.

The fundamental problem is if someone objects to a patch, and then
someone reasonably replies to that objection, lack of a reply means to
me that the last reply was valid. I realize this places a burden on
people to discuss patches, but I see no better way to do it. I don't
think we want to go down the road where we have individuals who object
to patches, but are not required to engage in discussion about the
patch. I think many dysfunctional open source projects have this
problem.

On a personal note, Tom, I do place great weight on your opinions, and
try to adjust things to make sure you, the submitter, and everyone else
is satisified with the result.

--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.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

Joshua D. Drake wrote:
>
> >>> I just spent 1/2 hour fixing the multi-value UPDATE
> >>> patch for the code drift caused by UPDATE/RETURNING. The patch is a
> >>> simple grammar macro. Any coder could have taken that, reviewed it, and
> >>> applied it, but no one did.
> >> Perhaps that's because nobody but you wanted it to go in.
> >
> > We got tons of people who wanted that.
>
> To further this, I didn't even know it was an issue. If it was only half
> an hour and it needed to be done, why wasn't it put out there?

I did, sort of:

http://archives.postgresql.org/pgsql...8/msg00869.php

The author replied to me privately, and I said I would adjust it.

> Thanks goes to Bruce for fixing it but I didn't know it was an issue, I
> have 5 C developers, if any of them could have done it -- then some
> communication is needed and I would have allocated someone to it.
>
> I am sure that is the case with others as well.
>
> I am not saying it is anyone's responsibility to speak up, but I can say
> had I known some help was needed (especially something so trivial) I
> would have gladly donated some time.

The patch also required a little adjustment, as I posted, so it wasn't a
trival issue you could have just thrown at someone.

--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com

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

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

Bruce Momjian wrote:
> Without a reply from Peter, I have to assume the patch is valid.

To make it more explicit: I think the patch is stupid, but if someone
wants to review it, go ahead. But I am not comfortable with the "if no
one objects, I'll just commit it" mode that is sometimes going on. Has
anyone actually tested the patch?

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

---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq

Peter Eisentraut wrote:
> Bruce Momjian wrote:
> > Without a reply from Peter, I have to assume the patch is valid.
>
> To make it more explicit: I think the patch is stupid, but if someone
> wants to review it, go ahead. But I am not comfortable with the "if no
> one objects, I'll just commit it" mode that is sometimes going on. Has
> anyone actually tested the patch?

Are you saying you don't like the patch, or the goal of resetting the
value to their defaults when they are commented out? If the first, do
you have a suggestion, if the later, it is something confuses many
users.

--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com

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

---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

Am Montag, 4. September 2006 04:10 schrieb Bruce Momjian:
> Are you saying you don't like the patch,

That's it.

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

---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

Peter Eisentraut <peter_e@gmx.net> writes:
> Bruce Momjian wrote:
>> Without a reply from Peter, I have to assume the patch is valid.

> To make it more explicit: I think the patch is stupid, but if someone
> wants to review it, go ahead. But I am not comfortable with the "if no
> one objects, I'll just commit it" mode that is sometimes going on. Has
> anyone actually tested the patch?

Perhaps more to the point: a refactorization patch is all about beauty
in the eye of the beholder. If Peter, the original author of the guc
code, thinks that it's a disimprovement, I think it's a hard argument
to make that the patch should go in anyway.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings