-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
feat: support invisible columns #14366
Conversation
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Signed-off-by: Andres Taylor <[email protected]>
d6e1636
to
adbad79
Compare
backport question on issue: #14367 (comment) |
@@ -126,6 +126,7 @@ message AutoIncrement { | |||
message Column { | |||
string name = 1; | |||
query.Type type = 2; | |||
bool invisible = 3; |
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.
I do not think we need this information in proto. possibly we directly use the proto struct so there might be not a better alternative here.
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.
I checked, looks like It does not need to be part of proto.
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.
@harshit-gangal I wonder though, do we need it in the proto for older versions? Or could we also fix it there without that?
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.
I think that we should be consistent and allow this setting in the proto until we deprecate the manual specifying of column information in the vschema.
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.
currently due to authoritative column information provided by vschema proto, we might keep this information and use it if provided. so let's keep this field till we do not deprecate this in vschema.
Description
Adds supports for invisible columns. An invisible column is normally hidden to queries, but can be accessed if explicitly referenced.
As an illustration of when invisible columns may be useful, suppose that an application uses
SELECT *
queries to access a table, and must continue to work without modification even if the table is altered to add a new column that the application does not expect to be there. In a SELECT * query, the * evaluates to all table columns, except those that are invisible, so the solution is to add the new column as an invisible column. The column remains “hidden” from SELECT * queries, and the application continues to work as previously. A newer version of the application can refer to the invisible column if necessary by explicitly referencing it.Related Issue(s)
Fixes #14367
Checklist