Merge pull request #4691 from dolthub/taylor/three-dot-log

Support three dot log
2026-02-12 02:58:53 -06:00 · 2022-11-07 10:29:05 -08:00
parent 20c0ae3010 a9289c0c4e
commit 9423991672
4 changed files with 195 additions and 52 deletions
--- a/go/cmd/dolt/commands/log.go
+++ b/go/cmd/dolt/commands/log.go
@@ -72,7 +72,11 @@ The command takes options to control what is shown and how.
 {{.EmphasisLeft}}dolt log <revisionB>..<revisionA>{{.EmphasisRight}}
 {{.EmphasisLeft}}dolt log <revisionA> --not <revisionB>{{.EmphasisRight}}
 {{.EmphasisLeft}}dolt log ^<revisionB> <revisionA>{{.EmphasisRight}}
-  Different ways to list two dot logs. These will list commit logs for revisionA, while excluding commits from revisionB. The table option is not supported for two dot log.`,
+  Different ways to list two dot logs. These will list commit logs for revisionA, while excluding commits from revisionB. The table option is not supported for two dot log.
+	
+{{.EmphasisLeft}}dolt log <revisionB>...<revisionA>{{.EmphasisRight}}
+{{.EmphasisLeft}}dolt log <revisionA> <revisionB> --not $(dolt merge-base <revisionA> <revisionB>){{.EmphasisRight}}
+  Different ways to list three dot logs. These will list commit logs reachable by revisionA OR revisionB, while excluding commits reachable by BOTH revisionA AND revisionB.`,
 	Synopsis: []string{
 		`[-n {{.LessThan}}num_commits{{.GreaterThan}}] [{{.LessThan}}revision-range{{.GreaterThan}}] [[--] {{.LessThan}}table{{.GreaterThan}}]`,
 	},
@@ -180,14 +184,37 @@ func (opts *logOpts) parseRefsAndTable(ctx context.Context, apr *argparser.ArgPa
 		return nil
 	}

-	// `dolt log <ref>..<ref>`
 	if strings.Contains(apr.Arg(0), "..") {
-		if strings.Contains(apr.Arg(0), "...") {
-			return fmt.Errorf("three dot log not yet supported")
-		}
 		if apr.NArg() > 1 {
-			return fmt.Errorf("Cannot use two dot when 2 or more arguments provided")
+			return fmt.Errorf("Cannot use two or three dot syntax when 2 or more arguments provided")
 		}
+
+		// `dolt log <ref>...<ref>`
+		if strings.Contains(apr.Arg(0), "...") {
+			refs := strings.Split(apr.Arg(0), "...")
+
+			for _, ref := range refs {
+				cs, err := getCommitSpec(ref)
+				if err != nil {
+					return err
+				}
+				opts.commitSpecs = append(opts.commitSpecs, cs)
+			}
+
+			mergeBase, verr := getMergeBaseFromStrings(ctx, dEnv, refs[0], refs[1])
+			if verr != nil {
+				return verr
+			}
+			notCs, err := getCommitSpec(mergeBase)
+			if err != nil {
+				return err
+			}
+			opts.excludingCommitSpecs = append(opts.excludingCommitSpecs, notCs)
+
+			return nil
+		}
+
+		// `dolt log <ref>..<ref>`
 		refs := strings.Split(apr.Arg(0), "..")
 		notCs, err := getCommitSpec(refs[0])
 		if err != nil {
--- a/go/libraries/doltcore/sqle/dolt_log_table_function.go
+++ b/go/libraries/doltcore/sqle/dolt_log_table_function.go
@@ -24,6 +24,7 @@ import (
 	"github.com/dolthub/dolt/go/cmd/dolt/cli"
 	"github.com/dolthub/dolt/go/libraries/doltcore/doltdb"
 	"github.com/dolthub/dolt/go/libraries/doltcore/env/actions/commitwalk"
+	"github.com/dolthub/dolt/go/libraries/doltcore/merge"
 	"github.com/dolthub/dolt/go/libraries/doltcore/sqle/dsess"
 	"github.com/dolthub/dolt/go/store/hash"
 )
@@ -303,7 +304,7 @@ func (ltf *LogTableFunction) validateRevisionExpressions() error {
 			return ltf.invalidArgDetailsErr(ltf.revisionExpr, "second revision must exist if first revision contains '^'")
 		}
 		if strings.Contains(revisionStr, "..") && strings.HasPrefix(revisionStr, "^") {
-			return ltf.invalidArgDetailsErr(ltf.revisionExpr, "revision cannot contain both '..' and '^'")
+			return ltf.invalidArgDetailsErr(ltf.revisionExpr, "revision cannot contain both '..' or '...' and '^'")
 		}
 	}

@@ -313,10 +314,10 @@ func (ltf *LogTableFunction) validateRevisionExpressions() error {
 			return sql.ErrInvalidArgumentDetails.New(ltf.FunctionName(), ltf.secondRevisionExpr.String())
 		}
 		if strings.Contains(secondRevisionStr, "..") {
-			return ltf.invalidArgDetailsErr(ltf.secondRevisionExpr, "second revision cannot contain '..'")
+			return ltf.invalidArgDetailsErr(ltf.secondRevisionExpr, "second revision cannot contain '..' or '...'")
 		}
 		if strings.Contains(revisionStr, "..") {
-			return ltf.invalidArgDetailsErr(ltf.revisionExpr, "revision cannot contain '..' if second revision exists")
+			return ltf.invalidArgDetailsErr(ltf.revisionExpr, "revision cannot contain '..' or '...' if second revision exists")
 		}
 	}

