RFC: proposals for zend-db 3.0.0

zend-db

#1

zend-db is one the oldest and largest libraries in the Zend Framework family. It’s designed as an abstraction for SQL databases, supporting MySQL, PgSQL, PDO, IBM DB2, Oracle, Microsoft SQL Server, and more.

However, The code base is quite old. It was originally developed for PHP 5, and I think it’s time to move it to PHP 7.1 or 7.2. We are working on some PRs and we plan to have a 2.10 release very soon. We would like also to plan a 3.0.0 release. The first idea for 3.0.0 is to use type hints and type declarations with strict_mode enabled. This will simplify implementation, and solve a number of current issues (see #330 as example).

The second idea is to split each database adapter to its own repository, such as zend-db-mysql, zend-db-pdo, etc. This will simplify testing and code maintenance, much as splitting ZF into its component repositories did, or as the planned split of zend-cache will.

The third idea (at the moment) is to merge new features as reported in the 3.0.0 milestone.

Last but not least, we need to improve the unit tests and integration tests. Right now, we have about 49% code coverage, and this is not an acceptable value for a critical library like zend-db. We would like also to improve the integration tests using Docker.

What other feedback or features would you like to see in zend-db 3.0.0?


#2

Great news!

It would be good to see more intuitive transaction handling. I like the way Doctrine does this with callables. Pinching that might look something like:

$adapter->getConnection()->transactional(function($adapter) {
    $stmt = $adapter->getDriver()->createStatement('INSERT INTO foo VALUES (?)');
    $stmt->execute(['bar']);
});

Just a thought!


#3

This is my suggestion.

1. Zend\Db\Adapter\Adapter::query() , make it simpler

Current:

Zend\Db\Adapter\Adapter::query(): Driver\StatementInterface|ResultSet\ResultSet

It bothers me. I don’t know what interface will received.

Expected:

Allways return StatementInterface

Zend\Db\Adapter\Adapter::query(): Driver\StatementInterface

Or split to query(): ResultSetInterface and prepare(): StatementInterface like as PDO.

2. FeatureSet::callMagicCall replace $feature->$method($arguments) to call_user_func_array()

Current:

class CommonCallFeature  extends AbstractFeature {
   public find($args): ArrayObject {}
   public fetchCount($args: int {}
   public fetchPaginator($args): Paginator {}
}

/* @var TableGateway|CommonCallFeature $table  */
$table = new TableGateway(/*...*/);
$table->find();  // I don't know what arguments are there.

Expected:

class CommonCallFeature  extends AbstractFeature {
   public find($id): ArrayObject {}
   public fetchCount($where = null): int {}
   public fetchPaginator($where = null): Paginator {}
}

// I can write annotation for IDE helper.
/* @var TableGateway|CommonCallFeature $table  */
$table = new TableGateway(/*...*/);
$table->find(123);
$table->fetchCount();
$table->fetchPaginator();

To be added


#4

My main pain-points with zend-db are its very squishy/type-unsafe nature. Nonetheless, here are some possible ideas for improving the component significantly:

  • It would be very cool to get rid of the various “resultset prototypes” as a default, leaving it as a niche feature, while having well defined return types for dedicated ->fetch*() operations instead.

  • Performance benchmarks (with PHPBench), and keeping them under surveillance in CI. No ZendFramework repository does this yet, but having a dedicated upstream branch containing committed benchmark reports would be useful to track performance improvements as the project goes on.

  • Get rid of the IBM DB2, as a clear marker that DB2 is technology that no longer fits this decade (yes, if you are still thinking of building new apps based on DB2 in 2018, you should rethink your technology stack a bit)

  • Introduce an SQL92 parser (not kidding), so that developers can write standard SQL instead of relying on the query builder, then adding facilities for transforming said SQL into the platform-specific DSL (this is something I’d also like for doctrine/dbal)

  • Introduce a way to mark secure/insecure endpoints, making it clear to developers as to which APIs are suited for user input, and which are supposed to be only for developer input

I can gladly propose these as new issues, if interesting.


#5

I understand your point, depending on the point of view, to remove support for DB2.

  • It’s hard to write unit/integration tests, since it is not the most common DB and mostly not supported on CI/CD systems attached to GitHub
  • There are minor, but significant, differences between the IBMi and non-IBMi version
  • You have to take the word for it that it works
  • Outside of an IBMi it might not be the best choice

However - there are many shops (thousands [no comparison against Linux installations]) that have no choice but to use DB2, since we are bound to the IBMi and we have to share our DB2 (on IBMi) with fellow RPG and Java developers on these systems.

Forgot to mention…
DB2 on the “I” itself has grown into something very powerful, over the last couple editions.


#6

Integration patterns are surely not the way forward there.


#7

Thanks to everyone for the feedback. I think we can improve zend-db with great features without upsetting the original idea of it, a simple database abstraction layer for PHP applications. The first idea is to update it with PHP 7.2+ using the type-hint features and doing more unit tests for a 100% code coverage. Then we can think to add or change some features, as you suggested.


#9

I see your points but I don’t want to rewrite a new zend-db, I think we have space for improvement without change to much the general goal and behaviour.

Here my replies to your points:

  • Removing the “resulset prototypes” will impact all the components in the library. This is a major change, and I think this will not be beneficial for existing users of zend-db.
  • I don’t think we can remove the support of DB2 since we have a significant number of people using it. I agree on the type-unsafe nature, IMHO this is the first priority for a 3.0.0 release. I’m actually trying to refactor all the code using type-hint with PHP 7.2.
  • I also agree on performance benchmarks, we need to add it in all the zendframework repos, as we did with zend-servicemanager.
  • The idea of a SQL92 parser is intriguing but I think it’s out of scope of zend-db.
  • I didn’t get your last poit, can you be more explicit? Thanks!

#10

My last point was:

  • Introduce a way to mark secure/insecure endpoints, making it clear to developers as to which APIs are suited for user input, and which are supposed to be only for developer input

What I generally see is people interpolating user input within SQL expressions instead of bound parameters.

It would be beneficial to add markers (annotations, maybe) as to where it is unsafe to let user input being passed at call time.

For example, the following is not safe, for obvious reasons:

someQueryBuilder->join(_POST[‘potato’]);


#11

Ok, I see what you mean. We can definitely work on it, feel free to jump in with any POC, thanks!


#12

Since we are on the topic… is there a chance, and I offer myself as a volunteer, to simplify the queries when it comes to Literals

  • option one -> I do not understand it - the separation between strings and numbers is to complicated
  • option two -> it is not as simple as in doctrine

Either way - that one needs a do-over!

In case you need an example, the difference between a string and a numeric value is in the quote sign. zend-db has some issues as far as I am concerned,
Putting codesniffer and stan on top of it makes it only worse.


#14

I’ve started to refactor zend-db with PHP 7.2+ and type hint support in my ezimuel/zend-db/tree/3.0.0 branch.
I also created a 3.0.0.md file to report all the works done.


#15

I would also suggest a change in nomenclature:

Zend\Db\Adapter\Driver\Mysqli\Pdo.php => Zend\Db\Adapter\Driver\Mysqli\Driver.php
Zend\Db\Adapter\Driver\Pdo\Pdo.php => Zend\Db\Adapter\Driver\Pdo\Driver.php
Zend\Db\Adapter\Driver\Pgsql\Pgsql.php => Zend\Db\Adapter\Driver\Pgsql\Driver.php
…etc

thus following the specific drivers’ “Connection”, “Result”, “Statement” classes nomenclature.
The driver name part in the namespace already identifies the nature of the driver.

To avoid BC we could just add an empty “Named” class extending from the specific Driver class.

kind regards


#16

I’d love to contribute. Do you also have a todo-list? As I see PHP 7.2+ is WIP. Would you like PRs on strict_mode and type declarations?


#17

@arueckauer — Yes! Enrico has created a project detailing what needs to be done, and is opening issues with checklists for each item: https://github.com/zendframework/zend-db/projects/1


#18

Thank you @matthew

@enrico I have not yet worked with GitHub projects. It seems, I have only read permissions, there are no assignees or details for the cards (or references to issues). I am not sure how to proceed. I would be interested in refactoring Zend\Db\Metadata for example. Let me know if that would help and what is expected.


#19

About Zend\Db\TableGateway\Feature .

I don’t know why TableGateway\Feature, and what’s different with event-driven .

Is it possable to change all the Feature to event-driven by using zend-eventmanager?

  • MasterSlaveListener
  • MetadataListener
  • GlobalAdapterListener

#20

You do not need extra permission here. If you open an issue or pull request then you can add the link to the related card. But this is not needed in the first step because we can add your issue or pull request to the project also later.

Great! Add your suggestions here for discussion or if you already have code, open a pull request.


Everything that should be implemented later has to be inserted into the project as issue report or pull request. The discussion itself can be done here.


#21

Please have a look at the issue tracker. @enrico has transformed some cards in issue reports.


#22

@enrico I provided some PRs and they are ready for review. Do you want me to mark them in any way?