Enable MySQL Driver #168

hanhan1978 · 2016-10-02T14:53:34Z

Change mysql setting [strict] false => true
=> avoid migration error for creatd_at default value(0)

Change migration files for MySQL

remove query builder method [after]
=> caused mysql error (may be fixed in Laravel 5.1~)
change token column data type from text to varchar(512)
=> cannot set unique index for blob data type in MySQL

Add setting in .env.example
=> default sqlite

Sync .env setting int .env.behat
=> add database config

Add MySQL specific repository for fulltext search
=> MySQL/ItemFtsRepository
=> MySQL/TagFtsRepository

sota1235 · 2016-10-03T01:07:36Z

Related #38

sota1235 · 2016-10-03T01:08:48Z

app/Http/Controllers/SearchController.php

-            $res = $this->searchService->itemMatchCount($q);
-            $pagination = new Paginator($results, $res[0]->count, $this->perPage, null, array('path' => '/search'));
+            $count = $this->searchService->itemMatchCount($q);
+            $pagination = new Paginator($results, $count, $this->perPage, null, array('path' => '/search'));


sota1235 · 2016-10-03T01:10:28Z

app/Repositories/Fluent/ItemFtsRepository.php

@@ -127,6 +127,7 @@ public function matchCount($str)
            WHERE
              fts.words MATCH :match
 __SQL__;
-        return \DB::select(\DB::raw($query), array( 'match' => FtsUtils::createMatchWord($str)));
+        $res = \DB::select(\DB::raw($query), array( 'match' => FtsUtils::createMatchWord($str)));
+        return $res[0]->count;


お手数ですがphpdocの@returnの方も修正していただけますか…！

sota1235 · 2016-10-03T01:37:18Z

database/migrations/2014_10_04_182525_create_item_tag.php

            $table->integer('tag_id')->unsigned();
-            $table->foreign('tag_id')->references('id')->on('tags');
+            $table->foreign('tag_id')->references('id')->on('tags')->onDelete('cascade');


対応ダン！

既存マイグレーションを修正してしまうと既にこのマイグレーションを実行済みの場合に変更を適用できなくなってしまうので、新しいマイグレーションファイルで modify columnしてもらいたいです...

参考
http://stackoverflow.com/questions/26820788/add-on-delete-cascade-to-existing-column-in-laravel

fortkle · 2016-10-08T18:21:16Z

app/Repositories/Fluent/MySQL/TagFtsRepository.php

+     *
+     * @param int $tag_id
+     * @param string $words
+     * @return stdClass


arrayにしましょ！そして申し訳ないんですが、 app/Repositories/Fluent/TagFtsRepository.php の同じ箇所も arrayに変えていただけると... 🙏

fortkle · 2016-10-08T18:26:22Z

database/migrations/2015_03_01_010748_create_login_tokens_table.php

                ->references('id')->on('users')
                ->onDelete('cascade')->onUpdate('cascade');
-            $table->text('token')->unique();
+            $table->string('token', 512)->unique();


mysql都合の変更なので、 if (env('DB_DRIVER') === 'mysql') していただけると..

（unsignedをsqlite側に適用するならここと同じ理由で別マイグレーションファイルにしたいです...。別ファイルが面倒だと思うのでmysql側だけunsignedでもいいかなと思います）

fortkle · 2016-10-08T18:26:40Z

database/migrations/2015_10_19_010501_create_reminder_tokens_table.php

@@ -17,7 +17,7 @@ public function up()
            $table->integer('user_id')
                ->references('id')->on('users')
                ->onDelete('cascade')->onUpdate('cascade');
-   
10000
         $table->text('token')->unique();
+            $table->string('token', 512)->unique();


https://github.com/owl/owl/pull/168/files#r82507378 と同様で！

fortkle · 2016-10-08T18:27:30Z

database/migrations/2015_02_27_013908_create_users_table.php

