Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix sql server queries formatting #17

Merged
merged 3 commits into from
May 5, 2022

Conversation

EvgeniyShvetcov
Copy link
Contributor

Разбирался, почему не получается накатить миграцию. Обнаружил, что некоторые запросы форматируются в '[SomeName]', хотя по факту должно быть или [SomeName] или 'SomeName'. Поправил запросы для Sql Server.
По логике мы можем использовать или {0:Name} или '{0}' форматирование

@dima117
Copy link
Member

dima117 commented Apr 28, 2022

Добрый день! Правильно использовать {0:NAME}. Поправьте, пожалуйста код, чтобы использовался этот способ.

@EvgeniyShvetcov
Copy link
Contributor Author

А в случаях с object_id(N'SomeObject') как быть? Сделать просто object({0:Name})?

@dima117
Copy link
Member

dima117 commented Apr 28, 2022

Сходу не могу сказать, давайте я сам проверю сегодня вечером, какие запросы генерируются.

Можете сделать пример миграции, которая не проходит? Напишу тест для него и буду на нем отлаживать.

@EvgeniyShvetcov
Copy link
Contributor Author

Вернул для object_id обратно. Эта функция оказывается умеет работать с квадратными скобками, а вот для условия сравнения они не нужны

@@ -281,7 +281,7 @@ protected string FindConstraints(SchemaQualifiedObjectName table, string column)
"dobj", "object_id", "col", "default_object_id", "type"));

sqlBuilder.Append(FormatSql(
"WHERE {0:NAME}.{1:NAME} = object_id(N'{2}') AND {0:NAME}.{3:NAME} = '{4}'",
"WHERE {0:NAME}.{1:NAME} = object_id(N'{2:NAME}') AND {0:NAME}.{3:NAME} = '{4}'",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вот здесь получается мы сравнивали с '[SomeColumnName]' вместо 'SomeColumnName'

@EvgeniyShvetcov
Copy link
Contributor Author

Миграция в духе: сначала добавили столбец с ограничениями или ключами. В следующей пытаемся его удалить. Мигратор не находил ограничения на столбец, пытался его удалить и уже падала ошибка из БД, что столбец зависим от ограничения.

@dima117
Copy link
Member

dima117 commented Apr 28, 2022

подскажите, вот этот тест похож на ситуацию, которую вы описываете?

https://github.com/thinking-home/migrator/blob/master/ThinkingHome.Migrator.Tests/TransformationProviderTestBase.cs#L966-L984

@EvgeniyShvetcov
Copy link
Contributor Author

Не совсем то. Надо скорее всего написать тест на то, что при удалении столбца так же удаляются связанные с ним ограничения т.е. добавляем столбец, добавляем ограничение. Удаляем столбец, ошибок нет и ограничения тоже.
Получается, что метод FindContraints неправильно отрабатывает.
Или можно проще сделать. Написать тест на FindContraints добавить ограничение и проверить, что он его найдёт

@EvgeniyShvetcov
Copy link
Contributor Author

Получилось посмотреть или может быть помощь какая-то нужна?) Можно обсудить где-то будет. Дискорд там или ещё что-то

@dima117
Copy link
Member

dima117 commented May 5, 2022

Добрый день! Извините за задержку с ответом!

Проверил, действительно, был баг в методе FindConstraints при сравнении имени колонки.

Ваши исправления корректны. Добавьте, пожалуйста, тест, который добавляет для колонок таблицы все типы ограничений и проверяет, что колонки удаляются успешно. Если нужна будет помощь с запуском тестов, то готов созвониться.

@dima117 dima117 merged commit fe3e383 into thinking-home:master May 5, 2022
@dima117
Copy link
Member

dima117 commented May 5, 2022

@EvgeniyShvetcov Опубликовал пакеты версии 3.4 с вашими изменениями.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants