Fix: MySQL年代別集計でBIGINT UNSIGNEDオーバーフローエラーを修正#1350
Conversation
YEAR()関数がunsigned integerを返すため、order_birthの年がcreate_dateの年以上の場合に 引き算が負数となりBIGINT UNSIGNEDオーバーフローが発生していた。 CAST(... AS SIGNED)で符号付き演算に変更することで修正。 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughYEAR()で取得した年の差算を行うSQLで、年値を減算前に明示的にSIGNEDキャストするよう修正。加えて、MySQL専用の単体テストが同ファイル内で同一内容のテストを重複追加している。 変更内容
推定コード審査所要時間🎯 3 (Moderate) | ⏱️ ~20 分 ポエム
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1350 +/- ##
=======================================
Coverage 54.42% 54.42%
=======================================
Files 84 84
Lines 10817 10817
=======================================
Hits 5887 5887
Misses 4930 4930
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@data/class/db/dbfactory/SC_DB_DBFactory_MYSQL.php`:
- Line 219: The current return expression in SC_DB_DBFactory_MYSQL.php uses
RIGHT(create_date, 5) and RIGHT(order_birth, 5), which extract minutes:seconds
for TIMESTAMP/DATETIME and break the month-day comparison; replace those
RIGHT(...) calls with DATE_FORMAT(create_date, '%m-%d') and
DATE_FORMAT(order_birth, '%m-%d') respectively so the month-day is correctly
compared while keeping the CAST(... AS SIGNED) changes and the TRUNC->TRUNCATE
behavior intact in the same return expression.
同年のorder_birthでRIGHT(col,5)の比較結果が1になるケースで 修正前のSQLがBIGINT UNSIGNEDオーバーフローを起こすことを検証するテスト。 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
dateTimeThisDecade() は直近10年の日付を生成するため、
0〜6歳の生年月日が生成されていた。
これにより売上集計の年代別集計で MySQL の BIGINT UNSIGNED
オーバーフローが発生する場合があった。
dateTimeBetween('-80 years', '-18 years') に変更し、
18〜80歳の現実的な生年月日を生成するようにした。
See: EC-CUBE/ec-cube2#1350
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/class/db/dbfactory/SC_DB_DBFactory_MYSQLTest.php (1)
152-197: 回帰テストとして適切です。BIGINT UNSIGNED オーバーフローのシナリオを正しく再現しています。テストデータの設定とクエリの実行ロジックは正しく、修正前のSQLでオーバーフローが発生するケースを正確に再現しています。
1点だけ提案：現在のアサーションはクエリがエラーなく完了することのみを検証していますが、
ageカラムの計算結果も検証すると、回帰テストとしてより堅牢になります。この修正後のSQLでは、YEAR差=0 から 1 を引いて -1、TRUNC(-1, -1) = -10 が返るはずです。💡 計算結果の検証を追加する提案
$this->assertIsArray($result); $this->assertCount(1, $result); + $this->assertEquals(-10, (int) $result[0]['age'], 'YEAR差0で誕生日未到来の場合、年齢は-10となること');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/class/db/dbfactory/SC_DB_DBFactory_MYSQLTest.php` around lines 152 - 197, The test currently only asserts the query succeeded; add an assertion that validates the computed age value from getOrderTotalAgeColSql to make the regression check stronger: after retrieving $result (from the test method testGetOrderTotalAgeColSqlは同年で誕生日未到来でもオーバーフローしない), assert that the 'age' field in $result[0] equals the expected value produced by the fixed SQL (TRUNC(-1, -1) => -10), e.g. use assertEquals(-10, (int)$result[0]['age']) or equivalent to tolerate string numeric results; keep the existing group/order/count assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/class/db/dbfactory/SC_DB_DBFactory_MYSQLTest.php`:
- Around line 152-197: The test currently only asserts the query succeeded; add
an assertion that validates the computed age value from getOrderTotalAgeColSql
to make the regression check stronger: after retrieving $result (from the test
method testGetOrderTotalAgeColSqlは同年で誕生日未到来でもオーバーフローしない), assert that the 'age'
field in $result[0] equals the expected value produced by the fixed SQL
(TRUNC(-1, -1) => -10), e.g. use assertEquals(-10, (int)$result[0]['age']) or
equivalent to tolerate string numeric results; keep the existing
group/order/count assertions.
RIGHT(datetime, 5)はMM:SS(分秒)を返すため月日比較が正しくなかった。 DATE_FORMAT(col, '%m-%d')に変更し正しく月日を比較するようにした。 テストでもage計算結果の値を検証するよう強化。 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
nanasess
left a comment
nanasess
left a comment
There was a problem hiding this comment.
レビュー結果: Approve
修正内容を確認しました。適切な修正です。
1. CAST(YEAR(...) AS SIGNED) — 適切
MySQL 8+ では YEAR() が UNSIGNED INT を返すため、同年で誕生日未到来の場合に 0 - 1 = -1 でオーバーフローします。CAST(... AS SIGNED) で符号付き演算に変換するのは正しいアプローチです。
2. RIGHT() → DATE_FORMAT() — 適切（既存バグの修正を含む）
RIGHT(datetime, 5) は DATETIME 値 '2026-02-01 00:00:00' の末尾5文字 00:00（分秒）を返してしまい、月日比較として元から壊れていました。DATE_FORMAT('%m-%d') に変更することで正しく月日を比較できるようになります。SQLite3版の strftime('%m-%d', ...) とも一貫性があります。
3. TRUNC の扱い — 問題なし
sfChangeTrunc() で MySQL の TRUNCATE に自動変換される仕組みは引き続き正常に機能します。TRUNCATE(-1, -1) = 0 となるため、同年で誕生日未到来の場合は年代 0 となり妥当です。
4. テスト — 適切
- オーバーフローを再現するエッジケースを正確にカバーしている
- age の値まで検証しており、回帰テストとして有効
DB_TYPEチェックで MySQL 環境のみ実行されるのも適切
🤖 Generated with Claude Code
2 participants
Summary
BIGINT UNSIGNED value is out of rangeエラーが発生するバグを修正YEAR()関数が unsigned integer を返すため、order_birthの年がcreate_dateの年以上の場合に引き算が負数となりオーバーフローしていたCAST(... AS SIGNED)で符号付き演算に変更原因
SC_DB_DBFactory_MYSQL::getOrderTotalAgeColSql()の SQL:YEAR(create_date) - YEAR(order_birth)が0で、かつ誕生日がまだ来ていない場合に0 - 1 = -1となり、MySQL 8+ の unsigned 演算でオーバーフローする。PostgreSQL 版は
AGE()関数で符号付き演算のため問題なし。Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit