Create index with include columns dont work

[cpistiner]

Describe the bug
Create an index using WithOptions().Include("ColumnName") but don't work. The index is created without INCLUDE clause.

To Reproduce
Create.Index("IX_AvisoDeDeudaDRC").OnTable("DetalleResumenDeCuenta")
.OnColumn("Saldo").Ascending()
.WithOptions().NonClustered()
.Include("IdComprobante").Include("IdResponsable");

Expected behavior
I expected this:
CREATE NONCLUSTERED INDEX [IX_AvisoDeDeudaDRC]
ON [dbo].[DetalleResumenDeCuenta]([Saldo] ASC)
INCLUDE([IdComprobante], [IdResponsable]);

But the result is:
CREATE NONCLUSTERED INDEX [IX_AvisoDeDeudaDRC]
ON [dbo].[DetalleResumenDeCuenta]([Saldo] ASC);

Information (please complete the following information):

• OS: Windows 11
• Platform [.NET Framework 4.8]
• FluentMigrator version [ e.g 5.1.0 ]
• FluentMigrator runner [ FluentMigrator.Console.5.1.0 ]
• Database Management System [ SQLServer ]
• Database Management System Version [ "SQL Server 2017" ]

Additional context

[schambers]

@cpistiner, I'm unable to reproduce this behavior. Could you provide a unit test or an exmaple program that demonstrates the issue? For instance, the following test passes:

[Test]
public void CanGenerateIndexWithColumns() {   
var expression = new CreateIndexExpression();
    expression.Index.Name = "IDX_Test";
    expression.Index.TableName = "TestTable";
    expression.Index.Columns.Add(new IndexColumnDefinition {
        Name = "Column1",
        Direction = Direction.Ascending
    });

    var includes = expression.Index.GetAdditionalFeature(SqlServerExtensions.IncludesList, () => new List<IndexIncludeDefinition>());
    includes.Add(new IndexIncludeDefinition { Name = "Id1" });
    includes.Add(new IndexIncludeDefinition { Name = "Id2" });

    var result = Generator.Generate(expression);
    result.ShouldBe("CREATE INDEX [IDX_Test] ON [dbo].[TestTable] ([Column1] ASC) INCLUDE ([Id1], [Id2])");
}

Results in a pass:
> CREATE INDEX [IDX_Test] ON [dbo].[TestTable] ([Column1] ASC) INCLUDE ([Id1], [Id2])

[jzabroski]

There might be a subtle bug introduced in 5.1.0 by my change to make Create.UniqueConstraint API as complete as Create.Index

It might also be that it generates the right sql but logs it to the console incorrectly.

[cpistiner]

Hi @schambers

I'm not sure it's the same scenario. I run a Migration class Up() method in project build events.

public class M0556 : Migration
{
	public override void Up()
	{
		Create.Index("IX_AvisoDeDeudaDRC").OnTable("DetalleResumenDeCuenta")
			.OnColumn("Saldo").Ascending()
			.WithOptions().NonClustered()
			.Include("IdComprobante").Include("IdResponsable");
	}

	public override void Down()
	{
		Delete.Index("IX_AvisoDeDeudaDRC").OnTable("DetalleResumenDeCuenta");
	}
}

[cpistiner]

Hi @jzabroski

Expected behavior
I expected this:
CREATE NONCLUSTERED INDEX [IX_AvisoDeDeudaDRC]
ON [dbo].[DetalleResumenDeCuenta]([Saldo] ASC)
INCLUDE([IdComprobante], [IdResponsable]);

But the result is:
CREATE NONCLUSTERED INDEX [IX_AvisoDeDeudaDRC]
ON [dbo].[DetalleResumenDeCuenta]([Saldo] ASC);

The result is exactly what SQL shows me. I select the index generated and then "View Code" action.

[schambers]

@cpistiner My test should end up being the same outcome but there's a chance I missed something there.

@jzabroski, If you're able to point me in a specific area I can try to identify if there's a bug here. I see some commits from our changes you mentioned in 5.1 but I'm unsure how they would cause this bug

[jzabroski]

@schambers I would just put the example code he gave in the Example app in the main branch. You can run it against SQL Server LocalDB on Windows. but with MAC OSX, you will need something like this using a docker container: https://www.marcusturewicz.com/blog/develop-against-sql-server-on-a-mac-with-docker-and-vs-code/

[jzabroski]

