Using `ActiveModel::AttributeMethods#[]` or `AttributeMethods#read_attribute` is
not suitable when association involves a model with `id` column which is
not a whole primary key but still used as a foreign key. Public reader
treats `id` as an idenfitier and thus returns value for the primary key
column and not the `id` column. By using `_read_attribute` reader we can
ensure we are reading the column value directly.
Co-authored-by: Paarth Madan <paarth.madan@shopify.com>
At GitHub we're trying to switch over from the GitHub
activerecord-trilogy-adapter to the one in Rails. One difference between
the adapters is that the GitHub one did not have the `#select_all`
method that abandons multi results.
This wouldn't be a problem, except we also have more aggressive query
retries. This led to an issue because the `super` call in `#select_all`
leads to a nested `#with_raw_connection` call. If we experienced
transient connection errors during the query, the inner
`#with_raw_connection` could reconnect leaving the outer block with a
closed connection. In that case, calling `#more_results_exist?` results
in a `Trilogy::ConnectionClosed Attempted to use closed connection`.
That's the scenario described in this comment
7e735451a7/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb (L981-L983)
Moving the call to `super` out of the `#with_raw_connection` block will
avoid the nesting and fix the issue. We prefer that solution over
detecting nested `#with_raw_connections` and preventing reconnects when
nested, since in our case we actually do want to reconnect and retry.
We're hoping this change is ok even though it's not strictly necessary
for general Rails users. We don't believe it has any downsides.
We did not add a test for this case since it's not possible to occur in
Rails itself. It's specific to custom GitHub code.
Co-authored-by: Daniel Colson <composerinteralia@github.com>
This test was added in 8f5095a to ensure that there was coverage
for `AbstractMysqlAdapter#execute`, but 63c0d6b refactored `#execute`
to be defined at the Abstract adapter level, and rely on concrete MySQL
adapters to implement `#raw_execute`.
This test won't pass without having the `ExampleMysqlAdapter` implement
`#raw_execute`, but this test is obsolete now that AbstractMysql2Adapter
doesn't implement `#execute` directly.
This deals with a problem introduced in #7743ab95b8e15581f432206245c691434a3993d1a751b9d451170956d59457a9R8
that was preventing query `Class` serialized attributes. Duplicating the original
`Class` argument generates an anonymous class that can't be serialized as YAML.
This change makes query attributes hasheable based on their frozen casted values
to prevent the problem.
This solution is based on an idea by @matthewd from https://github.com/rails/rails/issues/47338#issuecomment-1424402777.
The documentation uses the default pluralized model name with
the _count suffix (eg posts_count). However, the documentation
examples for #update_counters deviates from this.
Use the default-style counter column names in the example code
for #update_counters, to be consistent.
Ruby 3.1 added `intersects?` which is equivalent to `(a & b).any?`. Rubocop added a corresponding cop, `Style/ArrayIntersect`, which transforms the old style to use `intersect?`. Unfortunately as `intersects?` is not delegated on `CollectionProxy`, this leads to false positives that need to be disabled for no good reason other than the fact the method isn't delegated.
This PR add delegation of `intersects?` to `Relation` which fixes this.
`deferrable: true` is deprecated in favor of `deferrable: :immediate`, and
will be removed in Rails 7.2.
Because `deferrable: true` and `deferrable: :deferred` are hard to understand.
Both true and :deferred are truthy values.
This behavior is the same as the deferrable option of the add_unique_key method, added in #46192.
*Hiroyuki Ishii*
It's public API and we can't assume whether the query is read only
so we should clear the cache.
To perform read only queries, `select` and `select_all` can be used.
`#execute` is a public method and using it internaly forces us to expose
private arguments and other dirty things.
Other adapters don't do this, so this bring MySQL in line with others.
The trilogy test suite was imported directly, but many of
the test can't work as is in Rails test suite because they were
mutating DB state.
Most are redundant anyway, or should be implemented as shared test
that are executed for all adapters.
Fixes https://github.com/rails/rails/issues/48038
I don't think we need to use a proc here - it doesn't seem like there is any performance benefit in doing so. The default is being read from the `columns_hash`, which will already be loaded in memory. And then the `default` method is just reading a scalar value (or returning nil).
We had a mysql2_specific_schema.rb file but not one for trilogy that was
causing tests to break. I also removed the prepared statements from
trilogy because it is not supported.
I tried using the same config for mysql2 and trilogy but instead it
eneded up breaking the trilogy tests - they were only running in mysql2
mode. I wanted to do this so that the rake tasks for trilogy wouldn't
need to be duplicated but since that didn't work out quite right, I've
decide to duplicate the calls and add if exists / if not exists where
applicable. The configs should be the same but this will make sure that
if they do deviate, the dbs are always created/dropped.
Normally, it's valid syntax to pass the predicate builder a mapping from
primary key to a record, to an id, or to a relation.
With the composite case, there's a limitation on the types of syntax
supported. Namely, the only case we support right now is mapping a tuple
of columns to a tuple of corresponding values. For now, it's sufficient
to extract the ids instead of evaluating the SQL at a later time.
https://github.com/rails/rails/pull/42674 added the ability to have Rails verify foreign keys when creating fixtures. Feedback from users since then is it would be handy to know *which* foreign keys are being violated. See https://github.com/rails/rails/pull/44943 and https://github.com/rails/rails/pull/47780 for attempts to fix this.
This PR rolls up some of the ideas from those PRs into one that's hopefully mergable.
- [x] `all_foreign_keys_valid?` is deprecated in favour of `check_all_foreign_keys_valid!`.
- [x] Postgres and Sqlite adapters raise an error with detail about the foreign key error.
- [x] Authors of other PRs added as co-authors here to get credit.
- [x] Deprecations updated to work with new deprecation APIs.
- [x] Tests updated.
Here's what the error messages will now look like.
Postgres:
```
Foreign key violations found in your fixture data. Ensure you aren't referring to labels that don't exist on associations. Error from database:
PG::ForeignKeyViolation: ERROR: insert or update on table "fk_pointing_to_non_existent_objects" violates foreign key constraint "fk_that_will_be_broken"
DETAIL: Key (fk_object_to_point_to_id)=(980190962) is not present in table "fk_object_to_point_tos".
CONTEXT: SQL statement "UPDATE pg_constraint SET convalidated=false WHERE conname = 'fk_that_will_be_broken' AND connamespace::regnamespace = 'public'::regnamespace; ALTER TABLE public.fk_pointing_to_non_existent_objects VALIDATE CONSTRAINT fk_that_will_be_broken;"
PL/pgSQL function inline_code_block line 16 at EXECUTE
```
Sqlite:
```
Foreign key violations found in your fixture data. Ensure you aren't referring to labels that don't exist on associations. Error from database:
Foreign key violations found: fk_pointing_to_non_existent_objects
```
Closes https://github.com/rails/rails/pull/47780
Closes https://github.com/rails/rails/pull/44943
Co-Authored-By: s-mage <s-mage@users.noreply.github.com>
Co-Authored-By: danini-the-panini <danini-the-panini@users.noreply.github.com>
Single records in CPK contexts will take on an array to represent their
identifier. This was problematic because if only a single record is
passed, it's hard to distinguish the behaviour between a list of
multiple primary keys or a list representing a single primary key. As
such, it's helpful to always wrap the result of the single ID in an array,
so as to disambiguate the cases and create consistency in the shape of
the id structure.
Adds `:using_index` option to use an existing index when defining a unique constraint.
If you want to change an existing unique index to deferrable, you can use :using_index to create deferrable unique constraints.
```ruby
add_unique_key :users, deferrable: :immediate, using_index: 'unique_index_name'
```
A unique constraint internally constructs a unique index.
If an existing unique index has already been created, the unique constraint
can be created much faster, since there is no need to create the unique index
when generating the constraint.
This reverts commit 2815cb96207172f675a49860666f26167757f93c, reversing
changes made to 11a5e3771570f05cb888326765561d4cacbcac5a.
The change broke application code and if we are going to make this
change we will need to deprecate the prior behavior. See #47864 for more
details.
Until now, specifying uniqueness validations for records that have a composite key fail. This is because uniqueness validation relies on #id_in_database, which didn't yet consider the case when primary key is an array.
`ActiveSupport::MessagePack` is a serializer that integrates with the
`msgpack` gem to serialize a variety of Ruby objects. `AS::MessagePack`
supports several types beyond the base types that `msgpack` supports,
including `Time` and `Range`, as well as Active Support types such as
`AS::TimeWithZone` and `AS::HashWithIndifferentAccess`.
Compared to `JSON` and `Marshal`, `AS::MessagePack` can provide a
performance improvement and message size reduction. For example, when
used with `MessageVerifier`:
```ruby
# frozen_string_literal: true
require "benchmark/ips"
require "active_support/all"
require "active_support/message_pack"
marshal_verifier = ActiveSupport::MessageVerifier.new("secret", serializer: Marshal)
json_verifier = ActiveSupport::MessageVerifier.new("secret", serializer: JSON)
asjson_verifier = ActiveSupport::MessageVerifier.new("secret", serializer: ActiveSupport::JSON)
msgpack_verifier = ActiveSupport::MessageVerifier.new("secret", serializer: ActiveSupport::MessagePack)
ActiveSupport::Messages::Metadata.use_message_serializer_for_metadata = true
expiry = 1.year.from_now
data = { bool: true, num: 123456789, string: "x" * 50 }
Benchmark.ips do |x|
x.report("Marshal") do
marshal_verifier.verify(marshal_verifier.generate(data, expires_at: expiry))
end
x.report("JSON") do
json_verifier.verify(json_verifier.generate(data, expires_at: expiry))
end
x.report("AS::JSON") do
asjson_verifier.verify(asjson_verifier.generate(data, expires_at: expiry))
end
x.report("MessagePack") do
msgpack_verifier.verify(msgpack_verifier.generate(data, expires_at: expiry))
end
x.compare!
end
puts "Marshal size: #{marshal_verifier.generate(data, expires_at: expiry).bytesize}"
puts "JSON size: #{json_verifier.generate(data, expires_at: expiry).bytesize}"
puts "MessagePack size: #{msgpack_verifier.generate(data, expires_at: expiry).bytesize}"
```
```
Warming up --------------------------------------
Marshal 1.206k i/100ms
JSON 1.165k i/100ms
AS::JSON 790.000 i/100ms
MessagePack 1.798k i/100ms
Calculating -------------------------------------
Marshal 11.748k (± 1.3%) i/s - 59.094k in 5.031071s
JSON 11.498k (± 1.4%) i/s - 58.250k in 5.066957s
AS::JSON 7.867k (± 2.4%) i/s - 39.500k in 5.024055s
MessagePack 17.865k (± 0.8%) i/s - 89.900k in 5.032592s
Comparison:
MessagePack: 17864.9 i/s
Marshal: 11747.8 i/s - 1.52x (± 0.00) slower
JSON: 11498.4 i/s - 1.55x (± 0.00) slower
AS::JSON: 7866.9 i/s - 2.27x (± 0.00) slower
Marshal size: 254
JSON size: 234
MessagePack size: 194
```
Additionally, `ActiveSupport::MessagePack::CacheSerializer` is a
serializer that is suitable for use as an `ActiveSupport::Cache` coder.
`AS::MessagePack::CacheSerializer` can serialize `ActiveRecord::Base`
instances, including loaded associations. Like `AS::MessagePack`, it
provides a performance improvement and payload size reduction:
```ruby
# frozen_string_literal: true
require "benchmark/ips"
require "active_support/message_pack"
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Schema.define do
create_table :posts, force: true do |t|
t.string :body
t.timestamps
end
create_table :comments, force: true do |t|
t.integer :post_id
t.string :body
t.timestamps
end
end
class Post < ActiveRecord::Base
has_many :comments
end
class Comment < ActiveRecord::Base
belongs_to :post
end
post = Post.create!(body: "x" * 100)
2.times { post.comments.create!(body: "x" * 100) }
post.comments.load
cache_entry = ActiveSupport::Cache::Entry.new(post)
Rails70Coder = ActiveSupport::Cache::Coders::Rails70Coder
CacheSerializer = ActiveSupport::MessagePack::CacheSerializer
Benchmark.ips do |x|
x.report("Rails70Coder") do
Rails70Coder.load(Rails70Coder.dump(cache_entry))
end
x.report("CacheSerializer") do
CacheSerializer.load(CacheSerializer.dump(cache_entry))
end
x.compare!
end
puts "Rails70Coder size: #{Rails70Coder.dump(cache_entry).bytesize}"
puts "CacheSerializer size: #{CacheSerializer.dump(cache_entry).bytesize}"
```
```
Warming up --------------------------------------
Rails70Coder 329.000 i/100ms
CacheSerializer 492.000 i/100ms
Calculating -------------------------------------
Rails70Coder 3.285k (± 1.7%) i/s - 16.450k in 5.008447s
CacheSerializer 4.895k (± 2.4%) i/s - 24.600k in 5.028803s
Comparison:
CacheSerializer: 4894.7 i/s
Rails70Coder: 3285.4 i/s - 1.49x slower
Rails70Coder size: 808
CacheSerializer size: 593
```
Co-authored-by: Jean Boussier <jean.boussier@gmail.com>
The [Trilogy database client][trilogy-client] and corresponding
[Active Record adapter][ar-adapter] were both open sourced by GitHub last year.
Shopify has recently taken the plunge and successfully adopted Trilogy in their Rails monolith.
With two major Rails applications running Trilogy successfully, we'd like to propose upstreaming the adapter
to Rails as a MySQL-compatible alternative to Mysql2Adapter.
[trilogy-client]: https://github.com/github/trilogy
[ar-adapter]: https://github.com/github/activerecord-trilogy-adapter
Co-authored-by: Aaron Patterson <tenderlove@github.com>
Co-authored-by: Adam Roben <adam@roben.org>
Co-authored-by: Ali Ibrahim <aibrahim2k2@gmail.com>
Co-authored-by: Aman Gupta <aman@tmm1.net>
Co-authored-by: Arthur Nogueira Neves <github@arthurnn.com>
Co-authored-by: Arthur Schreiber <arthurschreiber@github.com>
Co-authored-by: Ashe Connor <kivikakk@github.com>
Co-authored-by: Brandon Keepers <brandon@opensoul.org>
Co-authored-by: Brian Lopez <seniorlopez@gmail.com>
Co-authored-by: Brooke Kuhlmann <brooke@testdouble.com>
Co-authored-by: Bryana Knight <bryanaknight@github.com>
Co-authored-by: Carl Brasic <brasic@github.com>
Co-authored-by: Chris Bloom <chrisbloom7@github.com>
Co-authored-by: Cliff Pruitt <cliff.pruitt@cliffpruitt.com>
Co-authored-by: Daniel Colson <composerinteralia@github.com>
Co-authored-by: David Calavera <david.calavera@gmail.com>
Co-authored-by: David Celis <davidcelis@github.com>
Co-authored-by: David Ratajczak <david@mockra.com>
Co-authored-by: Dirkjan Bussink <d.bussink@gmail.com>
Co-authored-by: Eileen Uchitelle <eileencodes@gmail.com>
Co-authored-by: Enrique Gonzalez <enriikke@gmail.com>
Co-authored-by: Garrett Bjerkhoel <garrett@github.com>
Co-authored-by: Georgi Knox <georgicodes@github.com>
Co-authored-by: HParker <HParker@github.com>
Co-authored-by: Hailey Somerville <hailey@hailey.lol>
Co-authored-by: James Dennes <jdennes@gmail.com>
Co-authored-by: Jane Sternbach <janester@github.com>
Co-authored-by: Jess Bees <toomanybees@github.com>
Co-authored-by: Jesse Toth <jesse.toth@github.com>
Co-authored-by: Joel Hawksley <joelhawksley@github.com>
Co-authored-by: John Barnette <jbarnette@github.com>
Co-authored-by: John Crepezzi <john.crepezzi@gmail.com>
Co-authored-by: John Hawthorn <john@hawthorn.email>
Co-authored-by: John Nunemaker <nunemaker@gmail.com>
Co-authored-by: Jonathan Hoyt <hoyt@github.com>
Co-authored-by: Katrina Owen <kytrinyx@github.com>
Co-authored-by: Keeran Raj Hawoldar <keeran@gmail.com>
Co-authored-by: Kevin Solorio <soloriok@gmail.com>
Co-authored-by: Leo Correa <lcorr005@gmail.com>
Co-authored-by: Lizz Hale <lizzhale@github.com>
Co-authored-by: Lorin Thwaits <lorint@gmail.com>
Co-authored-by: Matt Jones <al2o3cr@gmail.com>
Co-authored-by: Matthew Draper <matthewd@github.com>
Co-authored-by: Max Veytsman <mveytsman@github.com>
Co-authored-by: Nathan Witmer <nathan@zerowidth.com>
Co-authored-by: Nick Holden <nick.r.holden@gmail.com>
Co-authored-by: Paarth Madan <paarth.madan@shopify.com>
Co-authored-by: Patrick Reynolds <patrick.reynolds@github.com>
Co-authored-by: Rob Sanheim <rsanheim@gmail.com>
Co-authored-by: Rocio Delgado <rocio@github.com>
Co-authored-by: Sam Lambert <sam.lambert@github.com>
Co-authored-by: Shay Frendt <shay@github.com>
Co-authored-by: Shlomi Noach <shlomi-noach@github.com>
Co-authored-by: Sophie Haskins <sophaskins@github.com>
Co-authored-by: Thomas Maurer <tma@github.com>
Co-authored-by: Tim Pease <tim.pease@gmail.com>
Co-authored-by: Yossef Mendelssohn <ymendel@pobox.com>
Co-authored-by: Zack Koppert <zkoppert@github.com>
Co-authored-by: Zhongying Qiao <cryptoque@users.noreply.github.com>
Most of the support here is in implementing how to correctly substitute
multiple values in place of one, for composite caes. In composite cases,
it's not sufficient to hash a label into a single integer value.
Instead, we build an API that accepts a single label, and a list of
columns that we'd like to map to. The algorithm used internally is very
similar to #identify, with an additional bit shift and modulo to cycle
the hash and ensure it doesn't exceed a max.
