Horton

Thread

Commits

  1. Allow casting between bytea and integer types.

  1. [PATCH] Add get_bytes() and set_bytes() functions

    Aleksander Alekseev <aleksander@timescale.com> — 2024-08-14T11:01:22Z

    Hi,

While discussing another patch [1] it was discovered that we don't
have a convenient way of casting a bytea to an integer / bigint and
vice versa, extracting integers larger than one byte from byteas, etc.

For instance, casting '\x11223344' to 0x11223344 may look like this:

```
WITH vals AS (
      SELECT '\x11223344'::bytea AS x
)
SELECT
    (get_byte(x, 0) :: bigint << 24) |
    (get_byte(x, 1) << 16) |
    (get_byte(x, 2) << 8) |
    get_byte(x, 3)
FROM vals;
```

There seems to be a demand for this functionality [2][3] and it costs
us nothing to maintain it, so I propose adding it.

The proposed patch adds get_bytes() and set_bytes() functions. The
semantics is similar to get_byte() and set_byte() we already have but
the functions operate with bigints rather than bytes and the user can
specify the size of the integer. This allows working with int2s,
int4s, int8s or even int5s if needed.

Examples:

```
SELECT get_bytes('\x1122334455667788'::bytea, 1, 2) = 0x2233;
 ?column?
----------
 t

SELECT set_bytes('\x1122334455667788'::bytea, 1, 2, 0xAABB);
     set_bytes
--------------------
 \x11aabb4455667788
```

Thoughts?

[1]: https://postgr.es/m/CAJ7c6TNMTGnqnG%3DyXXUQh9E88JDckmR45H2Q%2B%3DucaCLMOW1QQw%40mail.gmail.com
[2]: https://stackoverflow.com/questions/32944267/postgresql-converting-bytea-to-bigint
[3]: https://postgr.es/m/AANLkTikip9xs8iXc8e%2BMgz1T1701i8Xk6QtbVB3KJQzX%40mail.gmail.com
-- 
Best regards,
Aleksander Alekseev

  2. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Joel Jacobson <joel@compiler.org> — 2024-08-14T11:21:49Z

    On Wed, Aug 14, 2024, at 13:01, Aleksander Alekseev wrote:
> The proposed patch adds get_bytes() and set_bytes() functions. The
> semantics is similar to get_byte() and set_byte() we already have but
> the functions operate with bigints rather than bytes and the user can
> specify the size of the integer. This allows working with int2s,
> int4s, int8s or even int5s if needed.

+1

I wanted this myself many times.

I wonder if get_bytes() and set_bytes() will behave differently
on little-endian vs big-endian systems?

If so, then I think it would be nice to enforce a consistent byte order
(e.g., big-endian), to ensure consistent behavior across platforms.

Regards,
Joel

  3. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Aleksander Alekseev <aleksander@timescale.com> — 2024-08-14T11:31:31Z

    Hi,

> +1
>
> I wanted this myself many times.
>
> I wonder if get_bytes() and set_bytes() will behave differently
> on little-endian vs big-endian systems?
>
> If so, then I think it would be nice to enforce a consistent byte order
> (e.g., big-endian), to ensure consistent behavior across platforms.

No, the returned value will not depend on the CPU endiness. Current
implementation uses big-endian / network order which in my humble
opinion is what most users would expect.

I believe we also need reverse(bytea) and repeat(bytea, integer)
functions e.g. for those who want little-endian. However I want to
propose them separately when we are done with this patch.

-- 
Best regards,
Aleksander Alekseev

  4. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Joel Jacobson <joel@compiler.org> — 2024-08-14T12:34:06Z

    On Wed, Aug 14, 2024, at 13:31, Aleksander Alekseev wrote:
>> I wonder if get_bytes() and set_bytes() will behave differently
>> on little-endian vs big-endian systems?
> No, the returned value will not depend on the CPU endiness. Current
> implementation uses big-endian / network order which in my humble
> opinion is what most users would expect.

Nice.

I've reviewed and tested the patch.
It looks straight-forward to me.
I don't see any potential problems.
I've marked it Ready for Committer.

> I believe we also need reverse(bytea) and repeat(bytea, integer)
> functions e.g. for those who want little-endian. However I want to
> propose them separately when we are done with this patch.

I agree those functions would be nice too.

I also think it would be nice to provide these convenience functions:
to_bytes(bigint) -> bytea
from_bytes(bytea) -> bigint

Since if not having a current bytea value,
and just wanting to convert a bigint to bytea,
then one would need to construct an zeroed bytea
of the proper size first, to then use set_bytes().

And if just wanting to convert the entire bytea to a bigint,
then one would need to pass 0 as offset and the length
of the bytea as size.

Regards,

Joel

  5. Re: [PATCH] Add get_bytes() and set_bytes() functions

    David Fetter <david@fetter.org> — 2024-08-14T14:43:51Z

    On Wed, Aug 14, 2024 at 02:34:06PM +0200, Joel Jacobson wrote:
> On Wed, Aug 14, 2024, at 13:31, Aleksander Alekseev wrote:
> >> I wonder if get_bytes() and set_bytes() will behave differently
> >> on little-endian vs big-endian systems?
> > No, the returned value will not depend on the CPU endiness. Current
> > implementation uses big-endian / network order which in my humble
> > opinion is what most users would expect.
> 
> Nice.

