When a `:method:` doc is immediately followed by the `private` keyword,
RDoc will hide that doc as if it were a private method.
In #48324, the `:method:` docs for `ActiveRecord::Core#slice` and
`ActiveRecord::Core#values_at` were swapped in an attempt to force
`values_at` to appear in the rendered docs. However, doing so causes
`slice` to dissappear from the rendered docs.
To ensure that `slice` and `values_at` both appear in the rendered docs,
this commit moves their `:method:` docs away from the `private` keyword
in `ActiveRecord::Core`.
Fixes#49502.
This bug was introduced in https://github.com/rails/rails/pull/48241. Since `IDENTITY` columns
in PostgreSQL do not have defaults, they are not auto populated via the changes made in that PR.
Thus we need to consider them separately.
The NullTransaction implements the null object pattern for the
Transaction class, but it was missing the `materialized?` method.
This caused issues in cases where we'd want to check if the current
transaction is materialized, where we'd need to check if the connection
is open first:
```ruby
current_transaction = ActiveRecord::Base.connection.current_transaction
current_transaction.open? && current_transaction.materialized?
```
With this change we can skip the extra check on `open?`
```ruby
current_transaction = ActiveRecord::Base.connection.current_transaction
current_transaction.materialized?
```
We noticed a couple of spots where transaction tracking events
were potentially incorrect.
When the connection reconnects with `restore_transactions: true`,
we were keeping the original start time for the transaction. In this case
it seems more accurate to treat these as separte transactions where the first
one finishes as incomplete.
Instead of forcing the adapter to tell the transaction manager to
suspend (or `detach` or `lost`) the transactions, we can keep that
logic encapsulated inside of the Transaction class for when to broadcast
a finish event. This means that we won't capture a finish event if you
manually call `reconnect!` without `restore_transactions: true`, but
that might be worthy of the tradeoff since this is a rare use-case anyway.
We also start raising here if the TransactionInstrumenter receives `#start`
when already started, or `#finish` when not started. This ensures that we don't
quietly end up in any weird states.
When marking that a transaction is incomplete, we also want to check
that the transaction is materialized to avoid finishing our instrumentation
if it hasn't already started. Also added a test to simulate losing a connection
without ever materializing a transaction.
Co-authored-by: Daniel Colson <composerinteralia@github.com>
Some protections like the one that checks if an enum is pointing to
a valid column in the table only works when the database schema is
loaded to the model.
Before this change, if schema cache wasn't present and the right
combinations of configurations were not set, developers would only see
this exception in production.
With this change, those errors would be caught on CI, as soon the
tests are loaded.
Fix: https://github.com/rails/rails/issues/49439
Because Postgres adapter types are connection dependent,
we can't lookup types without first connecting to the database.
Note, this really isn't good, ideally types should be stored in the
schema cache so that we don't have to extract types every time.
- An oversight of #48615 is that it changes the `Rails.logger` to be
a broadcast logger after the app is booted. Anything referencing
`Rails.logger` during the boot process will get a simple logger and
ultimately resulting in logs not being broadcasted.
For example `ActionController::Base.logger.info("abc")` would
just output logs in the `development.log` file, not on STDOUT.
----
The only solution I could think of is to create a BroadcastLogger
earlier at boot, and add logger to that broadcast when needed (instead
of modiyfing the `Rails.logger` variable).
Ideally we should be able to checkout an Adapter instance without
triggering a connection to the server.
For some adapters like Trilogy that is the case today, but only if
you have a schema cache loaded, otherwise `check_version` will trigger
a query.
I think this check can be delayed to when we actually use the connection.
Ref: https://github.com/rails/rails/pull/49378
As discussed with Matthew Draper, we have a bit of a chicken and egg
problem with the schema cache and the database version.
The database version is stored in the cache to avoid a query,
but the schema cache need to query the schema version in the database
to be revalidated.
So `check_version` depends on `schema_cache`, which depends on
`Migrator.current_version`, which depends on `configure_connection`
which depends on `check_version`.
But ultimately, we think storing the server version in the cache
is incorrect, because upgrading a DB server is orthogonal from
regenerating the schema cache.
So not persisting the version in cache is better. Instead we store
it in the pool config, so that we only check it once per process
and per database.
There is no reason to send `primary_key` to the record to get the value
of the primary key. `id` method does exactly that and it's a better
abstraction. By implication it also fixes the problem with composite
primary keys.
This change would force a lot of existing applications and libraries
to update their tests.
We included it in the beta to collect feedback from the community and
we had some comments about how negative this change would be.
Developers that care about the typography of their error messages
can easily change it in their applications using the translation
files, so there is no need to inflict pain in the upgrade process
by changing the default in the framework.
Revert "Merge PR #45463"
This reverts commit 9f60cd8dc7d963b1843b66d9639715b4a04c9c65, reversing
changes made to 35d574dbfda68d09fe1fb532f45a3e32f14c571d.
Mysql automatically escapes quotes in generated check constraints expressions.
When Rails dumps the schema (into schema.rb) the generated schema also contains,
the quotes with duplicated escaping (\\\\').
To fix this, we remove the escaping, that MySQL provides from the fetched
expression.
As we (I and @yahonda) talked about the naming in person, naming unique
constraints as unique keys is very confusing to me.
All documents and descriptions says it's unique constraints, but naming
unique keys leads to misunderstanding it's a short-hand of unique
indexes.
Just naming it unique constraints is not misleading.
- ## Context
While working on https://github.com/rails/rails/pull/44695, I
realised that Broadcasting was still a private API, although it’s
commonly used. Rafael mentioned that making it public would require
some refactor because of the original implementation which was hard
to understand and maintain.
### Changing how broadcasting works:
Broadcasting in a nutshell worked by “transforming” an existing
logger into a broadcasted one.
The logger would then be responsible to log and format its own
messages as well as passing the message along to other logger it
broadcasts to.
The problem with this approach was the following:
- Heavy use of metaprogramming.
- Accessing the loggers in the broadcast wasn’t possible.
Removing a logger from the broadcast either.
- More importantly, modifying the main logger (the one that broadcasts
logs to the others) wasn’t possible and the main source of
misunderstanding.
```ruby
logger = Logger.new(STDOUT)
stderr_logger = Logger.new(STDER))
logger.extend(AS::Logger.broadcast(stderr_logger))
logger.level = DEBUG # This modifies the level on all other loggers
logger.formatter = … # Modified the formatter on all other loggers
```
To keep the contract unchanged on what Rails.logger returns, the new
BroadcastLogger class implement duck typing with all methods
that has the vanilla Ruby Logger class.
It's a simple and boring PORO that keeps an array of all the loggers
that are part of the broadcast and iterate over whenever a log is
sent.
Now, users can access all loggers inside the broadcast and modify
them on the fly. They can also remove any logger from the broadcast
at any time.
```ruby
# Before
stdout_logger = Logger.new(STDOUT)
stderr_logger = Logger.new(STDER)
file_logger = Logger.new(“development.log”)
stdout_logger.extend(AS::Logger.broadcast(stderr_logger))
stdout_logger.extend(AS::Logger.broadcast(file_logger))
# After
broadcast = BroadcastLogger.new(stdout_logger, stderr_logger, file_logger)
```
I also think that now, it should be more clear for users that the
broadcast sole job is to pass everything to the whole loggers in
the broadcast. So there should be no surprise that all loggers in
the broadcast get their level modified when they call
`broadcast.level = DEBUG` .
It’s also easier to wrap your head around more complex setup such
as broadcasting logs to another broadcast:
`broadcast.broadcast_to(stdout_logger, other_broadcast)`
For Rails applications, the Write-Ahead-Log in normal syncing mode with a capped journal size, a healthy shared memory buffer and a shared cache will perform, on average, 2× better.
Retrying busy connections without delay is a preferred practice for performance-sensitive applications. Add support for a `database.yml` `retries` integer, which is used in a simple `busy_handler` function to retry busy connections without exponential backoff up to the max number of `retries`
.
This was added back in 0.9.5 because at the time single associations (e.g. has_one)
didn't directly return the record but a proxy
https://github.com/rails/rails/commit/97849debf33
But toaward 3.1, we got rid of that proxy: 1644663ba7
and the associated test was deleted.
However the `===` method stayed.
This has been around for so long that removing it may
break some rare applications, but I doubt it would be hard to
fix, and this isn't a documented behavior.
Co-Authored-By: Nate Matykiewicz <natematykiewicz@gmail.com>
Implementing the full `supports_insert_returning?` contract means the SQLite3 adapter supports auto-populated columns (#48241) as well as custom primary keys.
