dcapwell commented on code in PR #1962:
URL: https://github.com/apache/cassandra/pull/1962#discussion_r1038624365
##########
src/java/org/apache/cassandra/cql3/Constants.java:
##########
@@ -518,19 +517,39 @@ public Substracter(ColumnMetadata column, Term t)
super(column, t);
}
+ public boolean requiresRead()
+ {
+ return !(column.type instanceof CounterColumnType);
+ }
+
public void execute(DecoratedKey partitionKey, UpdateParameters
params) throws InvalidRequestException
{
- ByteBuffer bytes = t.bindAndGet(params.options);
- if (bytes == null)
- throw new InvalidRequestException("Invalid null value for
counter increment");
- if (bytes == ByteBufferUtil.UNSET_BYTE_BUFFER)
- return;
+ if (column.type instanceof CounterColumnType)
+ {
+ ByteBuffer bytes = t.bindAndGet(params.options);
+ if (bytes == null)
+ throw new InvalidRequestException("Invalid null value for
counter increment");
+ if (bytes == ByteBufferUtil.UNSET_BYTE_BUFFER)
+ return;
- long increment = ByteBufferUtil.toLong(bytes);
- if (increment == Long.MIN_VALUE)
- throw new InvalidRequestException("The negation of " +
increment + " overflows supported counter precision (signed 8 bytes integer)");
+ long increment = ByteBufferUtil.toLong(bytes);
+ if (increment == Long.MIN_VALUE)
+ throw new InvalidRequestException("The negation of " +
increment + " overflows supported counter precision (signed 8 bytes integer)");
- params.addCounter(column, -increment);
+ params.addCounter(column, -increment);
+ }
+ else if (column.type instanceof NumberType)
+ {
+ NumberType<?> type = (NumberType<?>) column.type;
+ ByteBuffer bytes = t.bindAndGet(params.options);
+ if (bytes == null)
+ throw new InvalidRequestException("Invalid null value for
number increment");
Review Comment:
interesting that Subtracter does this but not Adder...
##########
src/java/org/apache/cassandra/cql3/Operation.java:
##########
@@ -365,17 +389,23 @@ public Substraction(Term.Raw value)
}
public Operation prepare(TableMetadata metadata, ColumnMetadata
receiver, boolean canReadExistingState) throws InvalidRequestException
- {
+ {
if (!(receiver.type instanceof CollectionType))
{
- if (!(receiver.type instanceof CounterColumnType))
+ if (canReadExistingState)
Review Comment:
NOTE: `ParsedInsertJson` is missing checking for txn... need to make sure
its part of the TODO
##########
src/java/org/apache/cassandra/cql3/Operation.java:
##########
@@ -386,7 +416,7 @@ else if (!(receiver.type.isMultiCell()))
ColumnSpecification vr = new
ColumnSpecification(receiver.ksName,
receiver.cfName,
receiver.name,
-
SetType.getInstance(((MapType)receiver.type).getKeysType(), false));
+
SetType.getInstance(((MapType<?, ?>) receiver.type).getKeysType(), true));
Review Comment:
why change from `false` to `true`? it doesn't look to matter in this case?
##########
src/java/org/apache/cassandra/cql3/selection/Selection.java:
##########
@@ -126,6 +126,23 @@ public ResultSet.ResultMetadata getResultMetadata()
return resultMetadata;
}
+ public static Selection.Selectors noopSelector()
+ {
+ return new Selectors()
Review Comment:
im wondering if we can merge this with
`org.apache.cassandra.cql3.selection.Selection.SimpleSelection#newSelectors` as
its mostly overlaps... this would also make sure `getOutputRow` handles JSON
where as this doesn't
##########
src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java:
##########
@@ -1044,4 +1121,23 @@ public List<Pair<ColumnIdentifier, ColumnCondition.Raw>>
getConditions()
return conditions;
}
}
+
+ private static final Constants.Value ONE = new
Constants.Value(ByteBufferUtil.bytes(1));
+
+ public SelectStatement createSelectForTxn()
+ {
+ // TODO: get working with static-only updates that don't specify
any/all primary key columns
+
Preconditions.checkState(getRestrictions().hasAllPKColumnsRestrictedByEqualities());
+ Selection selection = Selection.forColumns(metadata,
Lists.newArrayList(requiresRead), false);
Review Comment:
`requiresRead` is computed in the constructor, which is before migration...
but if a Substitutions already exists, then won't that column be missing?
`org.apache.cassandra.cql3.statements.UpdateStatement.ParsedInsert#prepareInternal`
moves them there first.
##########
src/java/org/apache/cassandra/cql3/Operation.java:
##########
@@ -365,17 +389,23 @@ public Substraction(Term.Raw value)
}
public Operation prepare(TableMetadata metadata, ColumnMetadata
receiver, boolean canReadExistingState) throws InvalidRequestException
- {
+ {
if (!(receiver.type instanceof CollectionType))
{
- if (!(receiver.type instanceof CounterColumnType))
+ if (canReadExistingState)
+ {
+ if (!(receiver.type instanceof NumberType))
Review Comment:
should merge the 2 if statements for now, can split if we ever add a new type
##########
src/java/org/apache/cassandra/cql3/Operations.java:
##########
@@ -47,11 +49,34 @@ public final class Operations implements Iterable<Operation>
*/
private final List<Operation> staticOperations = new ArrayList<>();
+ private final List<ReferenceOperation> regularSubstitutions = new
ArrayList<>();
+ private final List<ReferenceOperation> staticSubstitutions = new
ArrayList<>();
Review Comment:
We should document why these are not included in `requiresRead`, the reason
is
`org.apache.cassandra.cql3.statements.ModificationStatement#readRequiredLists`
would then perform a read which we need to avoid.
##########
src/java/org/apache/cassandra/cql3/selection/Selection.java:
##########
@@ -126,6 +126,23 @@ public ResultSet.ResultMetadata getResultMetadata()
return resultMetadata;
}
+ public static Selection.Selectors noopSelector()
+ {
+ return new Selectors()
+ {
+ List<ByteBuffer> current;
+ @Override public ColumnFilter getColumnFilter() { return
ColumnFilter.NONE; }
+ @Override public boolean hasProcessing() { return false; }
+ @Override public boolean isAggregate() { return false; }
+ @Override public int numberOfFetchedColumns() { return 0; }
Review Comment:
TODO confirm `0`0 is correct...
--
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]
