rails-sqlserver / activerecord-sqlserver-adapter

SQL Server Adapter For Rails
MIT License
972 stars 558 forks source link

Rails3.1, SQLServer adapter "find" fails with tables #175

Closed linuxonrails closed 12 years ago

linuxonrails commented 12 years ago

My problem is similar to: https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/issues/118

but it is not with a view.

With other tables it works:

ruby-1.9.2-p290 :016 > User.find(2146957319)
  User Load (55.6ms)  EXEC sp_executesql N'SELECT TOP (1) [principal].[dbo].[Usuarios].* FROM [principal].[dbo].[Usuarios] WHERE [principal].[dbo].[Usuarios].[Id_Usuario] = @0', N'@0 int', @0 = 2146957319  [["Id_Usuario", 2146957319]]
 => #<User Id_Usuario: 2146957319, Fecha_Alta: "2011-11-28 11:42:00", Identificador:.....

But I have a problem with Ayuda_Consulta table:

ruby-1.9.2-p290 :016 > AyudaConsulta.last
  AyudaConsulta Load (50.8ms)  EXEC sp_executesql N'SELECT TOP (1) [zoconet_ctrl].[dbo].[Ayuda_Consulta].* FROM [zoconet_ctrl].[dbo].[Ayuda_Consulta] ORDER BY [zoconet_ctrl].[dbo].[Ayuda_Consulta].[Id_Ayuda] DESC'
 => #<AyudaConsulta Id_Ayuda: 77, Consulta: "Quiero darme de baja del servicio TSeleccion", Nivel: 2, Orden: 2900, Padre: nil, Id_Departamento: 2, Id_Operador: 8, Prioridad: 3, Requiere_Identificador: "s", Requiere_Vendedor: "s", Consejo: "", Rol: nil, Id_Lote: nil, Id_Pedido: nil, Fecha: nil, Ref_Liquidacion: nil, Id_Organizacion: 36> 
ruby-1.9.2-p290 :016 > AyudaConsulta.find(77)
  AyudaConsulta Load (51.0ms)  EXEC sp_executesql N'SELECT TOP (1) [zoconet_ctrl].[dbo].[Ayuda_Consulta].* FROM [zoconet_ctrl].[dbo].[Ayuda_Consulta] WHERE [zoconet_ctrl].[dbo].[Ayuda_Consulta].[Id_Ayuda] = @0', N'@0 int()', @0 = 77  [["Id_Ayuda", 77]]
ActiveRecord::StatementInvalid: TinyTds::Error: Must declare the scalar variable "@0".: EXEC sp_executesql N'SELECT TOP (1) [zoconet_ctrl].[dbo].[Ayuda_Consulta].* FROM [zoconet_ctrl].[dbo].[Ayuda_Consulta] WHERE [zoconet_ctrl].[dbo].[Ayuda_Consulta].[Id_Ayuda] = @0', N'@0 int()', @0 = 77
    from /home/davidm/.rvm/gems/ruby-1.9.2-p290@mobile/gems/activerecord-sqlserver-adapter-3.2.1/lib/active_record/connection_adapters/sqlserver/database_statements.rb:412:in `each'
    from /home/davidm/.rvm/gems/ruby-1.9.2-p290@mobile/gems/activerecord-sqlserver-adapter-3.2.1/lib/active_record/connection_adapters/sqlserver/database_statements.rb:412:in `handle_to_names_and_values_dblib'
    from /home/davidm/.rvm/gems/ruby-1.9.2-p290@mobile/gems/activerecord-sqlserver-adapter-3.2.1/lib/active_record/connection_adapters/sqlserver/database_statements.rb:401:in `handle_to_names_and_values'
    from /home/davidm/.rvm/gems/ruby-1.9.2-p290@mobile/gems/activerecord-sqlserver-adapter-3.2.1/lib/active_record/connection_adapters/sqlserver/database_statements.rb:373:in `_raw_select'
    from /home/davidm/.rvm/gems/ruby-1.9.2-p290@mobile/gems/activerecord-sqlserver-adapter-3.2.1/lib/active_record/connection_adapters/sqlserver/database_statements.rb:367:in `block in raw_select'
    from /home/davidm/.rvm/gems/ruby-1.9.2-p290@mobile/gems/activerecord-3.2.2/lib/active_record/connection_adapters/abstract_adapter.rb:280:in `block in log'
    from /home/davidm/.rvm/gems/ruby-1.9.2-p290@mobile/gems/activesupport-3.2.2/lib/active_support/notifications/instrumenter.rb:20:in `instrument'
    from /home/davidm/.rvm/gems/ruby-1.9.2-p290@mobile/gems/activerecord-3.2.2/lib/active_record/connection_adapters/abstract_adapter.rb:275:in `log'
    from /home/davidm/.rvm/gems/ruby-1.9.2-p290@mobile/gems/activerecord-sqlserver-adapter-3.2.1/lib/active_record/connection_adapters/sqlserver/database_statements.rb:367:in `raw_select'
    from /home/davidm/.rvm/gems/ruby-1.9.2-p290@mobile/gems/activerecord-sqlserver-adapter-3.2.1/lib/active_record/connection_adapters/sqlserver/database_statements.rb:350:in `do_exec_query'
    from /home/davidm/.rvm/gems/ruby-1.9.2-p290@mobile/gems/activerecord-sqlserver-adapter-3.2.1/lib/active_record/connection_adapters/sqlserver/database_statements.rb:24:in `exec_query'
    from /home/davidm/.rvm/gems/ruby-1.9.2-p290@mobile/gems/activerecord-sqlserver-adapter-3.2.1/lib/active_record/connection_adapters/sqlserver/database_statements.rb:293:in `select'
    from /home/davidm/.rvm/gems/ruby-1.9.2-p290@mobile/gems/activerecord-3.2.2/lib/active_record/connection_adapters/abstract/database_statements.rb:18:in `select_all'
    from /home/davidm/.rvm/gems/ruby-1.9.2-p290@mobile/gems/activerecord-3.2.2/lib/active_record/connection_adapters/abstract/query_cache.rb:63:in `select_all'
    from /home/davidm/.rvm/gems/ruby-1.9.2-p290@mobile/gems/activerecord-3.2.2/lib/active_record/querying.rb:38:in `block in find_by_sql'
    from /home/davidm/.rvm/gems/ruby-1.9.2-p290@mobile/gems/activerecord-3.2.2/lib/active_record/explain.rb:40:in `logging_query_plan'
