rdblue commented on code in PR #3212:
URL: https://github.com/apache/parquet-java/pull/3212#discussion_r2085656475
##########
parquet-variant/src/main/java/org/apache/parquet/variant/VariantArrayBuilder.java:
##########
@@ -22,15 +22,13 @@
* Builder for creating Variant arrays, used by VariantBuilder.
*/
public class VariantArrayBuilder extends VariantBuilder {
- /** The parent VariantBuilder. */
- private final VariantBuilder parent;
/** The offsets of the elements in this array. */
private final ArrayList<Integer> offsets;
/** The number of values appended to this array. */
protected long numValues = 0;
- VariantArrayBuilder(VariantBuilder parent) {
- this.parent = parent;
+ VariantArrayBuilder(VariantBuilder rootBuilder) {
Review Comment:
As long as we are changing the structure, would it make sense to separate
out the metadata?
Right now, we assume that the builder can add keys. But that's a very simple
interface with just `int addDictionaryKey(String)`. And we also have to add a
boolean for fixed metadata and a new `setFixedMetadata` method. It seems like
it may be easier to change the builder so that it accepts some
`MetadataBuilder`:
```java
interface Metadata {
int resolve(String key); // maybe a better name?
ByteBuffer getEncodedBuffer();
}
```
Then you could pass fixed metadata using a simple wrapper class,
`EncodedMetadata` that throws an exception if you're resolving a string that is
missing. And when building metadata from scratch, `resolve` would assign the
new ID and produce encoded metadata when `getEncodedBuffer` is called.
I think that would be simpler than having so much metadata-related state in
the builder itself.
--
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]
