primary key error message

On Thu, Jan 21, 2010 at 3:30 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

Here is a small patch that changes the error message

   duplicate key value violates unique constraint "%s"

into

   duplicate key value violates primary key "%s"

when the constraint is in fact a primary key.

Comments?

Why bother? And why bother now, when we're in the middle of the last
CommitFest and trying to move toward a release?

...Robert

Robert Haas <robertmhaas@gmail.com> writes:

On Thu, Jan 21, 2010 at 3:30 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

Here is a small patch that changes the error message

� �duplicate key value violates unique constraint "%s"

into

� �duplicate key value violates primary key "%s"

when the constraint is in fact a primary key.

Comments?

Why bother? And why bother now, when we're in the middle of the last
CommitFest and trying to move toward a release?

This patch fails to cover all cases (index build being the obvious
omission, but I think there might be other paths as well where the
information is not so readily available). And I agree with Robert
that the usefulness is at best highly debatable.

regards, tom lane

On tor, 2010-01-21 at 15:51 -0500, Tom Lane wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Thu, Jan 21, 2010 at 3:30 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

Here is a small patch that changes the error message

duplicate key value violates unique constraint "%s"

into

duplicate key value violates primary key "%s"

when the constraint is in fact a primary key.

Comments?

Why bother? And why bother now, when we're in the middle of the last
CommitFest and trying to move toward a release?

This patch fails to cover all cases (index build being the obvious
omission, but I think there might be other paths as well where the
information is not so readily available).

This is the user-visible error message, and that's the only place it's
generated.

And I agree with Robert that the usefulness is at best highly debatable.

The usefulness is that they are different kinds of objects that are
defined and listed in different ways and it would be slightly helpful if
the error message pointed in the righter direction.

On tor, 2010-01-21 at 15:47 -0500, Robert Haas wrote:

On Thu, Jan 21, 2010 at 3:30 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

Here is a small patch that changes the error message

duplicate key value violates unique constraint "%s"

into

duplicate key value violates primary key "%s"

when the constraint is in fact a primary key.

Comments?

Why bother?

Because unique constraints and primary keys are different things and it
would be slightly less confusing that way.

And why bother now, when we're in the middle of the last
CommitFest and trying to move toward a release?

Are you saying we can't discuss anything new during the commit fest?

Peter Eisentraut <peter_e@gmx.net> writes:

On tor, 2010-01-21 at 15:51 -0500, Tom Lane wrote:

This patch fails to cover all cases (index build being the obvious
omission, but I think there might be other paths as well where the
information is not so readily available).

This is the user-visible error message, and that's the only place it's
generated.

Hardly ...

regression=# create table foo (f1 int);
CREATE TABLE
regression=# insert into foo values(1),(1);
INSERT 0 2
regression=# alter table foo add primary key (f1);
NOTICE: ALTER TABLE / ADD PRIMARY KEY will create implicit index "foo_pkey" for table "foo"
ERROR: could not create unique index "foo_pkey"
DETAIL: Key (f1)=(1) is duplicated.

And I agree with Robert that the usefulness is at best highly debatable.

The usefulness is that they are different kinds of objects that are
defined and listed in different ways and it would be slightly helpful if
the error message pointed in the righter direction.

It is not incorrect to refer to a primary key as a unique constraint.
Will you next be wanting the error messages about null values to
distinguish whether the NOT NULL constraint came from a primary key?

regards, tom lane

On tor, 2010-01-21 at 16:29 -0500, Tom Lane wrote:

regression=# alter table foo add primary key (f1);
NOTICE: ALTER TABLE / ADD PRIMARY KEY will create implicit index "foo_pkey" for table "foo"
ERROR: could not create unique index "foo_pkey"
DETAIL: Key (f1)=(1) is duplicated.

He he, that one doesn't even refer to a "constraint". Might also need
fixing. ;-)

It is not incorrect to refer to a primary key as a unique constraint.

Some things need a primary key that is not only a unique constraint. So
having some clarity about that can be helpful.

Will you next be wanting the error messages about null values to
distinguish whether the NOT NULL constraint came from a primary key?

Hadn't thought of that, but it could actually become relevant when we
get named not null constraints.

But anyway, if there is no interest in this, then forget about it.

On Thu, Jan 21, 2010 at 4:24 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

Why bother?

Because unique constraints and primary keys are different things and it
would be slightly less confusing that way.

I don't really see why it would be any less confusing. You could
argue that someone might not know that the primary key acts like a
unique constraint, but if they don't, then they might be confused
about what it means to violate it.

Generally if the user is looking at something like \d they will see
the primary key as well as any unique indices. And the name of the
primary key will match the name from this error message. So I'm also
not sure how someone could get pointed in the wrong direction in the
first place.

