abrarsher23 commented on issue #18262:
URL: https://github.com/apache/iceberg/issues/18262#issuecomment-5956648553
**Known gaps in the shared-tracker approach**
These are cases I found while testing a fix for this issue: the approach in
#18288, where all writers of one table share the inserted-row tracker for the
length of a checkpoint. In each case the writers still can't share, so the
duplicate described above can still happen for a key that is written both
before and after the change within one checkpoint. I'm not proposing to fix
them in the same PR; I'm posting them here so they're tracked with this issue.
Happy to open separate issues if that's preferred.
1. **An equality field becomes optional.**
- **When it happens:** when equality fields are set explicitly and aren't
identifier fields (identifier fields must be required). `EvolveSchemaVisitor`
makes a column optional when the input field is optional.
- **Why the obvious fix is unsafe:** you can't simply ignore optionality
when comparing key types. For a required `int` or `long` field, the comparator
throws on null, and `StructLikeWrapper.equals` treats the exception as "not
equal". So a tracker built for a required key never matches null keys, and
null-key upserts through the new writer would duplicate.
- **A safe fix:** build the tracker's map from an all-optional copy of
the key type. Non-null keys compare the same as before, and null matches null,
as it does in an equality delete.
2. **An equality field's type is promoted** (`int` → `long`, `float` →
`double`, or decimal precision).
- **Why writers split:** `TableMetadataCache` resolves records still on
the old input schema to the old table schema. So a narrow writer and a wide
writer can both be open, and records can alternate between them.
- **Why the tracker can't be shared as is:** `Integer` and `Long` keys
never match in a `StructLikeMap`.
- **What sharing would need:** key conversion. For example:
- keep the first writer's key type as canonical
- give writers of a promoted type a converting view
- keep wide values that don't fit the narrow type in a separate map
- normalise identity-partition keys the same way
- **Decimal is the easy case:** same-scale decimal promotion keeps
identical `BigDecimal` values, so it would only need the key-type check relaxed.
3. **The equality-field set changes**, through per-record
`DynamicRecord.setEqualityFields`, or identifier fields changed outside the
sink.
- **Why I don't think it can be handled exactly:** the tracker stores
only key → position, so after any change one lookup direction is ambiguous. For
example, after `(id)` → `(id, tenant)`, an update for `(7, B)` projected back
to `id = 7` could retire tenant A's row.
- **Routing makes it worse:** with key-hashed distribution, the old and
new key of a row usually land on different subtasks anyway.
- **The realistic option** seems to be a WARN and a metric when a second
key set opens for the same table within a checkpoint.
4. **With a multi-column key, the key columns change order.**
`EvolveSchemaVisitor` can move columns to follow the input order, and the key
type follows table column order. So a position-based key-type comparison
doesn't match, and the writers don't share.
Cases 1, 2 and 4 could be handled in follow-ups once a sharing fix lands.
Case 3 is probably a documented limitation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
