Improved ActiveRecord injection

[leboshi]

These commits make the injections into AR more surgical. This refactoring started with the Arel::Table issue patched in #61.

Replace subclasses with prepended modules

A significant portion of the codebase is currently copied and pasted directly from ActiveRecord except for references to clickhouse-activerecord (e.g., ClickhouseActiverecord::MigrationContext, ClickhouseActiverecord::Migrator, ClickhouseActiverecord::Schema). Whenever the underlying AR code is changed, those changes will need to be manually merged into this gem again, and the gem may break in the meantime.

The reason for the copied and pasted code is to point AR to Clickhouse-specific variants of AR classes (e.g., ClickhouseActiverecord::InternalMetadata). The current approach is logically top-down, making Clickhouse-specific subclasses as far up the chain as necessary to point to classes with Clickhouse-specific behavior. However, this leads to a lot of hairy scenarios where AR code expects classes to be named a certain way or to be namespaced somewhere within the current module tree, so we end up brute-forcing our way through the inheritance chain with copy/pasted code to point AR elsewhere. (The fact that ClickhouseColumn is an unmodified subclass of Column is one particularly 🤨 manifestation of this smell.)

It's actually not necessary to use our own entire subclasses of things like ActiveRecord::Migrator, when all we actually want to do is override InternalMetadata::create_table. We can avoid subclasses entirely in most cases by prepending Clickhouse-specific behavior into the AR modules and classes. Ideally, we'd be able to get even more surgical by overriding only what we need to in methods like SchemaStatements#assume_migrated_upto_version and SchemaDumper.dump, but that's a more challenging refactoring that may not be feasible, given the way AR itself is currently implemented.

Files separated

There was a lot going on in files like clickhouse_adapter.rb and migration.rb.

Files and classes moved

In cases where we do need our own entire subclasses, moving them into the AR namespace lets AR naturally assume their presence or fall back to its own implementation, allowing us move much more freely between generic AR and Clickhouse-AR code.

• SchemaDumper moved from ClickhouseActiverecord to ActiveRecord::ConnectionAdapters::Clickhouse
• ClickhouseActiverecord::Arel::Visitors::ToSql moved to Arel::Visitors::Clickhouse

New loader

We moved the injection of clickhouse-activerecord into AR into ClickhouseActiverecord.load, and wrapped it in an initializer declaration and an on_load hook.

We also fixed injections into ActiveRecord::Base. Previously, ModelSchema::ClassMethods was being included too late and not actually being included in models.

New specs

The test suite actually isn't passing right now because some specs actually depend on your cluster name, but we've:

• fixed specs for column creation,
• added specs for type casting in SQL generation, and
• added specs for using Clickhouse as one of multiple databases.

[leboshi]

This contains a lot of major structural changes, so I'm happy to discuss at your convenience, if you have any questions! The content of the code should all be the same; it's just been shuffled around quite a bit.

[PNixx]

Thanks for your support. These are very big changes, I need more time to test it.

[leboshi]

Yeah, definitely. We've been using this version of the code in production for a few months now, but that doesn't count as comprehensive testing. Our next goal is increasing test coverage. (Like I said, we love this gem and use it heavily in production, which is why we're so interested in it.). Honestly I'd have loved to have a full test suite first to prevent regressions, but writing tests is actually how we discovered some of the stuff was broken, like is_view/is_view= not being included in models.

[stanhu]

Thanks for doing this! I had to insert a ClickhouseActiverecord.load in my initializer, or my Rails application would fail with:

	 4: from /Users/stanhu/.asdf/installs/ruby/2.7.5/lib/ruby/gems/2.7.0/gems/activerecord-6.1.4.6/lib/active_record/connection_handling.rb:90:in `connects_to'
	 3: from /Users/stanhu/.asdf/installs/ruby/2.7.5/lib/ruby/gems/2.7.0/gems/activerecord-6.1.4.6/lib/active_record/connection_handling.rb:90:in `each'
	 2: from /Users/stanhu/.asdf/installs/ruby/2.7.5/lib/ruby/gems/2.7.0/gems/activerecord-6.1.4.6/lib/active_record/connection_handling.rb:95:in `block in connects_to'
	 1: from /Users/stanhu/.asdf/installs/ruby/2.7.5/lib/ruby/gems/2.7.0/gems/activerecord-6.1.4.6/lib/active_record/connection_adapters/abstract/connection_pool.rb:1046:in `establish_connection'
/Users/stanhu/.asdf/installs/ruby/2.7.5/lib/ruby/gems/2.7.0/gems/activerecord-6.1.4.6/lib/active_record/connection_adapters/abstract/connection_pool.rb:1222:in `resolve_pool_config': database configuration specifies nonexistent clickhouse adapter (ActiveRecord::AdapterNotFound)

It looks this call normally happens in the Railtie, but if an initializer ever attempts to load the ActiveRecord#connection_class the lookup will fail.

[leboshi]

Hmmmm, I'll take a look at that. Thanks, @stanhu. 👍

@PNixx I pushed another refactor, moving things into the CoreExtensions module to stick to Rails best practices. Still working on expanding test coverage for everything.

The biggest motivator for me behind this work is that Rails 7 moved type map setup from an instance method to a class method, so actual IO operations all die. (Is anybody else having Rails 7 issues, or is everyone still running 6 too?)

[PNixx]

I try checking this PR and have caught errors:

• NameError: uninitialized constant ClickhouseActiverecord::Schema with rake task clickhouse:schema:load
• I try change in schema file ClickhouseActiverecord::Schema to ActiveRecord::Schema and have caught error ArgumentError: wrong number of arguments (given 4, expected 1)

And you need resolve conflicts