| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| ... | |
| |\ \ \ \ \ \ \
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
eileencodes/fix-query-cache-for-database-switching
Invalidate all query caches for current thread
|
| | | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
This change ensures that all query cahces are cleared across all
connections per handler for the current thread so if you write on one
connection the read will have the query cache cleared.
|
| |\ \ \ \ \ \ \ \
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | | |
eileencodes/allow-application-to-change-handler-names
Add ability to change the names of the default handlers
|
| | |/ / / / / / /
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
When I wrote the `connected_to` and `connects_to` API's I wrote them
with the idea in mind that it didn't really matter what the
handlers/roles were called as long as those connecting to the roles knew
which one wrote and which one read.
With the introduction of the middleware Rails begins to assume it's
`writing` and `reading` and there's no room for other roles. At GitHub
we've been using this method for a long time so we have a ton of legacy
code that uses different handler names `default` and `readonly`. We
could rename all our code but I think this is better for a few reasons:
- Legacy apps that have been using multiple databases for a long time
can have an eaiser time switching.
- If we later find this to cause more issues than it's worth we can
easily deprecate.
- We won't force old apps to rewrite the resolver middleware just to use
a different handler.
Adding the writing_role/reading_role required that I move the code that
creates the first handler for writing to the railtie. If I didn't move
this the core class would assign the handler before I was able to assign
a new one in my configuration and I'd end up with 3 handlers instead of
2.
|
| |\ \ \ \ \ \ \ \
| | | | | | | | |
| | | | | | | | | |
Hint at advanced options for foreign_key
|
| | | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | | |
We sometimes display simple examples of additional parameters that can be
supplied to table-wise methods like these and I found it particularly difficult
to figure out which options `t.foreign_key` accepts without drilling very deep
into the specific SchemaStatements docs.
Since it's relatively common to create foreign keys with custom column names or
primary keys, it seems like this should help quite a few people.
[ci skip]
|
| |\ \ \ \ \ \ \ \ \
| | | | | | | | | |
| | | | | | | | | | |
Refactor options for database selector middleware
|
| | | |/ / / / / / /
| |/| | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | | |
Right now we only have one option that's supported, the delay. However I
can see us supporting other options in the future.
This PR refactors the options to get passed into the resolver so whether
you're using middleware or using the config options you can pass options
to the resolver. This will also make it easy to add new options in the
future.
|
| |\ \ \ \ \ \ \ \ \
| |/ / / / / / / /
|/| | | | | | | | |
Eagerly materialize the fixtures transaction
|
| | | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | | |
The transaction used to restore fixtures is an implementation detail
that should be abstracted away. Idealy a test should behave the same
wether or not transactional fixtures are enabled.
However since transactions have been made lazy, the fixture
transaction started leaking into tests case. e.g. consider the
following (oversimplified) test:
```ruby
class SQLSubscriber
attr_accessor :sql
def initialize
@sql = []
end
def call(*, event)
sql << event[:sql]
end
end
subscriber = SQLSubscriber.new
ActiveSupport::Notifications.subscribe("sql.active_record", subscriber)
User.connection.execute('SELECT 1', 'Generic name')
assert_equal ['SELECT 1'], subscriber.sql
```
On Rails 6 it starts to break because the `sql` array will be `['BEGIN', 'SELECT 1']`.
Several things are wrong here:
- That transaction is not generated by the tested code, so it shouldn't be visible.
- The transaction is not even closed yet, which again doesn't reflect the reality.
|
| |\ \ \ \ \ \ \ \ \
| | | | | | | | | |
| | | | | | | | | | |
Fix has_many through association creation
|
| | | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | | |
This reverts commit ed1eda271c7ac82ecb7bd94b6fa1b0093e648a3e, reversing
changes made to 3d2caab7dc92a13d4dd369678d5b4ce659df8e52.
Reason: 7c3da6e0030aa080fcb89af58b094ed50d861a44
|
| | | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | | |
I deprecated two unused attr_writers `visitor` and `indexes` at 8056fe0
and f4bc364 conservatively, since those are accidentaly exposed in the
docs.
https://api.rubyonrails.org/v5.2/classes/ActiveRecord/ConnectionAdapters/AbstractAdapter.html
https://api.rubyonrails.org/v5.2/classes/ActiveRecord/ConnectionAdapters/TableDefinition.html
But I've found that `view_renderer` attr_writer is removed without
deprecation at #35093, that is also exposed in the doc.
https://api.rubyonrails.org/v5.2/classes/ActionView/Base.html
I'd like to also remove the deprecated attr_writers since I think that
removing `visitor` and `indexes` attr_writers is as safe as removing
`view_renderer` attr_writer.
|
| |/ / / / / / / / / |
|
| | | | | | | | | | |
|
| | | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | | |
We enabled `Style/RedundantBegin` cop at #34764, but it is hard to
detect an offence if returning value put after the block.
|
| | | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | | |
We need to update using the timestamp from the end of the request, not
the start. For example, if a request spends 5+ seconds writing, we still
want to wait another 5 seconds for replication lag.
Since we now run the update after we yield, we need to use ensure to
make sure we update the timestamp even if there is an exception.
|
| |\ \ \ \ \ \ \ \ \
| | | | | | | | | |
| | | | | | | | | | |
Part 8: Multi db improvements, Adds basic automatic database switching to Rails
|
| | |/ / / / / / / /
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | | |
The following PR adds behavior to Rails to allow an application to
automatically switch it's connection from the primary to the replica.
A request will be sent to the replica if:
* The request is a read request (`GET` or `HEAD`)
* AND It's been 2 seconds since the last write to the database (because
we don't want to send a user to a replica if the write hasn't made it
to the replica yet)
A request will be sent to the primary if:
* It's not a GET/HEAD request (ie is a POST, PATCH, etc)
* Has been less than 2 seconds since the last write to the database
The implementation that decides when to switch reads (the 2 seconds) is
"safe" to use in production but not recommended without adequate testing
with your infrastructure. At GitHub in addition to the a 5 second delay
we have a curcuit breaker that checks the replication delay
and will send the query to a replica before the 5 seconds has passed.
This is specific to our application and therefore not something Rails
should be doing for you. You'll need to test and implement more robust
handling of when to switch based on your infrastructure. The auto
switcher in Rails is meant to be a basic implementation / API that acts
as a guide for how to implement autoswitching.
The impementation here is meant to be strict enough that you know how to
implement your own resolver and operations classes but flexible enough
that we're not telling you how to do it.
The middleware is not included automatically and can be installed in
your application with the classes you want to use for the resolver and
operations passed in. If you don't pass any classes into the middleware
the Rails default Resolver and Session classes will be used.
The Resolver decides what parameters define when to
switch, Operations sets timestamps for the Resolver to read from. For
example you may want to use cookies instead of a session so you'd
implement a Resolver::Cookies class and pass that into the middleware
via configuration options.
```
config.active_record.database_selector = { delay: 2.seconds }
config.active_record.database_resolver = MyResolver
config.active_record.database_operations = MyResolver::MyCookies
```
Your classes can inherit from the existing classes and reimplment the
methods (or implement more methods) that you need to do the switching.
You only need to implement methods that you want to change. For example
if you wanted to set the session token for the last read from a replica
you would reimplement the `read_from_replica` method in your resolver
class and implement a method that updates a new timestamp in your
operations class.
|
| |/ / / / / / / /
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
Previously if the `url` key in a config hash was nil we'd ignore the
configuration as invalid. This can happen when you're relying on a
`DATABASE_URL` in the env and that is not set in the environment.
```
production:
<<: *default
url: ENV['DATABASE_URL']
```
This PR fixes that case by checking if there is a `url` key in the
config instead of checking if the `url` is not nil in the config.
In addition to changing the conditional we then need to build a url hash
to merge with the original hash in the `UrlConfig` object.
Fixes #35091
|
| |\ \ \ \ \ \ \ \
| | | | | | | | |
| | | | | | | | | |
MySQL: Support `:size` option to change text and blob size
|
| | | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | | |
In MySQL, the text column size is 65,535 bytes by default (1 GiB in
PostgreSQL). It is sometimes too short when people want to use a text
column, so they sometimes change the text size to mediumtext (16 MiB) or
longtext (4 GiB) by giving the `limit` option.
Unlike MySQL, PostgreSQL doesn't allow the `limit` option for a text
column (raises ERROR: type modifier is not allowed for type "text").
So `limit: 4294967295` (longtext) couldn't be used in Action Text.
I've allowed changing text and blob size without giving the `limit`
option, it prevents that migration failure on PostgreSQL.
|
| | | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | | |
This gets the PartialRenderer to be a bit closer to the
TemplateRenderer. TemplateRenderer already keeps its template in a
local variable.
|
| |/ / / / / / / /
| | | | | | | |
| | | | | | | |
| | | | | | | | |
Similar to 1853b0d0abf87dfdd4c3a277c3badb17ca19652e
|
| | | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
This improves performance of timestamp conversion and avoids
additional string allocations.
|
| | | | | | | | | |
|
| | | | | | | | | |
|
| | | | | | | | | |
|
| |\ \ \ \ \ \ \ \
| |/ / / / / / /
|/| | | | | | | |
Fix error raised when handler doesn't exist
|
| | | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
While working on another feature for multiple databases (auto-switching)
I observed that in development the first request won't autoload the
application record connection for the primary database and may not yet
know about the replica connection.
In my test application this caused the application to thrown an error if
I tried to send the first request to the replica before the replica was
connected. This wouldn't be an issue in production because the
application is preloaded.
In order to fix this I decided to leave the original error message and
delete the new error message. I updated the original error message to
include the `role` to make it a bit clearer that the connection isn't
established for that particular role.
The error now reads:
```
No connection pool with 'primary' found for the 'reading' role.
```
A single database application will continue uisng the original error
message:
```
No connection pool with 'primary' found.
```
|
| |/ / / / / / /
| | | | | | |
| | | | | | |
| | | | | | | |
Allows aliasing, predications, ordering, and various other functions on `And` and `Case` nodes. This brings them in line with other nodes like `Binary` and `Unary`.
|
| | | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
Currently `conn.column_exists?("testings", "created_at", "datetime")`
returns false even if the table has the `created_at` column.
That reason is that `column.type` is a symbol but passed `type` is not
normalized to symbol unlike `column_name`, it is surprising behavior to
me.
I've improved that to normalize a value before comparison.
|
| | | | | | | | |
|
| | | | | | | | |
|
| | | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
https://dev.mysql.com/doc/relnotes/mysql/8.0/en/news-8-0-14.html
> Error messages relating to creating and dropping foreign keys
> were improved to be more specific and informative. (Bug #28526309, Bug #92087)
https://dev.mysql.com/doc/refman/8.0/en/server-error-reference.html
> Error number: 3780; Symbol: ER_FK_INCOMPATIBLE_COLUMNS; SQLSTATE: HY000
> Message: Referencing column '%s' and referenced column '%s' in foreign key constraint '%s' are incompatible.
> ER_FK_INCOMPATIBLE_COLUMNS was added in 8.0.14.
|
| | | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
When `Arel` was merged into `ActiveRecord` we lost the ability to alias case nodes. This adds it back.
|
| | | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
Since #31230, `change_column` is executed as a bulk statement.
That caused incorrect type casting column default by looking up the
before changed type, not the after changed type.
In a bulk statement, we can't use `change_column_default_for_alter` if
the statement changes the column type.
This fixes the type casting to use the constructed target sql_type.
Fixes #34938.
|
| | | | | | | | |
|
| | | | | | | | |
|
| |\ \ \ \ \ \ \
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
kamipo/all_of_queries_should_return_correct_result
All of queries should return correct result even if including large number
|
| | | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
Since 31ffbf8d, finder methods no longer raise `RangeError`. So
`StatementCache#execute` is the only place to raise the exception for
finder queries.
`StatementCache` is used for simple equality queries in the codebase.
This means that if `StatementCache#execute` raises `RangeError`, the
result could always be regarded as empty.
So `StatementCache#execute` just return nil in that range error case,
and treat that as empty in the caller side, then we can avoid catching
the exception in much places.
|
| | | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
Currently several queries cannot return correct result due to incorrect
`RangeError` handling.
First example:
```ruby
assert_equal true, Topic.where(id: [1, 9223372036854775808]).exists?
assert_equal true, Topic.where.not(id: 9223372036854775808).exists?
```
The first example is obviously to be true, but currently it returns
false.
Second example:
```ruby
assert_equal topics(:first), Topic.where(id: 1..9223372036854775808).find(1)
```
The second example also should return the object, but currently it
raises `RecordNotFound`.
It can be seen from the examples, the queries including large number
assuming empty result is not always correct.
Therefore, This change handles `RangeError` to generate executable SQL
instead of raising `RangeError` to users to always return correct
result. By this change, it is no longer raised `RangeError` to users.
|
| |\ \ \ \ \ \ \ \
| |/ / / / / / /
|/| | | | | | | |
Fix error message when adapter is not specified
|
| | | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
When we added support for multiple databases through a 3-tiered config
and configuration objects this error message got a bit convoluted.
Previously if you had an application with a missing configuation and
multiple databases the error message would look like this:
```
'doesnexist' database is not configured. Available: development,
development, test, test, production, production
(ActiveRecord::AdapterNotSpecified)
```
That's not very descriptive since it duplicates the environments
(because there are multiple databases per environment for this
application).
To fix this I've constructed a bit more readable error message which now
reads like this if you have a multi db app:
```
The `doesntexist` database is not configured for the `production`
environment. (ActiveRecord::AdapterNotSpecified)
Available databases configurations are:
development: primary, primary_readonly
test: primary, primary_readonly
production: primary, primary_readonly
```
And like this if you have a single db app:
```
The `doesntexist` database is not configured for the `production`
environment. (ActiveRecord::AdapterNotSpecified)
Available databases configurations are:
development
test
```
This makes the error message more readable and presents the user all
available options for the database connections.
|
| | | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
The `unboundable?` behaves like the `infinite?`.
```ruby
inf = Topic.predicate_builder.build_bind_attribute(:id, Float::INFINITY)
inf.infinite? # => 1
oob = Topic.predicate_builder.build_bind_attribute(:id, 9999999999999999999999999999999)
oob.unboundable? # => 1
inf = Topic.predicate_builder.build_bind_attribute(:id, -Float::INFINITY)
inf.infinite? # => -1
oob = Topic.predicate_builder.build_bind_attribute(:id, -9999999999999999999999999999999)
oob.unboundable? # => -1
```
|
| |\ \ \ \ \ \ \ \
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | | |
dylanahsmith/better-composed-of-single-field-query
activerecord: Use a simpler query condition for aggregates with one mapping
|
| | | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | | |
methods by default
Co-Authored-By: dylanahsmith <dylan.smith@shopify.com>
|
| | | | | | | | | | |
|
| | |/ / / / / / / |
|
| | | | | | | | | |
|
