[
https://issues.apache.org/jira/browse/CASSANDRA-12060?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15490342#comment-15490342
]
Alex Petrov edited comment on CASSANDRA-12060 at 9/14/16 12:56 PM:
-------------------------------------------------------------------
Thanks for reviewing the patch and for elaborate comment.
I agree that having the behaviour consistent with the regular columns makes
most sense.
bq. I'm not sure I understand the reason for the change in
CQL3CasRequest.columnsToRead()
This was added to make sure that output in 3.x is same as in 2.x:
{code}
assertRows(execute("BEGIN BATCH " +
"UPDATE %1$s SET v='foobar' WHERE id=0 AND k='k1'; "
+
"UPDATE %1$s SET v='barfoo' WHERE id=0 AND k='k2'; "
+
"UPDATE %1$s SET version=3 WHERE id=0 IF version=1;
" +
"APPLY BATCH "),
row(false, 0, "k1", 2));
// vs
// row(false, 0, null, 2));
{code}
bq. I'm not fond of using null for empty partitions since we can just test with
isEmpty() directly.
Agree. This makes it much cleaner.
bq. I think the code would be easier to follow if we separated static
conditions in CQL3CasRequest
True, it is much easier to follow now. Special-casing static clause was quite
counter-intuitive.
I've ran the dtests locally and they're failing, but they're mostly cosmetic
(condition is being applied correctly, just returned results differ a bit). Do
we want to keep the resultset format for LWTs completely same as it was in
2.x?..
I would also leave {{testConditionalUpdatesWithNonExistingValues}} test to make
sure we cover both behaviours, given we've had quite a few iterations, so we
could keep this behaviour well-documented.
was (Author: ifesdjeen):
Thanks for reviewing the patch and for elaborate comment.
I agree that having the behaviour consistent with the regular columns makes
most sense.
bq. I'm not sure I understand the reason for the change in
CQL3CasRequest.columnsToRead()
This was added to make sure that output in 3.x is same as in 2.x:
{code}
assertRows(execute("BEGIN BATCH " +
"UPDATE %1$s SET v='foobar' WHERE id=0 AND k='k1'; "
+
"UPDATE %1$s SET v='barfoo' WHERE id=0 AND k='k2'; "
+
"UPDATE %1$s SET version=3 WHERE id=0 IF version=1;
" +
"APPLY BATCH "),
row(false, 0, "k1", 2));
// vs
// row(false, 0, null, 2));
{code}
bq. I'm not fond of using null for empty partitions since we can just test with
isEmpty() directly.
Agree. This makes it much cleaner.
bq. I think the code would be easier to follow if we separated static
conditions in CQL3CasRequest
True, it is much easier to follow now. Special-casing static clause was quite
counter-intuitive.
I've ran the dtests locally and they're failing, but they're mostly cosmetic
(condition is being applied correctly, just returned results differ a bit). Do
we want to keep the resultset format for LWTs completely same as it was in
2.x?..
> Establish consistent distinction between non-existing partition and NULL
> value for LWTs on static columns
> ---------------------------------------------------------------------------------------------------------
>
> Key: CASSANDRA-12060
> URL: https://issues.apache.org/jira/browse/CASSANDRA-12060
> Project: Cassandra
> Issue Type: Bug
> Reporter: Alex Petrov
> Assignee: Alex Petrov
>
> When executing following CQL commands:
> {code}
> CREATE KEYSPACE test WITH replication = {'class': 'NetworkTopologyStrategy',
> 'datacenter1': '1' };
> USE test;
> CREATE TABLE testtable (a int, b int, s1 int static, s2 int static, v int,
> PRIMARY KEY (a, b));
> INSERT INTO testtable (a,b,s1,s2,v) VALUES (2,2,2,null,2);
> DELETE s1 FROM testtable WHERE a = 2 IF s2 IN (10,20,30);
> {code}
> The output is different between {{2.x}} and {{3.x}}:
> 2.x:
> {code}
> cqlsh:test> DELETE s1 FROM testtable WHERE a = 2 IF s2 = 5;
> [applied] | s2
> -----------+------
> False | null
> {code}
> 3.x:
> {code}
> cqlsh:test> DELETE s1 FROM testtable WHERE a = 2 IF s2 = 5;
> [applied]
> -----------
> False
> {code}
> {{2.x}} would although return same result if executed on a partition that
> does not exist at all:
> {code}
> cqlsh:test> DELETE s1 FROM testtable WHERE a = 5 IF s2 = 5;
> [applied]
> -----------
> False
> {code}
> It _might_ be related to static column LWTs, as I could not reproduce same
> behaviour with non-static column LWTs. The most recent change was
> [CASSANDRA-10532], which enabled LWT operations on static columns with
> partition keys only. -Another possible relation is [CASSANDRA-9842], which
> removed distinction between {{null}} column and non-existing row.- (striked
> through since same happens on pre-[CASSANDRA-9842] code.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
