maedhroz commented on code in PR #2645:
URL: https://github.com/apache/cassandra/pull/2645#discussion_r1312141024
##########
src/java/org/apache/cassandra/index/sai/utils/PrimaryKey.java:
##########
@@ -145,172 +208,226 @@ public boolean equals(Object obj)
}
@Override
- @SuppressWarnings("ConstantConditions")
public String toString()
{
- return isTokenOnly() ? String.format("PrimaryKey: { token: %s
}", token())
- : String.format("PrimaryKey: { token: %s,
partition: %s, clustering: %s:%s } ",
- token(),
- partitionKey(),
- clustering() == null ?
null : clustering().kind(),
- clustering() == null ?
null : Arrays.stream(clustering().getBufferArray())
-
.map(ByteBufferUtil::bytesToHex)
-
.collect(Collectors.joining(", ")));
+ return String.format("PrimaryKey: { token: %s }", token());
}
}
- class TokenOnlyPrimaryKey extends AbstractPrimaryKey
+ class SkinnyPrimaryKey extends TokenOnlyPrimaryKey
{
- private final Token token;
+ protected final DecoratedKey partitionKey;
- TokenOnlyPrimaryKey(Token token)
+ SkinnyPrimaryKey(DecoratedKey partitionKey)
{
- this.token = token;
+ super(partitionKey.getToken());
+ this.partitionKey = partitionKey;
}
@Override
- public boolean isTokenOnly()
+ public Kind kind()
{
- return true;
+ return Kind.Skinny;
}
@Override
- public Token token()
+ public DecoratedKey partitionKey()
{
- return token;
+ return partitionKey;
}
@Override
- public DecoratedKey partitionKey()
+ public ByteSource asComparableBytes(Version version)
{
- throw new UnsupportedOperationException();
+ return ByteSource.of(partitionKey().getKey(), version);
}
@Override
- public Clustering<?> clustering()
+ public int compareTo(PrimaryKey o)
{
- throw new UnsupportedOperationException();
+ int cmp = super.compareTo(o);
+
+ // If the tokens don't match then we don't need to compare any
more of the key.
+ // Otherwise, if the other key is token only we can only
compare tokens
+ if ((cmp != 0) || o.kind() == Kind.Token)
+ return cmp;
+
+ // Finally compare the partition keys
+ return partitionKey().compareTo(o.partitionKey());
}
@Override
- public ByteSource asComparableBytes(Version version)
+ public int hashCode()
{
- throw new UnsupportedOperationException();
+ return Objects.hash(token(), partitionKey(), 64,
clusteringComparator);
+ }
+
+ @Override
+ public String toString()
+ {
+ return String.format("PrimaryKey: { token: %s, partition: %s
}", token(), partitionKey());
}
}
- class ImmutablePrimaryKey extends AbstractPrimaryKey
+ class StaticPrimaryKey extends SkinnyPrimaryKey
{
- private final Token token;
- private final DecoratedKey partitionKey;
- private final Clustering<?> clustering;
+ StaticPrimaryKey(DecoratedKey partitionKey)
+ {
+ super(partitionKey);
+ }
- ImmutablePrimaryKey(DecoratedKey partitionKey, Clustering<?>
clustering)
+ @Override
+ public Kind kind()
{
- this.token = partitionKey.getToken();
- this.partitionKey = partitionKey;
- this.clustering = clustering;
+ return Kind.Static;
}
@Override
- public Token token()
+ public Clustering<?> clustering()
{
- return token;
+ return Clustering.STATIC_CLUSTERING;
}
@Override
- public DecoratedKey partitionKey()
+ public ByteSource asComparableBytes(ByteComparable.Version version)
{
- return partitionKey;
+ ByteSource keyComparable =
ByteSource.of(partitionKey().getKey(), version);
+ // Static clustering cannot be serialized or made to a byte
comparable, so we use null as the component.
+ return ByteSource.withTerminator(version ==
ByteComparable.Version.LEGACY ? ByteSource.END_OF_STREAM
+
: ByteSource.TERMINATOR,
+ keyComparable,
+ null);
}
@Override
- public Clustering<?> clustering()
+ public int compareTo(PrimaryKey o)
{
- return clustering;
+ int cmp = super.compareTo(o);
+ if (cmp != 0 || o.kind() == Kind.Token || o.kind() ==
Kind.Skinny)
+ return cmp;
+ // The static clustering comes first in the sort order of if
the other key has static clustering we
+ // are equals otherwise we are less than the other
+ return o.kind() == Kind.Static ? 0 : -1;
+ }
+
+ @Override
+ public int hashCode()
+ {
+ return Objects.hash(token(), partitionKey(),
Clustering.STATIC_CLUSTERING, clusteringComparator);
+ }
+
+ @Override
+ public String toString()
+ {
+ return String.format("PrimaryKey: { token: %s, partition: %s,
clustering: STATIC } ", token(), partitionKey());
}
}
- class MutablePrimaryKey extends AbstractPrimaryKey
+ class WidePrimaryKey extends SkinnyPrimaryKey
{
- private final Token token;
- private final Supplier<PrimaryKey> primaryKeySupplier;
+ private final Clustering<?> clustering;
- private boolean notLoaded = true;
- private DecoratedKey partitionKey;
- private Clustering<?> clustering;
+ WidePrimaryKey(DecoratedKey partitionKey, Clustering<?> clustering)
+ {
+ super(partitionKey);
+ this.clustering = clustering;
+ }
- MutablePrimaryKey(Token token, Supplier<PrimaryKey>
primaryKeySupplier)
+ @Override
+ public Kind kind()
{
- this.token = token;
- this.primaryKeySupplier = primaryKeySupplier;
+ return Kind.Wide;
}
@Override
- public Token token()
+ public Clustering<?> clustering()
{
- return token;
+ return clustering;
}
@Override
- public DecoratedKey partitionKey()
+ public ByteSource asComparableBytes(ByteComparable.Version version)
{
- loadDeferred();
- return partitionKey;
+ ByteSource keyComparable =
ByteSource.of(partitionKey().getKey(), version);
+ // It is important that the
ClusteringComparator.asBytesComparable method is used
+ // to maintain the correct clustering sort order.
+ ByteSource clusteringComparable =
clusteringComparator.asByteComparable(clustering()).asComparableBytes(version);
+ return ByteSource.withTerminator(version ==
ByteComparable.Version.LEGACY ? ByteSource.END_OF_STREAM
+
: ByteSource.TERMINATOR,
+ keyComparable,
+ clusteringComparable);
}
@Override
- public Clustering<?> clustering()
+ public int compareTo(PrimaryKey o)
{
- loadDeferred();
- return clustering;
+ int cmp = super.compareTo(o);
+ if (cmp != 0 || o.kind() == Kind.Token || o.kind() ==
Kind.Skinny)
+ return cmp;
+ // At this point we will be greater than other if it is static
+ if (o.kind() == Kind.Static)
+ return 1;
+ return clusteringComparator.compare(clustering(),
o.clustering());
}
- private void loadDeferred()
+ @Override
+ public int hashCode()
{
- if (notLoaded)
- {
- PrimaryKey deferredPrimaryKey = primaryKeySupplier.get();
- this.partitionKey = deferredPrimaryKey.partitionKey();
- this.clustering = deferredPrimaryKey.clustering();
- notLoaded = false;
- }
+ return Objects.hash(token(), partitionKey(), clustering(),
clusteringComparator);
+ }
+
+ @Override
+ public String toString()
+ {
+ return String.format("PrimaryKey: { token: %s, partition: %s,
clustering: %s:%s } ",
+ token(),
+ partitionKey(),
+ clustering().kind(),
+
Arrays.stream(clustering().getBufferArray())
+ .map(ByteBufferUtil::bytesToHex)
+ .collect(Collectors.joining(", ")));
}
}
}
- default boolean isTokenOnly()
- {
- return false;
- }
+ /**
+ * Returns the {@link Kind} of the {@link PrimaryKey}. The {@link Kind} is
used locally in the {@link #compareTo(Object)}
+ * methods to determine how far the comparision needs to go between keys.
+ * <p>
+ * The {@link Kind} values have a categorization of {@code isClustering}.
This indicates whether the key belongs to
+ * a table with clustering tables or not.
+ */
+ Kind kind();
+ /**
+ * Returns the {@link Token} component of the {@link PrimaryKey}
+ */
Token token();
- @Nullable
+ /**
+ * Returns the {@link DecoratedKey} representing the partition key of the
{@link PrimaryKey}.
+ * <p>
+ * Note: This cannot be null but some {@link PrimaryKey} implementations
can throw {@link UnsupportedOperationException}
+ * if they do not support partition keys.
+ */
DecoratedKey partitionKey();
- @Nullable
- Clustering<?> clustering();
-
/**
- * Return whether the primary key has an empty clustering or not.
- * By default, the clustering is empty if the internal clustering
- * is null or is empty.
- *
- * @return {@code true} if the clustering is empty, otherwise {@code false}
+ * Returns the {@link Clustering} representing the clustering component of
the {@link PrimaryKey}.
+ * <p>
+ * Note: This cannot be null but some {@link PrimaryKey} implementations
can throw {@link UnsupportedOperationException}
+ * if they do not support clustering columns.
*/
- @SuppressWarnings("ConstantConditions")
- default boolean hasEmptyClustering()
- {
- return clustering() == null || clustering().isEmpty();
- }
+ Clustering<?> clustering();
/**
* Returns the {@link PrimaryKey} as a {@link ByteSource} byte comparable
representation.
* <p>
* It is important that these representations are only ever used with byte
comparables using
* the same elements. This means that {@code asComparableBytes} responses
can only be used
* together from the same {@link PrimaryKey} implementation.
+ * <p>
+ * Note: This method can throw {@link UnsupportedOperationException} by
some {@link PrimaryKey} implementations.
Review Comment:
nit: Perhaps just use a `@throws` section for this bit?
```
@throws UnsupportedOperationException for {@link PrimaryKey} implementations
that are not byte-comparable
```
--
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]
