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.

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

CLI command for reflog #7032

Merged
merged 13 commits into from
Dec 2, 2023
Merged

CLI command for reflog #7032

merged 13 commits into from
Dec 2, 2023

Conversation

stephkyou
Copy link
Contributor

Adds the CLI command dolt reflog which displays results form the dolt_reflog() table function

@stephkyou stephkyou marked this pull request as ready for review November 21, 2023 21:08
@stephkyou stephkyou requested a review from fulghum November 21, 2023 22:45
Copy link
Contributor

@fulghum fulghum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good so far! Mostly some minor comments and some clarification on how Dolt's reflog is a little different than Git's and has some different requirements.

There are also some comments about how the analyzer processes queries that you probably haven't seen before. I'm happy to explain more about how that works if you wanna know more details there or if any of those comments just don't make sense.

go/cmd/dolt/cli/arg_parser_helpers.go Outdated Show resolved Hide resolved
go/cmd/dolt/commands/reflog.go Outdated Show resolved Hide resolved
go/cmd/dolt/commands/reflog.go Outdated Show resolved Hide resolved
go/cmd/dolt/commands/reflog.go Outdated Show resolved Hide resolved
go/libraries/doltcore/sqle/reflog_table_function.go Outdated Show resolved Hide resolved
go/libraries/doltcore/sqle/reflog_table_function.go Outdated Show resolved Hide resolved
go/libraries/doltcore/sqle/reflog_table_function.go Outdated Show resolved Hide resolved
integration-tests/bats/reflog.bats Outdated Show resolved Hide resolved
go/cmd/dolt/commands/reflog.go Outdated Show resolved Hide resolved
Copy link
Contributor

@fulghum fulghum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking very, very close now! Just a couple of things to follow up on. Hit me up if the analyzer-related comments don't make sense and I'm more than happy to explain in more detail.

