Merge cockroachdb#122054 cockroachdb#122299

122054: asof: optimize DatumToHLC when converting from strings r=yuzefovich a=yuzefovich

See each commit for details.

The combined effect on the newly-added benchmark is:
```
name                           old time/op    new time/op    delta
DatumToHLC/stringTimestamp-24     684ns ± 0%     420ns ± 0%   -38.51%  (p=0.000 n=9+9)
DatumToHLC/stringDecimal-24      12.0µs ± 0%     1.3µs ± 0%   -89.48%  (p=0.000 n=10+10)
DatumToHLC/stringInterval-24     5.87µs ± 0%    2.97µs ± 0%   -49.41%  (p=0.000 n=10+10)

name                           old alloc/op   new alloc/op   delta
DatumToHLC/stringTimestamp-24      920B ± 0%        0B       -100.00%  (p=0.000 n=10+10)
DatumToHLC/stringDecimal-24      2.98kB ± 0%    0.26kB ± 0%   -91.40%  (p=0.000 n=10+10)
DatumToHLC/stringInterval-24     2.05kB ± 0%    0.66kB ± 0%   -67.76%  (p=0.000 n=10+10)

name                           old allocs/op  new allocs/op  delta
DatumToHLC/stringTimestamp-24      2.00 ± 0%      0.00       -100.00%  (p=0.000 n=10+10)
DatumToHLC/stringDecimal-24        56.0 ± 0%       7.0 ± 0%   -87.50%  (p=0.000 n=10+10)
DatumToHLC/stringInterval-24       28.0 ± 0%      18.0 ± 0%   -35.71%  (p=0.000 n=10+10)
```

Fixes: cockroachdb#121297.

122299: go.mod: bump Pebble to 26032dc0f80b r=RaduBerinde a=jbowens

Changes:

 * [`26032dc0`](cockroachdb/pebble@26032dc0) internal/base: add MakeInternalKV
 * [`d01e5e99`](cockroachdb/pebble@d01e5e99) *: remove InternalKV.UserKey func
 * [`d253ff7b`](cockroachdb/pebble@d253ff7b) Configurable initial and max retained batch sizes
 * [`978bd26c`](cockroachdb/pebble@978bd26c) sstable: move empty block property logic to encoder/decoder
 * [`af7e51a3`](cockroachdb/pebble@af7e51a3) *: return *InternalKV from internal iterators

Release note: none.
Epic: none.

Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: Jackson Owens <[email protected]>

DEPS.bzl

@@ -1683,10 +1683,10 @@ def go_deps():
         patches = [
             "@com_github_cockroachdb_cockroach//build/patches:com_github_cockroachdb_pebble.patch",
         ],
-        sha256 = "f6d30631ba0ac55abbc0d06b4d28ae1d80df380cc88d0060df8cf188bdecd391",
-        strip_prefix = "github.com/cockroachdb/[email protected]20240409231136-6cdb88d44473",
+        sha256 = "67130f354fa38ef229503f8872696a5f2534636e5419f48e8ae04808565f9fce",
+        strip_prefix = "github.com/cockroachdb/[email protected]20240412140635-26032dc0f80b",
         urls = [
-            "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20240409231136-6cdb88d44473.zip",
+            "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20240412140635-26032dc0f80b.zip",
         ],
     )
     go_repository(

build/bazelutil/distdir_files.bzl

@@ -329,7 +329,7 @@ DISTDIR_FILES = {
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/gostdlib/com_github_cockroachdb_gostdlib-v1.19.0.zip": "c4d516bcfe8c07b6fc09b8a9a07a95065b36c2855627cb3514e40c98f872b69e",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/logtags/com_github_cockroachdb_logtags-v0.0.0-20230118201751-21c54148d20b.zip": "ca7776f47e5fecb4c495490a679036bfc29d95bd7625290cfdb9abb0baf97476",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/metamorphic/com_github_cockroachdb_metamorphic-v0.0.0-20231108215700-4ba948b56895.zip": "28c8cf42192951b69378cf537be5a9a43f2aeb35542908cc4fe5f689505853ea",
-    "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20240409231136-6cdb88d44473.zip": "f6d30631ba0ac55abbc0d06b4d28ae1d80df380cc88d0060df8cf188bdecd391",
+    "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20240412140635-26032dc0f80b.zip": "67130f354fa38ef229503f8872696a5f2534636e5419f48e8ae04808565f9fce",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/redact/com_github_cockroachdb_redact-v1.1.5.zip": "11b30528eb0dafc8bc1a5ba39d81277c257cbe6946a7564402f588357c164560",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/returncheck/com_github_cockroachdb_returncheck-v0.0.0-20200612231554-92cdbca611dd.zip": "ce92ba4352deec995b1f2eecf16eba7f5d51f5aa245a1c362dfe24c83d31f82b",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/stress/com_github_cockroachdb_stress-v0.0.0-20220803192808-1806698b1b7b.zip": "3fda531795c600daf25532a4f98be2a1335cd1e5e182c72789bca79f5f69fcc1",

go.mod

@@ -124,7 +124,7 @@ require (
 	github.com/cockroachdb/go-test-teamcity v0.0.0-20191211140407-cff980ad0a55
 	github.com/cockroachdb/gostdlib v1.19.0
 	github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b
-	github.com/cockroachdb/pebble v0.0.0-20240409231136-6cdb88d44473
+	github.com/cockroachdb/pebble v0.0.0-20240412140635-26032dc0f80b
 	github.com/cockroachdb/redact v1.1.5
 	github.com/cockroachdb/returncheck v0.0.0-20200612231554-92cdbca611dd
 	github.com/cockroachdb/stress v0.0.0-20220803192808-1806698b1b7b

go.sum

@@ -505,8 +505,8 @@ github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b h1:r6VH0faHjZe
 github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b/go.mod h1:Vz9DsVWQQhf3vs21MhPMZpMGSht7O/2vFW2xusFUVOs=
 github.com/cockroachdb/metamorphic v0.0.0-20231108215700-4ba948b56895 h1:XANOgPYtvELQ/h4IrmPAohXqe2pWA8Bwhejr3VQoZsA=
 github.com/cockroachdb/metamorphic v0.0.0-20231108215700-4ba948b56895/go.mod h1:aPd7gM9ov9M8v32Yy5NJrDyOcD8z642dqs+F0CeNXfA=
-github.com/cockroachdb/pebble v0.0.0-20240409231136-6cdb88d44473 h1:/w0fCyZQgS0TowF2NBPGPF3SX1qmprPoDiYHrZ6VAnU=
-github.com/cockroachdb/pebble v0.0.0-20240409231136-6cdb88d44473/go.mod h1:gm/vT3lwZUKyB3iTDgWIZfC0hu0gLr+VcXr/tZeTdEU=
+github.com/cockroachdb/pebble v0.0.0-20240412140635-26032dc0f80b h1:Uw1oKQzeOIVtuIemNTH1RMGTfEzJzFQBQqBdtHYYTN8=
+github.com/cockroachdb/pebble v0.0.0-20240412140635-26032dc0f80b/go.mod h1:gm/vT3lwZUKyB3iTDgWIZfC0hu0gLr+VcXr/tZeTdEU=
 github.com/cockroachdb/redact v1.1.3/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg=
 github.com/cockroachdb/redact v1.1.5 h1:u1PMllDkdFfPWaNGMyLD1+so+aq3uUItthCFqzwPJ30=
 github.com/cockroachdb/redact v1.1.5/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg=

pkg/BUILD.bazel

@@ -561,6 +561,7 @@ ALL_TESTS = [
     "//pkg/sql/schemachanger/screl:screl_test",
     "//pkg/sql/schemachanger/scrun:scrun_test",
     "//pkg/sql/schemachanger:schemachanger_test",
+    "//pkg/sql/sem/asof:asof_test",
     "//pkg/sql/sem/builtins/pgformat:pgformat_test",
     "//pkg/sql/sem/builtins:builtins_disallowed_imports_test",
     "//pkg/sql/sem/builtins:builtins_test",
@@ -2114,6 +2115,7 @@ GO_TARGETS = [
     "//pkg/sql/scrub/scrubtestutils:scrubtestutils",
     "//pkg/sql/scrub:scrub",
     "//pkg/sql/sem/asof:asof",
+    "//pkg/sql/sem/asof:asof_test",
     "//pkg/sql/sem/builtins/builtinconstants:builtinconstants",
     "//pkg/sql/sem/builtins/builtinsregistry:builtinsregistry",
     "//pkg/sql/sem/builtins/pgcrypto:pgcrypto",

pkg/sql/colexec/execgen/cmd/execgen/cast_gen_util.go

@@ -589,7 +589,8 @@ func getStringToTimestampCastFunc(withoutTimezone bool) castFunc {
 		_roundTo := tree.TimeFamilyPrecisionToRoundDuration(%[4]s.Precision())
 		_now := %[3]s.GetRelativeParseTime()
 		_dateStyle := %[3]s.GetDateStyle()
-		_t, _, err := pgdate.ParseTimestamp%[5]s(_now, _dateStyle, string(%[2]s))
+		_h := %[3]s.GetDateHelper()
+		_t, _, err := pgdate.ParseTimestamp%[5]s(_now, _dateStyle, string(%[2]s), _h)
 		if err != nil {
 			colexecerror.ExpectedError(err)
 		}

pkg/sql/information_schema.go

@@ -2892,6 +2892,7 @@ func (r roleOptions) validUntil(p *planner) (tree.Datum, error) {
 		p.EvalContext().GetRelativeParseTime(),
 		pgdate.DefaultDateStyle(),
 		*validUntilText,
+		nil, /* h */
 	)
 	if err != nil {
 		return nil, errors.Errorf("rolValidUntil string %s could not be parsed with datestyle %s", *validUntilText, p.EvalContext().GetDateStyle())

pkg/sql/sem/asof/BUILD.bazel

@@ -1,4 +1,4 @@
-load("@io_bazel_rules_go//go:def.bzl", "go_library")
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
 
 go_library(
     name = "asof",
@@ -21,3 +21,16 @@ go_library(
         "@com_github_cockroachdb_errors//:errors",
     ],
 )
+
+go_test(
+    name = "asof_test",
+    srcs = ["as_of_test.go"],
+    deps = [
+        ":asof",
+        "//pkg/settings/cluster",
+        "//pkg/sql/sem/eval",
+        "//pkg/sql/sem/tree",
+        "//pkg/util/timeutil",
+        "@com_github_stretchr_testify//require",
+    ],
+)

pkg/sql/sem/asof/as_of.go

@@ -244,8 +244,15 @@ func DatumToHLC(
 	case *tree.DString:
 		s := string(*d)
 		// Attempt to parse as timestamp.
-		if dt, _, err := tree.ParseDTimestampTZ(evalCtx, s, time.Nanosecond); err == nil {
-			ts.WallTime = dt.Time.UnixNano()
+		//
+		// Disable error annotation since we don't care what the error is if it
+		// occurs.
+		defer func(origValue bool) {
+			evalCtx.GetDateHelper().SkipErrorAnnotation = origValue
+		}(evalCtx.GetDateHelper().SkipErrorAnnotation)
+		evalCtx.GetDateHelper().SkipErrorAnnotation = true
+		if t, _, err := tree.ParseTimestampTZ(evalCtx, s, time.Nanosecond); err == nil {
+			ts.WallTime = t.UnixNano()
 			break
 		}
 		// Attempt to parse as a decimal.
@@ -254,13 +261,13 @@ func DatumToHLC(
 			break
 		}
 		// Attempt to parse as an interval.
-		if iv, err := tree.ParseDInterval(evalCtx.GetIntervalStyle(), s); err == nil {
-			if (iv.Duration == duration.Duration{}) {
+		if iv, err := tree.ParseIntervalWithTypeMetadata(evalCtx.GetIntervalStyle(), s, types.DefaultIntervalTypeMetadata); err == nil {
+			if (iv == duration.Duration{}) {
 				convErr = errors.Errorf("interval value %v too small, absolute value must be >= %v", d, time.Microsecond)
-			} else if (usage == Split && iv.Duration.Compare(duration.Duration{}) < 0) {
+			} else if (usage == Split && iv.Compare(duration.Duration{}) < 0) {
 				convErr = errors.Errorf("interval value %v too small, SPLIT AT interval must be >= %v", d, time.Microsecond)
 			}
-			ts.WallTime = duration.Add(stmtTimestamp, iv.Duration).UnixNano()
+			ts.WallTime = duration.Add(stmtTimestamp, iv).UnixNano()
 			break
 		}
 		convErr = errors.Errorf("value is neither timestamp, decimal, nor interval")

pkg/sql/sem/asof/as_of_test.go

@@ -0,0 +1,45 @@
+// Copyright 2024 The Cockroach Authors.
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0, included in the file
+// licenses/APL.txt.
+
+package asof_test
+
+import (
+	"testing"
+
+	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
+	"github.com/cockroachdb/cockroach/pkg/sql/sem/asof"
+	"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
+	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
+	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
+	"github.com/stretchr/testify/require"
+)
+
+func BenchmarkDatumToHLC(b *testing.B) {
+	evalCtx := eval.MakeTestingEvalContext(cluster.MakeTestingClusterSettings())
+	stmtTimestamp := timeutil.Now()
+	usage := asof.AsOf
+
+	for _, tc := range []struct {
+		name string
+		d    tree.Datum
+	}{
+		{name: "stringTimestamp", d: tree.NewDString("2024-04-09 12:00:00.000000")},
+		{name: "stringDecimal", d: tree.NewDString("1580361670629466905.0000000001")},
+		{name: "stringInterval", d: tree.NewDString("-5s")},
+	} {
+		b.Run(tc.name, func(b *testing.B) {
+			var err error
+			for i := 0; i < b.N; i++ {
+				_, err = asof.DatumToHLC(&evalCtx, stmtTimestamp, tc.d, usage)
+			}
+			require.NoError(b, err)
+		})
+	}
+}

pkg/sql/sem/tree/datum.go

@@ -2094,7 +2094,7 @@ type ParseContext interface {
 	GetIntervalStyle() duration.IntervalStyle
 	// GetDateStyle returns the date style in the session.
 	GetDateStyle() pgdate.DateStyle
-	// GetParseHelper returns a helper to optmize date parsing.
+	// GetDateHelper returns a helper to optimize parsing of datetime types.
 	GetDateHelper() *pgdate.ParseHelper
 }
 
@@ -2383,7 +2383,7 @@ func ParseDTime(
 
 	s = timeutil.ReplaceLibPQTimePrefix(s)
 
-	t, dependsOnContext, err := pgdate.ParseTimeWithoutTimezone(now, dateStyle(ctx), s)
+	t, dependsOnContext, err := pgdate.ParseTimeWithoutTimezone(now, dateStyle(ctx), s, dateParseHelper(ctx))
 	if err != nil {
 		return nil, false, MakeParseError(s, types.Time, err)
 	}
@@ -2723,7 +2723,7 @@ func ParseDTimestamp(
 	ctx ParseContext, s string, precision time.Duration,
 ) (_ *DTimestamp, dependsOnContext bool, _ error) {
 	now := relativeParseTime(ctx)
-	t, dependsOnContext, err := pgdate.ParseTimestampWithoutTimezone(now, dateStyle(ctx), s)
+	t, dependsOnContext, err := pgdate.ParseTimestampWithoutTimezone(now, dateStyle(ctx), s, dateParseHelper(ctx))
 	if err != nil {
 		return nil, false, err
 	}
@@ -3012,25 +3012,39 @@ func MakeDTimestampTZFromDate(loc *time.Location, d *DDate) (*DTimestampTZ, erro
 	return MakeDTimestampTZ(t.Add(time.Duration(-offset)*time.Second), time.Microsecond)
 }
 
+// ParseTimestampTZ parses and returns the time.Time value represented by the
+// provided string in the provided location, or an error if parsing is
+// unsuccessful.
+//
+// The dependsOnContext return value indicates if we had to consult the
+// ParseContext (either for the time or the local timezone).
+func ParseTimestampTZ(
+	ctx ParseContext, s string, precision time.Duration,
+) (_ time.Time, dependsOnContext bool, _ error) {
+	now := relativeParseTime(ctx)
+	t, dependsOnContext, err := pgdate.ParseTimestamp(now, dateStyle(ctx), s, dateParseHelper(ctx))
+	if err != nil {
+		return time.Time{}, false, err
+	}
+	if t, err = checkTimeBounds(t, precision); err != nil {
+		return time.Time{}, false, err
+	}
+	return t, dependsOnContext, nil
+}
+
 // ParseDTimestampTZ parses and returns the *DTimestampTZ Datum value represented by
 // the provided string in the provided location, or an error if parsing is unsuccessful.
 //
 // The dependsOnContext return value indicates if we had to consult the
 // ParseContext (either for the time or the local timezone).
-//
-// Parts of this function are inlined into ParseAndRequireStringHandler, if this
-// changes materially the timestamp case arms there may need to change too.
 func ParseDTimestampTZ(
 	ctx ParseContext, s string, precision time.Duration,
 ) (_ *DTimestampTZ, dependsOnContext bool, _ error) {
-	now := relativeParseTime(ctx)
-	t, dependsOnContext, err := pgdate.ParseTimestamp(now, dateStyle(ctx), s)
+	t, dependsOnContext, err := ParseTimestampTZ(ctx, s, precision)
 	if err != nil {
 		return nil, false, err
 	}
-	// Always normalize time to the current location.
-	d, err := MakeDTimestampTZ(t, precision)
-	return d, dependsOnContext, err
+	return &DTimestampTZ{Time: t}, dependsOnContext, err
 }
 
 // DZeroTimestampTZ is the zero-valued DTimestampTZ.

pkg/sql/sem/tree/parse_string.go

@@ -257,20 +257,16 @@ func ParseAndRequireStringHandler(
 		s = truncateString(s, t)
 		vh.String(s)
 	case types.TimestampTZFamily:
-		// TODO(cucaroach): can we refactor the next 3 case arms to be simpler
-		// and avoid code duplication?
-		now := relativeParseTime(ctx)
 		var ts time.Time
-		if ts, _, err = pgdate.ParseTimestamp(now, dateStyle(ctx), s); err == nil {
-			// Always normalize time to the current location.
-			if ts, err = checkTimeBounds(ts, TimeFamilyPrecisionToRoundDuration(t.Precision())); err == nil {
-				vh.TimestampTZ(ts)
-			}
+		if ts, _, err = ParseTimestampTZ(ctx, s, TimeFamilyPrecisionToRoundDuration(t.Precision())); err == nil {
+			vh.TimestampTZ(ts)
 		}
 	case types.TimestampFamily:
+		// TODO(yuzefovich): can we refactor the next 2 case arms to be simpler
+		// and avoid code duplication?
 		now := relativeParseTime(ctx)
 		var ts time.Time
-		if ts, _, err = pgdate.ParseTimestampWithoutTimezone(now, dateStyle(ctx), s); err == nil {
+		if ts, _, err = pgdate.ParseTimestampWithoutTimezone(now, dateStyle(ctx), s, dateParseHelper(ctx)); err == nil {
 			// Always normalize time to the current location.
 			if ts, err = checkTimeBounds(ts, TimeFamilyPrecisionToRoundDuration(t.Precision())); err == nil {
 				vh.TimestampTZ(ts)

pkg/storage/mvcc_history_test.go

@@ -280,23 +280,23 @@ func TestMVCCHistories(t *testing.T) {
 				return err
 			}
 			defer func() { _ = iter.Close() }()
-			for k, lv := iter.SeekGE(nil, sstable.SeekGEFlags(0)); k != nil; k, lv = iter.Next() {
+			for kv := iter.SeekGE(nil, sstable.SeekGEFlags(0)); kv != nil; kv = iter.Next() {
 				if err := iter.Error(); err != nil {
 					return err
 				}
-				key, err := storage.DecodeMVCCKey(k.UserKey)
+				key, err := storage.DecodeMVCCKey(kv.K.UserKey)
 				if err != nil {
 					return err
 				}
-				v, _, err := lv.Value(nil)
+				v, _, err := kv.Value(nil)
 				if err != nil {
 					return err
 				}
 				value, err := storage.DecodeMVCCValue(v)
 				if err != nil {
 					return err
 				}
-				buf.Printf("%s: %s -> %s\n", strings.ToLower(k.Kind().String()), key, value)
+				buf.Printf("%s: %s -> %s\n", strings.ToLower(kv.Kind().String()), key, value)
 			}
 
 			// Dump rangedels.

pkg/util/timetz/timetz.go

@@ -120,7 +120,7 @@ func ParseTimeTZ(
 		s = timeutil.ReplaceLibPQTimePrefix(s)
 	}
 
-	t, dependsOnContext, err := pgdate.ParseTimestamp(now, dateStyle, s)
+	t, dependsOnContext, err := pgdate.ParseTimestamp(now, dateStyle, s, nil /* h */)
 	if err != nil {
 		// Build our own error message to avoid exposing the dummy date.
 		return TimeTZ{}, false, pgerror.Newf(