Skip to content
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

DynamicActiveQuery::prepare() should only select from current table #23

Open
enigmatix opened this issue Apr 18, 2017 · 1 comment
Open

Comments

@enigmatix
Copy link

enigmatix commented Apr 18, 2017

Hi,

I came across this issue when integrating DynamicActiveRecord into my project. I use scopes to restrict data access for the current user, and these scopes LEFT JOIN a secondary table to ensure that access is permitted.

Unfortunately, if this second table shares column names with the first (eg. id, name), it is these values from the second table that get assigned to the returned model.

The native Yii2 ActiveQuery class handles this by using SELECT tableAlias.* instead of SELECT *

To illustrate another way:

Table 1 - User has one record:

id name
1 joel

Table 2 - Account has one record:

id name details
1 mad max null

Example:

$user = User::findOne(1); echo $user->name; // prints 'mad max', should print 'joel'

The fix would ideally be completed at line 110 in DynamicActiveQuery like so:

    $isDefault = false;

    if (empty($this->select)) {
        list(, $alias) = $this->getTableNameAndAlias();
        $this->select = ["$alias.*"];
        $isDefault = true;

    }

    if (is_array($this->select) && $isDefault) {
        $db = $modelClass::getDb();
        $this->select[$this->_dynamicColumn] =
            'COLUMN_JSON(' . $db->quoteColumnName($this->_dynamicColumn) . ')';
    }

And by copying the getTableNameAndAlias from the native implementation in ActiveQuery

private function getTableNameAndAlias()
{
    if (empty($this->from)) {
        /* @var $modelClass ActiveRecord */
        $modelClass = $this->modelClass;
        $tableName = $modelClass::tableName();
    } else {
        $tableName = '';
        foreach ($this->from as $alias => $tableName) {
            if (is_string($alias)) {
                return [$tableName, $alias];
            } else {
                break;
            }
        }
    }

    if (preg_match('/^(.*?)\s+({{\w+}}|\w+)$/', $tableName, $matches)) {
        $alias = $matches[2];
    } else {
        $alias = $tableName;
    }

    return [$tableName, $alias];
}

If you accept the above and it sounds agreeable, I am happy to prepare a pull request for it.

@vuongminh-xx
Copy link

vuongminh-xx commented Apr 24, 2017

And specific case:

join with 2 tables both have dynamic column with the same name

 $this->select[$this->_dynamicColumn] =
            'COLUMN_JSON(' . $db->quoteColumnName($alias . '.' . $this->_dynamicColumn) . ')';

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

No branches or pull requests

2 participants