Calling yii\db\Query addSelect($fields) overwrites default select set

[undestroyer]

What steps will reproduce the problem?

$query = ArClass::find();
$query->addSelect('extraField');
$item = $query->one();

What is the expected result?

$item has all default fields + 1 extraField, defined in addSelect()

What do you get instead?

$item has only extraField and does not have default fields.

Additional info

Q	A
Yii version	2.0.13.1
PHP version	7.0.23
Operating system	Ubuntu 16.04.3 x86_64

1. Due to docs, select property means "all by default" yii\db\Query:58

   /**
   * @var array the columns being selected. For example, `['id', 'name']`.
   * This is used to construct the SELECT clause in a SQL statement. If not set, it means selecting all columns.
   * @see select()
   */
   public $select; 

2. Add statement ignores defauls if I did not defined defaults again. yii\db\Query:592

   if ($this->select === null) {
   $this->select = $columns;
   } else {
   $this->select = array_merge($this->select, $columns);
   }

Some use cases:

What I want	How do I do	Is goal Reached?
Select all default columns	ArClass::find()->all() // no call select()	yes
Select only 2 passed columns	ArClass::find()->select(['col1', 'col2'])->all()	yes
Select 2 passed columns +1 more	ArClass::find()->select(['col1', 'col2'])->addSelect('col3')->all()	yes
Select all default columns + 1 more	ArClass::find()->addSelect('col3')->all()	no
Select all default columns + 1 more	ArClass::find()->select('*')->addSelect('col3')->all()	sometimes
Select all default columns + 1 more	ArClass::find()->select(ArClass::tableName() . '.*')->addSelect('col3')->all()	yes

In my opinion, query must select all default columns until I define columns set manually. Add meand adding element to some set. $select defined as all by default, but when you call addSelect() without calling select(), that means default not exists, add this.

See also #12249

[alex-code]

How would this work if you have a custom ActiveQuery that uses addSelect?

class TestAR extends ActiveRecord
{
    public static function find()
    {
        return new TestQuery(get_called_class());
    }
}

class TestQuery extends ActiveQuery
{
    public function test()
    {
        return $this->addSelect('bar');
    }
}

TestAR::find()->select('foo')->test()->all();
// Will SELECT `foo`, `bar`

TestAR::find()->test()->all();
// Will SELECT *, `bar`

[undestroyer]

TestAR::find()->select('foo')->test()->all();
// Will SELECT `foo`, `bar`

TestAR::find()->test()->all();
// Will SELECT *, `bar`

What is the problem here? With find()->select('foo')->test() you define what you need to recieve. With find()->test() you say "give me all default fields and bar". If you want to build select statement without default columns, you should write something like TestAR::find()->unsetDefaultSelect()->....

Why I think it sould be changed: SELECT statement defines set of fields, DB will return you. When you call method nammed addSelect($field), exprected behavior is adding requested $field to SELECT set. Default SELECT set is "all columns". So, TestAR::find()->addSelect($field) means "all and $field".

If you have different opinion - please describe it below.

[alex-code]

$query = TestAR::find();
if ($myCondition) {
    $query->select('foo')
}
$query->test()->all();

If the behavior does change I just think it needs to be well noted. To me with the above example I'd expect SELECT foo, bar or SELECT bar not SELECT *, bar

[MysteryDragon]

Maybe in this case it's better to internally count which fields are selecting. If user selecting full set of base table field, that replace that ones with *. But it seems for me like dangerous logic... because I'm trying to avoid too smart logic, it's always become magical, and then - evil-witchcraft. But in this case maybe it's safe and will not cause negative side effects. Need tests.

[undestroyer]

@MysteryDragon I can't see any opportynity to detect if select set contains all fields in \yii\db\Query because Query doesn't know about table schema.

[alex-code]

This could produce an unexpected result too.

$query = (new Query);

$cols = ['col1', 'col2'];
foreach ($cols as $col) {
  $query->addSelect($col);
}

I'd be expecting

SELECT `col1`, `col2` ...

not

SELECT *, `col1`, `col2` ...

[undestroyer]

We used addSelect($columnn) to build some kind of report. $column was the result of subquery. For example, thats how we get comments to post with likes, using eager loading (eager loading is faster than lazy loading).

-- Create tables and insert some data
CREATE TABLE comment (id INT(11) PRIMARY KEY AUTO_INCREMENT, user_id INT(11) NOT NULL, content VARCHAR(1000));
CREATE TABLE comment_like (id INT(11) PRIMARY KEY AUTO_INCREMENT, comment_id INT(11) NOT NULL, liked_user_id INT(11) NOT NULL, created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP);
INSERT INTO comment (user_id, content) VALUES (1, "first comment"), (2, "last comment"), (1, "another one");
INSERT INTO comment_like (comment_id, liked_user_id, created_at) VALUES (1, 2, "2017-12-25 04:55:43"), (1, 3, "2017-12-26 12:05:32"), (3, 2, "2017-12-26 14:09:13"), (1, 8, "2017-12-27 05:23:12"); 

-- Main query: "get all comments with (likes since Dec, 26)"
SELECT comment.*, COUNT(likes.id) AS `likes`
  FROM comment
  LEFT JOIN comment_like likes ON comment.id = likes.comment_id AND likes.created_at > "2017-12-26 00:00:00"
  GROUP BY comment.id

How we made this:

1. Extend Comment active record to CommentWithLike
2. Defined likes public property
3. Rewrite finders using join and addSelect('likes')

We expected query would look like:

$ar::find()
->addSelect('likes')
->leftJoin('comment_like likes', 'comment.id = likes.comment_id AND likes.created_at > "2017-12-26 00:00:00"')
->groupBy('comment.id')
->all();

We were shure that we do it in the right way. Unfortunately it works differenly. I found explanation in [docs](http://www.yiiframework.com/doc-2.0/yii-db-query.html#addSelect()-detail), but I still have semantic issue here. I expect method names will describe what thay do without secrets (even if it is documented behavior).