Marti Raudsepp <ma...@juffo.org> writes: > On Wed, Sep 7, 2011 at 21:37, Tom Lane <t...@sss.pgh.pa.us> wrote: >> Hmm. I agree that this is a bug, but the proposed fix seems like a bit >> of a kluge. Wouldn't it be better to make get_last_relevant_decnum >> honor its contract, that is not delete any relevant digits?
> You're right, it was a kludge. > Here's an improved version. I need to take a NUMProc* argument to do > that right, because that's how it knows how many '0's to keep from the > format. Yeah, after fooling with it myself I saw that it was the interconnection of the don't-suppress-'0'-format-positions logic with the find-the-last- nonzero-digit logic that was confusing matters. (formatting.c may not be the most spaghetti-ish code I've ever worked with, but it's up there.) However, I don't think that inserting knowledge of the other consideration into get_last_relevant_decnum is really the way to make things cleaner. Also, the way yours is set up, I'm dubious that it does the right thing when the last '0' specifier is to the left of the decimal point. I'm currently testing this patch: diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c index 7efd988362889346af86c642f6ee660a4ae1b3d2..a7000250b0363165bee5697ad72036aad28b830e 100644 *** a/src/backend/utils/adt/formatting.c --- b/src/backend/utils/adt/formatting.c *************** NUM_prepare_locale(NUMProc *Np) *** 3908,3913 **** --- 3908,3916 ---- /* ---------- * Return pointer of last relevant number after decimal point * 12.0500 --> last relevant is '5' + * 12.0000 --> last relevant is '.' + * If there is no decimal point, return NULL (which will result in same + * behavior as if FM hadn't been specified). * ---------- */ static char * *************** get_last_relevant_decnum(char *num) *** 3921,3927 **** #endif if (!p) ! p = num; result = p; while (*(++p)) --- 3924,3931 ---- #endif if (!p) ! return NULL; ! result = p; while (*(++p)) *************** NUM_processor(FormatNode *node, NUMDesc *** 4458,4470 **** { Np->num_pre = plen; ! if (IS_FILLMODE(Np->Num)) { ! if (IS_DECIMAL(Np->Num)) ! Np->last_relevant = get_last_relevant_decnum( ! Np->number + ! ((Np->Num->zero_end - Np->num_pre > 0) ? ! Np->Num->zero_end - Np->num_pre : 0)); } if (Np->sign_wrote == FALSE && Np->num_pre == 0) --- 4462,4483 ---- { Np->num_pre = plen; ! if (IS_FILLMODE(Np->Num) && IS_DECIMAL(Np->Num)) { ! Np->last_relevant = get_last_relevant_decnum(Np->number); ! ! /* ! * If any '0' specifiers are present, make sure we don't strip ! * those digits. ! */ ! if (Np->last_relevant && Np->Num->zero_end > Np->num_pre) ! { ! char *last_zero; ! ! last_zero = Np->number + (Np->Num->zero_end - Np->num_pre); ! if (Np->last_relevant < last_zero) ! Np->last_relevant = last_zero; ! } } if (Np->sign_wrote == FALSE && Np->num_pre == 0) regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers