Skip to content

Implement typeTinyint for SQLite grammar#226

Open
Mrkbingham wants to merge 3 commits intowintercms:wip/1.3from
Mrkbingham:tiny_int_sql_lite_grammar
Open

Implement typeTinyint for SQLite grammar#226
Mrkbingham wants to merge 3 commits intowintercms:wip/1.3from
Mrkbingham:tiny_int_sql_lite_grammar

Conversation

@Mrkbingham
Copy link

@Mrkbingham Mrkbingham commented Mar 19, 2026

The compileChange override in vendor/winter/storm/src/Database/Schema/Grammars/SQLiteGrammar.php reads SQLite column type names as tinyint (from SQLite metadata for boolean columns), then calls getType() which looks for ->typeTinyint(... — but only typeTinyInteger exists in the base grammar file Illuminate\Database\Schema\Grammars\SQLiteGrammar.

This should resolve migration issues others might hit when upgrading to 1.3

Summary by CodeRabbit

  • Bug Fixes

    • SQLite schema operations now correctly handle tinyint column types, eliminating errors during column introspection and reconstruction.
  • Tests

    • Added validation tests for tinyint column type mapping in SQLite, ensuring proper SQL generation for schema operations.

Add typeTinyint method to handle tinyint type in SQLite.
@coderabbitai
Copy link

coderabbitai bot commented Mar 19, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 288823c7-dc68-4bed-9586-ef1869e17cb5

📥 Commits

Reviewing files that changed from the base of the PR and between 9d0e946 and e9afac0.

📒 Files selected for processing (2)
  • src/Database/Schema/Grammars/SQLiteGrammar.php
  • tests/Database/Schema/Grammars/SQLiteSchemaGrammarTest.php

Walkthrough

The changes introduce support for handling tinyint column types in SQLite schema grammar by adding a type alias method and corresponding test validation. A new typeTinyint() method in SQLiteGrammar maps tinyint to integer type to prevent BadMethodCallException during schema introspection and column re-creation.

Changes

Cohort / File(s) Summary
SQLiteGrammar Type Support
src/Database/Schema/Grammars/SQLiteGrammar.php
Added typeTinyint() protected method that returns 'integer' to handle tinyint column type aliasing during schema introspection and compilation.
SQLiteSchemaGrammarTest Updates
tests/Database/Schema/Grammars/SQLiteSchemaGrammarTest.php
Updated class inheritance to use unqualified GrammarTestCase, added imports for Blueprint and SQLiteConnection, and introduced testTypeTinyintTypeIsValid() test method to validate tinyint-to-integer type mapping in SQL generation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: implementing a typeTinyint method for SQLite grammar to handle tinyint column types during schema compilation.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

CodeRabbit can use your project's PHP CodeSniffer (phpcs) configuration to improve the quality of PHP code reviews.

Add a phpcs.xml or phpcs.xml.dist file to your project to customize how CodeRabbit runs phpcs. See PHP CodeSniffer documentation for more details.

@mjauvin mjauvin self-requested a review March 19, 2026 17:52
@mjauvin mjauvin added maintenance PRs that fix bugs, are translation changes or make only minor changes needs test case Issues/PRs that need a test case to be implemented labels Mar 19, 2026
@mjauvin mjauvin added this to the 1.3.0 milestone Mar 19, 2026
@mjauvin
Copy link
Member

mjauvin commented Mar 19, 2026

Would it be possible to submit a test case for this?

@Mrkbingham
Copy link
Author

@mjauvin added a test!

@mjauvin mjauvin removed the needs test case Issues/PRs that need a test case to be implemented label Mar 19, 2026
@mjauvin
Copy link
Member

mjauvin commented Mar 19, 2026

Thanks for adding the unit-test, however if I remove the typeTinyHint() method the test still passes.

@LukeTowers
Copy link
Member

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Mar 20, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@Mrkbingham
Copy link
Author

@mjauvin I went back and double-checked, I'm still getting this error when running PHPUnit tests in my WinterCMS app as well as the test case I wrote here. I'm running SQLite v3.51.0, not sure if it's a version issue.

I suppose as an alternative to lend more support downwards into Laravel rather than patching it here could be to just add a match statement on line 47:

'type' => match($column['type_name']) {                                                                                                                                    
      'tinyint'  => 'tinyInteger',
      default    => $column['type_name'],                                                                                                                                    
  }, 

Happy to provide more details if there's anything you're curious about here. Also thanks for getting Coderabbit to review this @LukeTowers !

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

Labels

maintenance PRs that fix bugs, are translation changes or make only minor changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants