Update SqlWalker.php #615

Closed
wants to merge 3 commits into
from

Projects

None yet
@mikemeier
Contributor

Always be sure that only a-z characters are used for table alias, otherwise use generic "t" for "table"

@mikemeier mikemeier Update SqlWalker.php
Always be sure that only a-z characters are used for table alias, otherwise use generic "t" for "table"
bd06f4d

Hello,

thank you for positing this Pull Request. I have automatically opened an issue on our Jira Bug Tracker for you with the details of this Pull-Request. See the Link:

http://doctrine-project.org/jira/browse/DDC-2352

Owner

@mikemeier please add a test case to avoid regressions

@Ocramius Ocramius commented on an outdated diff Mar 15, 2013
lib/Doctrine/ORM/Query/SqlWalker.php
@@ -284,7 +284,11 @@ public function getSQLTableAlias($tableName, $dqlAlias = '')
$tableName .= ($dqlAlias) ? '@[' . $dqlAlias . ']' : '';
if ( ! isset($this->tableAliasMap[$tableName])) {
- $this->tableAliasMap[$tableName] = strtolower(substr($tableName, 0, 1)) . $this->tableAliasCounter++ . '_';
+ $tablePrefixAlias = strtolower(substr($tableName, 0, 1));
+ if( ! preg_match("/[a-z]/", $tablePrefixAlias)) {
Ocramius
Ocramius Mar 15, 2013 Owner

missing newline before this line

@Ocramius Ocramius commented on an outdated diff Mar 15, 2013
lib/Doctrine/ORM/Query/SqlWalker.php
@@ -284,7 +284,11 @@ public function getSQLTableAlias($tableName, $dqlAlias = '')
$tableName .= ($dqlAlias) ? '@[' . $dqlAlias . ']' : '';
if ( ! isset($this->tableAliasMap[$tableName])) {
- $this->tableAliasMap[$tableName] = strtolower(substr($tableName, 0, 1)) . $this->tableAliasCounter++ . '_';
+ $tablePrefixAlias = strtolower(substr($tableName, 0, 1));
+ if( ! preg_match("/[a-z]/", $tablePrefixAlias)) {
+ $tablePrefixAlias = 't';
+ }
+ $this->tableAliasMap[$tableName] = $tablePrefixAlias . $this->tableAliasCounter++ . '_';
Ocramius
Ocramius Mar 15, 2013 Owner

Missing newline before this line

@Majkl578 Majkl578 commented on an outdated diff Mar 15, 2013
lib/Doctrine/ORM/Query/SqlWalker.php
@@ -284,7 +284,11 @@ public function getSQLTableAlias($tableName, $dqlAlias = '')
$tableName .= ($dqlAlias) ? '@[' . $dqlAlias . ']' : '';
if ( ! isset($this->tableAliasMap[$tableName])) {
- $this->tableAliasMap[$tableName] = strtolower(substr($tableName, 0, 1)) . $this->tableAliasCounter++ . '_';
+ $tablePrefixAlias = strtolower(substr($tableName, 0, 1));
+ if( ! preg_match("/[a-z]/", $tablePrefixAlias)) {
+ $tablePrefixAlias = 't';
+ }
Majkl578
Majkl578 Mar 15, 2013 Contributor

I'd use just:

$tablePrefixAlias = preg_match('~[a-z]~i', $tableName[0]) ? strtolower($tableName[0]) : 't'; 
Member
stof commented Mar 15, 2013

Please add a test for this

openin commented Mar 22, 2013

👍

Owner
beberlei commented Apr 6, 2013

What is the problem here? I understand what it does, but why should the aliases be non alphanumeric?

Because in postgresql database if you use old database with caps letter you have to add quote arround your table name and doctrine will take the quote instead of the first real letter. And if i use quote for alias of my table of course it will be brocken !

Gladhon commented Jul 10, 2013

+1

Contributor

any news on this pr?

Owner

@beberlei I think the best solution here would be to use iconv and ASCII//TRANSLIT (iconv is enabled by default), being able to use same char as it was done before.

Contributor

@beberlei merge or close?

Owner
beberlei commented Dec 9, 2013

@mikemeier Sorry, i am swamped with projects at the moment, i have a bunch of days allocated at the end of the year to go through the issues.

Contributor
mvrhov commented Dec 9, 2013

@guilhermeblanco ASCII//TRANSLIT is locale dependent. IMO nobody sets the locale inside the php application. Which means that in most cases calling iconv with above parameter won't do a thing.

Owner

@mvrhov It doesn't matter which locale you are in if you use ASCII combined with IGNORE.
I tried several locales (even removing the setting of it). Here's the response I get on all cases on a simple example:

echo iconv("UTF-8", "ASCII//IGNORE", "çáéíóúàèìòùäëïöü€Žťčýůňßtable");
// prints: table
nsaleh commented Dec 24, 2013

The patch is working with my postgres setup using escaped table names like """schema"".""table""". As without this patch sqls using this strategy are messed up i think it's a pretty important fix.

thanks!

Contributor

What's the current status of this issue?

Still active @mikemeier ?

Contributor
@Ocramius Ocramius added a commit that referenced this pull request Oct 19, 2014
@Ocramius Ocramius #615 - assignment alignment a4e9c23
@Ocramius Ocramius added a commit that referenced this pull request Oct 19, 2014
@Ocramius Ocramius #615 - assignment alignment d557a0e
@Ocramius Ocramius added a commit that referenced this pull request Oct 19, 2014
@Ocramius Ocramius #615 - EOF EOL fixes 435befd
@Ocramius Ocramius added a commit that referenced this pull request Oct 19, 2014
@Ocramius Ocramius #615 - Test coverage annotations 17c9388
@Ocramius Ocramius added a commit that referenced this pull request Oct 19, 2014
@Ocramius Ocramius #615 - Removing unused assignment b989175
@Ocramius Ocramius closed this in a9bd51c Oct 19, 2014
@Ocramius Ocramius self-assigned this Oct 19, 2014
Owner

Manually reviewed/rebased/cleaned/merged: thanks!

Owner

@guilhermeblanco @beberlei the regex match is good enough here, FYI.

Contributor

Thank you @Ocramius

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