datashard: add NULL value check to TValidatedWriteTxOperation::ParseOperation #21560
Conversation
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
🟢 |
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
|
||
| Cout << "========= Send immediate write with NULL value for NOT NULL column=========\n"; | ||
| { | ||
| TSerializedCellMatrix matrix({TCell{}}, 1, 1); |
There was a problem hiding this comment.
На всякий случай упомяну, что этого теста недостаточно. Если я правильно понял в этом тесте создаётся таблица с единственной ключевой колонкой, и по своей сути ключевая колонка должна обязательно быть указана. Интереснее случай, когда в таблице например 3 колонки (key, value1, value2 not null) и делают UPSERT с указанием (key, value1) - обе с not null значением, а колонку value2 не указывают. Из-за того, что на уровне локальной базы у неё нет дефолта туда вставится NULL. И здесь появляется интересная проблема, что если по ключу уже есть строка, то это не ошибка (т.к. это UPDATE), а если строки нет - то ошибка (т.к. это INSERT). То есть в таких случаях мы не можем проверить корректность операции на стадии валидации, из-за того что KQP нам UPDATE присылает как UPSERT, и это видимо можно проверить только во время выполнения. Однако если операция INSERT, или в списке колонок указали явный NULL, то проверить во время валидации можно.
There was a problem hiding this comment.
На кофепоинте gvit@ сказал, что все эти contraint'ы всё-равно несовместимы с быстрым UPSERT, и будет нормально всегда читать если есть NOT NULL колонки, которые не указаны в операции. Мы с дефолтами вобщем-то уже так читаем, и с тех пор как у нас незакомиченные изменения и волатильные транзакции появились вынуждены читать для проверки конфликтов, так что особых проблем вроде бы нет.
There was a problem hiding this comment.
Добавил проверку для INSERT и тесты.
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
…tion::ParseOperation
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
Fixes #21475
Changelog entry
Changelog category
Description for reviewers