[jira] [Commented] (CASSANDRA-7396) Allow selecting Map key, List index

["Sylvain Lebresne (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CASSANDRA-7396?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15359122#comment-15359122 ] 

Sylvain Lebresne commented on CASSANDRA-7396:
---------------------------------------------

Remarks on the patch:
* As this basically uses terms in select clauses, this should be rebased on top of CASSANDRA-10783, rather than redoing it's own thing. I'm in particular not at all a fan of the "dynamic" thing, especially when have already the concept of {{Terminal}} and {{NonTerminal}} terms to deal with the same thing.
* This only allows element/slice selection directly on a column name, and without nesting, which is imo overly restrictive (we don't have that restriction for field selection for instance). That does change a bit how we want this to work.
* The way {{SelectStatement}} deals with {{ColumnFilter}} feels a bit hacky to me. I understand that we cannot always compute the {{ColumnFilter}} at preparation time anymore, and that we may want to avoid doing it at execution time if we can, but I think that could be more cleanly abstracted.
* The patch seems to use {{null}} to handle the absence of {{from}} or {{to}} in the slice syntax. I'm not sure about that. I think we should refuse {{null}} but accept {{unset}} and make it equivalent to not having a value. That's more logical imo.
* I'm not sure about passing the Cell to the {{ResultSetBuilder}}. First because having an {{Object}} array is somewhat ugly, but also because I think trying to push along this line in CASSANDRA-7826 will get complicated. I think it's simpler to serialize what we get from the storage blindly, and let selector extract subselections from the serialized form aferwards (which they can do without deserializing, working directly on the serialized form).
* It's a bit of an edge case, but {{SELECT m, m\['4'..'6'\] FROM ...}} wasn't working as expected, as the {{ColumnFilter}} only ended up querying the selected slice, ignoring the full column selection.
* There is also a problem with {{SELECT m\[3..4\] FROM ...}} because the parser parse {{3.}} as a float and fails to recognize the slice syntax afterwards. Mor eon tat below.

I took a shot at fixing those [here|https://github.com/pcmanus/cassandra/commits/7396-trunk], which ends up looking quite a bit different. I did however struggled with ANTLR, and there is currently still a few parsing issue that prevent this from being "patch available":
# The problem with {{SELECT m\[3..4\] FROM ...}} where {{3.}} is lexed as a float. I tried to change the lexer using ANTLR a syntactic predicate to presumably not lex {{3.}} as a float if it's followed by another {{.}}, but I must have gotten that wrong as it didn't work. I also tried fixing in the parser, making the accept '\[' term '.' term '\]'  and rejecting that afterwards if the left-most term isn't what it should, but ANTLR ended with crazy conflicts. Anyway, I'm currently a bit out of options.
# For some weird reason, ANTLR also started complaining about {{DISTINCT}} and {{JSON}} not being reserved function names. That it complains isn't all that weird, since after all, something like {{SELECT DISTINCT (3, 4) FROM .. }} *is* ambiguous (it could either be a DISTINCT query on a tuple, or a function call), but what is weird is that it complains following the changes made by that patch, which ought to be unrelated. It should have complained in CASSANDRA-10783 where the ambiguity was created, but somehow didn't. I've currently resolved that by make the keywords reserved, which is strictly speaking a potential breaking change. That said, that's one problem I'm personally willing to live with: in hindsight it sounds like a bad idea to not have those be reserved, and there is an upgrade path for the few users that might use them as unreserved.
# I wasn't able to make ANTLR accept the new syntax in it's more general form. Basically, we only allow column names on the left-hand side of the new syntax. That is, we accept {{SELECT m\[3\]\['foo'..'bar'\] FROM}}, but not {{SELECT f(c)\[3\]}} for instance. I'd really rather avoid that limitation as we don't have it for UDT field selection, but I was unable to have ANTLR be reasonable.

Anyway, the patch is currently "blocked" by those parsing issues and if someone knowledgeable with ANTLR feels like having a look, I certainly wouldn't mind.


> Allow selecting Map key, List index
> -----------------------------------
>
>                 Key: CASSANDRA-7396
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-7396
>             Project: Cassandra
>          Issue Type: New Feature
>          Components: CQL
>            Reporter: Jonathan Ellis
>            Assignee: Robert Stupp
>              Labels: cql, docs-impacting
>             Fix For: 3.x
>
>         Attachments: 7396_unit_tests.txt
>
>
> Allow "SELECT map['key]" and "SELECT list[index]."  (Selecting a UDT subfield is already supported.)



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