Date: Sun, 20 Aug 2023 08:25:51 -0700
From: Nathan Bossart <nathandbossart@gmail.com>
To: Dean Rasheed <dean.a.rasheed@gmail.com>
Cc: John Naylor <john.naylor@enterprisedb.com>,
	Kirk Wolak <wolakk@gmail.com>,
	Peter Eisentraut <peter.eisentraut@enterprisedb.com>,
	Eric Radman <ericshane@eradman.com>, pgsql-hackers@postgresql.org
Subject: Re: [PATCH] Add function to_oct
Message-ID: <20230820152551.GB290672@nathanxps13>
References: <20230726001656.GC2992317@nathanxps13>
 <20230726032917.GA3151386@nathanxps13>
 <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>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="ibTvN161/egqYuK8"
Content-Disposition: inline
In-Reply-To: 
 <CAEZATCWaQ+-7A9AxxUcxdhdOa4p54rSvKreHMXhnTJg6VuAURQ@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/20230820152551.GB290672%40nathanxps13>
Precedence: bulk


--ibTvN161/egqYuK8
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Sat, Aug 19, 2023 at 08:35:46AM +0100, Dean Rasheed wrote:
> I note that there are no tests for negative inputs.

I added some in v8.

> 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.

> The way that negative inputs are handled really should be documented,
> or at least it should include a couple of examples.

I used your suggestion and noted that the output is the two's complement
representation [0].

[0] https://postgr.es/m/CAEZATCVbkL1ynqpsKiTDpch34%3DSCr5nnau%3DnfNmiy2nM3SJHtw%40mail.gmail.com

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

--ibTvN161/egqYuK8
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v8-0001-add-to_binary-and-to_oct.patch"

From a03c06d0b26ccbd49bda55c2efab565ab9fa90db Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Tue, 25 Jul 2023 16:09:01 -0700
Subject: [PATCH v8 1/1] add to_binary() and to_oct()

---
 doc/src/sgml/func.sgml                | 47 ++++++++++++++-
 src/backend/utils/adt/varlena.c       | 86 +++++++++++++++++++--------
 src/include/catalog/pg_proc.dat       | 12 ++++
 src/test/regress/expected/strings.out | 62 ++++++++++++++++++-
 src/test/regress/sql/strings.sql      | 15 ++++-
 5 files changed, 192 insertions(+), 30 deletions(-)

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index be2f54c914..835c08905e 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -3737,6 +3737,50 @@ repeat('Pg', 4) <returnvalue>PgPgPgPg</returnvalue>
        </para></entry>
       </row>
 
+      <row>
+       <entry role="func_table_entry"><para role="func_signature">
+        <indexterm>
+         <primary>to_binary</primary>
+        </indexterm>
+        <function>to_binary</function> ( <type>integer</type> )
+        <returnvalue>text</returnvalue>
+       </para>
+       <para role="func_signature">
+        <function>to_binary</function> ( <type>bigint</type> )
+        <returnvalue>text</returnvalue>
+       </para>
+       <para>
+        Converts the number to its equivalent two's complement binary
+        representation.
+       </para>
+       <para>
+        <literal>to_binary(2147483647)</literal>
+        <returnvalue>1111111111111111111111111111111</returnvalue>
+       </para></entry>
+      </row>
+
+      <row>
+       <entry role="func_table_entry"><para role="func_signature">
+        <indexterm>
+         <primary>to_oct</primary>
+        </indexterm>
+        <function>to_oct</function> ( <type>integer</type> )
+        <returnvalue>text</returnvalue>
+       </para>
+       <para role="func_signature">
+        <function>to_oct</function> ( <type>bigint</type> )
+        <returnvalue>text</returnvalue>
+       </para>
+       <para>
+        Converts the number to its equivalent two's complement octal
+        representation.
+       </para>
+       <para>
+        <literal>to_oct(2147483647)</literal>
+        <returnvalue>17777777777</returnvalue>
+       </para></entry>
+      </row>
+
       <row>
        <entry role="func_table_entry"><para role="func_signature">
         <indexterm>
@@ -3750,7 +3794,8 @@ repeat('Pg', 4) <returnvalue>PgPgPgPg</returnvalue>
         <returnvalue>text</returnvalue>
        </para>
        <para>
-        Converts the number to its equivalent hexadecimal representation.
+        Converts the number to its equivalent two's complement hexadecimal
+        representation.
        </para>
        <para>
         <literal>to_hex(2147483647)</literal>
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index b1ec5c32ce..cac3577480 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -4919,53 +4919,87 @@ array_to_text_internal(FunctionCallInfo fcinfo, ArrayType *v,
 	return result;
 }
 
-#define HEXBASE 16
 /*
- * Convert an int32 to a string containing a base 16 (hex) representation of
- * the number.
+ * Workhorse for to_binary, to_oct, and to_hex.  Note that base must be > 1 and
+ * <= 16.
  */
-Datum
-to_hex32(PG_FUNCTION_ARGS)
+static inline text *
+convert_to_base(uint64 value, int base)
 {
-	uint32		value = (uint32) PG_GETARG_INT32(0);
-	char	   *ptr;
 	const char *digits = "0123456789abcdef";
-	char		buf[32];		/* bigger than needed, but reasonable */
 
-	ptr = buf + sizeof(buf) - 1;
-	*ptr = '\0';
+	/* We size the buffer for to_binary's longest possible return value. */
+	char		buf[sizeof(uint64) * BITS_PER_BYTE];
+	char	   *const end = buf + sizeof(buf);
+	char	   *ptr = end;
+
+	Assert(base > 1);
+	Assert(base <= 16);
 
 	do
 	{
-		*--ptr = digits[value % HEXBASE];
-		value /= HEXBASE;
+		*--ptr = digits[value % base];
+		value /= base;
 	} while (ptr > buf && value);
 
-	PG_RETURN_TEXT_P(cstring_to_text(ptr));
+	return cstring_to_text_with_len(ptr, end - ptr);
+}
+
+/*
+ * Convert an integer to a string containing a base-2 (binary) representation
+ * of the number.
+ */
+Datum
+to_binary32(PG_FUNCTION_ARGS)
+{
+	uint64		value = (uint32) PG_GETARG_INT32(0);
+
+	PG_RETURN_TEXT_P(convert_to_base(value, 2));
+}
+Datum
+to_binary64(PG_FUNCTION_ARGS)
+{
+	uint64		value = (uint64) PG_GETARG_INT64(0);
+
+	PG_RETURN_TEXT_P(convert_to_base(value, 2));
 }
 
 /*
- * Convert an int64 to a string containing a base 16 (hex) representation of
+ * Convert an integer to a string containing a base-8 (oct) representation of
  * the number.
  */
 Datum
-to_hex64(PG_FUNCTION_ARGS)
+to_oct32(PG_FUNCTION_ARGS)
+{
+	uint64		value = (uint32) PG_GETARG_INT32(0);
+
+	PG_RETURN_TEXT_P(convert_to_base(value, 8));
+}
+Datum
+to_oct64(PG_FUNCTION_ARGS)
 {
 	uint64		value = (uint64) PG_GETARG_INT64(0);
-	char	   *ptr;
-	const char *digits = "0123456789abcdef";
-	char		buf[32];		/* bigger than needed, but reasonable */
 
-	ptr = buf + sizeof(buf) - 1;
-	*ptr = '\0';
+	PG_RETURN_TEXT_P(convert_to_base(value, 8));
+}
 
