Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1stq2p-0023u8-NN for pgsql-hackers@arkaria.postgresql.org; Thu, 26 Sep 2024 15:04:40 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1stq2p-009neM-4K for pgsql-hackers@arkaria.postgresql.org; Thu, 26 Sep 2024 15:04:39 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1stq2o-009neE-Qt for pgsql-hackers@lists.postgresql.org; Thu, 26 Sep 2024 15:04:38 +0000 Received: from sss.pgh.pa.us ([68.162.161.243]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1stq2l-001CuW-1P for pgsql-hackers@lists.postgresql.org; Thu, 26 Sep 2024 15:04:38 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.15.2/8.15.2) with ESMTP id 48QF4XCk320336; Thu, 26 Sep 2024 11:04:33 -0400 From: Tom Lane To: Nathan Bossart cc: pgsql-hackers@lists.postgresql.org, Maciek Sakrejda , Hunaid Sohail Subject: Re: Test improvements and minor code fixes for formatting.c. In-reply-to: References: <2956175.1725831136@sss.pgh.pa.us> Comments: In-reply-to Nathan Bossart message dated "Tue, 17 Sep 2024 16:59:37 -0500" MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-ID: <320334.1727363073.1@sss.pgh.pa.us> Date: Thu, 26 Sep 2024 11:04:33 -0400 Message-ID: <320335.1727363073@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk Nathan Bossart writes: > On Sun, Sep 08, 2024 at 05:32:16PM -0400, Tom Lane wrote: >> In looking at this, I found that there's also no test coverage >> for the EEEE, V, or PL format codes. Also, the possibility of >> overflow while converting an input value to int in order to >> pass it to int_to_roman was ignored. Attached is a patch that >> adds more test coverage and cleans up the Roman-numeral code >> a little bit. > I stared at the patch for a while, and it looks good to me. Pushed, thanks for looking! >> BTW, I also discovered that there is a little bit of support >> for a "B" format code: we can parse it, but then we ignore it. > AFAICT it's been like that since it was introduced [0]. I searched the > archives and couldn't find any discussion about this format code. Given > that, I don't have any concerns about removing it unless it causes ERRORs > for calls that currently succeed, but even then, it's probably fine. This > strikes me as something that might be fun for an aspiring hacker, though. Yeah, I left that alone for now. I don't have much interest in making it work, but perhaps someone else will. regards, tom lane