serramatutu opened a new pull request, #981: URL: https://github.com/apache/arrow-go/pull/981
- **Add `Truncate` to `Builder` interface with blanket impl** - **Implement `Truncate` for DictionaryBuilder** - **Implement `Truncate` for REE Builder** - **Implement `Truncate` for FixedSizeListBuilder** - **Implement `Truncate` for ListBuilder** - **Implement `Truncate` for MapBuilder** - **Implement `Truncate` for StructBuilder** - **Implement `Truncate` for Union Builders** - **Add consistency test for `Truncate` for all relevant types** ### Rationale for this change While working on [this change](https://github.com/apache/arrow-go/pull/833), I ran into an issue: when a struct builder runs into an error while decoding one of the columns from JSON, there is no way to "un-append" rows from its sibling rows. The two options are to either (1) `AppendNull()` to all siblings and return the error, which would leave a bad row behind, or (2) eagerly `return err` and leave the builder in a broken state. ``` for i := 0; i < b.schema.NumFields(); i++ { val, ok := keylist[b.schema.Field(i).Name] if !ok { b.fields[i].AppendNull() continue } valDec := json.NewDecoder(bytes.NewReader(val)) valDec.UseNumber() if err := b.fields[i].UnmarshalOne(valDec); err != nil { // TODO: what to do here? We might already have appended to previous siblings (i-1, i-2, ...) } } ``` What I want to do instead is: ``` if err := b.fields[i].UnmarshalOne(valDec); err != nil { for j := 0; j < i; j++ { b.fields[j].Truncate(b.fields[j].Len() - 1) } return err } ``` ### What changes are included in this PR? Add `Truncate(n int)` to the `Builder` interface. This new method will keep the underlying buffers but truncate the arrays to at most length `n`. It keeps `Len()` and `NullN()` sound. Calling `b.Truncate(b.Len() - 1)` is the equivalente of "un-appending" one element. ### Are these changes tested? Yes, I added a `TestBuilderTruncate` that checks that `Truncate` works for all data types. It's worth noting I wanted to call `ValidateFull()` after truncate, but it seems like REE arrays have an issue with `ValidateFull()` that precedes this PR. ### Are there any user-facing changes? Yes, this adds a new method to the public API of `Builder` -- 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]
