Merge #154872 #154878 #154917 #154963 #154965

154872: sql/inspect: fix progress tracking bugs causing over 100% completion r=spilchen a=spilchen

Fix two bugs in INSPECT job progress tracking that caused inaccurate completion percentages, sometimes exceeding 100%.

The first issue was in inspect_processor.go, where processSpan() checked the loop termination condition (!ok) before accounting for the final completed check. When the last check finished we would exit the loop before reporting the final check as complete.

The second issue was in inspect_job.go, where initProgressFromPlan() calculated total checks using PK spans rather than the partitioned spans actually used by processors. The number of total checks that we think need to be complete were much lower than the partitioned spans. This caused us to report completion rates in excess of 100%.

Epic: CRDB-30356

Informs: #154457

Release note: none

154878: sql: audit production callers of `rowenc.DatumToEncDatum` r=yuzefovich a=yuzefovich

**sql: audit production callers of `rowenc.DatumToEncDatum`**

This commit audits all callers of `rowenc.DatumToEncDatum` in order to replace possibly unsafe calls to `DatumToEncDatumEx` which requires explicit error handling (the former panics when the datum's resolved type is not equivalent to the passed-in type). I decided to not remove the former method altogether since it seems to be reasonable to use it when we're construcing the datum directly in the call site (and also in tests).

Fixes: #154607.

**rowenc: rename DatumToEncDatum methods**

This commit does the following renaming:
- `DatumToEncDatumEx` -> `DatumToEncDatum`
- `DatumToEncDatum` -> `DatumToEncDatumUnsafe`.

The goal is to encourage usage of safer version which requires explicit error handling (as opposed to panicking). The main use case (apart from tests) for the unsafe version has been documented.

Release note: None

154917: invertedidx: harden geo code to avoid panicking r=yuzefovich a=yuzefovich

Previously, we could panic when trying to construnct an inverted expression for a geo type. Such panics are not recovered from by the vectorized panic-catcher (since we haven't allow-listed the package), so it's better to propagate errors explicitly.

In particular, this allows us to prevent a crash when trying to evaluate `st_dwithin`-like functions with infinite distance (this hits an error in geos, so now we'll return an error, matching postgres).

Fixes: #151995.
Release note: None

154963: sql/parser: fix bug parsing WITH READ VIRTUAL CLUSTER with other opts r=dt a=dt

Release note (bug fix): Fix a bug that would cause WITH READ VIRTUAL CLUSTER to be ignored if any other options were passed when running CREATE VIRTUAL CLUSTER FROM REPLICATION.
Epic: none.

154965: roachtest: fix test failure in DSC compat test r=rafiss a=rafiss

The test was failing since it was renaming a table that is used by a view.

fixes #154809

Release note: None

Co-authored-by: Matt Spilchen <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: David Taylor <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>

Author: 5 people

pkg/backup/generative_split_and_scatter_processor.go

@@ -253,8 +253,8 @@ func routingDatumsForSQLInstance(
 	sqlInstanceID base.SQLInstanceID,
 ) (rowenc.EncDatum, rowenc.EncDatum) {
 	routingBytes := roachpb.Key(fmt.Sprintf("node%d", sqlInstanceID))
-	startDatum := rowenc.DatumToEncDatum(types.Bytes, tree.NewDBytes(tree.DBytes(routingBytes)))
-	endDatum := rowenc.DatumToEncDatum(types.Bytes, tree.NewDBytes(tree.DBytes(routingBytes.Next())))
+	startDatum := rowenc.DatumToEncDatumUnsafe(types.Bytes, tree.NewDBytes(tree.DBytes(routingBytes)))
+	endDatum := rowenc.DatumToEncDatumUnsafe(types.Bytes, tree.NewDBytes(tree.DBytes(routingBytes.Next())))
 	return startDatum, endDatum
 }
 
@@ -399,7 +399,7 @@ func (gssp *generativeSplitAndScatterProcessor) Next() (
 
 		row := rowenc.EncDatumRow{
 			routingDatum,
-			rowenc.DatumToEncDatum(types.Bytes, tree.NewDBytes(tree.DBytes(entryBytes))),
+			rowenc.DatumToEncDatumUnsafe(types.Bytes, tree.NewDBytes(tree.DBytes(entryBytes))),
 		}
 		return row, nil
 	}

pkg/ccl/changefeedccl/avro/avro.go

@@ -995,7 +995,10 @@ func (r *DataRecord) rowFromNative(native interface{}) (rowenc.EncDatumRow, erro
 		if err != nil {
 			return nil, err
 		}
-		row[r.colIdxByFieldIdx[fieldIdx]] = rowenc.DatumToEncDatum(field.typ, decoded)
+		row[r.colIdxByFieldIdx[fieldIdx]], err = rowenc.DatumToEncDatum(field.typ, decoded)
+		if err != nil {
+			return nil, err
+		}
 	}
 	return row, nil
 }

pkg/ccl/changefeedccl/avro/avro_test.go

@@ -119,7 +119,7 @@ func parseValues(tableDesc catalog.TableDescriptor, values string) ([]rowenc.Enc
 			if err != nil {
 				return nil, errors.Wrapf(err, "evaluating %s", typedExpr)
 			}
-			row = append(row, rowenc.DatumToEncDatum(col.GetType(), datum))
+			row = append(row, rowenc.DatumToEncDatumUnsafe(col.GetType(), datum))
 		}
 		rows = append(rows, row)
 	}
@@ -951,8 +951,8 @@ func randEncDatumRow(typ *types.T) rowenc.EncDatumRow {
 	const notNull = false
 	rnd, _ := randutil.NewTestRand()
 	return rowenc.EncDatumRow{
-		rowenc.DatumToEncDatum(typ, randgen.RandDatum(rnd, typ, allowNull)),
-		rowenc.DatumToEncDatum(types.Int, randgen.RandDatum(rnd, types.Int, notNull)),
+		rowenc.DatumToEncDatumUnsafe(typ, randgen.RandDatum(rnd, typ, allowNull)),
+		rowenc.DatumToEncDatumUnsafe(types.Int, randgen.RandDatum(rnd, types.Int, notNull)),
 	}
 }
 
@@ -1059,7 +1059,7 @@ func BenchmarkEncodeDecimal(b *testing.B) {
 	d := &tree.DDecimal{}
 	coeff := int64(rand.Uint64()) % 10000
 	d.Decimal.SetFinite(coeff, 2)
-	encRow[0] = rowenc.DatumToEncDatum(typ, d)
+	encRow[0] = rowenc.DatumToEncDatumUnsafe(typ, d)
 	benchmarkEncodeType(b, typ, encRow)
 }
 

pkg/ccl/changefeedccl/cdcevent/event.go

@@ -810,7 +810,7 @@ func TestingMakeEventRowFromDatums(datums tree.Datums) Row {
 	for i, d := range datums {
 		desc.cols = append(desc.cols, ResultColumn{ord: i})
 		desc.valueCols = append(desc.valueCols, i)
-		encRow = append(encRow, rowenc.DatumToEncDatum(d.ResolvedType(), d))
+		encRow = append(encRow, rowenc.DatumToEncDatumUnsafe(d.ResolvedType(), d))
 	}
 	return Row{
 		EventDescriptor: &desc,

pkg/ccl/changefeedccl/encoder_protobuf_test.go

@@ -158,8 +158,8 @@ func Test_ProtoEncoderAllTypes(t *testing.T) {
 
 			targets := mkTargets(tableDesc)
 			row := cdcevent.TestingMakeEventRow(tableDesc, 0, rowenc.EncDatumRow{
-				rowenc.DatumToEncDatum(types.Int, tree.NewDInt(1)),
-				rowenc.DatumToEncDatum(tc.typ, tc.datum),
+				rowenc.DatumToEncDatumUnsafe(types.Int, tree.NewDInt(1)),
+				rowenc.DatumToEncDatumUnsafe(tc.typ, tc.datum),
 			}, false)
 
 			opts := changefeedbase.EncodingOptions{Envelope: changefeedbase.OptEnvelopeBare}
@@ -442,7 +442,7 @@ func Test_ProtoEncoder_Escaping(t *testing.T) {
 
 	targets := mkTargets(tableDesc)
 	row := cdcevent.TestingMakeEventRow(tableDesc, 0, rowenc.EncDatumRow{
-		rowenc.DatumToEncDatum(types.Int, tree.NewDInt(123)),
+		rowenc.DatumToEncDatumUnsafe(types.Int, tree.NewDInt(123)),
 	}, false)
 
 	opts := changefeedbase.EncodingOptions{Envelope: changefeedbase.OptEnvelopeBare}
@@ -470,8 +470,8 @@ func TestProtoEncoder_BareEnvelope_WithMetadata(t *testing.T) {
 	require.NoError(t, err)
 
 	encRow := rowenc.EncDatumRow{
-		rowenc.DatumToEncDatum(types.Int, tree.NewDInt(1)),
-		rowenc.DatumToEncDatum(types.String, tree.NewDString("Alice")),
+		rowenc.DatumToEncDatumUnsafe(types.Int, tree.NewDInt(1)),
+		rowenc.DatumToEncDatumUnsafe(types.String, tree.NewDString("Alice")),
 	}
 	row := cdcevent.TestingMakeEventRow(tableDesc, 0, encRow, false)
 

pkg/ccl/changefeedccl/kcjsonschema/kafka_connect_json_schema_test.go

@@ -239,7 +239,7 @@ func parseValues(tableDesc catalog.TableDescriptor, values string) ([]rowenc.Enc
 			if err != nil {
 				return nil, errors.Wrapf(err, "evaluating %s", typedExpr)
 			}
-			row = append(row, rowenc.DatumToEncDatum(col.GetType(), datum))
+			row = append(row, rowenc.DatumToEncDatumUnsafe(col.GetType(), datum))
 		}
 		rows = append(rows, row)
 	}

pkg/cmd/roachtest/tests/mixed_version_job_compatibility_in_declarative_schema_changer.go

@@ -116,8 +116,8 @@ func executeSupportedDDLs(
 	// DDLs supported in V25_4.
 	v254DDLs := []string{
 		`TRUNCATE testdb.testsc.t3`,
-		`ALTER TABLE testdb.testsc.t RENAME TO t_renamed`,
-		`ALTER TABLE testdb.testsc.t_renamed RENAME TO t`,
+		`ALTER TABLE testdb.testsc.t2 RENAME TO t2_renamed`,
+		`ALTER TABLE testdb.testsc.t2_renamed RENAME TO t2`,
 		`ALTER TABLE testdb.testsc.t2 ALTER COLUMN j SET ON UPDATE j + 1`,
 		`ALTER TABLE testdb.testsc.t2 RENAME COLUMN k TO k_renamed`,
 		`ALTER TABLE testdb.testsc.t2 RENAME COLUMN k_renamed TO k`,

pkg/crosscluster/logical/logical_replication_writer_processor.go

@@ -376,7 +376,7 @@ func (lrw *logicalReplicationWriterProcessor) Next() (
 				return nil, lrw.DrainHelper()
 			}
 			row := rowenc.EncDatumRow{
-				rowenc.DatumToEncDatum(types.Bytes, tree.NewDBytes(tree.DBytes(progressBytes))),
+				rowenc.DatumToEncDatumUnsafe(types.Bytes, tree.NewDBytes(tree.DBytes(progressBytes))),
 			}
 			return row, nil
 		} else {

pkg/crosscluster/logical/offline_initial_scan_processor.go

@@ -269,7 +269,7 @@ func (o *offlineInitialScanProcessor) Next() (rowenc.EncDatumRow, *execinfrapb.P
 				return nil, o.DrainHelper()
 			}
 			row := rowenc.EncDatumRow{
-				rowenc.DatumToEncDatum(types.Bytes, tree.NewDBytes(tree.DBytes(progressBytes))),
+				rowenc.DatumToEncDatumUnsafe(types.Bytes, tree.NewDBytes(tree.DBytes(progressBytes))),
 			}
 			return row, nil
 		}

pkg/crosscluster/physical/stream_ingestion_processor.go

@@ -532,7 +532,7 @@ func (sip *streamIngestionProcessor) Next() (rowenc.EncDatumRow, *execinfrapb.Pr
 				return nil, sip.DrainHelper()
 			}
 			row := rowenc.EncDatumRow{
-				rowenc.DatumToEncDatum(types.Bytes, tree.NewDBytes(tree.DBytes(progressBytes))),
+				rowenc.DatumToEncDatumUnsafe(types.Bytes, tree.NewDBytes(tree.DBytes(progressBytes))),
 			}
 			return row, nil
 		}