TypeORMサポートのトライアル #1

sanak · 2024-01-18T16:12:19Z

以前、 feature/switch-better-sqlite3-to-typeorm ブランチで試行していたものを整理の上、JestのテストやPostgreSQL/MySQLで最低限の動作が通るようにしたものになります。

TODO:

システムテストで発生する Entityのimportエラーの解消

PostgreSQLのサポート

※初期データベース構築に30分ほどかかります。

確認手順:

# .env.example ファイルを .env にコピー後、パラメータを適宜調整
$ cp .env.example .env
# PostgreSQLデータベースを作成
$ createdb -U postgres ba000001
# downloadコマンドで初期データベース構築
$ npm start download

TypeORM対応前からのマイグレーションサポート(システムテストのマイグレーションエラーの解消)

MySQLのサポート

※初期データベース構築に30分ほどかかります。
~/.my.cnf で以下の設定後、mysqlサービス再起動が必要です。
```
[mysqld]
sql_mode = "ANSI_QUOTES,PIPES_AS_CONCAT"
```

確認手順:

# .env.example ファイルを .env にコピー後、mysql側のコメントに切り替え、パラメータを適宜調整
$ cp .env.example .env
# MySQLデータベースを作成
$ mysql -u root -p
:
> CREATE DATABASE ba000001;
> exit
# downloadコマンドで初期データベース構築
$ npm start download

座標値が小数点以下3-4桁に丸められてしまう不具合の対応

PostgreSQL/MySQLのdownloadコマンドパフォーマンス改善
対応した内容の整理
アップストリームへのリンク

sanak · 2024-01-23T04:11:08Z

MySQLで座標値が小数点以下3-4桁に丸められてしまう不具合については、以下のQiita記事、TypeORMドキュメントを参考に、 float 型から double precision 型に変更

合わせて、 integer で整数値に8桁を利用していたものの、以下のアドレス・ベース・レジストリデータ項目定義書(第1版)では、 整数 形式の最大値が13となっていたため、SQLite、PostgreSQL、MySQLのいずれでもサポートされている smallint 型に変更

アドレス・ベース・レジストリデータ項目定義書(第1版): https://www.digital.go.jp/assets/contents/node/basic_page/field_ref_resources/3d996df3-f87c-4439-b474-c0fbbf3ddad8/63a8ee41/20220422_policies_base_registry_format_02.xlsx

ただし、上記対応の影響かで、PostgreSQL/MySQLの初期データベース構築にさらに+1時間程度の時間がかかるようになっているかもしれません。

(可能であれば) PostgreSQL/MySQLのdownloadコマンドパフォーマンス改善

上記については、SQLite、PostgreSQL、MySQLでサポートされている複数行インサートを週末目処で少し試してみます。

maskatsum · 2024-01-24T02:33:43Z

src/controller/download/process/load-dataset-process.ts

+      // awaitがあるため、forEachでなくforループで回す
+      for (let i = 0; i < paramsList.length; i++) {
+        const params = paramsList[i];
+        await queryRunner.connection.query(preparedSql, params);
+      }


ここしか見ていないので、分かりませんが、たぶん、こんな感じでできませんかねー。

const tasks = paramsList.map(params => { return queryRunner.connection.query(preparedSql, params); }) Promise.all(tasks);

preparedSql が大きかったりすると、シリアルに実行したほうが良かったりしますが。

@maskatsum こちらにつきまして、アドバイス頂き、大変ありがとうございます！🙇
今朝の時点で、手元のPC (M1 MacBook Pro)で、PostgreSQLで約2時間、MySQLで約4時間かかっていたものが、
上記の対応のみで、いずれも30分程度で処理完了するようになりました。😊

修正前:

PostgreSQL: npm start download 1050.98s user 637.60s system 22% cpu 2:02:43.84 total

MySQL: npm start download 2806.36s user 1274.25s system 27% cpu 4:06:54.28 total

修正後:

PostgreSQL: npm start download 897.74s user 700.45s system 84% cpu 31:34.43 total

