Skip to content

[rust][client][server][gateway] Relax partial update nullability rule - #3885

Open
gstamatakis95 wants to merge 5 commits into
apache:mainfrom
gstamatakis95:fix-3849
Open

[rust][client][server][gateway] Relax partial update nullability rule#3885
gstamatakis95 wants to merge 5 commits into
apache:mainfrom
gstamatakis95:fix-3849

Conversation

@gstamatakis95

@gstamatakis95 gstamatakis95 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Partial update validation required every non-primary-key column of a primary key table to be nullable, including
columns explicitly listed in the target columns. A listed column is supplied by the writer on every request, so the
requirement rejected valid schemas. The loop's own comment described the intended behavior ("check the columns not in
targetColumns"), but the loop never consulted the target column set.

The check is duplicated across four layers that validate independently, and all four carried the defect:

Layer Location
Rust client fluss-rust/.../client/table/upsert.rs, UpsertWriterFactory::sanity_check
Java client fluss-client/.../writer/UpsertWriterImpl.java, sanityCheck
Server fluss-server/.../kv/partialupdate/PartialUpdater.java, sanityCheck
REST gateway fluss-gateway/src/protocol/rest/records.rs, sparse_targets

The reported symptom cannot be resolved in the Rust client alone. A client-only fix permits writer construction and
defers the same rejection to the server at first write, so all four layers change together.

Resulting validation rules

Column Requirement Rationale
Omitted from target columns Must be nullable updateRow writes null into it when the row does not yet exist
Listed in target columns May be NOT NULL The writer supplies a value on every request
Auto increment Must be nullable Always omitted, and assigned its value only after the merge
Non-primary-key target column, under partial delete Must be nullable deleteRow sets it to null unless the whole row is removed

Divergence from the fix proposed in the issue

The issue proposes retaining the auto-increment exemption
(!target_column_set[i] && !pk_column_set[i] && !auto_increment_column_set[i]) and this PR removes it. Removing the
exemption is the correct direction because of ordering on the write path. PartialUpdater.updateRow null-fills
omitted columns before AutoIncrementUpdater assigns a value (KvWriteProcessor.processUpsert to applyInsert to
updateAutoIncrementColumns). Relaxing the server instead would allow a NOT NULL auto increment column to reach
BinaryWriter.createNotNullValueWriter holding null, producing an NPE surfaced to the client as
UnknownServerException.

Runtime guards

The previous rule guaranteed that a row reaching PartialUpdater cannot carry null in a non-nullable slot. Relaxing
it needs guards, because the decode path is not defensive: InternalRow.createFieldGetter and CompactedRowReader
omit the isNullAt branch for non-nullable types, and CompactedRow is a sequential variable-length encoding, so a
null bit in a non-nullable slot corrupts every later field (IndexedRow, whose reader honors the null bit per column,
corrupts only that column).

  • updateRow rejects a null value supplied for a NOT NULL target column, primary key columns included. The check
    runs before any field getter, because the first getter deserializes the whole row with the non-null-checking
    readers and would fail first with an error dependent on the bytes that follow. isNullAt reads only the null-bit
    header, so the check itself never deserializes.
  • deleteRow rejects partial delete when a non-primary-key target column is NOT NULL. The guard is placed after
    the isFieldsNull short circuit, so whole-row removal, which nulls nothing, remains legal. The guard is server
    side only, since legality depends on the stored row, and a client-side equivalent would be stricter and would make
    a server-supported operation unreachable.
  • The Java client runs the same null check per row before encoding, with the server's message, since the encoder
    would otherwise fail with a bare NullPointerException before the request is sent.
  • The gateway requires a non-null value for every NOT NULL target column in each upsert entry of a sparse batch,
    using the decoder's existing per-row required-column enforcement. Delete entries carry only primary key values, so
    the server judges them against the stored row.

One path stays outside the server guards: target columns covering every schema column short circuit in
DefaultRowMerger.configureTargetColumns and never build a PartialUpdater. The exposure equals a plain full-row
upsert, the Java client guard covers the case client side, and the encoders reject a null before anything is stored.

Merge engines

The first_row and versioned mergers reject partial update outright, and the aggregation merger does not fill
omitted columns with null on the first write, so aggregation tables keep requiring every column except the primary key to
be nullable. The Java client now checks all three at writer creation with the server's wording. Before, first_row
and versioned failed at the first write as UnknownServerException, and aggregation was rejected at creation with
the generic message.
MergeMode.OVERWRITE is exempt, because KvWriteProcessor merges an overwrite with the default merger rather than
the configured merge engine.

The Rust client reads no merge engine configuration today. The table config is available at writer creation, but the
guard is not part of this PR, to keep the Rust diff small. A Rust user on such a table still sees the rejection at
the first write. The Rust client also has no per-row check for a null in a NOT NULL target column. Such a write fails
in the encoder with a type error before it is sent, so nothing bad reaches the server. The gateway performs no merge
engine check either, so a REST partial update on such a table is rejected by the backend writer rather than at
preflight.

Error message

Changed from:

Partial Update requires all columns except primary key to be nullable, but column %s is NOT NULL.

to:

Partial Update requires all columns omitted from the target columns to be nullable, but omitted column %s is NOT NULL.

A NOT NULL auto increment column gets a dedicated message, because the user cannot follow the generic advice of
listing the column, targeting an auto increment column is itself rejected:

Partial Update requires the auto increment column %s to be nullable, since it is always omitted from the target columns and assigned by the server.

The gateway keeps its own message style and now reports the omitted column the same way, including the dedicated
auto increment message.

Flink

A Flink sink writing a partial update (a column subset in the SQL INSERT, or partial update columns on
FlussSinkBuilder) with a NOT NULL target column initializes and writes, and the first retract record is rejected
by the server's delete guard, which is not retriable. Insert-only pipelines are unaffected. The sink does not check this at plan time, so the rejection surfaces when the first delete arrives.

Compatibility

No wire format, storage format, or public API signature changes. The change relaxes validation, so previously
accepted writes remain accepted. Three cases move their failure from the first write to writer creation: partial
update on first_row and versioned tables, and a NOT NULL auto increment column. The rejection message for
aggregation tables changes from Partial Update requires... to Partial aggregate requires....

Fixes #3849

🤖 AI-assisted changes - reviewed by human developer

@gstamatakis95
gstamatakis95 force-pushed the fix-3849 branch 2 times, most recently from d1d732a to caa1c14 Compare August 6, 2026 16:57
@gstamatakis95 gstamatakis95 changed the title [WIP] [rust][client][server] Require only omitted columns to be nullable in partial update [rust][client][server] Require only omitted columns to be nullable in partial update Aug 6, 2026
@gstamatakis95
gstamatakis95 marked this pull request as ready for review August 28, 2026 07:55
@gstamatakis95
gstamatakis95 force-pushed the fix-3849 branch 2 times, most recently from 87cd787 to 220a7ea Compare August 28, 2026 08:26
…al update

Partial update required every non-primary-key column to be nullable, even
columns explicitly listed in the target columns. A listed column is always
supplied by the writer, so the requirement rejected valid usage.

Restrict the requirement to omitted columns in the Rust client, the Java
client and the server. Fixing only the clients is not enough, the server
runs its own copy of the check.

Auto increment columns keep the requirement. They are always omitted and
only receive their value after the merge, so updateRow writes null into
them first.

Partial delete keeps the requirement on non-primary-key target columns,
since it sets them to null. The check sits after the whole-row-removal
short-circuit and only exists on the server, because legality depends on
the stored row.

Tables using the aggregation merge engine keep the stricter requirement on
every non-primary-key column. Its merger returns the new row unchanged on
the first write instead of null filling, so the server rejects the relaxed
schema. The client checks it too, otherwise the writer is created and every
write fails asynchronously instead. MergeMode.OVERWRITE is exempt, since it
bypasses the configured merge engine and merges with the default merger.
Document the relaxed rule on Upsert, the interface callers actually use,
align the website docs on the target columns term, and cover the
whole-row-removal delete exemption through KvTablet.
The gateway now requires only omitted columns to be nullable and keeps
the per row presence check. The Java client rejects a null in a NOT NULL
target column and rejects first_row and versioned partial update early.
@gstamatakis95 gstamatakis95 changed the title [rust][client][server] Require only omitted columns to be nullable in partial update [rust][client][server][gateway] Relax partial update nullability rule Aug 29, 2026
Reword the client guard javadoc for the full coverage case and drop a
colon from the aggregation paragraph in pk-table.md.
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.

[rust] Partial update wrongly rejects NOT NULL columns that are explicitly listed in target columns

1 participant