This commit fixes generated SQL for `UNION` and `UNION ALL` involving `LIMIT`
or `ORDER BY` by wrapping `SELECT` statements appearing in an `UNION` or
`UNION ALL` in parentheses.
Similar to 50515fb45f36dfad067adbdda9fee41fcb326ca9, make sure we
require `ostruct` where we use `OpenStruct`, to get the build back to
green, while we work to remove its usage on tests. (see #51510.)
Sample error:
```
Error:
ActiveRecord::Encryption::ConfigurableTest#test_installing_autofiltered_parameters_will_add_the_encrypted_attribute_as_a_filter_parameter_using_the_dot_notation:
NameError: uninitialized constant ActiveRecord::Encryption::ConfigurableTest::OpenStruct
test/cases/encryption/configurable_test.rb:45:in `block in <class:ConfigurableTest>'
```
Fix: https://github.com/rails/rails/issues/51386
This significantly reduce the depth of the tree for large `OR`
conditions. I was initially a bit on the fence about that fix,
but given that `And` is already implemented this way, I see no
reasons not to do the same.
Amusingly, the reported repro script now makes SQLite fail:
```ruby
SQLite3::SQLException: Expression tree is too large (maximum depth 1000)
```
Ref: https://github.com/rails/rails/pull/26103
Ref: https://github.com/rails/rails/pull/51426
A fairly common mistake with Rails is to enqueue a job from inside a
transaction, and a record as argumemnt, which then lead to a RecordNotFound
error when picked up by the queue.
This is even one of the arguments advanced for job runners backed by the
database such as `solid_queue`, `delayed_job` or `good_job`. But relying
on this is undesirable iin my opinion as it makes the Active Job abstraction
leaky, and if in the future you need to migrate to another backend or even
just move the queue to a separate database, you may experience a lot of race
conditions of the sort.
But more generally, being able to defer work to after the current transaction
has been a missing feature of Active Record. Right now the only way to do it
is from a model callback, and this forces moving things in Active Record
models that sometimes are better done elsewhere. Even as a self-proclaimed
"service object skeptic", I often wanted this capability over the last decade,
and I'm sure it got asked or desired by many more people.
Also there's some 3rd party gems adding this capability using monkey patches.
It's not a reason to upstream the capability, but it's a proof that there is
demand for it.
Implementation wise, this proof of concept shows that it's not really hard to
implement, even with nested multi-db transactions support.
Co-Authored-By: Cristian Bica <cristian.bica@gmail.com>
We had to revert rails/rails@6dd1929 due to some regressions it caused. Here are some tests that would prevent those regressions in the future. See previous commits for more detail.
This reverts commit 6dd1929b04de42cdff1264bb5618c7f6abe77a9c.
This introduced a change in behaviour since type_for_attribute is aware of custom types but lookup_cast_type does not.
Additionally, since we no longer to use the table and column info to get attributes, this introduced an issue where attribute types were not be correctly found for some queries, where we were joining a table that has a different name than the name of the reflection for that association.
This commit makes two types of queries retry-able by opting into our `allow_retry` flag:
1) SELECT queries we construct by walking the Arel tree via `#to_sql_and_binds`. We use a
new `retryable` attribute on collector classes, which defaults to true for most node types,
but will be set to false for non-idempotent node types (functions, SQL literals, etc). The
`retryable` value is returned from `#to_sql_and_binds` and used by `#select_all` and
passed down the call stack, eventually reaching the adapter's `#internal_exec_query` method.
Internally-generated SQL literals are marked as retryable via a new `retryable` attribute on
`Arel::Nodes::SqlLiteral`.
2) `#find` and `#find_by` queries with known attributes. We set `allow_retry: true` in `#cached_find_by`,
and pass this down to `#find_by_sql` and `#_query_by_sql`.
These changes ensure that queries we know are safe to retry can be retried automatically.
Some association options, e.g. `through`, were only added to
`.valid_options` list only if they were provided. It means that
sometimes ArgumentError's message would not mention all the
possible options.
Ref: https://github.com/rails/rails/pull/50284
While having the inverse association configured it generally positive
as it avoid some extra queries etc, infering it may break legecy code,
as evidenced by how it broke `ActiveStorage::Blob` in https://github.com/rails/rails/pull/50800
As such we can't just enable this behavior immediately, we need to provide
and upgrade path for users.
When `.connection` is called inside a `.with_connection` block
it cause the lease to become permanent (or sticky). This is to
ensure that legacy code that may hold onto this connection
won't cause thread safety issues.
However if you autidted the code that calls `.connection` and
know for sure it won't hold into the returned connection,
you can wrap it with `with_connection(prevent_permanent_checkout: true)`
to prevent the lease from becoming permanent and ensure the connection
is released at the end of the block.
Controls whether `ActiveRecord::Base.connection` raises an error, emits a deprecation warning, or neither.
`ActiveRecord::Base.connection` checkouts a database connection from the pool and keep it leased until the end of
the request or job. This behavior can be undesirable in environments that use many more threads or fibers than there
is available connections.
This configuration can be used to track down and eliminate code that calls `ActiveRecord::Base.connection` and
migrate it to use `ActiveRecord::Base.with_connection` instead.
The default behavior remains unchanged, and there is currently no plans to change the default.
