Patch: Allow substring/replace() to get/set bit values

Lists: pgsql-hackers
From: Leonardo F <m_lists(at)yahoo(dot)it>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Patch: Allow substring/replace() to get/set bit values
Date: 2010-01-07 12:02:16
Message-ID: 757953.70187.qm@web29001.mail.ird.yahoo.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hi all,

attached a patch that adds the following functions for bit string:

- overlay
- get_bit
- set_bit

Some info:

1) overlay is implemented as calls to substring; given the different way substring behaves when used with strings vs bit strings:

test=# SELECT substring(B'1111000000000001' from 1 for -1);
substring
------------------
1111000000000001
(1 row)

test=# SELECT substring('1111000000000001' from 1 for -1);
ERROR: negative substring length not allowed

I don't think that this overlay implementation is what we want?

Example:

test=# SELECT overlay(B'1111' placing B'01' from 1 for 2);
overlay
---------
0111
(1 row)

(looks ok)

test=# SELECT overlay(B'1111' placing B'01' from 0 for 2);
overlay
-----------
111101111
(1 row)

????
This happens because substring(bit, pos, -1) means substring(bit, pos, length(bit string)),
and < -1 values for bit substring parameters are allowed: is this a bug in bit substring???

2) I tried implementing bit_get and bit_set as calls to overlay/substring:

DATA(insert OID = 3032 ( get_bit PGNSP PGUID 14 1 0 0 f f f t f i 2 0 23 "1560 23" _null_ _null_ _null_ _null_ "select (pg_catalog.substring($1, $2, 1))::int4" _null_ _null_ _null_ ));
DESCR("get bit");
DATA(insert OID = 3033 ( set_bit PGNSP PGUID 14 1 0 0 f f f t f i 3 0 1560 "1560 23 23" _null_ _null_ _null_ _null_ "select pg_catalog.overlay($1, $3::bit, $2, 1)" _null_ _null_ _null_ ));
DESCR("set bit");

but this doesn't give any check on the values provided:
that the bit looked for is in fact in the right range, and that the bit in set_bit is in fact a bit
(I don't like the idea of writing "select set_bit(B'01010111', B'1')" instead of
"select set_bit(B'01010111', 1)" ).
So I coded them in proper C internal functions.

Leonardo

Attachment Content-Type Size
patch_bit.patch application/octet-stream 7.8 KB

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Leonardo F <m_lists(at)yahoo(dot)it>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Patch: Allow substring/replace() to get/set bit values
Date: 2010-01-07 15:36:55
Message-ID: 603c8f071001070736x17f823a1wcf8429ced3c75430@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Thu, Jan 7, 2010 at 7:02 AM, Leonardo F <m_lists(at)yahoo(dot)it> wrote:
> attached a patch that adds the following functions for bit string:

Thanks! Please add your patch here:

https://commitfest.postgresql.org/action/commitfest_view/open

The next CommitFest starts January 15th.

...Robert


From: Leonardo F <m_lists(at)yahoo(dot)it>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Patch: Allow substring/replace() to get/set bit values
Date: 2010-01-07 16:05:34
Message-ID: 250592.50414.qm@web29009.mail.ird.yahoo.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

> Thanks! Please add your patch here:
>
> https://commitfest.postgresql.org/action/commitfest_view/open
>

Ok; but what about what I said about the difference between bit/string substring?
That affects overlay behaviour for bit...

I've even got

"ERROR: invalid memory alloc request size 4244635647"

with:

SELECT substring(B'1111000000000001' from 5 for -2);

(this is 8.4.2, windows version, not modified...)

test=# SELECT substring(B'1111000000000001' from 1 for -1);
substring
------------------
1111000000000001
(1 row)

test=# SELECT substring('1111000000000001' from 1 for -1);
ERROR: negative substring length not allowed


From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Leonardo F <m_lists(at)yahoo(dot)it>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Patch: Allow substring/replace() to get/set bit values
Date: 2010-01-07 16:37:12
Message-ID: 603c8f071001070837y1f544978i61485aefb988c615@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Thu, Jan 7, 2010 at 11:05 AM, Leonardo F <m_lists(at)yahoo(dot)it> wrote:
>> Thanks!  Please add your patch here:
>>
>> https://commitfest.postgresql.org/action/commitfest_view/open
>>
>
>
> Ok; but what about what I said about the difference between bit/string substring?
> That affects overlay behaviour for bit...
>
>
> I've even got
>
> "ERROR:  invalid memory alloc request size 4244635647"
>
> with:
>
>
> SELECT substring(B'1111000000000001' from 5 for -2);
>
> (this is 8.4.2, windows version, not modified...)
>
>
>
> test=# SELECT substring(B'1111000000000001' from 1 for -1);
> substring
> ------------------
> 1111000000000001
> (1 row)
>
>
> test=# SELECT substring('1111000000000001' from 1 for -1);
> ERROR:  negative substring length not allowed

I haven't tried to reproduce it, but that sounds like a bug.

....Robert


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Leonardo F <m_lists(at)yahoo(dot)it>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Patch: Allow substring/replace() to get/set bit values
Date: 2010-01-07 16:58:14
Message-ID: 26851.1262883494@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Leonardo F <m_lists(at)yahoo(dot)it> writes:
> I've even got
> "ERROR: invalid memory alloc request size 4244635647"
> with:
> SELECT substring(B'1111000000000001' from 5 for -2);

Hm, yeah, somebody was sloppy about exposing the three-argument
form of varbit substring and using -1 to represent the two-argument
form.

What we can do in the back branches is make the code treat any
negative value as meaning two-arg form. To throw an error we'd
need to refactor the pg_proc representation ...

regards, tom lane


From: Leonardo F <m_lists(at)yahoo(dot)it>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Patch: Allow substring/replace() to get/set bit values
Date: 2010-01-08 09:49:42
Message-ID: 636559.28192.qm@web29002.mail.ird.yahoo.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

> What we can do in the back branches is make the code treat any
> negative value as meaning two-arg form. To throw an error we'd
> need to refactor the pg_proc representation ...

I was going to fix that myself, but I think it has just been done.

How can I keep up with "who's doing what"?


From: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
To: Leonardo F <m_lists(at)yahoo(dot)it>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Patch: Allow substring/replace() to get/set bit values
Date: 2010-01-08 13:33:24
Message-ID: 20100108133324.GA3635@alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Leonardo F wrote:
> > What we can do in the back branches is make the code treat any
> > negative value as meaning two-arg form. To throw an error we'd
> > need to refactor the pg_proc representation ...
>
> I was going to fix that myself, but I think it has just been done.
>
> How can I keep up with "who's doing what"?

Read this list and pgsql-committers.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support


From: Dimitri Fontaine <dfontaine(at)hi-media(dot)com>
To: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Cc: Leonardo F <m_lists(at)yahoo(dot)it>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Patch: Allow substring/replace() to get/set bit values
Date: 2010-01-08 15:44:07
Message-ID: m2zl4o4n5k.fsf@hi-media.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> Leonardo F wrote:
>> How can I keep up with "who's doing what"?
>
> Read this list and pgsql-committers.

Or subscribe to the RSS feed from:

http://git.postgresql.org/gitweb?p=postgresql.git;a=summary

--
dim