... 1 levels...
    from /home/davidm/.rvm/gems/ruby-1.9.2-p290@mobile/gems/activerecord-3.2.2/lib/active_record/relation.rb:171:in `exec_queries'
    from /home/davidm/.rvm/gems/ruby-1.9.2-p290@mobile/gems/activerecord-3.2.2/lib/active_record/relation.rb:160:in `block in to_a'
    from /home/davidm/.rvm/gems/ruby-1.9.2-p290@mobile/gems/activerecord-3.2.2/lib/active_record/explain.rb:33:in `logging_query_plan'
    from /home/davidm/.rvm/gems/ruby-1.9.2-p290@mobile/gems/activerecord-3.2.2/lib/active_record/relation.rb:159:in `to_a'
    from /home/davidm/.rvm/gems/ruby-1.9.2-p290@mobile/gems/activerecord-3.2.2/lib/active_record/relation/finder_methods.rb:377:in `find_first'
    from /home/davidm/.rvm/gems/ruby-1.9.2-p290@mobile/gems/activerecord-3.2.2/lib/active_record/relation/finder_methods.rb:122:in `first'
    from /home/davidm/.rvm/gems/ruby-1.9.2-p290@mobile/gems/activerecord-3.2.2/lib/active_record/relation/finder_methods.rb:335:in `find_one'
    from /home/davidm/.rvm/gems/ruby-1.9.2-p290@mobile/gems/activerecord-3.2.2/lib/active_record/relation/finder_methods.rb:311:in `find_with_ids'
    from /home/davidm/.rvm/gems/ruby-1.9.2-p290@mobile/gems/activerecord-3.2.2/lib/active_record/relation/finder_methods.rb:107:in `find'
    from /home/davidm/.rvm/gems/ruby-1.9.2-p290@mobile/gems/activerecord-3.2.2/lib/active_record/querying.rb:5:in `find'
    from (irb):16
    from /home/davidm/.rvm/gems/ruby-1.9.2-p290@mobile/gems/railties-3.2.2/lib/rails/commands/console.rb:47:in `start'
    from /home/davidm/.rvm/gems/ruby-1.9.2-p290@mobile/gems/railties-3.2.2/lib/rails/commands/console.rb:8:in `start'
    from /home/davidm/.rvm/gems/ruby-1.9.2-p290@mobile/gems/railties-3.2.2/lib/rails/commands.rb:41:in `<top (required)>'
    from script/rails:6:in `require'
    from script/rails:6:in `<main>'ruby-1.9.2-p290 :017 > 

The ActiveRecord Model AyudaConsulta:

cat app/models/ayuda_consulta.rb

# encoding: UTF-8

class AyudaConsulta < ActiveRecord::Base
  def self.table_name
    "zoconet_ctrl.dbo.Ayuda_Consulta"
  end

  self.primary_key = "Id_Ayuda"
end

$ rails --version

Rails 3.2.2

gems:

tiny_tds (0.5.1) activerecord-sqlserver-adapter (3.2.1)

linuxonrails commented 12 years ago

Find in User table (it works):

...].[Id_Usuario] = @0', N'@0 int' <- only int

Find in AyudaConsulta (it doesn't work):

...Ayuda_Consulta].[Id_Ayuda] = @0', N'@0 int()', @0 = 77 [["Id_Ayuda", 77]] <- int() !!!???

metaskills commented 12 years ago

Just glanced at the info above. Most of the time issue is related to permissions or (DB) schemas when ActiveRecord and the adapter are doing its schema reflection. IE, the #column_definitions method that does all the work to determine what your table's schema contains.

The way you can tell if things are working is to do AyudaConsulta.columns and QA what ActiveRecord finds.

Also, please use self.table_name = "zoconet_ctrl.dbo.Ayuda_Consulta" vs the method definition.

dtykocki commented 12 years ago

I'm also encountering this same issue on SQL Server 2005. Here's what I have:

class User < ActiveRecord::Base
  self.table_name = "pomegadb.dbo.tblusers"
  self.primary_key = "userid"
end
User.find(1)
  User Load (2.5ms)  EXEC sp_executesql N'SELECT TOP (1) [pomegadb].[dbo].[tblusers].* FROM [pomegadb].[dbo].[tblusers] WHERE [pomegadb].[dbo].[tblusers].[userid] = @0', N'@0 int()', @0 = 1  [["userid", 1]]
TinyTds::Error: Must declare the scalar variable "@0".: EXEC sp_executesql N'SELECT TOP (1) [pomegadb].[dbo].[tblusers].* FROM [pomegadb].[dbo].[tblusers] WHERE [pomegadb].[dbo].[tblusers].[userid] = @0', N'@0 int()', @0 = 1
ActiveRecord::StatementInvalid: TinyTds::Error: Must declare the scalar variable "@0".: EXEC sp_executesql N'SELECT TOP (1) [pomegadb].[dbo].[tblusers].* FROM [pomegadb].[dbo].[tblusers] WHERE [pomegadb].[dbo].[tblusers].[userid] = @0', N'@0 int()', @0 = 1
User.columns.first
#"tblusers", :numeric_scale=>0, :numeric_precision=>10, :ordinal_position=>1, :length=>nil, :is_primary=>true, :is_identity=>false, :database_year=>2005}, @name="userid", @sql_type="int()", @null=false, @limit=4, @precision=nil, @scale=nil, @type=:integer, @default=nil, @primary=true, @coder=nil> 

I'm not completely sure if this is a schema or permission problem since all of the findby* methods are working.

metaskills commented 12 years ago

I see the problem!

SELECT TOP (1) [pomegadb].[dbo].[tblusers].* 
FROM [pomegadb].[dbo].[tblusers] 
WHERE [pomegadb].[dbo].[tblusers].[userid] = @0', N'@0 int()', @0 = 1

This relates to ticket #157, basically for some reason unknown to me is the that your data type is coming back as int() vs int(4). I would fucking love to know why, but the fix to this and a host of other related issues may be this patch below.

       def sql_type_for_statement
         if is_integer? || is_real?
-          sql_type.sub(/\(\d+\)/,'')
+          sql_type.sub(/\((\d+)?\)/,'')
         else
           sql_type
         end
Altonymous commented 12 years ago

I'm seeing something similar in my setup... It's on bigint instead of tinyint

SQL Server 2008 FreeTDS, 0.9.1 TinyTDS, 0.5.1 Rails, 3.1.3 activerecord-sqserver-adapter, 3.1.6

I am currently attempting a rails upgrade to 3.2 so I can try the new adapter to see if that resolves my issue.

metaskills commented 12 years ago

The new adapter does not resolve this. It would have to be a patch to both 3-1-stable and 3-2. It would be easy for you to monkey patch with the patch above and let me know if that fixes it for you. Also, would be nice if someone could QA and run the tests, etc and preflight this for me to help out.

Altonymous commented 12 years ago

Any ideas on why it only occurs on some tables and not others? In my case the tables it works on are all in the same database and the ones it doesn't work on are in another database. All on the same server though.

I'm going to check all the settings on the database to see if I can find a difference.

metaskills commented 12 years ago

I thought I explained it well when I said, "I would fucking love to know why.." above. I also went into detail of my frustration in trying to duplicate others schema setups in ticket #157 which you commented on too.

@Altonymous so maybe you can do some research???

Altonymous commented 12 years ago

I'm doing it now. I'll let you know if I find anything.

metaskills commented 12 years ago

Thanks, the patch is easy and would fix it. But I had no idea how other's DBs return strings like int() vs the real int(4) for some. Seems like a crazy SQL Server bug to me. Either way, the fix is easy and will go in soon. But would be nice to know why and write a regression test if possible.

Altonymous commented 12 years ago

I ran the query that pulls the information on the schema as the user the application uses and it returned what I believe is the correct data for both databases. So I'm not sure it's a problem with the query or database.

Here's what I ran...

SELECT DISTINCT 
            columns.TABLE_NAME AS table_name,
            columns.COLUMN_NAME AS name,
            columns.DATA_TYPE AS type,
            columns.COLUMN_DEFAULT AS default_value,
            columns.NUMERIC_SCALE AS numeric_scale,
            columns.NUMERIC_PRECISION AS numeric_precision,
            columns.ordinal_position,
            CASE
              WHEN columns.DATA_TYPE IN ('nchar','nvarchar') THEN columns.CHARACTER_MAXIMUM_LENGTH
              ELSE COL_LENGTH(columns.TABLE_SCHEMA+'.'+columns.TABLE_NAME, columns.COLUMN_NAME)
            END AS [length],
            CASE
              WHEN columns.IS_NULLABLE = 'YES' THEN 1
              ELSE NULL
            END AS [is_nullable],
            CASE 
              WHEN KCU.COLUMN_NAME IS NOT NULL AND TC.CONSTRAINT_TYPE = N'PRIMARY KEY' THEN 1
              ELSE NULL
            END AS [is_primary],
            CASE 
              WHEN COLUMNPROPERTY(OBJECT_ID(columns.TABLE_SCHEMA+'.'+columns.TABLE_NAME), columns.COLUMN_NAME, 'IsIdentity') = 1 THEN 1
              ELSE NULL
            END AS [is_identity]
            FROM INFORMATION_SCHEMA.COLUMNS columns
            LEFT OUTER JOIN INFORMATION_SCHEMA.TABLE_CONSTRAINTS AS TC 
              ON TC.TABLE_NAME = columns.TABLE_NAME 
              AND TC.CONSTRAINT_TYPE = N'PRIMARY KEY' 
            LEFT OUTER JOIN INFORMATION_SCHEMA.KEY_COLUMN_USAGE AS KCU
              ON KCU.COLUMN_NAME = columns.COLUMN_NAME
              AND KCU.CONSTRAINT_NAME = TC.CONSTRAINT_NAME
              AND KCU.CONSTRAINT_CATALOG = TC.CONSTRAINT_CATALOG
              AND KCU.CONSTRAINT_SCHEMA = TC.CONSTRAINT_SCHEMA
            WHERE columns.TABLE_NAME = 'inode_hourly_readings'
              AND columns.TABLE_SCHEMA = schema_name()
            ORDER BY columns.ordinal_position

Heres the output...

table_name  name    type    default_value   numeric_scale   numeric_precision   ordinal_position    length  is_nullable is_primary  is_identity
inode_hourly_readings   id  bigint  NULL    0   19  1   8   NULL    1   1
inode_hourly_readings   inode_id    bigint  NULL    0   19  2   8   NULL    NULL    NULL
inode_hourly_readings   reading_hour    datetime    NULL    NULL    NULL    3   8   NULL    NULL    NULL
inode_hourly_readings   cpu bigint  NULL    0   19  4   8   NULL    NULL    NULL
inode_hourly_readings   memory  bigint  NULL    0   19  5   8   NULL    NULL    NULL
inode_hourly_readings   disk_io bigint  NULL    0   19  6   8   NULL    NULL    NULL
inode_hourly_readings   lan_io  bigint  NULL    0   19  7   8   NULL    NULL    NULL
inode_hourly_readings   wan_io  bigint  NULL    0   19  8   8   NULL    NULL    NULL
inode_hourly_readings   storage bigint  NULL    0   19  9   8   NULL    NULL    NULL
inode_hourly_readings   consumption decimal NULL    2   18  10  9   NULL    NULL    NULL

Sorry for the poor formatting.

This is the result from the database and table that is not working.

metaskills commented 12 years ago

Well the bigint is a different case. That would not be fixed by the patch above because is_integer? would return true and the data type would not have parens after it. So it would have to be something like this sql_type.sub(/\((\d+)?\)?/,'') or maybe even some way more basic like /\W/ to match any non word character.

So to conclude (a) still do not know why int() would come back, but we can test in isolation and certainly we can test for bigint too. Someone want to do the leg work for me and add a few tests and QA that nothing fails with changes for both? That way I can do a pull request and just get this out.

Altonymous commented 12 years ago

Okay in my case I only have bigint in my database.. this is the query that got me looking into this...

TinyTds::Error: Must declare the scalar variable "@1".: EXEC sp_executesql N'INSERT INTO [Reporting].[dbo].[inode_hourly_readings] ([consumption], [cpu], [disk_io], [inode_id], [lan_io], [memory], [reading_hour], [storage], [wan_io]) VALUES (@0, @1, @2, @3, @4, @5, @6, @7, @8); SELECT CAST(SCOPE_IDENTITY() AS bigint) AS Ident', N'@0 decimal(18,2), @1 bigint(), @2 bigint(), @3 bigint(), @4 bigint(), @5 bigint(), @6 datetime, @7 bigint(), @8 bigint()', @0 = 82.0, @1 = 12651, @2 = 142, @3 = 274, @4 = 152, @5 = 914, @6 = '2012-01-11T16:16:13.897', @7 = 114, @8 = 74

As you can see it's trying to do bigint() instead of bigint(8) or bigint.

Altonymous commented 12 years ago

Another piece of information in my scenario... I just found this out.

Instead of doing... self.table_name = "Reporting.dbo.inode_hourly_readings"

If I change it to... establish_connection :reporting self.table_name = "inode_hourly_readings"

OR.... establish_connection :reporting self.table_name = "Reporting.dbo.inode_hourly_readings"

Everything works fine.

The thing is a new connection is not needed as they are the same server. Not sure if that lends any insight or not.

dtykocki commented 12 years ago

Does your database.yml include an entry for the database you're connecting to? Everything started working as soon as I added that entry.

Altonymous commented 12 years ago

Okay I'm able to reproduce this issue in SQL Server directly.

Let's say you have 2 databases... DB1 & DB2.

When you connect you connect to DB1. Let's say you want to pull the schema for a table from DB2 using the SQL above with a slight change, qualify the views with DB2. (DB2.INFORMATION_SCHEMA) You will get the schema but the length column will be NULL. However, if you switch to DB2 first and run the query it populates the length fine.

Altonymous commented 12 years ago

@dtykocki Yes like I said it works fine on any table from one database. Read my latest comment for more details on how to reproduce this. I'm working with our Data Architect to see if there's a reason length isn't getting populated when we're cross-querying between databases for the schema.

Altonymous commented 12 years ago

I'll also point out on this page...

http://msdn.microsoft.com/en-us/library/ms181757.aspx

It repeatedly says DO NOT use INFORMATION_SCHEMA to determine the schema of an object.

"Do not use INFORMATION_SCHEMA views to determine the schema of an object. The only reliable way to find the schema of a object is to query the sys.objects catalog view."

metaskills commented 12 years ago

@Altonymous Thanks for digging. So it appears to me that we are at the following situation.

1) No matter what happens, a few simply patches in the #sql_type_for_statement will be good. If for nothing else, a sane guard against this type of thing.

2) That the root cause may be identified and related to use a single database connection to cross DBs? If that is so, I had to say that is an anti-pattern. ActiveRecord should have a connection/pool to each database. Typically this is done by giving a class the responsibility of the connection, using establish_connection and then sub class that for other models. So OtherDb::Connection would be the abstract connection class to establish_connection :other_db and models like OtherDb::InodeHourlyReading would have its super class set to the OtherDb::Connection.

3) That you have brought up the use of INFORMATION_SCHEMA. Which I am all ears for, but not in the context of this ticket as we have #1 and #2 in either isolation or concert that addresses the issue. If someone want to create a patch to use sys.objects and friends in an improvement issue, I am all for it.

Does that sound about right?

Altonymous commented 12 years ago

Yes but I have found a way to fix the cross database issues. I'm working on a patch for it right now. It's simple really.

Give me a few minutes.

metaskills commented 12 years ago

That's what I like to hear :)

Altonymous commented 12 years ago

Okay I have a partial fix. I'm still working on how to resolve the COLUMNPROPERTY() method. The OBJECTID is now being gathered correctly in order to identify the identity column. However, the COLUMNPROPERTY isn't working correctly yet.

SELECT COLUMNPROPERTY(OBJECT_ID('Reporting.dbo.inode_hourly_readings'), 'id', 'IsIdentity')

You can check my fork for the partial fix.