WIN1252 encoding - backend or not?

"Magnus Hagander" <mha@sollentuna.net> writes:

But further down we have:
#define PG_ENCODING_BE_LAST PG_WIN1250

I told Bruce this patch was not ready for prime time ...

regards, tom lane

"Magnus Hagander" <mha@sollentuna.net> writes:

But further down we have:
#define PG_ENCODING_BE_LAST PG_WIN1250

Scarier than that is grepping the source code for -i win125, and noting
the number of places that cover the existing WIN125[016] encodings but
fail to mention WIN1252. In particular I notice that pg_wchar_table[]
in wchar.c has no entry added for WIN1252, which means that this patch
broke every encoding with higher code numbers.

Bruce, I think this patch has to come out. We should not be in the
business of debugging a new feature post-RC1.

regards, tom lane

Tom Lane wrote:

"Magnus Hagander" <mha@sollentuna.net> writes:

But further down we have:
#define PG_ENCODING_BE_LAST PG_WIN1250

Scarier than that is grepping the source code for -i win125, and noting
the number of places that cover the existing WIN125[016] encodings but
fail to mention WIN1252. In particular I notice that pg_wchar_table[]
in wchar.c has no entry added for WIN1252, which means that this patch
broke every encoding with higher code numbers.

Bruce, I think this patch has to come out. We should not be in the
business of debugging a new feature post-RC1.

OK, patch backed out and all added files removed. I will save the idea
for 8.1.

As far as the patch itself, I don't think I ever claimed it was ready
for prime time --- rather, I followed process and it was applied. If
Tom saying "it isn't ready for prime time" meant "back it out", I didn't
read it that way. Of course anyone can ask for an applied patch to be
backed out, as per procedure, and it is easily done.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman@candle.pha.pa.us> writes:

As far as the patch itself, I don't think I ever claimed it was ready
for prime time --- rather, I followed process and it was applied.

Applying it on the day before we go RC is claiming that it is ready for
prime time. As Jan already told you, the "process" may include applying
patches by default during devel cycle, but it can't work that way in
late beta.

regards, tom lane

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

As far as the patch itself, I don't think I ever claimed it was ready
for prime time --- rather, I followed process and it was applied.

Applying it on the day before we go RC is claiming that it is ready for
prime time. As Jan already told you, the "process" may include applying
patches by default during devel cycle, but it can't work that way in
late beta.

OK, so what do we want the process to be?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

On Sat, 4 Dec 2004, Bruce Momjian wrote:

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

As far as the patch itself, I don't think I ever claimed it was ready
for prime time --- rather, I followed process and it was applied.

Applying it on the day before we go RC is claiming that it is ready for
prime time. As Jan already told you, the "process" may include applying
patches by default during devel cycle, but it can't work that way in
late beta.

OK, so what do we want the process to be?

During Dev Period, Apply whatever comes through if it looks/feels okay

During Beta/Release Cycle, unless you understanding what the patch is
doing, and how it can affect other aspects of the system, check with
someone that does before applying ... don't wait for someone to speak out
against it, but email someone you feel is in the know directly and if you
don't get a response, assume the patch shouldn't be applied vs should be
...

Basically, during Beta/Release, we should almost a policy where a third
party patch needs to be approved by a second committer *before* being
applied ... and that even applies to Tom >:) Your own patch, fine ... but
a third party patch, even submitted by someone who has submitted patch
previously, should be reviewed/approved by two committers ...

