From faeec7f34dfb4cae1b8a201ff857d6b01f2d0457 Mon Sep 17 00:00:00 2001 From: Alex Sarkesian Date: Thu, 29 Sep 2022 19:06:00 -0400 Subject: [PATCH 1/3] kvserver: decrease verbosity of replicate queue trace logging While logging of traces on replicate queue errors was recently added, logging these traces at the `WARNING` level appears to be too high, causing noisy logs. This change decreases the verbosity of these logs to the `INFO` level. Release note: None --- pkg/kv/kvserver/replicate_queue.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/kv/kvserver/replicate_queue.go b/pkg/kv/kvserver/replicate_queue.go index 95cdd448496b..ddb021664abb 100644 --- a/pkg/kv/kvserver/replicate_queue.go +++ b/pkg/kv/kvserver/replicate_queue.go @@ -814,7 +814,7 @@ func (rq *replicateQueue) processOneChangeWithTracing( } if err != nil { - log.KvDistribution.Warningf(ctx, "error processing replica: %v%s", err, traceOutput) + log.KvDistribution.Infof(ctx, "error processing replica: %v%s", err, traceOutput) } else if exceededDuration { log.KvDistribution.Infof(ctx, "processing replica took %s, exceeding threshold of %s%s", processDuration, loggingThreshold, traceOutput) From 7ca5fa1908e97db15c02148b9c0fa8d42f381ef8 Mon Sep 17 00:00:00 2001 From: Rebecca Taft Date: Fri, 30 Sep 2022 12:54:56 -0500 Subject: [PATCH 2/3] opt: fix min cardinality calculation of EXCEPT Prior to this commit, we assumed that the minimum cardinality of EXCEPT could never be lower than the minimum cardinality of the left side minus the maximum cardinality of the right side. However, this assumption is only true for EXCEPT ALL, not EXCEPT. For example, VALUES (1), (1) EXCEPT VALUES (1) produces 0 rows, even though the left side produces more rows than the right. Because of this invalid assumption, the optimizer could make some invalid transformations, resulting in incorrect results. Fixes #89101 Release note (bug fix): Fixed a bug that has existed since v2.1.0 where queries containing a subquery with EXCEPT could produce incorrect results. This could happen if the optimizer could guarantee that the left side of the EXCEPT always returned more rows than the right side. In this case, the optimizer made a faulty assupmtion that the EXCEPT subquery always returned at least one row, which could cause the optimizer to perform an invalid transformation, possibly causing the full query to return incorrect results. --- pkg/sql/opt/memo/logical_props_builder.go | 7 ++- pkg/sql/opt/memo/testdata/logprops/set | 54 +++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/pkg/sql/opt/memo/logical_props_builder.go b/pkg/sql/opt/memo/logical_props_builder.go index 996531bc0cda..5dc07d24b5ab 100644 --- a/pkg/sql/opt/memo/logical_props_builder.go +++ b/pkg/sql/opt/memo/logical_props_builder.go @@ -1928,9 +1928,14 @@ func (b *logicalPropsBuilder) makeSetCardinality( card = props.Cardinality{Min: 0, Max: left.Max} card = card.Limit(right.Max) - case opt.ExceptOp, opt.ExceptAllOp: + case opt.ExceptOp: // Use left Max cardinality. card = props.Cardinality{Min: 0, Max: left.Max} + + case opt.ExceptAllOp: + // Use left Max cardinality. Cardinality cannot be less than left Min minus + // right Max. + card = props.Cardinality{Min: 0, Max: left.Max} if left.Min > right.Max { card.Min = left.Min - right.Max } diff --git a/pkg/sql/opt/memo/testdata/logprops/set b/pkg/sql/opt/memo/testdata/logprops/set index 948970af0e93..9ae1125d2cb2 100644 --- a/pkg/sql/opt/memo/testdata/logprops/set +++ b/pkg/sql/opt/memo/testdata/logprops/set @@ -585,3 +585,57 @@ except-all └── eq [type=bool, outer=(6,7), constraints=(/6: (/NULL - ]; /7: (/NULL - ]), fd=(6)==(7), (7)==(6)] ├── variable: a:6 [type=int] └── variable: b:7 [type=int] + +# Regression test for #89101. EXCEPT can have cardinality 0 even if left side +# has more rows than the right. +norm +VALUES (1), (1) EXCEPT VALUES (1) +---- +except + ├── columns: column1:1(int!null) + ├── left columns: column1:1(int!null) + ├── right columns: column1:2(int) + ├── cardinality: [0 - 2] + ├── key: (1) + ├── values + │ ├── columns: column1:1(int!null) + │ ├── cardinality: [2 - 2] + │ ├── prune: (1) + │ ├── tuple [type=tuple{int}] + │ │ └── const: 1 [type=int] + │ └── tuple [type=tuple{int}] + │ └── const: 1 [type=int] + └── values + ├── columns: column1:2(int!null) + ├── cardinality: [1 - 1] + ├── key: () + ├── fd: ()-->(2) + ├── prune: (2) + └── tuple [type=tuple{int}] + └── const: 1 [type=int] + +# EXCEPT ALL cannot have cardinality lower than min left - max right. +norm +VALUES (1), (1) EXCEPT ALL VALUES (1) +---- +except-all + ├── columns: column1:1(int!null) + ├── left columns: column1:1(int!null) + ├── right columns: column1:2(int) + ├── cardinality: [1 - 2] + ├── values + │ ├── columns: column1:1(int!null) + │ ├── cardinality: [2 - 2] + │ ├── prune: (1) + │ ├── tuple [type=tuple{int}] + │ │ └── const: 1 [type=int] + │ └── tuple [type=tuple{int}] + │ └── const: 1 [type=int] + └── values + ├── columns: column1:2(int!null) + ├── cardinality: [1 - 1] + ├── key: () + ├── fd: ()-->(2) + ├── prune: (2) + └── tuple [type=tuple{int}] + └── const: 1 [type=int] From a1c893f007e91a1ac7c994f0c94387793dc230ec Mon Sep 17 00:00:00 2001 From: Yahor Yuzefovich Date: Fri, 30 Sep 2022 12:20:35 -0700 Subject: [PATCH 3/3] sql: correctly reset the planner in the copy shim Previously, we forgot to update the txn of the eval context of the planner. Release note: None --- pkg/sql/copyshim.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/sql/copyshim.go b/pkg/sql/copyshim.go index 659e626fbefa..cc833f9454e1 100644 --- a/pkg/sql/copyshim.go +++ b/pkg/sql/copyshim.go @@ -98,6 +98,7 @@ func RunCopyFrom( p.cancelChecker.Reset(ctx) p.optPlanningCtx.init(p) p.resetPlanner(ctx, txn, stmtTS, p.sessionDataMutatorIterator.sds.Top()) + p.extendedEvalCtx.Context.Txn = txn } p, cleanup := newInternalPlanner("copytest", txn,