Skip to content

Support staging new tables with dolt add -p#10591

Merged
jycor merged 5 commits intodolthub:mainfrom
nullun:improved-dolt-add-patch
Mar 4, 2026
Merged

Support staging new tables with dolt add -p#10591
jycor merged 5 commits intodolthub:mainfrom
nullun:improved-dolt-add-patch

Conversation

@nullun
Copy link
Copy Markdown
Contributor

@nullun nullun commented Feb 27, 2026

When using 'dolt add -p' to stage rows from a new table (one that exists in working but not in staging), the workspace table UPDATE mechanism previously failed with 'table not found' because GetTableWriter looked for the table in the staging root where it didn't exist yet.

This change adds ensureTableExistsInStaging() which:

  1. Checks if the table exists in staging (fast path - no-op)
  2. If not, checks if it exists in working
  3. If yes, creates an empty table in staging with the same schema
  4. Updates the session state to reflect the new staging root

The table is created empty (not copied with data) because 'dolt add -p' allows partial staging - each row selected by the user will be inserted individually into the staging table via the workspace table UPDATE.

Also adds unit tests and BATS integration tests for the new functionality.

Example usage: https://gist.github.com/nullun/e88cb2dab9568c6612c98d415a4a2efd

When using 'dolt add -p' to stage rows from a new table (one that exists
in working but not in staging), the workspace table UPDATE mechanism
previously failed with 'table not found' because GetTableWriter looked
for the table in the staging root where it didn't exist yet.

This change adds ensureTableExistsInStaging() which:
1. Checks if the table exists in staging (fast path - no-op)
2. If not, checks if it exists in working
3. If yes, creates an empty table in staging with the same schema
4. Updates the session state to reflect the new staging root

The table is created empty (not copied with data) because 'dolt add -p'
allows partial staging - each row selected by the user will be inserted
individually into the staging table via the workspace table UPDATE.

Also adds unit tests and BATS integration tests for the new functionality.
Copy link
Copy Markdown
Contributor

@jycor jycor left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the contribution

Copy link
Copy Markdown
Contributor

@jycor jycor left a comment

Choose a reason for hiding this comment

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

Looks like there are some failing tests.

@nullun
Copy link
Copy Markdown
Contributor Author

nullun commented Mar 2, 2026

Hmmm, just got the email about the test failing again.

2026-03-02T18:50:45.2740284Z # `[[ "$output" =~ "| true | 1 | alice |" ]] || false' failed

Not sure I understand why off the top of my head, unless the formatting is different somehow? I'll take a look in the morning if that's okay.

@jycor
Copy link
Copy Markdown
Contributor

jycor commented Mar 2, 2026

Hmm... that should be right. It's possible it's something silly like a missing/extra space.
Tomorrow morning is totally fine, take your time.

The individual BATS statements can just be run the in the shell if that helps.

nullun added 2 commits March 3, 2026 14:22
Locally dolt will render bool values as true/false, so the remote-engine
test fails because it shows 1/0. Additionally whitespace will probably
also an issue. Outputting in a csv format should result in the same data
to compare in both scenarios.
@nullun
Copy link
Copy Markdown
Contributor Author

nullun commented Mar 3, 2026

Should have fixed it. I didn't realise there was a different when using remote-engine, which were what was failing. So my previous checks for "true" and "false" were actually "1" and "0". I thought simply using -r csv would make the output identical on both, but realised that wasn't fixing the actual problem, so now the SQL query itself will convert 1/0 to true/false.

The previously failing tests are now working, but let me know if you want me to tidy anything up or change it to be another way.
Bats tests (ubuntu-22.04)

ok 61 add-patch: new table partial staging

Bats tests

Using remote engine for tests
ok 61 add-patch: new table partial staging

Copy link
Copy Markdown
Contributor

@jycor jycor left a comment

Choose a reason for hiding this comment

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

Everything looks good. Thanks for the contribution!

@jycor jycor merged commit 9f01761 into dolthub:main Mar 4, 2026
26 of 34 checks passed
@nullun nullun deleted the improved-dolt-add-patch branch March 6, 2026 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants