Cassandra Handler - #1662
Conversation
| */ | ||
| private String tableSchema; | ||
|
|
||
| public CassandraFailedQuery(String keyspaceName, String tableName, String tableSchema) { |
There was a problem hiding this comment.
add Objects.requireNonNull for those fields that should be non-null
|
|
||
| public CqlTableReference(String keyspaceName, String tableName) { | ||
| this.keyspaceName = keyspaceName; | ||
| this.tableName = tableName; |
There was a problem hiding this comment.
keyspaceName and tableName can be null? Otherwise add Objects.requireNonNull()
| * a SELECT/UPDATE/DELETE) | ||
| */ | ||
| public static CqlTableReference getTableReference(CqlParser.RootContext root) { | ||
| CqlParser.CqlContext cql = root.cqls() != null ? root.cqls().cql(0) : null; |
There was a problem hiding this comment.
if the command was not a SELECT/UPDATE/DELETE should this throw an IllegalArgumentException?
| /* | ||
| Names of the driver methods invoked via reflection throughout this class. | ||
| */ | ||
| public static final String METHOD_EXECUTE = "execute"; |
There was a problem hiding this comment.
why public? shouldn't these constants be private?
| public static final String CLUSTERING_COLUMN_SUFFIX = " CLUSTERING"; | ||
|
|
||
|
|
||
| public static final String SELECT_ALL_PREFIX = "SELECT * FROM "; |
| */ | ||
| public class CqlDistanceWithMetrics { | ||
|
|
||
| public final double cqlDistance; |
There was a problem hiding this comment.
replace this with getters, change visibility to private
|
|
||
| public final int numberOfEvaluatedRows; | ||
|
|
||
| public CqlDistanceWithMetrics(double cqlDistance, int numberOfEvaluatedRows) { |
There was a problem hiding this comment.
can the arguments be null? Otherwise add Objects.requireNonNull()
| } | ||
|
|
||
| private static CqlTableReference parseTableReference(String cql) { | ||
| return CqlParserUtils.getTableReference(CqlParserUtils.parseCqlCommand(cql)); |
There was a problem hiding this comment.
refactor CqlParserUtils.parseCqlCommand(cql) to a variable
| private final String tableName; | ||
|
|
||
| public TableKey(String keyspaceName, String tableName) { | ||
| this.keyspaceName = keyspaceName; |
There was a problem hiding this comment.
can keyspaceName be null? can tableName be null? Add Objects.requireNull and Javadoc
| return CassandraSchemaTracer.resolveKeyspaceName(cqlSession, null); | ||
| } catch (RuntimeException e) { | ||
| SimpleLogger.uniqueWarn("Failed to resolve Cassandra session's default keyspace"); | ||
| return null; |
There was a problem hiding this comment.
why should this be failing? If it is not expected it must throw exception
| */ | ||
| private final String tableName; | ||
|
|
||
| public CqlTableReference(String keyspaceName, String tableName) { |
There was a problem hiding this comment.
Add Javadoc if keyspaceName can be null
| List<TerminalNode> names = fromSpec.fromSpecElement().OBJECT_NAME(); | ||
| if (names.size() == 2) { | ||
| return new CqlTableReference(names.get(0).getText(), names.get(1).getText()); | ||
| } else { |
There was a problem hiding this comment.
what if names.size() == 0 or names.size() > 2? we need explicit handling of error scenarios (with meaningful logs)
| operations.clear(); | ||
| cqlCommandWithDistances.clear(); | ||
| failedQueries.clear(); | ||
| // tableSchemas is not cleared: each table's schema is captured once, the first time it's queried |
There was a problem hiding this comment.
isn't this going to be problematic when running E2E tests? this has to be handled somehow
3 participants
No description provided.