Open
Conversation
KnpPaginatorのCOUNT(DISTINCT)処理による性能問題を解決するため、 カスタムカウントクエリを実装しました。 ## 問題 - 受注/商品/会員管理の一覧画面で、データ量増加に伴い表示が遅延 - 原因: KnpPaginatorが自動実行するCOUNT(DISTINCT o.id) - JOINによる行の膨張（例: 1万注文×3商品=3万行） - DISTINCT処理で3万行→1万件に戻すコストが膨大 ## 解決策 COUNT用クエリとデータ取得用クエリを分離: - COUNT: JOINなしの高速クエリ（インデックス使用） - データ取得: 従来通りJOINあり（必要な50件だけ処理） - knp_paginator.countヒントでカスタムカウントを渡す ## 変更内容 - OrderRepository: countBySearchDataForAdmin()追加 - ProductRepository: countBySearchDataForAdmin()追加 - CustomerRepository: countBySearchData()追加 - 各Controller: JOIN不要な検索時にカスタムカウント使用 ## 期待効果 - COUNT処理時間: 5-10秒 → 0.1秒未満 - 初期表示時間: 数秒 → 1秒未満 - データベース負荷: JOIN処理を完全に排除 Fixes EC-CUBE#6527 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 4.3 #6571 +/- ##
============================================
- Coverage 82.67% 78.69% -3.98%
- Complexity 0 6816 +6816
============================================
Files 480 476 -4
Lines 26568 27060 +492
============================================
- Hits 21966 21296 -670
- Misses 4602 5764 +1162
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:
|
Contributor
|
@nobuhiko |
Contributor
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthrough管理画面の会員・受注・商品一覧で、JOIN不要と判定した検索時にJOINを含まない専用COUNTクエリを使う分岐を追加。各リポジトリにカウント専用メソッドを追加し、コントローラーは総件数をクエリのヒントとしてページネータに渡す実装を導入した。 Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as "Admin UI"
participant Controller as "Controller\n(e.g. Order/Product/CustomerController)"
participant Repo as "Repository\n(countBySearchData...)"
participant DB as "Database"
participant Paginator as "KnpPaginator"
Admin->>Controller: リクエスト（検索条件、page,size）
Controller->>Controller: 検索条件を評価（JOIN必須か判定）
alt JOIN不要
Controller->>Repo: countBySearchData*(searchData)
Repo->>DB: COUNT クエリ実行（JOIN無し）
DB-->>Repo: totalCount
Repo-->>Controller: totalCount
Controller->>Controller: 検索用 Query を取得
Controller->>Controller: Query.setHint('knp_paginator.count', totalCount)
Controller->>Paginator: paginate(Query, page, size)
else JOIN必要
Controller->>Controller: QueryBuilder を取得（JOIN含む）
Controller->>Paginator: paginate(QueryBuilder, page, size)
end
Paginator-->>Controller: ページング結果（items, total）
Controller-->>Admin: レンダリングデータ
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (3)
src/Eccube/Repository/ProductRepository.php (1)
417-417: 新規メソッドに型宣言を追加してください
countBySearchDataForAdminはarray $searchDataと: intを明示した方が安全です。修正例
- public function countBySearchDataForAdmin($searchData) + public function countBySearchDataForAdmin(array $searchData): intAs per coding guidelines
**/*.php: Use PHP type declarations for parameters and return types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Eccube/Repository/ProductRepository.php` at line 417, The method signature for countBySearchDataForAdmin lacks PHP type declarations; update the function declaration for countBySearchDataForAdmin to declare the parameter as array $searchData and the return type as : int, and ensure any docblock/comments reflect the types and that callers expecting the old signature still pass an array (or are updated) so type compatibility is preserved.src/Eccube/Repository/CustomerRepository.php (1)
446-446: 新規メソッドに型宣言を付与してください
countBySearchDataはarray $searchDataと: intの明示を推奨します。修正例
- public function countBySearchData($searchData) + public function countBySearchData(array $searchData): intAs per coding guidelines
**/*.php: Use PHP type declarations for parameters and return types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Eccube/Repository/CustomerRepository.php` at line 446, The method countBySearchData lacks PHP type declarations: change its signature to declare the parameter as array and the return type as int (i.e., declare array $searchData and : int on the method). Update any callers if needed to satisfy the stricter type and adjust the docblock/annotations if present to match the new parameter and return types.src/Eccube/Repository/OrderRepository.php (1)
486-486: 新規メソッドに型宣言を追加してください
countBySearchDataForAdminはarray $searchData/: intを明示するのが安全です。修正例
- public function countBySearchDataForAdmin($searchData) + public function countBySearchDataForAdmin(array $searchData): intAs per coding guidelines
**/*.php: Use PHP type declarations for parameters and return types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Eccube/Repository/OrderRepository.php` at line 486, The new method signature for countBySearchDataForAdmin lacks PHP type declarations; update the function declaration of countBySearchDataForAdmin to accept array $searchData and declare a return type of : int, and adjust any docblocks/comments accordingly (and update any callers or mocks if they relied on a looser signature).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Eccube/Controller/Admin/Customer/CustomerController.php`:
- Around line 175-187: The custom-count path uses
customerRepository->countBySearchData($searchData) which ignores runtime
modifications applied to $qb by the ADMIN_CUSTOMER_INDEX_SEARCH event; instead,
compute the count from the finalized $qb (after the event) by cloning the $qb,
removing ORDER BY, changing its select to a COUNT expression, executing
getSingleScalarResult() (preserving parameters/bindings) and use that value for
setHint('knp_paginator.count', $count); replace the call to countBySearchData
and ensure you still call $qb->getQuery() for pagination.
- Around line 172-174: Replace the isset() check with a non-empty (trimmed)
check so empty strings are treated as "not provided"; specifically, update the
$useCustomCount assignment (in CustomerController where $useCustomCount is set)
from using isset($searchData['buy_product_name']) to something like: set
$useCustomCount based on whether trim((string)($searchData['buy_product_name']
?? '')) is empty (e.g. $useCustomCount =
trim((string)($searchData['buy_product_name'] ?? '')) === ''; this ensures blank
or whitespace-only values do not bypass the custom count optimization).
In `@src/Eccube/Controller/Admin/Order/OrderController.php`:
- Around line 323-335: The current fast-path uses countBySearchDataForAdmin
(repository fixed logic) which ignores any modifications applied to $qb by the
ADMIN_ORDER_INDEX_SEARCH event, causing count/paging mismatches; instead, derive
the count from the actual (possibly modified) $qb: clone $qb (or use $query =
$qb->getQuery()), convert it to a COUNT query (e.g. select COUNT(DISTINCT o.id)
or COUNT(o.id) depending on semantics), execute getSingleScalarResult() to
obtain $count, then set the 'knp_paginator.count' hint on the query and continue
to paginate; update the block around countBySearchDataForAdmin usage to use this
query-derived count so event-extended conditions are honored.
- Around line 313-321: The useCustomCount calculation currently relies only on
isset() checks so it stays false when form keys exist but are empty; update the
condition that sets $useCustomCount to mirror countBySearchDataForAdmin() by
checking each relevant searchData key with isset() AND value non-emptiness (e.g.
use StringUtil::isNotBlank($searchData['...']) or !empty($searchData['...']))
for the same fields (buy_product_name, payment, shipping_mail, tracking_number,
shipping_delivery_datetime_start/end, shipping_delivery_date_start/end) so that
useCustomCount becomes true when all those values are effectively blank; ensure
this logic is applied where $useCustomCount is computed so
getQueryBuilderBySearchDataForAdmin() can still hit the custom-count
optimization when only blank values are present.
In `@src/Eccube/Controller/Admin/Product/ProductController.php`:
- Around line 274-286: The paging count diverges because $count is computed from
$searchData via productRepository->countBySearchDataForAdmin while $qb may be
modified by the ADMIN_PRODUCT_INDEX_SEARCH event; change the code to derive the
count from the same QueryBuilder ($qb) after the event instead of from
$searchData. Concretely, either call a repository method that accepts the
modified $qb (or add one, e.g., countByQueryBuilder(QueryBuilder $qb)) or
compute the total with a cloned $qb (clone $qb, replace select with COUNT(...)
and getSingleScalarResult()) and pass that value into
$query->setHint('knp_paginator.count', $count) so the paginator uses a count
consistent with the modified $qb.
- Around line 268-273: The custom-count flag useCustomCount is computed
incorrectly: add 'id' to the fields that disable custom count and treat empty
strings as "not provided" so empty inputs don't force JOINs; update the check
that sets $useCustomCount (used around ProductController::... in this diff) to
also consider $searchData['id'] and use a trimmed-empty check (e.g. isset(...)
&& trim((string)$searchData['id']) !== '') for each searchable key so that when
an id search is present (and non-empty) the code does not select the custom path
(countBySearchDataForAdmin handles id) and when inputs are empty strings the
fast-path custom count remains allowed.
In `@src/Eccube/Repository/CustomerRepository.php`:
- Around line 446-622: countBySearchData builds a QueryBuilder but does not
apply the same query customizations as the listing, causing mismatched totals;
modify countBySearchData to invoke the repository's query customization for
QueryKey::CUSTOMER_SEARCH on the QueryBuilder (e.g., call the existing method
that applies customizes such as applyCustomize or customizeQueryBuilder with
QueryKey::CUSTOMER_SEARCH and the $qb) immediately after creating $qb and before
adding filters so the same extensions/settings used by the list query are
applied to the count.
In `@src/Eccube/Repository/OrderRepository.php`:
- Around line 486-693: countBySearchDataForAdmin builds a filtered count
QueryBuilder but never applies the same Query customize used for the list
(QueryKey::ORDER_SEARCH_ADMIN), causing mismatched totals; update
countBySearchDataForAdmin to invoke the same query-customization logic used by
the listing flow (apply the QueryKey::ORDER_SEARCH_ADMIN customization) after
the QueryBuilder 'o' is constructed and before calling getSingleScalarResult(),
ensuring any joins, extra where clauses or tenant/site filters added by the
customizer (the same customization applied to the list query) are applied to
this count query as well.
In `@src/Eccube/Repository/ProductRepository.php`:
- Around line 417-507: countBySearchDataForAdmin omits the ProductCategory
join/filters used by the list query (pc.visible = :visible and pc.code LIKE ...)
and also does not run the same customization for QueryKey::PRODUCT_SEARCH_ADMIN,
causing count/list mismatches; fix by adding the same JOIN to ProductCategories
(alias pc) and applying the identical conditions for pc.visible and pc.code LIKE
when searchData contains those values, and ensure the repository's customization
hook for QueryKey::PRODUCT_SEARCH_ADMIN is invoked on the QueryBuilder (same
mechanism the list query uses) so the count query and list query share identical
filters.
---
Nitpick comments:
In `@src/Eccube/Repository/CustomerRepository.php`:
- Line 446: The method countBySearchData lacks PHP type declarations: change its
signature to declare the parameter as array and the return type as int (i.e.,
declare array $searchData and : int on the method). Update any callers if needed
to satisfy the stricter type and adjust the docblock/annotations if present to
match the new parameter and return types.
In `@src/Eccube/Repository/OrderRepository.php`:
- Line 486: The new method signature for countBySearchDataForAdmin lacks PHP
type declarations; update the function declaration of countBySearchDataForAdmin
to accept array $searchData and declare a return type of : int, and adjust any
docblocks/comments accordingly (and update any callers or mocks if they relied
on a looser signature).
In `@src/Eccube/Repository/ProductRepository.php`:
- Line 417: The method signature for countBySearchDataForAdmin lacks PHP type
declarations; update the function declaration for countBySearchDataForAdmin to
declare the parameter as array $searchData and the return type as : int, and
ensure any docblock/comments reflect the types and that callers expecting the
old signature still pass an array (or are updated) so type compatibility is
preserved.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/Eccube/Controller/Admin/Customer/CustomerController.phpsrc/Eccube/Controller/Admin/Order/OrderController.phpsrc/Eccube/Controller/Admin/Product/ProductController.phpsrc/Eccube/Repository/CustomerRepository.phpsrc/Eccube/Repository/OrderRepository.phpsrc/Eccube/Repository/ProductRepository.php
Comment on lines
+175
to
+187
| if ($useCustomCount) { | ||
| // カスタムカウントを使用して高速化 | ||
| $count = $this->customerRepository->countBySearchData($searchData); | ||
| $query = $qb->getQuery(); | ||
| $query->setHint('knp_paginator.count', $count); | ||
|
|
||
| $pagination = $paginator->paginate( | ||
| $query, | ||
| $page_no, | ||
| $pageCount, | ||
| $paginate_options | ||
| ); | ||
| } else { |
There was a problem hiding this comment.
イベントで拡張された qb が count に反映されません
ADMIN_CUSTOMER_INDEX_SEARCH で qb に条件が追加された場合、count は $searchData ベースの別計算なので件数がズレます。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Eccube/Controller/Admin/Customer/CustomerController.php` around lines 175
- 187, The custom-count path uses
customerRepository->countBySearchData($searchData) which ignores runtime
modifications applied to $qb by the ADMIN_CUSTOMER_INDEX_SEARCH event; instead,
compute the count from the finalized $qb (after the event) by cloning the $qb,
removing ORDER BY, changing its select to a COUNT expression, executing
getSingleScalarResult() (preserving parameters/bindings) and use that value for
setHint('knp_paginator.count', $count); replace the call to countBySearchData
and ensure you still call $qb->getQuery() for pagination.
Comment on lines
+323
to
+335
| if ($useCustomCount) { | ||
| // カスタムカウントを使用して高速化 | ||
| $count = $this->orderRepository->countBySearchDataForAdmin($searchData); | ||
| $query = $qb->getQuery(); | ||
| $query->setHint('knp_paginator.count', $count); | ||
|
|
||
| $pagination = $paginator->paginate( | ||
| $query, | ||
| $page_no, | ||
| $page_count, | ||
| $paginate_options | ||
| ); | ||
| } else { |
There was a problem hiding this comment.
イベント拡張後の検索条件が count に反映されません
ADMIN_ORDER_INDEX_SEARCH で qb が拡張されても、count は repository の固定条件で別計算です。結果件数とページング件数の乖離を起こします。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Eccube/Controller/Admin/Order/OrderController.php` around lines 323 -
335, The current fast-path uses countBySearchDataForAdmin (repository fixed
logic) which ignores any modifications applied to $qb by the
ADMIN_ORDER_INDEX_SEARCH event, causing count/paging mismatches; instead, derive
the count from the actual (possibly modified) $qb: clone $qb (or use $query =
$qb->getQuery()), convert it to a COUNT query (e.g. select COUNT(DISTINCT o.id)
or COUNT(o.id) depending on semantics), execute getSingleScalarResult() to
obtain $count, then set the 'knp_paginator.count' hint on the query and continue
to paginate; update the block around countBySearchDataForAdmin usage to use this
query-derived count so event-extended conditions are honored.
Comment on lines
+268
to
+273
| // JOIN必要な検索条件がない場合はカスタムカウントを使用 | ||
| $useCustomCount = !isset($searchData['category_id']) | ||
| && !isset($searchData['stock_status']) | ||
| && !isset($searchData['stock']) | ||
| && !isset($searchData['tag_id']); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the specific file and lines mentioned
wc -l src/Eccube/Controller/Admin/Product/ProductController.phpRepository: EC-CUBE/ec-cube
Length of output: 121
🏁 Script executed:
# Read the relevant section of the code
sed -n '260,300p' src/Eccube/Controller/Admin/Product/ProductController.phpRepository: EC-CUBE/ec-cube
Length of output: 1418
🏁 Script executed:
# Look for countBySearchDataForAdmin to understand its usage and what it expects
rg "countBySearchDataForAdmin" -A 5 -B 5 src/Eccube/Controller/Admin/Product/ProductController.phpRepository: EC-CUBE/ec-cube
Length of output: 470
🏁 Script executed:
# Search for searchData population and id field handling
rg "searchData\['id'\]" -A 2 -B 2 src/Eccube/Repository: EC-CUBE/ec-cube
Length of output: 2114
🏁 Script executed:
# Check if StringUtil class has isNotBlank method
rg "isNotBlank" src/Eccube/Repository: EC-CUBE/ec-cube
Length of output: 15679
🏁 Script executed:
# Look for Repository class that contains countBySearchDataForAdmin
find src/Eccube -name "*ProductRepository*" -o -name "*Repository.php" | head -20Repository: EC-CUBE/ec-cube
Length of output: 997
🏁 Script executed:
# Find the countBySearchDataForAdmin method in ProductRepository
rg "def countBySearchDataForAdmin|function countBySearchDataForAdmin" -A 50 src/Eccube/Repository/ProductRepository.phpRepository: EC-CUBE/ec-cube
Length of output: 2291
🏁 Script executed:
# Get the full context of both search methods (regular and count) in ProductRepository
sed -n '1,100p' src/Eccube/Repository/ProductRepository.php | head -40Repository: EC-CUBE/ec-cube
Length of output: 1041
🏁 Script executed:
# Search for the regular search method and countBySearchDataForAdmin to compare id handling
rg "public function.*SearchData" -A 3 src/Eccube/Repository/ProductRepository.phpRepository: EC-CUBE/ec-cube
Length of output: 540
🏁 Script executed:
# Get both methods side by side - search first
sed -n '/public function.*SearchDataForAdmin.*{/,/^ }/p' src/Eccube/Repository/ProductRepository.php | head -100Repository: EC-CUBE/ec-cube
Length of output: 41
🏁 Script executed:
# Look for countBySearchDataForAdmin specifically
rg "countBySearchDataForAdmin" -A 80 src/Eccube/Repository/ProductRepository.php | head -100Repository: EC-CUBE/ec-cube
Length of output: 3786
🏁 Script executed:
# Find getQueryBuilderBySearchDataForAdmin method to compare id handling
rg "public function getQueryBuilderBySearchDataForAdmin" -A 150 src/Eccube/Repository/ProductRepository.php | head -200Repository: EC-CUBE/ec-cube
Length of output: 6458
🏁 Script executed:
# Look specifically for the id field handling in getQueryBuilderBySearchDataForAdmin
rg "getQueryBuilderBySearchDataForAdmin" -A 200 src/Eccube/Repository/ProductRepository.php | grep -A 15 "// id"Repository: EC-CUBE/ec-cube
Length of output: 1478
カスタムカウント適用判定の不正確さ
useCustomCount の判定に id フィールドが含まれていないため、id 検索時に常にカスタムカウントが使用されます。しかし countBySearchDataForAdmin では id 検索を処理しており、さらに主クエリの id 検索は pc.code も対象に含みますが、カウント用メソッドでは除外されています。また !isset() ベースの判定では空文字列でも高速化が適用されず、不要に JOIN が実行されます。
修正例
+use Eccube\Util\StringUtil;
@@
- $useCustomCount = !isset($searchData['category_id'])
- && !isset($searchData['stock_status'])
- && !isset($searchData['stock'])
- && !isset($searchData['tag_id']);
+ $useCustomCount = empty($searchData['category_id'])
+ && !isset($searchData['stock_status']) // 0 は有効値
+ && empty($searchData['stock'])
+ && empty($searchData['tag_id'])
+ && !StringUtil::isNotBlank($searchData['id'] ?? null);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Eccube/Controller/Admin/Product/ProductController.php` around lines 268 -
273, The custom-count flag useCustomCount is computed incorrectly: add 'id' to
the fields that disable custom count and treat empty strings as "not provided"
so empty inputs don't force JOINs; update the check that sets $useCustomCount
(used around ProductController::... in this diff) to also consider
$searchData['id'] and use a trimmed-empty check (e.g. isset(...) &&
trim((string)$searchData['id']) !== '') for each searchable key so that when an
id search is present (and non-empty) the code does not select the custom path
(countBySearchDataForAdmin handles id) and when inputs are empty strings the
fast-path custom count remains allowed.
Comment on lines
+274
to
+286
| if ($useCustomCount) { | ||
| // カスタムカウントを使用して高速化 | ||
| $count = $this->productRepository->countBySearchDataForAdmin($searchData); | ||
| $query = $qb->getQuery(); | ||
| $query->setHint('knp_paginator.count', $count); | ||
|
|
||
| $pagination = $paginator->paginate( | ||
| $query, | ||
| $page_no, | ||
| $page_count, | ||
| $paginate_options | ||
| ); | ||
| } else { |
There was a problem hiding this comment.
イベント拡張後の qb と count が乖離します
ADMIN_PRODUCT_INDEX_SEARCH で qb が変更されても、count は $searchData から別計算です。イベントで条件追加される環境だとページング総件数がずれます。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Eccube/Controller/Admin/Product/ProductController.php` around lines 274 -
286, The paging count diverges because $count is computed from $searchData via
productRepository->countBySearchDataForAdmin while $qb may be modified by the
ADMIN_PRODUCT_INDEX_SEARCH event; change the code to derive the count from the
same QueryBuilder ($qb) after the event instead of from $searchData. Concretely,
either call a repository method that accepts the modified $qb (or add one, e.g.,
countByQueryBuilder(QueryBuilder $qb)) or compute the total with a cloned $qb
(clone $qb, replace select with COUNT(...) and getSingleScalarResult()) and pass
that value into $query->setHint('knp_paginator.count', $count) so the paginator
uses a count consistent with the modified $qb.
Contributor
|
@nobuhiko |
- Add customize() calls to all count methods (CustomerRepository, OrderRepository, ProductRepository) to apply QueryCustomizer conditions from plugins - ProductRepository: add pc.visible and pc.code conditions via EXISTS subquery to match listing query without JOIN overhead - Controllers: replace isset() with StringUtil::isNotBlank()/empty() for proper empty value handling in useCustomCount checks Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Eccube/Repository/CustomerRepository.php (1)
446-624: カスタマイズクエリの適用が正しく実装されています
$this->queries->customize(QueryKey::CUSTOMER_SEARCH, $qb, $searchData)の呼び出しにより、プラグインによる拡張条件がカウントクエリにも適用されるようになっています。これは以前のレビューで指摘された問題を解決しています。ただし、
getQueryBuilderBySearchDataとcountBySearchDataで検索条件のロジックが重複しています。将来の保守性を考慮すると、共通のフィルタ適用ロジックを抽出することを検討してください。また、PHPの型宣言として戻り値の型
intを明示することを推奨します。戻り値の型宣言追加の提案
- public function countBySearchData($searchData) + public function countBySearchData($searchData): int🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Eccube/Repository/CustomerRepository.php` around lines 446 - 624, The countBySearchData method duplicates filtering logic present in getQueryBuilderBySearchData; extract the shared filter application into a new private method (e.g., applySearchFilters(QueryBuilder $qb, array $searchData) or similar) that applies all "multi", pref, sex, birth, phone_number, buy_total/times, create/update/last_buy, status filters and calls $this->queries->customize(...), then reuse that helper from both getQueryBuilderBySearchData and countBySearchData to remove duplication; also add the PHP return type declaration "int" to the countBySearchData method signature to explicitly type the return value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/Eccube/Repository/CustomerRepository.php`:
- Around line 446-624: The countBySearchData method duplicates filtering logic
present in getQueryBuilderBySearchData; extract the shared filter application
into a new private method (e.g., applySearchFilters(QueryBuilder $qb, array
$searchData) or similar) that applies all "multi", pref, sex, birth,
phone_number, buy_total/times, create/update/last_buy, status filters and calls
$this->queries->customize(...), then reuse that helper from both
getQueryBuilderBySearchData and countBySearchData to remove duplication; also
add the PHP return type declaration "int" to the countBySearchData method
signature to explicitly type the return value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 73d1e5f9-6724-4e9d-a6cd-9397a5a3ad49
📒 Files selected for processing (6)
src/Eccube/Controller/Admin/Customer/CustomerController.phpsrc/Eccube/Controller/Admin/Order/OrderController.phpsrc/Eccube/Controller/Admin/Product/ProductController.phpsrc/Eccube/Repository/CustomerRepository.phpsrc/Eccube/Repository/OrderRepository.phpsrc/Eccube/Repository/ProductRepository.php
🚧 Files skipped from review as they are similar to previous changes (2)
- src/Eccube/Repository/OrderRepository.php
- src/Eccube/Repository/ProductRepository.php
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/Eccube/Controller/Admin/Customer/CustomerController.php (1)
176-180:⚠️ Potential issue | 🟠 Majorイベント後の
qbと件数計算がずれていますここで件数を
$searchDataベースで再計算しているため、ADMIN_CUSTOMER_INDEX_SEARCHでqbに条件が追加されると総件数だけが古い条件のままになります。プラグインやカスタマイズ経由で検索条件を足した場合、一覧件数や最終ページが実データと一致しません。件数はイベント適用後のqbから導出するか、少なくともqbが拡張され得る経路ではこの最適化を無効化した方が安全です。修正案
- $count = $this->customerRepository->countBySearchData($searchData); + $countQb = clone $qb; + $countQb->resetDQLPart('orderBy'); + $countQb->select('COUNT(DISTINCT c.id)'); + $count = (int) $countQb->getQuery()->getSingleScalarResult();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Eccube/Controller/Admin/Customer/CustomerController.php` around lines 176 - 180, Summary: The count is computed from $searchData before event listeners may modify $qb, causing stale total counts when ADMIN_CUSTOMER_INDEX_SEARCH or plugins add criteria. Fix: stop using customerRepository->countBySearchData($searchData) when useCustomCount is true; instead derive the total from the final query builder ($qb) after event dispatch (the same $qb used to build the list) or disable the optimization path where $qb can be extended. Concretely, update the logic around useCustomCount / customerRepository->countBySearchData and the block that calls $qb->getQuery() / setHint('knp_paginator.count', $count) so the count is computed from the event-modified $qb (or skip setting the knp_paginator.count hint when the ADMIN_CUSTOMER_INDEX_SEARCH event could alter $qb).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/Eccube/Controller/Admin/Customer/CustomerController.php`:
- Around line 176-180: Summary: The count is computed from $searchData before
event listeners may modify $qb, causing stale total counts when
ADMIN_CUSTOMER_INDEX_SEARCH or plugins add criteria. Fix: stop using
customerRepository->countBySearchData($searchData) when useCustomCount is true;
instead derive the total from the final query builder ($qb) after event dispatch
(the same $qb used to build the list) or disable the optimization path where $qb
can be extended. Concretely, update the logic around useCustomCount /
customerRepository->countBySearchData and the block that calls $qb->getQuery() /
setHint('knp_paginator.count', $count) so the count is computed from the
event-modified $qb (or skip setting the knp_paginator.count hint when the
ADMIN_CUSTOMER_INDEX_SEARCH event could alter $qb).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0ec44988-9998-498b-8bcd-053603432a5a
📒 Files selected for processing (1)
src/Eccube/Controller/Admin/Customer/CustomerController.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
管理画面の受注/商品/会員一覧ページにおいて、データ量増加に伴う表示遅延を解決します。
Fixes #6527
問題
KnpPaginatorが自動実行する
COUNT(DISTINCT)処理による性能問題:COUNT(DISTINCT o.id)で3万行→1万件に戻すDISTINCT処理解決策
COUNT用クエリとデータ取得用クエリを分離することで、根本的にパフォーマンスを改善:
従来の実装
改善後の実装
変更内容
1. Repository層
3つのRepositoryにカスタムカウントメソッドを追加:
OrderRepository::countBySearchDataForAdmin()ProductRepository::countBySearchDataForAdmin()CustomerRepository::countBySearchData()これらのメソッドは:
2. Controller層
3つのControllerでカスタムカウントを使用:
条件分岐の理由
一部の検索条件（商品名検索など）はJOINが不可避なため、その場合は従来のCOUNT処理を使用。基本的な検索条件のみの場合にカスタムカウントで高速化します。
期待される効果
定量的効果
対象ケース
テスト結果
実行したテスト
セキュリティチェック
チェックリスト
Summary by CodeRabbit