Requires a 4-parameter subbits function about emqx HOT 5 OPEN

PR with fix has been created:

#12652

from emqx.

@kjellwinblad Hi, I revisited the code of this PR once again, and found that there may be some problems in the implementation.

Neither float nor bits types distinguish between signed and unsigned. Instead, they should distinguish between big endian and little endian.

So the 5th param of the subbits/5 should be Endianness.

from emqx.

SELECT
    subbits(hexstr2bin('9F4E58'), 1, 16, 'float', 'unsigned', 'big') as t1,
    subbits(hexstr2bin('9F4E58'), 1, 16, 'float', 'signed', 'big') as t2,
    subbits(hexstr2bin('9F4E58'), 1, 16, 'float', 'unsigned', 'little') as t3,
    bin2hexstr(subbits(hexstr2bin('9F4E58'), 1, 16, 'bits', 'unsigned', 'big')) as t4,
    bin2hexstr(subbits(hexstr2bin('9F4E58'), 1, 16, 'bits', 'signed', 'big')) as t5,
    bin2hexstr(subbits(hexstr2bin('9F4E58'), 1, 16, 'bits', 'unsigned', 'little')) as t6
FROM "t/#"

Result:

{
    "t1": -0.00713348388671875,
    "t2": -0.00713348388671875,
    "t3": 26.484375,
    "t4": "9F4E",
    "t5": "9F4E",
    "t6": "9F4E"
}

Supplement: float distinguishes big and little endian, bits does not distinguish.

from emqx.

The argument list order is the same in subbits/4 and subbits/5 that I added as it is in subbits/6. This is also the order that is documented https://www.emqx.io/docs/en/latest/data-integration/rule-sql-builtin-functions.html#bit-sequence-functions .

I think we can still change subbits/5 as this function has not existed before (so we know that no one is using it) if we also document clearly that the order differ. However, I don't think this is a good idea. It is a bit strange to have a function with the same name and different arity, but where the argument list order differ. I think this will lead to more confusion and errors than the potential gain of sometimes not having to type one unnecessary argument. Of course, it would be be better if the order between the two last parameters in subbits/6 was swapped and subbits/5 had endianness as the fifth parameter, but unfortunately I think this is too late now since subbits/6 has been released and documented so it might already be in use in production systems.

Supplement: float distinguishes big and little endian, bits does not distinguish.

I guess this is how it should be? I'm not sure if we can change this now since the function has existed in earlier releases.

from emqx.

I agree with this.

but unfortunately I think this is too late now since subbits/6 has been released and documented so it might already be in use in production systems.

I think we could remove subbits/5, it just seems to make the situation more complicated.

from emqx.