rowland/activerecord-fb-adapter — pull request #44 by rzane (2014-11-26 02:26:06+00:00)
Title: Passing tests (most of them, at least)

## Changes

#### Organization

First off, I've split the adapter into modules. It makes it much easier to find the code you're looking for, cleans up the readability of public/private/protected and more closely resembles ActiveRecord's own organization. I also removed a lot of commented out methods that are probably not the actual definition from AbstractAdapter anymore.

#### Style

Ran Rubocop and made some of the recommended style changes.

#### Arel::Visitors::FB

Renamed to Arel::Visitors::Fb to correspond with the adapter name.

#### FbAdapter

__create_savepoint, rollback_to_savepoint, and release_savepoint__ Accept a name parameter for AR 4.1 compatability.

__disconnect!__ Call super so if a transaction is open, it gets reset. Necessary to pass `transaction state is reset after a reconnect` and `transaction state is reset after a disconnect`

#### FbColumn

__default__ Use the raw_connection to fetch the type casted value using the underlying Fb. Required to pass the following:

+ test_build
+ test_build_many
+ test_build_many_via_block
+ test_build_via_block
+ test_build_without_loading_association
+ test_new_aliased_to_build
+ test_transaction_when_deleting_new_record
+ test_transactions_when_adding_to_new_record
+ test_transactions_when_replacing_on_new_record
+ test_build_association_dont_create_transaction
+ test_has_one_transaction
+ test_build_before_save
+ test_build_many_before_save
+ test_build_many_via_block_before_save
+ test_build_via_block_before_save

#### Fb::DatabaseStatements

__select__ In AR 3, `select` should return an array of hashes. In AR 4, [select should return an ActiveRecord::Result instance](http://api.rubyonrails.org/classes/ActiveRecord/ConnectionAdapters/DatabaseStatements.html#method-i-select). I removed `FbArray` because it's a hack. FbArray mocks the behavior of an ActiveRecord::Result. In this PR, `select` calls`exec_query` and returns the correct type based on the AR version.

__select_rows__ Now accepts a binds array for AR 4 compatability. The new implementation is probably a bit slower now because it simply maps the values, instead of using the underlying C function from FB. It's exactly what the [AR-JDBC adapter](https://github.com/jruby/activerecord-jdbc-adapter/blob/d5dc0e8c394be75e3f8064297a0bdfb491e71db3/lib/arjdbc/jdbc/adapter.rb#L528) does though, so it's probably okay.

__next_sequence_value__ Uses the underlying Fb connection in order to pass the following tests:

+ test_partial_update
+ test_partial_update_with_optimistic_locking
+ test_create_without_loading_association
+ test_inverse_on_before_validate
+ test_associate_existing
+ test_associate_new_by_building
+ test_associate_with_create
+ test_associating_new
+ test_replace_association
+ test_build_before_save
+ test_build_many_before_save
+ test_build_many_via_block_before_save
+ test_build_via_block_before_save
+ test_not_resaved_when_unchanged

__reset_sequence!__ Will now accept an optional sequence name, rather than always using the default.

__rollback_db_transaction, commit_db_transaction, and begin_db_transaction__ Should not assign `@transaction`. About 100 tests were failing because of this (usually when creating fixtures). `@transaction` was conflicting with the `@transaction` assigned in `begin_transaction`, `commit_transaction`, and `rollback_transaction` from Abstract::DatabaseStatements.

#### Fb::SchemaStatements

__add_column_options!__ This method was removed in AR 4.1, so the old code was added to provide backward-compatibility.

__add_column, create_table__ Only rescue an ActiveRecord::Statement invalid when creating a boolean domain.

__rename_table, change_column_null__ These are limitations of Firebird and can't be done very easily (as far as I know). They'll output an error message to the logger. Maybe these should raise an exception. They used to raise `NotImplementedErrors`.

#### Fb::TableDefinition

__primary_key__ When the primary_key is manually defined using the table definition, a sequence wasn't being created. This change, as well as the override to `create_table` allows the following:

```ruby
create_table :dogs, id: false do |t|
t.primary_key :a_different_id
end
```

## Test Results

#### ActiveRecord 4.1.8

| | Tests | Assertions | Failures | Errors |
|---------|-------|------------|----------|--------|
| Master | 4540 | 9054 | 72 | 1072 |
| This PR | 4551 | 11617 | 73 | 61 |

#### ActiveRecord 4.0.12

| | Tests | Assertions | Failures | Errors |
|---------|-------|------------|----------|--------|
| Master | 4330 | 6013 | 89 | 1748 |
| This PR | 4341 | 11163 | 60 | 53 |

#### ActiveRecord 3.2.21

| | Tests | Assertions | Failures | Errors |
|---------|-------|------------|----------|--------|
| Master | 3828 | 10196 | 64 | 289 |
| This PR | 3839 | 10256 | 38 | 284 |

## Some of the remaining problems

These are some of the major issues that remain and may not have a solution. There are plenty more, but these are the ones causing the most problems in the test suite. Some of these will be either resolved or skipped in this PR: https://github.com/rails/rails/pull/17720. I'm still working on it, so if anybody wants to help out, feel free to send PR's to my fork.

+ No way to disable referential integrity
+ test_disable_referential_integrity
+ Decimal precision greater than 18
+ test_native_types
+ test_native_decimal_insert_manual_vs_automatic
+ test_schema_dump_keeps_large_precision_integer_columns_as_decimal
+ Can't rename tables
+ test_rename_table
+ test_rename_table_does_not_rename_custom_named_index
+ test_rename_table_with_an_index
+ test_rename_table_with_prefix_and_suffix
+ Can't change the nullability of a column, can't shrink varchar columns. (There's some other stuff going wrong with `change_column` as well).
+ test_change_column
+ test_change_column_default
+ test_change_column_nullability
+ test_change_column_with_custom_index_name
+ test_change_column_with_long_index_name
+ test_change_column_with_new_default
+ test_change_column_nil_default
+ test_change_column_with_null
+ test_change_column_quotes_column_names
+ test_cache_gets_cleared_after_migration
+ Index names. This adapter overrides `index_name`, so non-standard index names are generated. This method can be removed, but should be done with caution because it won't be backwards compatible.