Patch for PGunescapeBytea

Started by Ben Lambabout 23 years ago6 messagespatches
Jump to latest
#1Ben Lamb
pgsql-patches@zurgy.org

Hi,

I found the libpq function PGunescapeBytea a little slow. It was taking a
minute and a half to decode a 500Kb on a fairly fast machine. I think the
culprit is sscanf.

I attach a patch that replaces the function with one used to perform the same
task in pyPgSQL (a Python interface to PostgreSQL). This code was written by
Billy Allie, author of pyPgSQL. I've changed a few variable names to match
those in the original code and removed a bit of Pythonness.

Billy has kindly looked at the code and points out that it is slightly
stricter than the original implementation and if it encounters an invalid
bytea such as '\12C' it drops the unescape '\' and outputs '12C'.

The code is licensed by the author under a BSD license.

I've performed limited testing of the function by putting JPEGs into
PostgreSQL, extracting them using them using the new function and diffing
against the original files.

The new function is significantly faster on my machine with the JPEGs being
decoded in less than a second. I attach a modified libpq example program that
I used for my testing.

Regards,

Ben Lamb.

The patch modifies fe-exec.c in /src/interfaces/libpq from PostgreSQL 7.3.2.

Attachments:

fe-exec.c.patchtext/x-diff; charset=us-ascii; name=fe-exec.c.patchDownload+86-102
testlibpq.ctext/x-csrc; charset=us-ascii; name=testlibpq.cDownload
In reply to: Ben Lamb (#1)
Re: Patch for PGunescapeBytea

Ben Lamb <pgsql-patches@zurgy.org> wrote:

Hi,

I found the libpq function PGunescapeBytea a little slow. It was
taking a minute and a half to decode a 500Kb on a fairly fast
machine. I think the culprit is sscanf.

[snip]

I think these lines:

buffer = realloc(buffer, buflen);
---
if (buffer == NULL)
return NULL;
---

are wrong. Shouldn't one do:
---
tmpbuf=realloc(buf,...);
if (!tmpbuf)
free(buf), return 0;
---

to avoid a memory leak?

... just checking ;)

Magnus

#3Ben Lamb
pgsql-patches@zurgy.org
In reply to: Magnus Naeslund(f) (#2)
Re: Patch for PGunescapeBytea

On Monday 05 May 2003 5:45 pm, Magnus Naeslund(f) wrote:

I think these lines:

buffer = realloc(buffer, buflen);
---
if (buffer == NULL)
return NULL;
---

are wrong. Shouldn't one do:
---
tmpbuf=realloc(buf,...);
if (!tmpbuf)
free(buf), return 0;
---

Thanks for pointing this out, do I need to send an updated patch to the list?

Ben.

#4Ben Lamb
pgsql-patches@zurgy.org
In reply to: Magnus Naeslund(f) (#2)
Re: Patch for PGunescapeBytea

Here is a new patch for src/interfaces/libpq/fe-exec.c that incorporates the
change suggested by Magnus. The patch significantly improves the speed of
PGunescapeBytea().

Ben.

Show quoted text

[snip]

I think these lines:

buffer = realloc(buffer, buflen);
---
if (buffer == NULL)
return NULL;
---

are wrong. Shouldn't one do:
---
tmpbuf=realloc(buf,...);
if (!tmpbuf)
free(buf), return 0;
---

to avoid a memory leak?

... just checking ;)

Magnus

Attachments:

fe-exec.c.patchtext/x-diff; charset=iso-8859-1; name=fe-exec.c.patchDownload+42-75
#5Bruce Momjian
bruce@momjian.us
In reply to: Ben Lamb (#1)
Re: Patch for PGunescapeBytea

(I will apply the newer version.)

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

---------------------------------------------------------------------------

Ben Lamb wrote:

Hi,

I found the libpq function PGunescapeBytea a little slow. It was taking a
minute and a half to decode a 500Kb on a fairly fast machine. I think the
culprit is sscanf.

I attach a patch that replaces the function with one used to perform the same
task in pyPgSQL (a Python interface to PostgreSQL). This code was written by
Billy Allie, author of pyPgSQL. I've changed a few variable names to match
those in the original code and removed a bit of Pythonness.

Billy has kindly looked at the code and points out that it is slightly
stricter than the original implementation and if it encounters an invalid
bytea such as '\12C' it drops the unescape '\' and outputs '12C'.

The code is licensed by the author under a BSD license.

I've performed limited testing of the function by putting JPEGs into
PostgreSQL, extracting them using them using the new function and diffing
against the original files.

The new function is significantly faster on my machine with the JPEGs being
decoded in less than a second. I attach a modified libpq example program that
I used for my testing.

Regards,

Ben Lamb.

The patch modifies fe-exec.c in /src/interfaces/libpq from PostgreSQL 7.3.2.

[ Attachment, skipping... ]

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
#6Bruce Momjian
bruce@momjian.us
In reply to: Ben Lamb (#4)
Re: Patch for PGunescapeBytea

Patch applied. Thanks.

---------------------------------------------------------------------------

Ben Lamb wrote:

Here is a new patch for src/interfaces/libpq/fe-exec.c that incorporates the
change suggested by Magnus. The patch significantly improves the speed of
PGunescapeBytea().

Ben.

[snip]

I think these lines:

buffer = realloc(buffer, buflen);
---
if (buffer == NULL)
return NULL;
---

are wrong. Shouldn't one do:
---
tmpbuf=realloc(buf,...);
if (!tmpbuf)
free(buf), return 0;
---

to avoid a memory leak?

... just checking ;)

Magnus

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faqs/FAQ.html

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Attachments:

/bjm/difftext/plainDownload+98-115