Merge pull request #6893 from dolthub/fulghum/dolt-6891

Bug fix: for `dolt_history_` system tables

go/libraries/doltcore/sqle/enginetest/dolt_queries.go

@@ -1962,6 +1962,13 @@ var HistorySystemTableScriptTests = []queries.ScriptTest{
 				Query:    "select pk, c2 from dolt_history_t where commit_hash=@Commit2 order by pk;",
 				Expected: []sql.Row{{1, 3}, {4, 6}},
 			},
+			{
+				// When filtering on a column from the original table, we use the primary index here, but because
+				// column tags have changed in previous versions of the table, the index tags don't match up completely.
+				// https://github.com/dolthub/dolt/issues/6891
+				Query:    "select pk, c1, c2 from dolt_history_t where pk=4;",
+				Expected: []sql.Row{{4, 5, 6}},
+			},
 		},
 	},
 	{

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

@@ -281,8 +281,13 @@ func NewLookupBuilder(
 		}, nil
 	case idx.coversColumns(s, projections):
 		return newCoveringLookupBuilder(base), nil
+	case idx.ID() == "PRIMARY":
+		// If we are using the primary index, always use a covering lookup builder. In some cases, coversColumns
+		// can return false, for example if a column was modified in an older version and has a different tag than
+		// the current schema. In those cases, the primary index is still the best we have, so go ahead and use it.
+		return newCoveringLookupBuilder(base), nil
 	default:
-		return newNonCoveringLookupBuilder(s, base), nil
+		return newNonCoveringLookupBuilder(s, base)
 	}
 }
 
@@ -301,7 +306,17 @@ func newCoveringLookupBuilder(b *baseLookupBuilder) *coveringLookupBuilder {
 	}
 }
 
-func newNonCoveringLookupBuilder(s *durableIndexState, b *baseLookupBuilder) *nonCoveringLookupBuilder {
+// newNonCoveringLookupBuilder returns a LookupBuilder that uses the specified index state and
+// base lookup builder to create a nonCoveringLookupBuilder that uses the secondary index (from
+// |b|) to find the PK row identifier, and then uses that PK to look up the complete row from
+// the primary index (from |s|). If a baseLookupBuilder built on the primary index is passed in,
+// this function returns an error.
+func newNonCoveringLookupBuilder(s *durableIndexState, b *baseLookupBuilder) (*nonCoveringLookupBuilder, error) {
+	if b.idx.ID() == "PRIMARY" {
+		return nil, fmt.Errorf("incompatible index passed to newNonCoveringLookupBuilder: " +
+			"primary index passed, but only secondary indexes are supported")
+	}
+
 	primary := durable.ProllyMapFromIndex(s.Primary)
 	priKd, _ := primary.Descriptors()
 	tbBld := val.NewTupleBuilder(priKd)
@@ -316,7 +331,7 @@ func newNonCoveringLookupBuilder(s *durableIndexState, b *baseLookupBuilder) *no
 		keyMap:            keyProj,
 		valMap:            valProj,
 		ordMap:            ordProj,
-	}
+	}, nil
 }
 
 var _ LookupBuilder = (*baseLookupBuilder)(nil)
@@ -411,7 +426,7 @@ func (lb *coveringLookupBuilder) NewRowIter(ctx *sql.Context, part sql.Partition
 
 // nonCoveringLookupBuilder constructs row iters for non-covering lookups,
 // where we need to seek on the secondary table for key identity, and then
-// the primary table to fill all requrested projections.
+// the primary table to fill all requested projections.
 type nonCoveringLookupBuilder struct {
 	*baseLookupBuilder