Skip to content

feat(cli): fold substreams-sink-sql into the substreams CLI#830

Open
GabrielCartier wants to merge 29 commits into
developfrom
feature/sink-sql-cli
Open

feat(cli): fold substreams-sink-sql into the substreams CLI#830
GabrielCartier wants to merge 29 commits into
developfrom
feature/sink-sql-cli

Conversation

@GabrielCartier

@GabrielCartier GabrielCartier commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

What

substreams-sink-sql binary now lives in the CLI:

  • substreams sink postgres {setup,generate-csv,inject-csv,tools}
  • substreams sink clickhouse {setup,tools}

The engine command itself runs the sink (no run subcommand) and, like setup, auto-detects the mode from the output module type: sf.substreams.sink.database.v1.DatabaseChanges → schema.sql mode (db_out), anything else → relational mappings (from-proto).

Why

One CLI for sinks (follows noop/webhook/protojson precedent). Engine in the command name → per-engine flag surfaces, no mix-and-match.

Changes vs standalone binary

  • DSN: positional arg → --dsn flag / SUBSTREAMS_SINK_DSN env, scheme validated against the command's engine
  • Args like substreams run: <manifest> [<module>] + -s/--start-block -t/--stop-block; positional <start>:<stop> gone (inject-csv keeps its file range)
  • Engine prefixes stripped from flags: --cluster, --cursor-file-path, --sink-info-folder, --query-retry-*; clickhouse flags only on clickhouse commands
  • Metrics: house --prometheus-addr, custom --metrics-listen-addr gone
  • Bug fixes: dbt interval slept ns not seconds, zip-slip in dbt config extraction, tools cursor delete <hash> deleted ALL cursors (shadowing), bounded runs never flushed their final batch (exclusive stop block off-by-one, also latent in the standalone binary), manifests with a SQL sink: config could not parse (descriptors missing from system.pb; deprecated sql.v1 proto now bundled for existing packages)
  • Deprecated alias flags dropped (--flush-interval, --on-module-hash-mistmatch typo)

Testing

  • Integration tests (Docker, postgres+clickhouse) run on ubuntu CI leg via SF_SINK_SQL_INTEGRATION_TESTS, including TestSinker_Integration_BoundedRunFlushesOnCompletion locking the final-flush fix
  • Unit test locks the SQL sink Service descriptors in system.pb (manifest/sink_test.go)

Notes

  • Migration guide: docs/how-to-guides/sinks/sql/migration.md, with before/after command + DSN examples and operator notes (Docker image + container args mapping)
  • Relational-mappings reference (former FROM_PROTO.md): docs/references/sql/proto-annotations.md
  • create-user dropped; setup now also works for relational-mappings packages (creates schema, exits, idempotent)
  • Follow-ups: from-proto cursor unification, from-proto double manifest read, standalone repo deprecation + MIGRATION.md

Ported from substreams-sink-sql as-is: historical proto packages
(bare `schema`, `sf.substreams.sink.sql.service.v1`) kept for
compat with published .spkg files, hence the buf lint ignores.
Deprecated sf.substreams.sink.sql.v1 duplicate not ported; its
type URL stays accepted at the sink-config layer.
Verbatim port from github.com/streamingfast/substreams-sink-sql
under sink/sql/: db_changes (DatabaseChanges mode), db_proto
(relational from-proto mode), bytes, proto, services, and the
integration test harness.

- imports rewritten to in-repo pb/sf/substreams/sink/sql protos;
  the dead vendored options.pb.go copy was dropped
- integration tests (Docker) gated behind
  SF_SINK_SQL_INTEGRATION_TESTS, CI runners lack Docker
- testcontainers bumped to v0.43: wait.ForSQL now takes
  network.Port instead of nat.Port in helpers_test.go
Fold the substreams-sink-sql binary into the substreams CLI:
- sink sql {run,setup,create-user,generate-csv,inject-csv,tools}
  drives the DatabaseChanges mode
- sink from-proto drives the relational-mappings mode

Deliberate changes from the standalone binary:
- DSN is now --dsn (or SUBSTREAMS_SINK_SQL_DSN) instead of a
  positional arg, consistent across every subcommand
- metrics/pprof listeners are opt-in flags, no hardcoded :6060
- pgx v4 -> v5 for the inject-csv COPY path
- 'tools cursor delete <hash>' deleted all cursors in the
  standalone binary due to variable shadowing; now deletes only
  the given hash as documented
- dbt run interval slept nanoseconds instead of seconds
- reject dbt config zip entries escaping the extraction dir; the
  zip comes from untrusted .spkg files
- drop dead err param from FileDescriptorForOutputType
- port FROM_PROTO.md and point the clickhouse_table_options error
  at its real anchor (old link 404'd)
'sql' says nothing distinguishing: both sink modes target SQL
databases. New name matches the mode's proto (DatabaseChanges) and
the db_out module convention; 'db-changes' kept as alias. Review
fixes bundled:

- DSN env fallback renamed to SUBSTREAMS_SINK_DSN (shared with
  from-proto)
- pprof/delay flags honored by every subcommand, after cheap
  validation so a bad DSN fails before the delay sleep
- metrics via the standard --prometheus-addr sink flag; custom
  --metrics-listen-addr dropped
- block range arg: shared helper, errors on non-range input
  instead of silently ignoring, supports :stop and start 0
- generate-csv: final-blocks-only forced in config, flag removed
  from surface instead of silently overridden
- from-proto: dead endpoint-resolution block removed (extra
  manifest read), sink-service run only when a sink config exists
- create-user retry via derr.RetryContext (ctx-aware)
- load .substreams.env before streaming, like other sink commands
- deprecated flag aliases from the standalone binary dropped
  (--on-module-hash-mistmatch typo, --flush-interval)
- SF_SINK_SQL_INTEGRATION_TESTS set on the ubuntu leg only; macos
  runners have no Docker
- buf lint exemptions for the sink/sql protos narrowed from blanket
  ignore to the specific historical-naming rules
Examples still showed the standalone binary's positional DSN and a
DSN env var the command never reads; copy-pasting them failed with
'no DSN provided'. Also update a stale 'sink sql' panic message.
Transitive dep of the sink/sql port (via wk8/go-ordered-map);
Docker Scout blocks the image build on the HIGH out-of-bounds
read fixed in 1.1.2.
'sink database-changes' and 'sink from-proto' become
'sink postgres' and 'sink clickhouse'; run auto-detects the mode
from the output module type (DatabaseChanges proto -> schema.sql
mode, anything else -> relational mappings).

- engine-prefixed flags renamed: --cluster, --cursor-file-path,
  --sink-info-folder, --query-retry-count, --query-retry-sleep;
  clickhouse flags only exist on clickhouse commands
- positional <start>:<stop> range dropped; -s/--start-block and
  -t/--stop-block like 'substreams run' (inject-csv keeps its
  positional file range)
- DSN scheme validated against the command's engine
- generate-csv and inject-csv are postgres-only
Install/run instructions now use 'substreams sink postgres' /
'substreams sink clickhouse' with --dsn instead of the standalone
substreams-sink-sql binary; FROM_PROTO links point at the in-repo
copy.
Reviewer feedback:
- 'substreams sink postgres <manifest>' now runs the sink directly,
  'run' subcommand kept as the explicit form
- setup auto-detects the mode like run: DatabaseChanges packages
  execute schema.sql as before, anything else creates the database
  schema from the module's proto and exits; both idempotent. The
  schema step is shared with the run path via
  db_proto.SetupDatabaseSchema
- create-user dropped, with a clear error pointing migrators at
  their database
Streams live data into throwaway Docker databases and asserts rows
and cursor for both modes. Needs SUBSTREAMS_API_TOKEN.
The completion check compared the cursor against the exclusive stop
block, so the last streamed block (stop-1) always read as
'not completed' and bounded runs exited without flushing rows or
cursor. Found by the e2e script; latent in the standalone binary.
Manifests with a 'sink:' block typed sf.substreams.sink.sql.v1.Service
or ...sql.service.v1.Service failed to parse: the descriptors were
not in system.pb. Adds them, plus the deprecated sql.v1 proto that
existing packages still reference.
Locks in the stop-block off-by-one fix: fake server streams 3 final
blocks with an exclusive stop at last+1, all flush intervals raised
so only the completion flush can write, then asserts rows and cursor.
Fails on the pre-fix code with zero rows written.
@GabrielCartier
GabrielCartier marked this pull request as ready for review July 21, 2026 13:20
The package opens connections itself; importers should not need a
blank lib/pq import. Mirrors #761.
Dependency files taken from develop (grpc 1.82.1 clears the Docker
Scout gate), sink-sql pins re-applied (clickhouse-sql-parser v0.4.9,
substreams-sink-database-changes pseudo-version), changelog keeps
both sides.
@sduchesneau

sduchesneau commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

🔍 Vulnerabilities of ghcr.io/streamingfast/substreams:01e6c59

📦 Image Reference ghcr.io/streamingfast/substreams:01e6c59
digestsha256:f4e881adbbe4b61c1608d6dbd344f39bf593a371423d82bffb2deabd01b5951b
vulnerabilitiescritical: 0 high: 0 medium: 0 low: 0
platformlinux/amd64
size118 MB
packages381
📦 Base Image ubuntu:24.04
also known as
  • noble
  • noble-20260610
digestsha256:52df9b1ee71626e0088f7d400d5c6b5f7bb916f8f0c82b474289a4ece6cf3faf
vulnerabilitiescritical: 0 high: 0 medium: 14 low: 5

The merge took go.mod from develop and only two of the three
sink-sql pins were re-applied; CVE-2026-32285 gate tripped again.
Comment thread cmd/substreams/sink_clickhouse.go Outdated
Comment thread cmd/substreams/sink_clickhouse.go
Comment thread sink/sql/e2e/.gitignore Outdated
Comment thread docs/references/sql/sink-config.md Outdated
Comment thread docs/release-notes/change-log.md Outdated
Comment thread docs/how-to-guides/sinks/sql/migration.md
Comment thread sink/sql/FROM_PROTO.md Outdated
Review feedback on #830: e2e harness belongs next to the sink code it exercises, not in devel/.
Review feedback on #830. run is now a guarded removed-name like create-user.
…tion notes

Review feedback on #830: short command form everywhere, change-log entry points to the migration guide, migration guide gains a Docker image/args section for operators.
…erence

Review feedback on #830: reference content (annotations, type mappings, ClickHouse options) moves to docs/references/sql/proto-annotations.md, tutorial content already lives in the relational-mappings how-to. The clickhouse_table_options error message now points at the new page.
@GabrielCartier
GabrielCartier requested a review from maoueh July 23, 2026 11:16
@maoueh

maoueh commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

I think we are good to merge.

@GabrielCartier

Copy link
Copy Markdown
Contributor Author

I think we are good to merge.

I'll adjust the tests like you mentioned because I think it's relevant to this pr. After this we merge.

Review feedback on #830: the live-endpoint e2e script is replaced by automated coverage. The bounded-run flush bug already has TestSinker_Integration_BoundedRunFlushesOnCompletion in the CI integration suite; the missing-descriptor bug now has a unit test resolving both SQL sink Service types from system.pb.
@maoueh

maoueh commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Conflict on go.sum, after that ready to go on

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.

3 participants