absurdfarce commented on code in PR #1952:
URL:
https://github.com/apache/cassandra-java-driver/pull/1952#discussion_r1926361593
##########
core/src/main/java/com/datastax/oss/driver/internal/core/type/codec/VectorCodec.java:
##########
@@ -113,40 +139,63 @@ public CqlVector<SubtypeT> decode(
if (bytes == null || bytes.remaining() == 0) {
return null;
}
+ boolean isVarSized = !subtypeCodec.serializedSize().isPresent();
+ if (isVarSized) {
+ ByteBuffer input = bytes.duplicate();
+ List<SubtypeT> rv = new ArrayList<SubtypeT>(cqlType.getDimensions());
+ for (int i = 0; i < cqlType.getDimensions(); ++i) {
+ int size = VIntCoding.getUnsignedVInt32(input, input.position());
+ input.position(input.position() +
VIntCoding.computeUnsignedVIntSize(size));
Review Comment:
Agree with @joao-r-reis, we should be able to simplify this a bit. I was
able to get VectorCodecTest passing with the following:
```
@Nullable
@Override
public CqlVector<SubtypeT> decode(
@Nullable ByteBuffer bytes, @NonNull ProtocolVersion protocolVersion) {
if (bytes == null || bytes.remaining() == 0) {
return null;
}
// Upfront check for fixed-size types only
subtypeCodec.serializedSize().ifPresent((fixed_size) -> {
if (bytes.remaining() != cqlType.getDimensions() * fixed_size) {
throw new IllegalArgumentException(
String.format(
"Expected elements of uniform size, observed %d
elements with total bytes %d",
cqlType.getDimensions(), bytes.remaining()));
}
});
;
ByteBuffer slice = bytes.slice();
List<SubtypeT> rv = new ArrayList<SubtypeT>(cqlType.getDimensions());
for (int i = 0; i < cqlType.getDimensions(); ++i) {
int size = subtypeCodec.serializedSize().orElseGet(() -> {
int vint_size = VIntCoding.getUnsignedVInt32(slice,
slice.position());
// Side effects! Skip over the bytes that vint encode the size of
this element
slice.position(slice.position() +
VIntCoding.computeUnsignedVIntSize(vint_size));
return vint_size;
});
int originalPosition = slice.position();
slice.limit(originalPosition + size);
rv.add(this.subtypeCodec.decode(slice, protocolVersion));
// Move to the start of the next element
slice.position(originalPosition + size);
// Reset the limit to the end of the buffer
slice.limit(slice.capacity());
}
// if too many elements, throw
if (slice.hasRemaining()) {
throw new IllegalArgumentException(
String.format(
"Too many elements; must provide elements for %d
dimensions",
cqlType.getDimensions()));
}
return CqlVector.newInstance(rv);
}
```
I might be missing an error case or two there but I _think_ that covers
everything.
I don't _love_ the side effects in the orElseGet() Supplier
implementation... but at least they're constrained.
--
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: pr-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
