new Sql($adapter) should propagate with sql statement

[samsonasik]

re-create of https://github.com/zendframework/zendframework/pull/6767

[mwillbanks]

@samsonasik unit tests should be leveraging the mocks in this case, can you please take a look and update the unit tests?

[samsonasik]

@mwillbanks do you means I need to mock the Adapter ? The tests that was added already using real Adapter.

[samsonasik]

@mwillbanks do you means I need to mock the Adapter ? The tests that was added already using real Adapter.

[turrsis]

what I need to do if I need execute same sql on different db? create new sql statement?

[samsonasik]

I guess that should not be worked to be re-used query, if there is a case like that, we may do something like this:

$firstAdapter = new Adapter([...]);
$secondAdapter = new Adapter([...]);

function repeatSql(Sql $sql)
{
    // create new update query
    $update = $sql->update('table');
    $update->where(array('id'=>1));
    $update->set(array('foo'=>'bar'));

    return $update;
}

$sql = new Sql($firstAdapter);
echo repeatSql($sql)->getSqlString();

$sql = new Sql($secondAdapter);
echo repeatSql($sql)->getSqlString();

[alextech]

That looks so bad. An SQL statement builder that depends on adapter, even though the whole point is not worry about vendor specific, and then have to recreate whole object just because one dependency changed? Makes user feel as if strategy pattern or similar is used on surface, but misleads you and forces to recreate it. Constructor should maybe take adapter as optional parameter and then have a setter for it. That way,

$sql = new Sql();

$adapter1 = new Adapter();
$adapter2 = new Adapter();

$sql->setAdapter($adapter1);
echo $sql->getSqlString();

$sql->setAdapter($adapter2);
echo $sql->getSqlString();

And if forgotten to specify adapter, throw exception.

[samsonasik]

there is no setAdapter() in Sql class even before it. To do that, we may need to add Zend\Db\Adapter\AdapterAwareTrait into Sql class ?

[alextech]

Yes, I meant as an API improvement. But preferably without aware trait, since SM 3 worked on not using those anymore. Or maybe this is an acceptable exception.

[alextech]

But then @turrsis already has PR which separates this annoyance into SQL as abstract declarative object, and builder which parses it based on adapter (hope thats a decent description). Maybe can sit this out until that gets reviewed and pulled in as a proper solution.

[michalbundyra]

This repository has been moved to laminas/laminas-db. If you feel that this patch is still relevant, please re-open against that repository, and reference this issue. To re-open, we suggest the following workflow:

• Squash all commits in your branch (git rebase -i origin/{branch})
• Make a note of all changed files (`git diff --name-only origin/{branch}...HEAD
• Run the laminas/laminas-migration tool on the code.
• Clone laminas/laminas-db to another directory.
• Copy the files from the second bullet point to the clone of laminas/laminas-db.
• In your clone of laminas/laminas-db, commit the files, push to your fork, and open the new PR. We will be providing tooling via laminas/laminas-migration soon to help automate the process.

[michalbundyra]

This repository has been closed and moved to laminas/laminas-db; a new issue has been opened at https://github.com/laminas/laminas-db/issues/105.