adelapena commented on code in PR #2655:
URL: https://github.com/apache/cassandra/pull/2655#discussion_r1328606632
##########
src/java/org/apache/cassandra/cql3/SingleColumnRelation.java:
##########
@@ -111,20 +121,6 @@ public Term.Raw getMapKey()
return mapKey;
}
- @Override
Review Comment:
The methods `getMapKey` and `withNonStrictOperator` around this method are
unused. I think they were also unused before the patch. We can probably remove
them.
##########
src/java/org/apache/cassandra/db/marshal/TupleType.java:
##########
@@ -355,9 +365,35 @@ public static <V> V buildValue(ValueAccessor<V> accessor,
V... components)
return result;
}
- public static ByteBuffer buildValue(ByteBuffer... components)
+ @Override
+ public ByteBuffer pack(List<ByteBuffer> components)
+ {
+ return pack(ByteBufferAccessor.instance, components);
+ }
+
+ public ByteBuffer pack(ByteBuffer... components)
+ {
+ return pack(Arrays.asList(components));
+ }
+
+ @Override
+ public List<ByteBuffer> filterSortAndValidateElements(List<ByteBuffer>
buffers)
{
- return buildValue(ByteBufferAccessor.instance, components);
+ if (buffers.size() > size())
+ throw new MarshalException(String.format("Tuple value contained
too many fields (expected %s, got %s)", size(), buffers.size()));
Review Comment:
```suggestion
throw new MarshalException(String.format("Tuple value contained
too many fields (expected %s, got %s)", size(), buffers.size()));
```
```suggestion
throw new MarshalException(String.format("Tuple value contains
too many fields (expected %s, got %s)", size(), buffers.size()));
```
##########
test/unit/org/apache/cassandra/transport/SerDeserTest.java:
##########
@@ -96,7 +94,7 @@ public void collectionSerDeserTest() throws Exception
for (Integer i : l)
lb.add(Int32Type.instance.decompose(i));
- assertEquals(l,
lt.getSerializer().deserialize(CollectionSerializer.pack(lb, lb.size())));
+ assertEquals(l, lt.compose(lt.pack(lb)));
Review Comment:
Since we are here, we can remove the unneeded `throws Exception` at the
method signature.
##########
test/unit/org/apache/cassandra/db/marshal/CompositeAndTupleTypesTest.java:
##########
@@ -113,7 +114,12 @@ public <AT extends AbstractType> void
testSerializationDeserialization(TypeFacto
@Test
public void tuple()
{
- testSerializationDeserialization(TupleType::new,
TupleType::buildValue);
+ testSerializationDeserialization(TupleType::new,
getTupleValueCombiner());
+ }
+
+ private static <V> ValueCombiner<V> getTupleValueCombiner()
+ {
+ return (type, accessor, values) -> ((TupleType) type).pack(accessor,
Arrays.asList(values));
Review Comment:
This is calling the `TupleType.pack` static method. So `ValueCombiner`
doesn't need the type:
```java
interface ValueCombiner<V> { V combine(ValueAccessor<V> accessor, V[]
values); }
...
private static <V> ValueCombiner<V> getTupleValueCombiner()
{
return (accessor, values) -> TupleType.pack(accessor,
Arrays.asList(values));
}
```
--
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]
