jruby / activerecord-jdbc-adapter

JRuby's ActiveRecord adapter using JDBC.
BSD 2-Clause "Simplified" License
462 stars 387 forks source link

Postgres adapter multi-dimensional array support (AR 5.0 compat) #830

Open rdubya opened 6 years ago

rdubya commented 6 years ago

Currently multi-dimensional arrays are handled incorrectly and end up with weird quoting issues. There are many tests breaking that demonstrate the issues.

Linked to: https://github.com/jruby/activerecord-jdbc-adapter/issues/824

yahonda commented 6 years ago

Here are some findings of this. I have not found the fix yet but wanted to provide them.

Steps to reproduce:

$ git clone https://github.com/rails/rails.git
$ cd rails/activerecord
$ git checkout 5-1-stable
$ bundle install
$ ARCONN=jdbcpostgresql bin/test test/cases/adapters/postgresql/array_test.rb:157

Actual result:

$ ARCONN=jdbcpostgresql bin/test test/cases/adapters/postgresql/array_test.rb:157
Using jdbcpostgresql
Run options: --seed 8150

# Running:

F

Finished in 0.713566s, 1.4014 runs/s, 1.4014 assertions/s.

  1) Failure:
PostgresqlArrayTest#test_with_multi_dimensional_empty_strings [/home/yahonda/git/rails/activerecord/test/cases/adapters/postgresql/array_test.rb:157]:
--- expected
+++ actual
@@ -1 +1 @@
-[[["1", "2"], ["", "4"], ["", "5"]]]
+["[[\"1\", \"2\"], [\"\", \"4\"], [\"\", \"5\"]]"]

1 runs, 1 assertions, 1 failures, 0 errors, 0 skips
$

Additional information:

It is likely due to TextEncoder::Array#encode differences.

https://github.com/jruby/activerecord-jdbc-adapter/blob/54c94fd4fe74648acc6590841a2f3e59652f85a8/lib/arjdbc/postgresql/base/array_encoder.rb#L17-L19

Here are puts debug output. Also, assert_equal method has two assertions. When tested with JRuby it fails at the first assertion then commented out the second assertion to compare the result of the first one.

$ git diff
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb
index 92067b748b..3285667885 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb
@@ -123,8 +123,10 @@ def _type_cast(value)
           def encode_array(array_data)
             encoder = array_data.encoder
             values = type_cast_array(array_data.values)
+            puts "values: #{values}"

             result = encoder.encode(values)
+            puts "result: #{result}"
             if encoding = determine_encoding_of_strings_in_array(values)
               result.force_encoding(encoding)
             end
diff --git a/activerecord/test/cases/adapters/postgresql/array_test.rb b/activerecord/test/cases/adapters/postgresql/array_test.rb
index c78c6178ff..637e0a439f 100644
--- a/activerecord/test/cases/adapters/postgresql/array_test.rb
+++ b/activerecord/test/cases/adapters/postgresql/array_test.rb
@@ -338,10 +338,10 @@ def assert_cycle(field, array)
       assert_equal(array, x.public_send(field))

       # test updating
-      x = PgArray.create!(field => [])
-      x.public_send("#{field}=", array)
-      x.save!
-      x.reload
-      assert_equal(array, x.public_send(field))
+#      x = PgArray.create!(field => [])
+#      x.public_send("#{field}=", array)
+#      x.save!
+#      x.reload
+#      assert_equal(array, x.public_send(field))
     end
 end
$
$ ARCONN=jdbcpostgresql bin/test test/cases/adapters/postgresql/array_test.rb:157
Using jdbcpostgresql
Run options: --seed 13146

# Running:

values: []
result: {}
values: []
result: {}
values: [[["1", "2"], ["", "4"], ["", "5"]]]
result: {"[[\"1\", \"2\"], [\"\", \"4\"], [\"\", \"5\"]]"}
F

Finished in 0.861945s, 1.1602 runs/s, 1.1602 assertions/s.

  1) Failure:
PostgresqlArrayTest#test_with_multi_dimensional_empty_strings [/home/yahonda/git/rails/activerecord/test/cases/adapters/postgresql/array_test.rb:157]:
--- expected
+++ actual
@@ -1 +1 @@
-[[["1", "2"], ["", "4"], ["", "5"]]]
+["[[\"1\", \"2\"], [\"\", \"4\"], [\"\", \"5\"]]"]

1 runs, 1 assertions, 1 failures, 0 errors, 0 skips
$
$ ARCONN=postgresql bin/test test/cases/adapters/postgresql/array_test.rb:157
Using postgresql
Run options: --seed 9373

# Running:

values: []
result: {}
values: []
result: {}
values: [[["1", "2"], ["", "4"], ["", "5"]]]
result: {{{1,2},{"",4},{"",5}}}
.

Finished in 0.130186s, 7.6813 runs/s, 7.6813 assertions/s.

1 runs, 1 assertions, 0 failures, 0 errors, 0 skips
$

I do not write C code then unlikely able to provide fix for this failure. Hope this information helps.