Indeed!

> I've reviewed and tested the patch.
> It looks straight-forward to me.
> I don't see any potential problems.
> I've marked it Ready for Committer.
> 
> > I believe we also need reverse(bytea) and repeat(bytea, integer)
> > functions e.g. for those who want little-endian. However I want to
> > propose them separately when we are done with this patch.
> 
> I agree those functions would be nice too.
> 
> I also think it would be nice to provide these convenience functions:
> to_bytes(bigint) -> bytea
> from_bytes(bytea) -> bigint

Along with these, would it make sense to have other forms of these
that won't choke at 63 bits, e.g. NUMERIC or TEXT?

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

  6. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Joel Jacobson <joel@compiler.org> — 2024-08-14T15:39:32Z

    On Wed, Aug 14, 2024, at 16:43, David Fetter wrote:
>> I also think it would be nice to provide these convenience functions:
>> to_bytes(bigint) -> bytea
>> from_bytes(bytea) -> bigint
>
> Along with these, would it make sense to have other forms of these
> that won't choke at 63 bits, e.g. NUMERIC or TEXT?

I wonder what would be good names for such functions though?

Since NUMERIC can have decimal digits, and since BYTEA can already be casted to
TEXT, which will just be \x followed by the hex digits, maybe such names should
include "integer" or some other word, to indicate what is being returned?

It's already quite easy to convert to NUMERIC though,
for users who are aware of tricks like this:
SELECT ('0x'||encode('\xCAFEBABEDEADBEEF'::bytea,'hex'))::numeric;
       numeric
----------------------
 14627333968688430831
(1 row)

But, I think it would be better to provide functions,
since many users probably have to google+stackoverflow or gpt
to learn such tricks, which are not in the official documentation.

Regards,
Joel

  7. Re: [PATCH] Add get_bytes() and set_bytes() functions

    David Fetter <david@fetter.org> — 2024-08-14T16:31:44Z

    On Wed, Aug 14, 2024 at 05:39:32PM +0200, Joel Jacobson wrote:
> On Wed, Aug 14, 2024, at 16:43, David Fetter wrote:
> >> I also think it would be nice to provide these convenience functions:
> >> to_bytes(bigint) -> bytea
> >> from_bytes(bytea) -> bigint
> >
> > Along with these, would it make sense to have other forms of these
> > that won't choke at 63 bits, e.g. NUMERIC or TEXT?
> 
> I wonder what would be good names for such functions though?

decimal_to_bytes(numeric), decimal_to_bytes(text) on one side
decimal_from_bytes(bytea, typeoid)

Naming Things™ is one of the hard problems in computer science. Bad
joke that includes cache coherency and off-by-one included by
reference.

> Since NUMERIC can have decimal digits, and since BYTEA can already be casted to
> TEXT, which will just be \x followed by the hex digits, maybe such names should
> include "integer" or some other word, to indicate what is being returned?
> 
> It's already quite easy to convert to NUMERIC though,
> for users who are aware of tricks like this:
> SELECT ('0x'||encode('\xCAFEBABEDEADBEEF'::bytea,'hex'))::numeric;
>        numeric
> ----------------------
>  14627333968688430831
> (1 row)
> 
> But, I think it would be better to provide functions,
> since many users probably have to google+stackoverflow or gpt
> to learn such tricks, which are not in the official documentation.

As usual, I see "official documentation lacks helpful and/or
non-obvious examples" as a problem best approached by making good the
lack. I am aware that my ideas about pedagogy, documentation, etc. are
not shared universally, but they're widely shared by people whose main
interaction with documents is trying to get help from them.

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

  8. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Joel Jacobson <joel@compiler.org> — 2024-08-14T17:25:51Z

    On Wed, Aug 14, 2024, at 18:31, David Fetter wrote:
> On Wed, Aug 14, 2024 at 05:39:32PM +0200, Joel Jacobson wrote:
>> On Wed, Aug 14, 2024, at 16:43, David Fetter wrote:
>> >> I also think it would be nice to provide these convenience functions:
>> >> to_bytes(bigint) -> bytea
>> >> from_bytes(bytea) -> bigint
>> >
>> > Along with these, would it make sense to have other forms of these
>> > that won't choke at 63 bits, e.g. NUMERIC or TEXT?
>> 
>> I wonder what would be good names for such functions though?
>
> decimal_to_bytes(numeric), decimal_to_bytes(text) on one side
> decimal_from_bytes(bytea, typeoid)

I assume decimal_to_bytes() will only accept integer numerics,
that is, that don't have a decimal digits part?
Hmm, it's perhaps then a bit counter intuitive that the name
contains "decimal", since some people might associate the word
"decimal" stronger with "decimal digits" rather than the radix/base 10.

What do we want to happen if passing a numeric with decimal digits,
to decimal_to_bytes()? It must be an error, right?

Example: SELECT decimal_to_bytes(1.23);

> Naming Things™ is one of the hard problems in computer science. Bad
> joke that includes cache coherency and off-by-one included by
> reference.

So true :)

> As usual, I see "official documentation lacks helpful and/or
> non-obvious examples" as a problem best approached by making good the
> lack. I am aware that my ideas about pedagogy, documentation, etc. are
> not shared universally, but they're widely shared by people whose main
> interaction with documents is trying to get help from them.

