[RFC] [Discussion] Implement SQLite "openBlob" feature in PDO

  100773
September 26, 2017 02:03 php@bohwaz.net (BohwaZ/PHP)
Kia ora,

following my patch and discussions on this list, here is the RFC 
requested by some people here to implement "openBlob" in the pdo_sqlite 
driver, to match the "openBlob" method from the SQLite3 extension.

https://wiki.php.net/rfc/implement_sqlite_openblob_in_pdo

Discussion should happen in the next two weeks before going to vote.

The actual patch is here: https://github.com/php/php-src/pull/2698

Suggestions and discussions welcome.

Cheers.
  100777
September 27, 2017 08:10 adambaratz@php.net (Adam Baratz)
> > following my patch and discussions on this list, here is the RFC requested > by some people here to implement "openBlob" in the pdo_sqlite driver, to > match the "openBlob" method from the SQLite3 extension.
Thanks for taking the time to do the writeup. No objections from me. Adam
  100778
September 27, 2017 08:47 danack@basereality.com (Dan Ackroyd)
On 26 September 2017 at 03:03, BohwaZ/PHP <php@bohwaz.net> wrote:


Couple of questions:

> $stream = $pdo->sqliteOpenBlob('test', 'data', 1);
I tried reading the code but failed; what happens when this is called on a PDO connection that isn't to an SQLite database? Also, there should probably be tests around that behaviour.
> Proposed PHP Version(s) > Next PHP 7.x
Please can every RFC be explicit about the version it is aimed at? Although it might be 'obvious' to you, in the future when someone is looking back at declined RFCs trying to match up release dates with RFCs that have 'next' as the version number is confusing. cheers Dan Ack
  100779
September 27, 2017 09:00 lester@lsces.co.uk (Lester Caine)
On 27/09/17 09:47, Dan Ackroyd wrote:
>> https://wiki.php.net/rfc/implement_sqlite_openblob_in_pdo > > Couple of questions: > >> $stream = $pdo->sqliteOpenBlob('test', 'data', 1); > I tried reading the code but failed; what happens when this is called > on a PDO connection that isn't to an SQLite database? Also, there > should probably be tests around that behaviour.
The bigger question is - Should database specific extensions to PDO be allowed at all? The WHOLE base of PDO was that it would allow easy data management between DIFFERENT databases. This should be implemented in a way that mirrors blobs generically otherwise the generic database driver should be used since a switch to another PDO driver will fail. This should apply to any targeted extension to PDO, so anything that breaks the generic base data needs tidying up. -- Lester Caine - G8HFL ----------------------------- Contact - http://lsces.co.uk/wiki/?page=contact L.S.Caine Electronic Services - http://lsces.co.uk EnquirySolve - http://enquirysolve.com/ Model Engineers Digital Workshop - http://medw.co.uk Rainbow Digital Media - http://rainbowdigitalmedia.co.uk
  100780
September 27, 2017 09:04 ocramius@gmail.com (Marco Pivetta)
On Wed, Sep 27, 2017 at 11:00 AM, Lester Caine <lester@lsces.co.uk> wrote:

