Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make show command more resilient when resolving references #8730

Merged
merged 4 commits into from
Jan 13, 2025
Merged

Make show command more resilient when resolving references #8730

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
78d7d14
Make show command more resilient when resolving references
macneale4 Jan 10, 2025
d4b2722
Update go/cmd/dolt/commands/show.go
macneale4 Jan 10, 2025
c6cd6b4
Handle WORKING and STAGED as --no-pretty options
macneale4 Jan 10, 2025
6016c4a
PR Feedback
macneale4 Jan 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 42 additions & 21 deletions go/cmd/dolt/commands/show.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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` is not fully supported in the SQL shell.")
macneale4 marked this conversation as resolved.
Show resolved Hide resolved
return 1
}
}

if isDEnvRequired {
// use dEnv instead of the SQL engine
_, ok := queryist.(*engine.SqlEngine)
if !ok {
Expand All @@ -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)
Expand All @@ -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 to common use.")
macneale4 marked this conversation as resolved.
Show resolved Hide resolved
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)
Expand All @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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, "#")
}
Expand Down
9 changes: 4 additions & 5 deletions integration-tests/bats/show.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
macneale4 marked this conversation as resolved.
Show resolved Hide resolved
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" {
Expand Down
Loading