And why bother now, when we're in the middle of the last
CommitFest and trying to move toward a release?

Are you saying we can't discuss anything new during the commit fest?

Of course, I have no control over what anyone discusses. This is a
community and no one has a boss. But let me ask this. For which
release were you hoping to make this change? If 9.0, then it seems to
me that you've missed the deadline, which - according to my
understanding of the agreed-upon schedule - was six days ago. Or
perhaps you feel that that deadline should only apply to
non-committers? If so, we should be clear about that, because I have
a few things which I would have liked to submit but was unable to get
done before the start of the CommmitFest. I would be more than happy
to finish them up and propose them now, but my understanding is that
I'm not supposed to do that.

On the other hand, if you were hoping to make this change for 9.1,
then obviously there is no issue of a deadline. However, there is
still the issue of the CommitFest mandate, of which you are a
proponent, which, in your exact words, says that one should not "work
on new patches" during CommitFest and beta.

http://archives.postgresql.org/pgsql-hackers/2009-06/msg01514.php

I have previously expressed the view you might have been overreaching
a bit in that email, because I don't think it's realistic to say that
there is useful work for everyone to do during the entire time that we
are in beta, and if there is nothing someone can do to get the release
out sooner, then I think it's fine for them to work on their own
patches. But that certainly isn't the case right now.

...Robert

On tor, 2010-01-21 at 17:06 -0500, Robert Haas wrote:

But let me ask this. For which
release were you hoping to make this change? If 9.0, then it seems to
me that you've missed the deadline, which - according to my
understanding of the agreed-upon schedule - was six days ago.

By that logic, the next release must be called 8.5, because the deadline
for proposing changes was six days ago.

Or perhaps you feel that that deadline should only apply to
non-committers? If so, we should be clear about that, because I have
a few things which I would have liked to submit but was unable to get
done before the start of the CommmitFest. I would be more than happy
to finish them up and propose them now, but my understanding is that
I'm not supposed to do that.

My understanding is that the commit fest process is an offer or perhaps
even a promise to patch submitters that their stuff will be attended to
within 2 or 3 months, instead of the 10 months or infinity that might
have been the previous average. And in order to make that "attending"
happen, the development participants are encouraged to focus on
reviewing the submitted patches.

But I don't think that should mean everyone has to drop everything when
the clock strikes midnight and must now only look at things that the
magic commitfest page has pre-approved. Nobody does that anyway. It
just means what you submit now does not get the same "promise" of
attention. Of course if you start proposing new significant features
now then it might be obvious that the discussion and review process
cannot possibly be concluded by the time the release is scheduled, so
you might as well not bother. But if things have been discussed before
or are simple enough and you just didn't get the thing done in time, why
not finish it up. I don't think anyone could accuse you of neglecting
the CF, as you are known to do your share. So I personally encourage
you to try to finish what you have started. If no one wants to review
it and you don't want to take responsibility yourself, well then. And
if someone proposes something that might be as simple as the MySQL
compatibility thing, assuming a consensus, why not include it, instead
of bumping it to 2012-Next for the sake of the process.

Peter Eisentraut wrote:

But I don't think that should mean everyone has to drop everything when
the clock strikes midnight and must now only look at things that the
magic commitfest page has pre-approved.

Well, we used to have the idea of a feature freeze ... is that going to
apply at the end of the commitfest?

I generally agree that we need to have a bit of wiggle room at this
stage - small and non-controversial items can be allowed to creep in still.

cheers

andrew

On Jan 21, 2010, at 3:05 PM, Andrew Dunstan wrote:

Well, we used to have the idea of a feature freeze ... is that going to apply at the end of the commitfest?

I generally agree that we need to have a bit of wiggle room at this stage - small and non-controversial items can be allowed to creep in still.

Seems to me we can have discussion anytime, though a given patch might get more or less discussion at various times. But clearly something like this would just be entered for the first 9.1 commitfest, no?

Best,

David

Andrew Dunstan <andrew@dunslane.net> writes:

Peter Eisentraut wrote:

But I don't think that should mean everyone has to drop everything when
the clock strikes midnight and must now only look at things that the
magic commitfest page has pre-approved.

Well, we used to have the idea of a feature freeze ... is that going to
apply at the end of the commitfest?

Even a feature freeze would not IMO prevent considering the sort of
small adjustment Peter was suggesting. We will doubtless be making
far larger adjustments than that even quite late in the release cycle.
(Particularly in the new HS/SR code.)

I thought his patch wasn't a particularly good idea, but I didn't have
a problem with it from a schedule or process standpoint.

