Skip to content
This repository was archived by the owner on Jan 29, 2020. It is now read-only.

TableIdentifier schema parameter to allow arrays for use in quoteIdentifierChain to express full object path. #232

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

alextech
Copy link
Contributor

@alextech alextech commented Mar 11, 2017

#206 Allow expression of full path towards table by allowing array as parameter for $schema in TableIdentifier and use quoteIdentifierChain in Sql table resolver. Reasoning explained in the issue.

While working on this noticed that quoteIdentifierChain tries to preform near identical operation as identifierChain, hence the change in the AbstractSQL class, but in less safer way. The result is the same most of the time but it uses hardcoded strings, instead of tokens specified in the specification that quoteIdentifier is using, such as quoteIdentiferTo or quoteIdentifer[0] and getIdentifierSeparator().

I think this is an oversight since databases with more complex database object locator rules (PostgreSQL and SQL Server) are only now becoming more popular. Cleaning that up possibly in another PR.

…wing array as parameter for $schema in TableIdentifier
@@ -3,7 +3,7 @@
* Zend Framework (http://framework.zend.com/)
*
* @link http://github.com/zendframework/zf2 for the canonical source repository
* @copyright Copyright (c) 2005-2016 Zend Technologies USA Inc. (http://www.zend.com)
* @copyright Copyright (c) 2005-2017 Zend Technologies USA Inc. (http://www.zend.com)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -435,7 +435,7 @@ protected function resolveTable(
}

if ($schema && $table) {
$table = $platform->quoteIdentifier($schema) . $platform->getIdentifierSeparator() . $table;
$table = $platform->quoteIdentifierChain($schema).$platform->getIdentifierSeparator() . $table;
Copy link
Member

@froschdesign froschdesign Mar 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spaces are missing before and after the dot.

@@ -1,9 +1,11 @@
<?php

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*/
public function __construct($table, $schema = null)
{
if (! (is_string($table) || is_callable([$table, '__toString']))) {
if (!(is_string($table) || is_callable([$table, '__toString']))) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Space is missing after !.

@@ -45,14 +47,17 @@ public function __construct($table, $schema = null)
if (null === $schema) {
$this->schema = null;
} else {
if (! (is_string($schema) || is_callable([$schema, '__toString']))) {
if (!(is_string($schema) || is_array($schema) || is_callable([$schema, '__toString']))) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Space is missing after !.

is_object($schema) ? get_class($schema) : gettype($schema)
));
}

$this->schema = (string) $schema;
if (!is_array($schema)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Space is missing after !.

@@ -1,19 +1,23 @@
<?php

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above.


/**
* Tests for {@see \Zend\Db\Sql\TableIdentifier}
* Tests for {@see \Zend\Db\Sql\TableIdentifier}.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert

/**
* Data provider
* Data provider.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert

@@ -102,7 +116,7 @@ public function invalidTableProvider()
}

/**
* Data provider
* Data provider.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert

@froschdesign
Copy link
Member

@alextech
I think you should deactivate your automated CS corrections.

is_object($schema) ? get_class($schema) : gettype($schema)
));
}

$this->schema = (string) $schema;
if (! is_array($schema)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably check for callable [$schema, '__toString'] only to avoid re-cast $schema that already string ?

if (is_callable([$schema, '__toString'])) {

@alextech alextech mentioned this pull request Sep 26, 2017
@froschdesign froschdesign modified the milestones: 2.9.0, 2.10.0 Dec 7, 2017
@weierophinney
Copy link
Member

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
Copy link
Member

This repository has been closed and moved to laminas/laminas-db; a new issue has been opened at laminas/laminas-db#78.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants