Quote columns and table name

[JaBistDuNarrisch]

No description provided.

[Copilot]

Pull request overview

This pull request adds table and column name quoting to constraint methods to properly handle reserved words and special characters in database identifiers.

Changes:

• Removed unused System.Reflection import
• Added column and table name quoting to AddPrimaryKey method
• Changed AddUniqueConstraint to use QuoteColumnNamesIfRequired instead of the private QuoteColumnNames method

Comments suppressed due to low confidence (17)

src/Migrator/Providers/TransformationProvider.cs:1789

• This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.

        foreach (var col in columns)
        {
            if (col.IsPrimaryKey)
            {
                primaryKeys.Add(col.Name);
            }
        }

src/Migrator/Providers/TransformationProvider.cs:1840

• This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.

        foreach (var column in columns)
        {
            var indexSql = column.IndexSql;

            if (indexSql != null)
            {
                indexes.Add(indexSql);
            }
        }

src/Migrator/Providers/TransformationProvider.cs:2048

• Condition is always false because of ... is ....
  Condition is always false because of ... == ....

            parameter.Value = value == null ? null : ((DateTimeOffset?)value).Value.ToUniversalTime();

src/Migrator/Providers/TransformationProvider.cs:1046

• Redundant call to 'ToString' on a String object.

        var query = string.Format("SELECT {0} FROM {1}", builder.ToString(), table);

src/Migrator/Providers/TransformationProvider.cs:1050

• Redundant call to 'ToString' on a String object.

            query = string.Format("SELECT {0} FROM {1} WHERE ", builder.ToString(), table);

src/Migrator/Providers/TransformationProvider.cs:1200

• Redundant call to 'ToString' on a String object.

        var query = string.Format("UPDATE {0} SET {1}", table, builder.ToString());

src/Migrator/Providers/TransformationProvider.cs:1278

• Redundant call to 'ToString' on a String object.

        var query = string.Format("UPDATE {0} SET {1} WHERE {2}", table, builder.ToString(), GetWhereStringWithNullCheck(whereColumns, whereValues, values.Length));

src/Migrator/Providers/TransformationProvider.cs:321

• String concatenation in loop: use 'StringBuilder'.

                joins += string.Format("JOIN {0} {1} ON {1}.{2} = {3}.{4} ", viewField.TableName, " T" + nr,
                    viewField.KeyColumnName, viewField.ParentTableName, viewField.ParentKeyColumnName);

src/Migrator/Providers/TransformationProvider.cs:362

• String concatenation in loop: use 'StringBuilder'.

            joins += string.Format("{0} {1} {2} ON {2}.{3} = {4}.{5} ", joinType, viewJoin.TableName, tableAlias,
                viewJoin.ColumnName, viewJoin.ParentTableName, viewJoin.ParentColumnName);

src/Migrator/Providers/TransformationProvider.cs:1077

• This assignment to andNeeded is useless, since its value is never read.

            andNeeded = true;

src/Migrator/Providers/TransformationProvider.cs:1712

• These 'if' statements can be combined.

        if (_connection != null && _connection.State == ConnectionState.Open)
        {
            if (!_outsideConnection)
            {
                _connection.Close();
            }
        }

src/Migrator/Providers/TransformationProvider.cs:1720

• These 'if' statements can be combined.

        if (_connection != null)
        {
            if (!_outsideConnection)
            {
                _connection.Close();
            }
        }

src/Migrator/Providers/TransformationProvider.cs:1802

• These 'if' statements can be combined.

        if (defaultValue is DateTime defaultValueDateTime)
        {
            if (defaultValueDateTime.Kind != DateTimeKind.Utc)
            {
                throw new Exception("Only UTC values are accepted as default DateTime values.");
            }
        }

src/Migrator/Providers/TransformationProvider.cs:2216

• These 'if' statements can be combined.

        if (hasFilterItems)
        {
            if (!index.FilterItems.All(x => index.KeyColumns.Any(y => x.ColumnName.Equals(y, StringComparison.OrdinalIgnoreCase))))
            {
                throw new MigrationException($"All columns in the {nameof(index.FilterItems)} should exist in the {nameof(index.KeyColumns)}.");
            }
        }

src/Migrator/Providers/TransformationProvider.cs:2229

• These 'if' statements can be combined.

        if (index.IncludeColumns != null && index.IncludeColumns.Length > 0)
        {
            if (index.IncludeColumns.Any(x => index.KeyColumns.Any(y => x.Equals(y, StringComparison.OrdinalIgnoreCase))))
            {
                throw new MigrationException($"It is not allowed to use a column in {nameof(index.IncludeColumns)} that exist in {nameof(index.KeyColumns)}.");
            }
        }

src/Migrator/Providers/TransformationProvider.cs:39

• Field '_scope' can be 'readonly'.

    private string _scope;

src/Migrator/Providers/TransformationProvider.cs:1948

• Both branches of this 'if' statement return - consider using '?' to express intent better.

            if (null == val)
            {
                return "null";
            }
            else
            {
                return string.Format("'{0}'", val.Replace("'", "''"));
            }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Column names are being quoted twice. The call to QuoteColumnNamesIfRequired(columns) on line 727 modifies the array in-place, but then line 732 calls QuoteColumnNamesIfRequired(columns) again on the already-quoted column names. This will result in double-quoting. Either remove line 727 or use the array directly in line 732 without calling the method again.

Suggested change

	string.Join(",", QuoteColumnNamesIfRequired(columns))));
	string.Join(",", columns)));

[Copilot]

After quoting the column names on line 740, the code should use the quoted columns array that was returned, but line 743 uses the columns variable directly. Since QuoteColumnNamesIfRequired modifies the array in-place and returns it, the code works correctly but is inconsistent with how the method is called. For clarity and consistency, use the return value: string.Join(", ", QuoteColumnNamesIfRequired(columns)) or store the result in a variable on line 740.

Suggested change

	ExecuteNonQuery(string.Format("ALTER TABLE {0} ADD CONSTRAINT {1} UNIQUE({2}) ", table, name, string.Join(", ", columns)));
	ExecuteNonQuery(string.Format("ALTER TABLE {0} ADD CONSTRAINT {1} UNIQUE({2}) ", table, name, string.Join(", ", QuoteColumnNamesIfRequired(columns))));