Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

release-21.2: sql: fix overflow for soft limits #79924

Merged
merged 1 commit into from
Apr 18, 2022
Merged

release-21.2: sql: fix overflow for soft limits #79924

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/sql/distsql_physical_planner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2231,7 +2231,7 @@ func (dsp *DistSQLPlanner) createPlanForIndexJoin(
LockingWaitPolicy: n.table.lockingWaitPolicy,
MaintainOrdering: len(n.reqOrdering) > 0,
HasSystemColumns: n.table.containsSystemColumns,
LimitHint: int64(n.limitHint),
LimitHint: n.limitHint,
}

post := execinfrapb.PostProcessSpec{
Expand Down Expand Up @@ -2290,7 +2290,7 @@ func (dsp *DistSQLPlanner) createPlanForLookupJoin(
HasSystemColumns: n.table.containsSystemColumns,
LeftJoinWithPairedJoiner: n.isSecondJoinInPairedJoiner,
LookupBatchBytesLimit: dsp.distSQLSrv.TestingKnobs.JoinReaderBatchBytesLimit,
LimitHint: int64(n.limitHint),
LimitHint: n.limitHint,
}
joinReaderSpec.IndexIdx, err = getIndexIdx(n.table.index, n.table.desc)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/distsql_spec_exec_factory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,7 @@ func (e *distSQLSpecExecFactory) ConstructIndexJoin(
keyCols []exec.NodeColumnOrdinal,
tableCols exec.TableColumnOrdinalSet,
reqOrdering exec.OutputOrdering,
limitHint int,
limitHint int64,
) (exec.Node, error) {
return nil, unimplemented.NewWithIssue(47473, "experimental opt-driven distsql planning: index join")
}
Expand All @@ -662,7 +662,7 @@ func (e *distSQLSpecExecFactory) ConstructLookupJoin(
isSecondJoinInPairedJoiner bool,
reqOrdering exec.OutputOrdering,
locking *tree.LockingItem,
limitHint int,
limitHint int64,
) (exec.Node, error) {
// TODO (rohany): Implement production of system columns by the underlying scan here.
return nil, unimplemented.NewWithIssue(47473, "experimental opt-driven distsql planning: lookup join")
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/index_join.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ type indexJoinNode struct {

reqOrdering ReqOrdering

limitHint int
limitHint int64
}

func (n *indexJoinNode) startExec(params runParams) error {
Expand Down
4 changes: 4 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/limit
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,3 +351,7 @@ SELECT oid::INT, typname FROM pg_type ORDER BY oid LIMIT 3
16 bool
17 bytea
18 char

# Regression test for limit hint overflowing int64 range and becoming negative.
statement ok
SELECT * FROM t65171 WHERE x = 1 OFFSET 1 LIMIT 9223372036854775807
2 changes: 1 addition & 1 deletion pkg/sql/lookup_join.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ type lookupJoinNode struct {

reqOrdering ReqOrdering

limitHint int
limitHint int64
}

func (lj *lookupJoinNode) startExec(params runParams) error {
Expand Down
7 changes: 3 additions & 4 deletions pkg/sql/opt/exec/execbuilder/relational.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"bytes"
"context"
"fmt"
"math"

"github.com/cockroachdb/cockroach/pkg/server/telemetry"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo"
Expand Down Expand Up @@ -576,7 +575,7 @@ func (b *Builder) scanParams(
sqltelemetry.IncrementPartitioningCounter(sqltelemetry.PartitionConstrainedScan)
}

softLimit := int64(math.Ceil(reqProps.LimitHint))
softLimit := reqProps.LimitHintInt64()
hardLimit := scan.HardLimit.RowCount()

// If this is a bounded staleness query, check that it touches at most one
Expand Down Expand Up @@ -1628,7 +1627,7 @@ func (b *Builder) buildIndexJoin(join *memo.IndexJoinExpr) (execPlan, error) {
needed, output := b.getColumns(cols, join.Table)
res := execPlan{outputCols: output}
res.root, err = b.factory.ConstructIndexJoin(
input.root, tab, keyCols, needed, res.reqOrdering(join), int(math.Ceil(join.RequiredPhysical().LimitHint)),
input.root, tab, keyCols, needed, res.reqOrdering(join), join.RequiredPhysical().LimitHintInt64(),
)
if err != nil {
return execPlan{}, err
Expand Down Expand Up @@ -1721,7 +1720,7 @@ func (b *Builder) buildLookupJoin(join *memo.LookupJoinExpr) (execPlan, error) {
join.IsSecondJoinInPairedJoiner,
res.reqOrdering(join),
locking,
int(math.Ceil(join.RequiredPhysical().LimitHint)),
join.RequiredPhysical().LimitHintInt64(),
)
if err != nil {
return execPlan{}, err
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/opt/exec/factory.opt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ define IndexJoin {
KeyCols []exec.NodeColumnOrdinal
TableCols exec.TableColumnOrdinalSet
ReqOrdering exec.OutputOrdering
LimitHint int
LimitHint int64
}

# LookupJoin performs a lookup join.
Expand Down Expand Up @@ -280,7 +280,7 @@ define LookupJoin {
IsSecondJoinInPairedJoiner bool
ReqOrdering exec.OutputOrdering
Locking *tree.LockingItem
LimitHint int
LimitHint int64
}

# InvertedJoin performs a lookup join into an inverted index.
Expand Down
13 changes: 12 additions & 1 deletion pkg/sql/opt/props/physical/required.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ package physical
import (
"bytes"
"fmt"
"math"

"github.com/cockroachdb/cockroach/pkg/sql/opt"
"github.com/cockroachdb/cockroach/pkg/sql/opt/props"
Expand Down Expand Up @@ -51,7 +52,7 @@ type Required struct {
// that only the hinted number of rows will be needed.
// A LimitHint of 0 indicates "no limit". The LimitHint is an intermediate
// float64 representation, and can be converted to an integer number of rows
// using math.Ceil.
// using LimitHintInt64.
LimitHint float64
}

Expand Down Expand Up @@ -109,6 +110,16 @@ func (p *Required) Equals(rhs *Required) bool {
return p.Presentation.Equals(rhs.Presentation) && p.Ordering.Equals(&rhs.Ordering) && p.LimitHint == rhs.LimitHint
}

// LimitHintInt64 returns the limit hint converted to an int64.
func (p *Required) LimitHintInt64() int64 {
h := int64(math.Ceil(p.LimitHint))
if h < 0 {
// If we have an overflow, then disable the limit hint.
h = 0
}
return h
}

// Presentation specifies the naming, membership (including duplicates), and
// order of result columns that are required of or provided by an operator.
// While it cannot add unique columns, Presentation can rename, reorder,
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/opt_exec_factory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,7 @@ func (ef *execFactory) ConstructIndexJoin(
keyCols []exec.NodeColumnOrdinal,
tableCols exec.TableColumnOrdinalSet,
reqOrdering exec.OutputOrdering,
limitHint int,
limitHint int64,
) (exec.Node, error) {
tabDesc := table.(*optTable).desc
colCfg := makeScanColumnsConfig(table, tableCols)
Expand Down Expand Up @@ -656,7 +656,7 @@ func (ef *execFactory) ConstructLookupJoin(
isSecondJoinInPairedJoiner bool,
reqOrdering exec.OutputOrdering,
locking *tree.LockingItem,
limitHint int,
limitHint int64,
) (exec.Node, error) {
if table.IsVirtualTable() {
return ef.constructVirtualTableLookupJoin(joinType, input, table, index, eqCols, lookupCols, onCond)
Expand Down