Merge #156377

156377: sql/inspect: skip hash precheck for non-encodable types r=spilchen a=spilchen

The recent hash precheck optimization for INSPECT (added to speed up index consistency checks) relies on `crdb_internal.datums_to_bytes` to compute row hashes. However, this builtin uses `keyside.Encode` internally, which doesn't support all column types.

When INSPECT encountered tables with TSVECTOR, TSQUERY, or VECTOR (PGVector) columns stored in secondary indexes, the hash precheck would fail with "illegal argument N of type <typename>". This caused INSPECT to fail entirely on these tables.

This commit adds a check to skip the hash precheck if there are any types that cannot be encoded.

Fixes #156175

Epic: CRDB-55075

Release note: none

Co-authored-by: Matt Spilchen <[email protected]>

Author: craig[bot]

pkg/sql/inspect/index_consistency_check.go

@@ -17,6 +17,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/settings"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/catenumpb"
+	"github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descs"
 	"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
@@ -247,20 +248,30 @@ func (c *indexConsistencyCheck) Start(
 	}
 
 	if indexConsistencyHashEnabled.Get(&c.flowCtx.Cfg.Settings.SV) && len(allColNames) > 0 {
-		match, hashErr := c.hashesMatch(ctx, allColNames, predicate, queryArgs)
-		if hashErr != nil {
-			// If hashing fails, we usually fall back to the full check. But if the
-			// error stems from query construction, that's an internal bug and shouldn't
-			// be ignored.
-			if isQueryConstructionError(hashErr) {
-				return errors.WithAssertionFailure(hashErr)
+		// The hash precheck uses crdb_internal.datums_to_bytes, which depends on
+		// keyside.Encode. Skip if any column type isn’t encodable (i.e. TSQUERY, etc.).
+		if !allColumnsDatumsToBytesCompatible(c.columns) {
+			log.Dev.Infof(ctx, "skipping hash precheck for index %s: column type not compatible with datums_to_bytes",
+				c.secIndex.GetName())
+		} else {
+			match, hashErr := c.hashesMatch(ctx, allColNames, predicate, queryArgs)
+			if hashErr != nil {
+				if isQueryConstructionError(hashErr) {
+					// If hashing fails and the error stems from query construction,
+					// that's an internal bug and shouldn't be ignored.
+					return errors.WithAssertionFailure(hashErr)
+				}
+				// For all other hash errors, log and fall back.
+				log.Dev.Infof(ctx, "hash precheck failed; falling back to full check: %v", hashErr)
 			}
-			log.Dev.Infof(ctx, "hash precheck for index consistency did not match; falling back to full check: %v", hashErr)
-		}
-		if match {
-			// Hashes match, no corruption detected - skip the full check.
-			c.state = checkHashMatched
-			return nil
+			if match {
+				// Hashes match, no corruption detected - skip the full check.
+				c.state = checkHashMatched
+				return nil
+			}
+			// Hashes don't match - corruption detected. Fall back to full check.
+			log.Dev.Infof(ctx, "hash precheck detected mismatch for index %s; proceeding with full check",
+				c.secIndex.GetName())
 		}
 	}
 
@@ -937,3 +948,20 @@ func formatPlaceholders(placeholders []interface{}) []string {
 	}
 	return result
 }
+
+// allColumnsDatumsToBytesCompatible reports whether all columns can be
+// passed to crdb_internal.datums_to_bytes. Returns false if any column
+// has a type that cannot be key-encoded using keyside.Encode, which
+// datums_to_bytes relies on internally.
+//
+// REFCURSOR is technically supported by datums_to_bytes, but we still use
+// ColumnTypeIsIndexable to avoid duplicating type checks. REFCURSOR is
+// uncommon, so this trade-off keeps the code simpler.
+func allColumnsDatumsToBytesCompatible(columns []catalog.Column) bool {
+	for _, col := range columns {
+		if !colinfo.ColumnTypeIsIndexable(col.GetType()) {
+			return false
+		}
+	}
+	return true
+}

pkg/sql/logictest/testdata/logic_test/inspect

@@ -585,3 +585,136 @@ statement ok
 DROP TABLE t_aost;
 
 subtest end
+
+subtest inspect_non_key_encodable_types
+
+statement ok
+CREATE TABLE tsvector_tbl (
+  id INT PRIMARY KEY,
+  a INT,
+  tsv TSVECTOR NOT NULL,
+  INDEX idx_a_tsv (a) STORING (tsv)
+);
+
+statement ok
+INSERT INTO tsvector_tbl VALUES
+  (1, 10, 'hello world'::TSVECTOR),
+  (2, 20, 'foo bar'::TSVECTOR);
+
+# First verify that datums_to_bytes does not support TSVECTOR
+statement error pq: illegal argument 0 of type tsvector
+SELECT crdb_internal.datums_to_bytes(tsv) FROM tsvector_tbl LIMIT 1;
+
+# TSVECTOR cannot be key-encoded, so the hash precheck will be skipped.
+# The JOIN-based check should work fine.
+statement ok
+INSPECT TABLE tsvector_tbl WITH OPTIONS INDEX (idx_a_tsv);
+
+query TI
+SELECT * FROM last_inspect_job;
+----
+succeeded  1
+
+statement ok
+CREATE TABLE tsquery_tbl (
+  id INT PRIMARY KEY,
+  a INT,
+  tsq TSQUERY NOT NULL,
+  INDEX idx_a_tsq (a) STORING (tsq)
+);
+
+statement ok
+INSERT INTO tsquery_tbl VALUES
+  (1, 10, 'search & term'::TSQUERY),
+  (2, 20, 'another | query'::TSQUERY);
+
+# First verify that datums_to_bytes does not support TSQUERY
+statement error pq: illegal argument 0 of type tsquery
+SELECT crdb_internal.datums_to_bytes(tsq) FROM tsquery_tbl LIMIT 1;
+
+# TSQUERY cannot be key-encoded, so the hash precheck will be skipped.
+# The JOIN-based check should work fine.
+statement ok
+INSPECT TABLE tsquery_tbl WITH OPTIONS INDEX (idx_a_tsq);
+
+query TI
+SELECT * FROM last_inspect_job;
+----
+succeeded  1
+
+statement ok
+CREATE TABLE pgvector_tbl (
+  id INT PRIMARY KEY,
+  a INT,
+  vec VECTOR(3) NOT NULL,
+  INDEX idx_a_vec (a) STORING (vec)
+);
+
+statement ok
+INSERT INTO pgvector_tbl VALUES
+  (1, 10, '[1.0, 2.0, 3.0]'::VECTOR(3)),
+  (2, 20, '[4.0, 5.0, 6.0]'::VECTOR(3));
+
+# First verify that datums_to_bytes does not support VECTOR
+statement error pq: illegal argument 0 of type vector
+SELECT crdb_internal.datums_to_bytes(vec) FROM pgvector_tbl LIMIT 1;
+
+# VECTOR cannot be key-encoded, so the hash precheck will be skipped.
+# The JOIN-based check should work fine.
+statement ok
+INSPECT TABLE pgvector_tbl WITH OPTIONS INDEX (idx_a_vec);
+
+query TI
+SELECT * FROM last_inspect_job;
+----
+succeeded  1
+
+# Test with multiple problematic types in the same index.
+statement ok
+CREATE TABLE multi_type_tbl (
+  id INT PRIMARY KEY,
+  a INT,
+  tsv TSVECTOR,
+  tsq TSQUERY NOT NULL,
+  vec VECTOR(2),
+  INDEX idx_a_multi (a) STORING (tsv, tsq, vec)
+);
+
+statement ok
+INSERT INTO multi_type_tbl VALUES
+  (1, 10, 'word1 word2'::TSVECTOR, 'search'::TSQUERY, '[1.0, 2.0]'::VECTOR(2)),
+  (2, 20, NULL, 'query'::TSQUERY, NULL);
+
+# Verify that datums_to_bytes fails on each type individually.
+statement error pq: illegal argument 0 of type tsvector
+SELECT crdb_internal.datums_to_bytes('word1 word2'::TSVECTOR);
+
+statement error pq: illegal argument 0 of type tsquery
+SELECT crdb_internal.datums_to_bytes('search'::TSQUERY);
+
+statement error pq: illegal argument 0 of type vector
+SELECT crdb_internal.datums_to_bytes('[1.0, 2.0]'::VECTOR(2));
+
+# Multiple non-key-encodable types: hash precheck will be skipped.
+# The JOIN-based check should work fine.
+statement ok
+INSPECT TABLE multi_type_tbl WITH OPTIONS INDEX (idx_a_multi);
+
+query TI
+SELECT * FROM last_inspect_job;
+----
+succeeded  1
+
+statement ok
+DROP TABLE tsvector_tbl;
+
+statement ok
+DROP TABLE tsquery_tbl;
+
+statement ok
+DROP TABLE pgvector_tbl;
+
+statement ok
+DROP TABLE multi_type_tbl;
+
+subtest end