SQLの致命的なバグをいくつか修正#1371
Conversation
📝 Walkthrough概要このPRは、駅・路線データの検索・取得機能を改善するもので、COALESCEを用いた駅名・路線色のエイリアスデータ統合、LEFT JOINからINNER JOINへの結合タイプの厳密化、日本語駅名の正規化処理の追加、および包括的なユニットテストの拡充を行っています。 変更内容
推定レビュー工数🎯 3 (Moderate) | ⏱️ ~25分 関連する可能性のあるPR
推奨レビュアー
ポエム
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
stationapi/src/infrastructure/line_repository.rs (1)
214-281: 別名優先の名称解決と LEFT JOIN 化で、駅単位取得のバグは潰せていそうです
find_by_station_idのクエリで
COALESCE(alias_data.line_name, l.line_name)等により alias 側を優先して名称・カラーを解決し、LEFT JOIN station_station_types ... AND sst.pass <> 1によって、型情報が無い駅でも 1 行返るようになった点は、テストとコメントの意図に沿っていて良さそうです。なお、他のメソッドでは
s.e_status = 0/l.e_status = 0を明示的に条件に入れている箇所もあるので、挙動を統一したい場合はここにも同様の条件を追加することを検討しても良いかもしれません(既存仕様依存なので必須ではありません)。
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.sqlx/query-25923b43426ac7b1829aaf1c726635b00edeb2b60996e3e7333772ed9202f327.json.sqlx/query-269872b7f724f7361c016b696ddc05ca06ee06027d6a0e4bd07cf73e8554226b.jsonstationapi/src/domain/normalize.rsstationapi/src/infrastructure/line_repository.rsstationapi/src/infrastructure/station_repository.rsstationapi/src/use_case/interactor/query.rs
🧰 Additional context used
📓 Path-based instructions (6)
stationapi/src/domain/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
In Rust domain layer, entity definitions and repository abstractions should be organized in the
entity/module mirroring the gRPC schema, with repository interfaces usingasync_trait
Files:
stationapi/src/domain/normalize.rs
stationapi/src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Run
cargo test --lib --package stationapiormake test-unitfor unit tests focusing on entities and repository mocks without database
Files:
stationapi/src/domain/normalize.rsstationapi/src/infrastructure/line_repository.rsstationapi/src/use_case/interactor/query.rsstationapi/src/infrastructure/station_repository.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Before committing, runcargo fmton all Rust code
Before committing, runcargo clippy --all-targets --all-featuresand resolve new Clippy warnings unless covered by existing#![allow]attributes
Files:
stationapi/src/domain/normalize.rsstationapi/src/infrastructure/line_repository.rsstationapi/src/use_case/interactor/query.rsstationapi/src/infrastructure/station_repository.rs
stationapi/src/infrastructure/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
stationapi/src/infrastructure/**/*.rs: Repository implementations in infrastructure layer should share anArc<PgPool>connection pool
Integration tests in infrastructure layer must use#[cfg_attr(not(feature = "integration-tests"), ignore)]attribute to conditionally ignore tests when the feature is not enabled
Files:
stationapi/src/infrastructure/line_repository.rsstationapi/src/infrastructure/station_repository.rs
stationapi/src/use_case/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Application logic in use case layer should implement contract traits defined in
traits/module (e.g.,QueryUseCaseininteractor/query.rs)
Files:
stationapi/src/use_case/interactor/query.rs
stationapi/src/use_case/interactor/query.rs
📄 CodeRabbit inference engine (AGENTS.md)
When modifying
QueryInteractor, ensure enrichment steps (companies, train types, line symbols) still behave as expected and verify helper methods such asupdate_station_vec_with_attributesandbuild_route_tree_map
Files:
stationapi/src/use_case/interactor/query.rs
🧠 Learnings (9)
📓 Common learnings
Learnt from: TinyKitten
Repo: TrainLCD/StationAPI PR: 1305
File: stationapi/.sqlx/query-55bee6b45754d1da9bf555d319f015ce232f8574a41c3ef25b730d11071f8428.json:3-3
Timestamp: 2025-08-06T12:47:37.438Z
Learning: TinyKittenのStationAPIプロジェクトにおいて、SQLクエリの `pass <> 1` 条件は一部のクエリでは不要であり、PostgreSQL移行時にコードの最適化として意図的に除去された。この条件の有無についてはドメイン知識が必要で、機械的にチェックすべきではない。
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Implement text normalization logic in `normalize.rs` for search functionality
Applied to files:
stationapi/src/domain/normalize.rsstationapi/src/use_case/interactor/query.rsstationapi/src/infrastructure/station_repository.rs
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/**/*.rs : Run `cargo test --lib --package stationapi` or `make test-unit` for unit tests focusing on entities and repository mocks without database
Applied to files:
stationapi/src/domain/normalize.rsstationapi/src/infrastructure/line_repository.rsstationapi/src/use_case/interactor/query.rsstationapi/src/infrastructure/station_repository.rs
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/use_case/interactor/query.rs : When modifying `QueryInteractor`, ensure enrichment steps (companies, train types, line symbols) still behave as expected and verify helper methods such as `update_station_vec_with_attributes` and `build_route_tree_map`
Applied to files:
stationapi/src/domain/normalize.rsstationapi/src/infrastructure/line_repository.rsstationapi/src/use_case/interactor/query.rsstationapi/src/infrastructure/station_repository.rs.sqlx/query-269872b7f724f7361c016b696ddc05ca06ee06027d6a0e4bd07cf73e8554226b.json
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/domain/repository/**/*.rs : Use `async_trait` for defining repository abstractions in Rust
Applied to files:
stationapi/src/infrastructure/line_repository.rsstationapi/src/use_case/interactor/query.rsstationapi/src/infrastructure/station_repository.rs
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/use_case/**/*.rs : Application logic in use case layer should implement contract traits defined in `traits/` module (e.g., `QueryUseCase` in `interactor/query.rs`)
Applied to files:
stationapi/src/infrastructure/line_repository.rsstationapi/src/use_case/interactor/query.rs
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/domain/**/*.rs : In Rust domain layer, entity definitions and repository abstractions should be organized in the `entity/` module mirroring the gRPC schema, with repository interfaces using `async_trait`
Applied to files:
stationapi/src/infrastructure/line_repository.rsstationapi/src/use_case/interactor/query.rsstationapi/src/infrastructure/station_repository.rs
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/infrastructure/**/*.rs : Integration tests in infrastructure layer must use `#[cfg_attr(not(feature = "integration-tests"), ignore)]` attribute to conditionally ignore tests when the feature is not enabled
Applied to files:
stationapi/src/infrastructure/line_repository.rsstationapi/src/infrastructure/station_repository.rs
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Run integration tests with `source .env.test && cargo test --lib --package stationapi --features integration-tests` or `make test-integration` using a dedicated schema behind `TEST_DATABASE_URL`
Applied to files:
stationapi/src/infrastructure/line_repository.rsstationapi/src/infrastructure/station_repository.rs
🧬 Code graph analysis (2)
stationapi/src/infrastructure/line_repository.rs (2)
stationapi/src/use_case/interactor/query.rs (2)
find_by_station_id(1367-1369)find_by_station_id(1769-1771)stationapi/src/domain/repository/line_repository.rs (2)
find_by_station_id(8-8)find_by_station_id(194-196)
stationapi/src/infrastructure/station_repository.rs (1)
stationapi/src/domain/normalize.rs (1)
normalize_for_search(1-20)
🔇 Additional comments (8)
stationapi/src/domain/normalize.rs (1)
37-104: 正規化テストの網羅性は十分で、仕様とも整合していますひらがな→カタカナ、小文字、範囲端、全角数字/英字、混在ケース、実在駅名までカバーされていて、
normalize_for_searchの仕様とリポジトリ側の検索ロジックをしっかり守ってくれそうです。このままで問題ないと思います。.sqlx/query-25923b43426ac7b1829aaf1c726635b00edeb2b60996e3e7333772ed9202f327.json (1)
3-3:find_by_station_id用メタデータは SQL と構造体に整合しています
COALESCE(alias_data.*, l.*)による line_name 系と line_color_c の解決、sst.line_group_cd AS "line_group_cd?"/sst.type_cd AS "type_cd?"、およびnullable情報はいずれもLineRowのOptionフィールドと整合しており、InternalLineRepository::find_by_station_idのクエリ内容とも一致しているように見えます。このままで問題ないと思います。Also applies to: 23-55, 162-173
stationapi/src/use_case/interactor/query.rs (1)
195-217: 駅名検索の正規化をリポジトリ層に委譲した変更は妥当です
get_stations_by_nameで事前にnormalize_for_searchせず、そのまま文字列をStationRepository::get_by_nameに渡すようにしたことで、
- Kanji 等の「そのままマッチさせたい」パターンを壊さず、
- 正規化ロジック(ひらがな→カタカナ変換など)をリポジトリ側のパターン生成に集約できています。
get_lines_by_nameでは引き続き行名検索用に正規化を使っており、用途ごとの使い分けも整理されていると思います。.sqlx/query-269872b7f724f7361c016b696ddc05ca06ee06027d6a0e4bd07cf73e8554226b.json (1)
3-3:line_group_cdベースの INNER JOIN への変更は筋が良いが、仕様確認だけお願いします
station_station_types/typesを INNER JOIN にして
sst.line_group_cd = $1 AND sst.station_cd = s.station_cdt.type_cd = sst.type_cdで絞り込む形になっているため、指定された
line_group_cdに紐づき、かつ有効な種別情報を持つ駅だけが返るようになりました。
「station_station_typesにエントリが無い駅は、このクエリでは返さない」という仕様で問題ない想定であれば、この変更は妥当だと思いますが、運用上その前提が崩れないかだけ確認しておくと安心です。Also applies to: 247-300, 307-367
stationapi/src/infrastructure/line_repository.rs (1)
1525-1651:LineRowのオプション項目と station_station_types 欠如ケースを明確にテストできています
test_line_row_with_optional_line_group_cd_none/someで、line_group_cd/type_cdがNone/Someの両方をLineに正しく反映できることが確認できています。test_find_by_station_id_without_station_station_typesにより、station_station_typesに行が無い駅でもfind_by_station_idがSomeを返し、line_group_cd/type_cdがNoneになる、という今回の LEFT JOIN 仕様がきちんと担保されています。どちらも今回の SQL 変更の意図に沿ったテストで、リグレッション防止に有効だと思います。
stationapi/src/infrastructure/station_repository.rs (3)
9-13: 駅名検索の正規化パターン分離は意図どおりに機能しそうです
get_by_nameで
station_name_pattern = "%{station_name}%"をstation_name/station_name_rn/station_name_zh/station_name_koに使い、station_name_k_pattern = "%{}%"(normalize_for_search適用)をstation_name_k用にだけ使う、という分離は、
- ひらがな入力 → カタカナの
station_name_kでヒットさせる(例:"しんじゅく"→"シンジュク")、- 漢字入力やその他のスクリプトは元の文字列で検索する、
という要件にうまく合っています。
上で追加されているget_by_name_testsもこの振る舞いをカバーしており、正規化ロジックとクエリの整合性も取れていると思います。Also applies to: 1003-1148
1150-1233:get_by_line_group_idの JOIN 条件はシンプルかつ意図に沿っています
get_by_line_group_idの SQL が
JOIN station_station_types AS sst ON sst.line_group_cd = $1 AND sst.station_cd = s.station_cdJOIN types AS t ON t.type_cd = sst.type_cdという形に整理され、
WHEREから余計な関連条件を外しているので、指定されたline_group_idに属する列車種別付きの駅だけをきれいに取得できるようになっています。このクエリは「line_group に紐づかない駅は結果に含めない」という前提に立っているので、その前提がドメイン的に正しい前提であることだけ、他の利用箇所(例: 経路検索)とあわせて確認しておくと安心です。
1722-1800: 検索パターンと JOIN 構造のテスト追加は狙いが明確で良いです
get_by_name_testsで、ひらがな・カタカナ・漢字・混在入力それぞれに対する%...%パターンとnormalize_for_searchの挙動を直接確認しており、アプリケーションレベルの検索仕様をドキュメント代わりにテストできています。get_by_line_group_id_testsは JOIN 条件やパラメータ型変換自体のテストではありますが、過去にあった「JOIN 条件の抜けで意図しない駅が返る」バグに対する軽量なガードとしては十分意味があると思います。- 末尾の DB 統合テスト群は
#[ignore]が付いており、インフラ層テストを feature / 環境前提で切り替えるというガイドラインにも沿っています。このあたりのテスト追加は、今回の SQL 修正の意図を将来の変更時にも伝えてくれるので良いと思います。
Also applies to: 1806-1827, 1833-1851
* SQLの致命的なバグをいくつか修正 (#1371) * ひらがな混じりで駅検索ができないバグを修正 * get_train_types_by_station_idが動かないバグを修正 * GetStationsByLineGroupIdの修正 * ユニットテスト拡充 * cargo fmt * バス路線はline_typeをOtherLineTypeに強制 (#1372) * direction_idがNULLのトリップに存在するバス停も正しく取り込むよう修正 (#1373) * direction_idがNULLのトリップに存在するバス停も正しく取り込むよう修正 GTFSの仕様ではdirection_idはオプショナルであり、NULLの場合でも バス停は有効な停留所として扱うべきである。 これまではvariant_only_with_neighbors CTEで「direction_id IS NOT NULL」 のトリップにのみ存在するバス停をフィルタリングしていたため、 早81の原宿駅前や渋谷駅東口などのバス停がレスポンスから除外されていた。 * バリアントバス停の位置推定でメイントリップに存在する隣接バス停を優先 variant_only_with_neighborsでバス停の隣接情報を選択する際に、 隣接バス停(prev_stop_id/next_stop_id)がメイントリップに存在する レコードを優先するよう修正。 これにより、原宿駅前などのバス停がより正確な位置に挿入される。 --------- Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
* master<-dev (#1337) * GetRoutesMinimal RPCを実装 (#1334) * Add minimal route retrieval functionality and related proto types * Refactor code formatting for improved readability in grpc.rs and query.rs * Add line symbols to LineMinimal in get_lines_by_station_group_id * Update subproject commit reference in stationapi/proto * Add get_lines_by_id_list method and update QueryUseCase trait * Refactor import order for improved readability in grpc.rs * Use entry method for inserting line_minimal in all_lines to avoid overwriting * Refactor route_row_tree_map creation into a separate build_route_tree_map method for improved readability * Enhance line symbol handling in QueryInteractor: populate line_symbols for all lines and update line_minimal entries conditionally * Refactor line symbol handling in QueryInteractor: update all_lines conditionally based on line_symbols presence * Add AGENTS.md for automation agent and contributor workflow guidelines * Update AGENTS.md Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * 石橋阪大前駅ローカライズ修正 (#1336) --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * master<-dev (#1374) * SQLの致命的なバグをいくつか修正 (#1371) * ひらがな混じりで駅検索ができないバグを修正 * get_train_types_by_station_idが動かないバグを修正 * GetStationsByLineGroupIdの修正 * ユニットテスト拡充 * cargo fmt * バス路線はline_typeをOtherLineTypeに強制 (#1372) * direction_idがNULLのトリップに存在するバス停も正しく取り込むよう修正 (#1373) * direction_idがNULLのトリップに存在するバス停も正しく取り込むよう修正 GTFSの仕様ではdirection_idはオプショナルであり、NULLの場合でも バス停は有効な停留所として扱うべきである。 これまではvariant_only_with_neighbors CTEで「direction_id IS NOT NULL」 のトリップにのみ存在するバス停をフィルタリングしていたため、 早81の原宿駅前や渋谷駅東口などのバス停がレスポンスから除外されていた。 * バリアントバス停の位置推定でメイントリップに存在する隣接バス停を優先 variant_only_with_neighborsでバス停の隣接情報を選択する際に、 隣接バス停(prev_stop_id/next_stop_id)がメイントリップに存在する レコードを優先するよう修正。 これにより、原宿駅前などのバス停がより正確な位置に挿入される。 --------- Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> * 西鉄データ更新 (#1375) * 春日原駅を特急停車駅に指定 * 聖マリア病院前の駅名修正 * master<-dev (#1391) * GetStationsByLineIdList RPC追加 複数のline_idを一括で処理するバッチRPCを追加。 既存のGetStationByIdList/GetLineByIdListと同じバッチパターンに従う。 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * cargo fmt * get_by_line_id_vecのORDER BYを入力順序保持のCASE式に変更 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Merge pull request #1389 from TrainLCD/feature/tt-batch GetStationsByLineGroupIdList RPC追加 * get_stations_by_line_group_id_vecで列車種別を一括取得してセット (#1390) * get_stations_by_line_group_id_vecで列車種別を一括取得してセット N+1問題を回避しつつ、train_type_repository.get_by_line_group_id_vecで 複数line_group_idの列車種別を1クエリで取得し、top-levelとlines[].station の両方にセットするよう修正。 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * get_stations_by_line_group_id_vecのユニットテストを追加 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * ネストされた駅のtrain_typeをis_some()で明示的に検証するよう修正 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * train_typeをtrain_type_mapから常に上書きしてリーク防止 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * 近鉄種別修正 * 近鉄種別修正 駅コード修正 --------- Co-authored-by: Tsubasa SEKIGUCHI <oss@tinykitten.me> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
Summary by CodeRabbit
リリースノート
新機能
テスト
✏️ Tip: You can customize this high-level summary in your review settings.