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

Introduce more features to overwrite command #290

Merged
merged 65 commits into from
Dec 3, 2024

Conversation

CobbCoding1
Copy link
Contributor

Features:

  • remove subcommand
  • bulk-add subcommand
  • bulk-remove subcommand
  • --old-value flag
  • --force flag

The remove command works as expected, you pass the position you want to remove, and it deletes it from the database.
The bulk-add and bulk-remove flags both work similarly, as laid out in the usage message.
overwrite <dummy.csv> bulk-add <bulk.csv>
It can have the columns: row, col, val, author, timestamp, and old value.
The old-value and force flags also work as expected.

Some stylistic changes are that I've restructured the program, so there's an outer struct zsv_overwrite which passes the ctx, args, and overwrite around the functions, to avoid clutter in the functions. It also contains necessary information for the bulk operations, such as keeping track of the index of the columns.

Finally, I've added tests for every feature added here and their functionality.

@liquidaty
Copy link
Owner

@CobbCoding1 thank you. Could you pls make further changes as follows?

  1. Add test for these new features. For example, tests for insert should include at least one that fails due to an existing conflict, and one that overwrites an existing conflict using the --force flag
  2. Add remove --all which will entirely remove all overwrites (and the related sqlite3 db file)
  3. remove strdup_n and use zsv_memdup instead
  4. for insert: instead of checking for an existing value as a separate operation, just use "INSERT" if no --force flag is provided (and make sure to initially run "PRAGMA foreign_keys = on"), and use "INSERT OR REPLACE" otherwise
  5. bulk operations should be wrapped in a transaction (begin / commit)-- see 2db.c for an example-- and also should use the above INSERT/INSERT OR REPLACE approach, and so therefore will not require separate checking for data conflicts

@CobbCoding1
Copy link
Contributor Author

Alright, I'll get right on that.

@CobbCoding1
Copy link
Contributor Author

OK, I think that's everything. Let me know if there's any more changes I should make.

@liquidaty liquidaty merged commit 2d37382 into liquidaty:main Dec 3, 2024
13 checks passed
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