parse and pass streamlookup parameters, YDB part #12548
Conversation
[includes yql part, but it will disappear from resulting patch once merged on arc side]
|
⚪ |
|
⚪ |
|
⚪ 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 |
|
⚪ 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 |
| if (name == "LeftAny"sv) { | ||
| leftAny = true; | ||
| continue; | ||
| } else if (name == "RightAny"sv) { |
There was a problem hiding this comment.
Да, но, кмк, в случае исключающих альтернатив, использовать всегда if () {} else if () {} лучше для консистентности. Меняем/переносим continue, и, внезапно, надо менять и следующую строчку (или, хуже, забываем поменять).
| if (false) { // Tempoarily change to waring to allow for smooth transition | ||
| ctx.AddError(TIssue(ctx.GetPosition(join.Pos()), "Streamlookup: must be LEFT JOIN ANY")); | ||
| return {}; | ||
| } else { |
There was a problem hiding this comment.
Аналогично. И тут похожий код в обеих ветках, одинаковый отступ упрощает сравнение.
if () { /* коротко */ return; } else { /* длинно, иначе и вложенно */ } -- да, прямой смысл избавиться от else и уменьшить отступ. В данном случае, кмк, нет. (Тем более, что в будущем это if просто уйдёт)
yql/essentials/core/yql_join.cpp
Outdated
| result.JoinAlgo = EJoinAlgoType::StreamLookupJoin; | ||
| auto size = streamlookup->ChildrenSize(); | ||
| for (decltype(size) i = 1; i < size; ++i) { |
There was a problem hiding this comment.
у тебя же не шаблонный код, не мудри с decltype
There was a problem hiding this comment.
Зато не нужно помнить size -- это int, unsigned, ui32, ui64, size_t,...
There was a problem hiding this comment.
А хорошо бы помнить, иначе же overflow всякие можешь ловить
There was a problem hiding this comment.
Так наоборот: decltype не даёт сделать несогласованные типы, с которыми может быть overflow. Вот auto (=ui64) + ui32 -- можно и словить.
There was a problem hiding this comment.
Ну пожалуй действительно в этом случае выровнятся типы. Но все равно не понятно почему такое переусложнение появилось. Тип же ChilderSize определен, почему понадобилось так мучаться с decltype?
|
⚪ 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 |
…lookup-parameters-ydb-part
yumkam
changed the title
|
⚪ 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 |
(in line with yql-side change)
|
⚪ 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 |
…lookup-parameters-ydb-part
|
⚪ 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
|
|
⚪ 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 |
(cherry picked from commit d4e572d)
[includes yql part, but it will disappear from resulting patch once merged on arc side; NOT FOR MERGE before that - done]
Changelog entry
...
Changelog category
Additional information
...