GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Support for JSON output with `index` properties by mihaibudiu · Pull Request #7752 · feldera/feldera

Conversation

@mihaibudiu

@mihaibudiu mihaibudiu commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

Linked Issue

Fixes #7723
Fixes #7750

There are 4 separate commits, which should work independently.

Checklist

  • Unit tests added/updated
  • Documentation updated
  • Changelog updated

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@strix-security

strix-security Bot commented Oct 30, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Strix Security Review

✅ No open security findings.

PR summary: Adds a JSON output encoder path for connectors attached to a view index: Debezium-format change events keyed by the index columns, `u` events with `before`/`after`, optional tombstones after deletes (`tombstones_on_delete`), the Debezium value schema now declaring `before`, CSV rejecting `index`, and a refactor of the profiler support-bundle SQL panel.

Review summary

Reviewed the new indexed JSON/Debezium encoder path (encode_indexed, JsonEncoder::new, the new validate rules), the rewritten buffer-size accounting in the non-indexed encode loop, the new tombstones_on_delete config (Rust and Java), the Debezium schema change, the CSV index rejection, and the profiler frontend refactor. No vulnerability was confirmed: the message-size accounting in both the indexed and non-indexed paths is exact or fail-closed (and now includes key bytes, closing a pre-existing over-limit case), the index/key_schema pairing is structurally guaranteed by the catalog, attacker-controlled records cannot multiply per-key output or panic the error formatter, and the SQL panel renders bundle content as escaped text with no raw-HTML sink. Remaining observations are author-only configuration and correctness nits that cross no trust boundary.

Ruled out (6)
  • Can the new indexed encoder path be made to overflow the transport's max-buffer-size limit or panic on attacker-controlled data?
    The `required` size check equals the bytes actually pushed for both array modes, per-key output is bounded to one `before` and one `after` by `indexed_operation_type`, and the error formatter is UTF-8-safe and length-bounded.
  • Does the rewritten non-indexed buffer accounting drop records, underflow, or let a keyed message exceed the transport limit?
    The rewrite is purely additive with no subtraction left, `prev_len` truncation is always in range, deferred records keep their weight and sequence number, and including `key_buffer` makes the guard stricter than before.
  • Can a connector config reach the index encoder in an unsafe or unsupported state, or bypass transport capability checks?
    `is_index` is derived from the same condition that forces `index_handles` resolution, which only yields handles with a key schema, and the residual mismatches fail the author's own endpoint at runtime rather than crossing any security boundary.
  • Does the support-bundle viewer render bundle-supplied SQL or config through an unescaped HTML sink, or let one user's bundle break another user's session?
    No `{@​html}` sink exists on the bundle path, Monaco `setValue` is a plain text buffer, the new `sqlPanel` snippet has no production consumer, and bundle parsing is entirely client-side.
  • Is the unguarded `program_code.split('\n')` in the bundle parser a reportable availability issue?
    Server-generated bundles always carry a string `program_code`, and the TypeError is caught by the load guard and shown as escaped text in the opener's own tab only.
  • Should `array: true` combined with an index be treated as a security issue?
    It only affects the output shape of a sink the author configured themselves, is the documented meaning of the `array` option, and has no size-accounting or memory-safety consequence.

Review 1 on this PR · 8c9fc52 · requested Oct 30 20:31 UTC when the PR was opened


Reviewed by Strix
Re-run review · Configure security review settings

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

mythical-fred-oss[bot]
mythical-fred-oss Bot reviewed Oct 30, 2026
View reviewed changes
Comment thread ...rc/main/java/org/dbsp/sqlCompiler/compiler/frontend/connectors/config/JsonEncoderConfig.java Resolved

Uh oh!

There was an error while loading. Please reload this page.

mythical-fred-oss[bot]
mythical-fred-oss Bot reviewed Oct 30, 2026
View reviewed changes
Comment thread crates/connectors/src/format/json/output.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.

mythical-fred-oss[bot]
mythical-fred-oss Bot reviewed Oct 30, 2026
View reviewed changes
Comment thread crates/connectors/src/format/json/output.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.

@mythical-fred-oss

mythical-fred-oss Bot commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

Review: JSON Debezium output for index, CSV index rejection, buffer-size fix, before in the Kafka Connect schema

What I ran:

Check Result
cargo test -p static-connectors for format::json::output, format::csv::test, and the aliased-index controller::test cases 35/35 pass
cargo test -p dbsp_connectors kafka_connect_json_converter pass
mvn test -Dtest=ConnectorTests 161/161 pass
pre-commit run on the changed files pass
QA: new test for array: true with an index (in the inline comment) pass

The Kafka tests in the same filter failed only because no broker was running in my environment, so that failure is not from this PR.

Gates:

  • SQL type coverage (medium): this changes format/encoding, but the indexed path is tested only on the 4-column Basic relation with a single BIGINT key. Please add an every-type round-trip (see inline comment).
  • Breaking changes (CSV + index is now rejected; the Debezium value schema gains before) are documented in the changelog. User docs and the config rustdoc are updated. No new dependencies and no unsafe.

Cases the PR's tests don't cover:

  • missing (medium): a composite index key, or a key with a nullable, temporal, or DECIMAL column.
  • missing (low): array: true combined with an index.
  • pitfall (medium): the compiler accepts tombstones_on_delete without an index, so the error only appears at runtime (see inline comment).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mihaibudiu
mihaibudiu force-pushed the issue7723 branch from 8c9fc52 to 0faa961 Compare October 30, 2026 22:51
@mihaibudiu
mihaibudiu requested a review from anandbraman November 4, 2026 19:04
anandbraman
anandbraman approved these changes Nov 5, 2026
View reviewed changes

@anandbraman anandbraman left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Tested with sample data locally and everything worked well. Thank you!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
mihaibudiu added 6 commits November 4, 2026 18:12
@mihaibudiu
[connectors] Support INDEX for JSON output with the Debezium update f…
686b020
…ormat

With an index, the JSON encoder writes one Debezium event for each key
that a step changes: `c` with `after` for an insert, `u` with `before`
and `after` for an update, and `d` with `before` for a delete.  The
message key holds the index columns.  A tombstone follows each `d`,
unless `tombstones_on_delete` is false.  The output without an index
does not change.

Fixes #7723

Signed-off-by: Mihai Budiu <[email protected]>
@mihaibudiu
[connectors] Reject CSV outputs with an index property
8b3a5aa
Before, the CSV encoder ignored the index and wrote each update as a
delete and an insert.  The controller tests of aliased indexes now
attach JSON Debezium output to the index, on an in-memory endpoint
that keeps keyed messages.

Signed-off-by: Mihai Budiu <[email protected]>
@mihaibudiu
[connectors] Check that JSON output messages fit in the transport buffer
24f2644
The size check of the JSON encoder did not count the message key, the
`\n` after the last record, or the `]\n` after an array.  When a record
did not fit, the encoder wrote `\n` again, so the message ended in an
empty line.  A message could be larger than the transport's limit by
the size of its key plus two bytes.

Signed-off-by: Mihai Budiu <[email protected]>
@mihaibudiu
[connectors] Declare before in the Kafka Connect schema of Debezium…
284ed8a
… JSON output

A Debezium delete event holds the deleted row in
`before`, but the schema that the `kafka_connect_json_converter` flavor
writes declared only `after` and `op`.  The Kafka Connect JsonConverter
builds its record from the schema, so a delete reached Kafka Connect
without the row.  The value schema now declares `before`, as the
Debezium envelope does for every event.

Fixes #7750

Signed-off-by: Mihai Budiu <[email protected]>
@mihaibudiu
[docs] Document JSON index output with exactly-once Kafka
0410647
A Kafka output connector with exactly-once fault tolerance does not
write tombstones.  JSON `debezium` output with an index writes a
tombstone after each `d` event, unless `tombstones_on_delete` is false,
so such a pipeline must set it to false.

Signed-off-by: Mihai Budiu <[email protected]>
@mihaibudiu
[docs] Avro output with an index works with exactly-once Kafka
71ff610
Since exactly-once Kafka output accepts keyed messages, Avro output
with an index works in that mode.  Only the `confluent_jdbc` update
format cannot write its deletions, which are tombstones.

Signed-off-by: Mihai Budiu <[email protected]>
@mihaibudiu
mihaibudiu force-pushed the issue7723 branch from 0faa961 to 71ff610 Compare November 5, 2026 02:21
@mihaibudiu
mihaibudiu added this pull request to the merge queue Nov 5, 2026
Merged via the queue into main with commit 327d179 Nov 5, 2026
1 check passed

Uh oh!

There was an error while loading. Please reload this page.

@mihaibudiu
mihaibudiu deleted the issue7723 branch November 5, 2026 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mythical-fred-oss mythical-fred-oss[bot] mythical-fred-oss[bot] left review comments
@anandbraman anandbraman anandbraman approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[connectors] Debezium JSON output loses the deleted row for Kafka Connect consumers [connectors] Support index for JSON output

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@mihaibudiu @anandbraman
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.