Merge pull request #8730 from dolthub/macneale4/show-bandaid

Make show command more resilient when resolving references

go/cmd/dolt/commands/show.go

@@ -127,26 +127,43 @@ func (cmd ShowCmd) Exec(ctx context.Context, commandStr string, args []string, d
 		defer closeFunc()
 	}
 
+	resolvedRefs := make([]string, 0, len(opts.specRefs))
+	for _, specRef := range opts.specRefs {
+		if !hashRegex.MatchString(specRef) &&
+			!strings.EqualFold(specRef, "HEAD") &&
+			!strings.EqualFold(specRef, "WORKING") &&
+			!strings.EqualFold(specRef, "STAGED") {
+			// Call "dolt_hashof" to resolve the ref to a hash. the --no-pretty flag gets around the commit requirement, but
+			// requires the full object name so it will match the hashRegex and never hit this code block.
+			h, err2 := getHashOf(queryist, sqlCtx, specRef)
+			if err2 != nil {
+				cli.PrintErrln(fmt.Sprintf("branch not found: %s", specRef))
+				return 1
+			}
+			resolvedRefs = append(resolvedRefs, h)
+		} else {
+			resolvedRefs = append(resolvedRefs, specRef)
+		}
+	}
+	if len(resolvedRefs) == 0 {
+		resolvedRefs = []string{"HEAD"}
+	}
+
 	// There are two response formats:
 	//  - "pretty", which shows commits in a human-readable fashion
 	//  - "raw", which shows the underlying SerialMessage
 	// All responses should be in the same format.
 	// The pretty format is preferred unless the --no-pretty flag is provided.
 	// But only commits are supported in the "pretty" format.
 	// Thus, if the --no-pretty flag is not set, then we require that either all the references are commits, or none of them are.
