yifan-c commented on code in PR #254:
URL: 
https://github.com/apache/cassandra-analytics/pull/254#discussion_r4333023665


##########
cassandra-analytics-common/src/main/java/org/apache/cassandra/spark/data/FileSystemSSTable.java:
##########
@@ -91,6 +96,67 @@ public boolean isMissing(FileType fileType)
         return resolveComponentFile(fileType) == null;
     }
 
+    @NotNull
+    @Override
+    public Set<String> customComponentNames()
+    {
+        Path parent = dataFilePath.getParent();
+        if (parent == null)
+        {
+            return Collections.emptySet();
+        }
+
+        try (Stream<Path> files = Files.list(parent))
+        {
+            return files.filter(Files::isRegularFile)
+                        .map(p -> p.getFileName().toString())
+                        .filter(SSTable::isCustomComponentSupported)
+                        .collect(Collectors.toSet());

Review Comment:
   This lists the SAI components of all sstables in `parent` directory. Can you 
add another filter to only list the files matching the same filename prefix 
(i.e. same sstable)



##########
cassandra-analytics-common/src/main/java/org/apache/cassandra/spark/sparksql/filters/SaiFilter.java:
##########
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.cassandra.spark.sparksql.filters;
+
+import java.io.Serializable;
+import java.util.Objects;
+
+import org.apache.cassandra.spark.data.SaiIndex;
+import org.jetbrains.annotations.NotNull;
+
+/**
+ * SAI predicate used as an SSTable pruning hint.
+ */
+public final class SaiFilter implements Serializable
+{
+    private static final long serialVersionUID = 1L;
+
+    public enum Operator
+    {
+        EQ,
+        LT,
+        LTE,
+        GT,
+        GTE
+    }
+
+    @NotNull
+    private final SaiIndex index;
+    @NotNull
+    private final Operator operator;
+    @NotNull
+    private final String value;

Review Comment:
   Is string the best way to pass the predicate value? cc: @maedhroz 



##########
cassandra-analytics-core/src/main/java/org/apache/cassandra/spark/data/DataLayer.java:
##########
@@ -348,6 +368,219 @@ public Filter[] unsupportedPushDownFilters(Filter[] 
filters)
         return partitionKeys.size() > 0 ? filters : 
unsupportedFilters.toArray(new Filter[0]);
     }
 
+    /**
+     * @return SAI definitions available for this table.
+     */
+    @NotNull
+    public List<SaiIndex> saiIndexes()
+    {
+        return Collections.emptyList();
+    }
+
+    /**
+     * Extracts Spark predicates that can safely be used as SAI pruning hints.
+     *
+     * These predicates remain in {@link 
#unsupportedPushDownFilters(Filter[])} so Spark evaluates them after the
+     * Cassandra reader has reconciled all candidate partitions. SAI therefore 
narrows physical I/O without becoming
+     * part of the correctness boundary.
+     */
+    @NotNull
+    public List<SaiFilter> saiFilters(@NotNull Filter[] filters)
+    {
+        if (saiIndexes().isEmpty())
+        {
+            return Collections.emptyList();
+        }
+
+        List<SaiFilter> result = new ArrayList<>();
+        for (Filter filter : filters)
+        {
+            addSaiFilters(filter, result);
+        }
+        return result;
+    }
+
+    /**
+     * Adds SAI-capable predicates from a conjunctive Spark filter tree.
+     *
+     * A partially-indexable AND can still use its indexable branch because 
every final match must satisfy that
+     * branch. OR is not supported by Cassandra 5.0 SAI, so an OR subtree is 
never used for SAI pruning.
+     */
+    private void addSaiFilters(@NotNull Filter filter, @NotNull 
List<SaiFilter> result)
+    {
+        if (filter instanceof And)
+        {
+            And and = (And) filter;
+            addSaiFilters(and.left(), result);
+            addSaiFilters(and.right(), result);
+            return;
+        }
+        if (filter instanceof Or)
+        {
+            return;
+        }
+
+        SaiFilter saiFilter = toSaiFilter(filter);
+        if (saiFilter != null)
+        {
+            result.add(saiFilter);
+        }
+    }
+
+    @Nullable
+    private SaiFilter toSaiFilter(@NotNull Filter filter)
+    {
+        String attribute = filterAttribute(filter);
+        Object value = filterValue(filter);
+        SaiFilter.Operator operator = saiOperator(filter);
+        if (attribute == null || value == null || operator == null)
+        {
+            return null;
+        }
+
+        SaiIndex index = saiIndexForAttribute(attribute);
+        return index == null ? null : new SaiFilter(index, operator, 
String.valueOf(value));

Review Comment:
   It uses the string representation of the value for filtering. Are all types 
supported? If not, what types are not supported? It'd be nice to have a 
document on what types are supported by the SAI filtering in reader



##########
cassandra-analytics-common/src/main/java/org/apache/cassandra/spark/sparksql/CellIterator.java:
##########
@@ -59,8 +59,8 @@ public abstract class CellIterator implements Iterator<Cell>, 
AutoCloseable
     private final long startTimeNanos;
     @NotNull
     private final StreamScanner<RowData> scanner;
-    @NotNull
-    private final RowData rowData;
+    @Nullable
+    private RowData rowData;

Review Comment:
   What's driving the change from NotNull to Nullable? Do you hit a NPE?



##########
cassandra-analytics-common/src/main/java/org/apache/cassandra/spark/data/SSTable.java:
##########
@@ -94,6 +104,87 @@ public InputStream openDataStream()
 
     public abstract boolean isMissing(FileType fileType);
 
+    /**
+     * Returns non-standard SSTable component file names exposed by this 
SSTable.
+     *
+     * SAI uses dynamically named custom components (files beginning with 
{@code SAI+})
+     * which cannot be represented by {@link FileType} enumeration.
+     */
+    @NotNull
+    public Set<String> customComponentNames()

Review Comment:
   Wondering if renaming to `saiComponentNames` is more clear. 
   The custom component in Cassandra catches all unrecognized types. The SAI 
index components are not that. Each SAI component is a registered type with its 
own pattern.
   
   Plus the implementations are SAI specific. 



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

To unsubscribe, e-mail: [email protected]

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