PNixx / clickhouse-activerecord

A Ruby database ActiveRecord driver for ClickHouse
MIT License
198 stars 100 forks source link

Integer wrongly typecasted to strings in certain situations #92

Closed jbwl closed 1 year ago

jbwl commented 1 year ago

Gem version: 0.5.15 / branch: rails_7 Rails version: 7.0.8 (also tested on 6.x)

Expected result: This adapter should not cast int values to strings when a sum() with no column alias or an arbitrary alias is being used, like the MySQL adapter can. Actual result: This adapter fails to typecast correctly when either no, or an arbitrary alias is applied to a summed int column.

This works correctly:

=> MyModel.select("productid,sum(units) as units").group('productid').where(productid: 'abc')

#<MyModel:0x0000ffff8f682448> {
         "productid" => "abc",
         "units" => 999
    }

This does not (no alias name on sum column) leads to int value being cast to string:

=> MyModel.select("productid,sum(units)").group('productid').where(productid: 'abc')

#<MyModel:0x0000ffff8f682448> {
         "productid" => "abc",
         "sum(units)" => "999"
    }

This also does not work (alias name on sum column != column name) leads to int value being cast to string:

=> MyModel.select("productid,sum(units) as my_units").group('productid').where(productid: 'abc')

#<MyModel:0x0000ffff8f682448> {
         "productid" => "abc",
         "my_units" => "999"
    }

It looks like the type casting happens whenever the adapter is unable to detect the column used, i.e. "sum(units)" isn't detected as being the units column, and also "sum(units) as my_units" isn't. This flaw does not exist for example in the MySQL adapter.

Because we are using higher level query builders (i.e. ActiveCube) and have no control over the aliases used in the queries, it's mandatory that it is handled correctly on the adapter level.

jbwl commented 1 year ago

closing this as I was not using the rails_7 branch. Bug still exists there, so I'm reopening over there

PNixx commented 1 year ago

Fixed in https://github.com/PNixx/clickhouse-activerecord/commit/191ac67ed5eba2e1cbd20306d855b85b603e54d4

jbwl commented 1 year ago

Sorry to bother, but could you fix this for the rails_7 branch also, please? Currently it's still at 0.5.15. Thank you so much!

PNixx commented 1 year ago

Use can use:

gem 'clickhouse-activerecord', github: 'PNixx/clickhouse-activerecord', branch: 'rails_7'
jbwl commented 1 year ago

Yes, this is the exact line in my Gemfile, but still 0.5.15 will be installed by Bundler. It does not have the fix for typecasting.

Output of

$ bundle exec gem dep -R
Gem clickhouse-activerecord-0.5.15
  activerecord (>= 7.1)
  bundler (>= 1.13.4)
  bundler (>= 1.15, development)
  pry (~> 0.12, development)
  rake (~> 13.0, development)
  rspec (~> 3.4, development)
PNixx commented 1 year ago

latest v0.6.0

PNixx commented 1 year ago

rails_7 branch minimum required Rails 7.1

jbwl commented 1 year ago

latest v0.6.0

Yes, that's what I want, but bundler does not install it, it installs 0.5.15 with gem 'rails', '~> 7.1' My Gemfile definitely has

gem 'rails', '<8.0'
gem 'clickhouse-activerecord', github: 'PNixx/clickhouse-activerecord', branch: 'rails_7'

bundle update does not help, no update to 0.6.0 happens.

I am not familiar with bundler enough, how can I install the latest version 0.6.0 with Bundler? My Rails gem in this bundle is 7.1 (ActiveRecord etc.)

Thank you for your help!