[5.2] Return null instead of 0 for a default BelongsTo key

[ameliaikeda]

This is for an edge-case when using UUIDs (or any type other than integer, really) on postgres.

The scenario for this:

• You have model with a nullable foreign key, say other_id, of type uuid
• You grab a bunch out of the database, and every model's other_id is null
• You then attempt to lazy-load the set (or eager-load if you skip the previous point)

I'm not sure exactly how to unit-test this, as the error actually happens when you attempt to do a query like so, with key as a uuid type, and is in the database layer. Postgres is extremely pedantic about syntax (and will even reject invalid UUIDs).

select * from table where key in (0)

This results in:

Illuminate\Database\QueryException

> SQLSTATE[22P02]: Invalid text representation: 7 ERROR:  invalid input syntax for uuid: "0"

(SQL: select * from "table" where "table"."id" in (0))

I'm not exactly sure if this can be considered backwards-compatible, as it does change the query used by default, but the behaviour should be identical across all supported databases.

The rationale behind using null here is because it passes the initial syntax check for all types in all databases. The 0 approach is a side-effect of assuming primary keys are always integers (which can also be 0).

The only BC break possible would be if someone came to rely on a BelongsTo relationship with a "default" value in their database (with an ID of 0) that would always be returned.

[GrahamCampbell]

This is a breaking change?

[GrahamCampbell]

The only BC break possible would be if someone came to rely on a BelongsTo relationship with a "default" value in their database (with an ID of 0) that would always be returned.

0-indexing is relatively common surely?

[ameliaikeda]

An alternative to avoid a BC-break would be to check for $incrementing first, but that feels hackish.

[ameliaikeda]

Went with the incrementing change. Fixes the exception being thrown for non-integer users, while retaining the [0] return for people using $incrementing = true.

Should I rebase this?

[lucasmichot]

Tests?