adelapena commented on a change in pull request #1141:
URL: https://github.com/apache/cassandra/pull/1141#discussion_r691157474
##########
File path:
src/java/org/apache/cassandra/index/sasi/analyzer/DelimiterAnalyzer.java
##########
@@ -100,11 +108,11 @@ protected ByteBuffer computeNext() {
};
}
-
+ @Override
public boolean isTokenizing()
{
return true;
- }
+ }
Review comment:
Nit: misaligned
##########
File path:
test/unit/org/apache/cassandra/index/sasi/analyzer/DelimiterAnalyzerTest.java
##########
@@ -81,10 +83,11 @@ public void testBlankEntries() throws Exception
Assert.assertFalse(testString.toLowerCase().equals(output.toString()));
}
- @Test(expected = IllegalArgumentException.class)
+ @Test(expected = ConfigurationException.class)
public void ensureIncompatibleInputSkipped() throws Exception
Review comment:
Nit: doesn't need the `throws Exception`
##########
File path:
test/unit/org/apache/cassandra/index/sasi/analyzer/DelimiterAnalyzerTest.java
##########
@@ -81,10 +83,11 @@ public void testBlankEntries() throws Exception
Assert.assertFalse(testString.toLowerCase().equals(output.toString()));
}
- @Test(expected = IllegalArgumentException.class)
+ @Test(expected = ConfigurationException.class)
public void ensureIncompatibleInputSkipped() throws Exception
{
- new DelimiterAnalyzer().init(new HashMap(), Int32Type.instance);
+ new DelimiterAnalyzer().validate(new HashMap<>(),
Review comment:
Nit: we can use `Collections.emptyMap()`
##########
File path:
src/java/org/apache/cassandra/index/sasi/analyzer/AbstractAnalyzer.java
##########
@@ -38,15 +40,23 @@ public void remove()
throw new UnsupportedOperationException();
}
- public abstract void init(Map<String, String> options, AbstractType
validator);
+ public void validate(Map<String, String> options, ColumnDefinition cd)
throws ConfigurationException
+ {
+ if (!isCompatibleWith(cd.type))
+ throw new ConfigurationException(String.format("%s does not
support type %s",
+
this.getClass().getSimpleName(),
+
cd.type.asCQL3Type()));
+ }
+
+ public abstract void init(Map<String, String> options, AbstractType<?>
validator);
public abstract void reset(ByteBuffer input);
/**
* Test whether the given validator is compatible with the underlying
analyzer.
*
- * @param validator
- * @return
+ * @param validator the validator to test the compatibility with
+ * @return true if the give validator is compatible, false otherwise
Review comment:
I think that `isCompatibleWith` can be `protected`, since now it's only
called from `validate`.
--
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]
