cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sylvain Lebresne (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-8502) Static columns returning null for pages after first
Date Tue, 27 Jan 2015 11:53:34 GMT

    [ https://issues.apache.org/jira/browse/CASSANDRA-8502?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14293399#comment-14293399
] 

Sylvain Lebresne commented on CASSANDRA-8502:
---------------------------------------------

Remarks on the patch:
* I don't think this handle range queries (for the reversed case) as SliceFromReadCommand
is not used for those (the comment in DataRange that the reverse case is handled by SliceFromReadCommand
is thus incorrect). I believe this case should be dealt with in SStableScanner, though of
course this will add to the uglyness. Not saying we shouldn't do it but I'll admit that at
least as far as 2.0 goes, this is crossing the boundaries of my own confort zone.
* Not sure about the modifications in {{SliceQueryFilter.trim}} and {{AbstractQueryPager.discardHead}}.
For non-reversed queries, this logic is already handled by the column counter.  It's true
that it doesn't work for reversed queries, but the right place to fix that is imo the column
counter (which probably need to have a reversed variant) as this also affects {{collectReducedColumns}}
(and possibly other places).
* I think {{AbstractQueryPager.firstNonStaticColumns}} deserves a comment as to why we need
to skip static columns. And in fact, I believe we only need to because the detection of static
slices tests for {{EMPTY_BYTE_BUFFER}} which is fragile, so I'd personally prefer making the
detection more reliable (by comparing if a slice start before/stop after the end of the static
block).
* Not convinced {{lastKeyFilterWasUpdatedFor}} is actually an optimization (at least not in
all cases): the code may at best call {{sliceForKey}} with the same key a handful of times
(2, 3?), but if {{lastKeyFilterWasUpdatedFor}} is set, every other key (which is generally
a lot) will trigger a few more useless comparisons. So as it adds complexity, let's leave
that kind of unproven optimization out of this patch.
* Why do we need {{setStaticColumns}} in {{CFMetaData}}? It's only used by the patch with
an empty hashset which feels weird since that should be the default. If it's just to make
sure the staticColumns field in {{CFMetaData}} have been initialized in the test it's used
for, then let's maybe call {{CFMetaData.rebuild}} instead.
* Nit: there is a typo ({{s/once/one}}) in the javadoc of {{splitOutStaticSlice}}.


> Static columns returning null for pages after first
> ---------------------------------------------------
>
>                 Key: CASSANDRA-8502
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-8502
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Core
>            Reporter: Flavien Charlon
>            Assignee: Tyler Hobbs
>             Fix For: 2.1.3, 2.0.13
>
>         Attachments: 8502-2.0.txt, null-static-column.txt
>
>
> When paging is used for a query containing a static column, the first page contains the
right value for the static column, but subsequent pages have null null for the static column
instead of the expected value.
> Repro steps:
> - Create a table with a static column
> - Create a partition with 500 cells
> - Using cqlsh, query that partition
> Actual result:
> - You will see that first, the static column appears as expected, but if you press a
key after "---MORE---", the static columns will appear as null.
> See the attached file for a repro of the output.
> I am using a single node cluster.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message