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

Conversation

hafezdivandari
Copy link
Contributor

@hafezdivandari hafezdivandari commented Jul 14, 2024

Fixes #39

Add support for modifying columns on

  • columnstore tables - requires Laravel > 11.15
  • rowstore tables - requires Laravel > 10.0

Note 1: On Laravel 10.x, You have to call Schema::useNativeSchemaOperationsIfPossible() method within the boot method of your App\Providers\AppServiceProvider class. Not needed on Laravel 11.x

Note 2: I'm not sure how Singlestore handles indexes and foreign keys of the column being modified, couldn't find anything related on the docs.

Comment on lines +42 to +43
if (version_compare(Application::VERSION, '10.0', '<')) {
throw new LogicException('This database driver does not support modifying columns on Laravel < 10.0.');
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)

@AdalbertMemSQL
Copy link
Collaborator

It looks like Schema::getColumnType requires Doctrine\DBAL\Driver\AbstractMySQLDriver for 10.0.0 version of Laravel.
But from 10.30.0 it works well (laravel/framework#48357 )

@hafezdivandari
Copy link
Contributor Author

@AdalbertMemSQL yeah I fixed that function on Laravel 10.30, it was using Doctrine DBAL before that. We can limit that assertion to latest version maybe?

@AdalbertMemSQL
Copy link
Collaborator

Yep, that sounds like a good solution

@AdalbertMemSQL AdalbertMemSQL merged commit 10756db into singlestore-labs:main Jul 24, 2024
19 checks passed
@hafezdivandari hafezdivandari deleted the fix-column-modfiying branch July 24, 2024 07:40
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.

Add support for the "change"-function
2 participants