YDB-2568 Enable match_recognize in ydb#6807
Conversation
kardymonds
added
the
rebase-and-check
github-actions
bot
removed
the
rebase-and-check
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🔴
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🔴
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ ⚪ Details
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
ydb/core/kqp/host/kqp_host.h
Outdated
| NActors::TActorSystem* actorSystem = nullptr /*take from TLS by default*/, | ||
| NYql::TExprContext* ctx = nullptr); | ||
| NYql::TExprContext* ctx = nullptr, | ||
| const NKikimrConfig::TQueryServiceConfig& queryServiceConfig = NKikimrConfig::TQueryServiceConfig()); |
There was a problem hiding this comment.
Давай дефолт для конфига уберем. Иначе можно забыть его дотащить из конфига
There was a problem hiding this comment.
Сделал неопциональным
| const NKikimr::NMiniKQL::IFunctionRegistry* funcRegistry, bool keepConfigChanges, bool isInternalCall, | ||
| TKqpTempTablesState::TConstPtr tempTablesState = nullptr, NActors::TActorSystem* actorSystem = nullptr, | ||
| NYql::TExprContext* ctx = nullptr) | ||
| NYql::TExprContext* ctx = nullptr, const NKikimrConfig::TQueryServiceConfig& queryServiceConfig = NKikimrConfig::TQueryServiceConfig()) |
There was a problem hiding this comment.
Давай уберем дефолт для query service config
| TExprNode::TPtr sortOrder; | ||
| ExtractSortKeyAndOrder(pos, sortTraits, sortKey, sortOrder, ctx); | ||
| TExprNode::TPtr result; | ||
| YQL_ENSURE(sortOrder->ChildrenSize() == 1, "Expect ORDER BY timestamp for MATCH_RECOGNIZE"); |
There was a problem hiding this comment.
Зачем так? Потенциально могут быть сценарии использования без сортировки. И сортировка может быть не обязательно по столбцу timestamp. Как минимум текст ошибки нужно переписать
There was a problem hiding this comment.
давайте текст ошибки перепишем и даже уберем YQL_ENSURE в пользу if. сортировку сделали обязательной для дуализма аналитического и стримингового MR - код должен без проблем переноситься с аналитики на поток
zverevgeny
left a comment
zverevgeny
left a comment
There was a problem hiding this comment.
До этого изменения сортировка не была обязательной. Это валидный сценарий. Он работал, и на него написаны тесты. Непонятно, почему решили убрать поддержку MATCH_RECOGNIZE без ORDER BY.
сортировка была обязательной для стриминг режима. теперь она обязательна для всех режимов, что позволяет клиенту легко перекладывать M_R запрос с аналитики на поток. можно поддержать M_R без требования сортировки, но сразу для обоих режимов. но т.к. я ценности в такой обработке пока не вижу, то проще было отрезать ненужный функционал. чем писать его поддержку еще и для стриминга |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ ⚪ ⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ ⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
5 participants
Changelog entry
...
Changelog category
Additional information
...