Skip to content

fix: SQLSRV driver's decrement() method#10155

Open
patel-vansh wants to merge 5 commits intocodeigniter4:developfrom
patel-vansh:fix/sqlsrv-decrement-bug
Open

fix: SQLSRV driver's decrement() method#10155
patel-vansh wants to merge 5 commits intocodeigniter4:developfrom
patel-vansh:fix/sqlsrv-decrement-bug

Conversation

@patel-vansh
Copy link
Copy Markdown
Contributor

Description
As discovered by @paulbalandan in this comment, SQLSRV Database Driver was incorrectly adding instead of subtracting the decrement value when $castTextToInt was false.

This PR fixes that bug by flipping the sign in decrement() function.

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

@patel-vansh
Copy link
Copy Markdown
Contributor Author

patel-vansh commented May 3, 2026

I am quite unsure about the tests in this case. Do I need to add a new file in tests/system/Database/Live/SQLSRV directory? Or add a particular test inside the normal IncrementTest.php file?

@michalsn
Copy link
Copy Markdown
Member

michalsn commented May 4, 2026

The new class under the SQLSRV directory would be better. Please add tests for both increment() and decrement() with castTextToInt = false. You can use the existing job table and its integer created_at field for this.

@patel-vansh
Copy link
Copy Markdown
Contributor Author

The new class under the SQLSRV directory would be better. Please add tests for both increment() and decrement() with castTextToInt = false. You can use the existing job table and its integer created_at field for this.

Idk if I've added the tests as expected, as I didn't found any direct option to modify castTextToInt. Also, the assertInstanceOf assertion is to prevent PHPStan from complaining.

Comment thread tests/system/Database/Live/SQLSRV/IncrementTest.php Outdated
patel-vansh and others added 2 commits May 5, 2026 19:15
Co-authored-by: Michal Sniatala <michal@sniatala.pl>
@patel-vansh patel-vansh marked this pull request as ready for review May 5, 2026 14:11
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