On Tue, Aug 22, 2023 at 04:57:02PM +0200, Peter Eisentraut wrote:
> On 22.08.23 16:26, Nathan Bossart wrote:
>> > 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
On 22.08.23 16:26, Nathan Bossart wrote:
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
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);
>> >
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
--
fb2e
With patch:
select to_hex((-1234)::int);
to_hex
On Tue, Aug 22, 2023 at 3:10 AM Dean Rasheed
wrote:
>
> OK, cool. This looks good to me.
LGTM too.
--
John Naylor
EDB: http://www.enterprisedb.com
On Mon, 21 Aug 2023 at 20:15, Nathan Bossart wrote:
>
> I added some examples for negative inputs.
>
> I renamed it to to_bin().
>
> I reordered the functions in the docs.
>
OK, cool. This looks good to me.
Regards,
Dean
On Mon, Aug 21, 2023 at 09:31:37AM +0100, Dean Rasheed wrote:
> Hmm, I think just including the doc text update, without the examples
> of positive and negative inputs, might not be sufficient to make the
> meaning clear to everyone.
I added some examples for negative inputs.
> Something else
On Sun, 20 Aug 2023 at 16:25, Nathan Bossart wrote:
>
> On Sat, Aug 19, 2023 at 08:35:46AM +0100, Dean Rasheed wrote:
>
> > 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
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
On Sat, Aug 19, 2023 at 11:41:56AM +0700, John Naylor wrote:
> This looks nicer, but still doesn't save the starting pointer, and so needs
> to lug around that big honking macro. This is what I mean:
>
> static inline text *
> convert_to_base(uint64 value, int base)
> {
> const char *digits =
On Thu, 17 Aug 2023 at 16:26, Nathan Bossart wrote:
>
> Works for me. I did it that way in v7.
>
I note that there are no tests for negative inputs.
Doing a quick test, shows that this changes the current behaviour,
because all inputs are now treated as 64-bit:
HEAD:
select
On Thu, Aug 17, 2023 at 10:26 PM Nathan Bossart
wrote:
>
> On Thu, Aug 17, 2023 at 12:35:54PM +0700, John Naylor wrote:
> > That makes it a lexically-scoped global variable, which we don't need
> > either. Can we have the internal function allocate on the stack, then
> > call cstring_to_text() on
On Thu, Aug 17, 2023 at 12:35:54PM +0700, John Naylor wrote:
> That makes it a lexically-scoped global variable, which we don't need
> either. Can we have the internal function allocate on the stack, then
> call cstring_to_text() on that, returning the text result? That does its
> own palloc.
>
>
On Wed, Aug 16, 2023 at 9:24 PM Nathan Bossart
wrote:
>
> On Wed, Aug 16, 2023 at 10:35:27AM +0700, John Naylor wrote:
> > Now I'm struggling to understand why each and every instance has its own
> > nominal buffer, passed down to the implementation. All we care about is
the
> > result -- is
On Wed, Aug 16, 2023 at 10:35:27AM +0700, John Naylor wrote:
> ```
> *ptr = '\0';
>
> do
> ```
>
> to
>
> ```
> *ptr = '\0';
> do
> ```
Oh, I misunderstood. I thought you meant that there might be a whitespace
change on that line, not the surrounding ones. This is fixed in v6.
> Now I'm
On Wed, Aug 16, 2023 at 12:17 AM Nathan Bossart
wrote:
>
> On Tue, Aug 15, 2023 at 01:53:25PM +0700, John Naylor wrote:
> > - *ptr = '\0';
> > + Assert(base == 2 || base == 8 || base == 16);
> >
> > + *ptr = '\0';
> >
> > Spurious whitespace change?
>
> I think this might just be a weird
On Tue, Aug 15, 2023 at 01:53:25PM +0700, John Naylor wrote:
> If we're not going to have a general SQL conversion function, here are some
> comments on the current patch.
I appreciate the review.
> +static char *convert_to_base(uint64 value, int base);
>
> Not needed if the definition is above
On Tue, Aug 15, 2023 at 07:58:17AM +0200, Vik Fearing wrote:
> On 8/15/23 06:11, Nathan Bossart wrote:
>> If there are no objections, I'd like to commit this patch soon.
>
> I just took a look at this (and the rest of the thread). I am a little bit
> disappointed that we don't have a generic
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.
+ *
On 8/15/23 06:11, Nathan Bossart wrote:
On Tue, Jul 25, 2023 at 08:29:17PM -0700, Nathan Bossart wrote:
Here's a new version of the patch with the silly mistakes fixed.
If there are no objections, I'd like to commit this patch soon.
I just took a look at this (and the rest of the thread).
On Tue, Jul 25, 2023 at 08:29:17PM -0700, Nathan Bossart wrote:
> Here's a new version of the patch with the silly mistakes fixed.
If there are no objections, I'd like to commit this patch soon.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
On Tue, Jul 25, 2023 at 05:16:56PM -0700, Nathan Bossart wrote:
> On Tue, Jul 25, 2023 at 04:24:26PM -0700, Nathan Bossart wrote:
>> I started taking a look at this and ended up adding to_binary() and a
>> bigint version of to_oct() for completeness. While I was at it, I moved
>> the
On Tue, Jul 25, 2023 at 04:24:26PM -0700, Nathan Bossart wrote:
> I started taking a look at this and ended up adding to_binary() and a
> bigint version of to_oct() for completeness. While I was at it, I moved
> the base-conversion logic out to a separate static function that
> to_binary/oct/hex
On Tue, Apr 04, 2023 at 08:45:36PM -0400, Kirk Wolak wrote:
> Marked Ready for Committer.
I started taking a look at this and ended up adding to_binary() and a
bigint version of to_oct() for completeness. While I was at it, I moved
the base-conversion logic out to a separate static function that
On Thu, Feb 23, 2023 at 6:32 AM Peter Eisentraut <
peter.eisentr...@enterprisedb.com> wrote:
> On 20.12.22 23:08, Eric Radman wrote:
> > This patch is a new function based on the implementation of to_hex(int).
> >
> > Since support for octal integer literals was added, to_oct(int) allows
> >
On 20.12.22 23:08, Eric Radman wrote:
This patch is a new function based on the implementation of to_hex(int).
Since support for octal integer literals was added, to_oct(int) allows
octal values to be easily stored and returned in query results.
to_oct(0o755) = '755'
This is probably most
vignesh C writes:
> Few suggestions
> 1) We could use to_oct instead of to_oct32 as we don't have multiple
> implementations for to_oct
That seems (a) shortsighted and (b) inconsistent with the naming
pattern used for to_hex, so I doubt it'd be an improvement.
regards,
On Thu, 22 Dec 2022 at 23:11, Eric Radman wrote:
>
> On Thu, Dec 22, 2022 at 10:08:17AM +, 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
On Thu, Dec 22, 2022 at 10:08:17AM +, 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
>
>
The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: not tested
Documentation:not tested
This is a mini-review of to_oct :-)
The function seems useful to me, and is
2022年12月21日(水) 10:42 Eric Radman :
>
> On Wed, Dec 21, 2022 at 08:36:40AM +0900, Ian Lawrence Barwick wrote:
> > 2022年12月21日(水) 7:08 Eric Radman :>
> > > Hello!
> > >
> > > This patch is a new function based on the implementation of to_hex(int).
> > >
> > > Since support for octal integer literals
On Wed, Dec 21, 2022 at 08:36:40AM +0900, Ian Lawrence Barwick wrote:
> 2022年12月21日(水) 7:08 Eric Radman :>
> > Hello!
> >
> > This patch is a new function based on the implementation of to_hex(int).
> >
> > Since support for octal integer literals was added, to_oct(int) allows
> > octal values to
2022年12月21日(水) 7:08 Eric Radman :>
> Hello!
>
> This patch is a new function based on the implementation of to_hex(int).
>
> Since support for octal integer literals was added, to_oct(int) allows
> octal values to be easily stored and returned in query results.
>
> to_oct(0o755) = '755'
>
> This
Hello!
This patch is a new function based on the implementation of to_hex(int).
Since support for octal integer literals was added, to_oct(int) allows
octal values to be easily stored and returned in query results.
to_oct(0o755) = '755'
This is probably most useful for storing file system
34 matches
Mail list logo
