Skip to content

docs: add partition-table-workflow reference guide - #712

Open
vojtechnovotny-heu wants to merge 3 commits into
keboola:mainfrom
vojtechnovotny-heu:docs/partition-table-workflow
Open

docs: add partition-table-workflow reference guide#712
vojtechnovotny-heu wants to merge 3 commits into
keboola:mainfrom
vojtechnovotny-heu:docs/partition-table-workflow

Conversation

@vojtechnovotny-heu

Copy link
Copy Markdown

Summary

  • Generalized partition-table skill from bi-team-marketplace into a reference workflow guide
  • Covers the canonical create-table + swap-tables pattern for adding time partitioning/clustering to BigQuery-backed Storage tables
  • Emphasizes physical verification via bq show (not just kbagent's registered metadata) and handling of STORAGE_JOB_TIMEOUT
  • Includes rollback procedures and gotchas reference table
  • Verified against real 6.29M-row table on europe-west3 stack
  • Timestamp format: TABLE_part_YYYYMMDDHHMMSS (no underscore between date and time)

Details

This workflow guide follows the same pattern as existing references (e.g., typify-table-workflow.md, branch-workflow.md):

  • Canonical pattern section with ASCII diagram
  • Typical workflow with 10-step bash commands
  • Key details (permissions, physical verification, timeout handling, revert procedures)
  • When to use/skip decision rules
  • Gotchas reference table with root causes and prevention

The guide addresses findings from live testing of partition and retype operations against real Keboola Storage tables.

Generalized partition-table skill from bi-team-marketplace into a
reference workflow guide for partitioning BigQuery-backed Keboola
Storage tables. Covers the canonical create+swap pattern, permission
checks, physical verification via bq show, STORAGE_JOB_TIMEOUT handling,
and rollback procedures. Verified against real 6.29M-row table.

Matches the pattern of existing workflow references (e.g.,
typify-table-workflow.md, branch-workflow.md).
Change format from YYYYMMDD_HHMMSS to YYYYMMDDHHMMSS (remove underscore
between date and time for cleaner naming). Updates all command examples
and documentation references.
Add reference to the new partition-table-workflow guide in the main
kbagent skill's workflow references table, positioned with other
Storage-related operations (between storage-types and typify-table).

@padak padak left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Review: docs: add partition-table-workflow reference guide

Thanks for writing this up — the operational discipline in here (timestamped rollback artifact, "don't auto-retry a timeout", the when-to-skip rules) is genuinely valuable and not currently written down anywhere. But I verified the guide's central claims live and the core verification guidance is inverted, so I'd rather not merge it as-is.

Verification setup: kbagent 0.91.0 against the e2e-bigquery project (6100, us-east4.gcp), default branch 49138. Script: ~/kbagent/e2e/partition_table_workflow_e2e.py. Real bucket + typed table (id:INTEGER, event_date:DATE, region:STRING, 20 rows), full create-table → swap → revert cycle, disposable, cleaned up.

Both CI doc gates (make skill-check, make version-gate-check) pass — so nothing here is machine-catchable. This is precisely the silent-drift class CLAUDE.md convention #17 warns about.


🔴 1. "Don't rely on Storage's definition for validation" is refuted by the live run

The guide's most load-bearing claim:

After a swap, Keboola's own registered column-type metadata (from storage table-detail) can lag or fail to resync to match the new physical table. […] Don't rely on Storage's definition for validation.

and the matching gotcha row ("Storage metadata still shows old schema after swap → Use bq show for validation, not kbagent storage table-detail").

Live result — table-detail immediately after the swap, on the original table id:

[original AFTER swap]  timePartitioning={'type': 'DAY', 'field': 'event_date'}
                       clustering={'fields': ['region']}
[_part artifact]       timePartitioning=None
[original AFTER revert] timePartitioning=None

No lag, no stale read, no resync gap. definition tracked the swap exactly, in both directions.

This also contradicts the command's own contract — commands/storage.py:376:

The Storage API's definition object is returned as-is. On BigQuery it carries the registered timePartitioning / rangePartitioning / clusteringthe only way to confirm a create-table --source-table-id + swap-tables repartition landed, since the table ID is the same either way.

That is the whole point of #621 / v0.88.0. Telling agents to distrust it removes the one native verification path the CLI has.

A likely reconciliation, not an accusation: v0.88.0 shipped 2026-08-22, your test was 2026-08-25. On 0.87.0 and earlier, table-detail dropped definition entirelystorage-types-workflow.md says so explicitly. If the box you tested on was still on ≤0.87.0, you'd have seen an empty/typeless table-detail after a correct swap, which is indistinguishable from "the metadata didn't resync". Could you check what kbagent version reported on that run?

Caveat on my side, stated plainly: my repro is 20 rows, yours was 6.29M. The swap is a metadata-only operation so I don't expect size to matter, but I can't rule it out from this test. If definition really does go stale after a swap on a large table under ≥0.88.0, that's a kbagent/Storage bug worth its own issue — not a workaround to bake into an agent skill. Happy to help chase it if you can reproduce.


🔴 2. bq show is a hard dependency the skill's audience doesn't have

The guide makes bq show mandatory ("always verify with bq show") at steps 3, 6 and 9. Problems:

  • Not part of kbagent, and not present in a normal operator environment. On this machine which bq gcloud → both absent. The skill is loaded by agents driving kbagent, not the gcloud SDK.
  • The guide never explains how to get PROJECT_ID:DATASET from a Keboola table id. For a Keboola-managed BigQuery backend, the operator typically has no direct GCP credentials for that dataset at all.
  • No other reference file in this skill shells out to a cloud-vendor CLI. typify-table-workflow.md — the closest analogue, same swap-tables pattern — validates entirely through kbagent.

Net effect: an agent following this guide stalls at step 3 in most environments, having been told the kbagent-native alternative is untrustworthy.


🔴 3. This duplicates storage-types-workflow.md, and diverges from it

plugins/kbagent/skills/kbagent/references/storage-types-workflow.md:309 already has ## BigQuery repartition via create-table --source-table-id — the same create-table + swap-tables pattern, same --primary-key, plus material the new file drops:

Covered there In this PR
clone-table prerequisite on storage-branches projects (step 2b) — without it the swap fails with a misleading "bucket not found" ❌ omitted
Alias hazard for tables that have aliases: they keep pointing at the same physical position and expose the other table's data post-swap ❌ only covers "the target is an alias"
create-table's own output is not a substitute — it echoes the requested layout, and --if-not-exists nulls all three layout keys
.definition.partitions is one entry per physical partition (thousands on a daily table) — select keys with jq
BigQuery-only pre-flight, --column XOR --source-table-id, partitioning shapes, valid copy sources
Verification via table-detail .definition, version-gated (0.88.0+, #621) ⚠️ contradicted

The alias omission is the one I'd call a data-correctness hazard: swapping a table that has aliases silently repoints them at the other table's data, and this guide gives no warning.

After this PR, SKILL.md has two adjacent rows (Storage column types and Partition/cluster a BigQuery table) pointing at overlapping content with opposite verification advice. An agent matching on "partition a BigQuery table" lands on the new, weaker one.


🟡 4. The timeout section points at the wrong remedy and warns about the wrong step

  • create-table --source-table-id shares the same 60s budget. client/storage_tables.py:563 calls _wait_for_storage_job(...) with no max_wait override (STORAGE_JOB_MAX_WAIT = 60.0, constants.py:160). On a 6.29M-row copy that is the step likely to time out — it moves data. The swap is metadata-only (~10s observed on Snowflake per the existing doc). The guide warns only about the swap.
  • The CLI already tells you exactly where to look. The timeout message (client/_core.py:317) names the job id and GET /v2/storage/jobs/{id}, and states the job keeps running server-side. The guide routes you to bq show instead and never mentions the job id.
  • The kbagent-native "did it land?" check is, again, table-detail.definition.

The "do not automatically retry" rule itself is correct and worth keeping — retrying a landed swap swaps it back. Just point it at reachable evidence.


🟡 5. Internal contradiction on permissions

Body: "Never call permissions set or permissions reset". Gotchas table, same file: "ask user to grant via permissions set". In an agent-facing doc that needs to be one unambiguous rule.

Both operation names check out (permissions.py:228,247) — worth stating their classes, since they differ: storage.create-table = write, storage.swap-tables = destructive. So --deny-destructive blocks the swap but not the create.


✅ Claims that verified clean

Recording these so they don't get lost if the file is restructured — all PASS on the live run:

  • create-table --source-table-id applies the requested --time-partitioning-type/-field + --clustering-field to the copy
  • --primary-key is accepted alongside --source-table-id
  • swap-tables works on the default/production branch (no dev branch needed)
  • The swap is symmetric — re-running the identical command reverts it
  • The _part_<ts> artifact holds the old unpartitioned layout after the swap
  • STORAGE_JOB_TIMEOUT is a 60s poll limit (constants.py:160)

Suggested path

The four things this PR adds that aren't written down anywhere are worth landing:

  1. Timestamped <TABLE>_part_<YYYYMMDDHHMMSS> artifact + don't delete it until downstream is validated
  2. permissions check before the write, and never widen the policy yourself
  3. Don't auto-retry on STORAGE_JOB_TIMEOUT — check whether it landed first
  4. The when-to-use / when-to-skip decision rules

My recommendation: fold those four into the existing ## BigQuery repartition section of storage-types-workflow.md, drop partition-table-workflow.md and the second SKILL.md row. That keeps one authority for the pattern and inherits the storage-branches, alias and --if-not-exists caveats for free.

If you'd rather keep a standalone file, then it needs to: use table-detail .definition as the verification path (version-gated (since v0.88.0)), demote bq show to an optional extra for people who happen to have BigQuery access, defer to storage-types-workflow.md for the mechanics instead of restating them, and carry the alias + storage-branches warnings.

Happy to pair on either shape — and I'd genuinely like to get to the bottom of the 6.29M-row observation if it reproduces on ≥0.88.0.

@padak

padak commented Aug 28, 2026

Copy link
Copy Markdown
Member

Follow-up: re-ran the verification at your scale (7M rows / 800 MB) — plus a correction to my own finding #4

My first pass used a 20-row table, which was a fair objection to it. I rebuilt the test at 7,000,000 rows / 800 MB — above the 6.29M you cited — on e2e-bigquery (project 6100, us-east4.gcp), kbagent 0.91.0, default branch 49138.

Fixture: 4 columns (id:INTEGER, event_date:DATE, region:STRING, payload:STRING with a 72-char pad), no primary key, built by uploading one 102 MB CSV via storage file-upload and replaying it 7× with storage load-file --incremental. MONTH partitioning on event_date, clustered on region. Bucket, Storage file and local fixture all deleted afterwards.

Result

[copy BEFORE swap]      rows=7000000  tp={'type':'MONTH','field':'event_date'}  cl={'fields':['region']}  partitions=12
[original BEFORE swap]  rows=7000000  tp=None                                   cl=None                   partitions=0

create-table --source-table-id   exit=0   15s
swap-tables                      exit=0   31s

[original AFTER swap]   rows=7000000  tp={'type':'MONTH','field':'event_date'}  cl={'fields':['region']}  partitions=12
[artifact AFTER swap]   rows=7000000  tp=None                                   cl=None                   partitions=0

The staleness claim does not reproduce at 7M rows either. table-detail.definition reported the new layout on the original table id on the very first read after the swap, and the _part_ artifact correctly reported the old one. No lag, no resync gap. So finding #1 stands, now at your scale: I'd drop the "don't rely on Storage's definition" guidance and the matching gotcha row.

Neither step hit STORAGE_JOB_TIMEOUT — both finished well inside the 60s budget.

Correction to my finding #4 — I got the risk ordering backwards

I wrote that create-table --source-table-id is "the step likely to time out — it moves data" and that the swap is metadata-only. The measurement says the opposite: create was 15s, the swap was 31s. Your emphasis on the swap was right and mine was wrong; please disregard that part. (The ~10s figure in storage-types-workflow.md is a Snowflake observation and evidently doesn't carry over to BigQuery.)

What survives from that finding, and I still think is worth acting on:

  • create-table shares the same 60s budgetclient/storage_tables.py:563 calls _wait_for_storage_job(...) with no max_wait override (STORAGE_JOB_MAX_WAIT = 60.0, constants.py:160). It's the less likely of the two to trip, but the guide should say both steps can.
  • A 31s swap on 800 MB makes your timeout section genuinely valuable — that's already half the budget, so a substantially larger table really will exceed it. Keep the section; it's the best original material in the PR.
  • The remedy should point somewhere reachable. The timeout message (client/_core.py:317) already names the job id and GET /v2/storage/jobs/{id}, and says the job keeps running server-side. Combined with the result above, table-detail.definition is a reliable "did it land?" check — which is exactly the situation your "do not automatically retry" rule is written for.

Where that leaves the review

Findings #1, #2 and #3 are unchanged; #4 is narrowed as above; #5 (the permissions set contradiction) is unchanged. Also confirmed in passing: .definition.partitions really is one entry per physical partition (12 months → 12 entries), so the existing doc's advice to select keys with jq rather than dumping the object matters here too.

The reusable harness is at ~/kbagent/e2e/partition_table_workflow_e2e.py if you want to re-run any of it.

If your 6.29M-row observation still reproduces on ≥0.88.0, I'd really like to see the kbagent version and the raw table-detail --json from that run — at that point it's a bug report, not a doc change.

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