Sqlite #1669
Merged
Sqlite #1669
Conversation
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
- UUID support - Schema compatibility layer, ignore MySQL specific options - Search not working yet
Instead we'll compute the table name dynamically based on Current.account where needed. Also we'll prevent searchable records from being saved if Current.account is not set, otherwise the after commit callbacks will fail.
Needs some special handling for the FTS virtual table format. Set the primary key to rowid and manually specify the columns.
jorgemanrubia
approved these changes
Nov 22, 2025
Member
jorgemanrubia
left a comment
jorgemanrubia
left a comment
There was a problem hiding this comment.
Fantastic @djmb 👏. I will start my work to move private stuff out forking this one.
5 tasks
flavorjones
reviewed
Nov 24, 2025
app/models/search/record/trilogy.rb
Outdated
Comment on lines
16
to
22
| def compute_table_name | ||
| if Current.account | ||
| "search_records_#{shard_id_for_account(Current.account.id)}" | ||
| else | ||
| "search_records_0" | ||
| end | ||
| end |
Member
There was a problem hiding this comment.
I'm not sure I understand how this works in practice. It looks like Rails memoizes the table name in a class instance variable, when does this method get called?
I think I'd like to not have to rely on the presence of Current.account if we can avoid it. Since the Searchable models all have an account_id column, is there a way we can switch on that?
Contributor
Author
There was a problem hiding this comment.
Yeah you are right, it doesn't work properly.
Reverted back to the previous way of selecting the search record class in b571303
flavorjones
reviewed
Nov 24, 2025
flavorjones
reviewed
Nov 24, 2025
It doesn't actually work, and even if we could make it work reliably we are better off if the records always know to go to the right shard. It does make the interface a bit more complicated as we need to select the right shard class with `for(account_id)`.
Co-authored-by: Mike Dalessio <mike.dalessio@gmail.com>
Co-authored-by: Mike Dalessio <mike.dalessio@gmail.com>
To ensure consistent column lengths, we'll add limits to string and text columns which match MySQL defaults.
SQLite columns lengths are purely informational, so we'll enforce the limits via a concern that checks the lengths from the schema.
Patch the sqlite adapter to add CHECK constraints for string and text column limits. We'll do them inline, so that any column changes automatically update the constraints.
* main: (116 commits) Ensure avatar thumbnails are square Update useragent to recognize twitterbot/facebot Add defensive styles for non-square avatar images Update test for copy changes Missed commit AI: standardize on https://agents.md Make it clear this is just notifications, not comprehensive activity AI: configure MCP servers for Chrome, Grafana, and Sentry (#1727) Allow requests from Google Image Proxy Update to basecamp's useragent fork Clean up a little bit the CSRF reporting code Claude: production observability guidance (#1725) Prevent autoscroll to the root columns container to prevent jump on page load Include full name string so you can type your name to filter Prioritize current user and assigned users in assignment dropdown Check and report on Sec-Fetch-Site header for forgery protection bundle update Bump bootsnap from 1.18.6 to 1.19.0 Bump rails from `077c3ad` to `17f6e00` Fix cards getting stuck in edit mode ...
flavorjones
reviewed
Nov 26, 2025
Comment on lines
+44
to
+62
| module SQLiteColumnLimitCheckConstraints | ||
| def add_column_options!(sql, options) | ||
| super | ||
|
|
||
| column = options[:column] | ||
| if column && column.limit && %i[string text].include?(column.type) | ||
| check_expr = if column.type == :string | ||
| # VARCHAR limits are in characters | ||
| %(length("#{column.name}") <= #{column.limit}) | ||
| else | ||
| # TEXT limits are in bytes | ||
| %(length(CAST("#{column.name}" AS BLOB)) <= #{column.limit}) | ||
| end | ||
| sql << " CHECK(#{check_expr})" | ||
| end | ||
|
|
||
| sql | ||
| end | ||
| end |
Member
There was a problem hiding this comment.
@djmb Oooh, nice. Are you interested in upstreaming this into Rails? Seems like a good option to have, even if it's turned off by default.
Contributor
Author
There was a problem hiding this comment.
Would you see this as something you enable via the options for the SQLite adapter in database.yml?
Couple of thoughts.
- You can already do this manually via
add_check_constraint. But it does seem like something that it would be nice to do by default. - Changing that setting if we had one could have weird behaviour - migrations would do different things if run before or after changing it. Loading the schema as well unless we specifically stored whether a column had the check constraint.
Maybe it would need to be an option to create_table? I guess we could still have a single setting that controls whether it gets added to new migrations by default.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Run the app with SQLite by setting DATABASE_ADAPTER=sqlite.