Skip to content

Commit

Permalink
sql: fix OIDs in RowDescription in some cases
Browse files Browse the repository at this point in the history
Release note (bug fix): Previously, CockroachDB could not set the
`TableOID` and `TableAttributeNumber` attributes of `RowDescription`
message of pgwire protocol in some cases (these values would be left as
0), and this is now fixed.
  • Loading branch information
yuzefovich committed Oct 28, 2021
1 parent d5b1889 commit eff882a
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 15 deletions.
12 changes: 7 additions & 5 deletions pkg/sql/exec_factory_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,11 +123,11 @@ func makeScanColumnsConfig(table cat.Table, cols exec.TableColumnOrdinalSet) sca
}

// getResultColumnsForSimpleProject populates result columns for a simple
// projection. It supports two configurations:
// projection. inputCols must be non-nil and contain the result columns before
// the projection has been applied. It supports two configurations:
// 1. colNames and resultTypes are non-nil. resultTypes indicates the updated
// types (after the projection has been applied)
// 2. if colNames is nil, then inputCols must be non-nil (which are the result
// columns before the projection has been applied).
// 2. colNames is nil.
func getResultColumnsForSimpleProject(
cols []exec.NodeColumnOrdinal,
colNames []string,
Expand All @@ -143,8 +143,10 @@ func getResultColumnsForSimpleProject(
resultCols[i].Hidden = false
} else {
resultCols[i] = colinfo.ResultColumn{
Name: colNames[i],
Typ: resultTypes[i],
Name: colNames[i],
Typ: resultTypes[i],
TableID: inputCols[col].TableID,
PGAttributeNum: inputCols[col].PGAttributeNum,
}
}
}
Expand Down
7 changes: 1 addition & 6 deletions pkg/sql/opt_exec_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,11 +247,6 @@ func constructSimpleProjectForPlanNode(
r.reqOrdering = ReqOrdering(reqOrdering)
return r, nil
}
var inputCols colinfo.ResultColumns
if colNames == nil {
// We will need the names of the input columns.
inputCols = planColumns(n.(planNode))
}

var rb renderBuilder
rb.init(n, reqOrdering)
Expand All @@ -268,7 +263,7 @@ func constructSimpleProjectForPlanNode(
resultTypes[i] = exprs[i].ResolvedType()
}
}
resultCols := getResultColumnsForSimpleProject(cols, colNames, resultTypes, inputCols)
resultCols := getResultColumnsForSimpleProject(cols, colNames, resultTypes, planColumns(n.(planNode)))
rb.setOutput(exprs, resultCols)
return rb.res, nil
}
Expand Down
29 changes: 25 additions & 4 deletions pkg/sql/pgwire/testdata/pgtest/row_description
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,33 @@ RowDescription
----
{"Type":"RowDescription","Fields":[{"Name":"v1","TableOID":0,"TableAttributeNumber":1,"DataTypeOID":20,"DataTypeSize":8,"TypeModifier":-1,"Format":0},{"Name":"v2","TableOID":0,"TableAttributeNumber":2,"DataTypeOID":20,"DataTypeSize":8,"TypeModifier":-1,"Format":0}]}

# TODO(yuzefovich): we should not be ignoring table OIDs here, but currently we
# have a bug there (#71891).
until crdb_only ignore_table_oids
until crdb_only
RowDescription
----
{"Type":"RowDescription","Fields":[{"Name":"v1","TableOID":52,"TableAttributeNumber":1,"DataTypeOID":20,"DataTypeSize":8,"TypeModifier":-1,"Format":0},{"Name":"v2","TableOID":53,"TableAttributeNumber":2,"DataTypeOID":20,"DataTypeSize":8,"TypeModifier":-1,"Format":0}]}

until ignore_table_oids
ReadyForQuery
----
{"Type":"DataRow","Values":[{"text":"1"},{"text":"1"}]}
{"Type":"CommandComplete","CommandTag":"SELECT 1"}
{"Type":"ReadyForQuery","TxStatus":"I"}

# Regression test for not setting OIDs in some cases (#71891).
send
Query {"String": "SELECT a, tab1_a FROM tab2 INNER MERGE JOIN tab1 ON a = tab1_a WHERE a = 1"}
----

# With postgres we don't control the table OID.
until ignore_table_oids noncrdb_only
RowDescription
----
{"Type":"RowDescription","Fields":[{"Name":"a","TableOID":0,"TableAttributeNumber":1,"DataTypeOID":20,"DataTypeSize":8,"TypeModifier":-1,"Format":0},{"Name":"tab1_a","TableOID":0,"TableAttributeNumber":2,"DataTypeOID":20,"DataTypeSize":8,"TypeModifier":-1,"Format":0}]}

until crdb_only
RowDescription
----
{"Type":"RowDescription","Fields":[{"Name":"v1","TableOID":0,"TableAttributeNumber":0,"DataTypeOID":20,"DataTypeSize":8,"TypeModifier":-1,"Format":0},{"Name":"v2","TableOID":0,"TableAttributeNumber":0,"DataTypeOID":20,"DataTypeSize":8,"TypeModifier":-1,"Format":0}]}
{"Type":"RowDescription","Fields":[{"Name":"a","TableOID":52,"TableAttributeNumber":1,"DataTypeOID":20,"DataTypeSize":8,"TypeModifier":-1,"Format":0},{"Name":"tab1_a","TableOID":53,"TableAttributeNumber":2,"DataTypeOID":20,"DataTypeSize":8,"TypeModifier":-1,"Format":0}]}

until ignore_table_oids
ReadyForQuery
Expand Down

0 comments on commit eff882a

Please sign in to comment.