You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "adelapena (via GitHub)" <gi...@apache.org> on 2023/06/05 10:49:54 UTC
[GitHub] [cassandra] adelapena opened a new pull request, #2386: CASSANDRA-18566: Avoid unnecessary deserialization of terminal arguments when executing CQL functions
adelapena opened a new pull request, #2386:
URL: https://github.com/apache/cassandra/pull/2386
(no comment)
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] blerer commented on a diff in pull request #2386: CASSANDRA-18566: Avoid unnecessary deserialization of terminal arguments when executing CQL functions
Posted by "blerer (via GitHub)" <gi...@apache.org>.
blerer commented on code in PR #2386:
URL: https://github.com/apache/cassandra/pull/2386#discussion_r1221709655
##########
src/java/org/apache/cassandra/cql3/selection/AbstractFunctionSelector.java:
##########
@@ -74,19 +78,27 @@ protected Selector deserialize(DataInputPlus in, int version, TableMetadata meta
.collect(joining(", "))));
boolean isPartial = in.readBoolean();
+ // if the function is partial we need to retrieve the resolved parameters.
Review Comment:
We should may be replace `parameters` by `arguments`.
##########
src/java/org/apache/cassandra/cql3/functions/ScalarFunction.java:
##########
@@ -66,19 +65,19 @@ public default boolean isMonotonic()
* </pre>
* and such that for any value of {@code b} and {@code d}, {@code bar(b, d) == foo(3, b, 'bar', d)}.
*
- * @param protocolVersion protocol version used for parameters
- * @param partialParameters a list of input parameters for the function where some parameters can be {@link #UNRESOLVED}.
- * The input <b>must</b> be of size {@code this.argsType().size()}. For convenience, it is
- * allowed both to pass a list with all parameters being {@link #UNRESOLVED} (the function is
- * then returned directly) and with none of them unresolved (in which case, if the function is pure,
- * it is computed and a dummy no-arg function returning the result is returned).
+ * @param protocolVersion protocol version used for arguments
+ * @param partialArguments a list of input arguments for the function where some arguments can be {@link #UNRESOLVED}.
+ * The input <b>must</b> be of size {@code this.argsType().size()}. For convenience, it is
+ * allowed both to pass a list with all parameters being {@link #UNRESOLVED} (the function is
+ * then returned directly) and with none of them unresolved (in which case, if the function is pure,
+ * it is computed and a dummy no-arg function returning the result is returned).
* @return a function corresponding to the partial application of this function to the parameters of
- * {@code partialParameters} that are not {@link #UNRESOLVED}.
+ * {@code partialArguments} that are not {@link #UNRESOLVED}.
*/
- public default ScalarFunction partialApplication(ProtocolVersion protocolVersion, List<ByteBuffer> partialParameters)
+ default ScalarFunction partialApplication(ProtocolVersion protocolVersion, List<ByteBuffer> partialArguments)
{
int unresolvedCount = 0;
- for (ByteBuffer parameter : partialParameters)
+ for (ByteBuffer parameter : partialArguments)
Review Comment:
It is a minor thing but we should probably use argument instead of parameter to be consistent.
##########
src/java/org/apache/cassandra/cql3/functions/ToJsonFct.java:
##########
@@ -54,14 +54,28 @@ private ToJsonFct(String name, AbstractType<?> argType)
super(name, UTF8Type.instance, argType);
}
- public ByteBuffer execute(ProtocolVersion protocolVersion, List<ByteBuffer> parameters) throws InvalidRequestException
+ @Override
+ public Arguments newArguments(ProtocolVersion version)
{
- assert parameters.size() == 1 : format("Expected 1 argument for %s(), but got %d", name.name, parameters.size());
- ByteBuffer parameter = parameters.get(0);
- if (parameter == null)
+ return new FunctionArguments(version, (protocolVersion, buffer) -> {
+ AbstractType<?> argType = argTypes.get(0);
+
+ if (buffer == null || (!buffer.hasRemaining() && argType.isEmptyValueMeaningless()))
+ return null;
+
+ return argTypes.get(0).toJSONString(buffer, protocolVersion);
+ });
+ }
+
+ @Override
+ public ByteBuffer execute(Arguments arguments) throws InvalidRequestException
+ {
+ assert arguments.size() == 1 : "Expected 1 argument for toJson(), but got " + arguments.size();
Review Comment:
Is it possible?
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adelapena commented on a diff in pull request #2386: CASSANDRA-18566: Avoid unnecessary deserialization of terminal arguments when executing CQL functions
Posted by "adelapena (via GitHub)" <gi...@apache.org>.
adelapena commented on code in PR #2386:
URL: https://github.com/apache/cassandra/pull/2386#discussion_r1222889014
##########
src/java/org/apache/cassandra/cql3/functions/ScalarFunction.java:
##########
@@ -66,19 +65,19 @@ public default boolean isMonotonic()
* </pre>
* and such that for any value of {@code b} and {@code d}, {@code bar(b, d) == foo(3, b, 'bar', d)}.
*
- * @param protocolVersion protocol version used for parameters
- * @param partialParameters a list of input parameters for the function where some parameters can be {@link #UNRESOLVED}.
- * The input <b>must</b> be of size {@code this.argsType().size()}. For convenience, it is
- * allowed both to pass a list with all parameters being {@link #UNRESOLVED} (the function is
- * then returned directly) and with none of them unresolved (in which case, if the function is pure,
- * it is computed and a dummy no-arg function returning the result is returned).
+ * @param protocolVersion protocol version used for arguments
+ * @param partialArguments a list of input arguments for the function where some arguments can be {@link #UNRESOLVED}.
+ * The input <b>must</b> be of size {@code this.argsType().size()}. For convenience, it is
+ * allowed both to pass a list with all parameters being {@link #UNRESOLVED} (the function is
+ * then returned directly) and with none of them unresolved (in which case, if the function is pure,
+ * it is computed and a dummy no-arg function returning the result is returned).
* @return a function corresponding to the partial application of this function to the parameters of
- * {@code partialParameters} that are not {@link #UNRESOLVED}.
+ * {@code partialArguments} that are not {@link #UNRESOLVED}.
*/
- public default ScalarFunction partialApplication(ProtocolVersion protocolVersion, List<ByteBuffer> partialParameters)
+ default ScalarFunction partialApplication(ProtocolVersion protocolVersion, List<ByteBuffer> partialArguments)
{
int unresolvedCount = 0;
- for (ByteBuffer parameter : partialParameters)
+ for (ByteBuffer parameter : partialArguments)
Review Comment:
Right, changed.
##########
src/java/org/apache/cassandra/cql3/selection/AbstractFunctionSelector.java:
##########
@@ -74,19 +78,27 @@ protected Selector deserialize(DataInputPlus in, int version, TableMetadata meta
.collect(joining(", "))));
boolean isPartial = in.readBoolean();
+ // if the function is partial we need to retrieve the resolved parameters.
Review Comment:
Indeed we should, done.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adelapena closed pull request #2386: CASSANDRA-18566: Avoid unnecessary deserialization of terminal arguments when executing CQL functions
Posted by "adelapena (via GitHub)" <gi...@apache.org>.
adelapena closed pull request #2386: CASSANDRA-18566: Avoid unnecessary deserialization of terminal arguments when executing CQL functions
URL: https://github.com/apache/cassandra/pull/2386
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adelapena commented on a diff in pull request #2386: CASSANDRA-18566: Avoid unnecessary deserialization of terminal arguments when executing CQL functions
Posted by "adelapena (via GitHub)" <gi...@apache.org>.
adelapena commented on code in PR #2386:
URL: https://github.com/apache/cassandra/pull/2386#discussion_r1222888226
##########
src/java/org/apache/cassandra/cql3/functions/ToJsonFct.java:
##########
@@ -54,14 +54,28 @@ private ToJsonFct(String name, AbstractType<?> argType)
super(name, UTF8Type.instance, argType);
}
- public ByteBuffer execute(ProtocolVersion protocolVersion, List<ByteBuffer> parameters) throws InvalidRequestException
+ @Override
+ public Arguments newArguments(ProtocolVersion version)
{
- assert parameters.size() == 1 : format("Expected 1 argument for %s(), but got %d", name.name, parameters.size());
- ByteBuffer parameter = parameters.get(0);
- if (parameter == null)
+ return new FunctionArguments(version, (protocolVersion, buffer) -> {
+ AbstractType<?> argType = argTypes.get(0);
+
+ if (buffer == null || (!buffer.hasRemaining() && argType.isEmptyValueMeaningless()))
+ return null;
+
+ return argTypes.get(0).toJSONString(buffer, protocolVersion);
+ });
+ }
+
+ @Override
+ public ByteBuffer execute(Arguments arguments) throws InvalidRequestException
+ {
+ assert arguments.size() == 1 : "Expected 1 argument for toJson(), but got " + arguments.size();
Review Comment:
It isn't, the factory should have caught it before. Removed.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adelapena commented on pull request #2386: CASSANDRA-18566: Avoid unnecessary deserialization of terminal arguments when executing CQL functions
Posted by "adelapena (via GitHub)" <gi...@apache.org>.
adelapena commented on PR #2386:
URL: https://github.com/apache/cassandra/pull/2386#issuecomment-1582388272
> It seems that the getArgumentDeserializer method is missing for: DoubleType, DecimalType, Int32Type and IntegerType.
Good catch; added.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org