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.