On Thu, 22 Dec 2022 at 23:11, Eric Radman <ericsh...@eradman.com> wrote: > > On Thu, Dec 22, 2022 at 10:08:17AM +0000, Dag Lem wrote: > > > > The calculation of quotient and remainder can be replaced by less costly > > masking and shifting. > > > > Defining > > > > #define OCT_DIGIT_BITS 3 > > #define OCT_DIGIT_BITMASK 0x7 > > > > the content of the loop can be replaced by > > > > *--ptr = digits[value & OCT_DIGIT_BITMASK]; > > value >>= OCT_DIGIT_BITS; > > > > Also, the check for ptr > buf in the while loop can be removed. The > > check is superfluous, since buf cannot possibly be exhausted by a 32 > > bit integer (the maximum octal number being 37777777777). > > I integrated these suggestions in the attached -v2 patch and tested > range of values manually. > > Also picked an OID > 8000 as suggested by unused_oids.
Few suggestions 1) We could use to_oct instead of to_oct32 as we don't have multiple implementations for to_oct diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index 98d90d9338..fde0b24563 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -3687,6 +3687,9 @@ { oid => '2090', descr => 'convert int8 number to hex', proname => 'to_hex', prorettype => 'text', proargtypes => 'int8', prosrc => 'to_hex64' }, +{ oid => '8335', descr => 'convert int4 number to oct', + proname => 'to_oct', prorettype => 'text', proargtypes => 'int4', + prosrc => 'to_oct32' }, 2) Similarly we could change this to "to_oct" +/* + * Convert an int32 to a string containing a base 8 (oct) representation of + * the number. + */ +Datum +to_oct32(PG_FUNCTION_ARGS) +{ + uint32 value = (uint32) PG_GETARG_INT32(0); 3) I'm not sure if AS "77777777" is required, but I also noticed it is done similarly in to_hex too: +-- +-- test to_oct +-- +select to_oct(256*256*256 - 1) AS "77777777"; + 77777777 +---------- + 77777777 +(1 row) 4) You could add a commit message for the patch Regards, Vignesh