livehelpnow / tds_ecto

TDS Adapter for Ecto
57 stars 34 forks source link

do not check for schema in migrations if prefix is empty #60

Closed bottlenecked closed 6 years ago

bottlenecked commented 6 years ago

Hi all, when running migrations from the test suite I get back the following error

**  (Tds.Error) Line 1 (Error 2714): There is already an object named 'schema_migrations' in the database.

Digging in some more, it seems that the CREATE TABLE statement creates tables under the default schema when no schema is specified, ie. dbo. But the following query explicitly check for an empty schema name when running migrations, so that the create table statement is always executed and fails subsequent executions with the error above.

IF NOT EXISTS ( SELECT * FROM [INFORMATION_SCHEMA].[TABLES] info WHERE info.[TABLE_NAME] = 'schema_migrations' AND info.[TABLE_SCHEMA] = '' )
BEGIN
  CREATE TABLE [schema_migrations] ([version] bigint, [inserted_at] datetime, CONSTRAINT [PK__schema_migrations] PRIMARY KEY CLUSTERED ([version]))
END

So I propose the info.[TABLE_SCHEMA] = '#{prefix}' check is eschewed completely when no prefix is specified.

Running SELECT @@VERSION gives the following:

Microsoft SQL Server 2016 (SP1) (KB3182545) - 13.0.4001.0 (X64) 
Oct 28 2016 18:17:30 
Copyright (c) Microsoft Corporation
Express Edition (64-bit) on Windows 10 Pro 6.3 <X64> (Build 16299: )

Does this happen to anyone else?

mjaric commented 6 years ago

Hi, thanks for contributing.

I think no one had this issue since old version 1.x of tds_ecto forced dbo schema. I added prefix support when started working on version 2 and tried at first to force prefix to dbo if not specified. Later, I realised that user default schema could be set to something else. So I removed that code. This is probably point which I didn't check at that point.

Currently I'm preparing some improvements in tds related to varchar/nvarchars, so once I'm done I will merge this change too.

Thank you!

bottlenecked commented 6 years ago

Great, good to know :)