MySQL: npm start download 942.74s user 273.01s system 54% cpu 37:28.01 total

maskatsum · 2024-01-24T02:42:49Z

src/domain/prepare-sql-and-param-keys.ts

+    if (dsType === 'postgres') {
+      tempSql = tempSql.replace(matchedInsertOrReplace[0], 'INSERT INTO');
+      tempSql = tempSql.replace(/-- /g, '');
+    } else if (dsType === 'mysql') {
+      tempSql = tempSql.replace(matchedInsertOrReplace[0], 'REPLACE INTO');
+    }


ここは switch で分けたいですね。
あと dsType が string になっていますが、Enum にしたいですねー。

ありがとうございます。
別ブランチ上で作業中ですが、一旦下記コミットで対応しています。
(Enumに関しては良く分かりませんでしたので一旦保留としています。明日確認させてください。)
a8cb67f

maskatsum · 2024-01-24T02:54:19Z

src/domain/prepare-sql-and-param-keys.ts

+  if (matchedJSONFunctions) {
+    for (let i = 0; i < matchedJSONFunctions.length; i++) {
+      const matchedJSONFucntion = matchedJSONFunctions[i];
+      if (matchedJSONFucntion.toLowerCase() === 'json_group_array') {
+        if (dsType === 'postgres') {
+          tempSql = tempSql.replace(matchedJSONFucntion, 'json_agg');
+        } else if (dsType === 'mysql') {
+          tempSql = tempSql.replace(matchedJSONFucntion, 'json_arrayagg');
+        }
+      } else if (matchedJSONFucntion.toLowerCase() === 'json_object') {
+        if (dsType === 'postgres') {
+          tempSql = tempSql.replace(matchedJSONFucntion, 'json_build_object');
+        }
+      }
+    }
+  }
+  return {
+    preparedSql: tempSql,
+    paramKeys: keys,
+  };


こんな感じはどうですか？

// ネスティングはなるべく浅くすると良いです。コードの可読性が上がります。 if (!matchedJSONFunctions) { return { preparedSql: tempSql, paramKeys: keys, }; } matchedJSONFunctions.forEach(matchedJSONFucntion => { const functionName = matchedJSONFucntion.toLowerCase(); switch (`${functionName}-${dsType}`) { case 'json_group_array-postgress': tempSql = tempSql.replace(matchedJSONFucntion, 'json_agg'); break; case 'json_group_array-mysql': tempSql = tempSql.replace(matchedJSONFucntion, 'json_arrayagg'); break; case 'json_object-postgres': tempSql = tempSql.replace(matchedJSONFucntion, 'json_build_object'); break; default: break; } }); return { preparedSql: tempSql, paramKeys: keys, };

ありがとうございます。
別ブランチ上で作業中ですが、上記修正をそのまま採用させて頂き、以下のコミットで対応しています。
1de8fff

sanak · 2024-01-24T04:42:42Z

@maskatsum 色々とアドバイス頂き、ありがとうございます！ 🙇🙏
今晩、確認するようにいたしますので、よろしくお願いいたします。

1/25追記: 取り急ぎ、1つ目のコメント箇所に対応いたしました。残るコメントについては、週末にかけて確認するようにいたします。 🙇

maskatsum · 2024-01-25T06:55:14Z

ご対応ありがとうございますー。週末の打ち合わせでお話させていただくのを楽しみにしております。

maskatsum

明日、細かく説明しますが、気づいた点をコメントさせていただきますー

maskatsum · 2024-01-26T07:38:59Z

src/domain/prepare-sql-and-param-keys.ts

+  let tempSql = sql;
+  const keys = [];
+  const dsType = ds.options.type;
+  const matchedParams = tempSql.match(/@[a-zA-Z_0-9]+/g);


tempSql.match(/@[a-zA-Z_0-9]+/g);

の部分は、RegExpEx.create を使っていただくと、正規表現のインスタンスがキャッシュされます。

また、@ は、設定値から来るものなので、ハードコーディングしてしまうと設定を変更した場合に反映されなくなってしまいます。

abr-geocoder/src/settings/constant-values.ts

Line 66 in a850862

export const DASH: string = '@';

なので以下のようにしていただくのが良いです

tempSql.match(RegExpEx.create(`${DASH}[a-zA-Z_0-9]+`, 'g'));

maskatsum · 2024-01-26T07:39:33Z

src/domain/prepare-sql-and-param-keys.ts

+  if (matchedParams) {
+    for (let i = 0; i < matchedParams.length; i++) {
+      const matchedParam = matchedParams[i];
+      const paramName = matchedParam.replace('@', '');


matchedParam.replace('@', '');

は

matchedParam.replace(DASH, '');

ですねー。

ありがとうございます。
別ブランチで作業中ですが、一つ上のRegExpExと合わせて、以下のコミットで対応しています。
a971ad7

maskatsum · 2024-01-26T07:41:48Z

src/domain/replace-sql-insert-values.ts

+  if (matchedInsertValues) {
+    for (let rowIdx = 0; rowIdx < rowCount; rowIdx++) {
+      const placeHolders = [];
+      for (let paramIdx = 0; paramIdx < paramsCount; paramIdx++) {
+        const placeHolder =
+          dsType === 'postgres'
+            ? `$${rowIdx * paramsCount + paramIdx + 1}`
+            : '?';
+        placeHolders.push(placeHolder);
+      }
+      rows.push(`(${placeHolders.join(',')})`);
+    }
+    tempSql = tempSql.replace(
+      matchedInsertValues[0],
+      'VALUES ' + rows.join(',')
+    );
+  }


ここのネストが深い部分は解消したいですねー。

17行目の if (matchedInsertValues) { のブロックで1階層は減らせそうですが、

if (!matchedInsertValues) { return tempSql; } for (let rowIdx = 0; rowIdx < rowCount; rowIdx++) { const placeHolders = []; for (let paramIdx = 0; paramIdx < paramsCount; paramIdx++) { const placeHolder = dsType === 'postgres' ? `$${rowIdx * paramsCount + paramIdx + 1}` : '?'; placeHolders.push(placeHolder); } rows.push(`(${placeHolders.join(',')})`); } tempSql = tempSql.replace(matchedInsertValues[0], 'VALUES ' + rows.join(','));

さらにrow/paramの2階層ループのネストを減らそうとなると、難しそうな気がしました。

書き方にあまり慣れていませんが、以下のサイトなどを参考に、forループを短縮して書くことは
可能なようでしたが、どのレベルまで対応するかは、明日、確認させてください。
https://stackoverflow.com/questions/3746725/how-to-create-an-array-containing-1-n

const rows = [...Array(rowCount).keys()].map(rowIdx => { const placeHolders = [...Array(paramsCount).keys()].map(paramIdx => { return dsType === 'postgres' ? `$${rowIdx * paramsCount + paramIdx + 1}` : '?'; }); return `(${placeHolders.join(',')})`; });

maskatsum · 2024-01-26T07:47:59Z

src/migration/1705589170557-InitialSchema.ts

+      await queryRunner.query('DROP INDEX IF EXISTS "dataset_key"');
+      await queryRunner.query('DROP TABLE IF EXISTS "dataset_old"');
+      await queryRunner.query('DROP INDEX IF EXISTS "metadata_key"');
+      await queryRunner.query('DROP TABLE IF EXISTS "metadata_old"');
+      await queryRunner.query('DROP INDEX IF EXISTS "rsdtdsp_rsdt_code"');
+      await queryRunner.query('DROP TABLE IF EXISTS "rsdtdsp_rsdt_old"');
+      await queryRunner.query('DROP INDEX IF EXISTS "rsdtdsp_blk_code"');
+      await queryRunner.query('DROP TABLE IF EXISTS "rsdtdsp_blk_old"');
+      await queryRunner.query('DROP INDEX IF EXISTS "town_code"');
+      await queryRunner.query('DROP TABLE IF EXISTS "town_old"');
+      await queryRunner.query('DROP INDEX IF EXISTS "city_code"');
+      await queryRunner.query('DROP TABLE IF EXISTS "city_old"');
+      await queryRunner.query('DROP INDEX IF EXISTS "pref_code"');
+      await queryRunner.query('DROP TABLE IF EXISTS "pref_old"');


この辺、こんな感じに、1queryにまとめられるような気がします（試してないですー）。

await queryRunner.query(` DROP INDEX IF EXISTS "dataset_key"; DROP TABLE IF EXISTS "dataset_old"; DROP INDEX IF EXISTS "metadata_key"; DROP TABLE IF EXISTS "metadata_old"; .... DROP TABLE IF EXISTS "pref_old"; `);

ありがとうございます。
npm run test:system で上記の1クエリに集約できることを確認しました。
bd4193a

マイグレーションファイル内の他の箇所についても集約した方が良いか、明日、確認させてください。

Adjust dummy sqlite database path

Replace integer to smallint to save space

…mmand performance drastically

…d expect improving insert performance

…r MySQL performance

sanak · 2024-07-31T11:25:17Z

アップストリームのv2.0.0(地番レベル対応分)で、SQLiteデータベースが複数(各市区町村毎)に生成される形となったことに伴い、TypeORMでの対応が難しくなったようでしたので、こちらのPRは #4 と合わせてクローズします。
digital-go-jp#97 (comment)

sanak self-assigned this Jan 18, 2024

sanak mentioned this pull request Jan 22, 2024

調査：TypeORM への対応 digital-go-jp/abr-geocoder#97

Closed

maskatsum reviewed Jan 24, 2024

View reviewed changes

maskatsum reviewed Jan 26, 2024

View reviewed changes

Ko Nagase added 14 commits February 3, 2024 23:39

Add typeorm and generate initial schema from entity

22569c6

Switching better-sqlite3 to typeorm

ff3e69c

Update tests

16a6734

Fix system test error

aa84a73

Adjust dummy sqlite database path

Supporting PostgreSQL

dddd8b7

Update tests

b32cd99

Support original sqlite schema migration

057c635

Supporting MySQL

bad5305

Replace json_group_array to json_arrayagg for MySQL

771ffe4

Replace float to double precision for MySQL

e8f7986

Replace integer to smallint to save space

Supporting bulk insert for download command performance

7505873

Remove indices because primary keys cover those

451eea9

Multithreaded pos (lon/lat) update by @maskatsum improved download co…

a623516

…mmand performance drastically

Use BULK_INSERT_CHUNK_SIZE (500) to protect SQLite parameter limit an…

05f4bb3

…d expect improving insert performance

sanak force-pushed the feature/97-typeorm-trial branch from a850862 to 05f4bb3 Compare February 3, 2024 16:39

Revert BULK_INSERT_CHUNK_SIZE (500) and use BULK_INSERT_SIZE (500) fo…

fbaf1a0

…r MySQL performance

This was referenced Feb 5, 2024

[TypeORM] リファクタリング要望への対応 #2

Closed

[TypeORM] 2024/01/27 お打ち合わせでのリファクタリングのアドバイス適用 #4

Closed

sanak closed this Jul 31, 2024

sanak mentioned this pull request Jul 31, 2024

[TypeORM] データフォーマット（仕様確定版）への対応準備 #3

Closed

TypeORMサポートのトライアル #1

TypeORMサポートのトライアル #1

Uh oh!

Conversation

sanak commented Jan 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sanak commented Jan 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

maskatsum Jan 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sanak commented Jan 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maskatsum commented Jan 25, 2024

Uh oh!

maskatsum left a comment

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sanak commented Jul 31, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sanak commented Jan 18, 2024 •

edited

Loading

sanak commented Jan 23, 2024 •

edited

Loading

maskatsum Jan 24, 2024 •

edited

Loading

sanak commented Jan 24, 2024 •

edited

Loading