@cpistiner Can you confirm you are running both the migrations assembly and the runner with 5.1.0? I was thinking about this on my morning commute and thats the only thing that makes me doubt your report contains everything needed to repro.

[cpistiner]

Hi @jzabroski

Sorry for the delay in responding. Yes, I confirm. I'm using 5.1.0.

This is the command:

run Migrate.exe from the 5.1.0 version downloaded from nuget.

[cpistiner]

Hi @schambers

I'm trying to do a unit test based on your example but i dont know one thing. The class Generator is part of FluentMigrator?

var result = Generator.Generate(expression);

[TestMethod]
public void CanGenerateIndexWithColumns()
{
	var expression = new CreateIndexExpression();
	expression.Index.Name = "IDX_Test";
	expression.Index.TableName = "DetalleResumenDeCuenta";
	expression.Index.Columns.Add(new IndexColumnDefinition
	{
		Name = "Saldo",
		Direction = Direction.Ascending
	});

	var includes = expression.Index.GetAdditionalFeature(SqlServerExtensions.IncludesList, () => new List<IndexIncludeDefinition>());
	includes.Add(new IndexIncludeDefinition { Name = "IdComprobante" });
	includes.Add(new IndexIncludeDefinition { Name = "IdResponsable" });

	var result = Generator.Generate(expression);

	Assert.AreEqual(result, "CREATE INDEX [IDX_Test] ON [dbo].[DetalleResumenDeCuenta] ([Saldo] ASC) INCLUDE ([IdComprobante], [IdResponsable])");
}

[jzabroski]

@schambers I'm doing a deeper review to understand what is going on here. If you have time to run the below new unit tests and debug them, that would be very helpful. - I am just writing them here directly in the GitHub comments without testing.

This test seems to almost directly test your scenario:

fluentmigrator/test/FluentMigrator.Tests/Unit/Generators/SqlServer2008/SqlServer2008IndexTests.cs

Lines 268 to 275 in ce086ce

	[Test]
	public void CanCreateIndexWithIncludedColumnAndFilter()
	{
	var expression = GeneratorTestHelper.GetCreateIndexExpression();
	var x = new CreateIndexExpressionBuilder(expression).Filter("TestColumn2 IS NULL").Include("TestColumn3");
	var result = _generator.Generate(expression);
	result.ShouldBe("CREATE INDEX [TestIndex] ON [dbo].[TestTable1] ([TestColumn1] ASC) INCLUDE ([TestColumn3]) WHERE TestColumn2 IS NULL");
	}

The precise regression test should probably be:

 [Test] 
 public void CanCreateIndexWithMultipleIncludeColumnStatements() 
 { 
     var expression = GeneratorTestHelper.GetCreateIndexExpression(); 
     var x = new CreateIndexExpressionBuilder(expression).Include("TestColumn2").Include("TestColumn3"); 
     var result = _generator.Generate(expression); 
     result.ShouldBe("CREATE INDEX [TestIndex] ON [dbo].[TestTable1] ([TestColumn1] ASC) INCLUDE ([TestColumn2], [TestColumn3])"); 
 }

 [Test] 
 public void CanCreateIndexWithOneIncludeStatementMultipleColumns() 
 { 
     var expression = GeneratorTestHelper.GetCreateIndexExpression(); 
     var x = new CreateIndexExpressionBuilder(expression).Include("TestColumn2", "TestColumn3"); 
     var result = _generator.Generate(expression); 
     result.ShouldBe("CREATE INDEX [TestIndex] ON [dbo].[TestTable1] ([TestColumn1] ASC) INCLUDE ([TestColumn2], [TestColumn3])"); 
 }

Tangential but unrelated: I reviewed a bunch of the tests here this morning to understand if there is any other potential regression.

This test assertion surprises me, as I would expect the second test to produce a different output:

fluentmigrator/test/FluentMigrator.Tests/Unit/Generators/SqlServer2005/SqlServer2005IndexTests.cs

