[
https://issues.apache.org/jira/browse/CASSANDRA-9664?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14794372#comment-14794372
]
Sylvain Lebresne commented on CASSANDRA-9664:
---------------------------------------------
A few remarks on the patch:
* I don't think the {{Term.Literal}} thing works, because a collection can
contain a function call and yet {{FunctionCall.Raw}} is not a {{Term.Literal}}.
So the whole collection will pass the {{instanceof Term.Literal}} test in
{{CreateViewStatement}}, but {{getRawText}} will throw a
{{ClassCastException}}. Outside of that, I'm not sure there is a good reason to
disallow function calls (or type-cast for that matter). What I think we want is
just rejecting bind markers, and we can do that by just waiting in
{{CreateViewStatement}} to have prepared the statement
({{ParsedStatement.Prepared}} tells use if there is any bound variable in
particular). Once we do that, I think {{getRawText}} can just be put in
{{Term.Raw}} (and renamed {{getText}}).
* Regarding {{getRawText}}, I believe all or almost all {{Term.Raw}} are
already implementing the right thing in their {{toString()}} method, and we
somewhat rely on it for some error messages. I understand that adding a
specific method makes it a bit less likely that we'll add a new {{Term.Raw}}
and forgot to implement its {{toString()}}, but that still mean a bit of ugly
code duplication in practice so I wonder if it's worth it. The worth case
scenario is we add a new {{Term.Raw}}, forget about {{toString()}} and someone
tries to use it while creating a MV and it fails at creation time. This
wouldn't be great, but 1) this would be easily detected and fixed and 2) this
would imply we've released code without testing it, at which point I'd almost
rather have the MV creation fail (so we can fix _and_ test), rather than having
it pass but potentially do something bad (since it hasn't been tested).
Anyway, I don't really feel too strongly, but just to say I'd be fine with just
{{toString()}} and some clear message that it has to be properly implemented in
the class javadoc of {{Term.Raw}}.
* Not sure to understand why the initialization of {{select}} and {{query}} in
{{View.java}} is delayed. Are you worried about MV creation schema changes
arriving before other schema changes it depends on on some nodes? If so, other
places of the code where we have this kind of dependencies (creating a table
after a keyspace creation for instance) assume that the user has to wait for
schema agreement, and I'm not sure why we'd do differently here as it's less
consistent and complicate the code a bit.
* The fact that {{ReadCommand}} now has both a {{selects(DecoratedKey key,
Clustering clustering)}} and {{selectsClustering(DecoratedKey key, Clustering
clustering)}} which do roughly the same thing but not exactly and with no
comment as to the difference is quite a bit confusing. I think we should just
remove the existing {{selects}} and replace its current call by {{(selectsKey
|| selectsClustering)}}. I'm also not sure why those methods in
{{SinglePartitionReadCommand}} don't check the {{rowFilter}} (I suspect they
should).
* It's a bit weird/unintuitive that {{StatementRestrictions.isRestricted}}
ignores {{IS NOT NULL}} restriction. And it seems changing that would actually
simplify the code using that method.
* Nit: In the last {{if}} of {{View.createForDeletionInfo}}, having {{ReadQuery
selectQuery = getReadQuery();}} but using {{query}} on the next line assert is
a bit anti-social.
* Nit: in {{Operator.java}}, can make {{readFrom}} call the new {{fromValue}}.
Those are based on the previous rebase ({{CASSANDRA-9664-rebase-2}}) so I
assume the last rebase was just that, a rebase.
bq. since we're getting close to the deadline for 3.0.0-rc1, I think it might
be better to push that to another ticket targeting 3.2.
Yes, pretty please, let's push that to 3.2.
> Allow MV's select statements to be more complex
> -----------------------------------------------
>
> Key: CASSANDRA-9664
> URL: https://issues.apache.org/jira/browse/CASSANDRA-9664
> Project: Cassandra
> Issue Type: New Feature
> Reporter: Carl Yeksigian
> Assignee: Tyler Hobbs
> Labels: client-impacting, doc-impacting
> Fix For: 3.0.0 rc1
>
>
> [Materialized Views|https://issues.apache.org/jira/browse/CASSANDRA-6477] add
> support for a syntax which includes a {{SELECT}} statement, but only allows
> selection of direct columns, and does not allow any filtering to take place.
> We should add support to the MV {{SELECT}} statement to bring better parity
> with the normal CQL {{SELECT}} statement, specifically simple functions in
> the selected columns, as well as specifying a {{WHERE}} clause.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
