Skip to content

Commit

Permalink
opt: replace ColumnMeta.TableMeta with a TableID
Browse files Browse the repository at this point in the history 
The TableMeta field is problematic: it needs to be fixed up when
copying Metadata; and since it points into a slice that we append to,
it may or may not be aliased with the corresponding entry in `tables`.
Avoiding these complication by just storing the TabeID. The `md`
backpointer is also removed and QualifiedAlias is moved to Metadata.

Release note: None
  • Loading branch information
@RaduBerinde
RaduBerinde committed Feb 20, 2019
1 parent 3275eae commit cc04b06
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 89 deletions.
73 changes: 3 additions & 70 deletions pkg/sql/opt/column_meta.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@
package opt

import (
"strings"

"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sem/types"
"github.com/cockroachdb/cockroach/pkg/util"
)
Expand Down Expand Up @@ -58,73 +55,9 @@ type ColumnMeta struct {
// Type is the scalar SQL type of this column.
Type types.T

// TableMeta is the metadata for the base table to which this column belongs.
// If the column was synthesized (i.e. no base table), then is is null.
TableMeta *TableMeta

// md is a back-reference to the query metadata.
md *Metadata
}

// QualifiedAlias returns the column alias, possibly qualified with the table,
// schema, or database name:
//
// 1. If fullyQualify is true, then the returned alias is prefixed by the
// original, fully qualified name of the table: tab.Name().FQString().
//
// 2. If there's another column in the metadata with the same column alias but
// a different table name, then prefix the column alias with the table
// name: "tabName.columnAlias".
//
func (cm *ColumnMeta) QualifiedAlias(fullyQualify bool) string {
if cm.TableMeta == nil {
// Column doesn't belong to a table, so no need to qualify it further.
return cm.Alias
}
md := cm.md

// If a fully qualified alias has not been requested, then only qualify it if
// it would otherwise be ambiguous.
var tabAlias tree.TableName
qualify := fullyQualify
if !fullyQualify {
for i := range md.cols {
if i == int(cm.MetaID-1) {
continue
}

// If there are two columns with same alias, then column is ambiguous.
cm2 := &md.cols[i]
if cm2.Alias == cm.Alias {
tabAlias = cm.TableMeta.Alias
if cm2.TableMeta == nil {
qualify = true
} else {
// Only qualify if the qualified names are actually different.
tabAlias2 := cm2.TableMeta.Alias
if tabAlias.String() != tabAlias2.String() {
qualify = true
}
}
}
}
}

// If the column name should not even be partly qualified, then no more to do.
if !qualify {
return cm.Alias
}

var sb strings.Builder
if fullyQualify {
s := cm.TableMeta.Table.Name().FQString()
sb.WriteString(s)
} else {
sb.WriteString(tabAlias.String())
}
sb.WriteRune('.')
sb.WriteString(cm.Alias)
return sb.String()
// Table is the base table to which this column belongs.
// If the column was synthesized (i.e. no base table), then it is 0.
Table TableID
}