> On 27/09/17 09:47, Dan Ackroyd wrote: > >> https://wiki.php.net/rfc/implement_sqlite_openblob_in_pdo > > > > Couple of questions: > > > >> $stream = $pdo->sqliteOpenBlob('test', 'data', 1); > > I tried reading the code but failed; what happens when this is called > > on a PDO connection that isn't to an SQLite database? Also, there > > should probably be tests around that behaviour. > > The bigger question is - Should database specific extensions to PDO be > allowed at all? The WHOLE base of PDO was that it would allow easy data > management between DIFFERENT databases. This should be implemented in a > way that mirrors blobs generically otherwise the generic database driver > should be used since a switch to another PDO driver will fail. This > should apply to any targeted extension to PDO, so anything that breaks > the generic base data needs tidying up. >
First time I agree with Lester here, so please take note :-P Unless the type of the connection is PDOSQLiteConnection, this specific patch adds methods that are not interfaced, and need to be checked for existence every time. This is error-prone and just an annoyance that will likely need abstraction once it reaches "real world" (layers that isolate apps from PDO's inherent radioactivity). Marco Pivetta http://twitter.com/Ocramius http://ocramius.github.com/
  100782
September 27, 2017 09:28 php@beccati.com (Matteo Beccati)
Hey Marco,

On 27/09/2017 11:04, Marco Pivetta wrote:
> First time I agree with Lester here, so please take note :-P
Anything you say can and will be used against you ;-)
> Unless the type of the connection is PDOSQLiteConnection, this specific > patch adds methods that are not interfaced, and need to be checked for > existence every time. This is error-prone and just an annoyance that will > likely need abstraction once it reaches "real world" (layers that isolate > apps from PDO's inherent radioactivity).
This is the (possibly wrong) pattern that PDO has been using when providing something that's relevant only for a specific driver. Such methods should be used with caution. What's your suggestion? Cheers -- Matteo Beccati Development & Consulting - http://www.beccati.com/
  100781
September 27, 2017 09:17 php@beccati.com (Matteo Beccati)
Hi Lester,

On 27/09/2017 11:00, Lester Caine wrote:
> The bigger question is - Should database specific extensions to PDO be > allowed at all? The WHOLE base of PDO was that it would allow easy data > management between DIFFERENT databases. This should be implemented in a > way that mirrors blobs generically otherwise the generic database driver > should be used since a switch to another PDO driver will fail. This > should apply to any targeted extension to PDO, so anything that breaks > the generic base data needs tidying up.
That's a wrong assumption. PDO was born to allow quickly writing database drivers, not as an abstraction layer that allows you to seamlessly move from one another. I thought the same but I was corrected by someone that was involved in the process. Cheers -- Matteo Beccati Development & Consulting - http://www.beccati.com/
  100783
September 27, 2017 09:34 lester@lsces.co.uk (Lester Caine)
On 27/09/17 10:17, Matteo Beccati wrote:
> On 27/09/2017 11:00, Lester Caine wrote: >> The bigger question is - Should database specific extensions to PDO be >> allowed at all? The WHOLE base of PDO was that it would allow easy data >> management between DIFFERENT databases. This should be implemented in a >> way that mirrors blobs generically otherwise the generic database driver >> should be used since a switch to another PDO driver will fail. This >> should apply to any targeted extension to PDO, so anything that breaks >> the generic base data needs tidying up.
> That's a wrong assumption. PDO was born to allow quickly writing > database drivers, not as an abstraction layer that allows you to > seamlessly move from one another. I thought the same but I was corrected > by someone that was involved in the process.
The whole base that PDO was allowed to be bundled was that it provided a clean DATA abstraction that could be relied on. The fact that it sidesteps the problems of SQL abstraction was pushed to one side as something that could be handled later. If each driver is now producing DIFFERENT sets of data then the whole generic base is broken. Why not simply move back to the generic drivers which are a LOT more advanced these days and rely on higher level abstraction layers where database transparency is an advantage. openBlob is a specific feature of SQLite so the decision to use it already rules out any other database. IN PDO access to it via the generic blob functions is the proper way forward so that a call for a blob gives a blob what ever the underlying datbase. -- Lester Caine - G8HFL ----------------------------- Contact - http://lsces.co.uk/wiki/?page=contact L.S.Caine Electronic Services - http://lsces.co.uk EnquirySolve - http://enquirysolve.com/ Model Engineers Digital Workshop - http://medw.co.uk Rainbow Digital Media - http://rainbowdigitalmedia.co.uk
  100785
September 27, 2017 09:47 php@beccati.com (Matteo Beccati)
On 27/09/2017 11:34, Lester Caine wrote:
> openBlob is a specific feature of SQLite so the decision to use it > already rules out any other database. IN PDO access to it via the > generic blob functions is the proper way forward so that a call for a > blob gives a blob what ever the underlying datbase.
Seeing the RFC, I gave for granted that SQLite couldn't use the standard LOB api provided by PDO, but maybe that isn't the case? I'll leave the OP to reply. Cheers -- Matteo Beccati Development & Consulting - http://www.beccati.com/
  100806
October 1, 2017 07:41 php@bohwaz.net (BohwaZ)
On Wed, 27 Sep 2017 11:47:21 +0200 / Matteo Beccati <php@beccati.com>
said :

> On 27/09/2017 11:34, Lester Caine wrote: > > openBlob is a specific feature of SQLite so the decision to use it > > already rules out any other database. IN PDO access to it via the > > generic blob functions is the proper way forward so that a call for > > a blob gives a blob what ever the underlying datbase. > > Seeing the RFC, I gave for granted that SQLite couldn't use the > standard LOB api provided by PDO, but maybe that isn't the case? I'll > leave the OP to reply.
I wasn't aware of that API, I saw what the postgreSQL driver was doing and assumed it was the only way. See my response to @cmb but it seems like a nice option, I'll assess next week whether it can fit with the SQLite C API or not. Hopefully it will!
  100804
October 1, 2017 05:46 php@bohwaz.net (BohwaZ)
On Wed, 27 Sep 2017 09:47:51 +0100 / Dan Ackroyd
<danack@basereality.com> said :

> On 26 September 2017 at 03:03, BohwaZ/PHP <php@bohwaz.net> wrote: > > Kia ora, > > > > https://wiki.php.net/rfc/implement_sqlite_openblob_in_pdo > > > Couple of questions: > > > $stream = $pdo->sqliteOpenBlob('test', 'data', 1); > > I tried reading the code but failed; what happens when this is called > on a PDO connection that isn't to an SQLite database? Also, there > should probably be tests around that behaviour.
As with other driver-specific methods of PDO, the method won't be defined and an exception (error) will be raised. I'll add that detail ASAP thanks. This is an existing issue when you want to extend PDO to implement lazy connections (eg. you can't call PDOExtended::sqliteCreateFunction until the parent constructor of PDO has been called), but is out of the scope of this RFC.
  100784
September 27, 2017 09:41 cmbecker69@gmx.de ("Christoph M. Becker")
On 26.09.2017 at 04:03, BohwaZ/PHP wrote:

> following my patch and discussions on this list, here is the RFC > requested by some people here to implement "openBlob" in the pdo_sqlite > driver, to match the "openBlob" method from the SQLite3 extension. > > https://wiki.php.net/rfc/implement_sqlite_openblob_in_pdo > > Discussion should happen in the next two weeks before going to vote. > > The actual patch is here: https://github.com/php/php-src/pull/2698 > > Suggestions and discussions welcome.
PDO already has support for large objects (LOBs)[1]. I don't know if and how these are supported by the pdo_sqlite driver, but wouldn't it make sense to use the existing API instead of introducing a new method? [1] <http://www.php.net/manual/en/pdo.lobs.php> -- Christoph M. Becker
  100805
October 1, 2017 05:55 php@bohwaz.net (BohwaZ)
On Wed, 27 Sep 2017 11:41:50 +0200 / "Christoph M. Becker"
<cmbecker69@gmx.de> said :

> On 26.09.2017 at 04:03, BohwaZ/PHP wrote: > > > following my patch and discussions on this list, here is the RFC > > requested by some people here to implement "openBlob" in the > > pdo_sqlite driver, to match the "openBlob" method from the SQLite3 > > extension. > > > > https://wiki.php.net/rfc/implement_sqlite_openblob_in_pdo > > > > Discussion should happen in the next two weeks before going to vote. > > > > The actual patch is here: https://github.com/php/php-src/pull/2698 > > > > Suggestions and discussions welcome. > > PDO already has support for large objects (LOBs)[1]. I don't know if > and how these are supported by the pdo_sqlite driver, but wouldn't it > make sense to use the existing API instead of introducing a new > method? > > [1] <http://www.php.net/manual/en/pdo.lobs.php> >
Very interesting indeed, didn't know about that feature, I was expecting the creation of a new method was the only way, as this was the way PGSQL was doing it. There's even a bug report about it: https://bugs.php.net/bug.php?id=57691 I will look into that next week and see if it can fit and replace my RFC then. Thank you.
  100807
October 2, 2017 00:48 php@bohwaz.net (BohwaZ/PHP)
>> PDO already has support for large objects (LOBs)[1]. I don't know if >> and how these are supported by the pdo_sqlite driver, but wouldn't it >> make sense to use the existing API instead of introducing a new >> method? >> >> [1] <http://www.php.net/manual/en/pdo.lobs.php> >> > > Very interesting indeed, didn't know about that feature, I was > expecting the creation of a new method was the only way, as this was > the way PGSQL was doing it. > > There's even a bug report about it: > https://bugs.php.net/bug.php?id=57691 > > I will look into that next week and see if it can fit and replace my > RFC > then.
OK, I took some time to look into that feature and the fact is that it doesn't work at all currently with SQLite, it is not returning a resource handle but a string, and it is consuming a large amount of memory as it is just dumping the LOB in memory. The code seems to be there to handle it though so I don't know what's going on, if the person who implemented that could come forward and tell me more about the implementation. But I tried it and it doesn't cover one of the use of openBlob that I have which is to open, read, and write at the same time. The current way it's done in PDO is that you can either fetch a LOB from a result of a query and read from it, or bind a file resource handler to a statement for writing, but you cannot open a LOB, read from it and write from it without performing a query. So for me the use case is quite different here, and openBlob allows stuff that PDO::PARAM_LOB with bindColumn and bindParam cannot allow currently. In conclusion openBlob is still useful as it allows accessing a BLOB outside of a statement and allows to read and write at the same time without having to load the blob in memory.
  100808
October 2, 2017 08:47 lester@lsces.co.uk (Lester Caine)
On 02/10/17 01:48, BohwaZ/PHP wrote:
> So for me the use case is quite different here, and openBlob allows > stuff that PDO::PARAM_LOB with bindColumn and bindParam cannot allow > currently. In conclusion openBlob is still useful as it allows accessing > a BLOB outside of a statement and allows to read and write at the same > time without having to load the blob in memory.
This is where the limitations of some of the other database engines come into play. In many cases in shared hosting, the database is provided on another machine, so one has to transfer the data results between machines and do not have direct access to the data. PDO can't emulate this function so the question is still SHOULD something that can't be made generically functional be allowed in PDO. Personally I would prefer that for this sort of action the generic driver was used used rather than PDO and I have to do that for other functions in other databases currently anyway. So one does not have to overload PDO with more checks as to if your code will work on the different drivers. -- Lester Caine - G8HFL ----------------------------- Contact - http://lsces.co.uk/wiki/?page=contact L.S.Caine Electronic Services - http://lsces.co.uk EnquirySolve - http://enquirysolve.com/ Model Engineers Digital Workshop - http://medw.co.uk Rainbow Digital Media - http://rainbowdigitalmedia.co.uk
  100810
October 2, 2017 21:48 php@bohwaz.net (BohwaZ/PHP)
> On 02/10/17 01:48, BohwaZ/PHP wrote: >> So for me the use case is quite different here, and openBlob allows >> stuff that PDO::PARAM_LOB with bindColumn and bindParam cannot allow >> currently. In conclusion openBlob is still useful as it allows >> accessing >> a BLOB outside of a statement and allows to read and write at the same >> time without having to load the blob in memory. > > This is where the limitations of some of the other database engines > come > into play. In many cases in shared hosting, the database is provided on > another machine, so one has to transfer the data results between > machines and do not have direct access to the data. PDO can't emulate > this function so the question is still SHOULD something that can't be > made generically functional be allowed in PDO. Personally I would > prefer > that for this sort of action the generic driver was used used rather > than PDO and I have to do that for other functions in other databases > currently anyway. So one does not have to overload PDO with more checks > as to if your code will work on the different drivers.
I don't agree with that. You might want to be able to use a generic abstraction layer such as PDO to offer support for multiple database engines without having to create your own abstraction layer with every specific database extension (that would be huge work) but still be able to access driver-specific features if available. This is why I am pushing for PDO to be feature-full, so that you have the choice to use PDO and not have to implement your own abstraction layer just because you need one specific feature in one single case :) If you follow your logic, then PDO::sqliteCreateFunction shouldn't exist, and this would make the PDO sqlite driver pretty much useless as SQLite is missing a number of important functions.
  100813
October 3, 2017 13:59 danack@basereality.com (Dan Ackroyd)
On 2 October 2017 at 22:48, BohwaZ/PHP <php@bohwaz.net> wrote:
> > If you follow your logic, then PDO::sqliteCreateFunction shouldn't exist, > and this would make the PDO sqlite driver pretty much useless as SQLite is > missing a number of important functions.
That's taking it to the illogical conclusion. Taking it to a better solution is that the method sqliteCreateFunction shouldn't exist on the PDO class, but instead on a PDOSqlite that extends PDO. class PDOSqlite extends PDO { public function createFunction(...) {...} } class PDO { public static function connect(string $dsn [, string $username [, string $password [, array $options ]]]) { // if connecting to SQLite DB { return new PDOSqlite(...); } } } This might be a mistake in how it was implemented originally. Looking back it seems that it was implemented before we had the RFC process....and is exactly the type of 'sub-optimal' implementation the RFC process is meant to prevent.
> This is why I am pushing for PDO to be feature-full, so that you have the > choice to use PDO and not have to implement your own abstraction layer just > because you need one specific feature in one single case :) >
That's a great aim! But rather thank just hack in new features building on sub-optimal ways of doing things, I think it would be better to create a plan to clean up the way that connection specific methods are available, with something along the lines of that which I mentioned above. cheers Dan Ack
  100819
October 3, 2017 22:41 php@bohwaz.net (BohwaZ/PHP)
> Taking it to a better solution is that the method sqliteCreateFunction > shouldn't exist on the PDO class, but instead on a PDOSqlite that > extends PDO. > > class PDOSqlite extends PDO { > public function createFunction(...) {...} > } > > class PDO { > public static function connect(string $dsn [, string $username [, > string $password [, array $options ]]]) { > // if connecting to SQLite DB { > return new PDOSqlite(...); > } > } > } > > This might be a mistake in how it was implemented originally. Looking > back it seems that it was implemented before we had the RFC > process....and is exactly the type of 'sub-optimal' implementation the > RFC process is meant to prevent.
Yes I do agree that the method overloading of PDO by drivers is not the best to say the least. If you feel like rewriting a large part of PDO feel free :) but I don't have time for that, and it's not the subject of this RFC :)
  100814
October 3, 2017 14:22 adambaratz@php.net (Adam Baratz)
> > PDO already has support for large objects (LOBs)[1]. I don't know if >>> and how these are supported by the pdo_sqlite driver, but wouldn't it >>> make sense to use the existing API instead of introducing a new >>> method? >>> >>> [1] <http://www.php.net/manual/en/pdo.lobs.php> >>> >>> >> Very interesting indeed, didn't know about that feature, I was >> expecting the creation of a new method was the only way, as this was >> the way PGSQL was doing it. >> >> There's even a bug report about it: >> https://bugs.php.net/bug.php?id=57691 >> >> I will look into that next week and see if it can fit and replace my RFC >> then. >> > > > OK, I took some time to look into that feature and the fact is that it > doesn't work at all currently with SQLite, it is not returning a resource > handle but a string, and it is consuming a large amount of memory as it is > just dumping the LOB in memory. The code seems to be there to handle it > though so I don't know what's going on, if the person who implemented that > could come forward and tell me more about the implementation.
I believe that's how PDO::PARAM_LOB is intended to work (based on my reading of the docs and implementations for other drivers). It seems like more of a convenience than anything, though maybe someone had more ideas for how it should work across drivers and never got to follow through on it. The RFC is agreeable to me because it follows the existing ext/sqlite3 API and uses the existing pattern in pdo_sqlite for integrating driver-specific APIs. I'd love it if PDO had better BLOB/LOB types and if we had a better pattern for driver-specific APIs, but I'm comfortable lumping those goals under "future scope." Getting parity with ext/sqlite3 will make pdo_sqlite more usable, which will help grow its community, and the number of people who are able to contribute to these bigger projects. Deprecating the current set of driver-specific APIs in the future, if we have functional equivalents, isn't an impossible project. Thanks, Adam
  100820
October 3, 2017 22:49 php@bohwaz.net (BohwaZ/PHP)
> I believe that's how PDO::PARAM_LOB is intended to work (based on my > reading of the docs and implementations for other drivers). It seems > like > more of a convenience than anything, though maybe someone had more > ideas > for how it should work across drivers and never got to follow through > on it.
This is not how I understand the documentation: "PDO::PARAM_LOB tells PDO to map the data as a stream, so that you can manipulate it using the PHP Streams API." But this seems to be quite chaotic, reading https://secure.php.net/manual/en/pdostatement.bindcolumn.php "Since information about the columns is not always available to PDO until the statement is executed, portable applications should call this function after PDOStatement::execute(). However, to be able to bind a LOB column as a stream when using the PgSQL driver, applications should call this method before calling PDOStatement::execute(), otherwise the large object OID will be returned as an integer." This is quite confusing. And as stated above, with MySQL and SQLite it returns the LOB content as a string on PHP 7+ but a stream handle on PHP 5.6… To me it seems that the LOB handling of PDO via bindColumn/bindParam is completely broken and inconsistent currently :( If I have more time available after this RFC I'll look into fixing it for PHP 7.3.
> I'd love it if PDO had better BLOB/LOB types and if we had a better > pattern for driver-specific APIs, but I'm comfortable lumping those > goals > under "future scope." Getting parity with ext/sqlite3 will make > pdo_sqlite > more usable, which will help grow its community, and the number of > people > who are able to contribute to these bigger projects. Deprecating the > current set of driver-specific APIs in the future, if we have > functional > equivalents, isn't an impossible project.
Yeah sounds good to me :)