-
-	isDEnvRequired := false
-
 	if !opts.pretty {
-		isDEnvRequired = true
-	}
-	for _, specRef := range opts.specRefs {
-		if !hashRegex.MatchString(specRef) && !strings.EqualFold(specRef, "HEAD") {
-			isDEnvRequired = true
+		if dEnv == nil {
+			// Logic below requires a non-nil dEnv when --no-pretty is set.
+			// TODO: remove all usage of dEnv entirely from this command.
+			cli.PrintErrln("`\\show --no-pretty` is not supported in the SQL shell.")
+			return 1
 		}
-	}
 
-	if isDEnvRequired {
 		// use dEnv instead of the SQL engine
 		_, ok := queryist.(*engine.SqlEngine)
 		if !ok {
@@ -160,12 +177,7 @@ func (cmd ShowCmd) Exec(ctx context.Context, commandStr string, args []string, d
 		}
 	}
 
-	specRefs := opts.specRefs
-	if len(specRefs) == 0 {
-		specRefs = []string{"HEAD"}
-	}
-
-	for _, specRef := range specRefs {
+	for _, specRef := range resolvedRefs {
 		// If --no-pretty was supplied, always display the raw contents of the referenced object.
 		if !opts.pretty {
 			err := printRawValue(ctx, dEnv, specRef)
@@ -176,20 +188,25 @@ func (cmd ShowCmd) Exec(ctx context.Context, commandStr string, args []string, d
 		}
 
 		// If the argument is a commit, display it in the "pretty" format.
-		// But if it's a hash, we don't know whether it's a commit until we query the engine.
-		commitInfo, err := getCommitSpecPretty(queryist, sqlCtx, opts, specRef)
+		// But if it's a hash, we don't know whether it's a commit until we query the engine. If a non-commit was specified
+		// by the user, we are forced to display the raw contents of the object - which required a dEnv
+		commitInfo, err := getCommitSpecPretty(queryist, sqlCtx, specRef)
 		if commitInfo == nil {
 			// Hash is not a commit
 			_, ok := queryist.(*engine.SqlEngine)
 			if !ok {
 				cli.PrintErrln("`dolt show (NON_COMMIT_HASH)` only supported in local mode.")
 				return 1
 			}
-
-			if !opts.pretty && !dEnv.DoltDB.Format().UsesFlatbuffers() {
+			if dEnv == nil {
+				cli.PrintErrln("`dolt show (NON_COMMIT_HASH)` requires a local environment. Not intended for common use.")
+				return 1
+			}
+			if !dEnv.DoltDB.Format().UsesFlatbuffers() {
 				cli.PrintErrln("`dolt show (NON_COMMIT_HASH)` is not supported when using old LD_1 storage format.")
 				return 1
 			}
+
 			value, err := getValueFromRefSpec(ctx, dEnv, specRef)
 			if err != nil {
 				err = fmt.Errorf("error resolving spec ref '%s': %w", specRef, err)
@@ -211,6 +228,8 @@ func (cmd ShowCmd) Exec(ctx context.Context, commandStr string, args []string, d
 	return 0
 }
 
+// printRawValue prints the raw value of the object referenced by specRef. This function works directly on storage, and
+// requires a non-nil dEnv.
 func printRawValue(ctx context.Context, dEnv *env.DoltEnv, specRef string) error {
 	value, err := getValueFromRefSpec(ctx, dEnv, specRef)
 	if err != nil {
@@ -220,6 +239,8 @@ func printRawValue(ctx context.Context, dEnv *env.DoltEnv, specRef string) error
 	return nil
 }
 
+// getValueFromRefSpec returns the value of the object referenced by specRef. This
+// function works directly on storage, and requires a non-nil dEnv.
 func getValueFromRefSpec(ctx context.Context, dEnv *env.DoltEnv, specRef string) (types.Value, error) {
 	var refHash hash.Hash
 	var err error
@@ -303,7 +324,7 @@ func parseHashString(hashStr string) (hash.Hash, error) {
 	return parsedHash, nil
 }
 
-func getCommitSpecPretty(queryist cli.Queryist, sqlCtx *sql.Context, opts *showOpts, commitRef string) (commit *CommitInfo, err error) {
+func getCommitSpecPretty(queryist cli.Queryist, sqlCtx *sql.Context, commitRef string) (commit *CommitInfo, err error) {
 	if strings.HasPrefix(commitRef, "#") {
 		commitRef = strings.TrimPrefix(commitRef, "#")
 	}

integration-tests/bats/show.bats

@@ -191,7 +191,7 @@ assert_has_key_value() {
     dolt add .
     dolt sql -q "create table table3 (pk int PRIMARY KEY)"
     dolt sql -q "insert into table1 values (7), (8), (9)"
-    run dolt show WORKING
+    run dolt show --no-pretty WORKING
     [ $status -eq 0 ]
     [[ "$output" =~ "table1" ]] || false
     [[ "$output" =~ "table2" ]] || false
@@ -208,7 +208,7 @@ assert_has_key_value() {
     dolt add .
     dolt sql -q "create table table3 (pk int PRIMARY KEY)"
     dolt sql -q "insert into table1 values (7), (8), (9)"
-    run dolt show STAGED
+    run dolt show --no-pretty STAGED
     [ $status -eq 0 ]
     [[ "$output" =~ "table1" ]] || false
     [[ "$output" =~ "table2" ]] || false
@@ -225,16 +225,15 @@ assert_has_key_value() {
     dolt add .
     dolt sql -q "create table table3 (pk int PRIMARY KEY)"
     dolt sql -q "insert into table1 values (7), (8), (9)"
-    workingRoot=$(dolt show WORKING)
+    workingRoot=$(dolt show --no-pretty WORKING)
     tableAddress=$(extract_value table1 "$workingRoot")
 
-    run dolt show $tableAddress
+    run dolt show --no-pretty $tableAddress
     assert_has_key Schema "$output"
     assert_has_key Violations "$output"
     assert_has_key Autoinc "$output"
     assert_has_key "Primary index" "$output"
     assert_has_key "Secondary indexes" "$output"
-
 }
 
 @test "show: pretty commit from hash" {