New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/db story #41 #934

Merged
merged 11 commits into from Apr 2, 2012

Conversation

Projects
None yet
8 participants
@ralphschindler
Member

ralphschindler commented Mar 16, 2012

Changes in this Pull Request

Expression Objects

The expression object was created to abstract the concept of "SQL fragments". This was originally implemented in Zend\Db\Sql\Predicate\Literal, but Literal turns out to be a bad name, and the concept of an "expression" needs to be generic enough to apply to columns as well. For example:

COUNT("column_name") + ? AS "foo"

Using it with Zend\Db\Sql\Select

<?php
$select = new Select;
$select->from('foo')
    ->columns(array(
        'bar' => new Expression('COUNT(some_column)')
    )
);

$select->getSqlString(); // SELECT COUNT(some_column) AS "bar" FROM "foo"

More complex, with values:

<?php
$select7 = new Select;
$select7->from('foo')->columns(
    array(
        new Expression(
            '(COUNT(?) + ?) AS ?',
            array('some_column', 5, 'bar'),
            array(Expression::TYPE_IDENTIFIER, Expression::TYPE_VALUE, Expression::TYPE_IDENTIFIER)
        )
    )
);

/**
 * SQL: SELECT (COUNT("some_column") + ?) AS "bar" FROM "foo"
 * Params: array(5)
 */
$select->prepareStatement(…); 

Other interesting use cases can be found in the SelectTest.php class:

https://github.com/ralphschindler/zf2/blob/feature/db-story-41/tests/Zend/Db/Sql/SelectTest.php#L248-L340

BC ISSUES:

  • Zend\Db\Sql\Predicate\Literal has become Zend\Db\Sql\Predicate\Expression

Verbiage

No longer using "databaseOrSchema", or "databaseSchema". Instead, always using "schema". Regardless of implementation, terminology of the heirarchy is:

schema.table.column

TableGateway using Select objects

TableGateway now has a selectWith(Select $select) public method. The allows one to create their own Select objects and pass them into to this method for producing a result set. The only stipulation is that both the $table and $schema inside the Select object MUST match that inside TableGateway object.

Internal Stuffs

  • Better unit testing, particularly for ZendTest\Db\Adapter\AdapterTest and ZendTest\Db\Sql\SelectTest
  • Created a Zend\Db\Sql\Sql factory object. To be used by TableGateway in the future.
  • Added getPredicates() to PredicateSet
  • Where object is now simply an extension of PredicateSet. In other words, a facade.
  • Cleaned up internal implementations of Sql specifications and joins to use string keys as associative array
  • Fixed PDO Driver Connection to create proper SQLite connection string (was broken by previous patch)

rhunwicks and others added some commits Mar 7, 2012

Fix ZF2-202: PHP Fatal Error in Zend\Db\Adapter\Driver\Pdo\Connection…
…::getDefaultSchema() when using a Postgresql database
Zend\Db Features and Refactoring (story #41)
* Create Expression object + interface
* Refactor Predicates to Predicate Expression
* Formalized specification in Zend\Db\Sql
* cleaned TableGateway API
Zend\Db Features and Refactoring (story #41)
* Created AbstractSql
* cleaned up join internal implementation
Zend\Db Features and Refactoring (story #41)
* Fixed PDO Connection with sqlite
* Added a merge to the ParameterContainer
* Delete, Insert, Update now extends AbstractSql, uses shared implementation
* Created Sql factory, to be used as a dependency in TableGateway
* renamed "database" to "scheme" in a dozen places
* Cleaned up unit tests for refactoring
Zend\Db Features and Refactoring (story #41)
* Added selectWith() to TableGateway
Zend\Db Features and Refactoring (story #41)
* Cleaned up SelectTest, added testdox, split out where() test
* Added getPredicates() to PredicateSet
* Removed dead code in Where
* Fixed multiple predicate support in Select::where()
Merge branch 'master' into feature/db-story-41
Conflicts:
	library/Zend/Db/Sql/Predicate/Literal.php
Zend\Db Features and Refactoring (story #41)
Fixed unit tests, cleaned up TableGateway's __clone() and fixed statement issue in Update SQL.
@akrabat

View changes

Show outdated Hide outdated library/Zend/Db/Sql/Expression.php
@akrabat

View changes

Show outdated Hide outdated library/Zend/Db/Sql/Predicate/Expression.php
@akrabat

View changes

Show outdated Hide outdated library/Zend/Db/Sql/Select.php
@akrabat

View changes

Show outdated Hide outdated library/Zend/Db/Sql/Select.php
@akrabat

View changes

Show outdated Hide outdated library/Zend/Db/Sql/Select.php
@akrabat

View changes

Show outdated Hide outdated library/Zend/Db/Sql/Select.php
$joinSpecArgArray[$j][] = strtoupper($join['type']); // type
$joinSpecArgArray[$j][] = $platform->quoteIdentifier($join['name']); // table
$joinSpecArgArray[$j][] = $platform->quoteIdentifierInFragment($join['on'], array('=', 'AND', 'OR', '(', ')')); // on
foreach ($join['columns'] as /* $jColumnKey => */ $jColumn) {

This comment has been minimized.

@akrabat

akrabat Mar 28, 2012

Member

If we're not going to use $jColumnKey, why have it?

@akrabat

akrabat Mar 28, 2012

Member

If we're not going to use $jColumnKey, why have it?

} else {
$dbSchema = '';
$schema = '';

This comment has been minimized.

@akrabat

akrabat Mar 28, 2012

Member

$schema isn't used in this method. No need for else in order to assign to an empty string.

@akrabat

akrabat Mar 28, 2012

Member

$schema isn't used in this method. No need for else in order to assign to an empty string.

This comment has been minimized.

@ralphschindler

ralphschindler Apr 2, 2012

Member

This is fixed up in a later story/commit (schema is factored out)

@ralphschindler

ralphschindler Apr 2, 2012

Member

This is fixed up in a later story/commit (schema is factored out)

$joinSpecArgArray[$j][] = $platform->quoteIdentifier($join['name']); // table
$joinSpecArgArray[$j][] = $platform->quoteIdentifierInFragment($join['on'], array('=', 'AND', 'OR', '(', ')')); // on
foreach ($join['columns'] as /* $jColumnKey => */ $jColumn) {
$columns[] = $joinSpecArgArray[$j][1] . $separator . $platform->quoteIdentifierInFragment($jColumn);
}
}
}

This comment has been minimized.

@akrabat

akrabat Mar 28, 2012

Member

There's a lot of code here that seems identical to the code in prepareStatement(). Is there any benefit of refactoring out the commonality to a shared method?

@akrabat

akrabat Mar 28, 2012

Member

There's a lot of code here that seems identical to the code in prepareStatement(). Is there any benefit of refactoring out the commonality to a shared method?

@akrabat

View changes

Show outdated Hide outdated library/Zend/Db/Sql/Select.php
@akrabat

View changes

Show outdated Hide outdated library/Zend/Db/Sql/Select.php
@akrabat

View changes

Show outdated Hide outdated library/Zend/Db/Sql/Insert.php
}
$sql .= ' ' . sprintf($this->specifications[self::SPECIFICATION_WHERE], $whereParts['sql']);
}
$statement->setSql($sql);

This comment has been minimized.

@akrabat

akrabat Mar 28, 2012

Member

I'm sure I've seen this block of code at least twice before! Unsure of the policy related to copy/pasting blocks of code around. Can it be moved to AbstractSql?

@akrabat

akrabat Mar 28, 2012

Member

I'm sure I've seen this block of code at least twice before! Unsure of the policy related to copy/pasting blocks of code around. Can it be moved to AbstractSql?

This comment has been minimized.

@ralphschindler

ralphschindler Apr 2, 2012

Member

processException() is already inside the abstract sql. There's really nothing more that needs to be factored out.

@ralphschindler

ralphschindler Apr 2, 2012

Member

processException() is already inside the abstract sql. There's really nothing more that needs to be factored out.

@Freeaqingme

View changes

Show outdated Hide outdated library/Zend/Db/Sql/AbstractSql.php
@Freeaqingme

View changes

Show outdated Hide outdated library/Zend/Db/Sql/ExpressionInterface.php
@Freeaqingme

View changes

Show outdated Hide outdated library/Zend/Db/Sql/Select.php
@@ -166,71 +272,94 @@ public function where($predicate, $combination = Predicate\PredicateSet::OP_AND)
*/
public function prepareStatement(Adapter $adapter, StatementInterface $statement)

This comment has been minimized.

@Freeaqingme

Freeaqingme Mar 30, 2012

Member

Perhaps this method can also be split up into smaller chunks? Smaller methods = lots of benefits! :D

@Freeaqingme

Freeaqingme Mar 30, 2012

Member

Perhaps this method can also be split up into smaller chunks? Smaller methods = lots of benefits! :D

This comment has been minimized.

@ralphschindler

ralphschindler Apr 2, 2012

Member

This will be addressed later after limit and offset have been implemented

@ralphschindler

ralphschindler Apr 2, 2012

Member

This will be addressed later after limit and offset have been implemented

@@ -66,22 +66,22 @@ class TableGateway implements TableGatewayInterface
/**
* @var Select
*/
protected $sqlSelect = null;
protected $sqlSelectPrototype = null;

This comment has been minimized.

@Freeaqingme

Freeaqingme Mar 30, 2012

Member

Ralph,

Can you perhaps be a little more descriptive in the accompanying docblocks here? One would have to what this property represents (at least that's what I'm doing...)

@Freeaqingme

Freeaqingme Mar 30, 2012

Member

Ralph,

Can you perhaps be a little more descriptive in the accompanying docblocks here? One would have to what this property represents (at least that's what I'm doing...)

This comment has been minimized.

@ralphschindler

ralphschindler Apr 2, 2012

Member

This is refactored in a future story/pull request.

@ralphschindler

ralphschindler Apr 2, 2012

Member

This is refactored in a future story/pull request.

ralphschindler added some commits Apr 2, 2012

Added column prefixing in Zend\Db\Sql\Select
Cleaned up various components for coding standards, better commenting.

@EvanDotPro EvanDotPro merged commit 4eae196 into zendframework:master Apr 2, 2012

@mhujer

This comment has been minimized.

Show comment
Hide comment
@mhujer

mhujer Apr 2, 2012

Contributor

It is still not possible to use aliased columns in joins... http://framework.zend.com/issues/browse/ZF2-221

Contributor

mhujer commented on library/Zend/Db/Sql/Select.php in 23a1269 Apr 2, 2012

It is still not possible to use aliased columns in joins... http://framework.zend.com/issues/browse/ZF2-221

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment