morphTo fails on eager loading relation with numeric string key

[saulens22]

• Laravel Version: 8.1.0
• PHP Version: 7.4.3
• Database Driver & Version: Microsoft ODBC Driver 17 for SQL Server + SQLSRV 5.8.1

Description:

Laravel fails to eager load morphTo() relationship when morphed model's $keyType = 'string', but key itself is numeric.

Steps To Reproduce:

From example project https://github.com/saulens22/laravel8mssql/blob/master/tests/Feature/LaravelMssqlTest.php

$thing1 = Thing::firstOrCreate(['thekey' => 'xyz', 'data' => 'Lorem ipsum']);
$comment1 = $thing1->comments()->create([
    'comment' => 'A new comment.',
]);
// works OK
$collection = Comment::with('commentable')->get();

$thing2 = Thing::firstOrCreate(['thekey' => '123', 'data' => 'Lorem ipsum']);
$comment2 = $thing2->comments()->create([
    'comment' => 'Second comment.',
]);
// fails on next line
$collection = Comment::with('commentable')->get();

Error:

Illuminate\Database\QueryException

  SQLSTATE[22018]: [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]Conversion failed when converting the nvarchar value 'xyz' to data type int. (SQL: select * from [things] where [things].[thekey] in (xyz, 123))

  at vendor/laravel/framework/src/Illuminate/Database/Connection.php:671
    667▕         // If an exception occurs when attempting to run a query, we'll format the error
    668▕         // message to include the bindings with SQL, which will make this exception a
    669▕         // lot more helpful to the developer instead of just the database's errors.
    670▕         catch (Exception $e) {
  ➜ 671▕             throw new QueryException(
    672▕                 $query, $this->prepareBindings($bindings), $e
    673▕             );
    674▕         }
    675▕

      +16 vendor frames
  17  tests/Feature/LaravelMssqlTest.php:33
      Illuminate\Database\Eloquent\Builder::get()

[driesvints]

Will need some help here from people using SqlServer.

[saulens22]

My quick look trough query grammar code suggests PHP auto-casting to integer, which SqlServer doesn't like.
SqlServer can convert string to int (where id in ('5', '6') works if id is integer), but fails when converting int to string (as in my example).

Simplest solution would be to wrap all query parameters in quotes, but I'm not sure it's fail-proof.

[taylorotwell]

This probably comes down to the behavior of the bindValues method in the Connection:

Probably not possible for us to fix this at the moment. Maybe not ever to be honest.

[ollieread]

I think this is possibly fixable. The is_int() method specifically checks the type, rather than infers like is_numeric().

1. Model::firstOrCreate() calls Builder::firstOrCreate()
2. Which calls the eloquent Builder::where($attributes)
3. Which internally calls the base Builder::where($attributes).

Illumindate\Database\Eloquent\Builder::where() is as follows.

public function where($column, $operator = null, $value = null, $boolean = 'and')
{
    if ($column instanceof Closure && is_null($operator)) {
        $column($query = $this->model->newQueryWithoutRelationships());

        $this->query->addNestedWhereQuery($query->getQuery(), $boolean);
    } else {
        $this->query->where(...func_get_args());
    }

    return $this;
}

Since a nested query will almost always wrap back around to the final if..

$this->query->where(...func_get_args());

You could probably get away with something like....

$arguments = func_get_args();

if (count($arguments) === 1 && is_array($arguments)) {
	$key = null;

	if (array_key_exists($this->getModel()->getKeyName(), $arguments[0])) {
		$key = $arguments[0][$this->getModel()->getKeyName()];
		unset($arguments[0][$this->getModel()->getKeyName()]);
	} else if (array_key_exists($this->getModel()->getQualifiedKeyName(), $arguments[0])) {
		$key = $arguments[0][$this->getModel()->getQualifiedKeyName()];
		unset($arguments[0][$this->getModel()->getQualifiedKeyName()]);
	}

	if ($key !== null) {
		$this->whereKey($key);

		if (empty($arguments[0])) {
			return $this;
		}
	}
} else {
    if ($arguments[0] === $this->getModel()->getKeyName()) {
        $this->whereKey(array_pop($arguments));
		return $this;
	}
}

$this->query->where(...$arguments);

return $this;

This is just a pure example that I threw together now, it maybe needs tidying up. This would make sure that anywhere clause that included the key is cast correctly. Line 196 of the Builder class will cast to a string.

if ($id !== null && $this->model->getKeyType() === 'string') {
    $id = (string) $id;
}

This also supports the fully qualified name when passing an array, such as from the firstOrCreate.

[taylorotwell]

I think this may be fixed by #34531

[saulens22]

Can confirm fix working on SQL server.