Skip to content

catalog: convert the sink builtin tables to materialized views (SQL-493) - #38049

Merged
mtabebe merged 2 commits into
MaterializeInc:mainfrom
mtabebe:ma/convert-builtins/sql-493-sink-tables
Aug 13, 2026
Merged

catalog: convert the sink builtin tables to materialized views (SQL-493)#38049
mtabebe merged 2 commits into
MaterializeInc:mainfrom
mtabebe:ma/convert-builtins/sql-493-sink-tables

Conversation

@mtabebe

@mtabebe mtabebe commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Problem:

mz_sinks, mz_kafka_sinks and mz_iceberg_sinks are builtin tables that pack_sink_update fills on the leader environmentd, on every catalog change. Getting rid of those writes is the point of this whole series (SQL-118 / multi-envd), and the sink family is the last item in M1.

Everything the three tables hold is already sitting in the persisted create_sql, so none of it has to be packed by Rust in the first place.

Note mz_continual_tasks is named in the issue but no longer exists. The CT feature was removed in #35967, and the v81_to_v82 migration asserts no CT items remain, so there is nothing there to convert. The Linear description wants correcting.

Solution:

Two parts.

parse_catalog_create_sql gets a CreateSink arm, alongside the CreateSource one it mirrors. It emits sink_type, cluster_id, connection_id, envelope_type, format/key_format/value_format, the kafka topic, and the iceberg namespace/table.

The deprecated format collapse stays in Rust rather than moving into the view SQL, because its edges are fiddly and here they are unit-testable. Only avro/avro and json/json collapse to a single name, and a key format only exists once the sink declares a KEY, which is easy to get wrong reading the statement alone.

Then all three tables become BuiltinMaterializedViews over mz_catalog_raw and pack_sink_update goes away, since packing them was its only job. Each view keeps its columns, order, nullability, keys, comments, access and ontology as they were, and reuses its old OID so pg identity does not move. No privileges column, unlike what mz_sources grew, to keep this a straight parity conversion.

  • three MigrationStep::replacement entries at the current dev version.
  • the old 0.160.0 step naming mz_sinks as a Table had to go with them: it no longer resolves, and validate_migration_steps panics on that at catalog open. Since that only reproduces against a catalog already on disk, there is now a unit test asserting every step resolves.

Testing:

  • New test/sqllogictest/mz_sinks.slt pins the column shape of all three views, that they are materialized views, that non-sink items are excluded, and the JSON keys the view SQL expects back from the helper. Sinks cannot be created in sqllogictest at all, since purification always talks to the broker, so derived values are not covered here.
  • mz-sinks.td picks up what nothing asserted before: key_format on a keyed bare format, the composite key-text-value-text form, and mz_kafka_sinks.topic.
  • test/iceberg/mode-append.td picks up envelope_type = append and mz_iceberg_sinks.namespace/table. The iceberg path had no catalog coverage.
  • Unit tests in jsonb.rs for the new arm: bare and key/value formats with and without a KEY, all three envelope values, the avro/json collapse against text/text and text/bytes, a missing TOPIC and a missing TABLE, and a non-sink statement as a regression guard.
  • The tables.td "cannot insert into system table" assertions move off mz_kafka_sinks and onto mz_storage_usage_by_shard, which the migration validator refuses to convert.

@mtabebe
mtabebe force-pushed the ma/convert-builtins/sql-493-sink-tables branch 2 times, most recently from 2fc1a99 to 77a62d4 Compare August 10, 2026 20:14
@mtabebe
mtabebe requested a review from ggevay August 10, 2026 21:03
@mtabebe
mtabebe marked this pull request as ready for review August 10, 2026 21:03
@mtabebe
mtabebe requested review from a team as code owners August 10, 2026 21:03
// resolves. Nothing is lost: anything that needed the old step upgrades
// from further back than this one, so this one covers it too.
MigrationStep::replacement(
"26.37.0-dev.0",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

all should be 38 by now

@ggevay ggevay left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, just one minor comment

CatalogItemType::MaterializedView,
MZ_CATALOG_SCHEMA,
"mz_iceberg_sinks",
),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These three conversions also need entries in misc/python/materialize/checks/all_checks/builtin_version_pin.py: _STABLE_BUILTINS maps each stable mz_catalog builtin to the version that converted it to a view, and mz_sinks, mz_kafka_sinks and mz_iceberg_sinks currently sit at None there. The file's comment asks for this at conversion time, and #37465 recorded mz_kafka_sources in the conversion PR itself. Could you set all three to the version these steps end up at (currently that would be v26.38.0), keeping the two in lockstep?

Note nothing goes red when this is missed: every reader on current upgrade paths carries the #37610 tolerance fix (>= v26.35), so a None entry just keeps the check pinning through the conversion and passing. It only shows up by reading, which is presumably how #37725's tables got missed too: mz_kafka_connections, mz_ssh_tunnel_connections and mz_aws_privatelink_connections are still None on main despite converting in released v26.37.0, and mz_aws_connections is absent from the dict entirely (it was a stable mz_catalog table when the dict was seeded, so it looks like an oversight). Feel free to fold the backfill in here: those four would be v26.37.0.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch.

I left mz_aws_connections out, though. It's in mz_internal, not mz_catalog, and it was mz_internal even as a table before #37725, so I don't think it was ever a seeded mz_catalog entry.

The script builds CREATE VIEW ... FROM mz_catalog so it isn't safe to go in.

mtabebe and others added 2 commits August 11, 2026 10:49
Problem:

mz_sinks, mz_kafka_sinks and mz_iceberg_sinks are builtin tables that
pack_sink_update fills on the leader environmentd, on every catalog
change. Getting rid of those writes is the point of this whole series
(SQL-118 / multi-envd), and the sink family is the last item in M1.

Everything the three tables hold is already sitting in the persisted
create_sql, so none of it has to be packed by Rust in the first place.

Note mz_continual_tasks is named in the issue but no longer exists. The
CT feature was removed in MaterializeInc#35967, and the v81_to_v82 migration asserts
no CT items remain, so there is nothing there to convert. The Linear
description wants correcting.

Solution:

Two parts.

parse_catalog_create_sql gets a CreateSink arm, alongside the
CreateSource one it mirrors. It emits sink_type, cluster_id,
connection_id, envelope_type, format/key_format/value_format, the kafka
topic, and the iceberg namespace/table.

The deprecated format collapse stays in Rust rather than moving into the
view SQL, because its edges are fiddly and here they are unit-testable.
Only avro/avro and json/json collapse to a single name, and a key format
only exists once the sink declares a KEY, which is easy to get wrong
reading the statement alone.

Then all three tables become BuiltinMaterializedViews over
mz_catalog_raw and pack_sink_update goes away, since packing them was
its only job. Each view keeps its columns, order, nullability, keys,
comments, access and ontology as they were, and reuses its old OID so pg
identity does not move. No privileges column, unlike what mz_sources
grew, to keep this a straight parity conversion.

- three MigrationStep::replacement entries at the current dev version.
- the old 0.160.0 step naming mz_sinks as a Table had to go with them:
it no longer resolves, and validate_migration_steps panics on that at
catalog open. Since that only reproduces against a catalog already on
disk, there is now a unit test asserting every step resolves.

Testing:

- New test/sqllogictest/mz_sinks.slt pins the column shape of all three
views, that they are materialized views, that non-sink items are
excluded, and the JSON keys the view SQL expects back from the helper.
Sinks cannot be created in sqllogictest at all, since purification
always talks to the broker, so derived values are not covered here.
- mz-sinks.td picks up what nothing asserted before: key_format on a
keyed bare format, the composite key-text-value-text form, and
mz_kafka_sinks.topic.
- test/iceberg/mode-append.td picks up envelope_type = append and
mz_iceberg_sinks.namespace/table. The iceberg path had no catalog
coverage.
- Unit tests in jsonb.rs for the new arm: bare and key/value formats
with and without a KEY, all three envelope values, the avro/json
collapse against text/text and text/bytes, a missing TOPIC and a
missing TABLE, and a non-sink statement as a regression guard.
- The tables.td "cannot insert into system table" assertions move off
mz_kafka_sinks and onto mz_storage_usage_by_shard, which the migration
validator refuses to convert.
- Wants the full and upgrade nightly. The migration steps and the
fingerprint checks only run against an existing catalog.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mtabebe
mtabebe force-pushed the ma/convert-builtins/sql-493-sink-tables branch from 77a62d4 to e894378 Compare August 11, 2026 14:56
@mtabebe
mtabebe requested a review from a team as a code owner August 11, 2026 14:56
@mtabebe
mtabebe requested review from def- and ggevay August 11, 2026 15:29

@ggevay ggevay left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@mtabebe
mtabebe requested review from def- and removed request for def- August 13, 2026 15:00

@def- def- left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No further complaints from qa side

@mtabebe
mtabebe merged commit 5710707 into MaterializeInc:main Aug 13, 2026
87 checks passed
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