adelapena commented on a change in pull request #924:
URL: https://github.com/apache/cassandra/pull/924#discussion_r592348731
##########
File path: src/java/org/apache/cassandra/db/marshal/AbstractType.java
##########
@@ -435,11 +435,21 @@ public void writeValue(ByteBuffer value, DataOutputPlus
out) throws IOException
// This assumes that no empty values are passed
public <V> void writeValue(V value, ValueAccessor<V> accessor,
DataOutputPlus out) throws IOException
{
- assert !accessor.isEmpty(value);
- if (valueLengthIfFixed() >= 0)
- accessor.write(value, out);
+ assert !accessor.isEmpty(value) : "bytes should not be empty for type
" + this;
+ int expectedValueLength = valueLengthIfFixed();
+ if (expectedValueLength >= 0)
+ {
+ int actualValueLength = accessor.size(value);
+ if (actualValueLength == expectedValueLength)
+ accessor.write(value, out);
+ else
+ throw new IOException(String.format("Expected exactly %d
bytes, but was %d",
Review comment:
Not sure about whether `AssertionError` or `IOException` makes more
sense, since we seem to be mixing both types of exceptions across the `marshal`
package for similar cases, probably being `AssertionError` more common. If we
decide to use `IOException` maybe we could also use it in the first check for
empty values:
```java
int actualValueLength = accessor.size(value);
if (actualValueLength == 0)
throw new IOException("bytes should not be empty for type " + this);
```
And perhaps we could also use `IOException` in `EmptyValue#writeValue`,
which overrides this method, wdyt?
##########
File path: test/unit/org/apache/cassandra/db/marshal/TypeValidationTest.java
##########
@@ -19,16 +19,19 @@
package org.apache.cassandra.db.marshal;
import org.apache.cassandra.Util;
+import org.apache.cassandra.io.util.DataOutputPlus;
Review comment:
Not related to the changes, but we could fix the license header to be a
block comment instead of JavaDoc, replacing `/**` by `/*` in the very first
line of the file.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