@@ -15,7 +15,7 @@ public function up()
        Schema::create('users', function(Blueprint $table)
        {
            $table->increments('id');
-            $table->string('username')->after('id')->unique();
+            $table->string('username')->unique();


ここはcreateですし実質変更ないので大丈夫ですかね。

fortkle · 2016-10-08T18:27:54Z

database/migrations/2015_10_25_221700_create_user_roles_table.php

@@ -15,7 +15,7 @@ public function up()
        Schema::create('user_roles', function(Blueprint $table)
        {
            $table->increments('id');
-            $table->string('name')->after('id')->unique();


ここもcreateですし実質変更ないので大丈夫ですかね。

fortkle · 2016-10-08T18:30:36Z

database/migrations/2015_10_25_220452_alter_user_add_role.php

@@ -17,7 +17,7 @@ public function up()
    {
        Schema::table('users', function($table)
        {
-            $table->string('role')->after('password')->default(self::ROLE_ID_MEMBER);
+            $table->string('role')->default(self::ROLE_ID_MEMBER);


あれ、そもそもここで after()削除してるのって何か理由ありますか？
（MySQL onlyな指定なのでこれまでのsqliteでは効いてないのでちょっとアレですが、mysqlなら消さなくてもいいのでは？と。そして消さなくていいなら余分なdiffは作らない方がいいかなと!）

remove query builder method [after] => caused mysql error (may be fixed in Laravel 5.1~)

これでしたね。。 🙇
消してOKです！

fortkle · 2016-10-08T18:31:55Z

@hanhan1978
thanks for your PR.
please check some of my comments 🙏

fortkle · 2016-10-09T04:13:53Z

あー、、あと何よりPRの出し先がmasterではなくdevelop・・・・

hanhan1978 · 2016-10-09T04:15:41Z

そうだった。developだった...。久しぶりすぎてowlのルールを忘れてました・・・

fortkle · 2016-10-09T04:25:23Z

app/Providers/RepositoriesServiceProvider.php

        \App::bind('Owl\Repositories\ItemRepositoryInterface', 'Owl\Repositories\Fluent\ItemRepository');
-        \App::bind('Owl\Repositories\ItemFtsRepositoryInterface', 'Owl\Repositories\Fluent\ItemFtsRepository');
+        if(env('DB_DRIVER', 'sqlite') == 'mysql') {


ここ、PSR-2に合わせてifとelseの前後などに半角スペース1つください...

(phpcsの代わりにscrutinizerにしたのでエラー出てない.... 🙇 )

fortkle · 2016-10-09T04:25:48Z

database/migrations/2014_08_19_164055_create_full_text_search_related_tables.php

-            $fts->words = FtsUtils::toNgram($item->title . "\n\n" . $item->body);
-            $fts->save();
-        }
+		if(env('DB_DRIVER') === 'mysql'){


ここもスペース!

fortkle · 2016-10-09T04:25:55Z

database/migrations/2014_08_19_164055_create_full_text_search_related_tables.php

@@ -32,7 +39,15 @@ public function up()
 	 */
 	public function down()
 	{
-        DB::statement('DROP TABLE items_fts ;');
+		if(env('DB_DRIVER') === 'mysql') {


ここもスペース!

fortkle · 2016-10-09T04:26:03Z

database/migrations/2015_01_24_142354_create_tags_fts.php

-            $fts->words = FtsUtils::toNgram($tag->name);
-            $fts->save();
-        }
+		if(env('DB_DRIVER') === 'mysql'){


ここも（

fortkle · 2016-10-09T04:26:11Z

database/migrations/2015_01_24_142354_create_tags_fts.php

@@ -31,7 +37,13 @@ public function up()
 	 */
 	public function down()
 	{
-        DB::statement('DROP TABLE tags_fts ;');
+		if(env('DB_DRIVER') === 'mysql'){


ここｍ（

すいません、IDEに不慣れだったせいで、タブとスペースが混ざっていたことに今さっき気づきました。とりあえず指摘点を修正し終わったら、その辺相談させて下さい。

- Append space after if/else to follow psr2 - Fix php doc return type - Revert change in migrations - Separate migration by database schema - modify item_tag foreign key constraint -> Add delete cascade for mysql compatibility - Fix migration rollback for items table -> must drop fulltext index before changing collation of a related column.

Change mysql setting [strict] false => true => avoid migration error for creatd_at default value(0) Change migration files for MySQL 1. remove query builder method [after] => caused mysql error (may be fixed in Laravel 5.1~) 2. change token column data type from text to varchar(512) => cannot set unique index for blob data type in MySQL Add setting in .env.example => default sqlite Sync .env setting int .env.behat => add database config Add MySQL specific repository for fulltext search => MySQL/ItemFtsRepository => MySQL/TagFtsRepository

- Append space after if/else to follow psr2 - Fix php doc return type - Revert change in migrations - Separate migration by database schema - modify item_tag foreign key constraint -> Add delete cascade for mysql compatibility - Fix migration rollback for items table -> must drop fulltext index before changing collation of a related column.

hanhan1978 · 2016-10-13T05:04:36Z

@fortkle @sota1235 修正＆developに切り替えました。ご確認よろしくです。

fortkle · 2016-10-23T15:21:20Z

@hanhan1978
LGTM!!
最高にクールなPRでした！マージします！！

hanhan1978 changed the title ~~Enable MySQL Driver #38~~ 8000 Enable MySQL Driver Oct 2, 2016

sota1235 added feature improvement labels Oct 3, 2016

sota1235 reviewed Oct 3, 2016

View reviewed changes

sota1235 assigned fortkle Oct 3, 2016

hanhan1978 added a commit to hanhan1978/owl that referenced this pull request Oct 3, 2016

Fix phpdoc of matchCount return value owl#38 owl#168

17293a4

fortkle reviewed Oct 8, 2016

View reviewed changes

fortkle reviewed Oct 9, 2016

View reviewed changes

fortkle assigned hanhan1978 and unassigned fortkle Oct 9, 2016

hanhan1978 added 2 commits October 13, 2016 12:37

hanhan1978 changed the base branch from master to develop October 13, 2016 04:26

hanhan1978 force-pushed the features/mysql-driver branch from 3352f28 to 87e21c2 Compare October 13, 2016 05:00

fortkle merged commit 327bd83 into owl:develop Oct 23, 2016

fortkle mentioned this pull request Oct 11, 2017

[Release] 2.6.0 #173

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Enable MySQL Driver #168

Enable MySQL Driver #168

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Enable MySQL Driver #168

Enable MySQL Driver #168

Uh oh!

Conversation

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!