Skip to content

Commit

Permalink
[kvexec] Fix panic in non-covering strict lookup (dolthub#8166)
Browse files Browse the repository at this point in the history 
* [kvexec] Fix panic in non-covering strict lookup

* [ga-format-pr] Run go/utils/repofmt/format_repo.sh and go/Godeps/update.sh

---------

Co-authored-by: max-hoffman <[email protected]>
  • Loading branch information
@max-hoffman
max-hoffman and max-hoffman authored Jul 29, 2024
1 parent f4c9d4f commit c563747
Show file tree
Hide file tree
Showing 9 changed files with 27 additions and 26 deletions.
4 changes: 2 additions & 2 deletions go/cmd/dolt/commands/engine/sqlengine.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ import (
dblr "github.com/dolthub/dolt/go/libraries/doltcore/sqle/binlogreplication"
"github.com/dolthub/dolt/go/libraries/doltcore/sqle/cluster"
"github.com/dolthub/dolt/go/libraries/doltcore/sqle/dsess"
"github.com/dolthub/dolt/go/libraries/doltcore/sqle/kvexec"
"github.com/dolthub/dolt/go/libraries/doltcore/sqle/mysql_file_handler"
drowexec "github.com/dolthub/dolt/go/libraries/doltcore/sqle/rowexec"
"github.com/dolthub/dolt/go/libraries/doltcore/sqle/statsnoms"
"github.com/dolthub/dolt/go/libraries/doltcore/sqle/statspro"
"github.com/dolthub/dolt/go/libraries/doltcore/sqle/writer"
Expand Down Expand Up @@ -195,7 +195,7 @@ func NewSqlEngine(
statsPro := statspro.NewProvider(pro, statsnoms.NewNomsStatsFactory(mrEnv.RemoteDialProvider()))
engine.Analyzer.Catalog.StatsProvider = statsPro

engine.Analyzer.ExecBuilder = rowexec.NewOverrideBuilder(drowexec.Builder{})
engine.Analyzer.ExecBuilder = rowexec.NewOverrideBuilder(kvexec.Builder{})
sessFactory := doltSessionFactory(pro, statsPro, mrEnv.Config(), bcController, config.Autocommit)
sqlEngine.provider = pro
sqlEngine.contextFactory = sqlContextFactory()
Expand Down
4 changes: 2 additions & 2 deletions go/go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ require (
github.com/dolthub/fslock v0.0.3
github.com/dolthub/ishell v0.0.0-20240701202509-2b217167d718
github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81
github.com/dolthub/vitess v0.0.0-20240711213744-4232e1c4edae
github.com/dolthub/vitess v0.0.0-20240723185540-9f8cd7c03829
github.com/dustin/go-humanize v1.0.1
github.com/fatih/color v1.13.0
github.com/flynn-archive/go-shlex v0.0.0-20150515145356-3f9db97f8568
Expand Down Expand Up @@ -57,7 +57,7 @@ require (
github.com/cespare/xxhash/v2 v2.2.0
github.com/creasty/defaults v1.6.0
github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2
github.com/dolthub/go-mysql-server v0.18.2-0.20240726192758-72470d302577
github.com/dolthub/go-mysql-server v0.18.2-0.20240729182701-b4cec29302d1
github.com/dolthub/gozstd v0.0.0-20240423170813-23a2903bca63
github.com/dolthub/swiss v0.1.0
github.com/goccy/go-json v0.10.2
Expand Down
8 changes: 4 additions & 4 deletions go/go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,8 @@ github.com/dolthub/fslock v0.0.3 h1:iLMpUIvJKMKm92+N1fmHVdxJP5NdyDK5bK7z7Ba2s2U=
github.com/dolthub/fslock v0.0.3/go.mod h1:QWql+P17oAAMLnL4HGB5tiovtDuAjdDTPbuqx7bYfa0=
github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e h1:kPsT4a47cw1+y/N5SSCkma7FhAPw7KeGmD6c9PBZW9Y=
github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e/go.mod h1:KPUcpx070QOfJK1gNe0zx4pA5sicIK1GMikIGLKC168=
github.com/dolthub/go-mysql-server v0.18.2-0.20240726192758-72470d302577 h1:k1zmiHU6vXi9xn5EtSRrAuys4YvRl0bOUCce4afEbsU=
github.com/dolthub/go-mysql-server v0.18.2-0.20240726192758-72470d302577/go.mod h1:P6bG0p+3mH4LS4DLo3BySh10ZJTDqgWyfWBu8gGE3eU=
github.com/dolthub/go-mysql-server v0.18.2-0.20240729182701-b4cec29302d1 h1:ydrib93syT0bpnYjgeKuLkn0R0B++lPNWMN9knWGMJY=
github.com/dolthub/go-mysql-server v0.18.2-0.20240729182701-b4cec29302d1/go.mod h1:ctH+JB+7+NeXPOfcujw/dzzPPgT9Byb/nOE44RRAztg=
github.com/dolthub/gozstd v0.0.0-20240423170813-23a2903bca63 h1:OAsXLAPL4du6tfbBgK0xXHZkOlos63RdKYS3Sgw/dfI=
github.com/dolthub/gozstd v0.0.0-20240423170813-23a2903bca63/go.mod h1:lV7lUeuDhH5thVGDCKXbatwKy2KW80L4rMT46n+Y2/Q=
github.com/dolthub/ishell v0.0.0-20240701202509-2b217167d718 h1:lT7hE5k+0nkBdj/1UOSFwjWpNxf+LCApbRHgnCA17XE=
Expand All @@ -197,8 +197,8 @@ github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81 h1:7/v8q9X
github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81/go.mod h1:siLfyv2c92W1eN/R4QqG/+RjjX5W2+gCTRjZxBjI3TY=
github.com/dolthub/swiss v0.1.0 h1:EaGQct3AqeP/MjASHLiH6i4TAmgbG/c4rA6a1bzCOPc=
github.com/dolthub/swiss v0.1.0/go.mod h1:BeucyB08Vb1G9tumVN3Vp/pyY4AMUnr9p7Rz7wJ7kAQ=
github.com/dolthub/vitess v0.0.0-20240711213744-4232e1c4edae h1:SAYP6+hzkoYLWVzQTwQO0QbhVOwMsgy0dpRcL/QriS8=
github.com/dolthub/vitess v0.0.0-20240711213744-4232e1c4edae/go.mod h1:uBvlRluuL+SbEWTCZ68o0xvsdYZER3CEG/35INdzfJM=
github.com/dolthub/vitess v0.0.0-20240723185540-9f8cd7c03829 h1:/SEo3jm8zy0EVKIUPRcfagB9drQYI4KwQlSU/DNVRAQ=
github.com/dolthub/vitess v0.0.0-20240723185540-9f8cd7c03829/go.mod h1:uBvlRluuL+SbEWTCZ68o0xvsdYZER3CEG/35INdzfJM=
github.com/dustin/go-humanize v0.0.0-20171111073723-bb3d318650d4/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=
github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY=
github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto=
Expand Down
4 changes: 2 additions & 2 deletions go/libraries/doltcore/sqle/enginetest/dolt_harness.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import (
"github.com/dolthub/dolt/go/libraries/doltcore/env"
"github.com/dolthub/dolt/go/libraries/doltcore/sqle"
"github.com/dolthub/dolt/go/libraries/doltcore/sqle/dsess"
drowexec "github.com/dolthub/dolt/go/libraries/doltcore/sqle/rowexec"
"github.com/dolthub/dolt/go/libraries/doltcore/sqle/kvexec"
"github.com/dolthub/dolt/go/libraries/doltcore/sqle/statsnoms"
"github.com/dolthub/dolt/go/libraries/doltcore/sqle/statspro"
"github.com/dolthub/dolt/go/libraries/doltcore/sqle/writer"
Expand Down Expand Up @@ -252,7 +252,7 @@ func (d *DoltHarness) NewEngine(t *testing.T) (enginetest.QueryEngine, error) {
if err != nil {
return nil, err
}
e.Analyzer.ExecBuilder = rowexec.NewOverrideBuilder(drowexec.Builder{})
e.Analyzer.ExecBuilder = rowexec.NewOverrideBuilder(kvexec.Builder{})
d.engine = e

ctx := enginetest.NewContext(d)
Expand Down
2 changes: 1 addition & 1 deletion go/libraries/doltcore/sqle/index/index_reader.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,7 @@ func (ib *nonCoveringIndexImplBuilder) NewPartitionRowIter(ctx *sql.Context, par

func (ib *nonCoveringIndexImplBuilder) NewSecondaryIter(strict bool, cnt int, nullSafe []bool) SecondaryLookupIterGen {
if strict {
return &nonCovStrictSecondaryLookupGen{pri: ib.pri, sec: ib.sec, pkMap: ib.pkMap, pkBld: ib.pkBld, sch: ib.idx.tableSch}
return &nonCovStrictSecondaryLookupGen{pri: ib.pri, sec: ib.sec, pkMap: ib.pkMap, pkBld: ib.pkBld, sch: ib.idx.tableSch, prefixDesc: ib.secKd.PrefixDesc(cnt)}
} else {
return &nonCovLaxSecondaryLookupGen{pri: ib.pri, sec: ib.sec, pkMap: ib.pkMap, pkBld: ib.pkBld, sch: ib.idx.tableSch, prefixDesc: ib.secKd.PrefixDesc(cnt), nullSafe: nullSafe}
}
Expand Down
25 changes: 13 additions & 12 deletions go/libraries/doltcore/sqle/index/secondary_iter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,12 @@ func (c *covStrictSecondaryLookupGen) New(ctx context.Context, k val.Tuple) (pro
}

type nonCovStrictSecondaryLookupGen struct {
pri prolly.Map
sec prolly.Map
sch schema.Schema
k, v val.Tuple
pkMap val.OrdinalMapping
pkBld *val.TupleBuilder
pri prolly.Map
sec prolly.Map
prefixDesc val.TupleDesc
sch schema.Schema
pkMap val.OrdinalMapping
pkBld *val.TupleBuilder
}

func (c *nonCovStrictSecondaryLookupGen) InputKeyDesc() val.TupleDesc {
Expand All @@ -124,20 +124,23 @@ func (c *nonCovStrictSecondaryLookupGen) NodeStore() tree.NodeStore {

func (c *nonCovStrictSecondaryLookupGen) New(ctx context.Context, k val.Tuple) (prolly.MapIter, error) {
var idxKey val.Tuple
if err := c.sec.Get(ctx, k, func(key val.Tuple, value val.Tuple) error {
if err := c.sec.GetPrefix(ctx, k, c.prefixDesc, func(key val.Tuple, value val.Tuple) error {
idxKey = key
return nil
}); err != nil {
return nil, err
}
if idxKey == nil {
return &strictLookupIter{}, nil
}
for to := range c.pkMap {
from := c.pkMap.MapOrdinal(to)
c.pkBld.PutRaw(to, idxKey.GetField(from))
}
pk := c.pkBld.Build(sharePool)

iter := &strictLookupIter{k: pk}
if err := c.pri.Get(ctx, c.k, func(key val.Tuple, value val.Tuple) error {
if err := c.pri.Get(ctx, pk, func(key val.Tuple, value val.Tuple) error {
iter.v = value
return nil
}); err != nil {
Expand All @@ -150,7 +153,6 @@ type covLaxSecondaryLookupGen struct {
m prolly.Map
index *doltIndex
prefixDesc val.TupleDesc
k, v val.Tuple
nullSafe []bool
}

Expand Down Expand Up @@ -182,7 +184,7 @@ func (c *covLaxSecondaryLookupGen) New(ctx context.Context, k val.Tuple) (prolly
}

var err error
if c.prefixDesc.Count() == c.m.KeyDesc().Count() {
if c.prefixDesc.Count() >= c.m.KeyDesc().Count()-1 {
// key range optimization only works for full length key
start := k
stop, ok := prolly.IncrementTuple(start, c.prefixDesc.Count()-1, c.prefixDesc, c.m.Pool())
Expand All @@ -204,7 +206,6 @@ type nonCovLaxSecondaryLookupGen struct {
pri prolly.Map
sec prolly.Map
sch schema.Schema
k, v val.Tuple
prefixDesc val.TupleDesc
pkMap val.OrdinalMapping
pkBld *val.TupleBuilder
Expand Down Expand Up @@ -297,7 +298,7 @@ func (c *keylessSecondaryLookupGen) New(ctx context.Context, k val.Tuple) (proll
var err error
if c.prefixDesc.Count() == c.sec.KeyDesc().Count() {
// key range optimization only works if full key
// keyless indexs should include all rows
// keyless indexes should include all rows
start := k
stop, ok := prolly.IncrementTuple(start, c.prefixDesc.Count()-1, c.prefixDesc, c.sec.Pool())
if ok {
Expand Down
2 changes: 1 addition & 1 deletion ...ibraries/doltcore/sqle/rowexec/builder.go → go/libraries/doltcore/sqle/kvexec/builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

package rowexec
package kvexec

import (
"context"
Expand Down
2 changes: 1 addition & 1 deletion ...ries/doltcore/sqle/rowexec/lookup_join.go → ...aries/doltcore/sqle/kvexec/lookup_join.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

package rowexec
package kvexec

import (
"context"
Expand Down
2 changes: 1 addition & 1 deletion ...doltcore/sqle/rowexec/lookup_join_test.go → .../doltcore/sqle/kvexec/lookup_join_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

package rowexec
package kvexec

import (
"context"
Expand Down

0 comments on commit c563747

Please sign in to comment.