Skip to content

refactor(database): optimize groupGetType by caching it inside BaseBuilder loops#10340

Open
gr8man wants to merge 1 commit into
codeigniter4:developfrom
gr8man:perf/optimize-basebuilder
Open

refactor(database): optimize groupGetType by caching it inside BaseBuilder loops#10340
gr8man wants to merge 1 commit into
codeigniter4:developfrom
gr8man:perf/optimize-basebuilder

Conversation

@gr8man

@gr8man gr8man commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Description
Inside BaseBuilder's whereHaving() and _like() methods, $this->groupGetType($type) was being called repeatedly inside foreach loops.

This pull request optimizes these methods by:

  1. Caching the $prefix outside the loops before iteration starts.
  2. Dynamically updating the $prefix to $type at the end of the loop iteration to correctly mimic subsequent query builder conditions.
  3. Unifying the separate logical paths in _like() for RawSql vs array inputs into a single loop using structured associative arrays (avoiding PHP TypeError since array keys cannot be RawSql object instances).

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value (without duplication)
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide
  • We don't add entries for refactoring, as there are no real changes for end users. (No changelog entry needed)

@michalsn michalsn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Points 1 and 2 make sense - not necessarily as a performance optimization, but as a code-quality improvement.

Please revert point 3, as it changes the existing RawSql behavior.

If PR claims a performance improvement, please provide a reproducible benchmark and results demonstrating it. Otherwise, please use the refactor prefix. Performance claims should be supported by evidence provided by the PR author.

@gr8man gr8man force-pushed the perf/optimize-basebuilder branch from f7a23f0 to 07d2e7c Compare June 25, 2026 20:31
@gr8man gr8man changed the title perf(database): optimize groupGetType by caching it inside BaseBuilder loops refactor(database): optimize groupGetType by caching it inside BaseBuilder loops Jun 25, 2026
@gr8man gr8man force-pushed the perf/optimize-basebuilder branch from 07d2e7c to 521a3e1 Compare June 25, 2026 20:41
Comment on lines +1161 to +1162
$prefix = $this->{$clause} === [] ? $this->groupGetType('') : $this->groupGetType($type);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs the same empty-array guard as whereHaving(). groupGetType() mutates the group state, so calling it before an empty loop can break grouped queries like groupStart()->like([])->where(...), producing invalid SQL with AND immediately after (. Please add a regression test for that case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants