Skip to content

protocols/catalog: skip NULL-spec rows in LoadAll* queries#2959

Open
SeanWhelan wants to merge 2 commits into
masterfrom
sean/fix-delete-test-build-load
Open

protocols/catalog: skip NULL-spec rows in LoadAll* queries#2959
SeanWhelan wants to merge 2 commits into
masterfrom
sean/fix-delete-test-build-load

Conversation

@SeanWhelan
Copy link
Copy Markdown
Contributor

Description:

flowctl catalog delete --name <test> (and any publication that deletes a test) currently fails with:

test:N> FATA fatal error err="extracting from build: validating spec : missing Name"
Error: failed with status: testFailed

The Rust build layer represents a deletion as a built_* row with spec NULL (per BuiltRow::is_delete in built.rs). The Go-side LoadAll* queries in build_load.go did not filter those rows, so flowctl-go api test would Scan a NULL spec into an empty byte slice, Unmarshal it into a zero-value TestSpec, and trip TestSpec.Validate() with missing Name.

Fix: add WHERE spec IS NOT NULL to all four LoadAll* queries (collections, captures, materializations, tests). The same defect was latent in the other three loaders even though the visible failure was only in tests, since the test phase is the one that per-row-validates inside loadSpecs.

Workflow steps:

Repo before fix:

cat > flow.yaml <<'YAML'
collections:
  acmeCo/widgets:
    schema: { type: object, properties: { id: { type: string } }, required: [id] }
    key: [/id]
tests:
  acmeCo/widget-test:
    - ingest: { collection: acmeCo/widgets, documents: [{ id: "w1" }] }
    - verify: { collection: acmeCo/widgets, documents: [{ id: "w1" }] }
YAML
flowctl catalog publish --source flow.yaml --auto-approve
flowctl catalog delete --name acmeCo/widget-test
# → testFailed: validating spec : missing Name

Documentation links affected:

(list any documentation links that you created, or existing ones that you've identified as needing updates, along with a brief description)

Notes for reviewers:

(anything that might help someone review this PR)

@SeanWhelan SeanWhelan requested a review from a team May 20, 2026 10:55
Copy link
Copy Markdown
Member

@jgraettinger jgraettinger left a comment

Choose a reason for hiding this comment

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

LGTM

As a nit, LLMs tend to overly-extend comments to document the conditions of a change, including discussion of historical conditions that are no longer true, or handling which is obvious just by reading the code itself. In this case, I would have left the function comments as they are, and just added the WHERE guard.

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