[
https://issues.apache.org/jira/browse/CASSANDRA-13917?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17017897#comment-17017897
]
Alex Petrov edited comment on CASSANDRA-13917 at 1/17/20 12:51 PM:
-------------------------------------------------------------------
[~slebresne] you are right. In fact, I've found several more places where this
could cause a problem. What we should've done initially is just add a separate
method for CQL layer, and let the rest of the calls (e.g., all internal stuff)
to use the old one. Committed version of the patch also caused problems in
mixed version environments which was caught by one of the upgrade in-jvm
dtests. Also, things like thrift and compact storage compatibility layer was
impacted in a way, too.
I'm still thinking if {{RowUpdateBuilder}} should be using CQL columns or all
columns. I'd argue that CQL only (e.g., no hidden), since this is also how we
query them for the most part. We don't really those hidden column even on
internal paths.
I've made a patch and have added a couple more tests that validates the
situations that weren't covered previously. Thanks to [CASSANDRA-15506], we can
also make sure that upgrade tests are going to be running.
|[patch|https://github.com/apache/cassandra/compare/trunk...ifesdjeen:CASSANDRA-13917-followup]|[CI|https://circleci.com/gh/ifesdjeen/cassandra/tree/CASSANDRA-13917-followup]|
|[patch|https://github.com/apache/cassandra/compare/trunk...ifesdjeen:CASSANDRA-13917-followup-3.11]|[CI|https://circleci.com/gh/ifesdjeen/cassandra/tree/CASSANDRA-13917-followup-3.11]|
was (Author: ifesdjeen):
[~slebresne] you are right. In fact, I've found several more places where this
could cause a problem. What we should've done initially is just add a separate
method for CQL layer, and let the rest of the calls (e.g., all internal stuff)
to use the old one. Committed version of the patch also caused problems in
mixed version environments which was caught by one of the upgrade in-jvm
dtests. Also, things like thrift and compact storage compatibility layer was
impacted in a way, too.
I'm still thinking if {{RowUpdateBuilder}} should be using CQL columns or all
columns. I'd argue that CQL only (e.g., no hidden), since this is also how we
query them for the most part. We don't really those hidden column even on
internal paths.
I've made a patch and have added a couple more tests that validates the
situations that weren't covered previously. Thanks to [CASSANDRA-15506], we can
also make sure that upgrade tests are going to be running.
|[patch|https://github.com/apache/cassandra/compare/trunk...ifesdjeen:CASSANDRA-13917-followup]|[CI|https://circleci.com/gh/ifesdjeen/cassandra/tree/CASSANDRA-13917-followup]|
> COMPACT STORAGE queries on dense static tables accept hidden column1 and
> value columns
> --------------------------------------------------------------------------------------
>
> Key: CASSANDRA-13917
> URL: https://issues.apache.org/jira/browse/CASSANDRA-13917
> Project: Cassandra
> Issue Type: Bug
> Components: Legacy/Core
> Reporter: Alex Petrov
> Assignee: Aleksandr Sorokoumov
> Priority: Low
> Labels: lhf
> Fix For: 3.0.x, 3.11.x
>
> Attachments: 13917-3.0-testall-13.12.2019,
> 13917-3.0-testall-16.01.2020, 13917-3.0-testall-2.png,
> 13917-3.0-testall-20.11.2019.png, 13917-3.0-upgrade-16.01.2020,
> 13917-3.0.png, 13917-3.11-testall-13.12.2019,
> 13917-3.11-testall-16.01.2020.png, 13917-3.11-testall-2.png,
> 13917-3.11-testall-20.11.2019.png, 13917-3.11-upgrade-16.01.2020.png,
> 13917-3.11.png
>
>
> Test for the issue:
> {code}
> @Test
> public void testCompactStorage() throws Throwable
> {
> createTable("CREATE TABLE %s (a int PRIMARY KEY, b int, c int) WITH
> COMPACT STORAGE");
> assertInvalid("INSERT INTO %s (a, b, c, column1) VALUES (?, ?, ?,
> ?)", 1, 1, 1, ByteBufferUtil.bytes('a'));
> // This one fails with Some clustering keys are missing: column1,
> which is still wrong
> assertInvalid("INSERT INTO %s (a, b, c, value) VALUES (?, ?, ?, ?)",
> 1, 1, 1, ByteBufferUtil.bytes('a'));
> assertInvalid("INSERT INTO %s (a, b, c, column1, value) VALUES (?, ?,
> ?, ?, ?)", 1, 1, 1, ByteBufferUtil.bytes('a'), ByteBufferUtil.bytes('b'));
> assertEmpty(execute("SELECT * FROM %s"));
> }
> {code}
> Gladly, these writes are no-op, even though they succeed.
> {{value}} and {{column1}} should be completely hidden. Fixing this one should
> be as easy as just adding validations.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
