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

Adds dolt_merge_status system table #4277

Merged
merged 2 commits into from
Sep 7, 2022
Merged

Adds dolt_merge_status system table #4277

merged 2 commits into from
Sep 7, 2022

Conversation

druvv
Copy link
Contributor

@druvv druvv commented Sep 6, 2022

Closes #3504

The dolt_merge_status system table tells a user if a merge is active. It has the following schema:

CREATE TABLE `dolt_merge_status` (
-- Whether a merge is currently active or not
  `is_merging` tinyint NOT NULL, 
 -- The commit spec that was used to initiate the merge
  `source` text,
-- The commit that the commit spec resolved to at the time of merge
  `source_commit` text, 
-- The target destination working set
  `target` text, 
-- A list of tables that have conflicts or constraint violations
  `unmerged_tables` text 
)

For example, lets create a simple conflict:

dolt sql -q "CREATE TABLE t (a INT PRIMARY KEY, b INT);"
dolt add .
dolt commit -am "base"

dolt checkout -b right
dolt sql <<SQL
ALTER TABLE t ADD c INT;
INSERT INTO t VALUES (1, 2, 1);
SQL
dolt commit -am "right"

dolt checkout main
dolt sql -q "INSERT INTO t values (1, 3);"
dolt commit -am "left"

dolt merge right

Output of SELECT * from dolt_merge_status;:

+------------+--------+----------------------------------+-----------------+-----------------+
| is_merging | source | source_commit                    | target          | unmerged_tables |
+------------+--------+----------------------------------+-----------------+-----------------+
| true       | right  | fbghslue1k9cfgbi00ti4r8417frgbca | refs/heads/main | t               |
+------------+--------+----------------------------------+-----------------+-----------------+

@druvv druvv requested a review from zachmu September 6, 2022 23:48
Copy link
Member

@zachmu zachmu left a comment

Choose a reason for hiding this comment

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

Some questions about this specific design. Seems like a row per unmerged table might be a bit more ergonomic?

}

func (s MergeStatusTable) Schema() sql.Schema {
return []*sql.Column{
Copy link
Member

Choose a reason for hiding this comment

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

Is this actually dolt_unmerged_tables, with one row per unmerged table?

No rows if no unmerged tables?

Get rid of is_merging and replace with a merge_active() function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A flat design could work here as well, but determining which tables are unmerged isn't the primary use case. The main reason we want a merge status function is to know if we need to commit a merge or if we need to abort the active one.

The unmerged tables column was added for more similarity between dolt status and this new procedure.

{Name: "is_merging", Type: sql.Boolean, Source: doltdb.MergeStatusTableName, PrimaryKey: false, Nullable: false},
{Name: "source", Type: sql.Text, Source: doltdb.MergeStatusTableName, PrimaryKey: false, Nullable: true},
{Name: "source_commit", Type: sql.Text, Source: doltdb.MergeStatusTableName, PrimaryKey: false, Nullable: true},
{Name: "target", Type: sql.Text, Source: doltdb.MergeStatusTableName, PrimaryKey: false, Nullable: true},
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this just active_branch()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, target is just the active branch. Do you think we should name it differently than target?

@@ -1328,6 +1328,10 @@ var MergeScripts = []queries.ScriptTest{
Query: "CALL DOLT_MERGE('feature-branch')",
Expected: []sql.Row{{1, 0}},
},
{
Copy link
Member

Choose a reason for hiding this comment

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

Need some tests with more than one table unmerged

@druvv druvv merged commit 86972d9 into main Sep 7, 2022
@Hydrocharged Hydrocharged deleted the dhruv/merge-status branch October 13, 2022 12:46
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.

dolt_merge_state system table
2 participants