Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
89042: kvserver: decrease verbosity of replicate queue trace logging r=AlexTalks a=AlexTalks

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

89108: opt: fix min cardinality calculation of EXCEPT r=rytaft a=rytaft

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.

89112: sql: correctly reset the planner in the copy shim r=yuzefovich a=yuzefovich

Previously, we forgot to update the txn of the eval context of the planner.

Release note: None

Co-authored-by: Alex Sarkesian <[email protected]>
Co-authored-by: Rebecca Taft <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
  • Loading branch information
4 people committed Sep 30, 2022
4 parents 5990da6 + faeec7f + 7ca5fa1 + a1c893f commit 8e6b81b
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 2 deletions.
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/replicate_queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/copyshim.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 6 additions & 1 deletion pkg/sql/opt/memo/logical_props_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
54 changes: 54 additions & 0 deletions pkg/sql/opt/memo/testdata/logprops/set
Original file line number Diff line number Diff line change
Expand Up @@ -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]

0 comments on commit 8e6b81b

Please sign in to comment.