rails icon indicating copy to clipboard operation
rails copied to clipboard

ActiveRecord 6.1 and JSONB: Incorrect results value/type when using both arrow operator and double arrow operator in the same SELECT statement

Open shhavel opened this issue 1 year ago • 5 comments

Issue Description

Generally when using JSONB operators -> (arrow operator) and ->> (double arrow operator) in SELECT statement, ActiveRecord returns correct result types: objects for -> operator and string for ->> operator. But when using both operators in the same select statement, the returned results may have an unexpected type or be nil.

Steps to reproduce with Expected behaviour in tests and Actual behaviour in comments

# file: activerecord_6_1_jsonb_test.rb
# run: ruby activerecord_6_1_jsonb_test.rb

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  gem "activerecord", "6.1.6.1" # the same behaviour for version "7.0.3.1"
  gem "pg"
end

require "active_record"
require "minitest/autorun"
require "logger"

pg_opts = {adapter: "postgresql", database: "jsonb_activerecord_issue"}
begin
  ActiveRecord::Base.establish_connection(pg_opts.except(:database))
  ActiveRecord::Base.connection.create_database(pg_opts[:database])
  puts "Database #{pg_opts[:database]} was created."
rescue => ActiveRecord::DatabaseAlreadyExists
  puts "Database #{pg_opts[:database]} alredy exists."
end
ActiveRecord::Base.establish_connection(pg_opts)
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :entries, force: true do |t|
    t.jsonb :value
  end
end

class Entry < ActiveRecord::Base
end
Entry.create(value: {a: "a", b: "b", c: {}})

class BugTest < Minitest::Test
  # passed
  def test_jsonb_arrow_operator
    assert_equal [["a", "b"]], Entry.pluck(Arel.sql("value->'a', value->'b'"))
    assert_equal [[{}, "b"]], Entry.pluck(Arel.sql("value->'c', value->'b'"))
  end

  # passed
  def test_jsonb_double_arrow_operator
    assert_equal [["a", "b"]], Entry.pluck(Arel.sql("value->>'a', value->>'b'"))
    assert_equal [["{}", "b"]], Entry.pluck(Arel.sql("value->>'c', value->>'b'"))
  end

  # failed (this test results are differ in activerecord 6.0)
  def test_jsonb_arrow_operator_and_double_arrow_operator
    assert_equal [["a", "b"]], Entry.pluck(Arel.sql("value->'a', value->>'b'")) # Actual [["a", nil]]
    assert_equal [[{}, "b"]], Entry.pluck(Arel.sql("value->'c', value->>'b'")) # Actual [[{}, nil]]
  end

  # failed
  def test_jsonb_double_arrow_operator_and_arrow_operator
    assert_equal [["a", "b"]], Entry.pluck(Arel.sql("value->>'a', value->'b'")) # Actual [[nil, "b"]]
    assert_equal [["{}", "b"]], Entry.pluck(Arel.sql("value->>'c', value->'b'")) # Actual [[{}, "b"]]
  end
end

PostgreSQL results

SELECT
  pg_typeof(value->'a'), value->'a',
  pg_typeof(value->'b'), value->'b',
  pg_typeof(value->'c'), value->'c',
  pg_typeof(value->>'a'), value->>'a',
  pg_typeof(value->>'b'), value->>'b',
  pg_typeof(value->>'c'), value->>'c'
FROM "entries";

 pg_typeof | ?column? | pg_typeof | ?column? | pg_typeof | ?column? | pg_typeof | ?column? | pg_typeof | ?column? | pg_typeof | ?column?
-----------+----------+-----------+----------+-----------+----------+-----------+----------+-----------+----------+-----------+----------
 jsonb     | "a"      | jsonb     | "b"      | jsonb     | {}       | text      | a        | text      | b        | text      | {}
(1 row)

System configuration

Rails version: 6.1.6.1

Ruby version: 3.1.0

Similar issue in different versions of ActiveRecord

There is a similar issue but with slightly different behaviour in AR 6.0. Created another issue: #45757

Please confirm or comment on the expected behaviour. Thank you.

shhavel avatar Aug 03 '22 23:08 shhavel

I can confirm that your tests are failing on 7.0.3.1:

D, [2022-08-03T20:43:49.383983 #901625] DEBUG -- :   Entry Pluck (0.5ms)  SELECT value->'a', value->>'b' FROM "entries"

This is the query being run in test_jsonb_arrow_operator_and_double_arrow_operator, and I confirmed that this query run in postgres looks correct:

psql -d jsonb_activerecord_issue
psql (14.3)
Type "help" for help.

jsonb_activerecord_issue=# SELECT value->'a', value->>'b' FROM "entries";
 ?column? | ?column?
----------+----------
 "a"      | b
(1 row)

so I'm thinking the issue must be in parsing the result?

Edit:

irb(main):001:0> require 'pg'
=> true
irb(main):002:0> conn = PG.connect(dbname: 'jsonb_activerecord_issue')
=> #<PG::Connection:0x00007fe6781e9c98 @last_status=4>
irb(main):003:0> r = conn.exec("SELECT value->'a', value->'b' FROM \"entries\"")
=> #<PG::Result:0x00007fe6777e0760 status=PGRES_TUPLES_OK ntuples=1 nfields=2 cmd_tuples=1>
irb(main):005:0> r.each { |row| puts row }                                                                           
{"?column?"=>"\"b\""}
=> #<PG::Result:0x00007fe6777e0760 status=PGRES_TUPLES_OK ntuples=1 nfields=2 cmd_tuples=1>                          
irb(main):006:0> r.values                                                                                            
=> [["\"a\"", "\"b\""]]

Is it expected that values shows both but each only has a single value? Seems odd to me, but I haven't dug into this section of Active Record before so I'm trying things)

skipkayhil avatar Aug 04 '22 00:08 skipkayhil

Active Record gets the results correctly, but there is one problem.

#<ActiveRecord::Result:0x00000001071e3b58 @columns=["?column?", "?column?"], @rows=[["{}", "b"]], 
@hash_rows=nil, @column_types={"?column?"=>#<ActiveRecord::ConnectionAdapters::PostgreSQL::OID::Jsonb:0x000000010617dfc0 
@precision=nil, @scale=nil, @limit=nil>}>

Because these are not the real columns, PostgreSQL names them as ?column?. One of them is of type jsonb, so Active Record assigns jsonb type for ?column? here: https://github.com/rails/rails/blob/a0c1d33d2f934ebe050b8dd69336200cbca2ae3e/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb#L64-L74

But then, when we type cast the values here https://github.com/rails/rails/blob/a0c1d33d2f934ebe050b8dd69336200cbca2ae3e/activerecord/lib/active_record/result.rb#L144-L148, we get jsonb for both columns (because of the same names) and so nil for one of them.

I do not know how we should handle this. This is obviously not unique just to json[b] columns. In the meantime, aliasing n-1 columns should work, like Entry.pluck(Arel.sql("value->'a' AS a, value->'b'")).

fatkodima avatar Aug 04 '22 11:08 fatkodima

We can raise/warn, when we have duplicate columns names 🤔

fatkodima avatar Aug 04 '22 12:08 fatkodima

@skipkayhil @fatkodima Great thank you for looking into this!

So if column field name is possibly not unique, how about using column's index as types key instead of column field name?

This line: https://github.com/rails/rails/blob/a0c1d33d2f934ebe050b8dd69336200cbca2ae3e/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb#L72

change to:

        else types[i] = type

Then update ActiveRecord::Result method column_type to support column index:

https://github.com/rails/rails/blob/a0c1d33d2f934ebe050b8dd69336200cbca2ae3e/activerecord/lib/active_record/result.rb#L144-L148

If change only PosgreSQL adapter only then may support both name and index key:

        def column_type(name, index, type_overrides)
          type_overrides.fetch(name) do
            column_types.fetch(name) do
              column_types.fetch(index, Type.default_value)
            end
          end
        end

If update all adapters, then simply to:

      def column_type(name, index, type_overrides)
        type_overrides.fetch(name) do
          column_types.fetch(index, Type.default_value)
        end
      end

And column_type method invocations:

https://github.com/rails/rails/blob/a0c1d33d2f934ebe050b8dd69336200cbca2ae3e/activerecord/lib/active_record/result.rb#L112

https://github.com/rails/rails/blob/a0c1d33d2f934ebe050b8dd69336200cbca2ae3e/activerecord/lib/active_record/result.rb#L122

Update correspondingly to:

        column_type(columns.first, 0, type_overrides)

and

        columns.map.with_index { |name, i| column_type(name, i, type_overrides) }

Thank you again.

shhavel avatar Aug 05 '22 21:08 shhavel

@shhavel Seems legit. Would you like to open a PR?

fatkodima avatar Aug 05 '22 22:08 fatkodima

This issue has been automatically marked as stale because it has not been commented on for at least three months. The resources of the Rails team are limited, and so we are asking for your help. If you can still reproduce this error on the 7-0-stable branch or on main, please reply with all of the information you have about it in order to keep the issue open. Thank you for all your contributions.

rails-bot[bot] avatar Nov 03 '22 22:11 rails-bot[bot]

Relevant.

fatkodima avatar Nov 03 '22 23:11 fatkodima