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

Fix column modifying on columnstore tables #88

Merged
8 changes: 4 additions & 4 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@
],
"require": {
"php": "^7.3|^8.0",
"illuminate/container": "^8.0|^9.0|^10.0|^11.0",
"illuminate/database": "^8.0|^9.0|^10.0|^11.0",
"illuminate/events": "^8.0|^9.0|^10.0|^11.0",
"illuminate/support": "^8.0|^9.0|^10.0|^11.0"
"illuminate/container": "^8.0|^9.0|^10.0|^11.15",
"illuminate/database": "^8.0|^9.0|^10.0|^11.15",
"illuminate/events": "^8.0|^9.0|^10.0|^11.15",
"illuminate/support": "^8.0|^9.0|^10.0|^11.15"
},
"require-dev": {
"mockery/mockery": "^1.5",
Expand Down
57 changes: 57 additions & 0 deletions src/Schema/Grammar.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@
use Illuminate\Database\Connection;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Database\Schema\Grammars\MySqlGrammar;
use Illuminate\Foundation\Application;
use Illuminate\Support\Fluent;
use Illuminate\Support\Str;
use InvalidArgumentException;
use LogicException;
use SingleStore\Laravel\Schema\Blueprint as SingleStoreBlueprint;
use SingleStore\Laravel\Schema\Grammar\CompilesKeys;
use SingleStore\Laravel\Schema\Grammar\ModifiesColumns;
Expand All @@ -25,6 +27,61 @@ public function __construct()
$this->addSingleStoreModifiers();
}

/**
* Compile a change column command into a series of SQL statements.
*
* @param \Illuminate\Database\Schema\Blueprint $blueprint
* @param \Illuminate\Support\Fluent $command
* @param \Illuminate\Database\Connection $connection
* @return array|string
*
* @throws \RuntimeException
*/
public function compileChange(Blueprint $blueprint, Fluent $command, Connection $connection)
{
if (version_compare(Application::VERSION, '10.0', '<')) {
throw new LogicException('This database driver does not support modifying columns on Laravel < 10.0.');
Comment on lines +42 to +43
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason, why this is not supported for older versions of Laravel?

Copy link
Contributor Author

@hafezdivandari hafezdivandari Jul 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Older Laravel versions don't support native column modifying, you may check:

Laravel was using doctrine/dbal before this version and I'm quit sure that it is going to be a problem for your package as Doctrine DBAL doesn't support Singlestore. But you may add tests to see if it fallbacks to MySQL correctly or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the latest commit 9777d8a I removed this and added test to see if it works with lower Laravel version + Doctrine DBAL

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK the tests shows that we can't use doctrine:

SQLSTATE[HY000]: General error: 1706 Feature 'ALTER TABLE CHANGE to modify column type' is not supported by SingleStore. Use MODIFY to change a column's type. (Connection: mysql, SQL: ALTER TABLE test CHANGE data data TEXT CHARACTER SET utf8mb4 DEFAULT NULL COLLATE utf8mb4_unicode_ci)

}

$isColumnstoreTable = $connection->scalar(sprintf(
"select exists (select 1 from information_schema.tables where table_schema = %s and table_name = %s and storage_type = 'COLUMNSTORE') as is_columnstore",
$this->quoteString($connection->getDatabaseName()),
$this->quoteString($blueprint->getPrefix().$blueprint->getTable())
));

if (! $isColumnstoreTable) {
return parent::compileChange($blueprint, $command, $connection);
}

if (version_compare(Application::VERSION, '11.15', '<')) {
throw new LogicException('This database driver does not support modifying columns of a columnstore table on Laravel < 11.15.');
}

$tempCommand = clone $command;
$tempCommand->column = clone $command->column;
$tempCommand->column->change = false;
$tempCommand->column->name = '__temp__'.$command->column->name;
$tempCommand->column->after = is_null($command->column->after) && is_null($command->column->first)
? $command->column->name
: $command->column->after;

return [
$this->compileAdd($blueprint, $tempCommand),
sprintf('update %s set %s = %s',
$this->wrapTable($blueprint),
$this->wrap($tempCommand->column),
$this->wrap($command->column)
),
$this->compileDropColumn($blueprint, new Fluent([
'columns' => [$command->column->name],
])),
$this->compileRenameColumn($blueprint, new Fluent([
'from' => $tempCommand->column->name,
'to' => $command->column->name,
]), $connection),
];
}

/**
* Compile a primary key command.
*
Expand Down
123 changes: 123 additions & 0 deletions tests/Hybrid/ChangeColumnTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
<?php

namespace SingleStore\Laravel\Tests\Hybrid;

use Illuminate\Foundation\Application;
use Illuminate\Support\Facades\Schema;
use SingleStore\Laravel\Schema\Blueprint;
use SingleStore\Laravel\Tests\BaseTest;

class ChangeColumnTest extends BaseTest
{
use \SingleStore\Laravel\Tests\Hybrid\HybridTestHelpers;

protected function setUp(): void
{
parent::setUp();

if ($this->runHybridIntegrations()) {
$this->createTable(function (Blueprint $table) {
$table->id();
});
}
}

protected function tearDown(): void
{
if ($this->runHybridIntegrations()) {
Schema::dropIfExists('test_renamed');
}

parent::tearDown();
}

/** @test */
public function change_column_on_rowstore_table()
{
if (version_compare(Application::VERSION, '10.0', '<')) {
$this->markTestSkipped('requires higher laravel version');
}

if ($this->runHybridIntegrations()) {
$cached = $this->mockDatabaseConnection;

$this->mockDatabaseConnection = false;

$this->createTable(function (Blueprint $table) {
$table->rowstore();
$table->id();
$table->string('data');
});

Schema::table('test', function (Blueprint $table) {
$table->text('data')->nullable()->change();
});

$this->assertEquals(['id', 'data'], Schema::getColumnListing('test'));
$this->assertEquals('text', Schema::getColumnType('test', 'data'));

$this->mockDatabaseConnection = $cached;
}

$blueprint = new Blueprint('test');
$blueprint->text('data')->nullable()->change();

$connection = $this->getConnection();
$connection->shouldReceive('getDatabaseName')->andReturn('database');
$connection->shouldReceive('scalar')
->with("select exists (select 1 from information_schema.tables where table_schema = 'database' and table_name = 'test' and storage_type = 'COLUMNSTORE') as is_columnstore")
->andReturn(0);

$statements = $blueprint->toSql($connection, $this->getGrammar());

$this->assertCount(1, $statements);
$this->assertEquals('alter table `test` modify `data` text null', $statements[0]);
}

/** @test */
public function change_column_of_columnstore_table()
{
if (version_compare(Application::VERSION, '11.15', '<')) {
$this->markTestSkipped('requires higher laravel version');
}

if ($this->runHybridIntegrations()) {
$cached = $this->mockDatabaseConnection;

$this->mockDatabaseConnection = false;

$this->createTable(function (Blueprint $table) {
$table->id();
$table->string('data');
});

Schema::table('test', function (Blueprint $table) {
$table->text('data')->nullable()->change();
});

$this->assertEquals(['id', 'data'], Schema::getColumnListing('test'));
$this->assertEquals('text', Schema::getColumnType('test', 'data'));

$this->mockDatabaseConnection = $cached;
}

$blueprint = new Blueprint('test');
$blueprint->text('data')->nullable()->change();

$connection = $this->getConnection();
$connection->shouldReceive('getDatabaseName')->andReturn('database');
$connection->shouldReceive('scalar')
->with("select exists (select 1 from information_schema.tables where table_schema = 'database' and table_name = 'test' and storage_type = 'COLUMNSTORE') as is_columnstore")
->andReturn(1);

$statements = $blueprint->toSql($connection, $this->getGrammar());

$this->assertCount(4, $statements);
$this->assertEquals([
'alter table `test` add `__temp__data` text null after `data`',
'update `test` set `__temp__data` = `data`',
'alter table `test` drop `data`',
'alter table `test` change `__temp__data` `data`',
], $statements);
}
}
Loading