----
Marc G. Fournier Hub.Org Networking Services (http://www.hub.org)
Email: scrappy@hub.org Yahoo!: yscrappy ICQ: 7615664

Hello Magnus,

Magnus Hagander schrieb am 04.12.2004 18:08:

Hi!

I was going to add WIN1252 to the installer, because I was under the
impression it was a "server side" encoding. But initdb won't accept
it...

Looking at include/mb/pg_wchar.h, I notice that:
PG_WIN1252, /* windows-1252
*/

is listed *above* the line stating /* followings are for client encoding
only */.

But further down we have:
#define PG_ENCODING_BE_LAST PG_WIN1250

One of these has to be wrong. Eitehr it's a server encoding, and then
the #define should be changed to PG_WIN1252. Or it's a client only
encoding, in which case it should be moved down to that part of the
enum. Right?

I'm the one who originally posted the patch. But the
"include/mb/pg_wchar.h" was as complete source in the ZIP attached to
my mail on 02.11.04 as well as patch in my mail on 30.11.04 with
correct contents.

And also in the confirmation of Bruce Momjian from 30.11.04 "Your patch
has been added to the PostgreSQL unapplied patches list ..." there was
the correct file contents listed.

So what happened? - How can I make sure that my patches will be applied
correctly, also for further patches?

With best regards,

Roland.

Marc G. Fournier wrote:

On Sat, 4 Dec 2004, Bruce Momjian wrote:

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

As far as the patch itself, I don't think I ever claimed it was ready
for prime time --- rather, I followed process and it was applied.

Applying it on the day before we go RC is claiming that it is ready for
prime time. As Jan already told you, the "process" may include applying
patches by default during devel cycle, but it can't work that way in
late beta.

OK, so what do we want the process to be?

During Dev Period, Apply whatever comes through if it looks/feels okay

During Beta/Release Cycle, unless you understanding what the patch is
doing, and how it can affect other aspects of the system, check with
someone that does before applying ... don't wait for someone to speak out
against it, but email someone you feel is in the know directly and if you
don't get a response, assume the patch shouldn't be applied vs should be
...

Basically, during Beta/Release, we should almost a policy where a third
party patch needs to be approved by a second committer *before* being
applied ... and that even applies to Tom >:) Your own patch, fine ... but
a third party patch, even submitted by someone who has submitted patch
previously, should be reviewed/approved by two committers ...

Please find a cure that isn't worse than the disease. I don't have time
to apply patches as it is, let alone check with someone else. You may
notice some of the patches I applied were 2-3 weeks old because I
haven't been able to keep up. Also realize that several people did
comment on the patch before it was added to the queue and it went
through several revisions, so it did get attention.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Marc G. Fournier wrote:

On Sat, 4 Dec 2004, Bruce Momjian wrote:

OK, so what do we want the process to be?

Basically, during Beta/Release, we should almost a policy where a third
party patch needs to be approved by a second committer *before* being
applied ... and that even applies to Tom >:) Your own patch, fine ... but
a third party patch, even submitted by someone who has submitted patch
previously, should be reviewed/approved by two committers ...

Please find a cure that isn't worse than the disease. I don't have time
to apply patches as it is, let alone check with someone else.

That's a fair objection, but if it means that the default is that
patches don't get applied during late beta/RC, I'm not sure I'm unhappy
with that default.

In the particular case of this patch, although Bruce said that others
had already commented on the patch, the only comments I see in the
pgpatches archives said that the patch was unreviewable because it
wasn't offered as a diff. I think it would be reasonable to insist on
at least one concurrence ("looks ok to me") posted to pgsql-patches
before applying during late beta. We've gotten into a mode where
if you like a patch you say nothing, but I wonder whether we shouldn't
change that habit.

regards, tom lane

Tom Lane wrote:

I think it would be reasonable to insist on
at least one concurrence ("looks ok to me") posted to pgsql-patches
before applying during late beta. We've gotten into a mode where
if you like a patch you say nothing, but I wonder whether we shouldn't
change that habit.

Amen, brother! That would never be tolerated in any commercial setting
that I am aware of, and should not be here either, IMNSHO. Silence does
not mean consent, it is far more likely to mean that nobody had time to
look it over.And if you commit it then surely you own it to some extent.

cheers

andrew

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Marc G. Fournier wrote:

On Sat, 4 Dec 2004, Bruce Momjian wrote:

OK, so what do we want the process to be?

Basically, during Beta/Release, we should almost a policy where a third
party patch needs to be approved by a second committer *before* being
applied ... and that even applies to Tom >:) Your own patch, fine ... but
a third party patch, even submitted by someone who has submitted patch
previously, should be reviewed/approved by two committers ...

Please find a cure that isn't worse than the disease. I don't have time
to apply patches as it is, let alone check with someone else.

That's a fair objection, but if it means that the default is that
patches don't get applied during late beta/RC, I'm not sure I'm unhappy
with that default.

The actual affect would be to make beta last longer because in fairness
to patch appliers you have to give each patch a reasonable chance of
being applied. While it is temping to say, "No one was around to review
your patch so we couldnt' apply it", it isn't going to be received very
well by the submitters. This was the issue we had in getting 8.0 to
beta, if you remember.

In the particular case of this patch, although Bruce said that others
had already commented on the patch, the only comments I see in the
pgpatches archives said that the patch was unreviewable because it

Alvaro mentioned a incorrect comment:

http://archives.postgresql.org/pgsql-patches/2004-11/msg00344.php

That's the only correction I remember, aside from the diff vs. new file
discussion.

wasn't offered as a diff. I think it would be reasonable to insist on
at least one concurrence ("looks ok to me") posted to pgsql-patches
before applying during late beta. We've gotten into a mode where
if you like a patch you say nothing, but I wonder whether we shouldn't
change that habit.

If I get a "looks good to me" email I am much more likely to apply the
patch promptly, that's for sure. Lacking that, if I have seen someone
comment on the patch and a new version was generated, I assume it is OK.
Now, if I don't understand the patch, we can change the procedure so I
require someone to state it is OK rather than the fallback of quiet
acceptance, especially just before a beta or RC version.

Also, because the bad patch got in there is a temptation to believe our
process is flawed, but that assumes that a perfect procedure exists. A
perfect procedure doesn't exist because the world isn't perfect. We can
make adjustments, but never expect any process to be fool-proof.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Andrew Dunstan wrote:

Tom Lane wrote:

I think it would be reasonable to insist on
at least one concurrence ("looks ok to me") posted to pgsql-patches
before applying during late beta. We've gotten into a mode where
if you like a patch you say nothing, but I wonder whether we shouldn't
change that habit.

Amen, brother! That would never be tolerated in any commercial setting
that I am aware of, and should not be here either, IMNSHO. Silence does
not mean consent, it is far more likely to mean that nobody had time to
look it over.And if you commit it then surely you own it to some extent.

And your point is what? What suggestion for improvement do you have?
Have perfect knowledge of what patches will be bad and don't apply them?

You want me to claim ownership? Of what? Of applying the patch?
Everyone already knows that. Of the patch being bad? Everyone already
knows that too? What I shouldn't have applied it? Also known. But
what good does that do us now?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Now, if I don't understand the patch, we can change the procedure so I
require someone to state it is OK rather than the fallback of quiet
acceptance, especially just before a beta or RC version.

I am not suggesting that we need to tighten up during devel cycle,
and maybe not during early beta. But once we approach RC I think
we need a tighter process. We really want a "get it right the first
time" mentality to apply at this point, whereas during development
there's always time to catch problems later.

Also, because the bad patch got in there is a temptation to believe our
process is flawed,

Don't forget you applied *two* very questionable patches on Thursday.
Had it been only one I'm not sure there'd be this degree of unhappiness.

regards, tom lane

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Now, if I don't understand the patch, we can change the procedure so I
require someone to state it is OK rather than the fallback of quiet
acceptance, especially just before a beta or RC version.

I am not suggesting that we need to tighten up during devel cycle,
and maybe not during early beta. But once we approach RC I think
we need a tighter process. We really want a "get it right the first
time" mentality to apply at this point, whereas during development
there's always time to catch problems later.

OK, makes sense. Maybe we shouldn't have gone to RC so quickly but
given time to get reviews on those patches.

Also, because the bad patch got in there is a temptation to believe our
process is flawed,

Don't forget you applied *two* very questionable patches on Thursday.
Had it been only one I'm not sure there'd be this degree of unhappiness.

It was three weeks of back patches so two is not surprising, especially
related to encodings, which I don't understand. You know, Peter has
dealt with all the translation encodings for a long time. It would be
good for someone who understands encodings to take ownership of the
encoding patches.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian wrote:

Andrew Dunstan wrote:

Tom Lane wrote:

I think it would be reasonable to insist on
at least one concurrence ("looks ok to me") posted to pgsql-patches
before applying during late beta. We've gotten into a mode where
if you like a patch you say nothing, but I wonder whether we shouldn't
change that habit.

Amen, brother! That would never be tolerated in any commercial setting
that I am aware of, and should not be here either, IMNSHO. Silence does
not mean consent, it is far more likely to mean that nobody had time to
look it over.And if you commit it then surely you own it to some extent.

And your point is what? What suggestion for improvement do you have?
Have perfect knowledge of what patches will be bad and don't apply them?

You want me to claim ownership? Of what? Of applying the patch?
Everyone already knows that. Of the patch being bad? Everyone already
knows that too? What I shouldn't have applied it? Also known. But
what good does that do us now?

Bruce,

I'm sorry if I offended you.

As for suggestions, elsewhere you wrote:

"Now, if I don't understand the patch, we can change the procedure so I
require someone to state it is OK rather than the fallback of quiet
acceptance, especially just before a beta or RC version."

Take that as my suggested improvement.

Keep up the good work as always - you know we are grateful for it.

cheers

andrew

Andrew Dunstan wrote:

I'm sorry if I offended you.

As for suggestions, elsewhere you wrote:

"Now, if I don't understand the patch, we can change the procedure so I
require someone to state it is OK rather than the fallback of quiet
acceptance, especially just before a beta or RC version."

Take that as my suggested improvement.

Yes, that is what I am thinking, though I am hoping someone who
understands encodings will take ownership of all those patches like
Peter did with translations. If I don't understand it even simple
things can go wrong with patch application.

Keep up the good work as always - you know we are grateful for it.

Thanks.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

On Sun, 5 Dec 2004, Andrew Dunstan wrote:

As for suggestions, elsewhere you wrote:

"Now, if I don't understand the patch, we can change the procedure so I
require someone to state it is OK rather than the fallback of quiet
acceptance, especially just before a beta or RC version."

I believe that this was the direction Tom and I were looking at as well
...

----
Marc G. Fournier Hub.Org Networking Services (http://www.hub.org)
Email: scrappy@hub.org Yahoo!: yscrappy ICQ: 7615664

Bruce Momjian wrote:

Peter has dealt with all the translation encodings for a long time.
It would be good for someone who understands encodings to take
ownership of the encoding patches.

I would have dealt with this patch if someone had submitted a patch as I
had requested.

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

Peter Eisentraut wrote:

Bruce Momjian wrote:

Peter has dealt with all the translation encodings for a long time.
It would be good for someone who understands encodings to take
ownership of the encoding patches.

I would have dealt with this patch if someone had submitted a patch as I
had requested.

OK, what did he do wrong? The patch in the queue as a diff of the
changed files plus the new files attached.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

On Sun, Dec 05, 2004 at 04:06:57PM -0500, Andrew Dunstan wrote:

Amen, brother! That would never be tolerated in any commercial setting
that I am aware of, and should not be here either, IMNSHO. Silence does

I don't know what commercial settings you're familiar with, but I can
think of some where it _would_ be tolerated. That toleration,
however, certainly explains a number of really crappy pieces of
software I've had to work with.

A

--
Andrew Sullivan | ajs@crankycanuck.ca
A certain description of men are for getting out of debt, yet are
against all taxes for raising money to pay it off.
--Alexander Hamilton

On Mon, Dec 06, 2004 at 04:00:30PM -0500, Andrew Sullivan wrote:

however, certainly explains a number of really crappy pieces of
software I've had to work with.

Uh, that's not a swipe at Bruce -- rather praise for everyone
involved for being frank enough to discuss this.

A

--
Andrew Sullivan | ajs@crankycanuck.ca
Information security isn't a technological problem. It's an economics
problem.
--Bruce Schneier