-
Notifications
You must be signed in to change notification settings - Fork 702
Added a new sdk method DescribeSystemView and a corresponded Grpc service #20603
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
Added a new sdk method DescribeSystemView and a corresponded Grpc service #20603
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
15434ee
to
16def4c
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
48308d9
to
b78a9db
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
b78a9db
to
44a655a
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
44a655a
to
d406f44
Compare
d406f44
to
8342825
Compare
⚪ 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 |
@@ -1806,6 +1811,9 @@ class TSession { | |||
TAsyncDescribeExternalTableResult DescribeExternalTable(const std::string& path, | |||
const TDescribeExternalTableSettings& settings = {}); | |||
|
|||
TAsyncDescribeSystemViewResult DescribeSystemView(const std::string& path, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А почему это метод на сессии? Ему же Session Id не нужен.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DescribeExternalTable тоже не использует SessionId, сделал по аналогии.
Подскажи, а куда тогда поместить этот метод?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Хм, тогда и там значит неправильно сделано. С моей точки зрения, все такие методы должны быть на клиенте а не на сессии. Хотя ради консистентости можно и так оставить
Added dedicated method to describe system views via cpp sdk.
Before these changes describing system views was as describing table, but many options and attributes related to ordinal tables were inaccessible for system view, so DescribeTable method was redundant.