Skip to content

Commit

Permalink
chore: bump activerecord
Browse files Browse the repository at this point in the history
- Update supported versions of Cockroach and ActiveRecord
- Fix some tests, updgrade codebase to adapt to rails 7.2

Closes #305. Closes #336
  • Loading branch information
BuonOmo committed Sep 24, 2024
1 parent 8c1841b commit 0a73c7d
Show file tree
Hide file tree
Showing 80 changed files with 555 additions and 527 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ jobs:
fail-fast: false
matrix:
# https://www.cockroachlabs.com/docs/releases/release-support-policy
crdb: [v22.2, v23.1, v23.2]
ruby: [head]
crdb: [v23.2, v24.1, v24.2]
ruby: ["3.3"]
name: Test (crdb=${{ matrix.crdb }} ruby=${{ matrix.ruby }})
steps:
- name: Set Up Actions
Expand Down Expand Up @@ -88,4 +88,4 @@ jobs:
done
cat ${{ github.workspace }}/setup.sql | cockroach sql --insecure
- name: Test
run: bundle exec rake test TESTOPTS='--profile=3'
run: bundle exec rake test TESTOPTS='--profile=5'
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@
/docker_root/
/ruby-build/
/test/db/
/.ruby-version
1 change: 0 additions & 1 deletion .ruby-version

This file was deleted.

4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Ongoing

## 7.2.0 - 2024