If you want an example of something I *do* have a process problem
with, it's Kevin Grittner's attempts to get people to put a
significant number of cycles into thinking about true serializability.
Right now is not the time for that to be happening. I've been
politely ignoring that thread, but ...

regards, tom lane

Tom Lane <tgl@sss.pgh.pa.us> wrote:

If you want an example of something I *do* have a process problem
with, it's Kevin Grittner's attempts to get people to put a
significant number of cycles into thinking about true
serializability.
Right now is not the time for that to be happening. I've been
politely ignoring that thread, but ...

Not my motive, but if you think that'll be the effect of posting
milestone patches, I'll hold 'em back. Apologies if I caused a
disruption.

-Kevin

Tom Lane <tgl@sss.pgh.pa.us> wrote:

If you want an example of something I *do* have a process problem
with, it's Kevin Grittner's attempts

Hmmm.... Plural? I've made exactly one post on the subject since
the CF started, unless you count review of Markus's dtester code,
which he posted before the CF but didn't add to the CF page. Is
reviewing that a process violation? Or was discussing it before the
CF the process issue?

After looking at my emails, I'm honestly confused. If it was the
milestone patch, and your plural was a slip, I'll hold off on
further such posts. If it's something else, I want to understand
what.

-Kevin

"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:

Tom Lane <tgl@sss.pgh.pa.us> wrote:

If you want an example of something I *do* have a process problem
with, it's Kevin Grittner's attempts

Hmmm.... Plural? I've made exactly one post on the subject since
the CF started, unless you count review of Markus's dtester code,
which he posted before the CF but didn't add to the CF page. Is
reviewing that a process violation? Or was discussing it before the
CF the process issue?

I thought the whole topic should have been held off till after the CF,
probably till after the bulk of beta testing work is done. It's a
sufficiently large and difficult problem that nobody can really give you
any meaningful feedback without taking more time away from our current
set of problems than I think is appropriate.

Now your original posts back in December were okay, since you were just
letting people know that you intended to work on this over a long
period. But IIRC you've made more than one post with actual code in it
that you seemed to be hoping people would review, and that I thought
was a distraction at this late stage of the cycle.

regards, tom lane

On Thu, Jan 21, 2010 at 7:11 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:

Tom Lane <tgl@sss.pgh.pa.us> wrote:

If you want an example of something I *do* have a process problem
with, it's Kevin Grittner's attempts

Hmmm....  Plural?  I've made exactly one post on the subject since
the CF started, unless you count review of Markus's dtester code,
which he posted before the CF but didn't add to the CF page.  Is
reviewing that a process violation?  Or was discussing it before the
CF the process issue?

I thought the whole topic should have been held off till after the CF,
probably till after the bulk of beta testing work is done.  It's a
sufficiently large and difficult problem that nobody can really give you
any meaningful feedback without taking more time away from our current
set of problems than I think is appropriate.

Now your original posts back in December were okay, since you were just
letting people know that you intended to work on this over a long
period.  But IIRC you've made more than one post with actual code in it
that you seemed to be hoping people would review, and that I thought
was a distraction at this late stage of the cycle.

I actually thought that was a good thing to do, versus developing
totally out of site, though I admit I have had zero time to read the
code, and probably won't for a while.

...Robert

On Thu, Jan 21, 2010 at 5:35 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

On tor, 2010-01-21 at 17:06 -0500, Robert Haas wrote:

But let me ask this.  For which
release were you hoping to make this change?  If 9.0, then it seems to
me that you've missed the deadline, which - according to my
understanding of the agreed-upon schedule - was six days ago.

By that logic, the next release must be called 8.5, because the deadline
for proposing changes was six days ago.

Well, I was assuming we were talking about feature freeze rather than
"no one can ever commit anything", or we'd never get to a release,
which is the point of this exercise AIUI.

Or perhaps you feel that that deadline should only apply to
non-committers?  If so, we should be clear about that, because I have
a few things which I would have liked to submit but was unable to get
done before the start of the CommmitFest.  I would be more than happy
to finish them up and propose them now, but my understanding is that
I'm not supposed to do that.

My understanding is that the commit fest process is an offer or perhaps
even a promise to patch submitters that their stuff will be attended to
within 2 or 3 months, instead of the 10 months or infinity that might
have been the previous average.  And in order to make that "attending"
happen, the development participants are encouraged to focus on
reviewing the submitted patches.

Right. I agree.

But I don't think that should mean everyone has to drop everything when
the clock strikes midnight and must now only look at things that the
magic commitfest page has pre-approved.  Nobody does that anyway.  It

I don't believe that something being on the CommitFest page implies
any sort of approval. It just expresses the desire of the submitter
for it to be reviewed.

just means what you submit now does not get the same "promise" of
attention.  Of course if you start proposing new significant features
now then it might be obvious that the discussion and review process
cannot possibly be concluded by the time the release is scheduled, so
you might as well not bother.  But if things have been discussed before
or are simple enough and you just didn't get the thing done in time, why
not finish it up.  I don't think anyone could accuse you of neglecting
the CF, as you are known to do your share.  So I personally encourage
you to try to finish what you have started.  If no one wants to review
it and you don't want to take responsibility yourself, well then.  And

Well, that does seem to be endorsing a sort of two-tiered system. If
I submit a patch and nobody looks at it, I can decide that silence
means approval and commit it. If someone who is not a committer does
the same thing, it dies, no matter how technically excellent it is. I
am no longer in a position to be bothered by that, but I think if I
were not a committer I might be. I wonder what others think about
this.

There's another issue, too. If a committer submits a patch, everybody
else who cares about the issue has to drop what they're doing and look
at it. Because if they don't, there's a good chance that in 24 hours
plus or minus, it'll be in the tree. Several patches have blown by me
in the last month or two - already committed before I got around to
reading them, and I might have had an opinion on them, but it's too
late to do anything about it now. I mean, it's not, really: I could
still ask for something to be changed, but it's an uphill battle at
this point.

if someone proposes something that might be as simple as the MySQL
compatibility thing, assuming a consensus, why not include it, instead
of bumping it to 2012-Next for the sake of the process.

I agree to a certain extent, but if you are concerned about our
release cycle being too long, as indicated by your use of the name
2012-Next, proposing a whole series of changes for changes during the
last CommitFest may not be the best way to get there. Maybe they're
small enough that it doesn't matter much, but I still think it would
have been easier to deal with two weeks ago when we had a lot less
going on. Anyway, I just work here. It's certainly not the end of
the world... and there are certainly other things which are going to
delay the release by a lot more than this will.

...Robert

Tom Lane wrote:

Now your original posts back in December were okay, since you were
just letting people know that you intended to work on this over a
long period. But IIRC you've made more than one post with actual
code in it that you seemed to be hoping people would review, and
that I thought was a distraction at this late stage of the cycle.

Well, I've gotten to the first three milestones and have posted a
patch for each. The first two were before the start of the CF, but I
intentionally *didn't* add them to the CF and specifically asked
people *not* to let them divert efforts from the release, which has
apparently been respected. It wasn't a disingenuous request.
Robert, in particular, said before the CF that he would look at the
first patch when I had it ready, and has apparently not done so yet
because of other priorities, which is as I would wish.

I hit the third milestone after the start of the CF and posted it
"for the record". If people find that distracting, I'll hold off
until I get some indication that it won't be. I think it's a bit
unfair to single me out because of that one post, but I think I can
handle it. :-) I just want to make sure I'm not missing some gaff I
didn't realize I was making. Given how small a portion of each year
is actually officially open for feedback, I'd hate to think that
posting a couple WIP patches during that narrow window was considered
a breach of protocol.

-Kevin

On tor, 2010-01-21 at 18:05 -0500, Andrew Dunstan wrote:

Well, we used to have the idea of a feature freeze ... is that going
to apply at the end of the commitfest?

Feature freeze was used to discourage the submission of very big patches
shortly before beta. The commit fest process has IMO alleviated this
concern. Beta is still the definite cutoff; and the closer we get to
beta, the smaller the acceptable changes become. I think that formula
basically applies throughout the entire cycle.

On tor, 2010-01-21 at 19:45 -0500, Robert Haas wrote:

Well, that does seem to be endorsing a sort of two-tiered system.

In those words, yes, it's a multi-tiered system. The aim of the commit
fests is to make the "lower" tier more effective, but not necessarily to
bring the "upper" tier to a near halt.

If I submit a patch and nobody looks at it, I can decide that silence
means approval and commit it. If someone who is not a committer does
the same thing, it dies, no matter how technically excellent it is. I
am no longer in a position to be bothered by that, but I think if I
were not a committer I might be. I wonder what others think about
this.

Well, you have worked hard to get to that position, so those are the
perks.

There's another issue, too. If a committer submits a patch, everybody
else who cares about the issue has to drop what they're doing and look
at it. Because if they don't, there's a good chance that in 24 hours
plus or minus, it'll be in the tree. Several patches have blown by me
in the last month or two - already committed before I got around to
reading them, and I might have had an opinion on them, but it's too
late to do anything about it now. I mean, it's not, really: I could
still ask for something to be changed, but it's an uphill battle at
this point.

That would seem to ask that all committers funnel their patches through
the commit fest process. That might technically and morally be the
right thing, but it would probably not be a popular or realistic
proposal.