[jira] [Commented] (CASSANDRA-11136) SASI index options validation

["Sam Tunnicliffe (JIRA)" <ji...@apache.org>]

    [ 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)