exec: minor clean up

Previously, when converting a columnar value to a tree.Datum, we
would check for nulls separately. Now this is done as part of the
conversion which cleans up the code a bit.

Release note: None

pkg/sql/distsqlrun/materializer.go

@@ -18,7 +18,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/distsqlpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/exec"
 	"github.com/cockroachdb/cockroach/pkg/sql/exec/execerror"
-	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
 )
@@ -139,7 +138,7 @@ func (m *materializer) nextAdapter() {
 // next is the logic of Next() extracted in a separate method to be used by an
 // adapter to be able to wrap the latter with a catcher.
 func (m *materializer) next() (sqlbase.EncDatumRow, *distsqlpb.ProducerMetadata) {
-	for m.State == StateRunning {
+	if m.State == StateRunning {
 		if m.batch == nil || m.curIdx >= m.batch.Length() {
 			// Get a fresh batch.
 			m.batch = m.input.Next(m.Ctx)
@@ -161,14 +160,6 @@ func (m *materializer) next() (sqlbase.EncDatumRow, *distsqlpb.ProducerMetadata)
 		typs := m.OutputTypes()
 		for colIdx := 0; colIdx < len(typs); colIdx++ {
 			col := m.batch.ColVec(colIdx)
-			// TODO(asubiotto): we shouldn't have to do this check. Figure out who's
-			// not setting nulls.
-			if col.MaybeHasNulls() {
-				if col.Nulls().NullAt(rowIdx) {
-					m.row[colIdx].Datum = tree.DNull
-					continue
-				}
-			}
 			m.row[colIdx].Datum = exec.PhysicalTypeColElemToDatum(col, rowIdx, m.da, typs[colIdx])
 		}
 		return m.ProcessRowHelper(m.row), nil

pkg/sql/exec/builtin_funcs.go

@@ -63,12 +63,8 @@ func (b *defaultBuiltinFuncOperator) Next(ctx context.Context) coldata.Batch {
 
 		for j := range b.argumentCols {
 			col := batch.ColVec(b.argumentCols[j])
-			if col.MaybeHasNulls() && col.Nulls().NullAt(rowIdx) {
-				hasNulls = true
-				b.row[j] = tree.DNull
-			} else {
-				b.row[j] = PhysicalTypeColElemToDatum(col, rowIdx, b.da, b.columnTypes[b.argumentCols[j]])
-			}
+			b.row[j] = PhysicalTypeColElemToDatum(col, rowIdx, b.da, b.columnTypes[b.argumentCols[j]])
+			hasNulls = hasNulls || b.row[j] == tree.DNull
 		}
 
 		var (

pkg/sql/exec/vec_elem_to_datum.go

@@ -23,10 +23,17 @@ import (
 	"github.com/lib/pq/oid"
 )
 
-// PhysicalTypeColElemToDatum converts an element in a colvec to a datum of semtype ct.
+// PhysicalTypeColElemToDatum converts an element in a colvec to a datum of
+// semtype ct. Note that this function handles nulls as well, so there is no
+// need for a separate null check.
 func PhysicalTypeColElemToDatum(
 	col coldata.Vec, rowIdx uint16, da sqlbase.DatumAlloc, ct types.T,
 ) tree.Datum {
+	if col.MaybeHasNulls() {
+		if col.Nulls().NullAt(rowIdx) {
+			return tree.DNull
+		}
+	}
 	switch ct.Family() {
 	case types.BoolFamily:
 		if col.Bool()[rowIdx] {

pkg/sql/row/cfetcher.go

@@ -786,9 +786,6 @@ func (rf *CFetcher) pushState(state fetcherState) {
 // getDatumAt returns the converted datum object at the given (colIdx, rowIdx).
 // This function is meant for tracing and should not be used in hot paths.
 func (rf *CFetcher) getDatumAt(colIdx int, rowIdx uint16, typ types.T) tree.Datum {
-	if rf.machine.colvecs[colIdx].Nulls().NullAt(rowIdx) {
-		return tree.DNull
-	}
 	return exec.PhysicalTypeColElemToDatum(rf.machine.colvecs[colIdx], rowIdx, rf.table.da, typ)
 }
 

pkg/sql/sem/tree/expr.go

@@ -1326,7 +1326,8 @@ func (node *FuncExpr) IsDistSQLBlacklist() bool {
 	return node.fnProps != nil && node.fnProps.DistsqlBlacklist
 }
 
-// CanHandleNulls returns whether or not
+// CanHandleNulls returns whether or not the function can handle null
+// arguments.
 func (node *FuncExpr) CanHandleNulls() bool {
 	return node.fnProps != nil && node.fnProps.NullableArgs
 }