Allow blob.read() to take optional destination buffer

[rogerbinns]

From austin.bingham on September 02, 2010 07:25:55

Currently, blob.read() returns either a str or bytes, both read-only structures. To get the data into a read-write structure (e.g. bytearray) I'm forced to do a copy, though I can then pass that read-write structure to blob.write().

It would be nice if I could pass an existing writable buffer to read() as the destination for the data. This appears to be compatible with the underlying sqlite call.

A new method (e.g. blob.read_to_buffer(buff) -> int) might be preferable since the method would need to communicate over/underflow situations.

Original issue: http://code.google.com/p/apsw/issues/detail?id=109

[rogerbinns]

From rogerbinns on September 02, 2010 09:05:13

I'll need to do some checking how these are presented at the C level.

My inclination is to add a readinto() method which takes a writable buffer and length/offset with suitable defaults. SQLite blobs can't change size so bad length/offset would result in an exception rather than returning ints.

Status: Accepted
Labels: -Type-Defect Type-Enhancement

[rogerbinns]

From austin.bingham on September 22, 2010 12:02:41

Here's an implementation of blob.readinto() that seems to fit the bill. The existing unit test suite was segfaulting on my machine, so I put the new tests int their own file; they should be easy to merge into the main suite, I think.

This uses the new buffer interface, so I believe that it only works for 2.6+. I haven't tested it on anything below that.

Attachment: blob.c readinto_tests.py

[rogerbinns]

From rogerbinns on September 22, 2010 12:22:23

I've also been working on it. You should have said something and I could have saved you the work!

The signature I use for mine is readinto(buffer, offset, length). Offset defaults to zero (beginning of buffer) and length defaults to the amount of buffer space remaining. (It is an error if there is more buffer space remaining than length of blob remaining.)

I use a different Python C api which works on 2.3+ including the 3.x family.

[rogerbinns]

From austin.bingham on September 22, 2010 21:10:30

That sounds great. I put this together so I'd have something to experiment against at work, and I figured I'd pass it along; the version you're working on sounds like the better option. How soon can we expect it to be available?

[rogerbinns]

From rogerbinns on September 22, 2010 21:40:26

My usual release schedule is a day or two after the next SQLite release. That varies from every one to three months.

I should have the code completed this weekend and then you'll be able to work with it.

I would like to know more about the segfault you are seeing running the tests. I do use valgrind etc to make sure the code is pristine under all circumstances!

[rogerbinns]

From austin.bingham on September 22, 2010 22:26:55

Here's the output I see from running "python setup.py test":

---

austinb% python setup.py test ~/projects/apsw
running test
Python /usr/bin/python (2, 6, 5, 'final', 0)
Testing with APSW file /usr/local/lib/python2.6/dist-packages/apsw.so
APSW version 3.6.18- r1 SQLite lib version 3.6.22
SQLite headers version 3006022
Using amalgamation True
Not doing LoadExtension test. You need to compile the extension first
gcc -fPIC -shared -o ./testextension.sqlext -I. -Isqlite3 src/testextension.c
...E.........EE...zsh: segmentation fault python setup.py test

---

I'll try to look into it some more, get a stacktrace, etc. when I get a chance.

[rogerbinns]

From austin.bingham on September 22, 2010 22:34:38

As soon as I posted that last comment I realized what the problem is. Brains are funny things sometimes.

The test suite was picking up my system installation of apsw, not the module built by "setup.py build". Once I forced it to load the proper module, things are working fine. Perhaps "setup.py test" ought to ensure that it's looking at the apsw.so in "build/lib.".

[rogerbinns]

From rogerbinns on September 26, 2010 00:33:20

Trying to dig into the directories that distutils makes wouldn't be particularly reliable. If there is an apsw in the same directory as tests.py then that will be picked up. To do so 'python setup.py build_ext --inplace". You can also add "--force" if distutils appears to have outsmarted itself and claimed that no compilation is necessary.

I have committed my blob.readinto implementation. The tests need a few more error conditions added but other than that it should work.

[rogerbinns]

From austin.bingham on October 04, 2010 05:25:03

I finally got a chance to grab your changes and integrate it into my work, and it looks really promising. Some simple initial experiments show readinto performing twice as fast for read sizes above a certain threshold (100K on my machine.) Thanks for the update.

[rogerbinns]

From austin.bingham on October 05, 2010 00:43:55

It looks like there's a small bug in the readinto implementation. You throw a ValueError after comparing offset with bloblen. I think that instead you need to compare offset with bufsize. I've attached a diff with the changes that I think are appropriate.

Attachment: offset_bug.diff

[rogerbinns]

From rogerbinns on October 06, 2010 08:08:43

Thanks for spotting this - fix has checked in.

Status: Verified