cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sylvain Lebresne (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-7396) Allow selecting Map key, List index
Date Fri, 01 Jul 2016 15:22:11 GMT

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

Mime
View raw message