apache / apache/datafusion

ComposedPhysicalExtensionCodec should not rely on positional encoding

オープン
#24,331 コメント 4 件 リアクション 0 件 担当者 1 名 @imtherealnaska が担当を希望しています GitHub で見る
enhancement
主要言語
Rust
スター
9.3k
フォーク
2.4k
平均マージ
3日 11時間
マージ済み PR(30日)
360

説明

### Is your feature request related to a problem or challenge?

ComposedPhysicalExtensionCodec should identify codecs by stable ID instead of list position

`ComposedPhysicalExtensionCodec` records the position of each codec in the `DataEncoderTuple`:

```rust
struct DataEncoderTuple {
pub encoder_position: u32,
pub blob: Vec,
}

The problem is that if someone adds, removes, or reorders a codec, you get really confusing error messages. For example:
1. A writer with [CodecA, CodecB] encodes protobuf `A` and specifies `encoder_position: 0`
2. A reader with [CodecB, CodecA] decodes the proto using `encoder_position: 0` and errors with "CodecB cannot decode message"
```
It's hard to see that this was caused a breaking protocol change. Since both codecs are present, it doesn't seem like a breaking change, but it is.

### Describe the solution you'd like

Maybe we can introduce an id to look up encoders:
```rust
struct DataEncoderTuple {
// Retained for decoding payloads written by older versions.
pub encoder_position: u32,
pub blob: Vec,
pub codec_id: Option,
}

Decoding would:

1. Use codec_id when present.
2. Return an explicit error containing the unknown identifier when it is not registered.
3. Fall back to encoder_position for legacy payloads.
```

### Describe alternatives you've considered

_No response_

### Additional context

_No response_

コントリビューションガイド

コントリビューションガイドを開く

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。