Mailing-List: contact commits-help@cassandra.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cassandra.apache.org
Date: Mon, 26 Oct 2015 11:50:27 +0000 (UTC)
From: "Sam Tunnicliffe (JIRA)" <jira@apache.org>
To: commits@cassandra.apache.org
Message-ID: <JIRA.12856609.1439816845000.58101.1445860227774@Atlassian.JIRA>
In-Reply-To: <JIRA.12856609.1439816845000@Atlassian.JIRA>
References: <JIRA.12856609.1439816845000@Atlassian.JIRA>
 <JIRA.12856609.1439816845818@arcas>
Subject: [jira] [Commented] (CASSANDRA-10092) Generalize
 PerRowSecondaryIndex validation
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable


    [ https://issues.apache.org/jira/browse/CASSANDRA-10092?page=3Dcom.atla=
ssian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=
=3D14974081#comment-14974081 ]=20

Sam Tunnicliffe commented on CASSANDRA-10092:
---------------------------------------------

Thanks for the updated patch, though I'm afraid there are a couple of issue=
s with it.

The validation in {{CassandraServer.createMutationList}} isn't really doing=
 what it's supposed to. It simply creates an empty columnfamily and passes =
that to the validation method. This case isn't actually covered by the new =
tests (but it would be good to do so), but even if it were, the implementat=
ion of the new method in the test index impl would let it pass as the valid=
ation is based solely on the key. So if you could change that validation & =
extend the test to exercise each of the places where it's called (i.e. regu=
lar, batch and CAS mutations), that'd be good.

The patch also doesn't merge cleanly to 2.2; there are some trivial merge c=
onflicts, but once those are resolved the several of the tests in {{PerRowS=
econdaryIndexTest}} error. I suspect this is something to do with the way {=
{testInvalidRowInsertThrift}} sets up {{Embedded CassandraService}}, you mi=
ght want to check out the other tests which use that without errors & verif=
y that they're equivalent.

> Generalize PerRowSecondaryIndex validation
> ------------------------------------------
>
>                 Key: CASSANDRA-10092
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-1009=
2
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Core
>            Reporter: Andr=C3=A9s de la Pe=C3=B1a
>            Assignee: Andr=C3=A9s de la Pe=C3=B1a
>            Priority: Minor
>              Labels: 2i, secondary_index, validation
>             Fix For: 2.1.x, 2.2.x
>
>         Attachments: CASSANDRA-10092_v2.patch, improve_2i_validation.patc=
h
>
>
> Index validation is currently done in a per-cell basis. However, per-row =
secondary index developers can be interested in validating all the written =
columns at once, because some implementations need to check the validity of=
 a row write by comparing some column values against others. For example, a=
 per row 2i implementation indexing time ranges (composed by a start date c=
olumn and an end date column) should check that the start date is before th=
e stop date.
> I'm attaching a patch adding a new method to {{PerRowSecondaryIndex}}:
> {code:java}
> public void validate(ByteBuffer key, ColumnFamily cf) throws InvalidReque=
stException {}
> {code}
> and a new method to {{SecondaryIndexManager}}:
> {code:java}
> public void validateRowLevelIndexes(ByteBuffer key, ColumnFamily cf) thro=
ws InvalidRequestException
>   {
>       for (SecondaryIndex index : rowLevelIndexMap.values())
>       {
>           ((PerRowSecondaryIndex) index).validate(key, cf);
>       }
>   }
> {code}
> This method is invoked in CQL {{UpdateStatement#validateIndexedColumns}}.=
 This way, {{PerRowSecondaryIndex}} could perform complex write validation.
> I have tried to do the patch in the least invasive way possible. Maybe th=
e current method {{SecondaryIndex#validate(ByteBuffer, Cell)}} should be mo=
ved to {{PerColumnSecondaryIndex}}, and the {{InvalidRequestException}} tha=
t informs about the particular 64k limitation should be thrown by {{Abstrac=
tSimplePerColumnSecondaryIndex}}. However, given the incoming  [CASSANDRA-9=
459|https://issues.apache.org/jira/browse/CASSANDRA-9459], I think that the=
 proposed patch is more than enough to provide rich validation features to =
2i implementations based on 2.1.x and 2.2.x.


--
This message was sent by Atlassian JIRA
(v6.3.4#6332)