I wrote:
> It would be easier to get this done if you had addressed any of the
> objections to the patch as given. Integer-overflow handling is still
> missing, and you still are assuming that it's okay to change catalog
> entries in released branches.
Since we are hard upon the feature freeze
"movead...@highgo.ca" writes:
> After several patch change by hacker's proposal, I think it's ready to
> commit, can we commit it before doing the code freeze for pg-13?
It would be easier to get this done if you had addressed any of the
objections to the patch as given. Integer-overflow
Hello hackers,
After several patch change by hacker's proposal, I think it's ready to
commit, can we commit it before doing the code freeze for pg-13?
Regards,
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca
EMAIL: mailto:movead(dot)li(at)highgo(dot)ca
>In existing releases, the SQL definitions are set_bit(bytea,int4,int4)
>and get_bit(bytea,int4) and cannot be changed to not break the API.
>So the patch meant for existing releases has to deal with a too-narrow
>int32 bit number.
>Internally in the C functions, you may convert that number to
movead...@highgo.ca wrote:
> >I believe the formula for the upper limit in the 32-bit case is:
> > (len <= PG_INT32_MAX / 8) ? (len*8 - 1) : PG_INT32_MAX;
>
> >These functions could benefit from a comment mentioning that
> >they cannot reach the full extent of a bytea, because of the
>Some comments about the set_bit/get_bit parts.
>I'm reading long_bytea_string_bug_fix_ver6_pg10.patch, but that
>applies probably to the other files meant for the existing releases
>(I think you could get away with only one patch for backpatching
>and one patch for v13, and committers will sort
ilm...@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes:
> Tom Lane writes:
>> Yeah, I'd noticed those on previous readings of the patch. They'd almost
>> certainly fail on some of our older/smaller buildfarm members, so they're
>> not getting committed, even if they didn't require
Tom Lane writes:
> "Daniel Verite" writes:
>> These 2 tests need to allocate big chunks of contiguous memory, so they
>> might fail for lack of memory on tiny machines, and even when not failing,
>> they're pretty slow to run. Are they worth the trouble?
>
> Yeah, I'd noticed those on previous
"Daniel Verite" writes:
> These 2 tests need to allocate big chunks of contiguous memory, so they
> might fail for lack of memory on tiny machines, and even when not failing,
> they're pretty slow to run. Are they worth the trouble?
Yeah, I'd noticed those on previous readings of the patch.
movead...@highgo.ca wrote:
> A little update for the patch, and patches for all stable avilable.
Some comments about the set_bit/get_bit parts.
I'm reading long_bytea_string_bug_fix_ver6_pg10.patch, but that
applies probably to the other files meant for the existing releases
(I think
>Sorry about that, attached is the changed patch for PG13, and the one
>for older branches will send sooner.
A little update for the patch, and patches for all stable avilable.
Regards,
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca
EMAIL: mailto:movead(dot)li(at)highgo(dot)ca
>I don't think this has really solved the overflow hazards. For example,
>in binary_encode we've got
>resultlen = enc->encode_len(VARDATA_ANY(data), datalen);
>result = palloc(VARHDRSZ + resultlen);
>and all you've done about that is changed resultlen from int to int64.
>On a 64-bit machine,
"movead...@highgo.ca" writes:
> [ long_bytea_string_bug_fix_ver5.patch ]
I don't think this has really solved the overflow hazards. For example,
in binary_encode we've got
resultlen = enc->encode_len(VARDATA_ANY(data), datalen);
result = palloc(VARHDRSZ + resultlen);
and all
>+ int64 res,resultlen;
>It's better to have them on separate lines.
Sorry for that, done.
>-unsigned
>+int64
> hex_decode(const char *src, unsigned len, char *dst)
>Do we want to explicitly cast the return value to int64? Will build on some
>platform crib if not done so?
>I don't know of such
Thanks for the changes,
+ int64 res,resultlen;
It's better to have them on separate lines.
-unsigned
+int64
hex_decode(const char *src, unsigned len, char *dst)
Do we want to explicitly cast the return value to int64? Will build on some
platform crib if not done so? I don't know of such a
I want to resent the mail, because last one is in wrong format and hardly to
read.
In addition, I think 'Size' or 'size_t' is rely on platform, they may can't
work on 32bit
system. So I choose 'int64' after ashutosh's review.
>>I think the second argument indicates the bit position, which
>I think the second argument indicates the bit position, which would be max
>bytea length * 8. If max bytea length covers whole int32, the second argument
>>needs to be wider i.e. int64.
Yes, it makes sence and followed.
>I think we need a similar change in byteaGetBit() and byteaSetBit() as
"Daniel Verite" writes:
> So aside from the integer overflow bug, isn't there the issue that the
> "offset" argument of get_bit() and set_bit() should have been an
> int8 in the first place?
Good point, but a fix for that wouldn't be back-patchable.
It does suggest that we should just make all
Ashutosh Bapat wrote:
> I think we need a similar change in byteaGetBit() and byteaSetBit()
> as well.
get_bit() and set_bit() as SQL functions take an int4 as the "offset"
argument representing the bit number, meaning that the maximum value
that can be passed is 2^31-1.
But the maximum
On Thu, 26 Mar 2020 at 19:39, Tom Lane wrote:
> Ashutosh Bapat writes:
> > On Wed, 18 Mar 2020 at 08:18, movead...@highgo.ca
> > wrote:
> >> if we change return type of all those functions to int64, we won't need
> >> this cast.
> >> I change the 'encode' function, it needs an int64 return
Ashutosh Bapat writes:
> On Wed, 18 Mar 2020 at 08:18, movead...@highgo.ca
> wrote:
>> if we change return type of all those functions to int64, we won't need
>> this cast.
>> I change the 'encode' function, it needs an int64 return type, but keep
>> other
>> functions in 'pg_encoding', because
On Wed, 18 Mar 2020 at 08:18, movead...@highgo.ca
wrote:
>
> Hello thanks for the detailed review,
>
> >I think the second argument indicates the bit position, which would be
> max bytea length * 8. If max bytea length covers whole int32, the second
> argument >needs to be wider i.e. int64.
>
Hello thanks for the detailed review,
>I think the second argument indicates the bit position, which would be max
>bytea length * 8. If max bytea length covers whole int32, the second argument
>>needs to be wider i.e. int64.
Yes, it makes sence and followed.
> Some more comments on the patch
On Fri, 13 Mar 2020 at 08:48, movead...@highgo.ca
wrote:
> Thanks for the reply.
>
> >Why have you used size? Shouldn't we use int64?
> Yes, thanks for the point, I have changed the patch.
>
>
Thanks for the patch.
> >If get_bit()/set_bit() accept the second argument as int32, it can not
>
Thanks for the reply.
>Why have you used size? Shouldn't we use int64?
Yes, thanks for the point, I have changed the patch.
>If get_bit()/set_bit() accept the second argument as int32, it can not
>be used to set bits whose number does not fit 32 bits. I think we need
>to change the type of the
Hi
On Thu, Mar 12, 2020 at 9:21 AM movead...@highgo.ca wrote:
>
> Hello hackers,
>
> I found an issue about get_bit() and set_bit() function,here it is:
>
> postgres=# select
> get_bit(pg_read_binary_file('/home/movead/temp/file_seek/f_512M'), 0);
> 2020-03-12
Hello hackers,
I found an issue about get_bit() and set_bit() function,here it is:
postgres=# select
get_bit(pg_read_binary_file('/home/movead/temp/file_seek/f_512M'), 0);
2020-03-12 10:05:23.296 CST [10549] ERROR: index 0 out of valid range, 0..-1
2020-03-12
27 matches
Mail list logo
