[jira] [Commented] (CASSANDRA-13867) Make PartitionUpdate and Mutation immutable

["Aleksey Yeschenko (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CASSANDRA-13867?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16306424#comment-16306424 ] 

Aleksey Yeschenko commented on CASSANDRA-13867:
-----------------------------------------------


Nicely done. This will make the codebase a bit more sane and safe - +1 from me. Have 3 very minor notes and a few nits highlighted by IDEA:

1. {{BatchUpdatesCollector.toMutations()}} creates ands throws away an unnecessary list in each iteration of the loop. Could replace that for-loop body with something like {{ksMap.values().forEach(b -> ms.add(b.build()));}} instead.
2. {{SingleTableUpdatesCollector.toMutations()}} validates indexed columns, but {{BatchUpdatesCollector.toMutations()}} doesn’t. Also, I don’t feel like that validation code completely belongs to {{UpdatesCollector}} - at least the nested calls. Maybe should put {{validateIndexedColumns()}} in {{PartitionUpdate}} and in {{Mutation}}, encapsulated?
3. There are a few places duplicating the logic for {{cdcEnabled}} calculation. I feel like it might be better to move it back to {{Mutation}} constructor instead. 

The nits in no particular order:
- {{update}} variable in {{RowUpdateBuilder.buildUpdate()}} is redundant, IDEA is nagging about it
- {{BatchUpdatesCollector.IMutationBuilder}} has some javadoc problems that IDEA doesn’t like
- {{BatchUpdatesCollector.MutationBuilder.add()}} calls default impl of {{PartitionUpdate.Builder.toString()}} on duplicate add() - should override {{toString()}} to I guess, or just log {{prev.metadata().id}} or something instead.
- {{BatchUpdatesCollector.CounterMutationBuilder}} can also be private, like {{BatchUpdatesCollector.MutationBuilder}} already is
- {{SingleTableUpdatesCollector}} has a bunch of unused imports

> Make PartitionUpdate and Mutation immutable
> -------------------------------------------
>
>                 Key: CASSANDRA-13867
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-13867
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Core
>            Reporter: Marcus Eriksson
>            Assignee: Marcus Eriksson
>             Fix For: 4.x
>
>
> To avoid issues like CASSANDRA-13619 we should make PartitionUpdate and Mutation immutable.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org