[
https://issues.apache.org/jira/browse/CASSANDRA-11136?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15138926#comment-15138926
]
Sam Tunnicliffe commented on CASSANDRA-11136:
---------------------------------------------
Collating the review comments here, rather than on the sub tasks...
* Seeing as the {{CassandraIndex::parseTarget}} methods are being shared, maybe
they'd be better moved somewhere else. Perhaps an {{o.a.c.i.TargetParser}}
helper or similar?.
* While we're there, I don't recall why the original method throws
{{RuntimeException}} rather than {{ConfigurationException}}. If you agree, do
you mind changing that while you're in the area?
* {{getColumnFamilyStoreIncludingIndexes}} in {{SASIIndex::validateOptions}} is
overkill, it could just use {{Keyspace::openAndGetStore(cfm)}}
* {{SASIIndex::validateOptions}} should probably call {{validate}} on the
{{IndexMode}} once it's created it. Not doing so means that an invalid analyzer
class is logged, but doesn't cause the index creation to fail.
* Related to the previous: {{ColumnIndex::validate()}} doesn't appear to be
called from anywhere, which is currently the only call site of
{{IndexMode::validate}}.
* I don't know if this is intentional (for forwards compat) but
unknown/unrecognised options are silently allowed. Personally, I'd prefer the
validation were strict and any unrecognised options resulted in an error.
* Very minor: The reformatting of imports in {{SASIIndex}} isn't in line with
the [Code Style|https://wiki.apache.org/cassandra/CodeStyle]
* It isn't new or specific to this issue, but I noticed there are still some
links in {{SASI.md}} which point to your github fork.
> SASI index options validation
> -----------------------------
>
> Key: CASSANDRA-11136
> URL: https://issues.apache.org/jira/browse/CASSANDRA-11136
> Project: Cassandra
> Issue Type: Bug
> Components: Core
> Reporter: Pavel Yaskevich
> Assignee: Pavel Yaskevich
> Fix For: 3.4
>
>
> This is an umbrella issue for CASSANDRA-\{11129, 11132, 11133, 11134\}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
