[
https://issues.apache.org/jira/browse/CASSANDRA-12060?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15490098#comment-15490098
]
Sylvain Lebresne commented on CASSANDRA-12060:
----------------------------------------------
I'll have to apologize here because while reviewing this I'm realizing that
I've been confused and have pushed for the wrong thing.
My confusion had to do with the fact that CQL doesn't behave as I though it
did. Let me explain. This ticket and CASSANDRA-9842 are about what differences
CAS queries with conditions on static columns make between existing and
non-existing partitions. For that, my guideline was to make it consistent with
how CAS queries with conditions on regular columns differenciate between
existing and non-existing rows, and that where I was wrong.
Let's consider the following example:
{noformat}
CREATE TABLE test (k int, t int, v int, PRIMARY KEY (k, t));
SELECT v FROM test WHERE k = 0 AND t = 0;
> v
> ------
>
> (0 rows)
INSERT INTO test(k, t) VALUES (0, 0);
SELECT v FROM test WHERE k = 0 AND t = 0;
> v
> ------
> null
>
> (1 rows)
UPDATE test SET v = 42 WHERE k = 0 AND t = 1 IF v = 1; // Note we try updating
another row
> [applied]
> -----------
> False
UPDATE test SET v = 42 WHERE k = 0 AND t = 0 IF v = 1; // The row we've
inserted before, but v is null
> [applied] | v
> -----------+------
> False | null
{noformat}
Both in SELECT and in CAS queries result set, we make a difference between a
row existing or not existing. For some reason, I though this difference
somewhat carried on to whether a {{IF v = null}} would apply (or any {{IF v !=
<something>}} condition). I was sure that it was applied if the row existed but
had no value for {{v}} but did *not* applied if the row didn't existed at all.
In other words, what I _though_ we had was:
{noformat}
UPDATE test SET v = 42 WHERE k = 0 AND t = 1 IF v = null; // Trying to updated
a non existing row
> [applied]
> -----------
> False
UPDATE test SET v = 42 WHERE k = 0 AND t = 0 IF v = null; // The row exist and
v is null
> [applied]
> -----------
> True
{noformat}
and I advocated we did the same for static columns and existing/non-existing
partitions. But that's *not* how it works and the first query above does apply.
That is, even though we always make a difference in result sets between
existing row (but with null value) and non-existing row, we don't make one when
evaluating if a {{IF v = null}} condition applies.
So anyway, what semantic would be better in theory doesn't matter much. The way
{{IF v = null}} currently works for normal column is that it makes no
difference between the row existing (and having no value for v) or not existing
and it's probably too late to change that, so we should make that work
consistently for static column and hence you should just have ignored me on
this ticket since that's how it works in 3.0 after CASSANDRA-9842.
With that all said, we still should do what this ticket initiall sets to do: we
should make that existing versus not existing partition difference for static
columns in CAS result sets because that's how it works for rows/normal columns
and we should be consistent.
Doing that last part requires most of the linked patch, though I have the
following remarks on said patch:
* I'm not sure I understand the reason for the change in
{{CQL3CasRequest.columnsToRead()}}. In particular, the comment talks about
having to query a row, but that method is about which columns we bother
fetching, not which row we query.
* I'm not fond of using {{null}} for empty partitions since we can just test
with {{isEmpty()}} directly. Granted, the current code is confusing as
{{appliesTo()}} test for {{null}} even though it's neither passed currently and
that's really oversight from CASSANDRA-8099. I think we should stick to
{{isEmpty()}} but remove the useless code.
* Regarding querying the first row of a partition when we only have static
conditions, I think the code would be easier to follow if we separated static
conditions in {{CQL3CasRequest}}. In fact, once we do that, we can also use
names queries for "normal" conditions (instead of doing slices of one row, the
former being potentially more optimized), which is not all that related to this
patch tbh, but is kind of an oversight of the code so we might as well fix it
while we're at it.
Anyway, as I felt bad about the back and forth on this, I took the liberty to
made the changes related to what's above:
| [12060-3.0-v2|https://github.com/pcmanus/cassandra/commits/12060-3.0-v2] |
[utests|http://cassci.datastax.com/job/pcmanus-12060-3.0-v2-testall] |
[dtests|http://cassci.datastax.com/job/pcmanus-12060-3.0-v2-dtest] |
| [12060-trunk-v2|https://github.com/pcmanus/cassandra/commits/12060-trunk-v2]
| [utests|http://cassci.datastax.com/job/pcmanus-12060-trunk-v2-testall] |
[dtests|http://cassci.datastax.com/job/pcmanus-12060-trunk-v2-dtest] |
> 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)
