iamaleksey commented on a change in pull request #1429:
URL: https://github.com/apache/cassandra/pull/1429#discussion_r799581128
##########
File path: src/java/org/apache/cassandra/db/PartitionRangeReadCommand.java
##########
@@ -449,7 +488,42 @@ public ReadCommand deserialize(DataInputPlus in,
throws IOException
{
DataRange range = DataRange.serializer.deserialize(in, version,
metadata);
- return new PartitionRangeReadCommand(isDigest, digestVersion,
acceptsTransient, metadata, nowInSec, columnFilter, rowFilter, limits, range,
index, false);
+ return PartitionRangeReadCommand.create(isDigest, digestVersion,
acceptsTransient, metadata, nowInSec, columnFilter, rowFilter, limits, range,
index, false);
+ }
+ }
+
+ public static class VirtualTablePartitionRangeReadCommand extends
PartitionRangeReadCommand
+ {
+ private VirtualTablePartitionRangeReadCommand(boolean isDigest, int
digestVersion, boolean acceptsTransient, TableMetadata metadata, int nowInSec,
ColumnFilter columnFilter, RowFilter rowFilter, DataLimits limits, DataRange
dataRange, IndexMetadata index, boolean trackWarnings)
Review comment:
Formatting nit: gone a bit long here.
##########
File path: src/java/org/apache/cassandra/db/virtual/AbstractVirtualTable.java
##########
@@ -52,7 +53,9 @@ protected AbstractVirtualTable(TableMetadata metadata)
if (!metadata.isVirtual())
throw new IllegalArgumentException("Cannot instantiate a
non-virtual table");
- this.metadata = metadata;
+ this.metadata = metadata.unbuild()
+ .id(TableId.forSystemTable(metadata.keyspace,
metadata.name))
Review comment:
Would probably be nicer if we gave virtual tables deterministic ids from
the very beginning, via `TableMetadata.Builder#build()`?
##########
File path: src/java/org/apache/cassandra/db/PartitionRangeReadCommand.java
##########
@@ -70,24 +72,61 @@ private PartitionRangeReadCommand(boolean isDigest,
this.dataRange = dataRange;
}
- public static PartitionRangeReadCommand create(TableMetadata metadata,
- int nowInSec,
- ColumnFilter columnFilter,
- RowFilter rowFilter,
- DataLimits limits,
- DataRange dataRange)
+ private static PartitionRangeReadCommand create(boolean isDigest,
+ int digestVersion,
+ boolean acceptsTransient,
+ TableMetadata metadata,
+ int nowInSec,
+ ColumnFilter columnFilter,
+ RowFilter rowFilter,
+ DataLimits limits,
+ DataRange dataRange,
+ IndexMetadata index,
+ boolean trackWarnings)
{
- return new PartitionRangeReadCommand(false,
- 0,
- false,
+ if (metadata.isVirtual())
+ return new VirtualTablePartitionRangeReadCommand(isDigest,
+ digestVersion,
+ acceptsTransient,
+ metadata,
+ nowInSec,
+ columnFilter,
+ rowFilter,
+ limits,
+ dataRange,
+ index,
+ trackWarnings);
Review comment:
I think we generally prefer to wrap multiline statement bodies like that
in curly braces. I don't mind either way, but would prefer to change the style
here and elsewhere across the PR (here and in `SinglePartitionReadCommand`).
--
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]
