-
Couldn't load subscription status.
- Fork 11.6k
[11.x] Introduce method Blueprint::rawColumn()
#53496
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
Conversation
|
I think |
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 can simply be called column, $table->column('name', 'bit').
I also suggest to accept the type argument as string and also Closure. The Closure may have Connection as the first argument:
$table->column('name', function ($connection) {
return $connection->isMaria() ? 'mariadb_type' : 'mysql_type';
})
$table->column('name', function ($connection) {
return version_compare($connection->getServerVersion(), '>=', '3.35')
? 'new_type'
: 'old_type';
})| /** | ||
| * Create a new custom column on the table. | ||
| * | ||
| * @param string $column | ||
| * @param string $statement | ||
| * @return \Illuminate\Database\Schema\ColumnDefinition | ||
| */ | ||
| public function custom($column, $statement) | ||
| { | ||
| return $this->addColumn('custom', $column, compact('statement')); | ||
| } |
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.
The second argument is not a SQL statement, a but SQL type.
| /** | |
| * Create a new custom column on the table. | |
| * | |
| * @param string $column | |
| * @param string $statement | |
| * @return \Illuminate\Database\Schema\ColumnDefinition | |
| */ | |
| public function custom($column, $statement) | |
| { | |
| return $this->addColumn('custom', $column, compact('statement')); | |
| } | |
| /** | |
| * Create a new custom type column on the table. | |
| * | |
| * @param string $column | |
| * @param string|(\Closure(\Illuminate\Database\Connection): string) $type | |
| * @return \Illuminate\Database\Schema\ColumnDefinition | |
| */ | |
| public function column(string $column, string|Closure $type) | |
| { | |
| return $this->addColumn('custom', $column, ['definition' => $type]); | |
| } |
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.
Not sure I feel like the closure has an added value, perhaps Taylor might feel otherwise, still open to that.
Blueprint::custom()Blueprint::rawColumn()
|
Not sure that I'm a fan of using the callback as I feel like it doesn't really have added value. The connection is already accessible via Requested changes have been applied, feel free to re-check. |
This PR introduces a new column method when creating/updating table schema.
Currently, when trying to create columns which are not natively supported by any of the grammars, we have to instead use database statements. This creates a rather big inconvenience as we cannot use a single table creation callback, because database statements are fired immediately and therefore cannot be used in
Schema::create(<table>, <callback>).Imagine a scenario, where we would like to create a:
postsid1namedlegacy_booleanwith a default value of0The example code to solve this would likely be as follows:
This PR introduces a new column creation method which supports directly specifiying the column type definition used for creating a column:
This however has one possible drawback - this is not grammar-agnostic and therefore would be specific to the database driver used, even though, by-default, working with columns is grammar-agnostic as the underlying column definitions are compiled by the grammar implementation used by the driver.
On the other hand, the previous solution using database statements is also not grammar-agnostic and therefore we're just moving the drawback from one place to another, meaning, this should not be an issue after all, as we were already facing it elsewhere.