Skip to content

Conversation

@LukaSkrlj
Copy link

ForeignIdFor issue

Hi, when changing Models $keyType to string, foreignIdFor() method does not work as expected.

Example/Reproduction

Models

class Basket extends Model
{
    protected $keyType = 'string';
    public $incrementing = false;
}

class Customer extends Model
{} 

Migrations

public function up(): void
    {
        Schema::create('customers', function (Blueprint $table) {
            $table->id();
            $table->timestamps();
            $table->foreignIdFor(Basket::class);
        });
    }

public function up(): void
    {
        Schema::create('baskets', function (Blueprint $table) {
            $table->string('id')->primary();
            $table->timestamps();
        });
    }

Tinker

$customer = new App\Models\Customer()
= App\Models\Customer {#5202}

> $basket = new App\Models\Basket()
= App\Models\Basket {#5211}

> $basket->id = "1"
= "1"

> $customer->basket_id = $basket->id
= "1"

> $basket->save()
= true

> $customer->save()

   Illuminate\Database\QueryException  SQLSTATE[22P02]: Invalid text representation: 7 ERROR:  invalid input syntax for type uuid: "1"
CONTEXT:  unnamed portal parameter $1 = '...' (Connection: pgsql, SQL: insert into "customers" ("basket_id", "updated_at", "created_at") values (1, 2025-04-18 12:26:56, 2025-04-18 12:26:56) returning "id").

Env setup

Setup laravel app with sail and choose pgsql. I tested this issue only for pgsql and mysql (I did not test maria, sqlite, etc.). Mysql works fine but pgsql does not work because it expects UUID (see the code) and not a custom string.

(Braking) changes

Model will have to use HasUuids; if it uses UUID as its primary key when specifying a foreign id using foreignIdFor() method

foreignString method - I am not sure if it should be extracted or the code inside the function should be embedded into foreignIdFor method.

Current workaround

when building migrations use foreign()

$table->string('basket_id');
$table->foreign('basket_id')->references('id')->on('baskets');

I had to also reorder (rename timestamps) of migration files so that previously created migration does not depend on future migrations. When using foreignIdFor, the order of running migrations does not matter.

Other possible workaround is using mysql (since it works there) but DB migration is complicated.

Future work

If this or similar change gets accepted I will update Laravel docs. There is the same problem with morph relations, i could not use morph method and instead had to specify custom keys in migrations.
Feel free to contribute to PR and ask any questions, I will open an issue for this bug/feature if needed.

@taylorotwell
Copy link
Member

It's not meant to be used with strings. It's the counter-part to the id method.

@LukaSkrlj
Copy link
Author

Hi Taylor, love your work and thank you for the fast response. In small/medium sized projects (my cases) there is no point for using UUID or ULID in the project. Each product_id (Ex. cable_id, cable_type_id etc.) is guaranteed to be a unique string defined by the client and my client does not want/need UUID/ULID/INT's as primary keys. So instead of defining id(as UUID or ULID or INT) and product_id(string), I can use only product_id(string) as a primary key. Also, I do not have to fetch the whole entity from the database when displaying its id as a foreign key of another model which is making things simpler.

If anyone encounters this issue and is wondering why does foreignIdFor with custom string as an id work with mysql but not with postgre, it is because postgre has UUID column type but mysql does not so foreignUuid method stores the key as VARCHAR in mysql (other DBMS may also cause problems). In my opinion, this is inconsistent behavior which could trouble someone who is using foreignIdFor method with string primary keys (Ex. people who are new to Laravel or migrating to other DBMS), but maybe that is just a rare use case so keep the PR closed and I will continue using the "workaround". Sorry for bothering and keep up the good work :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants