Merge #155064 #157908 #157956

155064: opt: normalize LIKE ... ESCAPE r=mgartner a=mgartner

#### logictest: make test case deterministic

This commit makes a test case in `distsql_agg` deterministic no matter
how many optimizer rules have been defined. It now explicitly disables
an optimizer rule, which is required to reproduce the behavior, rather
than using the `testing_optimizer_random_seed` and
`testing_optimizer_disable_rule_probability` session settings.

Release note: None

#### opt: make scalar list custom func accessor safer

The custom functions `FirstScalarListExpr` and `SecondScalarListExpr`
have been replaced by `ScalarExprAt`. This function is safer because it
returns ok=false if the given index is out-of-bounds instead of
panicking. This function has been added to `general_funcs.go` instead of
`comp_funcs.go` in preparation for usage outside of `comp.opt`.

Release note: None

#### opt: normalize LIKE ... ESCAPE

An expression in the form `a LIKE b ESCAPE '/'`, which is parsed as
`like_escape(a, b, '/')`, is now normalized to `a LIKE b`. This is valid
because `\` is the default escape character, so the expressions are
equivalent.

This normalization allows for further optimization, for example:

```sql
CREATE TABLE t (s STRING, INDEX (s));

EXPLAIN SELECT s FROM t WHERE s LIKE 'foo\_bar' ESCAPE '\';
-- BEFORE:
--  • filter
--  │ filter: like_escape(s, e'foo\\_bar', e'\\')
--  │
--  └── • scan
--        table: t@t_pkey
--        spans: FULL SCAN

-- AFTER:
--   • scan
--     table: t@t_s_idx
--     spans: [/'foo_bar' - /'foo_bar']
```

Informs #30192

Release note (performance improvement): Queries with filters in the form
`a LIKE b ESCAPE '\'` are now index-accelerated in some cases which they
were not before.


157908: kvpb: add (*RangeFeedEvent).EventType method r=wenyihu6 a=stevendanna

A small helper for printing the type of an event since this type is enum-like.

Informs #135332
Release note: None

157956: roachtest/vecindex: Always download datasets when testing r=mw5h a=mw5h

Previously, the vecindex roachtest would use the vecann default of attempting to reuse cached dataset files. This led to a situation where a test runner apparently cached a truncated file, causing the test to fail repeatedly when run from that runner. This patch changes the behavior to always download the dataset, so the test doesn't repeatedly flake.

Fixes: #157119
Release note: None

Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: Steven Danna <[email protected]>
Co-authored-by: Matt White <[email protected]>

Author: 4 people

pkg/cmd/roachtest/tests/vecindex.go

