Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Update framework/db/ar/CActiveRecord.php #2048

Closed
wants to merge 4 commits into from

3 participants

@s-larionov

Если в ActiveRecord переопределить метод getTableAlias, то будут возникать ошибки неизвестной таблицы t, а так он будет использовать текущий алиас у таблицы

@s-larionov s-larionov Update framework/db/ar/CActiveRecord.php
Если в ActiveRecord переопределить метод getTableAlias, то будут возникать ошибки неизвестной таблицы t, а так он будет использовать текущий алиас у таблицы
8afb3b5
@samdark samdark was assigned
@cebe cebe commented on the diff
framework/db/ar/CActiveRecord.php
@@ -1294,7 +1294,7 @@ protected function query($criteria,$all=false)
{
if(!$all)
$criteria->limit=1;
- $command=$this->getCommandBuilder()->createFindCommand($this->getTableSchema(),$criteria);
+ $command=$this->getCommandBuilder()->createFindCommand($this->getTableSchema(),$criteria,$this->getTableAlias(false));
@cebe Owner
cebe added a note

$this->getTableAlias() is enough as false is default for 1st param.
Issue is valid and fix is fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@samdark samdark closed this pull request from a commit
@samdark samdark Fixes #2048: AR now uses alias from CActiveRecord::getTableAlias inst…
…ead of always using default "t"
356c002
@samdark samdark closed this in 356c002
@samdark
Owner

Thanks for the fix.

@cebe
Owner

Whats the actual problem being solved here? This change breaks the logic in createFindCommand(). Issue reported in #2884. Haven't seen the problem before but when getTableAlias() is called the criteria has already been reset and we get the wrong alias. Will revert this change.

@samdark
Owner

@cebe the original issue was getting unknown table t error in case one overrides ActiveRecord::getTableAlias.

@cebe cebe referenced this pull request from a commit in cebe/yii
@cebe cebe reverted #2048 as it breaks logic in createFindCommand()
fixes #2884
9356b42
@cebe
Owner

@samdark how got getTableAlias() overridden? There is definitively something wrong in the overriding implementation if it fails.

@cebe
Owner

Calling getTableAlias() after $this->applyScopes() will give wrong results when alias is set by default scope.
We have to make sure the code works with the default implementation of getTableAlias() before thinking about what happens when it gets overridden. Can you or @s-larionov show an example of how to override getTableAlias() ?

@samdark
Owner

I guess it's just

public function getTableAlias()
{
  return 'my_cool_alias';
}
@cebe
Owner

Well, this is definitively not correct as it breaks every code that tries to set alias via criteria.
One should use setTableAlias() or default scope to set a fix alias like this.

@cebe cebe referenced this pull request from a commit
@cebe cebe Merge PR #2885 branch 'revert-2048' of https://github.com/cebe/yii
* 'revert-2048' of https://github.com/cebe/yii:
  added changelog line for #2884
  added unit test for #2884
  fixed wrong dependency in unit test
  reverted #2048 as it breaks logic in createFindCommand()

Conflicts:
	CHANGELOG
1273438
@twiesenthal twiesenthal referenced this pull request from a commit in twiesenthal/yii
@cebe cebe reverted #2048 as it breaks logic in createFindCommand()
fixes #2884
f7cd2c5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 28, 2013
  1. @s-larionov

    Update framework/db/ar/CActiveRecord.php

    s-larionov authored
    Если в ActiveRecord переопределить метод getTableAlias, то будут возникать ошибки неизвестной таблицы t, а так он будет использовать текущий алиас у таблицы
Commits on Jan 31, 2013
  1. @s-larionov

    Update framework/db/ar/CActiveRecord.php

    s-larionov authored
    method save() always returns true, even if the record does not exist in the database
Commits on Feb 4, 2013
  1. @s-larionov
Commits on Feb 5, 2013
  1. @s-larionov
This page is out of date. Refresh to see the latest.
Showing with 2 additions and 2 deletions.
  1. +2 −2 framework/db/ar/CActiveRecord.php
View
4 framework/db/ar/CActiveRecord.php
@@ -1294,7 +1294,7 @@ protected function query($criteria,$all=false)
{
if(!$all)
$criteria->limit=1;
- $command=$this->getCommandBuilder()->createFindCommand($this->getTableSchema(),$criteria);
+ $command=$this->getCommandBuilder()->createFindCommand($this->getTableSchema(),$criteria,$this->getTableAlias(false));
@cebe Owner
cebe added a note

$this->getTableAlias() is enough as false is default for 1st param.
Issue is valid and fix is fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
return $all ? $this->populateRecords($command->queryAll(), true, $criteria->index) : $this->populateRecord($command->queryRow());
}
else
@@ -1611,7 +1611,7 @@ public function exists($condition='',$params=array())
$this->applyScopes($criteria);
if(empty($criteria->with))
- return $builder->createFindCommand($table,$criteria)->queryRow()!==false;
+ return $builder->createFindCommand($table,$criteria,$this->getTableAlias(false, false))->queryRow()!==false;
else
{
$criteria->select='*';
Something went wrong with that request. Please try again.