lidavidm commented on code in PR #40340:
URL: https://github.com/apache/arrow/pull/40340#discussion_r1547573863
##########
java/vector/src/main/java/org/apache/arrow/vector/AbstractVariableWidthVector.java:
##########
Review Comment:
This should probably be an interface? If you want this to be an abstract
base class, then see if there's any actual logic that can live here (e.g. maybe
setSafe can be implemented in terms of set?)
##########
java/vector/src/main/java/org/apache/arrow/vector/AddOrGetResult.java:
##########
@@ -20,7 +20,7 @@
import org.apache.arrow.util.Preconditions;
/**
- * Tuple class containing a vector and whether is was created.
+ * Tuple class containing a vector and whether is created.
Review Comment:
```suggestion
* Tuple class containing a vector and whether it was created.
```
##########
java/vector/src/main/java/org/apache/arrow/vector/compare/TypeEqualsVisitor.java:
##########
@@ -85,6 +86,11 @@ public Boolean visit(BaseLargeVariableWidthVector left, Void
value) {
return compareField(left.getField(), right.getField());
}
+ @Override
+ public Boolean visit(BaseVariableWidthViewVector left, Void value) {
+ throw new UnsupportedOperationException("View vectors are not supported.");
Review Comment:
Is this really so difficult to implement that we can't include it here?
##########
java/vector/src/main/java/org/apache/arrow/vector/util/Text.java:
##########
@@ -235,9 +235,7 @@ public void set(Text other) {
* @param len the number of bytes of the new string
*/
public void set(byte[] utf8, int start, int len) {
- setCapacity(len, false);
- System.arraycopy(utf8, start, bytes, 0, len);
- this.length = len;
+ super.set(utf8, start, len);
Review Comment:
If we're deferring to super, either this should be `@Override` or we just
shouldn't define this in the first place.
##########
java/vector/src/main/codegen/data/ValueVectorTypes.tdd:
##########
@@ -189,7 +189,9 @@
fields: [{name: "start", type: "int"}, {name: "end", type: "int"},
{name: "buffer", type: "ArrowBuf"}],
minor: [
{ class: "VarBinary" , friendlyType: "byte[]" },
- { class: "VarChar" , friendlyType: "Text" }
+ { class: "VarChar" , friendlyType: "Text" },
+ { class: "ViewVarBinary" , friendlyType: "byte[]" }
+ { class: "ViewVarChar" , friendlyType: "Text" }
Review Comment:
```suggestion
{ class: "ViewVarBinary" , friendlyType: "byte[]" },
{ class: "ViewVarChar" , friendlyType: "Text" }
```
##########
java/vector/src/main/java/org/apache/arrow/vector/TypeLayout.java:
##########
@@ -347,11 +360,23 @@ public Integer visit(Binary type) {
return VARIABLE_WIDTH_BUFFER_COUNT;
}
+ @Override
+ public Integer visit(ArrowType.BinaryView type) {
Review Comment:
This is going to have to be overhauled, since the buffer count is not fixed
anymore.
--
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]
