Capture TablePathPrefix (and other parts of the parser context) in CREATE VIEW #8991
Conversation
…EATE VIEW The captured context is used later to compile the view query in order to rewrite the read from the view node during a select from the view.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
| return TStatus::Error; | ||
| Y_UNUSED(node); | ||
| Y_UNUSED(ctx); | ||
| return TStatus::Ok; |
There was a problem hiding this comment.
This change is needed for the intent determination transformer to be able to handle the following query:
DROP VIEW some_view;
CREATE VIEW some_other_view ...;The same (i.e. nothing) is done in HandleCreateObject 12 lines above.
| @@ -408,6 +408,46 @@ Y_UNIT_TEST_SUITE(TSelectFromViewTest) { | |||
| CompareResults(etalonResults, selectFromViewResults); | |||
| } | |||
|
|
|||
| Y_UNIT_TEST(OneTableUsingRelativeName) { | |||
There was a problem hiding this comment.
I would like to write an extensive test suite for views after this PR, so I add only a basic test of the new feature.
| @@ -1239,8 +1239,12 @@ bool TSqlQuery::Statement(TVector<TNodePtr>& blocks, const TRule_sql_stmt_core& | |||
| } | |||
|
|
|||
| std::map<TString, TDeferredAtom> features; | |||
| ParseViewOptions(features, node.GetRule_with_table_settings4()); | |||
| ParseViewQuery(features, node.GetRule_select_stmt6()); | |||
| if (!ParseViewOptions(features, node.GetRule_with_table_settings4())) { | |||
There was a problem hiding this comment.
Here I fix a pretty impactful bug: the results of the view query verification by building an AST of the query text were discarded previously. Early validation of the queries stored in views didn't work.
| @@ -1,6 +1,7 @@ | |||
| #pragma once | |||
|
|
|||
| #include <ydb/library/yql/core/pg_settings/guc_settings.h> | |||
| #include <ydb/library/yql/sql/settings/protos/translation_settings.pb.h> | |||
There was a problem hiding this comment.
не надо ссылаться на protobuf-ы в основной либе
сделайте отдельную library, в которой будет save/load в protobuf классы
и вообще этот protobuf и library унесите в ydb
There was a problem hiding this comment.
Не знаю, как унести в YDB, учитывая, что эта библиотека нужна в sql_translation.cpp для валидации текста запроса view.
В отдельную библиотеку выделил, но она по-прежнему лежит в YQL юрисдикции: ydb/library/yql/sql/settings/serializer/serializer.h.
There was a problem hiding this comment.
Унёс всё в YDB. Мотивация следующая:
- KQP создаёт
TTransltaionSettings, с которыми будет запущен парсер. Парсер не меняет эти настройки. - Сериализация нужна в KQP в момент создания TModifyScheme для CreateView.
- Раз translation settings и создаются, и используются вне парсера, то логично их там и сериализовывать. Парсер в таком подходе ничего не знает об этой части сохранения контекста для view.
Важно отметить, что теперь, когда мы сериализуем контекст в KQP, этот процесс становится по смыслу равен сохранению применённых дефолтных настроек парсера в теле view. Query replay для тех же целей запоминает query type, с которым был выполнен запрос. Мы могли бы делать так же (query type было бы вполне достаточно), но поскольку view - это объект из мира парсера, то мы выбрали формат более подходящий для объекта из парсера.
- Inject dependencies on translation settings serializer to TSqlTranslation class. - Move translation settings serializer to a separate library.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
- capture TTranslationSettings in KQP - runtime TContext parts as a view query text
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
github-actions
bot
removed
the
rebase-and-check
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Remove duplicate SqlToYql function
|
⚪
🟢
*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 |
#8992
We need to capture the parser context in order to use it later during a
SELECTfrom the view to ensure that the results of the compilation of the view query text are the same as they were duringCREATE VIEW. Postgres achieves the same by persisting AST of the view query, but our parser cannot handle this. More details on the problem can be found in the RFC.