Conversation
b10e9bb to
94b8dad
Compare
There was a problem hiding this comment.
This one is failing on <= 2.1 for some reason, though looks like it should work...looking into it.
There was a problem hiding this comment.
Just needed to selectively create the index based on major version much like should_create_metadata_for_index_on_map_values, i.e.: https://gist.github.com/tolbertam/4be3efec48975fe4b4d4
a9f08b3 to
d56dd0b
Compare
|
Added a version requirement check to one of the tests (feel free to squash that when merging). This looks good to me 👍 |
|
Added another test (for testing with multiple indexes on same column - not really necessary but figured would be good to add) and also added 'kind' validation. PR still looks good to me 👍 |
|
I noticed via cassandra-dtests that it was possible to add secondary indexes to materialized views. I noticed that the driver didn't support that yet, so i added it here in 92dabb3 since it seemed pretty straightforward, feel free to back out/change/etc. if it isn't an appropriate change. Secondary indexes are semi-busted on C* trunk right now (added comment to CASSANDRA-9921), so the test I added currently fails (unless you put a breakpoint before the execute statement and add the index manually twice), but should pass once fixed in C*. |
|
lgtm 👍 thanks for finding out that 2is on mvs are allowed! |
|
@tolbertam I just pushed a commit reverting indexes on materialized views following the decision in CASSANDRA-9921. Could you kindly review it again? |
|
Sounds good, taking a look :) |
There was a problem hiding this comment.
It'd probably be fine to just delete this test and if support gets added back we can readd it then
|
looks good to me 👍 |
This commit contains substantial contributions by Andrew Tolbert (@tolbertam).
JAVA-913: Redesign IndexMetadata API.
See CASSANDRA-10216 for details.