Skip to content

feat(delete,backup): predicate delete, full backup and restore (PR8) - #8

Merged
xe-nvdk merged 1 commit into
mainfrom
feat/pr8-delete-backup
Sep 7, 2026
Merged

xe-nvdk merged 1 commit into
mainfrom
feat/pr8-delete-backup

Conversation

@xe-nvdk

@xe-nvdk xe-nvdk commented Sep 7, 2026

Copy link
Copy Markdown
Member

Stacked on #7 (which is stacked on #6). GitHub retargets as the stack merges; the diff shown is PR8 only.

Summary

  • arcli delete --database DB --measurement M --where SQL [--dry-run] [--yes] over POST /api/v1/delete/ (admin; the server refuses with 403 unless delete.enabled=true). Arc rewrites affected Parquet files without the matching rows and removes a file when every row matches.
    • NULL semantics: Arc's rewrite keeps NOT (w) rows, which under three-valued logic also drops rows whose predicate is NULL, while its preview counts only TRUE rows. arcli sends (w) IS TRUE on both calls so preview and rewrite agree and NULL rows are kept (standard DELETE semantics). Verified on real data in the smoke. Worth an Arc fix: rewrite with WHERE NOT COALESCE((w), FALSE).
    • Without --yes: non-interactive stdin refused first, then a server dry run (dry_run:true, confirm:true, so a preview above the confirmation threshold isn't refused) feeds a prompt with the server's row/file counts and its configured limits; full-table predicates get "ALL rows" wording. --dry-run only reports; --yes skips the preflight scan and the prompt.
    • The server reports zeros both for "no match" and for a predicate that does not evaluate (per-file errors are swallowed). arcli verifies a zero-file result with SELECT count(*) … WHERE (w) IS TRUE through the query endpoint: an HTTP 400 → "the WHERE clause does not evaluate against DB.M: …"; any other failure → warning, zero result stands.
    • Partial results: 207, a mid-run 5xx carrying counts, or a 2xx with success:false/failed_files → PartialDeleteError with the decoded result; the command prints the failed files and exits 1. Preview-vs-real count mismatch → warning.
    • Client-side: leading WHERE stripped; structural checks (;, --, /*, unbalanced quotes/parens); the server's keyword/function denylist is authoritative and surfaced verbatim. --timeout defaults to 30m; on a client timeout the server keeps rewriting (message says so).
  • arcli backup {create,list,show,status,delete,restore} over /api/v1/backup/* (admin; FeatureDisabledError when backup.enabled=false).
    • The 202 from create carries no id and the worker goroutine publishes progress after the handler returns, with the previous operation's record sticky until then. create/restore snapshot the status before the POST and wait (bounded) for an operation of our kind that is newer than the snapshot before printing the id or entering --wait. --wait (default --wait-timeout 2h, the server's budget) exits non-zero on failed in table and JSON modes.
    • backup delete GETs first (the server answers 500 for an unknown id) and re-GETs on a 500 to report "no longer exists". backup restore GETs the manifest first (the server 202s a missing id), derives "metadata staged / config restored" from the manifest ∩ flags, refuses --with-config when the backup has none, prompts with the databases, overwrite semantics, and quiescence advice, and prints the restart note whenever metadata/config were included. idle while waiting → "no longer tracked (server restarted?)".
    • 409 → "a backup or restore operation is already in progress (backup|restore|delete)" from the response body (HTTPError now carries the raw body for structured errors).
  • Hardening: the query-endpoint error decoder returns a scrubbed typed HTTPError (also benefits arcli query).

Test plan

  • gofmt -l . empty, go vet ./..., go test -race -count=1 ./... green
  • Unit: WHERE normalisation and structural checks, full-table detection, DeleteRows on 200/207/500-with-counts/403/400, backup id regex, 409 operation, list/show/status shapes incl. idle, restore body; command fakes for the delete preflight/prompt/real sequence (asserting exactly one dry-run request after "N"), zero-match disambiguation in all modes, partial failure, disabled server; backup fake modelling the delayed publish, second create after a completed one reporting the new id, restore prompt content, --with-config refusal, data-only vs default restart note, JSON --wait failure exits non-zero
  • Smoke against arc serve (HEAD e5c3f5e, ARC_DELETE_ENABLED=true, local backup path) with rows incl. a NULL tag: preview 3 / deleted 3 / NULL row survived; non-TTY refused before preflight; structural and server denylist errors; unknown measurement zeros; unknown column → "does not evaluate" in dry-run, --yes, and interactive; genuine zero match → zeros; full-table JSON; backup create prints a new id even right after a completed backup; --wait table and JSON; list newest-first; show manifest; bad id client-side; delete missing → 404 before prompt; restore --data-only --wait with no staged note; default restore prints the restart note; --with-config refused; restore --wait tracks the right id
  • Second server with ARC_BACKUP_ENABLED=false → "backups are disabled"; default server (delete.enabled=false) → 403 verbatim

Review notes

Internal: adversarial plan review (NULL-predicate server bug; backup-id publish race; zero-match ambiguity; restore flags computed from the request not the manifest), deep review (High: JSON --wait exited 0 on failure; start detection reworked to compare against the snapshot's own operation after a clock-slack version was caught by a unit test), security review (checklist PASS; query error decoder scrubbed). Follow-ups noted in README: composed --before/--after flags.

arcli delete --database --measurement --where over /api/v1/delete/:
the predicate is sent as "(w) IS TRUE" so Arc's rewrite (NOT (w),
which also drops NULL rows) matches its own preview; without --yes a
server dry run feeds the confirmation prompt; a zero-match preview is
verified with a COUNT query so a mistyped column is an error, not
"0 rows"; partial results (207 or a mid-run 5xx) are reported.

arcli backup {create,list,show,status,delete,restore} over
/api/v1/backup/*. The server publishes progress from a goroutine after
the 202, so create/restore snapshot the status first and wait for an
operation that is newer than it; --wait polls to completion and exits
non-zero on failure in every output mode. Restore is prompted, derives
its staged-metadata warning from the manifest, and refuses
--with-config for a backup that has none.
@xe-nvdk
xe-nvdk changed the base branch from feat/pr7-retention-cq to main September 7, 2026 22:19
@xe-nvdk
xe-nvdk merged commit f0cf4af into main Sep 7, 2026
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.

1 participant