Welcome! Log In Create A New Profile

Advanced

[PHP-DEV] [RFC] [Discussion] Implement SQLite "openBlob" feature in PDO

Posted by 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.

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
>
> 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
On 26 September 2017 at 03:03, BohwaZ/PHP <[email protected]> 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.

> 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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
On Wed, Sep 27, 2017 at 11:00 AM, Lester Caine <[email protected]> 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/
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/

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
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/

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
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/

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
On Wed, 27 Sep 2017 09:47:51 +0100 / Dan Ackroyd
<[email protected]> said :

> On 26 September 2017 at 03:03, BohwaZ/PHP <[email protected]> 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.

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
On Wed, 27 Sep 2017 11:41:50 +0200 / "Christoph M. Becker"
<[email protected]> 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.

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
On Wed, 27 Sep 2017 11:47:21 +0200 / Matteo Beccati <[email protected]>
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!

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.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.

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.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.

--
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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.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.

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
On 2 October 2017 at 22:48, BohwaZ/PHP <[email protected]> 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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.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.


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
> 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 :)


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.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 :)

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Sorry, only registered users may post in this forum.

Click here to login