Trace Cassandra Tables' Metadata - #1657
Conversation
…data-cassandra-replacement
…data-cassandra-replacement
…data-cassandra-replacement
|
|
||
| public CassandraTableMetadata(String keyspaceName, String tableName, List<CassandraColumnMetadata> columns) { | ||
| this.keyspaceName = keyspaceName; | ||
| this.tableName = tableName; |
There was a problem hiding this comment.
The keyspaceName might be null, but the tableName can't. Queries with a null tableName would not have their schema traced due to the condition in line 81 of CqlSessionClassReplacement
There was a problem hiding this comment.
therefore a Objects.requireNull for tableName is required, right?
| this.keyspaceName = keyspaceName; | ||
| this.tableName = tableName; | ||
| this.columns = Collections.unmodifiableList(columns); | ||
| } |
There was a problem hiding this comment.
should you check if column names are repeated?
There was a problem hiding this comment.
There should be no duplicate column names in a table
There was a problem hiding this comment.
Shouldn't you double check that when the list is copied?
| return Objects.hash(keyspaceName, tableName, asSet(columns)); | ||
| } | ||
|
|
||
| private static Set<CassandraColumnMetadata> asSet(List<CassandraColumnMetadata> columns) { |
There was a problem hiding this comment.
this shouldn't be a method, right?
| private static TableReference extractTableReference(String query) { | ||
| if (query == null) { | ||
| return new TableReference(null, null); | ||
| return new TableReference(null, null, null, null); |
There was a problem hiding this comment.
if the query is not supported it should throw an IllegalArgumentException, right?
|
|
||
| public CassandraTableMetadata(String keyspaceName, String tableName, List<CassandraColumnMetadata> columns) { | ||
| this.keyspaceName = keyspaceName; | ||
| this.tableName = tableName; |
There was a problem hiding this comment.
therefore a Objects.requireNull for tableName is required, right?
| this.keyspaceName = keyspaceName; | ||
| this.tableName = tableName; | ||
| this.columns = Collections.unmodifiableList(columns); | ||
| } |
There was a problem hiding this comment.
Shouldn't you double check that when the list is copied?
No description provided.