Skip to content
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

Merged
merged 1 commit into from
Oct 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 47 additions & 37 deletions go/vt/proto/vschema/vschema.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

38 changes: 36 additions & 2 deletions go/vt/proto/vschema/vschema_vtproto.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions go/vt/sqlparser/ast_funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -2496,3 +2496,7 @@ func IsLiteral(expr Expr) bool {
return false
}
}

func (ct *ColumnType) Invisible() bool {
return ct.Options.Invisible != nil && *ct.Options.Invisible
}
22 changes: 22 additions & 0 deletions go/vt/vtgate/planbuilder/testdata/select_cases.json
Original file line number Diff line number Diff line change
Expand Up @@ -4765,5 +4765,27 @@
"user.customer"
]
}
},
{
"comment": "invisible column is not expanded, but valid in predicate",
"query": "select * from samecolvin where secret = 12",
"plan": {
"QueryType": "SELECT",
"Original": "select * from samecolvin where secret = 12",
"Instructions": {
"OperatorType": "Route",
"Variant": "Scatter",
"Keyspace": {
"Name": "user",
"Sharded": true
},
"FieldQuery": "select col from samecolvin where 1 != 1",
"Query": "select col from samecolvin where secret = 12",
"Table": "samecolvin"
},
"TablesUsed": [
"user.samecolvin"
]
}
}
]
4 changes: 4 additions & 0 deletions go/vt/vtgate/planbuilder/testdata/vschemas/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,10 @@
"columns": [
{
"name": "col"
},
{
"name": "secret",
"invisible": true
}
],
"column_list_authoritative": true
Expand Down
1 change: 1 addition & 0 deletions go/vt/vtgate/schema/tracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,7 @@ func getColumns(tblSpec *sqlparser.TableSpec) []vindexes.Column {
Name: column.Name,
Type: column.Type.SQLType(),
CollationName: colCollation,
Invisible: column.Type.Invisible(),
})
}
return cols
Expand Down
7 changes: 7 additions & 0 deletions go/vt/vtgate/semantics/early_rewriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,9 @@ func (e *expanderState) processColumnsFor(tbl TableInfo) error {
outer:
// in this first loop we just find columns used in any JOIN USING used on this table
for _, col := range tbl.getColumns() {
if col.Invisible {
continue
}
ts, found := usingCols[col.Name]
if found {
for i, ts := range ts.Constituents() {
Expand All @@ -590,6 +593,10 @@ outer:

// and this time around we are printing any columns not involved in any JOIN USING
for _, col := range tbl.getColumns() {
if col.Invisible {
continue
}

if ts, found := usingCols[col.Name]; found && currTable.IsSolvedBy(ts) {
continue
}
Expand Down
4 changes: 4 additions & 0 deletions go/vt/vtgate/semantics/early_rewriter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ func TestExpandStar(t *testing.T) {
}, {
Name: sqlparser.NewIdentifierCI("c"),
Type: sqltypes.VarChar,
}, {
Name: sqlparser.NewIdentifierCI("secret"),
Type: sqltypes.Decimal,
Invisible: true,
}},
ColumnListAuthoritative: true,
},
Expand Down
1 change: 1 addition & 0 deletions go/vt/vtgate/semantics/real_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ func vindexTableToColumnInfo(tbl *vindexes.Table) []ColumnInfo {
Type: col.Type,
Coll: collation,
},
Invisible: col.Invisible,
})
nameMap[col.Name.String()] = nil
}
Expand Down
5 changes: 3 additions & 2 deletions go/vt/vtgate/semantics/semantic_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,9 @@ type (

// ColumnInfo contains information about columns
ColumnInfo struct {
Name string
Type evalengine.Type
Name string
Type evalengine.Type
Invisible bool
}

// ExprDependencies stores the tables that an expression depends on as a map
Expand Down
5 changes: 4 additions & 1 deletion go/vt/vtgate/vindexes/vschema.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,9 @@ type Column struct {
Name sqlparser.IdentifierCI `json:"name"`
Type querypb.Type `json:"type"`
CollationName string `json:"collation_name"`

// Invisible marks this as a column that will not be automatically included in `*` projections
Invisible bool `json:"invisible"`
}

// MarshalJSON returns a JSON representation of Column.
Expand Down Expand Up @@ -610,7 +613,7 @@ func buildTables(ks *vschemapb.Keyspace, vschema *VSchema, ksvschema *KeyspaceSc
)
}
colNames[name.Lowered()] = true
t.Columns = append(t.Columns, Column{Name: name, Type: col.Type})
t.Columns = append(t.Columns, Column{Name: name, Type: col.Type, Invisible: col.Invisible})
}

// Initialize ColumnVindexes.
Expand Down
1 change: 1 addition & 0 deletions proto/vschema.proto
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ message AutoIncrement {
message Column {
string name = 1;
query.Type type = 2;
bool invisible = 3;
Copy link
Member

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.

Copy link
Member

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.

Copy link
Contributor

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?

Copy link
Collaborator Author

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.

Copy link
Member

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.

}

// SrvVSchema is the roll-up of all the Keyspace schema for a cell.
Expand Down
6 changes: 6 additions & 0 deletions web/vtadmin/src/proto/vtadmin.d.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading