Inconsistencies in Builder::addSelect

[liamduckett]

Laravel Version

11.36.1

PHP Version

8.4

Database Driver & Version

No response

Description

Hey, I'd usually try a PR for this sort of thing - I felt it better to ask for some guidance on this one first.

I'm using SQLite, although I don't think this is too relevant here.

$one = User::query()
    ->addSelect(['power' => DB::table('items')
        ->selectRaw('SUM(items.power)')
        ->whereColumn('items.user_id', 'users.id')]
    );

$two = User::query()
    ->addSelect(['power' => DB::raw('select SUM(items.power) from "items" where "items"."user_id" = "users"."id"')]);

I'd expect these two queries to result in the same raw SQL. Yet:

dd(
    $one->toRawSql(),
    $two->toRawSql(),
);

// select "users".*, (select SUM(items.power) from "items" where "items"."user_id" = "users"."id") as "power" from "users"
// select select SUM(items.power) from "items" where "items"."user_id" = "users"."id" from "users"

The first one adds selects all users columns, whilst the second one discards them. I want to say I'd expect the former (as I'd assume addSelect doesn't discard existing selected columns, and without specifying it, all user columns would be selected?).

This happens because of:

// src/Illuminate/Database/Query/Builder.php (424)

/**
 * Add a new select column to the query.
 *
 * @param  array|mixed  $column
 * @return $this
 */
public function addSelect($column)
{
    $columns = is_array($column) ? $column : func_get_args();

    foreach ($columns as $as => $column) {
        // $one matches this condition, but $two doesn't (as it isn't queryable)
        if (is_string($as) && $this->isQueryable($column)) {
            if (is_null($this->columns)) {
                $this->select($this->from.'.*');
            }

            $this->selectSub($column, $as);
        } else {
            if (is_array($this->columns) && in_array($column, $this->columns, true)) {
                continue;
            }

            $this->columns[] = $column;
        }
    }

    return $this;
}

My questions:

1. Is this actually an inconsistency, or am I missing something?
2. If so, is the fix to move the $this->select($this->from.'.*'); out of the isQueryable check? Appreciate this would be a breaking change (in quite a key part of the framework!), and would need to be handled accordingly.

May be worth noting the following cases:

$three = User::query()
    ->addSelect(DB::raw('select SUM(items.power) from "items" where "items"."user_id" = "users"."id"'));

$four = User::query()
    ->selectRaw('select SUM(items.power) from "items" where "items"."user_id" = "users"."id"');

Which result in the same result as $two. Appreciate these may be more involved to change (if desired) - so currently plan to handle them afterwards.

Steps To Reproduce

Repeat of above:

$one = User::query()
    ->addSelect(['power' => DB::table('items')
        ->selectRaw('SUM(items.power)')
        ->whereColumn('items.user_id', 'users.id')]
    );

$two = User::query()
    ->addSelect(['power' => DB::raw('select SUM(items.power) from "items" where "items"."user_id" = "users"."id"')]);

I'd expect these two queries to result in the same raw SQL. Yet:

dd(
    $one->toRawSql(),
    $two->toRawSql(),
);

// select "users".*, (select SUM(items.power) from "items" where "items"."user_id" = "users"."id") as "power" from "users"
// select select SUM(items.power) from "items" where "items"."user_id" = "users"."id" from "users"

[nick-nds]

Hey @liamduckett , I came accross same issue last night and I found out a workaround.

When we are using DB::raw(), it returns Illuminate\Database\Query\Expression which returns false by IsQueryable function in Builder class and thus SELECT * query is not added.
A Closure is considered Queryable by Builder instance, so this works,

$two = User::query()
    ->addSelect([
        'power' => function ($query) {
            $query->selectRaw("SUM(items.power))
                ->from('items')
               ->whereColumn('items.user_id', 'users.id')
              ->limit(1);
            }
        ]);

Now a Closure is passed to addSelect and isQueryable check returns true.