*: iterator should visit trees recursively after main tree

There was an inconsistency between the results returned by the iterators and the ones using the tables and joins because the tables look for all trees and visit them. Instead, the iterator chained with commits only visited the main tree. Now, there is another iterator for this case that visits the main tree and puts other trees that are found in a queue to visit them as well. This mimics the behaviour of joinin commits and tree entries.

Signed-off-by: Miguel Molina <[email protected]>

Author: erizocosmico

internal/rule/squashjoins.go

@@ -277,6 +277,7 @@ func buildSquashedTable(
 					false,
 				)
 			case gitbase.CommitsIter:
+				onlyMainTree := hasMainTreeFilter(filters)
 				var f sql.Expression
 				f, filters, err = filtersForJoin(
 					gitbase.CommitsTableName,
@@ -288,7 +289,11 @@ func buildSquashedTable(
 					return nil, err
 				}
 
-				iter = gitbase.NewCommitTreeEntriesIter(it, f, false)
+				if onlyMainTree {
+					iter = gitbase.NewCommitMainTreeEntriesIter(it, f, false)
+				} else {
+					iter = gitbase.NewCommitTreeEntriesIter(it, f, false)
+				}
 			case nil:
 				var f sql.Expression
 				f, filters, err = filtersForTable(
@@ -342,7 +347,7 @@ func buildSquashedTable(
 				}
 
 				iter = gitbase.NewTreeEntryBlobsIter(
-					gitbase.NewCommitTreeEntriesIter(
+					gitbase.NewCommitMainTreeEntriesIter(
 						it,
 						nil,
 						true,
@@ -745,6 +750,19 @@ func hasRefHEADFilter(filters []sql.Expression) bool {
 	return false
 }
 
+func hasMainTreeFilter(filters []sql.Expression) bool {
+	for _, f := range filters {
+		ok := isEq(
+			isCol(gitbase.CommitsTableName, "tree_hash"),
+			isCol(gitbase.TreeEntriesTableName, "tree_hash"),
+		)(f)
+		if ok {
+			return true
+		}
+	}
+	return false
+}
+
 // hasChainableJoinCondition tells whether the given join condition contains
 // any filter that can be used to build a chainable iterator.
 // t1 with t2. t1 MUST be higher in the table hierarchy than t2.

internal/rule/squashjoins_test.go

@@ -330,7 +330,12 @@ func TestBuildSquashedTable(t *testing.T) {
 		col(0, gitbase.TreeEntriesTableName, "entry_hash"),
 	)
 
-	commitTreeEntriesRedundantFilter := eq(
+	commitTreeEntriesRedundantFilter := commitHasTree(
+		col(0, gitbase.CommitsTableName, "hash"),
+		col(0, gitbase.TreeEntriesTableName, "tree_hash"),
+	)
+
+	commitMainTreeEntriesRedundantFilter := eq(
 		col(0, gitbase.CommitsTableName, "tree_hash"),
 		col(0, gitbase.TreeEntriesTableName, "tree_hash"),
 	)
@@ -523,6 +528,32 @@ func TestBuildSquashedTable(t *testing.T) {
 		{
 			"commits with tree entries",
 			[]sql.Table{commits, treeEntries},
+			[]sql.Expression{
+				commitFilter,
+				treeEntryFilter,
+				commitTreeEntriesFilter,
+				commitMainTreeEntriesRedundantFilter,
+			},
+			nil,
+			newSquashedTable(
+				gitbase.NewCommitMainTreeEntriesIter(
+					gitbase.NewAllCommitsIter(
+						fixIdx(t, commitFilter, gitbase.CommitsSchema),
+					),
+					and(
+						fixIdx(t, treeEntryFilter, commitTreeEntriesSchema),
+						fixIdx(t, commitTreeEntriesFilter, commitTreeEntriesSchema),
+					),
+					false,
+				),
+				nil,
+				gitbase.CommitsTableName,
+				gitbase.TreeEntriesTableName,
+			),
+		},
+		{
+			"commits with tree entries using commit_has_tree",
+			[]sql.Table{commits, treeEntries},
 			[]sql.Expression{
 				commitFilter,
 				treeEntryFilter,
@@ -674,7 +705,7 @@ func TestBuildSquashedTable(t *testing.T) {
 			nil,
 			newSquashedTable(
 				gitbase.NewTreeEntryBlobsIter(
-					gitbase.NewCommitTreeEntriesIter(
+					gitbase.NewCommitMainTreeEntriesIter(
 						gitbase.NewAllCommitsIter(
 							fixIdx(t, commitFilter, commitBlobsSchema),
 						),