// ToSet converts a column id list to a column id set.
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/opt/exec/execbuilder/format.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func fmtInterceptor(f *memo.ExprFmtCtx, tp treeprinter.Node, nd opt.Expr) bool {
fmtCtx := tree.NewFmtCtx(tree.FmtSimple)
fmtCtx.SetIndexedVarFormat(func(ctx *tree.FmtCtx, idx int) {
fullyQualify := !f.HasFlags(memo.ExprFmtHideQualifications)
alias := md.ColumnMeta(opt.ColumnID(idx + 1)).QualifiedAlias(fullyQualify)
alias := md.QualifiedAlias(opt.ColumnID(idx+1), fullyQualify)
ctx.WriteString(alias)
})
expr.Format(fmtCtx)
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/opt/memo/expr_format.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,7 @@ func formatCol(
colMeta := md.ColumnMeta(id)
if label == "" {
fullyQualify := !f.HasFlags(ExprFmtHideQualifications)
label = colMeta.QualifiedAlias(fullyQualify)
label = md.QualifiedAlias(id, fullyQualify)
}

if !isSimpleColumnName(label) {
Expand Down Expand Up @@ -752,7 +752,7 @@ func FormatPrivate(f *ExprFmtCtx, private interface{}, physProps *physical.Requi
switch t := private.(type) {
case *opt.ColumnID:
fullyQualify := !f.HasFlags(ExprFmtHideQualifications)
label := f.Memo.metadata.ColumnMeta(*t).QualifiedAlias(fullyQualify)
label := f.Memo.metadata.QualifiedAlias(*t, fullyQualify)
fmt.Fprintf(f.Buffer, " %s", label)

case *TupleOrdinal:
Expand Down
67 changes: 64 additions & 3 deletions pkg/sql/opt/metadata.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"fmt"
"math/bits"
"strings"

"github.com/cockroachdb/cockroach/pkg/sql/opt/cat"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
Expand Down Expand Up @@ -274,7 +275,6 @@ func (md *Metadata) AddTableWithAlias(tab cat.Table, alias *tree.TableName) Tabl
md.tables = make([]TableMeta, 0, 4)
}
md.tables = append(md.tables, TableMeta{MetaID: tabID, Table: tab, Alias: *alias})
tabMeta := md.TableMeta(tabID)

colCount := tab.DeletableColumnCount()
if md.cols == nil {
Expand All @@ -284,7 +284,7 @@ func (md *Metadata) AddTableWithAlias(tab cat.Table, alias *tree.TableName) Tabl
for i := 0; i < colCount; i++ {
col := tab.Column(i)
colID := md.AddColumn(string(col.ColName()), col.DatumType())
md.ColumnMeta(colID).TableMeta = tabMeta
md.ColumnMeta(colID).Table = tabID
}

return tabID
Expand Down Expand Up @@ -327,7 +327,7 @@ func (md *Metadata) AddColumn(alias string, typ types.T) ColumnID {
alias = fmt.Sprintf("column%d", len(md.cols)+1)
}
colID := ColumnID(len(md.cols) + 1)
md.cols = append(md.cols, ColumnMeta{MetaID: colID, Alias: alias, Type: typ, md: md})
md.cols = append(md.cols, ColumnMeta{MetaID: colID, Alias: alias, Type: typ})
return colID
}

Expand All @@ -343,6 +343,67 @@ func (md *Metadata) ColumnMeta(colID ColumnID) *ColumnMeta {
return &md.cols[colID.index()]
}

// QualifiedAlias returns the column alias, possibly qualified with the table,
// schema, or database name:
//
// 1. If fullyQualify is true, then the returned alias is prefixed by the
// original, fully qualified name of the table: tab.Name().FQString().
//
// 2. If there's another column in the metadata with the same column alias but
// a different table name, then prefix the column alias with the table
// name: "tabName.columnAlias".
//
func (md *Metadata) QualifiedAlias(colID ColumnID, fullyQualify bool) string {
cm := md.ColumnMeta(colID)
if cm.Table == 0 {
// Column doesn't belong to a table, so no need to qualify it further.
return cm.Alias
}

// If a fully qualified alias has not been requested, then only qualify it if
// it would otherwise be ambiguous.
var tabAlias tree.TableName
qualify := fullyQualify
if !fullyQualify {
for i := range md.cols {
if i == int(cm.MetaID-1) {
continue
}

// If there are two columns with same alias, then column is ambiguous.
cm2 := &md.cols[i]
if cm2.Alias == cm.Alias {
tabAlias = md.TableMeta(cm.Table).Alias
if cm2.Table == 0 {
qualify = true
} else {
// Only qualify if the qualified names are actually different.
tabAlias2 := md.TableMeta(cm2.Table).Alias
if tabAlias.String() != tabAlias2.String() {
qualify = true
}
}
}
}
}

// If the column name should not even be partly qualified, then no more to do.
if !qualify {
return cm.Alias
}

var sb strings.Builder
if fullyQualify {
s := md.TableMeta(cm.Table).Table.Name().FQString()
sb.WriteString(s)
} else {
sb.WriteString(tabAlias.String())
}
sb.WriteRune('.')
sb.WriteString(cm.Alias)
return sb.String()
}

// SequenceID uniquely identifies the usage of a sequence within the scope of a
// query. SequenceID 0 is reserved to mean "unknown sequence".
type SequenceID uint64
Expand Down
28 changes: 15 additions & 13 deletions pkg/sql/opt/norm/join.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ func (c *CustomFuncs) JoinFiltersMatchAllLeftRows(

md := c.f.Metadata()

var leftTabMeta, rightTabMeta *opt.TableMeta
var leftTab, rightTab opt.TableID

// Any left columns that don't match conditions 1-4 end up in this set.
var remainingLeftColIDs opt.ColSet
Expand Down Expand Up @@ -386,25 +386,25 @@ func (c *CustomFuncs) JoinFiltersMatchAllLeftRows(
return false
}

if leftTabMeta == nil {
leftTabMeta = md.ColumnMeta(leftColID).TableMeta
rightTabMeta = md.ColumnMeta(rightColID).TableMeta
if leftTabMeta == nil || rightTabMeta == nil {
if leftTab == 0 {
leftTab = md.ColumnMeta(leftColID).Table
rightTab = md.ColumnMeta(rightColID).Table
if leftTab == 0 || rightTab == 0 {
// Condition #2: Columns don't come from base tables.
return false
}
} else if md.ColumnMeta(leftColID).TableMeta != leftTabMeta {
} else if md.ColumnMeta(leftColID).Table != leftTab {
// Condition #2: All left columns don't come from same table.
return false
} else if md.ColumnMeta(rightColID).TableMeta != rightTabMeta {
} else if md.ColumnMeta(rightColID).Table != rightTab {
// Condition #2: All right columns don't come from same table.
return false
}

if leftTabMeta.Table == rightTabMeta.Table {
if md.TableMeta(leftTab).Table == md.TableMeta(rightTab).Table {
// Check self-join case.
leftColOrd := leftTabMeta.MetaID.ColumnOrdinal(leftColID)
rightColOrd := rightTabMeta.MetaID.ColumnOrdinal(rightColID)
leftColOrd := leftTab.ColumnOrdinal(leftColID)
rightColOrd := rightTab.ColumnOrdinal(rightColID)
if leftColOrd != rightColOrd {
// Condition #4: Left and right column ordinals do not match.
return false
Expand All @@ -421,6 +421,8 @@ func (c *CustomFuncs) JoinFiltersMatchAllLeftRows(

var leftRightColMap map[opt.ColumnID]opt.ColumnID
// Condition #5: All remaining left columns correspond to a foreign key relation.
leftTabMeta := md.TableMeta(leftTab)
rightTabMeta := md.TableMeta(rightTab)
for i, cnt := 0, leftTabMeta.Table.IndexCount(); i < cnt; i++ {
index := leftTabMeta.Table.Index(i)
fkRef, ok := index.ForeignKey()
Expand Down Expand Up @@ -458,7 +460,7 @@ func (c *CustomFuncs) JoinFiltersMatchAllLeftRows(
var leftIndexCols opt.ColSet
for j := 0; j < fkPrefix; j++ {
ord := index.Column(j).Ordinal
leftIndexCols.Add(int(leftTabMeta.MetaID.ColumnID(ord)))
leftIndexCols.Add(int(leftTab.ColumnID(ord)))
}

if !remainingLeftColIDs.SubsetOf(leftIndexCols) {
Expand All @@ -480,14 +482,14 @@ func (c *CustomFuncs) JoinFiltersMatchAllLeftRows(
// referenced) that it's being equated to.
fkMatch := true
for j := 0; j < fkPrefix; j++ {
indexLeftCol := leftTabMeta.MetaID.ColumnID(index.Column(j).Ordinal)
indexLeftCol := leftTab.ColumnID(index.Column(j).Ordinal)

// Not every fk column needs to be in the equality conditions.
if !remainingLeftColIDs.Contains(int(indexLeftCol)) {
continue
}

indexRightCol := rightTabMeta.MetaID.ColumnID(fkIndex.Column(j).Ordinal)
indexRightCol := rightTab.ColumnID(fkIndex.Column(j).Ordinal)

if rightCol, ok := leftRightColMap[indexLeftCol]; !ok || rightCol != indexRightCol {
fkMatch = false
Expand Down

0 comments on commit cc04b06

Please sign in to comment.