-	do
-	{
-		*--ptr = digits[value % HEXBASE];
-		value /= HEXBASE;
-	} while (ptr > buf && value);
+/*
+ * Convert an integer to a string containing a base-16 (hex) representation of
+ * the number.
+ */
+Datum
+to_hex32(PG_FUNCTION_ARGS)
+{
+	uint64		value = (uint32) PG_GETARG_INT32(0);
+
+	PG_RETURN_TEXT_P(convert_to_base(value, 16));
+}
+Datum
+to_hex64(PG_FUNCTION_ARGS)
+{
+	uint64		value = (uint64) PG_GETARG_INT64(0);
 
-	PG_RETURN_TEXT_P(cstring_to_text(ptr));
+	PG_RETURN_TEXT_P(convert_to_base(value, 16));
 }
 
 /*
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 12fac15ceb..b653351605 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -3707,6 +3707,18 @@
 { oid => '2768', descr => 'split string by pattern',
   proname => 'regexp_split_to_array', prorettype => '_text',
   proargtypes => 'text text text', prosrc => 'regexp_split_to_array' },
+{ oid => '9030', descr => 'convert int4 number to binary',
+  proname => 'to_binary', prorettype => 'text', proargtypes => 'int4',
+  prosrc => 'to_binary32' },
+{ oid => '9031', descr => 'convert int8 number to binary',
+  proname => 'to_binary', prorettype => 'text', proargtypes => 'int8',
+  prosrc => 'to_binary64' },
+{ oid => '9032', descr => 'convert int4 number to oct',
+  proname => 'to_oct', prorettype => 'text', proargtypes => 'int4',
+  prosrc => 'to_oct32' },
+{ oid => '9033', descr => 'convert int8 number to oct',
+  proname => 'to_oct', prorettype => 'text', proargtypes => 'int8',
+  prosrc => 'to_oct64' },
 { oid => '2089', descr => 'convert int4 number to hex',
   proname => 'to_hex', prorettype => 'text', proargtypes => 'int4',
   prosrc => 'to_hex32' },
diff --git a/src/test/regress/expected/strings.out b/src/test/regress/expected/strings.out
index 62698569e1..d24e6fa0cd 100644
--- a/src/test/regress/expected/strings.out
+++ b/src/test/regress/expected/strings.out
@@ -2129,8 +2129,68 @@ select split_part('@joeuser@mydatabase@','@',-2) AS "mydatabase";
 (1 row)
 
 --
--- test to_hex
+-- test to_binary, to_oct, and to_hex
 --
+select to_binary(-1234) AS "11111111111111111111101100101110";
+ 11111111111111111111101100101110 
+----------------------------------
+ 11111111111111111111101100101110
+(1 row)
+
+select to_binary(-1234::bigint);
+                            to_binary                             
+------------------------------------------------------------------
+ 1111111111111111111111111111111111111111111111111111101100101110
+(1 row)
+
+select to_binary(256*256*256 - 1) AS "111111111111111111111111";
+ 111111111111111111111111 
+--------------------------
+ 111111111111111111111111
+(1 row)
+
+select to_binary(256::bigint*256::bigint*256::bigint*256::bigint - 1) AS "11111111111111111111111111111111";
+ 11111111111111111111111111111111 
+----------------------------------
+ 11111111111111111111111111111111
+(1 row)
+
+select to_oct(-1234) AS "37777775456";
+ 37777775456 
+-------------
+ 37777775456
+(1 row)
+
+select to_oct(-1234::bigint) AS "1777777777777777775456";
+ 1777777777777777775456 
+------------------------
+ 1777777777777777775456
+(1 row)
+
+select to_oct(256*256*256 - 1) AS "77777777";
+ 77777777 
+----------
+ 77777777
+(1 row)
+
+select to_oct(256::bigint*256::bigint*256::bigint*256::bigint - 1) AS "37777777777";
+ 37777777777 
+-------------
+ 37777777777
+(1 row)
+
+select to_hex(-1234) AS "fffffb2e";
+ fffffb2e 
+----------
+ fffffb2e
+(1 row)
+
+select to_hex(-1234::bigint) AS "fffffffffffffb2e";
+ fffffffffffffb2e 
+------------------
+ fffffffffffffb2e
+(1 row)
+
 select to_hex(256*256*256 - 1) AS "ffffff";
  ffffff 
 --------
diff --git a/src/test/regress/sql/strings.sql b/src/test/regress/sql/strings.sql
index ca32f6bba5..437713fdc6 100644
--- a/src/test/regress/sql/strings.sql
+++ b/src/test/regress/sql/strings.sql
@@ -685,10 +685,21 @@ select split_part('joeuser@mydatabase','@',-3) AS "empty string";
 select split_part('@joeuser@mydatabase@','@',-2) AS "mydatabase";
 
 --
--- test to_hex
+-- test to_binary, to_oct, and to_hex
 --
-select to_hex(256*256*256 - 1) AS "ffffff";
+select to_binary(-1234) AS "11111111111111111111101100101110";
+select to_binary(-1234::bigint);
+select to_binary(256*256*256 - 1) AS "111111111111111111111111";
+select to_binary(256::bigint*256::bigint*256::bigint*256::bigint - 1) AS "11111111111111111111111111111111";
+
+select to_oct(-1234) AS "37777775456";
+select to_oct(-1234::bigint) AS "1777777777777777775456";
+select to_oct(256*256*256 - 1) AS "77777777";
+select to_oct(256::bigint*256::bigint*256::bigint*256::bigint - 1) AS "37777777777";
 
+select to_hex(-1234) AS "fffffb2e";
+select to_hex(-1234::bigint) AS "fffffffffffffb2e";
+select to_hex(256*256*256 - 1) AS "ffffff";
 select to_hex(256::bigint*256::bigint*256::bigint*256::bigint - 1) AS "ffffffff";
 
 --
-- 
2.25.1


--ibTvN161/egqYuK8--