Remove the system procedure for dolt admin createchunk commit, make it work entirely at the filesystem level. This guarantees that an unauthenticated SQL user can't use the procedure to edit branches (whereas anyone with CLI access is assumed to already have filesystem (and thus admin) access anyway.)

Author: nicktobey

go/cmd/dolt/commands/admin/createchunk/createcommit.go

@@ -17,15 +17,16 @@ package createchunk
 import (
 	"bytes"
 	"context"
-
-	"github.com/dolthub/go-mysql-server/sql"
-	"github.com/gocraft/dbr/v2"
-	"github.com/gocraft/dbr/v2/dialect"
+	"errors"
 
 	"github.com/dolthub/dolt/go/cmd/dolt/cli"
 	"github.com/dolthub/dolt/go/cmd/dolt/errhand"
+	"github.com/dolthub/dolt/go/libraries/doltcore/doltdb"
 	"github.com/dolthub/dolt/go/libraries/doltcore/env"
+	"github.com/dolthub/dolt/go/libraries/doltcore/ref"
 	"github.com/dolthub/dolt/go/libraries/utils/argparser"
+	"github.com/dolthub/dolt/go/store/datas"
+	"github.com/dolthub/dolt/go/store/hash"
 )
 
 // CreateCommitCmd creates a new commit chunk, printing the new chunk's hash on success.
@@ -121,7 +122,7 @@ func (cmd CreateCommitCmd) ArgParser() *argparser.ArgParser {
 	return cli.CreateCreateCommitParser()
 }
 
-func (cmd CreateCommitCmd) Exec(ctx context.Context, commandStr string, args []string, _ *env.DoltEnv, cliCtx cli.CliContext) int {
+func (cmd CreateCommitCmd) Exec(ctx context.Context, commandStr string, args []string, dEnv *env.DoltEnv, cliCtx cli.CliContext) int {
 	ap := cmd.ArgParser()
 	usage, _ := cli.HelpAndUsagePrinters(cli.CommandDocsForCommandString(commandStr, cli.CommandDocumentationContent{}, ap))
 
@@ -133,39 +134,94 @@ func (cmd CreateCommitCmd) Exec(ctx context.Context, commandStr string, args []s
 		return 1
 	}
 
-	queryist, sqlCtx, closeFunc, err := cliCtx.QueryEngine(ctx)
-	if err != nil {
-		cli.PrintErrln(errhand.VerboseErrorFromError(err))
+	desc, _ := apr.GetValue("desc")
+	root, _ := apr.GetValue("root")
+	parents, _ := apr.GetValueList("parents")
+	branch, isBranchSet := apr.GetValue(cli.BranchParam)
+	force := apr.Contains(cli.ForceFlag)
+
+	var name, email string
+	var err error
+	if authorStr, ok := apr.GetValue(cli.AuthorParam); ok {
+		name, email, err = cli.ParseAuthor(authorStr)
+		if err != nil {
+			cli.PrintErrln(errhand.VerboseErrorFromError(err))
+			return 1
+		}
+	} else {
+		name, email, err = env.GetNameAndEmail(cliCtx.Config())
+		if err != nil {
+			cli.PrintErrln(errhand.VerboseErrorFromError(err))
+			return 1
+		}
+	}
+
+	db := dEnv.DbData(ctx).Ddb
+	commitRootHash, ok := hash.MaybeParse(root)
+	if !ok {
+		cli.PrintErrf("invalid root value hash")
 		return 1
 	}
-	if closeFunc != nil {
-		defer closeFunc()
+
+	var parentCommits []hash.Hash
+	for _, parent := range parents {
+		commitSpec, err := doltdb.NewCommitSpec(parent)
+		if err != nil {
+			cli.PrintErrln(errhand.VerboseErrorFromError(err))
+			return 1
+		}
+
+		headRef := dEnv.RepoState.CWBHeadRef()
+
+		optionalCommit, err := db.Resolve(ctx, commitSpec, headRef)
+		if err != nil {
+			cli.PrintErrln(errhand.VerboseErrorFromError(err))
+			return 1
+		}
+		parentCommits = append(parentCommits, optionalCommit.Addr)
 	}
 
-	querySql, params, err := generateCreateCommitSQL(cliCtx, apr)
+	commitMeta, err := datas.NewCommitMeta(name, email, desc)
 	if err != nil {
 		cli.PrintErrln(errhand.VerboseErrorFromError(err))
 		return 1
 	}
-	interpolatedQuery, err := dbr.InterpolateForDialect(querySql, params, dialect.MySQL)
+
+	// This isn't technically an amend, but the Amend field controls whether the commit must be a child of the ref's current commit (if any)
+	commitOpts := datas.CommitOptions{
+		Parents: parentCommits,
+		Meta:    commitMeta,
+		Amend:   force,
+	}
+
+	rootVal, err := db.ValueReadWriter().ReadValue(ctx, commitRootHash)
 	if err != nil {
 		cli.PrintErrln(errhand.VerboseErrorFromError(err))
 		return 1
 	}
 
-	_, rowIter, _, err := queryist.Query(sqlCtx, interpolatedQuery)
+	var commit *doltdb.Commit
+	if isBranchSet {
+		commit, err = db.CommitValue(ctx, ref.NewBranchRef(branch), rootVal, commitOpts)
+		if errors.Is(err, datas.ErrMergeNeeded) {
+			cli.PrintErrf("branch %s already exists. If you wish to overwrite it, add the --force flag", branch)
+			return 1
+		}
+	} else {
+		commit, err = db.CommitDangling(ctx, rootVal, commitOpts)
+	}
 	if err != nil {
 		cli.PrintErrln(errhand.VerboseErrorFromError(err))
 		return 1
 	}
 
-	rows, err := sql.RowIterToRows(sqlCtx, rowIter)
+	commitHash, err := commit.HashOf()
 	if err != nil {
 		cli.PrintErrln(errhand.VerboseErrorFromError(err))
 		return 1
 	}
 
-	cli.Println(rows[0][0])
+	cli.Println(commitHash.String())
 
 	return 0
 }

go/libraries/doltcore/sqle/dprocedures/admin/dolt_admin_createchunk_commit.go

@@ -59,8 +59,6 @@ var DoltProcedures = []sql.ExternalStoredProcedureDetails{
 	{Name: "dolt_stats_once", Schema: statsFuncSchema, Function: statsFunc(statsOnce)},
 	{Name: "dolt_stats_gc", Schema: statsFuncSchema, Function: statsFunc(statsGc)},
 	{Name: "dolt_stats_timers", Schema: statsFuncSchema, Function: statsFunc(statsTimers)},
-
-	{Name: "dolt_admin_createchunk_commit", Schema: stringSchema("hash"), Function: admin.CreateCommit},
 }
 
 // stringSchema returns a non-nullable schema with all columns as LONGTEXT.

go/libraries/doltcore/sqle/dprocedures/init.go

@@ -98,6 +98,7 @@ teardown() {
 
     # but overwriting an existing branch with a different history is an error
     run dolt admin createchunk commit --root "$newRootValueHash" --desc "flattened history" --parents "$newCommitHash" --branch existingBranch
+    echo "$output"
     [ "$status" -eq 1 ]
     [[ "$output" =~ "branch existingBranch already exists. If you wish to overwrite it, add the --force flag" ]] || false
 
@@ -120,62 +121,3 @@ teardown() {
     [ "$status" -eq 1 ]
     [[ "$output" =~ "the --branch flag is required when creating a chunk using the CLI" ]] || false
 }
-
-@test "createchunk: create commit in SQL on existing branch" {
-    dolt branch existingBranch
-    run dolt sql -q "CALL DOLT_ADMIN_CREATECHUNK_COMMIT('--root', '$newRootValueHash', '--author', 'a <b@c.com>', \
-      '--desc', 'flattened history', '--parents', 'refs/internal/create', '--branch', 'existingBranch', '--force');" -r csv
-    echo "$output"
-    [ "$status" -eq 0 ]
-    flattenedCommitHash=$(echo "$output" | tail -n 1)
-    run dolt show existingBranch
-    echo "$output"
-    [ "$status" -eq 0 ]
-    [[ "$output" =~ "Author: a <b@c.com>" ]] || false
-    [[ "$output" =~ "flattened history" ]] || false
-    [[ "$output" =~ "added table" ]] || false
-    [[ "$output" =~ "| + | 1  |" ]] || false
-
-    # check that there are only two commits in the history
-    run dolt log existingBranch
-    [ "$status" -eq 0 ]
-    [ "$(echo "$output" | grep -c commit)" -eq 2 ]
-    [[ "$output" =~ "Initialize data repository" ]] || false
-    [[ "$output" =~ "flattened history" ]] || false
-}
-
-@test "createchunk: create commit in SQL on new branch" {
-    flattenedCommitHash=$(dolt sql -q "CALL DOLT_ADMIN_CREATECHUNK_COMMIT('--root', '$newRootValueHash', '--author', 'a <b@c.com>', '--desc',\
-     'flattened history', '--parents', 'refs/internal/create', '--branch', 'newBranch');" -r csv | tail -n 1)
-
-    run dolt show newBranch
-    echo "$output"
-    [ "$status" -eq 0 ]
-    [[ "$output" =~ "Author: a <b@c.com>" ]] || false
-    [[ "$output" =~ "flattened history" ]] || false
-    [[ "$output" =~ "added table" ]] || false
-    [[ "$output" =~ "| + | 1  |" ]] || false
-
-    # check that there are only two commits in the history
-    run dolt log newBranch
-    [ "$status" -eq 0 ]
-    [ "$(echo "$output" | grep -c commit)" -eq 2 ]
-    [[ "$output" =~ "Initialize data repository" ]] || false
-    [[ "$output" =~ "flattened history" ]] || false
-}
-
-@test "createchunk: create commit in SQL with no provided branch" {
-    run dolt sql -r csv <<SQL
-    CALL DOLT_ADMIN_CREATECHUNK_COMMIT('--root', '$newRootValueHash', '--author', 'a <b@c.com>', '--desc',
-     'flattened history', '--parents', '$initialCommitHash', '--branch', 'newBranch');
-    SELECT * from dolt_log;
-SQL
-    [ "$status" -eq 0 ]
-    # Just capture the last four lines (the select)
-    run echo "$(echo "$output" | tail -n 4)"
-    echo "$output"
-    [[ "${lines[0]}" =~ "commit_hash,committer,email,date,message" ]] || false
-    [[ "${lines[1]}" =~ "insert into table" ]] || false
-    [[ "${lines[2]}" =~ "create table" ]] || false
-    [[ "${lines[3]}" =~ "Initialize data repository" ]] || false
-}