Merge pull request #1102 from dolthub/andy/table-refactor

go/libraries/doltcore/{doltdb,table}: remove row access methods from doltdb.Table

This refactor is in preparation for keyless tables. Keyless tables have different row storage semantics than keyed tables, and row access must account for this.

go/cmd/dolt/commands/blame.go

@@ -20,7 +20,7 @@ import (
 	"strings"
 	"time"
 
-	"github.com/jedib0t/go-pretty/table"
+	pretty "github.com/jedib0t/go-pretty/table"
 
 	"github.com/dolthub/dolt/go/cmd/dolt/cli"
 	eventsapi "github.com/dolthub/dolt/go/gen/proto/dolt/services/eventsapi/v1alpha1"
@@ -29,6 +29,7 @@ import (
 	"github.com/dolthub/dolt/go/libraries/doltcore/env/actions/commitwalk"
 	"github.com/dolthub/dolt/go/libraries/doltcore/row"
 	"github.com/dolthub/dolt/go/libraries/doltcore/schema"
+	"github.com/dolthub/dolt/go/libraries/doltcore/table"
 	"github.com/dolthub/dolt/go/libraries/utils/argparser"
 	"github.com/dolthub/dolt/go/libraries/utils/filesys"
 	"github.com/dolthub/dolt/go/store/hash"
@@ -365,20 +366,20 @@ func maybeTableFromCommit(ctx context.Context, c *doltdb.Commit, tableName strin
 
 // maybeRowFromTable takes a table and a primary key and returns a (possibly nil) pointer to a row
 func maybeRowFromTable(ctx context.Context, t *doltdb.Table, rowPK types.Value) (*row.Row, error) {
-	schema, err := t.GetSchema(ctx)
+	sch, err := t.GetSchema(ctx)
 	if err != nil {
 		return nil, fmt.Errorf("error getting schema from table: %v", err)
 	}
 
-	row, ok, err := t.GetRow(ctx, rowPK.(types.Tuple), schema)
+	r, ok, err := table.GetRow(ctx, t, sch, rowPK.(types.Tuple))
 	if err != nil {
 		return nil, fmt.Errorf("error getting row from table: %v", err)
 	}
 	if !ok {
 		return nil, nil
 	}
 
-	return &row, err
+	return &r, err
 }
 
 func schemaFromCommit(ctx context.Context, c *doltdb.Commit, tableName string) (schema.Schema, error) {
@@ -544,7 +545,7 @@ func (bg *blameGraph) String(ctx context.Context, pkColNames []string) string {
 		header = append(header, cellText)
 	}
 
-	t := table.NewWriter()
+	t := pretty.NewWriter()
 	t.AppendHeader(header)
 	for _, v := range *bg {
 		pkVals := getPKStrs(ctx, v.Key)

go/cmd/dolt/commands/sql_test.go

@@ -23,6 +23,7 @@ import (
 
 	"github.com/dolthub/dolt/go/libraries/doltcore/dtestutils"
 	"github.com/dolthub/dolt/go/libraries/doltcore/row"
+	"github.com/dolthub/dolt/go/libraries/doltcore/table"
 	"github.com/dolthub/dolt/go/store/types"
 )
 
@@ -367,27 +368,28 @@ func TestInsert(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
+			ctx := context.Background()
 			dEnv := dtestutils.CreateEnvWithSeedData(t)
 
 			args := []string{"-q", test.query}
 
 			commandStr := "dolt sql"
-			result := SqlCmd{}.Exec(context.TODO(), commandStr, args, dEnv)
+			result := SqlCmd{}.Exec(ctx, commandStr, args, dEnv)
 			assert.Equal(t, test.expectedRes, result)
 
 			if result == 0 {
-				root, err := dEnv.WorkingRoot(context.Background())
+				root, err := dEnv.WorkingRoot(ctx)
 				assert.Nil(t, err)
 
 				// Assert that all expected IDs exist after the insert
 				for _, expectedid := range test.expectedIds {
-					table, _, err := root.GetTable(context.Background(), tableName)
+					tbl, _, err := root.GetTable(ctx, tableName)
 					assert.NoError(t, err)
 					taggedVals := row.TaggedValues{dtestutils.IdTag: types.UUID(expectedid)}
 					key := taggedVals.NomsTupleForPKCols(types.Format_7_18, dtestutils.TypedSchema.GetPKCols())
-					kv, err := key.Value(context.Background())
+					kv, err := key.Value(ctx)
 					assert.NoError(t, err)
-					_, ok, err := table.GetRow(context.Background(), kv.(types.Tuple), dtestutils.TypedSchema)
+					_, ok, err := table.GetRow(ctx, tbl, dtestutils.TypedSchema, kv.(types.Tuple))
 					assert.NoError(t, err)
 					assert.True(t, ok, "expected id not found")
 				}
@@ -456,22 +458,22 @@ func TestUpdate(t *testing.T) {
 			args := []string{"-q", test.query}
 
 			commandStr := "dolt sql"
-			result := SqlCmd{}.Exec(context.TODO(), commandStr, args, dEnv)
+			result := SqlCmd{}.Exec(ctx, commandStr, args, dEnv)
 			assert.Equal(t, test.expectedRes, result)
 
 			if result == 0 {
-				root, err := dEnv.WorkingRoot(context.Background())
+				root, err := dEnv.WorkingRoot(ctx)
 				assert.Nil(t, err)
 
 				// Assert that all rows have been updated
 				for i, expectedid := range test.expectedIds {
-					table, _, err := root.GetTable(context.Background(), tableName)
+					tbl, _, err := root.GetTable(ctx, tableName)
 					assert.NoError(t, err)
 					taggedVals := row.TaggedValues{dtestutils.IdTag: types.UUID(expectedid)}
 					key := taggedVals.NomsTupleForPKCols(types.Format_7_18, dtestutils.TypedSchema.GetPKCols())
 					kv, err := key.Value(ctx)
 					assert.NoError(t, err)
-					row, ok, err := table.GetRow(ctx, kv.(types.Tuple), dtestutils.TypedSchema)
+					row, ok, err := table.GetRow(ctx, tbl, dtestutils.TypedSchema, kv.(types.Tuple))
 					assert.NoError(t, err)
 					assert.True(t, ok, "expected id not found")
 					ageVal, _ := row.GetColVal(dtestutils.AgeTag)
@@ -535,22 +537,22 @@ func TestDelete(t *testing.T) {
 			args := []string{"-q", test.query}
 
 			commandStr := "dolt sql"
-			result := SqlCmd{}.Exec(context.TODO(), commandStr, args, dEnv)
+			result := SqlCmd{}.Exec(ctx, commandStr, args, dEnv)
 			assert.Equal(t, test.expectedRes, result)
 
 			if result == 0 {
-				root, err := dEnv.WorkingRoot(context.Background())
+				root, err := dEnv.WorkingRoot(ctx)
 				assert.Nil(t, err)
 
 				// Assert that all rows have been deleted
 				for _, expectedid := range test.deletedIds {
-					table, _, err := root.GetTable(context.Background(), tableName)
+					tbl, _, err := root.GetTable(ctx, tableName)
 					assert.NoError(t, err)
 					taggedVals := row.TaggedValues{dtestutils.IdTag: types.UUID(expectedid)}
 					key := taggedVals.NomsTupleForPKCols(types.Format_7_18, dtestutils.TypedSchema.GetPKCols())
 					kv, err := key.Value(ctx)
 					assert.NoError(t, err)
-					_, ok, err := table.GetRow(ctx, kv.(types.Tuple), dtestutils.TypedSchema)
+					_, ok, err := table.GetRow(ctx, tbl, dtestutils.TypedSchema, kv.(types.Tuple))
 					assert.NoError(t, err)
 					assert.False(t, ok, "row not deleted")
 				}

go/libraries/doltcore/doltdb/dolt_docs.go

@@ -0,0 +1,43 @@
+// Copyright 2020 Dolthub, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package doltdb
+
+import (
+	"context"
+
+	"github.com/dolthub/dolt/go/libraries/doltcore/row"
+	"github.com/dolthub/dolt/go/libraries/doltcore/schema"
+	"github.com/dolthub/dolt/go/store/types"
+)
+
+func DocTblKeyFromName(fmt *types.NomsBinFormat, name string) (types.Tuple, error) {
+	return types.NewTuple(fmt, types.Uint(DocNameTag), types.String(name))
+}
+
+func getDocRow(ctx context.Context, docTbl *Table, sch schema.Schema, key types.Tuple) (r row.Row, ok bool, err error) {
+	rowMap, err := docTbl.GetRowData(ctx)
+	if err != nil {
+		return nil, false, err
+	}
+
+	var fields types.Value
+	fields, ok, err = rowMap.MaybeGet(ctx, key)
+	if err != nil || !ok {
+		return nil, ok, err
+	}
+
+	r, err = row.FromNoms(sch, key, fields.(types.Tuple))
+	return
+}

go/libraries/doltcore/doltdb/foreign_key_coll.go

@@ -19,14 +19,10 @@ import (
 	"context"
 	"encoding/binary"
 	"fmt"
-	"io"
 	"sort"
 	"strings"
 
-	"github.com/dolthub/dolt/go/libraries/doltcore/row"
-	"github.com/dolthub/dolt/go/libraries/doltcore/rowconv"
 	"github.com/dolthub/dolt/go/libraries/doltcore/schema"
-	"github.com/dolthub/dolt/go/libraries/doltcore/table/typed/noms"
 	"github.com/dolthub/dolt/go/libraries/utils/set"
 	"github.com/dolthub/dolt/go/store/hash"
 	"github.com/dolthub/dolt/go/store/marshal"
@@ -111,77 +107,6 @@ func (fk ForeignKey) HashOf() hash.Hash {
 	return hash.Of(bb.Bytes())
 }
 
-// ConstraintIsSatisfied ensures that the foreign key is valid by comparing the index data from the given table against the index
-// data from the referenced table.
-func (fk ForeignKey) ConstraintIsSatisfied(ctx context.Context, childIdx, parentIdx types.Map, childDef, parentDef schema.Index) error {
-	if fk.ReferencedTableIndex != parentDef.Name() {
-		return fmt.Errorf("cannot validate data as wrong referenced index was given: expected `%s` but received `%s`",
-			fk.ReferencedTableIndex, parentDef.Name())
-	}
-
-	tagMap := make(map[uint64]uint64, len(fk.TableColumns))
-	for i, childTag := range fk.TableColumns {
-		tagMap[childTag] = fk.ReferencedTableColumns[i]
-	}
-
-	// FieldMappings ignore columns not in the tagMap
-	fm, err := rowconv.NewFieldMapping(childDef.Schema(), parentDef.Schema(), tagMap)
-	if err != nil {
-		return err
-	}
-
-	rc, err := rowconv.NewRowConverter(fm)
-	if err != nil {
-		return err
-	}
-
-	rdr, err := noms.NewNomsMapReader(ctx, childIdx, childDef.Schema())
-	if err != nil {
-		return err
-	}
-
-	for {
-		childIdxRow, err := rdr.ReadRow(ctx)
-		if err == io.EOF {
-			break
-		}
-		if err != nil {
-			return err
-		}
-
-		parentIdxRow, err := rc.Convert(childIdxRow)
-		if err != nil {
-			return err
-		}
-		if row.IsEmpty(parentIdxRow) {
-			continue
-		}
-
-		partial, err := parentIdxRow.ReduceToIndexPartialKey(parentDef)
-		if err != nil {
-			return err
-		}
-
-		indexIter := noms.NewNomsRangeReader(parentDef.Schema(), parentIdx,
-			[]*noms.ReadRange{{Start: partial, Inclusive: true, Reverse: false, Check: func(tuple types.Tuple) (bool, error) {
-				return tuple.StartsWith(partial), nil
-			}}},
-		)
-
-		switch _, err = indexIter.ReadRow(ctx); err {
-		case nil:
-			continue // parent table contains child key
-		case io.EOF:
-			indexKeyStr, _ := types.EncodedValue(ctx, partial)
-			return fmt.Errorf("foreign key violation on `%s`.`%s`: `%s`", fk.Name, fk.TableName, indexKeyStr)
-		default:
-			return err
-		}
-	}
-
-	return nil
-}
-
 // ValidateReferencedTableSchema verifies that the given schema matches the expectation of the referenced table.
 func (fk ForeignKey) ValidateReferencedTableSchema(sch schema.Schema) error {
 	allSchCols := sch.GetAllCols()

go/libraries/doltcore/doltdb/root_val.go

@@ -1366,11 +1366,12 @@ func addValuesToDocs(ctx context.Context, tbl *Table, sch *schema.Schema, docDet
 // AddValueToDocFromTbl updates the Value field of a docDetail using the provided table and schema.
 func AddValueToDocFromTbl(ctx context.Context, tbl *Table, sch *schema.Schema, docDetail DocDetails) (DocDetails, error) {
 	if tbl != nil && sch != nil {
-		pkTaggedVal := row.TaggedValues{
-			DocNameTag: types.String(docDetail.DocPk),
+		key, err := DocTblKeyFromName(tbl.Format(), docDetail.DocPk)
+		if err != nil {
+			return DocDetails{}, err
 		}
 
-		docRow, ok, err := tbl.GetRowByPKVals(ctx, pkTaggedVal, *sch)
+		docRow, ok, err := getDocRow(ctx, tbl, *sch, key)
 		if err != nil {
 			return DocDetails{}, err
 		}
@@ -1390,11 +1391,12 @@ func AddValueToDocFromTbl(ctx context.Context, tbl *Table, sch *schema.Schema, d
 // AddNewerTextToDocFromTbl updates the NewerText field of a docDetail using the provided table and schema.
 func AddNewerTextToDocFromTbl(ctx context.Context, tbl *Table, sch *schema.Schema, doc DocDetails) (DocDetails, error) {
 	if tbl != nil && sch != nil {
-		pkTaggedVal := row.TaggedValues{
-			DocNameTag: types.String(doc.DocPk),
+		key, err := DocTblKeyFromName(tbl.Format(), doc.DocPk)
+		if err != nil {
+			return DocDetails{}, err
 		}
 
-		docRow, ok, err := tbl.GetRowByPKVals(ctx, pkTaggedVal, *sch)
+		docRow, ok, err := getDocRow(ctx, tbl, *sch, key)
 		if err != nil {
 			return DocDetails{}, err
 		}

go/libraries/doltcore/doltdb/root_val_test.go

@@ -136,7 +136,7 @@ func TestDocDiff(t *testing.T) {
 
 	// Create tbl1 with one license row
 	sch := createTestDocsSchema()
-	licRow := getDocRow(t, sch, LicensePk, types.String("license row"))
+	licRow := makeDocRow(t, sch, LicensePk, types.String("license row"))
 	m, _ := createTestRows(t, ddb.ValueReadWriter(), sch, []row.Row{licRow})
 	tbl1, err := createTestTable(ddb.ValueReadWriter(), sch, m)
 	assert.NoError(t, err)
@@ -154,7 +154,7 @@ func TestDocDiff(t *testing.T) {
 	}
 
 	// Create tbl2 with one readme row
-	readmeRow := getDocRow(t, sch, ReadmePk, types.String("readme row"))
+	readmeRow := makeDocRow(t, sch, ReadmePk, types.String("readme row"))
 	m, _ = createTestRows(t, ddb.ValueReadWriter(), sch, []row.Row{readmeRow})
 	tbl2, err := createTestTable(ddb.ValueReadWriter(), sch, m)
 	assert.NoError(t, err)
@@ -172,7 +172,7 @@ func TestDocDiff(t *testing.T) {
 	}
 
 	// Create tbl3 with 2 doc rows (readme, license)
-	readmeRowUpdated := getDocRow(t, sch, ReadmePk, types.String("a different readme"))
+	readmeRowUpdated := makeDocRow(t, sch, ReadmePk, types.String("a different readme"))
 	m, _ = createTestRows(t, ddb.ValueReadWriter(), sch, []row.Row{readmeRowUpdated, licRow})
 	tbl3, err := createTestTable(ddb.ValueReadWriter(), sch, m)
 	assert.NoError(t, err)
@@ -319,15 +319,15 @@ func createTestDocsSchema() schema.Schema {
 
 func getDocRows(t *testing.T, sch schema.Schema, rowVal types.Value) []row.Row {
 	rows := make([]row.Row, 2)
-	row1 := getDocRow(t, sch, LicensePk, rowVal)
+	row1 := makeDocRow(t, sch, LicensePk, rowVal)
 	rows[0] = row1
-	row2 := getDocRow(t, sch, ReadmePk, rowVal)
+	row2 := makeDocRow(t, sch, ReadmePk, rowVal)
 	rows[1] = row2
 
 	return rows
 }
 
-func getDocRow(t *testing.T, sch schema.Schema, pk string, rowVal types.Value) row.Row {
+func makeDocRow(t *testing.T, sch schema.Schema, pk string, rowVal types.Value) row.Row {
 	row, err := row.New(types.Format_7_18, sch, row.TaggedValues{
 		DocNameTag: types.String(pk),
 		DocTextTag: rowVal,