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

getTableAlias() in defaultScope() broken in 1.1.11 #1072

Closed
MonkeyMaster opened this issue Jul 30, 2012 · 9 comments
Closed

getTableAlias() in defaultScope() broken in 1.1.11 #1072

MonkeyMaster opened this issue Jul 30, 2012 · 9 comments
Assignees
Labels

Comments

@MonkeyMaster
Copy link

This is related to #1043
I use $this->getTableAlias(false, false) in model's defaultScope() to avoid ambiguous column errors. It was working fine till YII 1.1.11. Now I get "Unknown column in on clause" sql errors due to wrong alias in defaultScope(). Example:

<?php
class AcCraft extends ActiveRecord
{
    public function defaultScope() {
        $alias = $this->getTableAlias(false, false);
...

$crafts = AcCraft::model()->sorted()->with( ... )->findAll();
...
$arrivals = AcGroundActivity::model()->with(array('craft_r', ...))->findAll();

The first find statement creates AcCraft's dbCriteria with default tableAlias t.
The second find statement ignores relation's alias "craft_r" and still uses "t"!

I found temporary solution: removing model's dbCriteria between these two statements forces second statement to re-create model's dbCriteria with right alias:

<?php
AcCraft::model()->dbCriteria = null;

Anyway this looks like a bug to me

@resurtm
Copy link
Contributor

resurtm commented Jul 30, 2012

Another possible temporary workaround:

<?php

public function defaultScope()
{
  $ta=$this->getTableAlias(false,false);
  return array(
    'alias'=>$ta,
    // ... additional criteria stuff here
  );
}

@ghost ghost assigned samdark Jul 30, 2012
@samdark
Copy link
Member

samdark commented Jul 30, 2012

@MonkeyMaster can you provide code for models involved? Thanks.

@creocoder
Copy link
Contributor

@MonkeyMaster and check bug with latest code from GitHub.

@MonkeyMaster
Copy link
Author

@samdark
I will try to make a simple example for this issue

@creocoder
The latest code from GitHub fixes this issue. Also sql queries slightly differ - no default scopes in on clauses. Maybe it's even better this way since joins use primary keys and conditions from default scopes are useless.

1.1.10:
LEFT OUTER JOIN ac_craft craft_r ON (t.craft=craft_r.id) AND ((craft_r.stn=:ycp13) AND (craft_r.company=:ycp14))

latest code:
LEFT OUTER JOIN ac_craft craft_r ON (t.craft=craft_r.id)

@creocoder
Copy link
Contributor

@MonkeyMaster Please provide full SQL query and AcCraft::defaultScope() dump. Need to ensure that all ok there.

@MonkeyMaster
Copy link
Author

@samdark
Here's simple application for testing (is it possible to attach zip-file here?..)

<?php
class Airline extends CActiveRecord
{
    public static function model($className=__CLASS__)
    {
        return parent::model($className);
    }
    public function tableName()
    {
        return 'test_airline';
    }
}

class Craft extends CActiveRecord
{
    public static function model($className = __CLASS__)
    {
        return parent::model($className);
    }
    public function tableName()
    {
        return 'test_craft';
    }
    public function relations()
    {
        return array(
            'airline_r' => array(self::BELONGS_TO, 'Airline', 'airline'),
        );
    }
    public function defaultScope()
    {
        return array(
            'condition' => $this->dbConnection->quoteColumnName($this->getTableAlias(false, false).'.company').'=2',
        );
    }
}

class GroundActivity extends CActiveRecord
{
    public static function model($className = __CLASS__)
    {
        return parent::model($className);
    }
    public function tableName()
    {
        return 'test_groundactivity';
    }
    public function relations()
    {
        return array(
            'craft_r' => array(self::BELONGS_TO, 'Craft', 'craft'),
        );
    }
}

class TestController extends Controller
{
    public function actionIndex()
    {
        Craft::model()->with('airline_r')->findAll();
        GroundActivity::model()->with('craft_r')->findAll();
    }
}
CREATE TABLE `test_airline` (
  `id` int(11) NOT NULL AUTO_INCREMENT,
  PRIMARY KEY (`id`)
);
CREATE TABLE `test_craft` (
  `id` int(11) NOT NULL AUTO_INCREMENT,
  `company` int(11) DEFAULT NULL,
  `airline` int(11) DEFAULT NULL,
  PRIMARY KEY (`id`)
);
CREATE TABLE `test_groundactivity` (
  `id` int(11) NOT NULL AUTO_INCREMENT,
  `craft` int(11) DEFAULT NULL,
  PRIMARY KEY (`id`)
);

1.1.10

SELECT `t`.`id` AS `t0_c0`, `t`.`craft` AS `t0_c1`, `craft_r`.`id` AS `t1_c0`, `craft_r`.`company` AS `t1_c1`, `craft_r`.`airline` AS `t1_c2`
FROM `test_groundactivity` `t`
LEFT OUTER JOIN `test_craft` `craft_r` ON (`t`.`craft`=`craft_r`.`id`) AND (`craft_r`.`company`=2) 

1.1.11 (Column not found: 1054 Unknown column 't.company' in 'on clause'):

SELECT `t`.`id` AS `t0_c0`, `t`.`craft` AS `t0_c1`, `craft_r`.`id` AS `t1_c0`, `craft_r`.`company` AS `t1_c1`, `craft_r`.`airline` AS `t1_c2`
FROM `test_groundactivity` `t`
LEFT OUTER JOIN `test_craft` `craft_r` ON (`t`.`craft`=`craft_r`.`id`) AND (`t`.`company`=2) 

Latest code from GitHub:

SELECT `t`.`id` AS `t0_c0`, `t`.`craft` AS `t0_c1`, `craft_r`.`id` AS `t1_c0`, `craft_r`.`company` AS `t1_c1`, `craft_r`.`airline` AS `t1_c2`
FROM `test_groundactivity` `t`
LEFT OUTER JOIN `test_craft` `craft_r` ON (`t`.`craft`=`craft_r`.`id`)

@samdark samdark closed this as completed Jul 31, 2012
@samdark
Copy link
Member

samdark commented Jul 31, 2012

Since everything is fine now, closing issue.

@creocoder, @MonkeyMaster thanks.

@mdomba
Copy link
Member

mdomba commented Jul 31, 2012

@samdark isn't something missing here?

If the default scope is limiting the test_craft relation for only those with the company = 2, shouldn't that be in the SQL like it was before ?

@MonkeyMaster
Copy link
Author

Sorry, I copied just CActiveFinder.php when testing the latest build and missed changes in CActiveRecord.php.
So actually queries are the same as it was in 1.1.10

@resurtm resurtm mentioned this issue Aug 2, 2012
fernandezekiel added a commit to fernandezekiel/yii-1 that referenced this issue Sep 29, 2014
this is to address this issue: yiisoft#1072 
what do you guys think?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants