-
Notifications
You must be signed in to change notification settings - Fork 11.6k
Add configurable default id type for schema builder #56748
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 12.x
Are you sure you want to change the base?
Add configurable default id type for schema builder #56748
Conversation
…remental key or uuid [9.x] Added idFor method Build migration schema id based on using incremental key or uuid
Signed-off-by: Mior Muhammad Zaki <[email protected]>
Signed-off-by: Mior Muhammad Zaki <[email protected]>
* fetchargs * add fetchArgs() function * fetchargs * add fetchArgs() function * add afterquery callbacks * fix typo * flip afterquerycallback * fix expectations * add expectations * add expectations * add expectations * add raw expression test * rename to fetchUsing * style * Remove test from PR laravel#54396 * Revert most Query\Builder changes * remove DB::raw test * Revert QueryBuilder pluck tests * Add base tests for $query->fetchUsing() * style * formatting * Update Builder.php * Fix filled columns before calling pluck * query db compatibility --------- Co-authored-by: Taylor Otwell <[email protected]>
Signed-off-by: Mior Muhammad Zaki <[email protected]>
Signed-off-by: Mior Muhammad Zaki <[email protected]>
* Add tests * Remove any scopes removed in a nested where * Update tests to expect calls to `removedScopes`
* Remove function existence checks * remove unused JsonException import --------- Co-authored-by: Xurshudyan <[email protected]>
Signed-off-by: Mior Muhammad Zaki <[email protected]>
Signed-off-by: Mior Muhammad Zaki <[email protected]>
Signed-off-by: Mior Muhammad Zaki <[email protected]>
Signed-off-by: Mior Muhammad Zaki <[email protected]>
* fetchargs * add fetchArgs() function * fetchargs * add fetchArgs() function * add afterquery callbacks * fix typo * flip afterquerycallback * fix expectations * add expectations * add expectations * add expectations * add raw expression test * rename to fetchUsing * style * Remove test from PR laravel#54396 * Revert most Query\Builder changes * remove DB::raw test * Revert QueryBuilder pluck tests * Add base tests for $query->fetchUsing() * style * formatting * Update Builder.php * Fix filled columns before calling pluck * query db compatibility * remove unrelated test --------- Co-authored-by: Taylor Otwell <[email protected]>
…el#55508) * Allow Listeners to dynamically specify deleteWhenMissingModels * Update BroadcastEvent.php * Update CallQueuedListener.php --------- Co-authored-by: Taylor Otwell <[email protected]>
Signed-off-by: Mior Muhammad Zaki <[email protected]>
…ravel#55685) * Add failing test * Only mark a model class as booted after the boot process has been completed * Mark a model class as booting as soon as the boot process starts - A model class is removed from the `booting` array as soon as the boot process has completed. * Throw an exception if `bootIfNotBooted` is called while booting - This infers that something in the boot process expected the boot to already be finished. - If we just ignore the call then the model class would be in an inconsistent state for whichever code called it. * formatting * formatting --------- Co-authored-by: Taylor Otwell <[email protected]>
Signed-off-by: Mior Muhammad Zaki <[email protected]>
It still works, because of php case sensitive rules, but it's better correct.
* feat: add support in grammar * feat: add functions in builder * test: add tests * test: adjust syntax mysql * test: adjust syntax mysql * test: remove line noused * test: execute compileJoins * formatting --------- Co-authored-by: Taylor Otwell <[email protected]>
… a domain (laravel#55921) * wip * wip * wip * linting
Signed-off-by: Mior Muhammad Zaki <[email protected]>
* [13.x] Supports Symfony 7.4 Signed-off-by: Mior Muhammad Zaki <[email protected]> * Update `Application` to use `addCommand` insteads of deprecated `add` Signed-off-by: Mior Muhammad Zaki <[email protected]> * wip Signed-off-by: Mior Muhammad Zaki <[email protected]> * Remove `addToParent()` method. Signed-off-by: Mior Muhammad Zaki <[email protected]> * Revert "Remove `addToParent()` method." This reverts commit 7cf2aa7. * wip Signed-off-by: Mior Muhammad Zaki <[email protected]> * Update Application.php --------- Signed-off-by: Mior Muhammad Zaki <[email protected]>
Signed-off-by: Mior Muhammad Zaki <[email protected]>
* Change to hyphenate prefixes * Use `Str::snake()` as intended
* remove superfluous element we don't need both the `<div>` and the `<p>` elements, as their purpose can be handled by a single `<div>`. while minor, having less DOM is always better for performance. * do the same for the BS5 view
Adds `eventStream` to `Illuminate\Contracts\Routing\ResponseFactory` in order to add type support for this method, aligning this with the same behavior that we have for `stream` of which `eventStream` is a convenient wrapper.
…#55954) * :feat: add touch for cache ttl extension * :test: fix failing memoized store test * :clean: remove inadvertent inclusions * :test: remove unnecessary imports in tests * formatting * formatting * fix fake * adjust wording --------- Co-authored-by: Yitz Willroth <[email protected]> Co-authored-by: Taylor Otwell <[email protected]>
This adds a new Builder::$defaultIdType property (similar to $defaultMorphKeyType), allowing developers to globally configure the default column type used by $table->id(). Supported values: bigIncrements, uuid, ulid. - Defaults to bigIncrements (backward compatible). - Keeps migration files cleaner for teams using UUID or ULID as their primary keys. - Provides consistency with existing $defaultMorphKeyType configuration.
Thanks for submitting a PR! Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface. Pull requests that are abandoned in draft may be closed due to inactivity. |
Are you sure, you didn't accidentally merge the wrong branch into yours? |
/** | ||
* The default id key type. | ||
* | ||
* @var string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice if you could narrow this further down
* @var string | |
* @var 'int'|'uuid'|'ulid' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe, this could be handled via an Enum
* | ||
* @var string | ||
*/ | ||
public static $defaultIdType = 'int'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even though, it's done similarly with the default morph column type and the mapping to big integer mostly makes sense, what if, for some reason, someone can not or doesn't want to use the non-big integer type?
return $model->getKeyType() === 'int' && $model->getIncrementing() | ||
? $this->id($column ?: $model->getKeyName()) | ||
: $this->uuid($column ?: $model->getKeyName()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why no ULID?
if (Builder::$defaultIdType === 'uuid') { | ||
return $this->uuid($column)->primary(); | ||
} elseif (Builder::$defaultIdType === 'ulid') { | ||
return $this->ulid($column)->primary(); | ||
} | ||
return $this->bigIncrements($column); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could add some error handling here
if (Builder::$defaultIdType === 'uuid') { | |
return $this->uuid($column)->primary(); | |
} elseif (Builder::$defaultIdType === 'ulid') { | |
return $this->ulid($column)->primary(); | |
} | |
return $this->bigIncrements($column); | |
return match (Builder::$defaultIdType) { | |
'uuid' => $this->uuid($column)->primary(), | |
'ulid' => $this->ulid($column)->primary(), | |
'id' => $this->bigIncrements($column), | |
default => throw new InvalidArgumentException("Key type must be 'int', 'uuid', or 'ulid'."), | |
}; |
return $this->uuid($column)->primary(); | ||
} elseif (Builder::$defaultIdType === 'ulid') { | ||
return $this->ulid($column)->primary(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your indention is off here
return $this->uuid($column)->primary(); | |
} elseif (Builder::$defaultIdType === 'ulid') { | |
return $this->ulid($column)->primary(); | |
return $this->uuid($column)->primary(); | |
} elseif (Builder::$defaultIdType === 'ulid') { | |
return $this->ulid($column)->primary(); |
Need to fix merge conflict. |
Won't this break third party migrations? |
Description
This PR introduces a new static property on the schema Builder to configure the default column type used by $table->id().
Currently, $table->id() always produces a bigIncrements column. This is fine for many applications, but more and more projects default to UUID or ULID for their primary keys. Today, the only way to achieve that is to either:
Changes in this PR
When changed, $table->id() will respect this global default.
Example
Now, any migration that calls:
Will produce:
Motivation
Backward Compatibility
Community Tip
If you want all of your migrations to default to ULID or UUID primary keys, you can configure this once in your AppServiceProvider
Now, every $table->id() call in your migrations will automatically create the correct column type without needing to repeat $table->uuid('id')->primary() or $table->ulid('id')->primary() everywhere.