Hello, I don't think so many people have used shift operators
with too-large or negative shift amount relying on the
undetermined behavior.
But if explicit definition is required, I prefer the result of a
shift operation with too-large shift mount is simplly zero. And
shift left with negative shift amount should do right
shift. Addition to that, no error nor warning won't be needed.
Like this,
Datum int8shl(PG_FUNCTION_ARGS) { int64 arg1 = PG_GETARG_INT64(0); int32 arg2
=PG_GETARG_INT32(1);
if (arg2 > 63 || arg2 < -63) PG_RETURN_INT64(0L); if (arg2 < 0) PG_RETURN_INT64(arg1
>>(-arg2));
PG_RETURN_INT64(arg1 << arg2); }
The obvious problem on this is the lack of compatibility with
existing behavior:(
Thoughts? Opinions?
regards,
At Wed, 16 Sep 2015 15:16:27 +0800, Craig Ringer <craig@2ndquadrant.com> wrote in
<CAMsr+YE+0KJuOJfbB2nLVfU+14R50Yi90e_8DewLV9jX+ro1zg@mail.gmail.com>
> Hi all
>
> Our implementation of << is a direct wrapper around the C operator. It
> does not check the right-hand side's value.
>
>
> Datum
> int8shl(PG_FUNCTION_ARGS)
> {
> int64 arg1 = PG_GETARG_INT64(0);
> int32 arg2 = PG_GETARG_INT32(1);
>
> PG_RETURN_INT64(arg1 << arg2);
> }
>
> This means that an operation like:
>
> 1::bigint << 65
>
> directly relies on the compiler and platforms' handling of the
> undefined shift. On x64 intel gcc linux it does a rotation but that's
> not AFAIK guaranteed by anything, and we should probably not be
> relying on this or exposing it at the user level.
>
>
>
> Pg returns:
>
> test=> SELECT BIGINT '1' << 66;
> ?column?
> ----------
> 4
> (1 row)
>
> A test program:
>
> #include "stdio.h"
> int main(int argc, char * argv[])
> {
> printf("Result is %ld", 1l << 66);
> return 0;
> }
>
> returns zero when the compiler constant-folds, but when done at runtime:
>
> #include <stdio.h>
> #include <stdlib.h>
> int main(int argc, char * argv[])
> {
> const char * num = "66";
> printf("Result is %ld", 1l << atoi(num));
> return 0;
> }
>
>
> IMO we should specify the behaviour in this case. Then issue a WARNING
> that gets promoted to an ERROR in a few versions.
>
> Consideration of << with a negative right-operand, and of
> out-of-bounds >>, is probably also needed.
>
> Thoughts?
--
Kyotaro Horiguchi
NTT Open Source Software Center
