jacek-lewandowski commented on a change in pull request #1031:
URL: https://github.com/apache/cassandra/pull/1031#discussion_r643808671
##########
File path: src/java/org/apache/cassandra/db/filter/ColumnFilter.java
##########
@@ -502,149 +561,473 @@ public ColumnFilter build()
boolean isFetchAll = metadata != null;
RegularAndStaticColumns queried = queriedBuilder == null ? null :
queriedBuilder.build();
+
// It's only ok to have queried == null in ColumnFilter if
isFetchAll. So deal with the case of a selectionBuilder
// with nothing selected (we can at least happen on some backward
compatible queries - CASSANDRA-10471).
if (!isFetchAll && queried == null)
queried = RegularAndStaticColumns.NONE;
- SortedSetMultimap<ColumnIdentifier, ColumnSubselection> s = null;
- if (subSelections != null)
+ SortedSetMultimap<ColumnIdentifier, ColumnSubselection> s =
buildSubSelections();
+
+ if (isFetchAll)
{
- s = TreeMultimap.create(Comparator.naturalOrder(),
Comparator.naturalOrder());
- for (ColumnSubselection subSelection : subSelections)
+ // When fetchAll is enabled on pre CASSANDRA-10657 (3.4-),
queried columns are not considered at all, and it
+ // is assumed that all columns are queried. CASSANDRA-10657
(3.4+) brings back skipping values of columns
+ // which are not in queried set when fetchAll is enabled. That
makes exactly the same filter being
+ // interpreted in a different way on 3.4- and 3.4+.
+ //
+ // Moreover, there is no way to convert the filter with
fetchAll and queried != null so that it is
+ // interpreted the same way on 3.4- because that Cassandra
version does not support such filtering.
+ //
+ // In order to avoid inconsitencies in data read by 3.4- and
3.4+ we need to avoid creation of incompatible
+ // filters when the cluster contains 3.4- nodes. We do that by
forcibly setting queried to null.
+ //
+ // see CASSANDRA-10657, CASSANDRA-15833, CASSANDRA-16415
+ if (queried == null || isUpgradingFromVersionLowerThan34())
+ {
+ return new
WildCardColumnFilter(metadata.regularAndStaticColumns());
+ }
+
+ // pre CASSANDRA-12768 (4.0-) all static columns should be
fetched along with all regular columns.
+ if (isUpgradingFromVersionLowerThan40())
{
- if (fullySelectedComplexColumns == null ||
!fullySelectedComplexColumns.contains(subSelection.column()))
- s.put(subSelection.column().name, subSelection);
+ return
SelectionColumnFilter.newInstance(FetchingStrategy.ALL_COLUMNS, metadata,
queried, s);
}
+
+ // pre CASSANDRA-16686 (4.0-RC2-) static columns where not
fetched unless queried witch lead to some wrong results
+ // for some queries
+ if (isUpgradingFromVersionLowerThan40RC2() ||
!returnStaticContentOnPartitionWithNoRows)
+ {
+ return
SelectionColumnFilter.newInstance(FetchingStrategy.ALL_REGULARS_AND_QUERIED_STATICS_COLUMNS,
metadata, queried, s);
+ }
+
+ return
SelectionColumnFilter.newInstance(FetchingStrategy.ALL_COLUMNS, metadata,
queried, s);
+ }
+
+ return
SelectionColumnFilter.newInstance(FetchingStrategy.ONLY_QUERIED_COLUMNS,
(TableMetadata) null, queried, s);
+ }
+
+ private SortedSetMultimap<ColumnIdentifier, ColumnSubselection>
buildSubSelections()
+ {
+ if (subSelections == null)
+ return null;
+
+ SortedSetMultimap<ColumnIdentifier, ColumnSubselection> s =
TreeMultimap.create(Comparator.naturalOrder(), Comparator.naturalOrder());
+ for (ColumnSubselection subSelection : subSelections)
+ {
+ if (fullySelectedComplexColumns == null ||
!fullySelectedComplexColumns.contains(subSelection.column()))
+ s.put(subSelection.column().name, subSelection);
}
- // When fetchAll is enabled on pre CASSANDRA-10657 (3.4-), queried
columns are not considered at all, and it
- // is assumed that all columns are queried. CASSANDRA-10657 (3.4+)
brings back skipping values of columns
- // which are not in queried set when fetchAll is enabled. That
makes exactly the same filter being
- // interpreted in a different way on 3.4- and 3.4+.
- //
- // Moreover, there is no way to convert the filter with fetchAll
and queried != null so that it is
- // interpreted the same way on 3.4- because that Cassandra version
does not support such filtering.
- //
- // In order to avoid inconsitencies in data read by 3.4- and 3.4+
we need to avoid creation of incompatible
- // filters when the cluster contains 3.4- nodes. We do that by
forcibly setting queried to null.
- //
- // see CASSANDRA-10657, CASSANDRA-15833, CASSANDRA-16415
- return new ColumnFilter(isFetchAll, isFetchAll &&
shouldFetchAllStatics(), metadata, isFetchAll && shouldQueriedBeNull() ? null :
queried, s);
+ return s;
}
}
- @Override
- public boolean equals(Object other)
+ /**
+ * {@code ColumnFilter} sub-class for wildcard queries.
+ *
+ * <p>The class does not rely on TableMetadata and expect a fix set of
columns to prevent issues
+ * with Schema race propagation. See CASSANDRA-15899.</p>
+ */
+ public static class WildCardColumnFilter extends ColumnFilter
{
- if (other == this)
+ /**
+ * The queried and fetched columns.
+ */
+ private final RegularAndStaticColumns fetchedAndQueried;
+
+ /**
+ * Creates a {@code ColumnFilter} for wildcard queries.
+ *
+ * <p>The class does not rely on TableMetadata and expect a fix set
of columns to prevent issues
+ * with Schema race propagation. See CASSANDRA-15899.</p>
+ *
+ * @param fetchedAndQueried the fetched and queried columns
+ */
+ private WildCardColumnFilter(RegularAndStaticColumns fetchedAndQueried)
+ {
+ this.fetchedAndQueried = fetchedAndQueried;
+ }
+
+ @Override
+ public RegularAndStaticColumns fetchedColumns()
+ {
+ return fetchedAndQueried;
+ }
+
+ @Override
+ public RegularAndStaticColumns queriedColumns()
+ {
+ return fetchedAndQueried;
+ }
+
+ @Override
+ public boolean fetchesAllColumns(boolean isStatic)
+ {
return true;
+ }
- if (!(other instanceof ColumnFilter))
- return false;
+ @Override
+ public boolean allFetchedColumnsAreQueried()
+ {
+ return true;
+ }
- ColumnFilter otherCf = (ColumnFilter) other;
+ @Override
+ public boolean fetches(ColumnMetadata column)
+ {
+ return true;
+ }
- return otherCf.fetchAllRegulars == this.fetchAllRegulars &&
- otherCf.fetchAllStatics == this.fetchAllStatics &&
- Objects.equals(otherCf.fetched, this.fetched) &&
- Objects.equals(otherCf.queried, this.queried) &&
- Objects.equals(otherCf.subSelections, this.subSelections);
- }
+ @Override
+ public boolean fetchedColumnIsQueried(ColumnMetadata column)
+ {
+ return true;
+ }
- @Override
- public String toString()
- {
- String prefix = "";
+ @Override
+ public boolean fetchedCellIsQueried(ColumnMetadata column, CellPath
path)
+ {
+ return true;
Review comment:
I was going to ask about whether it works correctly and I noticed that
you removed the relevant test case - why?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
