1

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

 1 year ago
source link: https://externals.io/message/100773
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.

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

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.

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.

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

Lester Caine4 years ago by Lester Caineview source
unread

Couple of questions:

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

Marco Pivetta4 years ago by Marco Pivettaview source
unread

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/

Matteo Beccati4 years ago by Matteo Beccatiview source
unread

Hey Marco,

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/

Matteo Beccati4 years ago by Matteo Beccatiview source

unread

Hi Lester,

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/

Lester Caine4 years ago by Lester Caineview source
unread

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

Matteo Beccati4 years ago by Matteo Beccatiview source
unread

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/

On Wed, 27 Sep 2017 11:47:21 +0200 / Matteo Beccati [email protected]
said :

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!

On Wed, 27 Sep 2017 09:47:51 +0100 / Dan Ackroyd
[email protected] said :

Couple of questions:

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.

Christoph M. Becker4 years ago by Christoph M. Beckerview source

unread

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

On Wed, 27 Sep 2017 11:41:50 +0200 / "Christoph M. Becker"
[email protected] said :

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.

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.

Lester Caine4 years ago by Lester Caineview source
unread

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

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.

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

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

PDO already has support for large objects (LOBs)[1]. I don't know if

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

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


About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK