lidavidm commented on code in PR #697: URL: https://github.com/apache/arrow-java/pull/697#discussion_r2032494847
########## vector/src/test/java/org/apache/arrow/vector/complex/impl/UuidWriterImpl.java: ########## @@ -0,0 +1,49 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.arrow.vector.complex.impl; + +import java.nio.ByteBuffer; +import java.util.UUID; +import org.apache.arrow.vector.UuidVector; +import org.apache.arrow.vector.holder.UuidHolder; +import org.apache.arrow.vector.holders.ExtensionHolder; + +public class UuidWriterImpl extends AbstractExtensionTypeWriter { Review Comment: ```suggestion public class UuidWriterImpl extends AbstractExtensionTypeWriter<UuidVector> { ``` ########## vector/src/test/java/org/apache/arrow/vector/complex/impl/UuidWriterImpl.java: ########## @@ -0,0 +1,49 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.arrow.vector.complex.impl; + +import java.nio.ByteBuffer; +import java.util.UUID; +import org.apache.arrow.vector.UuidVector; +import org.apache.arrow.vector.holder.UuidHolder; +import org.apache.arrow.vector.holders.ExtensionHolder; + +public class UuidWriterImpl extends AbstractExtensionTypeWriter { + + public UuidWriterImpl(UuidVector vector) { + super(vector); + } + + @Override + public void writeExtensionType(Object value) { + UUID uuid = (UUID) value; + ByteBuffer bb = ByteBuffer.allocate(16); + bb.putLong(uuid.getMostSignificantBits()); + bb.putLong(uuid.getLeastSignificantBits()); + ((UuidVector) this.vector).setSafe(this.idx(), bb.array()); Review Comment: ```suggestion vector.setSafe(idx(), bb.array()); ``` ########## vector/src/main/java/org/apache/arrow/vector/complex/impl/AbstractExtensionTypeWriter.java: ########## @@ -0,0 +1,71 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.arrow.vector.complex.impl; + +import org.apache.arrow.vector.ExtensionTypeVector; +import org.apache.arrow.vector.types.pojo.Field; + +/** + * Base {@link AbstractFieldWriter} class for an {@link + * org.apache.arrow.vector.ExtensionTypeVector}. + * + * @param <T> a specific {@link ExtensionTypeVector}. + */ +public class AbstractExtensionTypeWriter<T extends ExtensionTypeVector> + extends AbstractFieldWriter { + protected final T vector; + + public AbstractExtensionTypeWriter(T vector) { + this.vector = vector; + } + + @Override + public Field getField() { + return this.vector.getField(); + } + + @Override + public int getValueCapacity() { + return this.vector.getValueCapacity(); + } + + @Override + public void allocate() { + this.vector.allocateNew(); + } + + @Override + public void close() { + this.vector.close(); + } + + @Override + public void clear() { + this.vector.clear(); + } + + @Override + protected int idx() { + return super.idx(); Review Comment: Hmm, is the only reason we override here to make it `protected` instead of package-private? (Should we just use `getPosition()` instead as it is already public and does the same thing?) ########## vector/src/main/codegen/templates/BaseWriter.java: ########## @@ -101,6 +103,35 @@ public interface MapWriter extends ListWriter { MapWriter value(); } + public interface ExtensionWriter extends BaseWriter { + + /** + * Writes a null value. + */ + void writeNull(); + + /** + * Writes vlaue from the given extension holder. Review Comment: ```suggestion * Writes value from the given extension holder. ``` ########## vector/src/main/codegen/templates/BaseWriter.java: ########## @@ -101,6 +103,35 @@ public interface MapWriter extends ListWriter { MapWriter value(); } + public interface ExtensionWriter extends BaseWriter { + + /** + * Writes a null value. + */ + void writeNull(); + + /** + * Writes vlaue from the given extension holder. + * + * @param holder the extension holder to write + */ + void write(ExtensionHolder holder); + + /** + * Writes the given extension type value. + * + * @param value the extension type value to write + */ + void writeExtensionType(Object value); + + /** + * Adds the given extension type factory. This factory allows configuring writer implementations for specific ExtensionTypeVector. + * + * @param factory the extension type factory to add + */ + void addExtensionTypeFactory(ExtensionTypeWriterFactory factory); Review Comment: ```suggestion void addExtensionTypeWriterFactory(ExtensionTypeWriterFactory factory); ``` for consistency as well ########## vector/src/test/java/org/apache/arrow/vector/complex/impl/UuidWriterImpl.java: ########## @@ -0,0 +1,49 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.arrow.vector.complex.impl; + +import java.nio.ByteBuffer; +import java.util.UUID; +import org.apache.arrow.vector.UuidVector; +import org.apache.arrow.vector.holder.UuidHolder; +import org.apache.arrow.vector.holders.ExtensionHolder; + +public class UuidWriterImpl extends AbstractExtensionTypeWriter { + + public UuidWriterImpl(UuidVector vector) { + super(vector); + } + + @Override + public void writeExtensionType(Object value) { + UUID uuid = (UUID) value; + ByteBuffer bb = ByteBuffer.allocate(16); + bb.putLong(uuid.getMostSignificantBits()); + bb.putLong(uuid.getLeastSignificantBits()); + ((UuidVector) this.vector).setSafe(this.idx(), bb.array()); + this.vector.setValueCount(this.idx() + 1); + } + + @Override + public void write(ExtensionHolder holder) { + if (holder instanceof UuidHolder) { Review Comment: Either delete the check to be consistent with writeExtensionType or explicitly error if the holder is the wrong type ########## vector/src/main/codegen/templates/BaseWriter.java: ########## @@ -101,6 +103,35 @@ public interface MapWriter extends ListWriter { MapWriter value(); } + public interface ExtensionWriter extends BaseWriter { + + /** + * Writes a null value. + */ + void writeNull(); + + /** + * Writes vlaue from the given extension holder. + * + * @param holder the extension holder to write + */ + void write(ExtensionHolder holder); + + /** + * Writes the given extension type value. + * + * @param value the extension type value to write + */ + void writeExtensionType(Object value); Review Comment: This is a nit but now I'm thinking it should be just `writeExtension` to parallel `writeVarChar` ########## vector/src/test/java/org/apache/arrow/vector/complex/impl/UuidWriterImpl.java: ########## @@ -0,0 +1,49 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.arrow.vector.complex.impl; + +import java.nio.ByteBuffer; +import java.util.UUID; +import org.apache.arrow.vector.UuidVector; +import org.apache.arrow.vector.holder.UuidHolder; +import org.apache.arrow.vector.holders.ExtensionHolder; + +public class UuidWriterImpl extends AbstractExtensionTypeWriter { + + public UuidWriterImpl(UuidVector vector) { + super(vector); + } + + @Override + public void writeExtensionType(Object value) { + UUID uuid = (UUID) value; + ByteBuffer bb = ByteBuffer.allocate(16); + bb.putLong(uuid.getMostSignificantBits()); + bb.putLong(uuid.getLeastSignificantBits()); + ((UuidVector) this.vector).setSafe(this.idx(), bb.array()); + this.vector.setValueCount(this.idx() + 1); + } + + @Override + public void write(ExtensionHolder holder) { + if (holder instanceof UuidHolder) { + UuidHolder uuidHolder = (UuidHolder) holder; + ((UuidVector) this.vector).setSafe(this.idx(), uuidHolder.value); + this.vector.setValueCount(this.idx() + 1); Review Comment: ```suggestion vector.setSafe(idx(), uuidHolder.value); vector.setValueCount(idx() + 1); ``` -- 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]