@@ -135,6 +135,7 @@ SKIP_SERVER_TESTS=$(cat <<-EOM
~cli-hosted.bats~
~profile.bats~
~ls.bats~
~reflog.bats~
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(minor) It would be nice if we could somehow document why we're skipping some of these. It'll be hard to remember in the future. I know you mentioned we couldn't run reflog.bats through the local-remote test because it depends on dolt gc which won't there yet. That would be a nice note to include, but I'm not sure this format would allow it.

go/cmd/dolt/commands/reflog.go Outdated Show resolved Hide resolved
go/cmd/dolt/commands/reflog.go Outdated Show resolved Hide resolved
go/libraries/doltcore/sqle/reflog_table_function.go Outdated Show resolved Hide resolved
go/libraries/doltcore/sqle/reflog_table_function.go Outdated Show resolved Hide resolved
Copy link
Contributor

@fulghum fulghum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! 🙌🏻 Just two really small things I noticed that should be super quick to fix before you merge this one into main.

Comment on lines 35 to 36
tabId sql.TableId
colset sql.ColSet
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two members don't seem to be used anywhere, so should be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were added in a separate PR, looks like they're being used a couple lines down.

if rltf.refExpr != nil {
return rltf.refExpr.Resolved()
for _, expr := range rltf.refAndArgExprs {
return expr.Resolved()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't want to always return on the first expression – you need to check that all the expressions are resolved.

You probably want something like if expr.Resolved() == false { return false } instead.

@stephkyou stephkyou merged commit bbe8c32 into main Dec 2, 2023
17 checks passed
@stephkyou stephkyou deleted the steph/reflog branch December 2, 2023 08:20
Copy link

github-actions bot commented Dec 3, 2023

@coffeegoddd DOLT

test_name detail row_cnt sorted mysql_time sql_mult cli_mult
batching LOAD DATA 10000 1 0.05 0.6
batching batch sql 10000 1 0.07 1.43
batching by line sql 10000 1 0.07 1.43
blob 1 blob 200000 1 0.88 3.19 3.64
blob 2 blobs 200000 1 0.9 3.98 4.58
blob no blob 200000 1 0.88 1.32 1.41
col type datetime 200000 1 0.81 1.83 1.93
col type varchar 200000 1 0.69 1.87 1.91
config width 2 cols 200000 1 0.77 1.92 1.35
config width 32 cols 200000 1 1.87 1.37 2.45
config width 8 cols 200000 1 1.06 1.23 1.44
pk type float 200000 1 0.96 1.1 1.11
pk type int 200000 1 1.01 0.97 0.99
pk type varchar 200000 1 1.78 0.89 0.83
row count 1.6mm 1600000 1 5.62 1.5 1.53
row count 400k 400000 1 1.43 1.4 1.44
row count 800k 800000 1 2.85 1.46 1.48
secondary index four index 200000 1 3.6 1.08 0.92
secondary index no secondary 200000 1 0.91 1.26 1.35
secondary index one index 200000 1 1.12 1.53 1.45
secondary index two index 200000 1 1.95 1.25 1.12
sorting shuffled 1mm 1000000 0 5.11 1.89 1.83
sorting sorted 1mm 1000000 1 5.16 1.83 1.82

Copy link

github-actions bot commented Dec 3, 2023

@coffeegoddd DOLT

name detail mean_mult
dolt_blame_basic system table 1.11
dolt_blame_commit_filter system table 2.71
dolt_commit_ancestors_commit_filter system table 0.96
dolt_commits_commit_filter system table 0.93
dolt_diff_log_join_from_commit system table 1.6
dolt_diff_log_join_to_commit system table 1.6
dolt_diff_table_from_commit_filter system table 1.11
dolt_diff_table_to_commit_filter system table 1.09
dolt_diffs_commit_filter system table 1
dolt_history_commit_filter system table 1.14
dolt_log_commit_filter system table 0.87

Copy link

github-actions bot commented Dec 3, 2023

@coffeegoddd DOLT

name add_cnt delete_cnt update_cnt latency
adds_only 60000 0 0 0.79
adds_updates_deletes 60000 60000 60000 3.77
deletes_only 0 60000 0 1.89
updates_only 0 0 60000 2.44

Copy link

github-actions bot commented Dec 4, 2023

@coffeegoddd DOLT

test_name detail row_cnt sorted mysql_time sql_mult cli_mult
batching LOAD DATA 10000 1 0.05 0.8
batching batch sql 10000 1 0.07 1.29
batching by line sql 10000 1 0.07 1.43
blob 1 blob 200000 1 0.87 3.34 3.63
blob 2 blobs 200000 1 0.86 4.23 4.8
blob no blob 200000 1 0.87 1.3 1.45
col type datetime 200000 1 0.8 1.81 1.99
col type varchar 200000 1 0.66 2 2.12
config width 2 cols 200000 1 0.74 1.35 1.35
config width 32 cols 200000 1 1.86 1.53 2.46
config width 8 cols 200000 1 0.98 1.31 1.7
pk type float 200000 1 0.91 1.15 1.18
pk type int 200000 1 0.78 1.26 1.28
pk type varchar 200000 1 1.54 0.97 0.93
row count 1.6mm 1600000 1 5.46 1.55 1.55
row count 400k 400000 1 1.41 1.43 1.48
row count 800k 800000 1 2.74 1.51 1.54
secondary index four index 200000 1 3.59 1.07 0.92
secondary index no secondary 200000 1 0.88 1.28 1.41
secondary index one index 200000 1 1.1 1.53 1.48
secondary index two index 200000 1 1.92 1.26 1.13
sorting shuffled 1mm 1000000 0 5.43 1.74 1.76
sorting sorted 1mm 1000000 1 5.23 1.86 1.86

Copy link

github-actions bot commented Dec 4, 2023

@coffeegoddd DOLT

name detail mean_mult
dolt_blame_basic system table 1.15
dolt_blame_commit_filter system table 2.75
dolt_commit_ancestors_commit_filter system table 0.9
dolt_commits_commit_filter system table 0.93
dolt_diff_log_join_from_commit system table 1.65
dolt_diff_log_join_to_commit system table 1.63
dolt_diff_table_from_commit_filter system table 1.11
dolt_diff_table_to_commit_filter system table 1.07
dolt_diffs_commit_filter system table 1.07
dolt_history_commit_filter system table 1.32
dolt_log_commit_filter system table 0.93

Copy link

github-actions bot commented Dec 4, 2023

@coffeegoddd DOLT

name add_cnt delete_cnt update_cnt latency
adds_only 60000 0 0 0.71
adds_updates_deletes 60000 60000 60000 3.73
deletes_only 0 60000 0 1.87
updates_only 0 0 60000 2.37

Copy link

github-actions bot commented Dec 4, 2023

@coffeegoddd DOLT

test_name detail row_cnt sorted mysql_time sql_mult cli_mult
batching LOAD DATA 10000 1 0.05 0.8
batching batch sql 10000 1 0.07 1.43
batching by line sql 10000 1 0.07 1.43
blob 1 blob 200000 1 0.88 3.25 3.73
blob 2 blobs 200000 1 0.89 4.22 4.92
blob no blob 200000 1 0.89 1.3 1.37
col type datetime 200000 1 0.8 1.84 2.01
col type varchar 200000 1 0.65 1.92 1.98
config width 2 cols 200000 1 0.77 1.3 1.32
config width 32 cols 200000 1 1.88 1.38 2.46
config width 8 cols 200000 1 0.94 1.67 1.65
pk type float 200000 1 0.89 1.21 1.19
pk type int 200000 1 0.79 1.23 1.29
pk type varchar 200000 1 1.52 1.01 0.96
row count 1.6mm 1600000 1 5.66 1.51 1.6
row count 400k 400000 1 1.4 1.44 1.49
row count 800k 800000 1 2.81 1.52 1.51
secondary index four index 200000 1 3.6 1.09 0.93
secondary index no secondary 200000 1 0.88 1.34 1.4
secondary index one index 200000 1 1.11 1.5 1.48
secondary index two index 200000 1 2.01 1.2 1.09
sorting shuffled 1mm 1000000 0 5.76 1.9 1.88
sorting sorted 1mm 1000000 1 5.72 1.95 1.94

Copy link

github-actions bot commented Dec 4, 2023

@coffeegoddd DOLT

name detail mean_mult
dolt_blame_basic system table 1.12
dolt_blame_commit_filter system table 2.76
dolt_commit_ancestors_commit_filter system table 0.87
dolt_commits_commit_filter system table 0.87
dolt_diff_log_join_from_commit system table 1.61
dolt_diff_log_join_to_commit system table 1.62
dolt_diff_table_from_commit_filter system table 1.04
dolt_diff_table_to_commit_filter system table 1.09
dolt_diffs_commit_filter system table 1
dolt_history_commit_filter system table 1.23
dolt_log_commit_filter system table 0.93

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants