Merge #89329 #89781 #89850 #89867 #89919

89329: cdcevent: only fetch relevant columns in the rowfetcher r=jayshrivastava a=jayshrivastava

Previously, a rowfetcher would be configured to fetch all public columns in a
table, including virtual columns. Then, if we only wanted to read columns
belonging to a particular family, we would index into this row. This change
updates row fetchers to only fetch relevant data, including only the primary
key columns and columns in a specified family.

This change also updates the row fetcher such that it does not fetch
virtual columns. Instead, they are added on by the event descriptor.
If the descriptor specifies a column outside of the physical row,
the row iterator will fill in a null value upon iteration, representing
a virtual column.

Release note (enterprise change): When a changefeed is run with the option
virtual_columns = "null", the virtual column will be ordered last in each
row.

Informs: #80976
Epic: none

89781: sql: fix logic test option typos r=mgartner a=mgartner

This commit fixes a few tests that incorrectly separated test options with a space instead of a comma.

Release note: None

89850: roachprod: log Get failures in roachtests r=srosenberg a=renatolabs

`roachprod.Get` and `roachprod.Put` behaved differently when it comes to logging of errors: the former would not log errors found while running `scp` if there was a file backing the logger, while the latter would.

However, there is no reason for this difference, and it was probably an oversight introduced in an old, really large PR (#74223). This commit makes behavior consistent in both calls and should allow us to see `Get` errors in logs, especially important in `roachtest` failures.

Epic: None.

Release note: None.

89867: ui: fix undefined table stats r=maryliag a=maryliag

Previously, if no value was being passed as totalCount, the table stats were showing an "undefined value". This commit as a default value as 0 for the total.

Fixes #89869

Before
<img width="411" alt="Screen Shot 2022-10-11 at 8 43 04 PM" src="https://user-images.githubusercontent.com/1017486/195453624-52ad8c79-95ee-4dda-a703-b7492b7e2fe3.png">

After
<img width="281" alt="Screen Shot 2022-10-12 at 5 21 52 PM" src="https://user-images.githubusercontent.com/1017486/195453705-e4fff03c-770f-4f32-8b87-3d26b9b072ff.png">


Release note (bug fix): show correct value on table stats on UI, when there is no values to show.

89919: release: generate archives once, fix checksum format r=celiala a=rail

Previously, the release archives were generated multiple times and we ended up uploading different binary content to different cloud providers. Also, the checksum format was missing an extra space, what prevents non coreutils checksum tools from working properly.

This change makes the releae process to generate archives only once and fixes the checksum format.

Fixes: #89915
Fixes: #89917

Release note: None

Co-authored-by: Jayant Shrivastava <[email protected]>
Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: Renato Costa <[email protected]>
Co-authored-by: maryliag <[email protected]>
Co-authored-by: Rail Aliiev <[email protected]>

pkg/ccl/changefeedccl/cdcevent/BUILD.bazel

@@ -70,6 +70,7 @@ go_test(
         "//pkg/sql/sem/tree",
         "//pkg/sql/types",
         "//pkg/testutils/serverutils",
+        "//pkg/testutils/skip",
         "//pkg/testutils/sqlutils",
         "//pkg/testutils/testcluster",
         "//pkg/util/leaktest",

pkg/ccl/changefeedccl/cdcevent/event.go

@@ -33,6 +33,8 @@ import (
 	"github.com/cockroachdb/redact"
 )
 
+const virtualColOrd = 1<<31 - 1
+
 // Metadata describes event metadata.
 type Metadata struct {
 	TableID          descpb.ID                // Table ID.
@@ -129,18 +131,29 @@ func (r Row) HasValues() bool {
 
 // forEachColumn is a helper which invokes fn for reach column in the ordColumn list.
 func (r Row) forEachDatum(fn DatumFn, colIndexes []int) error {
+	numVirtualCols := 0
 	for _, colIdx := range colIndexes {
 		col := r.cols[colIdx]
-		if col.ord >= len(r.datums) {
-			return errors.AssertionFailedf("index [%d] out of range for column %q", col.ord, col.Name)
-		}
-		encDatum := r.datums[col.ord]
-		if err := encDatum.EnsureDecoded(col.Typ, r.alloc); err != nil {
-			return errors.Wrapf(err, "error decoding column %q as type %s", col.Name, col.Typ.String())
-		}
+		// A datum row will never contain virtual columns. If we encounter a column that is virtual,
+		// then we need to offset each subsequent col.ord by 1. This offset is tracked by numVirtualCols.
+		physicalOrd := col.ord - numVirtualCols
+		if physicalOrd < len(r.datums) {
+			encDatum := r.datums[physicalOrd]
+			if err := encDatum.EnsureDecoded(col.Typ, r.alloc); err != nil {
+				return errors.Wrapf(err, "error decoding column %q as type %s", col.Name, col.Typ.String())
+			}
 
-		if err := fn(encDatum.Datum, col); err != nil {
-			return iterutil.Map(err)
+			if err := fn(encDatum.Datum, col); err != nil {
+				return iterutil.Map(err)
+			}
+		} else if col.ord == virtualColOrd {
+			// Insert null values as placeholders for virtual columns.
+			if err := fn(tree.DNull, col); err != nil {
+				return iterutil.Map(err)
+			}
+			numVirtualCols++
+		} else {
+			return errors.AssertionFailedf("index [%d] out of range for column %q", physicalOrd, col.Name)
 		}
 	}
 	return nil
@@ -248,22 +261,29 @@ func NewEventDescriptor(
 		primaryKeyOrdinal.Set(desc.PublicColumns()[ord].GetID(), i)
 	}
 
-	// Remaining columns go in same order as public columns.
+	// Remaining columns go in same order as public columns,
+	// with the exception that virtual columns are reordered
+	// to be at the end.
 	inFamily := catalog.MakeTableColSet(family.ColumnIDs...)
-	for ord, col := range desc.PublicColumns() {
+	ord := 0
+	for _, col := range desc.PublicColumns() {
 		isInFamily := inFamily.Contains(col.GetID())
 		virtual := col.IsVirtual() && includeVirtualColumns
-		isValueCol := isInFamily || virtual
 		pKeyOrd, isPKey := primaryKeyOrdinal.Get(col.GetID())
-		if isValueCol || isPKey {
+		if isInFamily || isPKey {
 			colIdx := addColumn(col, ord)
-			if isValueCol {
+			if isInFamily {
 				sd.valueCols = append(sd.valueCols, colIdx)
 			}
 
 			if isPKey {
 				sd.keyCols[pKeyOrd] = colIdx
 			}
+			ord++
+		} else if virtual {
+			colIdx := addColumn(col, virtualColOrd)
+			sd.valueCols = append(sd.valueCols, colIdx)
+			ord++
 		}
 	}
 
@@ -633,3 +653,32 @@ func TestingMakeEventRowFromEncDatums(
 		alloc:           &alloc,
 	}
 }
+
+// getRelevantColumnsForFamily returns an array of column ids for public columns
+// including only primary key columns and columns in the specified familyDesc,
+// If includeVirtual is true, virtual columns, which may be outside the specified
+// family, will be included.
+func getRelevantColumnsForFamily(
+	tableDesc catalog.TableDescriptor, familyDesc *descpb.ColumnFamilyDescriptor,
+) ([]descpb.ColumnID, error) {
+	cols := tableDesc.GetPrimaryIndex().CollectKeyColumnIDs()
+	for _, colID := range familyDesc.ColumnIDs {
+		cols.Add(colID)
+	}
+
+	// Maintain the ordering of tableDesc.PublicColumns(), which is
+	// matches the order of columns in the SQL table.
+	idx := 0
+	result := make([]descpb.ColumnID, cols.Len())
+	for _, colID := range tableDesc.PublicColumnIDs() {
+		if cols.Contains(colID) {
+			result[idx] = colID
+			idx++
+		}
+	}
+
+	// Some columns in familyDesc.ColumnIDs may not be public, so
+	// result may contain fewer columns than cols.
+	result = result[:idx]
+	return result, nil
+}