Skip to content
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

[6.x] Change MySql nullable modifier to allow generated columns to be not null #31452

Merged
merged 2 commits into from
Feb 13, 2020
Merged

[6.x] Change MySql nullable modifier to allow generated columns to be not null #31452

merged 2 commits into from
Feb 13, 2020

Conversation

DrCake
Copy link
Contributor

@DrCake DrCake commented Feb 12, 2020

This PR addresses this issue where it is impossible to make a MySql generated column be not null.
I ran into this issue when trying to run this migration:

class CreatePlacesTable extends Migration
{
    /**
     * Run the migrations.
     *
     * @return void
     */
    public function up()
    {
        Schema::create('places', function (Blueprint $table) {
            $table->bigIncrements('id');

            $table->string('name');
            $table->decimal('latitude', 8, 6);
            $table->decimal('longitude', 9, 6);
            $table->point('location', 4326)->storedAs('ST_SRID(Point(`longitude`, `latitude`), 4326)');
            $table->spatialIndex('location');

            $table->timestamps();
        });
    }

    /**
     * Reverse the migrations.
     *
     * @return void
     */
    public function down()
    {
        Schema::dropIfExists('places');
    }
}

which fails when trying to add the spatial index as it cannot be added to a 'nullable' column.

The change allows using the existing functionality of the nullable() method where you can pass false as the parameter to explictly mark the column as not null. This should hopefully limit the effect this change has on existing migrations by not changing the normal behaviour.

With this change, the migration would work by using this for the 'location' column

$table->point('location', 4326)->storedAs('ST_SRID(Point(`longitude`, `latitude`), 4326)')->nullable(false);

@DrCake DrCake changed the title Change MySql nullable modifier to allow generated columns to be not null [6.x] Change MySql nullable modifier to allow generated columns to be not null Feb 12, 2020
@taylorotwell
Copy link
Member

Instead of this change, isn't the root of the problem the check above?

image

If that is unnecessary or unneeded, why are we not changing that instead?

@DrCake
Copy link
Contributor Author

DrCake commented Feb 13, 2020

@staudenmeir mentioned in this issue #31398 that it's because we need to keep the old functionality to be able to support MariaDB.

@taylorotwell taylorotwell merged commit 1144352 into laravel:6.x Feb 13, 2020
voku added a commit to voku/framework that referenced this pull request Feb 15, 2020
* upstream/master: (78 commits)
  [7.x] Implement anonymous components (laravel#31363)
  Apply fixes from StyleCI (laravel#31480)
  Apply fixes from StyleCI (laravel#31479)
  revert broken table feature
  move files
  add test for event payload of type object (laravel#31477)
  [7.x] Refactor route caching (laravel#31188)
  [6.x] Fixes appendRow on console table (laravel#31469)
  Apply fixes from StyleCI (laravel#31474)
  formatting
  Throw exception on empty collection (laravel#31471)
  Add tests for Query Builder when array value given (laravel#31464)
  Remove addHidden method (laravel#31463)
  allow afterResponse chain
  add getRawOriginal
  Remove unused use-statement
  Update comment
  [6.x] Change MySql nullable modifier to allow generated columns to be not null (laravel#31452)
  [6.x] Test for pushed events (laravel#31451)
  Fixed phpdoc
  ...
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