cassandra-commits mailing list archives

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

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

Mime
View raw message