Well spoken.

Regards,
Joel

  9. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Joel Jacobson <joel@compiler.org> — 2024-08-15T04:19:36Z

    On Wed, Aug 14, 2024, at 19:25, Joel Jacobson wrote:
> What do we want to happen if passing a numeric with decimal digits,
> to decimal_to_bytes()? It must be an error, right?
>
> Example: SELECT decimal_to_bytes(1.23);

Hmm, an error feels quite ugly on second thought.
Would be nicer if all numerics could be represented,
in a bytea representation that is meaningful to other systems.

I think we need a tuple somehow.

Example:

SELECT numeric_to_bytes(223195403574957);
(\xcafebabedeadbeef,0,false)

SELECT numeric_to_bytes(-223195403574957);
(\xcafebabedeadbeef,0,true)

SELECT numeric_to_bytes(2231954035749.57);
(\xcafebabedeadbeef,2,false)

SELECT numeric_from_bytes('\xcafebabedeadbeef'::bytea,0,false);
223195403574957

SELECT numeric_from_bytes('\xcafebabedeadbeef'::bytea,0,true);
-223195403574957

SELECT numeric_from_bytes('\xcafebabedeadbeef'::bytea,2,false);
2231954035749.57

But then what about Inf,-Inf,NaN?

Regards,
Joel

  10. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2024-08-15T10:14:55Z

    On Thu, 15 Aug 2024 at 05:20, Joel Jacobson <joel@compiler.org> wrote:
>
> On Wed, Aug 14, 2024, at 19:25, Joel Jacobson wrote:
> > What do we want to happen if passing a numeric with decimal digits,
> > to decimal_to_bytes()? It must be an error, right?
> >
> > Example: SELECT decimal_to_bytes(1.23);
>
> Hmm, an error feels quite ugly on second thought.
> Would be nicer if all numerics could be represented,
>
> But then what about Inf,-Inf,NaN?
>

Perhaps we should also add casts between bytea and the integer/numeric
types. That might be easier to use than functions in some
circumstances.

When casting a numeric to an integer, the result is rounded to the
nearest integer, and NaN/Inf generate errors, so we should probably do
the same here.

Regards,
Dean

  11. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Aleksander Alekseev <aleksander@timescale.com> — 2024-08-15T10:58:03Z

    Hi,

> Perhaps we should also add casts between bytea and the integer/numeric
> types. That might be easier to use than functions in some
> circumstances.
>
> When casting a numeric to an integer, the result is rounded to the
> nearest integer, and NaN/Inf generate errors, so we should probably do
> the same here.

Yes, I was also thinking about adding NUMERIC versions of get_bytes()
/ set_bytes(). This would allow converting more than 8 bytes to/from
an integer. I dropped this idea because I thought there would be not
much practical use for it. On the flip side you never know who uses
Postgres and for what purpose.

I will add corresponding casts unless the idea will get a push-back
from the community. IMO the existence of these casts will at least not
make things worse.

-- 
Best regards,
Aleksander Alekseev

  12. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Yugo Nagata <nagata@sraoss.co.jp> — 2024-08-16T07:49:36Z

    On Thu, 15 Aug 2024 13:58:03 +0300
Aleksander Alekseev <aleksander@timescale.com> wrote:

> Hi,
> 
> > Perhaps we should also add casts between bytea and the integer/numeric
> > types. That might be easier to use than functions in some
> > circumstances.
> >
> > When casting a numeric to an integer, the result is rounded to the
> > nearest integer, and NaN/Inf generate errors, so we should probably do
> > the same here.
> 
> Yes, I was also thinking about adding NUMERIC versions of get_bytes()
> / set_bytes(). This would allow converting more than 8 bytes to/from
> an integer. I dropped this idea because I thought there would be not
> much practical use for it. On the flip side you never know who uses
> Postgres and for what purpose.
> 
> I will add corresponding casts unless the idea will get a push-back
> from the community. IMO the existence of these casts will at least not
> make things worse.

When we add such casts between bytea and the integer/numeric types,
one of the problems mentioned the first of the thread, that is, 
"we don't have a convenient way of casting a bytea to an integer / bigint
and vice versa", would seem be resolved. 

On the other hand, I suppose get_bytes() and set_bytes() are still useful
for extracting bytes from byteas, etc. If casting is no longer the main
purpose of these functions, are variations that get_bytes returns bytea
instead of bigint, and set_bytes receives bytea as the newvalue argument
useful? I wonder it would eliminate the restrict that size cannot be larger
than 8.


Here are my very trivial comments on the patch.

+ * this routine treats "bytea" as an array of bytes.

Maybe, the sentence should start with "This ... ".

+	while(size)
+	{

I wonder inserting a space after "while" is the standard style.

Regards,
Yugo Nagata

-- 
Yugo Nagata <nagata@sraoss.co.jp>

  13. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Aleksander Alekseev <aleksander@timescale.com> — 2024-08-16T08:41:37Z

    Hi,

> When we add such casts between bytea and the integer/numeric types,
> one of the problems mentioned the first of the thread, that is,
> "we don't have a convenient way of casting a bytea to an integer / bigint
> and vice versa", would seem be resolved.
>
> On the other hand, I suppose get_bytes() and set_bytes() are still useful
> for extracting bytes from byteas, etc. If casting is no longer the main
> purpose of these functions, are variations that get_bytes returns bytea
> instead of bigint, and set_bytes receives bytea as the newvalue argument
> useful? I wonder it would eliminate the restrict that size cannot be larger
> than 8.

No, casting between bytea and numeric will not replace get_bytes() /
set_bytes() for performance reasons.

Consider the case when you want to extract an int4 from a bytea.
get_bytes() is going to be very fast while substr() -> ::numeric ->
::integer chain will require unnecessary copying and conversions.
Casting between bytea and numeric is only useful when one has to deal
with integers larger than 8 bytes. Whether this happens often is a
debatable question.

> Here are my very trivial comments on the patch.
>
> + * this routine treats "bytea" as an array of bytes.
>
> Maybe, the sentence should start with "This ... ".
>
> +       while(size)
> +       {
>
> I wonder inserting a space after "while" is the standard style.

Thanks, fixed.

-- 
Best regards,
Aleksander Alekseev

  14. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Peter Eisentraut <peter@eisentraut.org> — 2024-08-16T10:11:18Z

    On 14.08.24 13:01, Aleksander Alekseev wrote:
> The proposed patch adds get_bytes() and set_bytes() functions. The
> semantics is similar to get_byte() and set_byte() we already have but
> the functions operate with bigints rather than bytes and the user can
> specify the size of the integer. This allows working with int2s,
> int4s, int8s or even int5s if needed.
> 
> Examples:
> 
> ```
> SELECT get_bytes('\x1122334455667788'::bytea, 1, 2) = 0x2233;
>   ?column?
> ----------
>   t
> 
> SELECT set_bytes('\x1122334455667788'::bytea, 1, 2, 0xAABB);
>       set_bytes
> --------------------
>   \x11aabb4455667788
> ```

I think these functions do about three things at once, and I don't think 
they address the originally requested purpose very well.

Converting between integers and byte arrays of matching size seems like 
reasonable functionality.  (You can already do one half of that by 
calling int2send(), int4send(), and int8send(), but the other direction 
(intXrecv()) is not user-callable).

The other things are extracting that byte array from a larger byte array 
and sticking it back into a larger byte array; those seem like separate 
operations.  There is already substr() for bytea for the first part, and 
there might be another string-like operationg for the second part, or 
maybe we could add one.

  15. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Yugo Nagata <nagata@sraoss.co.jp> — 2024-08-16T10:32:24Z

    On Fri, 16 Aug 2024 11:41:37 +0300
Aleksander Alekseev <aleksander@timescale.com> wrote:

> Hi,
> 
> > When we add such casts between bytea and the integer/numeric types,
> > one of the problems mentioned the first of the thread, that is,
> > "we don't have a convenient way of casting a bytea to an integer / bigint
> > and vice versa", would seem be resolved.
> >
> > On the other hand, I suppose get_bytes() and set_bytes() are still useful
> > for extracting bytes from byteas, etc. If casting is no longer the main
> > purpose of these functions, are variations that get_bytes returns bytea
> > instead of bigint, and set_bytes receives bytea as the newvalue argument
> > useful? I wonder it would eliminate the restrict that size cannot be larger
> > than 8.
> 
> No, casting between bytea and numeric will not replace get_bytes() /
> set_bytes() for performance reasons.
> 
> Consider the case when you want to extract an int4 from a bytea.
> get_bytes() is going to be very fast while substr() -> ::numeric ->
> ::integer chain will require unnecessary copying and conversions.
> Casting between bytea and numeric is only useful when one has to deal
> with integers larger than 8 bytes. Whether this happens often is a
> debatable question.

Thank you for explanation. I understood the performance drawback.

I supposed interfaces similar to lo_get, lo_put, loread, lowrite of
large objects since they might be useful to access or modify a part of
bytea like a binary file read by pg_read_binary_file. 

> 
> > Here are my very trivial comments on the patch.
> >
> > + * this routine treats "bytea" as an array of bytes.
> >
> > Maybe, the sentence should start with "This ... ".
> >
> > +       while(size)
> > +       {
> >
> > I wonder inserting a space after "while" is the standard style.
> 
> Thanks, fixed.

Should we fix the comment on byteaGetByte in passing, too?

Regards,
Yugo Nagata

-- 
Yugo Nagata <nagata@sraoss.co.jp>

  16. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Aleksander Alekseev <aleksander@timescale.com> — 2024-08-19T14:10:15Z

    Hi Peter,

Thanks for the feedback.

> I think these functions do about three things at once, and I don't think
> they address the originally requested purpose very well.

The amount of things that the function does is a matter of
interpretation. I can claim that it does one thing ("extracts an
integer from a bytea"), or as many things as there are lines of code.
IMO the actual question is whether this is a good user interface or
not. Since we already have get_byte() / set_byte() the interface is
arguably OK.

> Converting between integers and byte arrays of matching size seems like
> reasonable functionality.  (You can already do one half of that by
> calling int2send(), int4send(), and int8send(), but the other direction
> (intXrecv()) is not user-callable).
>
> The other things are extracting that byte array from a larger byte array
> and sticking it back into a larger byte array; those seem like separate
> operations.  There is already substr() for bytea for the first part, and
> there might be another string-like operationg for the second part, or
> maybe we could add one.

If I understand correctly, you propose doing (1):

```
SELECT substr('\x1122334455667788'::bytea, 2, 2) :: int2;
```

... instead of:

```
SELECT get_bytes('\x1122334455667788'::bytea, 1, 2)
```

... and (2):

```
WITH vals AS (
    SELECT '\x1122334455667788'::bytea AS x
) SELECT substr(x, 1, 1) || int2send(1234::int2) || substr(x, 4, 5) FROM vals;
```

... instead of:

```
SELECT set_bytes('\x1122334455667788'::bytea, 1, 2, 0xAABB);
```

There is nothing to do for query (2), it already works. It's not much
better than the query from my first email though.

To clarify, supporting bytea<->integer (and/or bytea<->numeric) casts
doesn't strike me as a terrible idea but it doesn't address the issue
I'm proposing to solve.

-- 
Best regards,
Aleksander Alekseev

  17. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Peter Eisentraut <peter@eisentraut.org> — 2024-08-21T16:39:21Z

    On 19.08.24 16:10, Aleksander Alekseev wrote:
> To clarify, supporting bytea<->integer (and/or bytea<->numeric) casts
> doesn't strike me as a terrible idea but it doesn't address the issue
> I'm proposing to solve.

What is the issue you are proposing to solve?

You linked to a couple of threads and stackoverflow pages, and you 
concluded from that that we should add get_bytes() and set_bytes() 
functions.  It's not obvious how you get from the former to the latter, 
and I don't think the functions you propose are well-designed in isolation.

  18. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Aleksander Alekseev <aleksander@timescale.com> — 2024-08-26T11:31:37Z

    Hi,

> On 19.08.24 16:10, Aleksander Alekseev wrote:
> > To clarify, supporting bytea<->integer (and/or bytea<->numeric) casts
> > doesn't strike me as a terrible idea but it doesn't address the issue
> > I'm proposing to solve.
>
> What is the issue you are proposing to solve?
>
> You linked to a couple of threads and stackoverflow pages, and you
> concluded from that that we should add get_bytes() and set_bytes()
> functions.  It's not obvious how you get from the former to the latter,
> and I don't think the functions you propose are well-designed in isolation.

I guess there are in fact two problems, not one.

1. Converting between bytea and integer types
2. Multibyte versions of get_byte() / set_byte()

As you rightly pointed out, for (1) we just need to add missing casts.
Here is the corresponding patch, v3-0001. Note that I couldn't re-use
int{2,4,8}recv because its first argument is StringInfo, so I ended up
implementing my own bytea->int{2,4,8} functions.

I think there may be value in (2) as well. It's implemented in v3-0002
and I did my best to clarify the commit message. On the flip side the
situation when one wants something like extracting int4 from a
bytea(or vice versa) and is not happy with convenience and/or
performance of substr()+casts is arguably rare. I'll be fine with
whatever consensus the community reaches about this patch.

-- 
Best regards,
Aleksander Alekseev

  19. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Aleksander Alekseev <aleksander@timescale.com> — 2024-10-08T13:41:51Z

    Hi,

> I guess there are in fact two problems, not one.
>
> 1. Converting between bytea and integer types
> 2. Multibyte versions of get_byte() / set_byte()
>
> As you rightly pointed out, for (1) we just need to add missing casts.
> Here is the corresponding patch, v3-0001. Note that I couldn't re-use
> int{2,4,8}recv because its first argument is StringInfo, so I ended up
> implementing my own bytea->int{2,4,8} functions.
>
> I think there may be value in (2) as well. It's implemented in v3-0002
> and I did my best to clarify the commit message. On the flip side the
> situation when one wants something like extracting int4 from a
> bytea(or vice versa) and is not happy with convenience and/or
> performance of substr()+casts is arguably rare. I'll be fine with
> whatever consensus the community reaches about this patch.

PFA the rebased patch set v4.

-- 
Best regards,
Aleksander Alekseev

  20. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Aleksander Alekseev <aleksander@timescale.com> — 2024-10-18T14:20:42Z

    Hi,

> > I guess there are in fact two problems, not one.
> >
> > 1. Converting between bytea and integer types
> > 2. Multibyte versions of get_byte() / set_byte()
> >
> > As you rightly pointed out, for (1) we just need to add missing casts.
> > Here is the corresponding patch, v3-0001. Note that I couldn't re-use
> > int{2,4,8}recv because its first argument is StringInfo, so I ended up
> > implementing my own bytea->int{2,4,8} functions.
> >
> > I think there may be value in (2) as well. It's implemented in v3-0002
> > and I did my best to clarify the commit message. On the flip side the
> > situation when one wants something like extracting int4 from a
> > bytea(or vice versa) and is not happy with convenience and/or
> > performance of substr()+casts is arguably rare. I'll be fine with
> > whatever consensus the community reaches about this patch.
>
> PFA the rebased patch set v4.

Rebased, v5.

-- 
Best regards,
Aleksander Alekseev

  21. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Michael Paquier <michael@paquier.xyz> — 2025-01-09T07:55:18Z

    On Fri, Oct 18, 2024 at 05:20:42PM +0300, Aleksander Alekseev wrote:
> Rebased, v5.

v5-0001 includes the following output:

--- a/src/test/regress/expected/opr_sanity.out
+++ b/src/test/regress/expected/opr_sanity.out
@@ -126,9 +126,12 @@ WHERE p1.oid < p2.oid AND
      p1.proretset != p2.proretset OR
      p1.provolatile != p2.provolatile OR
      p1.pronargs != p2.pronargs);
- oid | proname | oid | proname 
------+---------+-----+---------
-(0 rows)
+ oid  | proname  | oid  | proname 
+------+----------+------+---------
+ 2405 | int2send | 8577 | bytea
+ 2407 | int4send | 8578 | bytea
+ 2409 | int8send | 8579 | bytea
+(3 rows)

This should not happen, as you are using multiple times the same
function.
--
Michael

  22. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Aleksander Alekseev <aleksander@timescale.com> — 2025-01-13T17:35:59Z

    Hi Michael,

> v5-0001 includes the following output:
>
> --- a/src/test/regress/expected/opr_sanity.out
> +++ b/src/test/regress/expected/opr_sanity.out
> @@ -126,9 +126,12 @@ WHERE p1.oid < p2.oid AND
>       p1.proretset != p2.proretset OR
>       p1.provolatile != p2.provolatile OR
>       p1.pronargs != p2.pronargs);
> - oid | proname | oid | proname
> ------+---------+-----+---------
> -(0 rows)
> + oid  | proname  | oid  | proname
> +------+----------+------+---------
> + 2405 | int2send | 8577 | bytea
> + 2407 | int4send | 8578 | bytea
> + 2409 | int8send | 8579 | bytea
> +(3 rows)
>
> This should not happen, as you are using multiple times the same
> function.

Thanks. Here is the corrected patch.

Besides fixing opr_sanity test I corrected error messages:

-ERROR:  bytea size 3 out of valid range, 0..2
+ERROR:  bytea out of valid range, ''..'\xFFFF'

... and also included tests for min/max integer values.

I discarded the 0002 patch that implemented get_bytes() / set_bytes().
This part doesn't seem to get much support, so let's focus on casting.

--
Best regards,
Aleksander Alekseev

  23. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2025-01-13T18:24:37Z

    On Mon, 13 Jan 2025 at 17:36, Aleksander Alekseev
<aleksander@timescale.com> wrote:
>
> Besides fixing opr_sanity test I corrected error messages:
>
> -ERROR:  bytea size 3 out of valid range, 0..2
> +ERROR:  bytea out of valid range, ''..'\xFFFF'
>

"smallint out of range", "integer out of range" and "bigint out of
range" would be more consistent with existing error messages.

Regards,
Dean

  24. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2025-01-13T19:23:33Z

    On 2025-Jan-13, Dean Rasheed wrote:

> On Mon, 13 Jan 2025 at 17:36, Aleksander Alekseev
> <aleksander@timescale.com> wrote:
> >
> > Besides fixing opr_sanity test I corrected error messages:
> >
> > -ERROR:  bytea size 3 out of valid range, 0..2
> > +ERROR:  bytea out of valid range, ''..'\xFFFF'
> 
> "smallint out of range", "integer out of range" and "bigint out of
> range" would be more consistent with existing error messages.

But these don't show the acceptable range. We have these that do:

#: utils/adt/varbit.c:1824 utils/adt/varbit.c:1882
#, c-format
msgid "bit index %d out of valid range (0..%d)"

#: utils/adt/varlena.c:3218 utils/adt/varlena.c:3285
#, c-format
msgid "index %d out of valid range, 0..%d"

#: utils/adt/varlena.c:3249 utils/adt/varlena.c:3321
#, c-format
msgid "index %lld out of valid range, 0..%lld"

#: utils/misc/guc.c:3130
#, c-format
msgid "%d%s%s is outside the valid range for parameter \"%s\" (%d .. %d)"


The quoting in Aleksander's proposal is not great.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"Update: super-fast reaction on the Postgres bugs mailing list. The report
was acknowledged [...], and a fix is under discussion.
The wonders of open-source !"
             https://twitter.com/gunnarmorling/status/1596080409259003906

  25. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2025-01-13T21:53:22Z

    On Mon, 13 Jan 2025 at 19:23, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> But these don't show the acceptable range. We have these that do:
>
> #: utils/adt/varbit.c:1824 utils/adt/varbit.c:1882
> #, c-format
> msgid "bit index %d out of valid range (0..%d)"
>
> #: utils/adt/varlena.c:3218 utils/adt/varlena.c:3285
> #, c-format
> msgid "index %d out of valid range, 0..%d"
>
> #: utils/adt/varlena.c:3249 utils/adt/varlena.c:3321
> #, c-format
> msgid "index %lld out of valid range, 0..%lld"
>
> #: utils/misc/guc.c:3130
> #, c-format
> msgid "%d%s%s is outside the valid range for parameter \"%s\" (%d .. %d)"
>

Those are all instances of a value that's outside a specific range
that you might not otherwise know, rather than being out of range of
the type itself. For that, we generally don't say what the range of
the type is. For example, we currently do:

select repeat('1', 50)::bit(50)::int;
ERROR:  integer out of range

Regards,
Dean

  26. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Aleksander Alekseev <aleksander@timescale.com> — 2025-01-14T13:25:19Z

    Hi Dean,

> Those are all instances of a value that's outside a specific range
> that you might not otherwise know, rather than being out of range of
> the type itself. For that, we generally don't say what the range of
> the type is. For example, we currently do:
>
> select repeat('1', 50)::bit(50)::int;
> ERROR:  integer out of range

Thanks. I agree that the proposed error messages look nicer than the
one I used in v6. Here is the corrected patch.


--
Best regards,
Aleksander Alekseev

  27. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2025-01-17T16:29:07Z

    On Tue, 14 Jan 2025 at 13:25, Aleksander Alekseev
<aleksander@timescale.com> wrote:
>
> Thanks. I agree that the proposed error messages look nicer than the
> one I used in v6. Here is the corrected patch.
>

This should use ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE, rather than
ERRCODE_INVALID_PARAMETER_VALUE, for consistency with other similar
errors.

The bytea -> int[248] cast functions should not be marked as leakproof
-- see the docs on the CREATE FUNCTION page: functions that raise
errors for some input values but not others, are not leakproof. This
is why, for example, the int -> bigint cast is leakproof, but the
bigint -> int cast is not.

Functions working with int8 values should normally go in
utils/adt/int8.c, not utils/adt/int.c. However, I think that
utils/adt/varlena.c would be a better place for all these functions,
because they have more to do with bytea than integer types, and this
would allow them to be kept together, similar to how all the bit <->
integer cast functions are in utils/adt/varbit.c.

There's no documentation for these new casts. The obvious place to put
it would be in section 9.5 "Binary String Functions and Operators",
which would be consistent with the idea that these are being regarded
primarily as bytea operations, rather than integer operations (just as
the bit <-> integer casts are documented in 9.6 "Bit String Functions
and Operators").

Regards,
Dean

  28. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Aleksander Alekseev <aleksander@timescale.com> — 2025-01-20T14:01:53Z

    Hi Dean,

> This should use ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE, rather than
> ERRCODE_INVALID_PARAMETER_VALUE, for consistency with other similar
> errors.
>
> The bytea -> int[248] cast functions should not be marked as leakproof
> -- see the docs on the CREATE FUNCTION page: functions that raise
> errors for some input values but not others, are not leakproof. This
> is why, for example, the int -> bigint cast is leakproof, but the
> bigint -> int cast is not.
>
> Functions working with int8 values should normally go in
> utils/adt/int8.c, not utils/adt/int.c. However, I think that
> utils/adt/varlena.c would be a better place for all these functions,
> because they have more to do with bytea than integer types, and this
> would allow them to be kept together, similar to how all the bit <->
> integer cast functions are in utils/adt/varbit.c.
>
> There's no documentation for these new casts. The obvious place to put
> it would be in section 9.5 "Binary String Functions and Operators",
> which would be consistent with the idea that these are being regarded
> primarily as bytea operations, rather than integer operations (just as
> the bit <-> integer casts are documented in 9.6 "Bit String Functions
> and Operators").

Many thanks for your great feedback. Here is the corrected patch.


On Fri, Jan 17, 2025 at 7:29 PM Dean Rasheed <dean.a.rasheed@gmail.com>
wrote:

> On Tue, 14 Jan 2025 at 13:25, Aleksander Alekseev
> <aleksander@timescale.com> wrote:
> >
> > Thanks. I agree that the proposed error messages look nicer than the
> > one I used in v6. Here is the corrected patch.
> >
>
> This should use ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE, rather than
> ERRCODE_INVALID_PARAMETER_VALUE, for consistency with other similar
> errors.
>
> The bytea -> int[248] cast functions should not be marked as leakproof
> -- see the docs on the CREATE FUNCTION page: functions that raise
> errors for some input values but not others, are not leakproof. This
> is why, for example, the int -> bigint cast is leakproof, but the
> bigint -> int cast is not.
>
> Functions working with int8 values should normally go in
> utils/adt/int8.c, not utils/adt/int.c. However, I think that
> utils/adt/varlena.c would be a better place for all these functions,
> because they have more to do with bytea than integer types, and this
> would allow them to be kept together, similar to how all the bit <->
> integer cast functions are in utils/adt/varbit.c.
>
> There's no documentation for these new casts. The obvious place to put
> it would be in section 9.5 "Binary String Functions and Operators",
> which would be consistent with the idea that these are being regarded
> primarily as bytea operations, rather than integer operations (just as
> the bit <-> integer casts are documented in 9.6 "Bit String Functions
> and Operators").
>
> Regards,
> Dean
>


-- 
Best regards,
Aleksander Alekseev

  29. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Peter Eisentraut <peter@eisentraut.org> — 2025-01-22T18:44:52Z

    On 20.01.25 15:01, Aleksander Alekseev wrote:
>  > This should use ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE, rather than
>  > ERRCODE_INVALID_PARAMETER_VALUE, for consistency with other similar
>  > errors.
>  >
>  > The bytea -> int[248] cast functions should not be marked as leakproof
>  > -- see the docs on the CREATE FUNCTION page: functions that raise
>  > errors for some input values but not others, are not leakproof. This
>  > is why, for example, the int -> bigint cast is leakproof, but the
>  > bigint -> int cast is not.
>  >
>  > Functions working with int8 values should normally go in
>  > utils/adt/int8.c, not utils/adt/int.c. However, I think that
>  > utils/adt/varlena.c would be a better place for all these functions,
>  > because they have more to do with bytea than integer types, and this
>  > would allow them to be kept together, similar to how all the bit <->
>  > integer cast functions are in utils/adt/varbit.c.
>  >
>  > There's no documentation for these new casts. The obvious place to put
>  > it would be in section 9.5 "Binary String Functions and Operators",
>  > which would be consistent with the idea that these are being regarded
>  > primarily as bytea operations, rather than integer operations (just as
>  > the bit <-> integer casts are documented in 9.6 "Bit String Functions
>  > and Operators").
> 
> Many thanks for your great feedback. Here is the corrected patch.

These casts appear to use a particular endianness, but they don't 
document which one, and there is no explanation anywhere why that one is 
the right one.

  30. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Aleksander Alekseev <aleksander@timescale.com> — 2025-01-23T14:30:19Z

    Hi Peter,

> These casts appear to use a particular endianness, but they don't
> document which one, and there is no explanation anywhere why that one is
> the right one.

Right, I choose network order / big-endian. I agree that it would make
sense to emphasise this fact in the documentation below the examples.
Unfortunately I have no good reason for this particular design choice
other than "well this is how it is custom to represent numbers in a
consistent manner between different platforms but other than that the
choice was rather arbitrary". Should I just rephrase it a bit and add
to the documentation?

-- 
Best regards,
Aleksander Alekseev

  31. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2025-01-23T15:46:47Z

    On Thu, 23 Jan 2025 at 14:30, Aleksander Alekseev
<aleksander@timescale.com> wrote:
>
> Hi Peter,
>
> > These casts appear to use a particular endianness, but they don't
> > document which one, and there is no explanation anywhere why that one is
> > the right one.
>
> Right, I choose network order / big-endian. I agree that it would make
> sense to emphasise this fact in the documentation below the examples.
> Unfortunately I have no good reason for this particular design choice
> other than "well this is how it is custom to represent numbers in a
> consistent manner between different platforms but other than that the
> choice was rather arbitrary". Should I just rephrase it a bit and add
> to the documentation?
>

IMO big-endian is the most convenient byte-ordering to use here,
because then the string representation of the bytea is consistent with
the hex representation of the integer. It's also consistent with the
integer-to-bit casts, which output the most significant bits first,
starting with the sign bit.

As far as the docs go, it's important to document precisely what
format is used, but I don't think it needs to explain why that choice
was made. It should also mention the size of the result and that it's
the two's complement representation, since there are other possible
representations of integers. So I think it would be sufficient for the
initial paragraph to say something like "Casting an integer to a bytea
produces 2, 4, or 8 bytes, depending on the width of the integer type.
The result is the two's complement representation of the integer, with
the most significant byte first.", and then list the examples to
demonstrate that.

Regards,
Dean

  32. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Aleksander Alekseev <aleksander@timescale.com> — 2025-01-24T13:00:29Z

    Hi Dean,

> IMO big-endian is the most convenient byte-ordering to use here,
> because then the string representation of the bytea is consistent with
> the hex representation of the integer. It's also consistent with the
> integer-to-bit casts, which output the most significant bits first,
> starting with the sign bit.
>
> As far as the docs go, it's important to document precisely what
> format is used, but I don't think it needs to explain why that choice
> was made. It should also mention the size of the result and that it's
> the two's complement representation, since there are other possible
> representations of integers. So I think it would be sufficient for the
> initial paragraph to say something like "Casting an integer to a bytea
> produces 2, 4, or 8 bytes, depending on the width of the integer type.
> The result is the two's complement representation of the integer, with
> the most significant byte first.", and then list the examples to
> demonstrate that.

Thank you. Here is the corrected patch.

--
Best regards,
Aleksander Alekseev

  33. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2025-03-01T11:30:47Z

    On Fri, 24 Jan 2025 at 13:00, Aleksander Alekseev
<aleksander@timescale.com> wrote:
>
> Thank you. Here is the corrected patch.
>

This looks pretty good to me. I have a just a couple of minor comments:

* The loop in bytea_integer() can be written more simply as a "for"
loop. Given that it's only a few lines of code, it might as well just
be coded directly in each cast function, which avoids the need to go
via a 64-bit integer for every case. In addition, it should use the
BITS_PER_BYTE macro rather than "8". Doing that leads to code that's
consistent with bittoint4() and bittoint8().

* In pg_proc.dat, it's not necessary to write "proleakproof => 'f'",
because that's the default, and no other function does that.

* I think it's worth using slightly more non-trivial doc examples,
including positive and negative cases, to make the behaviour more
obvious.

* I also tweaked the regression tests a bit, and copied the existing
test style which displays both the expected and actual results from
each test.

With those updates, I think this is ready for commit, which I'll do in
a day or two, if there are no further comments.

Regards,
Dean

  34. Re: [PATCH] Add get_bytes() and set_bytes() functions

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2025-03-07T10:08:41Z

    On Sat, 1 Mar 2025 at 11:30, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
>
> With those updates, I think this is ready for commit, which I'll do in
> a day or two, if there are no further comments.
>

Committed.

Regards,
Dean