- Add support for Rails 7.2 ([#337](https://github.com/cockroachdb/activerecord-cockroachdb-adapter/pull/337))

## 7.1.1 - 2024-05-01

- Enable precision support ([#318](https://github.com/cockroachdb/activerecord-cockroachdb-adapter/pull/318))
Expand Down
125 changes: 35 additions & 90 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -1,18 +1,5 @@
# Getting started


## ActiveRecord adapters and you

There are two repositories for the ActiveRecord adapter. The one you're in
currently, [activerecord-cockroachdb-adapter], is the CockroachDB specific
ActiveRecord code. Users install this alongside ActiveRecord then use
CockroachDBAdapter to initialize ActiveRecord for their projects.

This adapter extends the PostgreSQL ActiveRecord adapter in order to
override and monkey-patch functionality.

[activerecord-cockroachdb-adapter]: https://github.com/cockroachdb/activerecord-cockroachdb-adapter/

## Setup and running tests

### CockroachDB
Expand All @@ -23,6 +10,7 @@ create two databases to be used by the ActiveRecord test suite:
activerecord_unittest and activerecord_unittest2.

```sql
-- See /setup.sql file for the whole setup.
CREATE DATABASE activerecord_unittest;
CREATE DATABASE activerecord_unittest2;
```
Expand All @@ -36,30 +24,6 @@ Ruby version.
tests similarly to TeamCity. The database is destroyed between each
test file.)

Install rbenv with ruby-build on MacOS:

```bash
brew install rbenv ruby-build
echo 'eval "$(rbenv init - zsh)"' >> ~/.profile
```

Install rbenv with ruby-build on Ubuntu:

```bash
sudo apt-get install rbenv
echo 'eval "$(rbenv init - zsh)"' >> ~/.profile

git clone https://github.com/sstephenson/ruby-build.git ~/ruby-build
sh ~/ruby-build/install.sh
```

Use rbenv to install version of Ruby specified by `.ruby-version`.

```bash
rbenv install
rbenv rehash
```

Using [bundler](http://bundler.io/), install the dependencies of Rails.

```bash
Expand Down Expand Up @@ -87,9 +51,12 @@ TEST_FILES="test/cases/adapter_test.rb,test/cases/associations/left_outer_join_a
To run a specific test case, use minitest's `-n` option to run tests that match a given pattern. All minitest options are set via the `TESTOPTS` environemnt variable. For example, to run `test_indexes` from CockroachDB's `test/cases/adapter_test.rb` file

```bash
TEST_FILES="test/cases/adapter_test.rb" TESTOPTS=`-n=/test_indexes/` bundle exec rake test
TEST_FILES="test/cases/adapter_test.rb" TESTOPTS=`--name=/test_indexes/` bundle exec rake test
```

You can also use the constant `COCKROACH_SKIP_LOAD_SCHEMA` to avoid reloading the schema every time (faster).
Only do it if you know the schema was left in a correct state.

`test/config.yml` assumes CockroachDB will be running at localhost:26257 with a root user. Make changes to `test/config.yml` as needed.

### Run Tests from a Backup
Expand Down Expand Up @@ -121,7 +88,6 @@ master branch, with an alpha build of CockroachDB. it would be even
better to be able to test multiple versions of the adapter, and do so
against different versions of CockroachDB.


## Adding feature support

As CockroachDB improves, so do the features that can be supported in
Expand All @@ -131,15 +97,36 @@ gates should be toggled. Something that would help this process would be
linking those issues back to this adapter so that part of the feature
completing includes updating the adapter.


## Execute only tests that run with a connection

I have not investigated if this is already possible, but I would assume
no.

A possible way to approach this would be to add a shim to cause any
tests that use it to fail, and grep the tests that pass and then skip
them.
## Upgrading Rails

Whenever you upgrade rails version, loads of things will change.
This section intent to help you with a checklist.

- Check for TODO or NOTE tags that are referencing the old or new version of
rails.
```bash
rg 'TODO|NOTE' --after-context=2
```
- Check postgresql_specific_schema.rb changelog in rails, and apply the changes
you want. Ex:
```bash
git diff v7.1.4..v7.2.1 -- $(fd postgresql_specific_schema)
```
- Verify the written text at the beginning of the test suite, there are likely
some changes in excluded tests.
- Check for some important methods, some will change for sure:
- [x] `def new_column_from_field(`
- [x] `def column_definitions(`
- [x] `def pk_and_sequence_for(`
- [ ] ...
- Check for setups containing `drop_table` in the test suite.
Especially if you have tons of failure, this is likely the cause.
- In the same way, run `test/cases/fixtures_test.rb` first, and check
if this corrupted the test database for other tests.
- For both of the above, the diff of `schema.rb` can be useful:
```bash
git diff v7.1.2..v7.2.1 -- activerecord/test/schema/schema.rb
```
## Publishing to Rubygems
Expand All @@ -151,7 +138,6 @@ gem build ...
gem publish <output file>
```

# Notes
When executing the test suite, each test file will reload fixtures. This
Expand Down Expand Up @@ -182,49 +168,8 @@ cleaned up, or skipped until passing.
The purpose of these was to make the tests grep-able while going through
all the failures.

[cockroachdb/cockroach#20753]: https://github.com/cockroachdb/cockroach/issues/20753#issuecomment-352810425

## Tracked test failures

Some of the skipped failures are:

- `default:` key is not working for columns in table schema
definitions. This causes tests to fail due to unexpected data.

- `array:` key is not working for columns in table schema definitions.

- `"Salary is not appearing in list"` is being raised in a number of
places. Likely during fixture setup.

- `sum` function seems to result in a different type in ActiveRecord.
Instead of returning a Ruby `int`, it returns a Ruby `string`. It
appears that MySQL2 also does this. A suspected cause might be how
`Decimal` is handled if `sum` consumes integers and return a
decimal.

- Potentially fork the PostgreSQL::SchemaDumper to handle anything
specific to CockroachDB, like primary keys or bigints.

- You can call `@connection.create_table(..., id: :bigint)`, but this
will not changes things for CockroachDB (I think...), so it would be
not allowed. Even better, our adapter could interpret this and
generate the appropriate explicit pkey column. Not sure what string
pkeys look like...

- `string` types are introspected to `text` types.

- A user can do an update, delete, and insert on views.

- Postgres specific bit strings are not properly supported.

Grepping for `FIXME(joey)`, `TODO(joey)`, and `NOTE(joey)` will yeild
most of the touchpoints including test failures and temporary monkey
patches. Some monkey patches were made directly to Rails, which will
need to be cleaned up.


# Notes for the non-Rubyer
rbenv is an environment manager that lets you manage and swap between
Expand Down
7 changes: 4 additions & 3 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ group :development, :test do

# Needed for the test suite
gem "msgpack", ">= 1.7.0"
gem "mutex_m", "~> 0.2.0"

gem "rake"
gem "debug"
Expand All @@ -61,8 +62,8 @@ group :development, :test do
gem "parser"

# Gems used by the ActiveRecord test suite
gem "bcrypt", "~> 3.1.18"
gem "sqlite3", "~> 1.4.4"
gem "bcrypt", "~> 3.1"
gem "sqlite3", "~> 1.4"

gem "minitest", "~> 5.15.0"
gem "minitest", "~> 5.15"
end
12 changes: 7 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,20 +1,22 @@
# ActiveRecord CockroachDB Adapter

CockroachDB adapter for ActiveRecord 5, 6, and 7. This is a lightweight extension of the PostgreSQL adapter that establishes compatibility with [CockroachDB](https://github.com/cockroachdb/cockroach).
CockroachDB adapter for ActiveRecord. This is a lightweight extension
of the PostgreSQL adapter that establishes compatibility with [CockroachDB](https://github.com/cockroachdb/cockroach).

## Installation

Add this line to your project's Gemfile:

```ruby
gem 'activerecord-cockroachdb-adapter', '~> 7.0.0'
gem 'activerecord-cockroachdb-adapter', '~> 7.2.0'
```

If you're using Rails 5.2, use the `5.2.x` versions of this gem.
If you're using Rails 7.0, use the `7.0.x` versions of this gem.

If you're using Rails 6.0, use the `6.0.x` versions of this gem.
If you're using Rails 7.1, use the `7.1.x` versions of this gem.

If you're using Rails 7.0, use the `7.0.x` versions of this gem.
If you're using Rails 7.2, use the `7.2.x` versions of this gem.
The minimal CockroachDB version required is 23.1.12 for this version.

In `database.yml`, use the following adapter setting:

Expand Down
4 changes: 2 additions & 2 deletions activerecord-cockroachdb-adapter.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ Gem::Specification.new do |spec|
spec.description = "Allows the use of CockroachDB as a backend for ActiveRecord and Rails apps."
spec.homepage = "https://github.com/cockroachdb/activerecord-cockroachdb-adapter"

spec.add_dependency "activerecord", "~> 7.1.0"
spec.add_dependency "pg", "~> 1.2"
spec.add_dependency "activerecord", "~> 7.2.0"
spec.add_dependency "pg", "~> 1.5"
spec.add_dependency "rgeo-activerecord", "~> 7.0.0"

spec.add_development_dependency "benchmark-ips", "~> 2.9.1"
Expand Down
2 changes: 1 addition & 1 deletion bin/console
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ $:.unshift(File.expand_path("../lib", __dir__))
# require "bundler/setup"
# Bundler.require :development

require "active_record"
require "activerecord-cockroachdb-adapter"
# This allows playing with the rake task as well. Ex:
#
# ActiveRecord::Tasks::DatabaseTasks.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,14 @@ def structure_dump(filename, extra_flags=nil)
ActiveRecord.dump_schemas
end

conn = ActiveRecord::Base.connection
conn = ActiveRecord::Base.lease_connection
begin
old_search_path = conn.schema_search_path
conn.schema_search_path = search_path
File.open(filename, "w") do |file|
# NOTE: There is no issue with the crdb_internal schema, it is ignored by SHOW CREATE.
%w(SCHEMAS TYPES).each do |object_kind|
ActiveRecord::Base.connection.execute("SHOW CREATE ALL #{object_kind}").each_row { file.puts _1 }
conn.execute("SHOW CREATE ALL #{object_kind}").each_row { file.puts _1 }
end

ignore_tables = ActiveRecord::SchemaDumper.ignore_tables.to_set
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@ def primary_key(table_name)
end
end

# OVERRIDE: CockroachDB does not support deferrable constraints.
# See: https://go.crdb.dev/issue-v/31632/v23.1
def foreign_key_options(from_table, to_table, options)
options = super
options.delete(:deferrable) unless supports_deferrable_constraints?
options
end

# OVERRIDE: Added `unique_rowid` to the last line of the second query.
# This is a CockroachDB-specific function used for primary keys.
# Also make sure we don't consider `NOT VISIBLE` columns.
Expand Down
Loading

0 comments on commit 0a73c7d

Please sign in to comment.