cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sam Tunnicliffe (JIRA)" <>
Subject [jira] [Commented] (CASSANDRA-11136) SASI index options validation
Date Tue, 09 Feb 2016 13:19:18 GMT


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
* {{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|]

* It isn't new or specific to this issue, but I noticed there are still some links in {{}}
which point to your github fork.

> SASI index options validation
> -----------------------------
>                 Key: CASSANDRA-11136
>                 URL:
>             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

View raw message