On Tue, Jul 25, 2023 at 05:16:56PM -0700, Nathan Bossart wrote:
> [v4]
If we're not going to have a general SQL conversion function, here are some comments on the current patch.
+static char *convert_to_base(uint64 value, int base);
Not needed if the definition is above the callers.
+ * Workhorse for to_binary, to_oct, and to_hex. Note that base must be either
+ * 2, 8, or 16.
Why wouldn't it work with any base <= 16?
- *ptr = '\0';
+ Assert(base == 2 || base == 8 || base == 16);
+ *ptr = '\0';
Spurious whitespace change?
- char buf[32]; /* bigger than needed, but reasonable */
+ char *buf = palloc(sizeof(uint64) * BITS_PER_BYTE + 1);
Why is this no longer allocated on the stack? Maybe needs a comment about the size calculation.
+static char *
+convert_to_base(uint64 value, int base)
On my machine this now requires a function call and a DIV instruction, even though the patch claims not to support anything but a power of two. It's tiny enough to declare it inline so the compiler can specialize for each call site.
+{ oid => '5101', descr => 'convert int4 number to binary',
Needs to be over 8000.
--
John Naylor
EDB:
http://www.enterprisedb.com
