[ https://issues.apache.org/jira/browse/CASSANDRA-16217?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17224690#comment-17224690 ]
Marcus Eriksson commented on CASSANDRA-16217: --------------------------------------------- this looks good in general, first pass of review; * we need to forward-port https://issues.apache.org/jira/browse/CASSANDRA-13917 to 4.0 * In {{Selection.java}}, this was changed in 13994: {code} public boolean containsStaticColumns() { - if (table.isStaticCompactTable() || !table.hasStaticColumns()) + if (!table.hasStaticColumns()) return false; ... {code} note that I don't see how we can actually hit this (especially with 13917 backported), just wanted to make sure you left this out on purpose * {{RowFilter#deserialize}}: {code} - if (!metadata.isCompactTable() && column == null) + if (column == null) {code} * {{ViewUpdateGenerator#updateAction}} missing: {code} if (baseMetadata.isCompactTable()) { Clustering clustering = mergedBaseRow.clustering(); for (int i = 0; i < clustering.size(); i++) { if (clustering.get(i) == null) return UpdateAction.NONE; } } {code} * {{TableMetadata#fixupCompactTable}} needs {code} if (hasCounters) flags.add(TableMetadata.Flag.COUNTER); {code} * we should probably re-add all the (relevant) tests removed in CASSANDRA-10857 > Minimal 4.0 COMPACT STORAGE backport > ------------------------------------ > > Key: CASSANDRA-16217 > URL: https://issues.apache.org/jira/browse/CASSANDRA-16217 > Project: Cassandra > Issue Type: Bug > Components: Legacy/CQL > Reporter: Alex Petrov > Assignee: Alex Petrov > Priority: Normal > > There are several behavioural changes related to compact storage, and these > differences are larger than most of us have anticipated: we first thought > there’ll be that “appearing column”, but there’s implicit nulls in > clusterings thing, and row vs column deletion. > Some of the recent issues on the subject are: CASSANDRA-16048, which allows > to ignore these differences. The other one was trying to improve user > experience of anyone still using compact storage: CASSANDRA-15811. > Easily reproducible differernces are: > (1) hidden columns show up, which breaks SELECT * queries > (2) DELETE v and UPDATE v WITH TTL would result into row removals in > non-dense compact tables (CASSANDRA-16069) > (3) INSERT allows skipping clusterings, which are filled with nulls by > default. > Some of these are tricky to support, as 15811 has shown. Anyone on OSS side > who might want to upgrade to 4.0 while still using compact storage might be > affected by being forced into one of these behaviours. > Possible solutions are to document these behaviours, or to bring back a > minimal set of COMPACT STORAGE to keep supporting these. > It looks like it is possible to leave some of the functionality related to > DENSE flag and allow it to be present in 4.0, but only for these three (and > potential related, however not direrclty visible) cases. > [~e.dimitrova] since you were working on removal on compact storage, wanted > to reassure that this is not a revert of your patch. On contrary: your patch > was instrumental in identifying the right places. > cc [~slebresne] [~aleksey] [~benedict] [~marcuse] > |[patch|https://github.com/apache/cassandra/pull/785]|[ci|https://app.circleci.com/pipelines/github/ifesdjeen/cassandra?branch=13994-followup]| -- This message was sent by Atlassian Jira (v8.3.4#803005) --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org