ActiveRecord::StatementInvalid (TinyTds::Error: Incorrect syntax near the keyword 'clustered'.)

[jjimenez]

Issue

When trying to create a clustered index, the wrong sql statement is generated. We do this when we create reporting tables from our rails data. We drop and recreate tables and re-create indexes that were removed.

Expected behavior

connection.add_index "testings", "last_name", type: :clustered should generate sql: "CREATE CLUSTERED INDEX [index_testings_on_last_name] ON [testings] ([last_name])"

Actual behavior

sql generated is "CREATE INDEX clustered [index_testings_on_last_name] ON [testings] ([last_name])"

How to reproduce

require "bundler/inline"

gemfile(true) do source "https://rubygems.org" gem "tiny_tds" gem "activerecord", "6.1.0" gem "activerecord-sqlserver-adapter", "6.1.0" end

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

ActiveRecord::Base.establish_connection( adapter: "sqlserver", timeout: 5000, pool: 100, encoding: "utf8", database: "activerecord_unittest", username: "rails", password: "", host: "localhost", port: 1433, ) ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do drop_table :testings rescue nil

create_table :testings, force: true do |t| t.column :foo, :string, limit: 100 end end

class Testings < ActiveRecord::Base end

class TestBugTest < Minitest::Test def test_index_type Testings.connection.add_index "testings", "id", type: :clustered end end

Details

• Rails version: 6.1.0
• SQL Server adapter version: 6.1.0
• TinyTDS version: 2.1.5

• FreeTDS details:

  
  run `tsql -C` and paste here the output.

Compile-time settings (established with the "configure" script) Version: freetds v1.3.7 freetds.conf directory: /opt/homebrew/etc MS db-lib source compatibility: no Sybase binary compatibility: yes Thread safety: yes iconv library: yes TDS version: 7.3 iODBC: no unixodbc: yes SSPI "trusted" logins: no Kerberos: yes OpenSSL: yes GnuTLS: no MARS: yes

[wpolicarpo]

Thanks for the detailed report. I'll look into this soon.

[jjimenez]

Thanks, @wpolicarpo.

if it helps in activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb, rails defines visit_CreateIndexDefinition as:

     def visit_CreateIndexDefinition(o)
          index = o.index

          sql = ["CREATE"]
          sql << "UNIQUE" if index.unique
          sql << "INDEX"
          sql << o.algorithm if o.algorithm
          sql << "IF NOT EXISTS" if o.if_not_exists
          sql << index.type if index.type
          sql << "#{quote_column_name(index.name)} ON #{quote_table_name(index.table)}"
          sql << "USING #{index.using}" if supports_index_using? && index.using
          sql << "(#{quoted_columns(index)})"
          sql << "WHERE #{index.where}" if supports_partial_index? && index.where

          sql.join(" ")
        end

which has the type in the wrong place for sqlserver which expects it after "UNIQUE" see https://docs.microsoft.com/en-us/sql/t-sql/statements/create-index-transact-sql?view=sql-server-ver15#syntax-for-sql-server-and-azure-sql-database

in lib/active_record/connection_adapters/sqlserver/schema_creation.rb for the adapter, the method is overwritten to apply the appropriate "IF NOT EXISTS", and calls super. I wasn't able to find the place that the behavior changed from version to version in either rails or the adapter, but I'm fairly certain that is where the issue comes from.

On my prod system I've created my own adapter override that reads:

def visit_CreateIndexDefinition(o)
          index = o.index

          sql = ["CREATE"]
          sql << "UNIQUE" if index.unique
          sql << index.type if index.type
          sql << "INDEX"
          sql << o.algorithm if o.algorithm
          sql << "#{quote_column_name(index.name)} ON #{quote_table_name(index.table)}"
          sql << "USING #{index.using}" if supports_index_using? && index.using
          sql << "(#{quoted_columns(index)})"
          sql << "WHERE #{index.where}" if supports_partial_index? && index.where

          sql = sql.join(" ")

          if o.if_not_exists
            sql = "IF NOT EXISTS (SELECT name FROM sysindexes WHERE name = '#{o.index.name}') #{sql}"
          end

          sql
        end

I've been attempting to do a pull request, for the adapter, but I'm still working on it.

[wpolicarpo]

Fixed in #1002. I'll cut a new release soon.