From 7d88e1873deecaba9bcd20964382c16c470043c2 Mon Sep 17 00:00:00 2001 From: Marcus Gartner Date: Fri, 11 Oct 2024 10:33:13 -0700 Subject: [PATCH 1/4] pgwire: add CHAR(N) benchmarks `BenchmarkWriteTextColumnarChar` and `BenchmarkWriteBinaryColumnarChar` have been added. They are similar to existing benchmarks for strings, but use the `CHAR(16)` type. Release note: None --- pkg/sql/pgwire/types_test.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/pkg/sql/pgwire/types_test.go b/pkg/sql/pgwire/types_test.go index 37c1a7b9cde6..ffc320473748 100644 --- a/pkg/sql/pgwire/types_test.go +++ b/pkg/sql/pgwire/types_test.go @@ -504,6 +504,10 @@ func benchmarkWriteColumnarString(b *testing.B, format pgwirebase.FormatCode) { benchmarkWriteColumnar(b, getBatch(types.String), format) } +func benchmarkWriteColumnarChar(b *testing.B, format pgwirebase.FormatCode) { + benchmarkWriteColumnar(b, getBatch(types.MakeChar(16)), format) +} + func benchmarkWriteDate(b *testing.B, format pgwirebase.FormatCode) { d, _, err := tree.ParseDDate(nil, "2010-09-28") if err != nil { @@ -671,6 +675,13 @@ func BenchmarkWriteBinaryColumnarString(b *testing.B) { benchmarkWriteColumnarString(b, pgwirebase.FormatBinary) } +func BenchmarkWriteTextColumnarChar(b *testing.B) { + benchmarkWriteColumnarChar(b, pgwirebase.FormatText) +} +func BenchmarkWriteBinaryColumnarChar(b *testing.B) { + benchmarkWriteColumnarChar(b, pgwirebase.FormatBinary) +} + func BenchmarkWriteTextDate(b *testing.B) { benchmarkWriteDate(b, pgwirebase.FormatText) } From 5fb51e40962e234e2ef32a71651fb6d01d4bc72a Mon Sep 17 00:00:00 2001 From: Marcus Gartner Date: Thu, 10 Oct 2024 11:53:12 -0700 Subject: [PATCH 2/4] pgwire: reduce allocations when writing CHAR(N) datums `ResolveBlankPaddedCharBytes` has been replaced with a more efficient method of padding `CHAR(N)` datums with trailing spaces. Release note: None --- pkg/sql/pgwire/types.go | 19 +++++++++++++++++-- pkg/sql/sem/tree/pgwire_encode.go | 11 ----------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/pkg/sql/pgwire/types.go b/pkg/sql/pgwire/types.go index 1a0dd3a9f813..d24c4090c793 100644 --- a/pkg/sql/pgwire/types.go +++ b/pkg/sql/pgwire/types.go @@ -515,15 +515,30 @@ func writeBinaryDecimal(b *writeBuffer, v *apd.Decimal) { } } +// spaces is used for padding CHAR(N) datums. +var spaces = [16]byte{ + ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', +} + func writeBinaryBytes(b *writeBuffer, v []byte, t *types.T) { - v = tree.ResolveBlankPaddedCharBytes(v, t) if t.Oid() == oid.T_char && len(v) == 0 { // Match Postgres and always explicitly include a null byte if we have // an empty string for the "char" type in the binary format. v = []byte{0} } - b.putInt32(int32(len(v))) + pad := 0 + if t.Oid() == oid.T_bpchar && len(v) < int(t.Width()) { + // Pad spaces on the right of the byte slice to make it of length + // specified in the type t. + pad = int(t.Width()) - len(v) + } + b.putInt32(int32(len(v) + pad)) b.write(v) + for pad > 0 { + n := min(pad, len(spaces)) + b.write(spaces[:n]) + pad -= n + } } func writeBinaryString(b *writeBuffer, v string, t *types.T) { diff --git a/pkg/sql/sem/tree/pgwire_encode.go b/pkg/sql/sem/tree/pgwire_encode.go index d45424afb7af..43503749f3a0 100644 --- a/pkg/sql/sem/tree/pgwire_encode.go +++ b/pkg/sql/sem/tree/pgwire_encode.go @@ -32,17 +32,6 @@ func ResolveBlankPaddedChar(s string, t *types.T) string { return s } -// ResolveBlankPaddedCharBytes is similar to ResolveBlankPaddedChar but operates -// on a slice of bytes instead of a string. -func ResolveBlankPaddedCharBytes(v []byte, t *types.T) []byte { - if t.Oid() == oid.T_bpchar && len(v) < int(t.Width()) { - // Pad spaces on the right of the byte slice to make it of length - // specified in the type t. - return append(v, bytes.Repeat([]byte(" "), int(t.Width())-len(v))...) - } - return v -} - func (d *DTuple) pgwireFormat(ctx *FmtCtx) { // When converting a tuple to text in "postgres mode" there is // special behavior: values are printed in "postgres mode" then the From 67515168049e7e4473a915615122a65ae1a0a7a1 Mon Sep 17 00:00:00 2001 From: Marcus Gartner Date: Fri, 11 Oct 2024 16:10:04 -0700 Subject: [PATCH 3/4] opt: remove outdated TODO A TODO that was addressed in #129007 has been removed. Release note: None --- pkg/sql/opt/optbuilder/testdata/scalar | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/sql/opt/optbuilder/testdata/scalar b/pkg/sql/opt/optbuilder/testdata/scalar index 8d320a5f39ce..7c74aa12f572 100644 --- a/pkg/sql/opt/optbuilder/testdata/scalar +++ b/pkg/sql/opt/optbuilder/testdata/scalar @@ -1664,7 +1664,6 @@ project └── filters └── NOT ((t:1 >= t:1) AND (t:1 <= CASE NULL WHEN true THEN c:5 ELSE t:1::BPCHAR END)) -# TODO(#108360): implicit casts from STRING to CHAR should use bpchar. exec-ddl CREATE TABLE t108360_1 (t TEXT) ---- From 119d5f6ded60b132d99e6750cde025be22910fb3 Mon Sep 17 00:00:00 2001 From: Pavel Kalinnikov Date: Mon, 14 Oct 2024 09:26:05 +0100 Subject: [PATCH 4/4] kvserver: skip empty RACv1 dispatches This commit skips creating a RaftMessageRequest in the fallback RAC admission dispatch code if there are no pending RACv1 dispatches. Previously, it would send an empty RaftMessageRequest which would cause an error in the receiver's handleRaftRequest, message drops and the receiver's message queue to restart after the following error: unable to accept Raft message from (n0,s0):?: no handler registered for (n0,s0):? Epic: none Release note: none --- pkg/kv/kvserver/raft_transport.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/pkg/kv/kvserver/raft_transport.go b/pkg/kv/kvserver/raft_transport.go index caecb427baf2..4b6426ac441f 100644 --- a/pkg/kv/kvserver/raft_transport.go +++ b/pkg/kv/kvserver/raft_transport.go @@ -885,10 +885,12 @@ func (t *RaftTransport) processQueue( dispatchPendingFlowTokensTimer.Reset(0) } - req := newRaftMessageRequest() - maybeAnnotateWithAdmittedRaftLogEntries(req, pendingDispatches) - batch.Requests = append(batch.Requests, *req) - releaseRaftMessageRequest(req) + if len(pendingDispatches) != 0 { + req := newRaftMessageRequest() + maybeAnnotateWithAdmittedRaftLogEntries(req, pendingDispatches) + batch.Requests = append(batch.Requests, *req) + releaseRaftMessageRequest(req) + } maybeAnnotateWithStoreIDs(batch) annotateWithClockTimestamp(batch)