Lines 121 to 139 in ce086ce

	[Test]
	public void CanCreateUniqueIndexWithDistinctNulls()
	{
	var expression = GeneratorTestHelper.GetCreateUniqueIndexExpression();
	expression.Index.Columns.First().SetAdditionalFeature(SqlServerExtensions.IndexColumnNullsDistinct, true);
	var result = Generator.Generate(expression);
	result.ShouldBe("CREATE UNIQUE INDEX [TestIndex] ON [dbo].[TestTable1] ([TestColumn1] ASC)");
	}
	[Test]
	public void CanCreateUniqueIndexIgnoringNonDistinctNulls()
	{
	var expression = GeneratorTestHelper.GetCreateUniqueIndexExpression();
	expression.Index.Columns.First().SetAdditionalFeature(SqlServerExtensions.IndexColumnNullsDistinct, false);
	var result = Generator.Generate(expression);
	result.ShouldBe("CREATE UNIQUE INDEX [TestIndex] ON [dbo].[TestTable1] ([TestColumn1] ASC)");
	}

It looks like this IndexColumnNullsDistinct feature was introduced in SqlServer2008Generator :

fluentmigrator/src/FluentMigrator.Runner.SqlServer/Generators/SqlServer/SqlServer2008Generator.cs

Lines 78 to 107 in ce086ce

	public virtual string GetWithNullsDistinctString(IndexDefinition index)
	{
	bool? GetNullsDistinct(IndexColumnDefinition column)
	{
	return column.GetAdditionalFeature(SqlServerExtensions.IndexColumnNullsDistinct, (bool?)null);
	}
	var indexNullsDistinct = index.GetAdditionalFeature(SqlServerExtensions.IndexColumnNullsDistinct, (bool?)null);
	var nullDistinctColumns = index.Columns.Where(c => indexNullsDistinct != null || GetNullsDistinct(c) != null).ToList();
	if (nullDistinctColumns.Count != 0 && !index.IsUnique)
	{
	// Should never occur
	CompatibilityMode.HandleCompatibilty("With nulls distinct can only be used for unique indexes");
	return string.Empty;
	}
	// The "Nulls (not) distinct" value of the column
	// takes higher precedence than the value of the index
	// itself.
	var conditions = nullDistinctColumns
	.Where(x => (GetNullsDistinct(x) ?? indexNullsDistinct ?? true) == false)
	.Select(c => $"{Quoter.QuoteColumnName(c.Name)} IS NOT NULL");
	var condition = string.Join(" AND ", conditions);
	if (condition.Length == 0)
	return string.Empty;
	return $" WHERE {condition}";
	}

[cpistiner]

Hi all,

I can do the unit test and it's wokrs! But i dont know if the same code to run with Migrate class.

Again I tell you how I execute the database update.

I have a command to run Migrate.exe when compile the solution.

The command is:

packages\FluentMigrator.Console.5.1.0\tools\net48\any\Migrate /configPath=Web\Web.config /connectionString=transoftWeb /timeout=600 /db sqlserver2012 /target Infraestructura\bin\Debug\Infraestructura.dll /task=migrate

This command execute the migrate class.

Migrate class

public class M0556 : Migration
{
	public override void Up()
	{
		Create.Index("IX_AvisoDeDeudaDRC").OnTable("DetalleResumenDeCuenta")
			.OnColumn("Saldo").Ascending()
			.WithOptions().NonClustered()
			.Include("IdComprobante").Include("IdResponsable");
	}

	public override void Down()
	{
		Delete.Index("IX_AvisoDeDeudaDRC").OnTable("DetalleResumenDeCuenta");
	}
}

Unit test

[TestInitialize]
public void Initialize()
{
	_generator = new SqlServer2008Generator();
}

[TestMethod]
public void CanGenerateIndexWithColumns()
{
	var expression = new CreateIndexExpression();
	expression.Index.Name = "IDX_Test";
	expression.Index.TableName = "DetalleResumenDeCuenta";
	expression.Index.Columns.Add(new IndexColumnDefinition
	{
		Name = "Saldo",
		Direction = Direction.Ascending
	});

	var includes = expression.Index.GetAdditionalFeature(SqlServerExtensions.IncludesList, () => new List<IndexIncludeDefinition>());
	includes.Add(new IndexIncludeDefinition { Name = "IdComprobante" });
	includes.Add(new IndexIncludeDefinition { Name = "IdResponsable" });

	var result = _generator.Generate(expression);

	Assert.AreEqual(result, "CREATE INDEX [IDX_Test] ON [dbo].[DetalleResumenDeCuenta] ([Saldo] ASC) INCLUDE ([IdComprobante], [IdResponsable])");
}

[jzabroski]

@cpistiner The code works fine. See attached PR which I will make part of the core demo.

My guess is you accidentally included Postgres extension methods rather than SqlServer.