Fix: Apply table mapping in restoreSchemaRegular for DROP/DETACH/CREA…

[siqueiraa]

fix for #1278

Summary

• Fixed the --restore-table-mapping workflow so that schema and data restores both target the mapped table names, preventing accidental drops of the original table and the subsequent "… is not created" failure.
• Normalised CLI-provided table mappings (e.g. transaction_archive:transaction_archive_v4 and db.table:newdb.newtable) so they are stored in a consistent form and applied everywhere in the restore pipeline.
• Hardened the data-restore phase to query ClickHouse with both original and mapped patterns, ensuring it locates newly created tables and surfaces actionable diagnostics if a mapping is misconfigured.
• Enhanced the missing-table check to consider mapped/original combinations and to re-query ClickHouse before emitting an error, reducing false negatives and clarifying real issues.
• Added focused unit coverage to lock the new mapping behaviour in place.

Context

Restoring m_views_tables.transaction_archive with --restore-table-mapping transaction_archive:transaction_archive_v4 previously dropped the target table (transaction_archive_v4), created it during schema restore, but the data phase still probed ClickHouse for the original name. That caused the restore to abort with 'm_views_tables.transaction_archive' is not created, leaving data un-restored.

Root Cause

restoreSchemaRegular operated on metadata that already carried the mapped name, while restoreDataRegular continued to query ClickHouse (and validate presence) using the original identifier. Because GetTables never saw transaction_archive_v4 under the old pattern, the data phase declared the table missing even though schema creation succeeded.

Solution Details

• Normalised table mapping ingestion (prepareRestoreMapping) so both CLI formats (bare table and db.table) collapse to the canonical table key.
• Updated restoreDataRegular to:
  • Log both metadata tables and the system tables returned by GetTables.
  • Query ClickHouse with a merged pattern containing original and mapped names, ensuring visibility of the newly created table.
  • Fall back to targeted lookups inside checkMissingTables before erroring out.
• Adjusted checkMissingTables to consider original, mapped, and reverse-mapped combinations and to re-populate its cache from ClickHouse on demand.
• Kept restoreSchemaRegular intact per upstream guidance; all fixes are isolated to mapping ingestion and data-phase validation.
• Added unit tests covering the mapping normalisation and pattern rewriting paths to guard against regressions.

Testing

• GOCACHE=$(pwd)/.gocache go test ./pkg/backup -run TestRestore
• Manual restore via API:

  restore --table=m_views_tables.transaction_archive \
          --restore-table-mapping transaction_archive:transaction_archive_v4 \
          chi-ch-cluster-ch-deployment-0-0-full-2025-10-20-10-41-51
  

  Observed data restore completing against m_views_tables.transaction_archive_v4 without missing-table errors.

Rollout Notes

• Requires rebuilding/publishing the container image (make build-docker + docker build … --target image_short).
• After deployment, existing restore jobs using --restore-table-mapping will pick up the fix automatically; no config updates needed.
• Optional: log verbosity can be reduced (or gated behind DEBUG) once operators finish validating the new behaviour.

[Slach]

please stop using vibe-coding if you can't programming and don't understand what exactly was generated

[Slach]

thanks for catching bug anyway, will try to fix it

[siqueiraa]

Are you talking about the treatment in pkg/backup/restore.go being redundant? (408:507) @Slach
If so, you could have just asked me to change it instead of saying I’m only “vibecoding.” I’ve been coding in Go for at least two years, and I do have a basic understanding of how the language works

[Slach]

My apologies, please excuse my rude words. I just in stress.
I will explain later

Was I right about vibe-coded PR?

[siqueiraa]

No, I only used AI to generate the PR description because I couldn’t find any template, and I wanted to provide the correct context.
As for the code, I’ve been looking into it since Friday generating tests and learning your code before suggesting the solution.

[Slach]

Ok, yep, too much words for PR description than usual
sorry again for my rude words, will provide more details later

[coveralls]

Pull Request Test Coverage Report for Build 19500019936

Details

• 125 of 150 (83.33%) changed or added relevant lines in 1 file are covered.
• 257 unchanged lines in 11 files lost coverage.
• Overall coverage decreased (-1.4%) to 67.088%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pkg/backup/restore.go	125	150	83.33%

Files with Coverage Reduction	New Missed Lines	%
cmd/clickhouse-backup/main.go	2	75.21%
pkg/backup/restore.go	2	72.02%
pkg/storage/sftp.go	2	75.96%
pkg/storage/object_disk/object_disk.go	5	66.3%
pkg/config/config.go	8	72.25%
pkg/server/server.go	9	58.49%
pkg/status/status.go	9	67.43%
pkg/storage/general.go	14	60.66%
pkg/backup/backuper.go	17	72.18%
pkg/storage/s3.go	23	47.39%

Totals
Change from base Build 19466046207:	-1.4%
Covered Lines:	10891
Relevant Lines:	16234

---

💛 - Coveralls