@@ -253,6 +253,7 @@ func runVectorIndex(ctx context.Context, t test.Test, c cluster.Cluster, opts ve
 	t.L().Printf("Loading dataset %s", opts.dataset)
 	loader := vecann.DatasetLoader{
 		DatasetName: opts.dataset,
+		ResetCache:  true,
 		OnProgress: func(ctx context.Context, format string, args ...any) {
 			t.L().Printf(format, args...)
 		},

pkg/kv/kvpb/api.go

@@ -2185,6 +2185,28 @@ func (e *RangeFeedEvent) ShallowCopy() *RangeFeedEvent {
 	return &cpy
 }
 
+// EventType returns a string description of the type of event..
+func (e *RangeFeedEvent) EventType() string {
+	switch {
+	case e.Val != nil:
+		return "Value"
+	case e.Checkpoint != nil:
+		return "Checkpoint"
+	case e.SST != nil:
+		return "SST"
+	case e.DeleteRange != nil:
+		return "DeleteRange"
+	case e.Metadata != nil:
+		return "Metadata"
+	case e.Error != nil:
+		return "Error"
+	case e.BulkEvents != nil:
+		return "BulkEvents"
+	default:
+		return "Unknown"
+	}
+}
+
 // Timestamp is part of rangefeedbuffer.Event.
 func (e *RangeFeedValue) Timestamp() hlc.Timestamp {
 	return e.Value.Timestamp

pkg/kv/kvserver/rangefeed/sender_helper_test.go

@@ -97,22 +97,7 @@ func (s *testServerStream) String() string {
 	for streamID, eventList := range s.streamEvents {
 		fmt.Fprintf(&str, "\tStreamID:%d, Len:%d", streamID, len(eventList))
 		for _, ev := range eventList {
-			switch {
-			case ev.Val != nil:
-				fmt.Fprintf(&str, "\t\tvalue")
-			case ev.Checkpoint != nil:
-				fmt.Fprintf(&str, "\t\tcheckpoint")
-			case ev.SST != nil:
-				fmt.Fprintf(&str, "\t\tsst")
-			case ev.DeleteRange != nil:
-				fmt.Fprintf(&str, "\t\tdelete")
-			case ev.Metadata != nil:
-				fmt.Fprintf(&str, "\t\tmetadata")
-			case ev.Error != nil:
-				fmt.Fprintf(&str, "\t\terror")
-			default:
-				panic("unknown event type")
-			}
+			fmt.Fprintf(&str, "\t\t%s", ev.EventType())
 		}
 		fmt.Fprintf(&str, "\n")
 	}

pkg/sql/logictest/testdata/logic_test/distsql_agg

@@ -730,8 +730,7 @@ statement ok
 CREATE TABLE t108901 AS SELECT g::FLOAT8 AS _float8 FROM generate_series(1, 5) AS g;
 ALTER TABLE t108901 SPLIT AT VALUES (1), (2), (3);
 ALTER TABLE t108901 RELOCATE VOTERS VALUES (ARRAY[1], 1), (ARRAY[2], 2), (ARRAY[3], 3);
-SET testing_optimizer_random_seed = 1613845022891698972;
-SET testing_optimizer_disable_rule_probability = 0.500000;
+SET disable_optimizer_rules = PruneWindowOutputCols;
 
 statement error integer out of range
 SELECT 1 FROM t108901 WHERE

pkg/sql/opt/norm/comp_funcs.go

@@ -76,16 +76,6 @@ func (c *CustomFuncs) NormalizeTupleEquality(left, right memo.ScalarListExpr) op
 	return result
 }
 
-// FirstScalarListExpr returns the first ScalarExpr in the given list.
-func (c *CustomFuncs) FirstScalarListExpr(list memo.ScalarListExpr) opt.ScalarExpr {
-	return list[0]
-}
-
-// SecondScalarListExpr returns the second ScalarExpr in the given list.
-func (c *CustomFuncs) SecondScalarListExpr(list memo.ScalarListExpr) opt.ScalarExpr {
-	return list[1]
-}
-
 // MakeTimeZoneFunction constructs a new timezone() function with the given zone
 // and timestamp as arguments. The type of the function result is TIMESTAMPTZ if
 // ts is of type TIMESTAMP, or TIMESTAMP if is of type TIMESTAMPTZ.

pkg/sql/opt/norm/general_funcs.go

@@ -658,6 +658,22 @@ func (c *CustomFuncs) IsLeakproof(expr memo.RelExpr) bool {
 	return expr.Relational().VolatilitySet.IsLeakproof()
 }
 
+// ----------------------------------------------------------------------
+//
+// ScalarListExpr functions
+//   General functions related to ScalarListExpr.
+//
+// ----------------------------------------------------------------------
+
+// ScalarExprAt returns the ScalarExpr in the i-th position in the given list.
+// Returns ok=false if i is out of bounds.
+func (c *CustomFuncs) ScalarExprAt(list memo.ScalarListExpr, i int) (_ opt.ScalarExpr, ok bool) {
+	if i >= 0 && i < len(list) {
+		return list[i], true
+	}
+	return nil, false
+}
+
 // ----------------------------------------------------------------------
 //
 // Ordering functions

pkg/sql/opt/norm/rules/comp.opt

@@ -220,14 +220,13 @@
     (Function $args:* $private:(FunctionPrivate "timezone"))
     $right:(ConstValue) &
         (IsTimestampTZ $right) &
-        (IsTimestamp $ts:(SecondScalarListExpr $args)) &
-        ^(IsConstValueOrGroupOfConstValues $ts)
+        (Let ($ts $tsOk):(ScalarExprAt $args 1) $tsOk) &
+        (IsTimestamp $ts) &
+        ^(IsConstValueOrGroupOfConstValues $ts) &
+        (Let ($zone $zoneOk):(ScalarExprAt $args 0) $zoneOk)
 )
 =>
-((OpName)
-    $ts
-    (MakeTimeZoneFunction (FirstScalarListExpr $args) $right)
-)
+((OpName) $ts (MakeTimeZoneFunction $zone $right))
 
 # NormalizeCmpTimeZoneFunctionTZ normalizes timezone functions within
 # comparison operators. It only matches expressions when:
@@ -249,14 +248,13 @@
     (Function $args:* $private:(FunctionPrivate "timezone"))
     $right:(ConstValue) &
         (IsTimestamp $right) &
-        (IsTimestampTZ $tz:(SecondScalarListExpr $args)) &
-        ^(IsConstValueOrGroupOfConstValues $tz)
+        (Let ($tz $tzOk):(ScalarExprAt $args 1) $tzOk) &
+        (IsTimestampTZ $tz) &
+        ^(IsConstValueOrGroupOfConstValues $tz) &
+        (Let ($zone $zoneOk):(ScalarExprAt $args 0) $zoneOk)
 )
 =>
-((OpName)
-    $tz
-    (MakeTimeZoneFunction (FirstScalarListExpr $args) $right)
-)
+((OpName) $tz (MakeTimeZoneFunction $zone $right))
 
 # FoldEqTrue replaces x = True with x.
 [FoldEqTrue, Normalize]

pkg/sql/opt/norm/rules/scalar.opt

@@ -452,3 +452,21 @@ $udf
 (Indirection $input:* $index:* & (IsJSON $input))
 =>
 (FetchVal $input $index)
+
+# ConvertLikeEscapeToLike converts a LIKE x ESCAPE '\' expression, which is
+# parsed as a like_escape function call, to a LIKE expression. This is valid
+# because the default escape character is '\'.
+#
+# Note that the FunctionExpr match pattern is enough to ensure that we are not
+# transforming a UDF because UDF invocations are always built as UDFCallExprs.
+[ConvertLikeEscapeToLike, Normalize]
+(Function
+    $args:*
+    $private:(FunctionPrivate "like_escape") &
+        (Let ($input $iOk):(ScalarExprAt $args 0) $iOk) &
+        (Let ($pattern $pOk):(ScalarExprAt $args 1) $pOk) &
+        (Let ($escape $eOk):(ScalarExprAt $args 2) $eOk) &
+        (ConstStringEquals $escape "\\")
+)
+=>
+(Like $input $pattern)

pkg/sql/opt/norm/testdata/rules/scalar

@@ -2689,3 +2689,60 @@ project
  └── projections
       ├── b.arr:5[1] [as=arr:8, outer=(5)]
       └── b.arr:5[2] [as=arr:9, outer=(5)]
+
+# --------------------------------------------------
+# ConvertLikeEscapeToLike
+# --------------------------------------------------
+
+norm expect=ConvertLikeEscapeToLike
+SELECT s LIKE 'foo%' ESCAPE '\' FROM a
+----
+project
+ ├── columns: like_escape:8
+ ├── scan a
+ │    └── columns: s:4
+ └── projections
+      └── s:4 LIKE 'foo%' [as=like_escape:8, outer=(4)]
+
+norm expect=ConvertLikeEscapeToLike
+SELECT like_escape(s, 'foo%', '\') FROM a
+----
+project
+ ├── columns: like_escape:8
+ ├── scan a
+ │    └── columns: s:4
+ └── projections
+      └── s:4 LIKE 'foo%' [as=like_escape:8, outer=(4)]
+
+norm expect-not=ConvertLikeEscapeToLike
+SELECT s LIKE 'foo%' ESCAPE '/' FROM a
+----
+project
+ ├── columns: like_escape:8
+ ├── immutable
+ ├── scan a
+ │    └── columns: s:4
+ └── projections
+      └── like_escape(s:4, 'foo%', '/') [as=like_escape:8, outer=(4), immutable]
+
+norm expect-not=ConvertLikeEscapeToLike
+SELECT s LIKE 'foo%' ESCAPE 'f' FROM a
+----
+project
+ ├── columns: like_escape:8
+ ├── immutable
+ ├── scan a
+ │    └── columns: s:4
+ └── projections
+      └── like_escape(s:4, 'foo%', 'f') [as=like_escape:8, outer=(4), immutable]
+
+norm expect-not=ConvertLikeEscapeToLike
+SELECT not_like_escape(s, 'foo%', '\') FROM a
+----
+project
+ ├── columns: not_like_escape:8
+ ├── immutable
+ ├── scan a
+ │    └── columns: s:4
+ └── projections
+      └── not_like_escape(s:4, 'foo%', e'\\') [as=not_like_escape:8, outer=(4), immutable]

pkg/workload/vecann/datasets.go

@@ -90,6 +90,8 @@ type DatasetLoader struct {
 	// CacheFolder is the path to the temporary folder where datasets will be
 	// cached. It defaults to ~/.cache/workload-datasets.
 	CacheFolder string
+	// ResetCache indicates that the cache should be re-populated.
+	ResetCache bool
 
 	// OnProgress logs the progress of the loading process.
 	OnProgress func(ctx context.Context, format string, args ...any)
@@ -128,12 +130,12 @@ func (dl *DatasetLoader) loadFiles(ctx context.Context) error {
 	neighbors := fmt.Sprintf("%s/%s-neighbors-%s.ibin", baseDir, baseName, metric)
 
 	// Download test and neighbors files if missing.
-	if !fileExists(test) {
+	if dl.ResetCache || !fileExists(test) {
 		if err := dl.downloadAndUnzip(ctx, baseName, baseName+"-test.fbin.zip", test); err != nil {
 			return err
 		}
 	}
-	if !fileExists(neighbors) {
+	if dl.ResetCache || !fileExists(neighbors) {
 		fileName := baseName + "-neighbors-" + metric + ".ibin.zip"
 		if err := dl.downloadAndUnzip(ctx, baseName, fileName, neighbors); err != nil {
 			return err
@@ -179,7 +181,7 @@ func (dl *DatasetLoader) downloadTrainFiles(
 	// First, check for files in the cache.
 	onlyFileName := fmt.Sprintf("%s/%s.fbin", baseDir, baseName)
 	firstPartName := fmt.Sprintf("%s/%s-1.fbin", baseDir, baseName)
-	if !fileExists(onlyFileName) && !fileExists(firstPartName) {
+	if dl.ResetCache || (!fileExists(onlyFileName) && !fileExists(firstPartName)) {
 		// No files in cache, download them.
 		partNum := 0
 		for {