blerer commented on a change in pull request #1031:
URL: https://github.com/apache/cassandra/pull/1031#discussion_r648425904



##########
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:
       In the 3.0 version the only use case where sub-selections are used is 
for slice queries on super column and the code always use a `selectionBuilder` 
for those scenarios. By consequence, the test use case is not a true use case 
in 3.0.  
   Moreover, sub selection through CQL queries is only possible since 4.0 (see 
CASSANDRA-7396) and Thrift support has been removed in 4.0 so mix-cluster 
queries (3.0-4.0 or 3.11-4.0)  involving sub-selections should never occurs.




-- 
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]

Reply via email to