-
Notifications
You must be signed in to change notification settings - Fork 10.8k
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
[12.x] feat: configure default datetime precision on per-grammar basis #51821
base: master
Are you sure you want to change the base?
Conversation
Just want your 👀 on this one if you have a sec @hafezdivandari. |
We have framework/src/Illuminate/Database/Schema/Builder.php Lines 37 to 42 in ae63a5b
I suggest to add public function dateTime($column, $precision = null)
{
$precision ??= Builder::$defaultTimePrecision;
return $this->addColumn('dateTime', $column, compact('precision'));
} No need to change anything on |
This would mean that the precision is no longer configurable on a per grammar basis (just like the I didn't like adding the logic in the I've created a fixup commit (which can be squshed later) so you can preview the changes. If we don't like it then I can pop that commit off |
f604439
to
2dc1cd1
Compare
Why per grammer and not per schema builder? You can do |
I originally put it on the grammar right underneath the
That changes the default value for all the schema Builders, not just for Postgres: |
This feels like a lot of changes for such a simple change 🙃 |
Yeah 🫤, it was more involved than I would like. Most of this is the fallout from injecting the Connection/Grammar via the Blueprint constructor instead of the I can split the refactor (injecting the Connection/Grammar into the Blueprint constructor) into a separate PR if you like, or I can update if you have other ideas on a better way to do this (being able to configure the default precision on a per-grammar basis). |
As I said on this comment, we already have properties with similar functionality on the
You may define class Builder
{
public static $defaultTimePrecision = 0;
public static function defaultTimePrecision(?int $precision)
{
static::$defaultTimePrecision = $precision;
}
}
class PostgresBuilder extends Builder
{
public static $defaultTimePrecision = 0;
} Then use it like this: PostgresBuilder::$defaultTimePrecision; // 0
MySqlBuilder::$defaultTimePrecision; // 0
PostgresBuilder::defaultTimePrecision(6);
PostgresBuilder::$defaultTimePrecision; // 6
MySqlBuilder::$defaultTimePrecision; // 0 Using connection: $connection->getSchemaBuilder()::$defaultTimePrecision; |
Ah okay, but we'd still need the connection injected into the Blueprint constructor so we have access to it when the time methods execute |
Yes, and it may be considered as a useful change IMO - Having direct access to connection instance on |
991aae2
to
7fe952c
Compare
7fe952c
to
e5101b0
Compare
@hafezdivandari, I've moved the precision to the builder |
Hello!
Please see my previous PR (#51793) for context. This PR adds a
getDatetimePrecision
method to the base grammar which allows overriding the default precision on a per grammar basis. The Blueprint datetime methods now default to anull
precision (instead of0
) which defers to the precision configured on the grammar.Note that there has been no change in behavior---so this could possibly be merged into 11.x (just let me know).
Thanks!