Обсуждение: WIN1252 encoding - backend or not?
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? //Magnus
"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 Index: src/backend/utils/mb/encnames.c =================================================================== RCS file: /cvsroot/pgsql/src/backend/utils/mb/encnames.c,v retrieving revision 1.20 retrieving revision 1.21 diff -c -c -r1.20 -r1.21 *** src/backend/utils/mb/encnames.c 27 Sep 2004 23:24:33 -0000 1.20 --- src/backend/utils/mb/encnames.c 2 Dec 2004 22:14:36 -0000 1.21 *************** *** 2,8 **** * Encoding names and routines for work with it. All * in this file is shared bedween FE and BE. * ! * $PostgreSQL: pgsql/src/backend/utils/mb/encnames.c,v 1.20 2004/09/27 23:24:33 momjian Exp $ */ #ifdef FRONTEND #include "postgres_fe.h" --- 2,8 ---- * Encoding names and routines for work with it. All * in this file is shared bedween FE and BE. * ! * $PostgreSQL: pgsql/src/backend/utils/mb/encnames.c,v 1.21 2004/12/02 22:14:36 momjian Exp $ */ #ifdef FRONTEND #include "postgres_fe.h" *************** *** 194,199 **** --- 194,202 ---- "win1251", PG_WIN1251 }, /* alias for Windows-1251 */ { + "win1252", PG_WIN1252 + }, /* alias for Windows-1252 */ + { "win1256", PG_WIN1256 }, /* alias for Windows-1256 */ { *************** *** 221,226 **** --- 224,232 ---- "windows1251", PG_WIN1251 }, /* Windows-1251; Microsoft */ { + "windows1252", PG_WIN1252 + }, /* Windows-1252; Microsoft */ + { "windows1256", PG_WIN1256 }, /* Windows-1256; Microsoft */ { *************** *** 344,349 **** --- 350,358 ---- "WIN1250", PG_WIN1250 }, { + "WIN1252", PG_WIN1252 + }, + { "SJIS", PG_SJIS }, { Index: src/include/mb/pg_wchar.h =================================================================== RCS file: /cvsroot/pgsql/src/include/mb/pg_wchar.h,v retrieving revision 1.52 retrieving revision 1.53 diff -c -c -r1.52 -r1.53 *** src/include/mb/pg_wchar.h 17 Sep 2004 21:59:57 -0000 1.52 --- src/include/mb/pg_wchar.h 2 Dec 2004 22:14:38 -0000 1.53 *************** *** 1,4 **** ! /* $PostgreSQL: pgsql/src/include/mb/pg_wchar.h,v 1.52 2004/09/17 21:59:57 petere Exp $ */ #ifndef PG_WCHAR_H #define PG_WCHAR_H --- 1,4 ---- ! /* $PostgreSQL: pgsql/src/include/mb/pg_wchar.h,v 1.53 2004/12/02 22:14:38 momjian Exp $ */ #ifndef PG_WCHAR_H #define PG_WCHAR_H *************** *** 178,183 **** --- 178,184 ---- PG_ISO_8859_7, /* ISO-8859-7 */ PG_ISO_8859_8, /* ISO-8859-8 */ PG_WIN1250, /* windows-1250 */ + PG_WIN1252, /* windows-1252 */ /* followings are for client encoding only */ PG_SJIS, /* Shift JIS (Winindows-932) */
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, Pennsylvania19073
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, Pennsylvania19073
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, Pennsylvania19073
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,
Pennsylvania19073
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, Pennsylvania19073
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, Pennsylvania19073
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, Pennsylvania19073
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