@@ -334,7 +335,7 @@ func (ltf *LogTableFunction) validateRevisionExpressions() error {
 			return ltf.invalidArgDetailsErr(ltf.revisionExpr, "must have revision in order to use --not")
 		}
 		if ltf.revisionExpr != nil && (strings.Contains(revisionStr, "..") || strings.HasPrefix(revisionStr, "^")) {
-			return ltf.invalidArgDetailsErr(ltf.revisionExpr, "cannot use --not if '..' or '^' present in revision")
+			return ltf.invalidArgDetailsErr(ltf.revisionExpr, "cannot use --not if dots or '^' present in revision")
 		}
 		if ltf.secondRevisionExpr != nil && strings.HasPrefix(secondRevisionStr, "^") {
 			return ltf.invalidArgDetailsErr(ltf.secondRevisionExpr, "cannot use --not if '^' present in second revision")
@@ -352,7 +353,7 @@ func (ltf *LogTableFunction) validateRevisionExpressions() error {

 // RowIter implements the sql.Node interface
 func (ltf *LogTableFunction) RowIter(ctx *sql.Context, row sql.Row) (sql.RowIter, error) {
-	revisionVal, excludingRevisionVal, err := ltf.evaluateArguments()
+	revisionVal, secondRevisionVal, threeDot, err := ltf.evaluateArguments()
 	if err != nil {
 		return nil, err
 	}
@@ -392,18 +393,40 @@ func (ltf *LogTableFunction) RowIter(ctx *sql.Context, row sql.Row) (sql.RowIter
 		return nil, err
 	}

-	// Two dot log
-	if len(excludingRevisionVal) > 0 {
-		exCs, err := doltdb.NewCommitSpec(excludingRevisionVal)
+	// Two and three dot log
+	if len(secondRevisionVal) > 0 {
+		secondCs, err := doltdb.NewCommitSpec(secondRevisionVal)
 		if err != nil {
 			return nil, err
 		}

-		excludingCommit, err := sqledb.ddb.Resolve(ctx, exCs, nil)
+		secondCommit, err := sqledb.ddb.Resolve(ctx, secondCs, nil)
 		if err != nil {
 			return nil, err
 		}
-		return ltf.NewDotDotLogTableFunctionRowIter(ctx, sqledb.ddb, commit, excludingCommit, matchFunc, cHashToRefs)
+
+		if threeDot {
+			mergeBase, err := merge.MergeBase(ctx, commit, secondCommit)
+			if err != nil {
+				return nil, err
+			}
+
+			mergeCs, err := doltdb.NewCommitSpec(mergeBase.String())
+			if err != nil {
+				return nil, err
+			}
+
+			// Use merge base as excluding commit
+			mergeCommit, err := sqledb.ddb.Resolve(ctx, mergeCs, nil)
+			if err != nil {
+				return nil, err
+			}
+
+			return ltf.NewDotDotLogTableFunctionRowIter(ctx, sqledb.ddb, []*doltdb.Commit{commit, secondCommit}, mergeCommit, matchFunc, cHashToRefs)
+		}
+
+		return ltf.NewDotDotLogTableFunctionRowIter(ctx, sqledb.ddb, []*doltdb.Commit{commit}, secondCommit, matchFunc, cHashToRefs)
+
 	}

 	return ltf.NewLogTableFunctionRowIter(ctx, sqledb.ddb, commit, matchFunc, cHashToRefs)
@@ -455,39 +478,40 @@ func getCommitHashToRefs(ctx *sql.Context, ddb *doltdb.DoltDB, decoration string
 	return cHashToRefs, nil
 }

-// evaluateArguments returns revisionValStr and excludingRevisionValStr.
+// evaluateArguments returns revisionValStr, secondRevisionValStr, and three dot boolean.
 // It evaluates the argument expressions to turn them into values this LogTableFunction
 // can use. Note that this method only evals the expressions, and doesn't validate the values.
-func (ltf *LogTableFunction) evaluateArguments() (string, string, error) {
+func (ltf *LogTableFunction) evaluateArguments() (string, string, bool, error) {
 	var revisionValStr string
-	var excludingRevisionValStr string
+	var secondRevisionValStr string
 	var err error
+	threeDot := false

 	if ltf.revisionExpr != nil {
-		revisionValStr, excludingRevisionValStr, err = getRevisionsFromExpr(ltf.ctx, ltf.revisionExpr, true)
+		revisionValStr, secondRevisionValStr, threeDot, err = getRevisionsFromExpr(ltf.ctx, ltf.revisionExpr, true)
 		if err != nil {
-			return "", "", err
+			return "", "", false, err
 		}
 	}

 	if ltf.secondRevisionExpr != nil {
-		rvs, ervs, err := getRevisionsFromExpr(ltf.ctx, ltf.secondRevisionExpr, false)
+		rvs, srvs, _, err := getRevisionsFromExpr(ltf.ctx, ltf.secondRevisionExpr, false)
 		if err != nil {
-			return "", "", err
+			return "", "", false, err
 		}
 		if len(rvs) > 0 {
 			revisionValStr = rvs
 		}
-		if len(ervs) > 0 {
-			excludingRevisionValStr = ervs
+		if len(srvs) > 0 {
+			secondRevisionValStr = srvs
 		}
 	}

 	if len(ltf.notRevision) > 0 {
-		excludingRevisionValStr = ltf.notRevision
+		secondRevisionValStr = ltf.notRevision
 	}

-	return revisionValStr, excludingRevisionValStr, nil
+	return revisionValStr, secondRevisionValStr, threeDot, nil
 }

 func mustExpressionToString(ctx *sql.Context, expr sql.Expression) string {
@@ -512,23 +536,28 @@ func expressionToString(ctx *sql.Context, expr sql.Expression) (string, error) {
 	return valStr, nil
 }

-// Gets revisionName and/or excludingRevisionName from sql expression
-func getRevisionsFromExpr(ctx *sql.Context, expr sql.Expression, canDot bool) (string, string, error) {
+// getRevisionsFromExpr returns the revisionName and/or secondRevisionName, as
+// well as a threeDot boolean from sql expression
+func getRevisionsFromExpr(ctx *sql.Context, expr sql.Expression, canDot bool) (string, string, bool, error) {
 	revisionValStr, err := expressionToString(ctx, expr)
 	if err != nil {
-		return "", "", err
+		return "", "", false, err
 	}

 	if canDot && strings.Contains(revisionValStr, "..") {
+		if strings.Contains(revisionValStr, "...") {
+			refs := strings.Split(revisionValStr, "...")
+			return refs[0], refs[1], true, nil
+		}
 		refs := strings.Split(revisionValStr, "..")
-		return refs[1], refs[0], nil
+		return refs[1], refs[0], false, nil
 	}

 	if strings.HasPrefix(revisionValStr, "^") {
-		return "", strings.TrimPrefix(revisionValStr, "^"), nil
+		return "", strings.TrimPrefix(revisionValStr, "^"), false, nil
 	}

-	return revisionValStr, "", nil
+	return revisionValStr, "", false, nil
 }

 //------------------------------------
@@ -566,10 +595,15 @@ func (ltf *LogTableFunction) NewLogTableFunctionRowIter(ctx *sql.Context, ddb *d
 	}, nil
 }

-func (ltf *LogTableFunction) NewDotDotLogTableFunctionRowIter(ctx *sql.Context, ddb *doltdb.DoltDB, commit, excludingCommit *doltdb.Commit, matchFn func(*doltdb.Commit) (bool, error), cHashToRefs map[hash.Hash][]string) (*logTableFunctionRowIter, error) {
-	h, err := commit.HashOf()
-	if err != nil {
-		return nil, err
+func (ltf *LogTableFunction) NewDotDotLogTableFunctionRowIter(ctx *sql.Context, ddb *doltdb.DoltDB, commits []*doltdb.Commit, excludingCommit *doltdb.Commit, matchFn func(*doltdb.Commit) (bool, error), cHashToRefs map[hash.Hash][]string) (*logTableFunctionRowIter, error) {
+	hashes := make([]hash.Hash, len(commits))
+
+	for i, commit := range commits {
+		h, err := commit.HashOf()
+		if err != nil {
+			return nil, err
+		}
+		hashes[i] = h
 	}

 	exHash, err := excludingCommit.HashOf()
@@ -577,17 +611,23 @@ func (ltf *LogTableFunction) NewDotDotLogTableFunctionRowIter(ctx *sql.Context,
 		return nil, err
 	}

-	child, err := commitwalk.GetDotDotRevisionsIterator(ctx, ddb, []hash.Hash{h}, ddb, []hash.Hash{exHash}, matchFn)
+	child, err := commitwalk.GetDotDotRevisionsIterator(ctx, ddb, hashes, ddb, []hash.Hash{exHash}, matchFn)
 	if err != nil {
 		return nil, err
 	}

+	var headHash hash.Hash
+
+	if len(hashes) == 1 {
+		headHash = hashes[0]
+	}
+
 	return &logTableFunctionRowIter{
 		child:       child,
 		showParents: ltf.showParents,
 		decoration:  ltf.decoration,
 		cHashToRefs: cHashToRefs,
-		headHash:    h,
+		headHash:    headHash,
 	}, nil
 }

--- a/go/libraries/doltcore/sqle/enginetest/dolt_queries.go
+++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries.go
@@ -5198,14 +5198,26 @@ var LogTableFunctionScriptTests = []queries.ScriptTest{
 				Query:       "SELECT * from dolt_log('^main..branch1');",
 				ExpectedErr: sql.ErrInvalidArgumentDetails,
 			},
+			{
+				Query:       "SELECT * from dolt_log('^main...branch1');",
+				ExpectedErr: sql.ErrInvalidArgumentDetails,
+			},
 			{
 				Query:       "SELECT * from dolt_log(@Commit1, 'main..branch1');",
 				ExpectedErr: sql.ErrInvalidArgumentDetails,
 			},
+			{
+				Query:       "SELECT * from dolt_log(@Commit1, 'main...branch1');",
+				ExpectedErr: sql.ErrInvalidArgumentDetails,
+			},
 			{
 				Query:       "SELECT * from dolt_log('main..branch1', '--not', @Commit1);",
 				ExpectedErr: sql.ErrInvalidArgumentDetails,
 			},
+			{
+				Query:       "SELECT * from dolt_log('main...branch1', '--not', @Commit1);",
+				ExpectedErr: sql.ErrInvalidArgumentDetails,
+			},
 			{
 				Query:       "SELECT * from dolt_log('^main', '--not', @Commit1);",
 				ExpectedErr: sql.ErrInvalidArgumentDetails,
@@ -5366,30 +5378,39 @@ var LogTableFunctionScriptTests = []queries.ScriptTest{
 			"insert into t values (5, 'five', 'six');",
 			"set @Commit5 = dolt_commit('-am', 'inserting into t 5');",
 		},
+		/* Commit graph:
+		          3 - 4 (new-branch)
+		         /
+		0 - 1 - 2 - 5 (main)
+		*/
 		Assertions: []queries.ScriptTestAssertion{
 			{
 				Query:    "SELECT count(*) from dolt_log('^main', 'new-branch');",
-				Expected: []sql.Row{{2}},
+				Expected: []sql.Row{{2}}, // 4, 3
 			},
 			{
 				Query:    "SELECT count(*) from dolt_log('main..new-branch');",
-				Expected: []sql.Row{{2}},
+				Expected: []sql.Row{{2}}, // 4, 3
+			},
+			{
+				Query:    "SELECT count(*) from dolt_log('main...new-branch');",
+				Expected: []sql.Row{{3}}, // 5, 4, 3
 			},
 			{
 				Query:    "SELECT count(*) from dolt_log('new-branch', '--not', 'main');",
-				Expected: []sql.Row{{2}},
+				Expected: []sql.Row{{2}}, // 4, 3
 			},
 			{
 				Query:    "SELECT count(*) from dolt_log('new-branch', '^main');",
-				Expected: []sql.Row{{2}},
+				Expected: []sql.Row{{2}}, // 4, 3
 			},
 			{
 				Query:    "SELECT count(*) from dolt_log('^new-branch', 'main');",
-				Expected: []sql.Row{{1}},
+				Expected: []sql.Row{{1}}, // 5
 			},
 			{
 				Query:    "SELECT count(*) from dolt_log('main', '--not', 'new-branch');",
-				Expected: []sql.Row{{1}},
+				Expected: []sql.Row{{1}}, // 5
 			},
 			{
 				Query:    "SELECT count(*) from dolt_log('^main', 'main');",
@@ -5399,6 +5420,10 @@ var LogTableFunctionScriptTests = []queries.ScriptTest{
 				Query:    "SELECT count(*) from dolt_log('main..main');",
 				Expected: []sql.Row{{0}},
 			},
+			{
+				Query:    "SELECT count(*) from dolt_log('main...main');",
+				Expected: []sql.Row{{0}},
+			},
 			{
 				Query:    "SELECT count(*) from dolt_log('main', '--not', 'main');",
 				Expected: []sql.Row{{0}},
@@ -5409,7 +5434,7 @@ var LogTableFunctionScriptTests = []queries.ScriptTest{
 			},
 			{
 				Query:    "SELECT count(*) from dolt_log('^main^', 'main');",
-				Expected: []sql.Row{{1}},
+				Expected: []sql.Row{{1}}, // 5
 			},
 			{
 				Query:    "SELECT count(*) from dolt_log('^main', 'main^');",
@@ -5421,15 +5446,15 @@ var LogTableFunctionScriptTests = []queries.ScriptTest{
 			},
 			{
 				Query:    "SELECT count(*) from dolt_log('^new-branch', @Commit5);",
-				Expected: []sql.Row{{1}},
+				Expected: []sql.Row{{1}}, // 5
 			},
 			{
 				Query:    "SELECT count(*) from dolt_log(@Commit3, '--not', @Commit2);",
-				Expected: []sql.Row{{1}},
+				Expected: []sql.Row{{1}}, // 3
 			},
 			{
 				Query:    "SELECT count(*) from dolt_log(@Commit4, '--not', @Commit2);",
-				Expected: []sql.Row{{2}},
+				Expected: []sql.Row{{2}}, // 4, 3
 			},
 		},
 	},
@@ -5492,6 +5517,11 @@ var LogTableFunctionScriptTests = []queries.ScriptTest{
 			"insert into t values (5, 'five', 'six');",
 			"set @Commit5 = dolt_commit('-am', 'inserting into t 5');",
 		},
+		/* Commit graph:
+		          3 - 4 (new-branch)
+		         /
+		0 - 1 - 2 - 5 (main)
+		*/
 		Assertions: []queries.ScriptTestAssertion{
 			{
 				Query: "SELECT commit_hash = @Commit4, commit_hash = @Commit3, committer, email, message from dolt_log('^main', 'new-branch');",
@@ -5507,6 +5537,14 @@ var LogTableFunctionScriptTests = []queries.ScriptTest{
 					{false, true, "John Doe", "johndoe@example.com", "inserting into t 3"},
 				},
 			},
+			{
+				Query: "SELECT commit_hash = @Commit5, commit_hash = @Commit4, commit_hash = @Commit3, committer, email, message from dolt_log('main...new-branch');",
+				Expected: []sql.Row{
+					{true, false, false, "billy bob", "bigbillieb@fake.horse", "inserting into t 5"},
+					{false, true, false, "John Doe", "johndoe@example.com", "inserting into t 4"},
+					{false, false, true, "John Doe", "johndoe@example.com", "inserting into t 3"},
+				},
+			},
 			{
 				Query: "SELECT commit_hash = @Commit4, commit_hash = @Commit3, committer, email, message from dolt_log('new-branch', '--not', 'main');",
 				Expected: []sql.Row{
--- a/integration-tests/bats/log.bats
+++ b/integration-tests/bats/log.bats
@@ -40,7 +40,7 @@ teardown() {
    [[ ! "$output" =~ "BRANCH1" ]] || false
 }

-@test "log: two dot log" {
+@test "log: two and three dot log" {
    dolt sql -q "create table testtable (pk int PRIMARY KEY)"
    dolt add .
    dolt commit -m "commit 1 MAIN"
@@ -121,6 +121,29 @@ teardown() {
    run dolt log ^main ^branchA
    [ $status -eq 0 ]

+    # Valid three dot
+    run dolt log main...branchA
+    [ $status -eq 0 ]
+    [[ ! "$output" =~ "MAIN" ]] || false
+    [[ "$output" =~ "AFTER" ]] || false
+    [[ "$output" =~ "BRANCHA" ]] || false
+    run dolt log branchA...main
+    [ $status -eq 0 ]
+    [[ ! "$output" =~ "MAIN" ]] || false
+    [[ "$output" =~ "AFTER" ]] || false
+    [[ "$output" =~ "BRANCHA" ]] || false
+    run dolt log main branchA --not $(dolt merge-base main branchA)
+    [ $status -eq 0 ]
+    [[ ! "$output" =~ "MAIN" ]] || false
+    [[ "$output" =~ "AFTER" ]] || false
+    [[ "$output" =~ "BRANCHA" ]] || false
+    run dolt log branchB...branchA
+    [ $status -eq 0 ]
+    [[ ! "$output" =~ "MAIN" ]] || false
+    [[ ! "$output" =~ "AFTER" ]] || false
+    [[ "$output" =~ "BRANCHA" ]] || false
+    [[ "$output" =~ "BRANCHB" ]] || false
+
    # Multiple refs
    run dolt log branchB branchA
    [ $status -eq 0 ]
@@ -134,6 +157,12 @@ teardown() {
    [[ "$output" =~ "AFTER" ]] || false
    [[ "$output" =~ "BRANCHA" ]] || false
    [[ ! "$output" =~ "BRANCHB" ]] || false
+    run dolt log main branchB branchA
+    [ $status -eq 0 ]
+    [[ "$output" =~ "MAIN" ]] || false
+    [[ "$output" =~ "AFTER" ]] || false
+    [[ "$output" =~ "BRANCHA" ]] || false
+    [[ "$output" =~ "BRANCHB" ]] || false
    run dolt log branchB main ^branchA
    [ $status -eq 0 ]
    [[ ! "$output" =~ "MAIN" ]] || false
@@ -176,6 +205,10 @@ teardown() {
    [ $status -eq 1 ]
    run dolt log testtable main..branchA 
    [ $status -eq 1 ]
+    run dolt log main...branchA testtable
+    [ $status -eq 1 ]
+    run dolt log testtable main...branchA 
+    [ $status -eq 1 ]
    run dolt log ^main branchA testtable
    [ $status -eq 1 ]
    run dolt log branchA testtable --not main
@@ -184,6 +217,10 @@ teardown() {
    [ $status -eq 1 ]
     run dolt log main main..branchA
    [ $status -eq 1 ]
+    run dolt log main...branchA main
+    [ $status -eq 1 ]
+     run dolt log main main...branchA
+    [ $status -eq 1 ]
    run dolt log ^main testtable
    [ $status -eq 1 ]
    run dolt log testtable ^main
@@ -194,8 +231,9 @@ teardown() {
    [ $status -eq 1 ]
    run dolt log main..branchA --not ^main
    [ $status -eq 1 ]
-
-    run dolt log main...branchA
+    run dolt log main...branchA --not main
+    [ $status -eq 1 ]
+    run dolt log main...branchA --not ^main
    [ $status -eq 1 ]
 }