adelapena commented on code in PR #2310:
URL: https://github.com/apache/cassandra/pull/2310#discussion_r1221898559
##########
src/java/org/apache/cassandra/db/marshal/AbstractType.java:
##########
@@ -478,6 +493,21 @@ public final boolean isValueLengthFixed()
return valueLengthIfFixed() != VARIABLE_LENGTH;
}
+ public boolean allowsEmpty()
Review Comment:
This method could get some JavaDoc
##########
src/java/org/apache/cassandra/serializers/TypeSerializer.java:
##########
@@ -55,11 +63,45 @@ public final void validate(ByteBuffer bytes) throws
MarshalException
public abstract Class<T> getType();
- public String toCQLLiteral(ByteBuffer buffer)
+ protected String toCQLLiteralNonNull(@Nonnull ByteBuffer buffer)
Review Comment:
We could move this method down so it stays together with the other
`toCQLLiteral*` methods.
##########
src/java/org/apache/cassandra/db/marshal/UserType.java:
##########
@@ -293,6 +293,12 @@ public UserType freeze()
return this;
}
+ @Override
+ public UserType unfreeze()
+ {
+ return isMultiCell ? this : new UserType(keyspace, name, fieldNames,
fieldTypes(), true);
Review Comment:
Nit: can we change the `freeze` method right above to use the ternary
operator, for better symmetry between `freeze` and `unfreeze`? The same would
apply to `SetType`, `ListType` and `MapType`.
##########
src/java/org/apache/cassandra/db/marshal/DynamicCompositeType.java:
##########
@@ -68,13 +72,40 @@
*/
public class DynamicCompositeType extends AbstractCompositeType
{
+ public static class Serializer extends BytesSerializer
+ {
+ // aliases are held to make sure the serializer is unique for each
collection of types, this is to make sure it's
+ // safe to cache in all cases
+ private final Map<Byte, AbstractType<?>> aliases;
+
+ public Serializer(Map<Byte, AbstractType<?>> aliases)
+ {
+ this.aliases = aliases;
+ }
+
+ @Override
+ public boolean equals(Object o)
+ {
+ if (this == o) return true;
+ if (o == null || getClass() != o.getClass()) return false;
+ Serializer that = (Serializer) o;
+ return aliases.equals(that.aliases);
+ }
+
+ @Override
+ public int hashCode()
+ {
+ return Objects.hash(aliases);
+ }
+ }
private static final Logger logger =
LoggerFactory.getLogger(DynamicCompositeType.class);
private static final ByteSource[] EMPTY_BYTE_SOURCE_ARRAY = new
ByteSource[0];
private static final String REVERSED_TYPE =
ReversedType.class.getSimpleName();
- private final Map<Byte, AbstractType<?>> aliases;
+ public final Map<Byte, AbstractType<?>> aliases;
Review Comment:
Nit: This could use `@VisibleForTesting`
##########
src/java/org/apache/cassandra/db/marshal/ValueAccessor.java:
##########
@@ -311,6 +319,24 @@ default boolean getBoolean(V value, int offset)
int toInt(V value);
/** returns an int from offset {@param offset} */
int getInt(V value, int offset);
+ default long getUnsignedVInt(V value, int offset)
+ {
+ return VIntCoding.getUnsignedVInt(value, this, offset);
+ }
+ default int getUnsignedVInt32(V value, int offset)
+ {
+ return VIntCoding.getUnsignedVInt32(value, this, offset);
+ }
+ default long getVInt(V value, int offset)
+ {
+ return VIntCoding.getVInt(value, this, offset);
+ }
+ default int getVInt32(V value, int offset)
+ {
+ return VIntCoding.getVInt32(value, this, offset);
+ }
Review Comment:
Of these new methods, only `getUnsignedVInt32` seems used. Also, I'd
separate them with blank lines.
##########
src/java/org/apache/cassandra/db/marshal/ValueAccessor.java:
##########
@@ -311,6 +319,24 @@ default boolean getBoolean(V value, int offset)
int toInt(V value);
/** returns an int from offset {@param offset} */
int getInt(V value, int offset);
+ default long getUnsignedVInt(V value, int offset)
+ {
+ return VIntCoding.getUnsignedVInt(value, this, offset);
+ }
+ default int getUnsignedVInt32(V value, int offset)
+ {
+ return VIntCoding.getUnsignedVInt32(value, this, offset);
+ }
+ default long getVInt(V value, int offset)
+ {
+ return VIntCoding.getVInt(value, this, offset);
+ }
+ default int getVInt32(V value, int offset)
+ {
+ return VIntCoding.getVInt32(value, this, offset);
+ }
Review Comment:
Conversely, for the new "put" methods, only `putUnsignedVInt32` seems used.
##########
src/java/org/apache/cassandra/serializers/TypeSerializer.java:
##########
@@ -55,11 +63,45 @@ public final void validate(ByteBuffer bytes) throws
MarshalException
public abstract Class<T> getType();
- public String toCQLLiteral(ByteBuffer buffer)
+ protected String toCQLLiteralNonNull(@Nonnull ByteBuffer buffer)
+ {
+ return toString(deserialize(buffer));
+ }
+
+ public final boolean isNull(@Nullable ByteBuffer buffer)
+ {
+ return isNull(buffer, ByteBufferAccessor.instance);
+ }
+
+ public <V> boolean isNull(@Nullable V buffer, ValueAccessor<V> accessor)
+ {
+ return buffer == null || accessor.isEmpty(buffer);
+ }
+
+ public final @Nonnull String toCQLLiteral(@Nullable ByteBuffer buffer)
{
- return buffer == null || !buffer.hasRemaining()
+ return isNull(buffer)
? "null"
- : toString(deserialize(buffer));
+ : maybeQuote(toCQLLiteralNonNull(buffer));
+ }
+
+ public final @Nonnull String toCQLLiteralNoQuote(@Nullable ByteBuffer
buffer)
+ {
+ return isNull(buffer)
+ ? "null"
+ : toCQLLiteralNonNull(buffer);
+ }
+
+ public boolean shouldQuoteCQL()
Review Comment:
Maybe we could call this `shouldQuoteCQLLiterals`.
##########
src/java/org/apache/cassandra/utils/vint/VIntCoding.java:
##########
@@ -134,6 +135,11 @@ public static int getVInt32(ByteBuffer input, int
readerIndex)
return checkedCast(decodeZigZag64(getUnsignedVInt(input,
readerIndex)));
}
+ public static long getVInt(ByteBuffer input, int readerIndex)
Review Comment:
This method seems unused
--
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]
