absurdfarce commented on code in PR #1931:
URL:
https://github.com/apache/cassandra-java-driver/pull/1931#discussion_r1794308525
##########
query-builder/src/main/java/com/datastax/oss/driver/api/querybuilder/select/Select.java:
##########
@@ -146,6 +147,16 @@ default Select orderBy(@NonNull String columnName,
@NonNull ClusteringOrder orde
return orderBy(CqlIdentifier.fromCql(columnName), order);
}
+ /**
+ * Shortcut for {@link #orderByAnnOf(CqlIdentifier, CqlVector)}, adding an
ORDER BY ... ANN OF ...
+ * clause
+ */
+ @NonNull
+ Select orderByAnnOf(@NonNull String columnName, @NonNull CqlVector<? extends
Number> ann);
+
+ /** Adds the ORDER BY ... ANN OF ... clause */
+ @NonNull
+ Select orderByAnnOf(@NonNull CqlIdentifier columnId, @NonNull CqlVector<?
extends Number> ann);
Review Comment:
I think I'm changing my answer on this: we should remove the type bound here
and just make this a `CqlVector<?>`. My rationale goes as follows.
The fact that ANN comparisons only support float vectors actually isn't a
constraint on the underlying Java type used here. In theory _any_ Java type
whose codec will generate a serialized value that _can be understood as a
float_ when the server receives the message would work here. That means we
could pass in a CqlVector of floats, decimals or doubles and still have the
server handle that without issue. I'll also note that there's no common
supertype for Float, BigDecimal or Double (the corresponding Java types based
on the
[docs](https://github.com/apache/cassandra-java-driver/tree/4.x/manual/core#cql-to-java-type-mapping))
other than Number... and we can't use that here because it includes types
other than these three. So there's no meaningful supertype we can use for a
type bound at this point.
A second point: the query builder is built around the notion of generating
CQL based on whatever the user passes in; there's almost no checking whether
types correlate to things the user specified. So, for example, OngoingValues
doesn't have any kind of type bounds around it's various value() methods. I
accept that this isn't _exactly_ the same as the float constraint we're
discussing here... but it _is_ an indication that the query builder largely
doesn't concern itself with type checking with the expectation that the server
will handle that when it receives the query.
Finally, I'll note that avoiding the type bound here at least exposes the
idea (at an API level) of using external/custom types which serialize to CQL
float values by way of their own codecs. Scala users, for example, might want
to support using [Spire](https://typelevel.org/spire/) numerics for their CQL
queries. This actually won't work for now since we [constrain the codec
registry
used](https://github.com/apache/cassandra-java-driver/pull/1931/files#diff-77b4c675f8250e6cac9db1ded659755f046b725a13f30d68dc7aa4a897c78bd1R447)
to the (immutable) default which doesn't include any support for Spire
types... but that's an implementation detail. By avoiding type bounds at an
API level we've left that door open for such a change _without_ having to make
a (potentially breaking) API change and without sacrificing our design
principles elsewhere.
--
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]
