alex-ninja commented on a change in pull request #219:
URL: https://github.com/apache/cassandra/pull/219#discussion_r710991104



##########
File path: src/java/org/apache/cassandra/streaming/StreamReceiveTask.java
##########
@@ -53,7 +53,13 @@
     public StreamReceiveTask(StreamSession session, TableId tableId, int 
totalStreams, long totalSize)
     {
         super(session, tableId);
-        this.receiver = 
ColumnFamilyStore.getIfExists(tableId).getStreamManager().createStreamReceiver(session,
 totalStreams);
+        ColumnFamilyStore cfs = ColumnFamilyStore.getIfExists(tableId);
+        if (cfs != null) {
+               this.receiver = 
cfs.getStreamManager().createStreamReceiver(session, 
+                               totalStreams);
+        }else {
+               this.receiver = null;

Review comment:
       This fixed is definitely not enough to guarantee safety. It leads to 
potential NPE in other places in the same class:
   ```
   receiver.discardStream(stream);
   ```
   ```
   receiver.received(stream);
   ```
   
   Maybe it makes sense to add a meaningful error and throw it if cfs is null. 
I guess that would be enough.

##########
File path: src/java/org/apache/cassandra/cql3/statements/AlterViewStatement.java
##########
@@ -62,6 +62,8 @@ public void validate(ClientState state)
             throw new InvalidRequestException("Cannot use ALTER MATERIALIZED 
VIEW on Table");
 
         ViewMetadata current = Schema.instance.getView(keyspace(), 
columnFamily());
+        if (current == null)

Review comment:
       This class looks different now. From the first glance I can see there is 
a null check:
   ```
   if (null == view)
               throw ire("Materialized view '%s.%s' doesn't exist", 
keyspaceName, viewName);
   ```
   Could you please rebase your changes and double check whether the issue 
still actual.

##########
File path: src/java/org/apache/cassandra/repair/consistent/LocalSessions.java
##########
@@ -406,7 +406,9 @@ private void syncTable()
     {
         TableId tid = Schema.instance.getTableMetadata(keyspace, table).id;
         ColumnFamilyStore cfm = 
Schema.instance.getColumnFamilyStoreInstance(tid);
-        cfm.forceBlockingFlush();

Review comment:
       I'm not sure this check is ever required, in fact `keyspace` and `table` 
are hardcoded and always exist:
   ```
   private final String keyspace = SchemaConstants.SYSTEM_KEYSPACE_NAME;
   private final String table = SystemKeyspace.REPAIRS;
   ```




-- 
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]

Reply via email to