Date: Tue, 22 Aug 2023 07:26:08 -0700
From: Nathan Bossart <nathandbossart@gmail.com>
To: Peter Eisentraut <peter@eisentraut.org>
Cc: Dean Rasheed <dean.a.rasheed@gmail.com>,
	John Naylor <john.naylor@enterprisedb.com>,
	Kirk Wolak <wolakk@gmail.com>, Eric Radman <ericshane@eradman.com>,
	pgsql-hackers@postgresql.org
Subject: Re: [PATCH] Add function to_oct
Message-ID: <20230822142608.GA651404@nathanxps13>
References: <20230815041129.GB2294364@nathanxps13>
 <CAFBsxsGkBKO276L476BWDgNGbkaH0S_ZcnevOK_0s=Dz0=Utrw@mail.gmail.com>
 <20230815171715.GA2312399@nathanxps13>
 <CAFBsxsEm0VGAipmQNK-EZ1a8ej6PDF+2aEkYGbktKH9aHFb4Dw@mail.gmail.com>
 <20230816142410.GA2651383@nathanxps13>
 <CAFBsxsEVR_M4_eGgP_zdugerq8iTUHeVJP-DBRj7yPh2xcuh4g@mail.gmail.com>
 <20230817152628.GA3148583@nathanxps13>
 <CAEZATCWaQ+-7A9AxxUcxdhdOa4p54rSvKreHMXhnTJg6VuAURQ@mail.gmail.com>
 <20230820152551.GB290672@nathanxps13>
 <0161f4fa-5cd4-ea9b-01a6-8b6b535bd812@eisentraut.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <0161f4fa-5cd4-ea9b-01a6-8b6b535bd812@eisentraut.org>
Archived-At: 
 <https://www.postgresql.org/message-id/20230822142608.GA651404%40nathanxps13>
Precedence: bulk

On Tue, Aug 22, 2023 at 04:20:02PM +0200, Peter Eisentraut wrote:
> On 20.08.23 17:25, Nathan Bossart wrote:
>> > Doing a quick test, shows that this changes the current behaviour,
>> > because all inputs are now treated as 64-bit:
>> > 
>> > HEAD:
>> > 
>> > select to_hex((-1234)::int);
>> >    to_hex
>> > ----------
>> >   fffffb2e
>> > 
>> > With patch:
>> > 
>> > select to_hex((-1234)::int);
>> >        to_hex
>> > ------------------
>> >   fffffffffffffb2e
>> Good catch.  In v8, I fixed this by first casting the input to uint32 for
>> the 32-bit versions of the functions.  This prevents the conversion to
>> uint64 from setting the rest of the bits.  AFAICT this behavior is pretty
>> well defined in the standard.
> 
> What standard?

C99

> I don't understand the reason for this handling of negative values.  I would
> expect that, say, to_hex(-1234) would return '-' || to_hex(1234).

For this patch set, I was trying to retain the current behavior, which is
to return the two's complement representation.  I'm open to changing this
if there's agreement on what the output should be.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com