[
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)
