Merge pull request #5868 from dolthub/james/index

go through primary keys backwards

go/go.mod

@@ -59,7 +59,7 @@ require (
 	github.com/cespare/xxhash v1.1.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.15.1-0.20230508230804-c4ccc1ffdd14
+	github.com/dolthub/go-mysql-server v0.15.1-0.20230509182237-d52f00655399
 	github.com/dolthub/swiss v0.1.0
 	github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
 	github.com/jmoiron/sqlx v1.3.4

go/go.sum

@@ -166,8 +166,8 @@ github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2 h1:u3PMzfF8RkKd3lB9pZ2bfn0qEG+1G
 github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2/go.mod h1:mIEZOHnFx4ZMQeawhw9rhsj+0zwQj7adVsnBX7t+eKY=
 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-mysql-server v0.15.1-0.20230508230804-c4ccc1ffdd14 h1:SlYjx7X3ojqNf92RrNhim8LO+A5Ns2nZmjcbrvcVRpU=
-github.com/dolthub/go-mysql-server v0.15.1-0.20230508230804-c4ccc1ffdd14/go.mod h1:YO4FtMULZ/HuKxlvm7QfvTE8uBKEv+1LtK2A3FrrGe4=
+github.com/dolthub/go-mysql-server v0.15.1-0.20230509182237-d52f00655399 h1:0m0huPD01Ax41IT9OmKX1YuZucHaWaHqgVm6gN4Q/Ak=
+github.com/dolthub/go-mysql-server v0.15.1-0.20230509182237-d52f00655399/go.mod h1:YO4FtMULZ/HuKxlvm7QfvTE8uBKEv+1LtK2A3FrrGe4=
 github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488 h1:0HHu0GWJH0N6a6keStrHhUAK5/o9LVfkh44pvsV4514=
 github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488/go.mod h1:ehexgi1mPxRTk0Mok/pADALuHbvATulTh6gzr7NzZto=
 github.com/dolthub/jsonpath v0.0.1 h1:Nd+T3U+XisK3kOuxtABS5IIbZqXVIlOR9VYquyjQ0u0=

go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go

@@ -248,12 +248,32 @@ func TestQueryPlans(t *testing.T) {
 	// Parallelism introduces Exchange nodes into the query plans, so disable.
 	// TODO: exchange nodes should really only be part of the explain plan under certain debug settings
 	harness := newDoltHarness(t).WithParallelism(1).WithSkippedQueries(skipped)
+	if !types.IsFormat_DOLT(types.Format_Default) {
+		// only new format supports reverse IndexTableAccess
+		reverseIndexSkip := []string{
+			"SELECT * FROM one_pk ORDER BY pk",
+			"SELECT * FROM two_pk ORDER BY pk1, pk2",
+			"SELECT * FROM two_pk ORDER BY pk1",
+			"SELECT pk1 AS one, pk2 AS two FROM two_pk ORDER BY pk1, pk2",
+			"SELECT pk1 AS one, pk2 AS two FROM two_pk ORDER BY one, two",
+			"SELECT i FROM (SELECT i FROM mytable ORDER BY i DESC LIMIT 1) sq WHERE i = 3",
+			"SELECT i FROM (SELECT i FROM (SELECT i FROM mytable ORDER BY DES LIMIT 1) sql1)sql2 WHERE i = 3",
+			"SELECT s,i FROM mytable order by i DESC",
+			"SELECT s,i FROM mytable as a order by i DESC",
+			"SELECT pk1, pk2 FROM two_pk order by pk1 asc, pk2 asc",
+			"SELECT pk1, pk2 FROM two_pk order by pk1 desc, pk2 desc",
+			"SELECT i FROM (SELECT i FROM (SELECT i FROM mytable ORDER BY i DESC  LIMIT 1) sq1) sq2 WHERE i = 3",
+		}
+		harness = harness.WithSkippedQueries(reverseIndexSkip)
+	}
+
 	defer harness.Close()
 	enginetest.TestQueryPlans(t, harness, queries.PlanTests)
 }
 
 func TestIntegrationQueryPlans(t *testing.T) {
 	harness := newDoltHarness(t).WithParallelism(1)
+
 	defer harness.Close()
 	enginetest.TestIntegrationPlans(t, harness)
 }

go/libraries/doltcore/sqle/index/dolt_index.go

@@ -773,6 +773,10 @@ func (di *doltIndex) Order() sql.IndexOrder {
 	return di.order
 }
 
+func (di *doltIndex) Reversible() bool {
+	return di.doltBinFormat
+}
+
 // Database implement sql.Index
 func (di *doltIndex) Database() string {
 	return di.dbName

go/libraries/doltcore/sqle/index/index_lookup.go

@@ -117,6 +117,7 @@ func NewRangePartitionIter(ctx *sql.Context, t DoltTableable, lookup sql.IndexLo
 		prollyRanges: prollyRanges,
 		curr:         0,
 		isDoltFmt:    isDoltFmt,
+		isReverse:    lookup.IsReverse,
 	}, nil
 }
 
@@ -166,6 +167,7 @@ type rangePartitionIter struct {
 	prollyRanges []prolly.Range
 	curr         int
 	isDoltFmt    bool
+	isReverse    bool
 }
 
 // Close is required by the sql.PartitionIter interface. Does nothing.
@@ -194,6 +196,7 @@ func (itr *rangePartitionIter) nextProllyPartition() (sql.Partition, error) {
 	return rangePartition{
 		prollyRange: pr,
 		key:         bytes[:],
+		isReverse:   itr.isReverse,
 	}, nil
 }
 
@@ -210,6 +213,7 @@ func (itr *rangePartitionIter) nextNomsPartition() (sql.Partition, error) {
 	return rangePartition{
 		nomsRange: nr,
 		key:       bytes[:],
+		isReverse: itr.isReverse,
 	}, nil
 }
 
@@ -217,6 +221,7 @@ type rangePartition struct {
 	nomsRange   *noms.ReadRange
 	prollyRange prolly.Range
 	key         []byte
+	isReverse   bool
 }
 
 func (rp rangePartition) Key() []byte {
@@ -364,7 +369,11 @@ func (lb *baseLookupBuilder) rangeIter(ctx *sql.Context, part sql.Partition) (pr
 	case pointPartition:
 		return lb.newPointLookup(ctx, p.r)
 	case rangePartition:
-		return lb.sec.IterRange(ctx, p.prollyRange)
+		if p.isReverse {
+			return lb.sec.IterRangeReverse(ctx, p.prollyRange)
+		} else {
+			return lb.sec.IterRange(ctx, p.prollyRange)
+		}
 	default:
 		panic(fmt.Sprintf("unexpected prolly partition type: %T", part))
 	}

go/store/prolly/tree/map.go

@@ -491,6 +491,35 @@ type OrderedTreeIter[K, V ~[]byte] struct {
 	stop func(*Cursor) bool
 }
 
+func ReverseOrderedTreeIterFromCursors[K, V ~[]byte](
+	ctx context.Context,
+	root Node, ns NodeStore,
+	findStart, findEnd SearchFn,
+) (*OrderedTreeIter[K, V], error) {
+	start, err := newCursorFromSearchFn(ctx, ns, root, findStart)
+	if err != nil {
+		return nil, err
+	}
+	end, err := newCursorFromSearchFn(ctx, ns, root, findEnd)
+	if err != nil {
+		return nil, err
+	}
+	err = end.retreat(ctx)
+	if err != nil {
+		return nil, err
+	}
+
+	stopFn := func(curr *Cursor) bool {
+		return curr.compare(start) < 0
+	}
+
+	if stopFn(end) {
+		end = nil // empty range
+	}
+
+	return &OrderedTreeIter[K, V]{curr: end, stop: stopFn, step: end.retreat}, nil
+}
+
 func OrderedTreeIterFromCursors[K, V ~[]byte](
 	ctx context.Context,
 	root Node, ns NodeStore,

go/store/prolly/tuple_map.go

@@ -305,6 +305,15 @@ func (m Map) IterRange(ctx context.Context, rng Range) (MapIter, error) {
 	return filteredIter{iter: iter, rng: rng}, nil
 }
 
+// IterRangeReverse returns a mutableMapIter that iterates over a Range backwards.
+func (m Map) IterRangeReverse(ctx context.Context, rng Range) (MapIter, error) {
+	iter, err := treeIterFromRangeReverse(ctx, m.tuples.Root, m.tuples.NodeStore, rng)
+	if err != nil {
+		return nil, err
+	}
+	return filteredIter{iter: iter, rng: rng}, nil
+}
+
 // IterKeyRange iterates over a physical key range defined by |start| and
 // |stop|. If |startInclusive| and/or |stop| is nil, the range will be open
 // towards that end.
@@ -347,6 +356,16 @@ func treeIterFromRange(
 	return tree.OrderedTreeIterFromCursors[val.Tuple, val.Tuple](ctx, root, ns, findStart, findStop)
 }
 
+func treeIterFromRangeReverse(
+	ctx context.Context,
+	root tree.Node,
+	ns tree.NodeStore,
+	rng Range,
+) (*tree.OrderedTreeIter[val.Tuple, val.Tuple], error) {
+	findStart, findStop := rangeStartSearchFn(rng), rangeStopSearchFn(rng)
+	return tree.ReverseOrderedTreeIterFromCursors[val.Tuple, val.Tuple](ctx, root, ns, findStart, findStop)
+}
+
 func NewPointLookup(k, v val.Tuple) *pointLookup {
 	return &pointLookup{k, v}
 }

integration-tests/bats/import-update-tables.bats

@@ -1360,7 +1360,7 @@ DELIM
     [ $status -eq 0 ]
     [[ "$output" =~ "Rows Processed: 4, Additions: 4, Modifications: 0, Had No Effect: 0" ]] || false
 
-    run dolt sql -r csv -q "select * from word;"
+    run dolt sql -r csv -q "select * from word order by pk;"
     [ $status -eq 0 ]
     [[ "$output" = "$expected" ]] || false
 }