[jira] [Commented] (CASSANDRA-10721) Altering a UDT might break UDA deserialisation
[
https://issues.apache.org/jira/browse/CASSANDRA-10721?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15150294#comment-15150294
]
Benjamin Lerer commented on CASSANDRA-10721:
+1
> Altering a UDT might break UDA deserialisation
> --
>
> Key: CASSANDRA-10721
> URL: https://issues.apache.org/jira/browse/CASSANDRA-10721
> Project: Cassandra
> Issue Type: Bug
> Components: CQL, Distributed Metadata
>Reporter: Aleksey Yeschenko
>Assignee: Robert Stupp
> Fix For: 3.0.x
>
>
> CASSANDRA-10650 switched UDA's {{initcond}} serialisation in schema to its
> CQL literal. This means that if any particular field is renamed in the UDT,
> or of its type gets changes, we will not be able to parse initcond back.
> We should either:
> 1) Forbid renames and type switches in UDTs that are being used in UDAs, or
> 2) Make sure we alter the UDAs in schema alongside the new UDT at all times
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-10721) Altering a UDT might break UDA deserialisation
[
https://issues.apache.org/jira/browse/CASSANDRA-10721?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15148979#comment-15148979
]
Robert Stupp commented on CASSANDRA-10721:
--
Oh - heh - seems, that I coded the stuff on one machine but pushed to github
from another. No, you didn't miss something.
> Altering a UDT might break UDA deserialisation
> --
>
> Key: CASSANDRA-10721
> URL: https://issues.apache.org/jira/browse/CASSANDRA-10721
> Project: Cassandra
> Issue Type: Bug
> Components: CQL, Distributed Metadata
>Reporter: Aleksey Yeschenko
>Assignee: Robert Stupp
> Fix For: 3.0.x
>
>
> CASSANDRA-10650 switched UDA's {{initcond}} serialisation in schema to its
> CQL literal. This means that if any particular field is renamed in the UDT,
> or of its type gets changes, we will not be able to parse initcond back.
> We should either:
> 1) Forbid renames and type switches in UDTs that are being used in UDAs, or
> 2) Make sure we alter the UDAs in schema alongside the new UDT at all times
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-10721) Altering a UDT might break UDA deserialisation
[
https://issues.apache.org/jira/browse/CASSANDRA-10721?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15138917#comment-15138917
]
Benjamin Lerer commented on CASSANDRA-10721:
{quote}Moved the referencesUserType function to AbstractType. This is now
effectively a refactoring of the (replaced) references function to specifically
check the user-type's name.{quote}
I do not see the changes that you mentioned. In your {{10721-udt-alter-3.0}}
branch. Did I miss something?
> Altering a UDT might break UDA deserialisation
> --
>
> Key: CASSANDRA-10721
> URL: https://issues.apache.org/jira/browse/CASSANDRA-10721
> Project: Cassandra
> Issue Type: Bug
> Components: CQL, Distributed Metadata
>Reporter: Aleksey Yeschenko
>Assignee: Robert Stupp
> Fix For: 3.0.x
>
>
> CASSANDRA-10650 switched UDA's {{initcond}} serialisation in schema to its
> CQL literal. This means that if any particular field is renamed in the UDT,
> or of its type gets changes, we will not be able to parse initcond back.
> We should either:
> 1) Forbid renames and type switches in UDTs that are being used in UDAs, or
> 2) Make sure we alter the UDAs in schema alongside the new UDT at all times
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-10721) Altering a UDT might break UDA deserialisation
[
https://issues.apache.org/jira/browse/CASSANDRA-10721?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15130397#comment-15130397
]
Robert Stupp commented on CASSANDRA-10721:
--
Moved the {{referencesUserType}} function to {{AbstractType}}. This is now
effectively a refactoring of the (replaced) {{references}} function to
specifically check the user-type's name. Kicked off the CI tests for 3.0 and
trunk (nothing special in 3.3 and trunk however - just a merge w/o conflicts).
Can you take a look at the latest changes ([complete 3.0 diff is
here|https://github.com/apache/cassandra/compare/20813858297b375e1e3f7f4b09dcba2756f9c58f...snazy:10721-udt-alter-3.0])?
> Altering a UDT might break UDA deserialisation
> --
>
> Key: CASSANDRA-10721
> URL: https://issues.apache.org/jira/browse/CASSANDRA-10721
> Project: Cassandra
> Issue Type: Bug
> Components: CQL, Distributed Metadata
>Reporter: Aleksey Yeschenko
>Assignee: Robert Stupp
> Fix For: 3.0.x
>
>
> CASSANDRA-10650 switched UDA's {{initcond}} serialisation in schema to its
> CQL literal. This means that if any particular field is renamed in the UDT,
> or of its type gets changes, we will not be able to parse initcond back.
> We should either:
> 1) Forbid renames and type switches in UDTs that are being used in UDAs, or
> 2) Make sure we alter the UDAs in schema alongside the new UDT at all times
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-10721) Altering a UDT might break UDA deserialisation
[
https://issues.apache.org/jira/browse/CASSANDRA-10721?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15110892#comment-15110892
]
Benjamin Lerer commented on CASSANDRA-10721:
The changes look good.
For the {{referencesUserType}}, I am even more convinced than before that it
should go into into {{AbstractType}}. Those {{asCQL3Type()}} calls everywere
are a clear sign to me that it will be a better place. At the same time, I do
not want to discuss it for too long. I let you choose.
> Altering a UDT might break UDA deserialisation
> --
>
> Key: CASSANDRA-10721
> URL: https://issues.apache.org/jira/browse/CASSANDRA-10721
> Project: Cassandra
> Issue Type: Bug
> Components: CQL, Distributed Metadata
>Reporter: Aleksey Yeschenko
>Assignee: Robert Stupp
> Fix For: 3.0.x
>
>
> CASSANDRA-10650 switched UDA's {{initcond}} serialisation in schema to its
> CQL literal. This means that if any particular field is renamed in the UDT,
> or of its type gets changes, we will not be able to parse initcond back.
> We should either:
> 1) Forbid renames and type switches in UDTs that are being used in UDAs, or
> 2) Make sure we alter the UDAs in schema alongside the new UDT at all times
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-10721) Altering a UDT might break UDA deserialisation
[
https://issues.apache.org/jira/browse/CASSANDRA-10721?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15070830#comment-15070830
]
Robert Stupp commented on CASSANDRA-10721:
--
Removed the {{isBroken}} and added more tests.
Left the {{referencesUserType}} in {{CQL3Type}} - but leave this for
discussion. It feels to fit better in CQL3Type - but has perf benefits in
AbstractType.
> Altering a UDT might break UDA deserialisation
> --
>
> Key: CASSANDRA-10721
> URL: https://issues.apache.org/jira/browse/CASSANDRA-10721
> Project: Cassandra
> Issue Type: Bug
> Components: CQL, Distributed Metadata
>Reporter: Aleksey Yeschenko
>Assignee: Robert Stupp
> Fix For: 3.0.x
>
>
> CASSANDRA-10650 switched UDA's {{initcond}} serialisation in schema to its
> CQL literal. This means that if any particular field is renamed in the UDT,
> or of its type gets changes, we will not be able to parse initcond back.
> We should either:
> 1) Forbid renames and type switches in UDTs that are being used in UDAs, or
> 2) Make sure we alter the UDAs in schema alongside the new UDT at all times
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-10721) Altering a UDT might break UDA deserialisation
[
https://issues.apache.org/jira/browse/CASSANDRA-10721?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15046397#comment-15046397
]
Robert Stupp commented on CASSANDRA-10721:
--
bq. referencesUserType(String name) does not check the keyspace for the UDT.
AFAIK we don't allow cross-keyspace references at all - so it felt ok to
basically copy what's in {{CQL3Type.Raw.referencesUserType}}.
bq. referencesUserType ... was in AbstractType
That's correct. Moved that to CQL3Type as I thought it's better to not add new
stuff to old API classes. And since the code's not used in a critical path,
that change looked maybe not optimal but reasonable. But I won't fight against
moving it back to AbstractType. However we could thing of keeping a reference
to the CQL3Type in AbstractType to optimize that if it is used in a critical
code path.
Will fix the other issues (two points for more utests + removal of isBroken)
you mentioned.
> Altering a UDT might break UDA deserialisation
> --
>
> Key: CASSANDRA-10721
> URL: https://issues.apache.org/jira/browse/CASSANDRA-10721
> Project: Cassandra
> Issue Type: Bug
> Components: CQL, Distributed Metadata
>Reporter: Aleksey Yeschenko
>Assignee: Robert Stupp
> Fix For: 3.0.x
>
>
> CASSANDRA-10650 switched UDA's {{initcond}} serialisation in schema to its
> CQL literal. This means that if any particular field is renamed in the UDT,
> or of its type gets changes, we will not be able to parse initcond back.
> We should either:
> 1) Forbid renames and type switches in UDTs that are being used in UDAs, or
> 2) Make sure we alter the UDAs in schema alongside the new UDT at all times
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-10721) Altering a UDT might break UDA deserialisation
[
https://issues.apache.org/jira/browse/CASSANDRA-10721?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15038525#comment-15038525
]
Benjamin Lerer commented on CASSANDRA-10721:
* The {{referencesUserType(String name)}} does not check the keyspace for the
UDT. I think it could lead to some problems if we had a UDT with the same name
in a different keyspace. It might be good to add a unit test for it.
* It seems to me that if the {{referencesUserType}} method was in
{{AbstractType}} (something like: {{referencesUserType(String keyspace,
ByteBuffer name)}}), it would removes all these {{asCQL3Type()}} calls and make
the code more readable. What do you think?
* The {{isBroken}} and the follow-up notes should be removed. They are not
needed by the patch.
* It might be good to add some extra unit tests to check UDT nested in (Sets,
Maps or Tulpes) for extra security.
> Altering a UDT might break UDA deserialisation
> --
>
> Key: CASSANDRA-10721
> URL: https://issues.apache.org/jira/browse/CASSANDRA-10721
> Project: Cassandra
> Issue Type: Bug
> Components: CQL, Distributed Metadata
>Reporter: Aleksey Yeschenko
>Assignee: Robert Stupp
> Fix For: 3.0.x
>
>
> CASSANDRA-10650 switched UDA's {{initcond}} serialisation in schema to its
> CQL literal. This means that if any particular field is renamed in the UDT,
> or of its type gets changes, we will not be able to parse initcond back.
> We should either:
> 1) Forbid renames and type switches in UDTs that are being used in UDAs, or
> 2) Make sure we alter the UDAs in schema alongside the new UDT at all times
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-10721) Altering a UDT might break UDA deserialisation
[
https://issues.apache.org/jira/browse/CASSANDRA-10721?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15036184#comment-15036184
]
Robert Stupp commented on CASSANDRA-10721:
--
userTypeUsedBy: right - it's now completely superfluous (and therefore
removed). I've copied the functionality of {{referencesUserType}} from
{{CQL3Type.Raw}} to {{CQL3Type}} and also removed the
{{AbstractType.references()}} method.
Rebased and pushed a commit for the review changes to the branch and triggered
CI for 3.0+trunk versions.
> Altering a UDT might break UDA deserialisation
> --
>
> Key: CASSANDRA-10721
> URL: https://issues.apache.org/jira/browse/CASSANDRA-10721
> Project: Cassandra
> Issue Type: Bug
> Components: CQL, Distributed Metadata
>Reporter: Aleksey Yeschenko
>Assignee: Robert Stupp
> Fix For: 3.0.x
>
>
> CASSANDRA-10650 switched UDA's {{initcond}} serialisation in schema to its
> CQL literal. This means that if any particular field is renamed in the UDT,
> or of its type gets changes, we will not be able to parse initcond back.
> We should either:
> 1) Forbid renames and type switches in UDTs that are being used in UDAs, or
> 2) Make sure we alter the UDAs in schema alongside the new UDT at all times
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-10721) Altering a UDT might break UDA deserialisation
[
https://issues.apache.org/jira/browse/CASSANDRA-10721?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15035850#comment-15035850
]
Benjamin Lerer commented on CASSANDRA-10721:
It seems that the branch contains a left over from some additional work (the
second commit).
For the first commit:
* I think that we should probably refactor the {{userTypeUsedBy}} method. All
those {{instanceof}} are a clear sign that we should be using polymorphism
(e.g. adding a {{useUserType(keyspaceName, typeName)}} to {{AbstractType}}).
* {{checkTypeNotUsedByAggregate}} could be simplified by using
{{ksm.functions.udas.anyMatch()}}.
* In the unit test, I think that the name {{alterDropSequence}} is a bit
confusing. It should probably be changed to something that express better the
fact that the method is checking that we cannot alter or drop the user type.
> Altering a UDT might break UDA deserialisation
> --
>
> Key: CASSANDRA-10721
> URL: https://issues.apache.org/jira/browse/CASSANDRA-10721
> Project: Cassandra
> Issue Type: Bug
> Components: CQL, Distributed Metadata
>Reporter: Aleksey Yeschenko
>Assignee: Robert Stupp
> Fix For: 3.0.x
>
>
> CASSANDRA-10650 switched UDA's {{initcond}} serialisation in schema to its
> CQL literal. This means that if any particular field is renamed in the UDT,
> or of its type gets changes, we will not be able to parse initcond back.
> We should either:
> 1) Forbid renames and type switches in UDTs that are being used in UDAs, or
> 2) Make sure we alter the UDAs in schema alongside the new UDT at all times
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-10721) Altering a UDT might break UDA deserialisation
[
https://issues.apache.org/jira/browse/CASSANDRA-10721?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15008844#comment-15008844
]
Robert Stupp commented on CASSANDRA-10721:
--
I think preventing such a change is the better option. Otherwise we'd also have
check permissions on the aggregates. Additionally changing nested
tuple/collection/type structures might become a nightmare.
Will also see whether we need a check for "garbaged" initcond values into
LegacySchemaMigrator.
> Altering a UDT might break UDA deserialisation
> --
>
> Key: CASSANDRA-10721
> URL: https://issues.apache.org/jira/browse/CASSANDRA-10721
> Project: Cassandra
> Issue Type: Bug
> Components: CQL, Distributed Metadata
>Reporter: Aleksey Yeschenko
>Assignee: Robert Stupp
> Fix For: 3.0.x
>
>
> CASSANDRA-10650 switched UDA's {{initcond}} serialisation in schema to its
> CQL literal. This means that if any particular field is renamed in the UDT,
> or of its type gets changes, we will not be able to parse initcond back.
> We should either:
> 1) Forbid renames and type switches in UDTs that are being used in UDAs, or
> 2) Make sure we alter the UDAs in schema alongside the new UDT at all times
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-10721) Altering a UDT might break UDA deserialisation
[
https://issues.apache.org/jira/browse/CASSANDRA-10721?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15008851#comment-15008851
]
Aleksey Yeschenko commented on CASSANDRA-10721:
---
I'm fine with simply preventing it for 3.0.1/3.1, to fix the bug itself, but
I'd prefer that we at least eventually address this properly.
> Altering a UDT might break UDA deserialisation
> --
>
> Key: CASSANDRA-10721
> URL: https://issues.apache.org/jira/browse/CASSANDRA-10721
> Project: Cassandra
> Issue Type: Bug
> Components: CQL, Distributed Metadata
>Reporter: Aleksey Yeschenko
>Assignee: Robert Stupp
> Fix For: 3.0.x
>
>
> CASSANDRA-10650 switched UDA's {{initcond}} serialisation in schema to its
> CQL literal. This means that if any particular field is renamed in the UDT,
> or of its type gets changes, we will not be able to parse initcond back.
> We should either:
> 1) Forbid renames and type switches in UDTs that are being used in UDAs, or
> 2) Make sure we alter the UDAs in schema alongside the new UDT at all times
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
