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>
Right now we are using both to test the Rails applications we generate
and to test Rails itself. Let's keep CI for the app and BUILDKITE to
the framework.
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.
This commit ignores the sqlite3 database file under the `activerecord/test/storage/`.
- Steps to reproduce
```ruby
cd activerecord
bundle update sdoc --conservative # not relevant to this issue, just to update sdoc
bin/test test/cases/schema_dumper_test.rb -n test_do_not_dump_foreign_keys_when_bypassed_by_config
git status
```
- Without this fix, `git status` shows `test/storage/` as `Untracked files`.
Here `Gemfile.lock` difference is due to `bundle update sdoc --conservative`
```ruby
$ git status
On branch main
You are currently bisecting, started from branch 'main'.
(use "git bisect reset" to get back to the original branch)
Changes not staged for commit:
(use "git add <file>..." to update what will be committed)
(use "git restore <file>..." to discard changes in working directory)
modified: ../Gemfile.lock
Untracked files:
(use "git add <file>..." to include in what will be committed)
test/storage/
no changes added to commit (use "git add" and/or "git commit -a")
$
```
- With this fix, `git status` does not show any `Untracked files`.
```
$ git status
On branch add_gitignore_test_storage
You are currently bisecting, started from branch 'main'.
(use "git bisect reset" to get back to the original branch)
Changes not staged for commit:
(use "git add <file>..." to update what will be committed)
(use "git restore <file>..." to discard changes in working directory)
modified: ../Gemfile.lock
no changes added to commit (use "git add" and/or "git commit -a")
$
```
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.
It's possible since Rails 6 (3ea2857943dc294d7809930b4cc5b318b9c39577) to let the framework create Event objects, but the guides and docs weren't updated to lead with this example.
Manually instantiating an Event doesn't record CPU time and allocations, I've seen it more than once that people copy-pasting the example code get confused about these stats returning 0. The tests here show that - just like the apps I've worked on - the old pattern keeps getting copy-pasted.
- 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.
