rdblue commented on a change in pull request #3400:
URL: https://github.com/apache/iceberg/pull/3400#discussion_r740529694
##########
File path:
spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkBatchQueryScan.java
##########
@@ -104,30 +144,100 @@
}
}
- if (splitSize != null) {
- scan = scan.option(TableProperties.SPLIT_SIZE, splitSize.toString());
+ for (Expression filter : filterExpressions()) {
+ scan = scan.filter(filter);
}
- if (splitLookback != null) {
- scan = scan.option(TableProperties.SPLIT_LOOKBACK,
splitLookback.toString());
+ try (CloseableIterable<FileScanTask> filesIterable = scan.planFiles()) {
+ this.files = Lists.newArrayList(filesIterable);
+ } catch (IOException e) {
+ throw new UncheckedIOException("Failed to close table scan: " + scan,
e);
}
+ }
- if (splitOpenFileCost != null) {
- scan = scan.option(TableProperties.SPLIT_OPEN_FILE_COST,
splitOpenFileCost.toString());
- }
+ return files;
+ }
- for (Expression filter : filterExpressions()) {
- scan = scan.filter(filter);
+ @Override
+ protected List<CombinedScanTask> tasks() {
+ if (tasks == null) {
+ CloseableIterable<FileScanTask> splitFiles = TableScanUtil.splitFiles(
+ CloseableIterable.withNoopClose(files()),
+ splitSize);
+ CloseableIterable<CombinedScanTask> scanTasks = TableScanUtil.planTasks(
+ splitFiles, splitSize,
+ splitLookback, splitOpenFileCost);
+ tasks = Lists.newArrayList(scanTasks);
+ }
+
+ return tasks;
+ }
+
+ @Override
+ public NamedReference[] filterAttributes() {
+ Set<Integer> partitionFieldSourceIds = Sets.newHashSet();
+
+ for (Integer specId : specIds()) {
+ PartitionSpec spec = table().specs().get(specId);
+ for (PartitionField field : spec.fields()) {
+ partitionFieldSourceIds.add(field.sourceId());
}
+ }
- try (CloseableIterable<CombinedScanTask> tasksIterable =
scan.planTasks()) {
- this.tasks = Lists.newArrayList(tasksIterable);
- } catch (IOException e) {
- throw new RuntimeIOException(e, "Failed to close table scan: %s",
scan);
+ Map<Integer, String> nameById =
TypeUtil.indexQuotedNameById(table().schema().asStruct());
Review comment:
I don't think there's a problem here. Iceberg is going to use a
consistent set of names for a table that is determined at table load time. And
Iceberg metadata is tracked using field IDs, so even if a name changes it's
okay.
If you have a table partitioned by 1:a and change that to b in the schema,
then you have a table partitioned by 1:b. Because only one schema is reported
to Spark, you'll either use a or b and when Spark passes the filter back you'll
consistently get a or b in the predicate.
##########
File path:
spark/v3.2/spark/src/test/java/org/apache/iceberg/spark/source/TestRuntimeFiltering.java
##########
@@ -0,0 +1,300 @@
+/*
+ * 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.iceberg.spark.source;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import org.apache.iceberg.FileScanTask;
+import org.apache.iceberg.Table;
+import org.apache.iceberg.expressions.Expression;
+import org.apache.iceberg.expressions.Expressions;
+import org.apache.iceberg.io.CloseableIterable;
+import org.apache.iceberg.io.FileIO;
+import org.apache.iceberg.relocated.com.google.common.collect.Sets;
+import org.apache.iceberg.spark.SparkCatalogTestBase;
+import org.apache.iceberg.spark.SparkWriteOptions;
+import org.apache.spark.sql.Dataset;
+import org.apache.spark.sql.Row;
+import org.apache.spark.sql.catalyst.analysis.NoSuchTableException;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Test;
+
+import static org.apache.spark.sql.functions.date_add;
+import static org.apache.spark.sql.functions.expr;
+
+public class TestRuntimeFiltering extends SparkCatalogTestBase {
+
+ public TestRuntimeFiltering(String catalogName, String implementation,
Map<String, String> config) {
+ super(catalogName, implementation, config);
+ }
+
+ @After
+ public void removeTables() {
+ sql("DROP TABLE IF EXISTS %s", tableName);
+ sql("DROP TABLE IF EXISTS dim");
+ }
+
+ @Test
+ public void testIdentityPartitionedTable() throws NoSuchTableException {
+ sql("CREATE TABLE %s (id BIGINT, data STRING, date DATE, ts TIMESTAMP) " +
+ "USING iceberg " +
+ "PARTITIONED BY (date)", tableName);
+
+ Dataset<Row> df = spark.range(1, 100)
+ .withColumn("date", date_add(expr("DATE '1970-01-01'"), expr("CAST(id
% 4 AS INT)")))
+ .withColumn("ts", expr("TO_TIMESTAMP(date)"))
+ .withColumn("data", expr("CAST(date AS STRING)"))
+ .select("id", "data", "date", "ts");
+
+ df.coalesce(1).writeTo(tableName).option(SparkWriteOptions.FANOUT_ENABLED,
"true").append();
+
+ sql("CREATE TABLE dim (id BIGINT, date DATE) USING parquet");
+ Dataset<Row> dimDF = spark.range(1, 10)
+ .withColumn("date", expr("DATE '1970-01-02'"))
+ .select("id", "date");
+ dimDF.coalesce(1).write().mode("append").insertInto("dim");
+
+ String query = String.format(
+ "SELECT f.* FROM %s f JOIN dim d ON f.date = d.date AND d.id = 1 ORDER
BY id",
+ tableName);
+
+ assertQueryContainsRuntimeFilter(query);
+
+ deleteNotMatchingFiles(Expressions.equal("date", 1), 3);
Review comment:
This would be `DATE '1970-01-02'` if the "date" column's type is `date`.
##########
File path:
spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkBatchQueryScan.java
##########
@@ -74,60 +101,158 @@
throw new IllegalArgumentException("Cannot only specify option
end-snapshot-id to do incremental scan");
}
- // look for split behavior overrides in options
- this.splitSize = Spark3Util.propertyAsLong(options,
SparkReadOptions.SPLIT_SIZE, null);
- this.splitLookback = Spark3Util.propertyAsInt(options,
SparkReadOptions.LOOKBACK, null);
- this.splitOpenFileCost = Spark3Util.propertyAsLong(options,
SparkReadOptions.FILE_OPEN_COST, null);
+ this.splitSize = table instanceof BaseMetadataTable ?
readConf.metadataSplitSize() : readConf.splitSize();
+ this.splitLookback = readConf.splitLookback();
+ this.splitOpenFileCost = readConf.splitOpenFileCost();
+ this.runtimeFilterExpressions = Lists.newArrayList();
+ }
+
+ private Set<Integer> specIds() {
+ if (specIds == null) {
+ Set<Integer> specIdSet = Sets.newHashSet();
+ for (FileScanTask file : files()) {
+ specIdSet.add(file.spec().specId());
+ }
+ this.specIds = specIdSet;
+ }
+
+ return specIds;
+ }
+
+ private List<FileScanTask> files() {
+ if (files == null) {
+ this.files = planFiles();
+ }
+
+ return files;
+ }
+
+ private List<FileScanTask> planFiles() {
+ TableScan scan = table()
+ .newScan()
+ .option(TableProperties.SPLIT_SIZE, String.valueOf(splitSize))
+ .option(TableProperties.SPLIT_LOOKBACK, String.valueOf(splitLookback))
+ .option(TableProperties.SPLIT_OPEN_FILE_COST,
String.valueOf(splitOpenFileCost))
+ .caseSensitive(caseSensitive())
+ .project(expectedSchema());
+
+ if (snapshotId != null) {
+ scan = scan.useSnapshot(snapshotId);
+ }
+
+ if (asOfTimestamp != null) {
+ scan = scan.asOfTime(asOfTimestamp);
+ }
+
+ if (startSnapshotId != null) {
+ if (endSnapshotId != null) {
+ scan = scan.appendsBetween(startSnapshotId, endSnapshotId);
+ } else {
+ scan = scan.appendsAfter(startSnapshotId);
+ }
+ }
+
+ for (Expression filter : filterExpressions()) {
+ scan = scan.filter(filter);
+ }
+
+ try (CloseableIterable<FileScanTask> filesIterable = scan.planFiles()) {
+ return Lists.newArrayList(filesIterable);
+ } catch (IOException e) {
+ throw new UncheckedIOException("Failed to close table scan: " + scan, e);
+ }
}
@Override
protected List<CombinedScanTask> tasks() {
if (tasks == null) {
- TableScan scan = table()
- .newScan()
- .caseSensitive(caseSensitive())
- .project(expectedSchema());
+ CloseableIterable<FileScanTask> splitFiles = TableScanUtil.splitFiles(
+ CloseableIterable.withNoopClose(files()),
+ splitSize);
+ CloseableIterable<CombinedScanTask> scanTasks = TableScanUtil.planTasks(
+ splitFiles, splitSize,
+ splitLookback, splitOpenFileCost);
+ tasks = Lists.newArrayList(scanTasks);
+ }
- if (snapshotId != null) {
- scan = scan.useSnapshot(snapshotId);
- }
+ return tasks;
+ }
- if (asOfTimestamp != null) {
- scan = scan.asOfTime(asOfTimestamp);
- }
+ @Override
+ public NamedReference[] filterAttributes() {
+ Set<Integer> partitionFieldSourceIds = Sets.newHashSet();
- if (startSnapshotId != null) {
- if (endSnapshotId != null) {
- scan = scan.appendsBetween(startSnapshotId, endSnapshotId);
- } else {
- scan = scan.appendsAfter(startSnapshotId);
- }
+ for (Integer specId : specIds()) {
+ PartitionSpec spec = table().specs().get(specId);
+ for (PartitionField field : spec.fields()) {
+ partitionFieldSourceIds.add(field.sourceId());
}
+ }
- if (splitSize != null) {
- scan = scan.option(TableProperties.SPLIT_SIZE, splitSize.toString());
- }
+ Map<Integer, String> quotedNameById = TypeUtil.indexQuotedNameById(
+ expectedSchema().asStruct(),
+ name -> String.format("`%s`", name.replace("`", "``")));
- if (splitLookback != null) {
- scan = scan.option(TableProperties.SPLIT_LOOKBACK,
splitLookback.toString());
- }
+ return partitionFieldSourceIds.stream()
+ .filter(fieldId -> expectedSchema().findField(fieldId) != null)
+ .map(fieldId ->
Spark3Util.toNamedReference(quotedNameById.get(fieldId)))
+ .toArray(NamedReference[]::new);
+ }
- if (splitOpenFileCost != null) {
- scan = scan.option(TableProperties.SPLIT_OPEN_FILE_COST,
splitOpenFileCost.toString());
+ @Override
+ public void filter(Filter[] filters) {
+ Expression runtimeFilterExpr = convertRuntimeFilters(filters);
+
+ if (runtimeFilterExpr != Expressions.alwaysTrue()) {
+ Map<Integer, Evaluator> evaluatorsBySpecId = Maps.newHashMap();
+
+ for (Integer specId : specIds()) {
+ PartitionSpec spec = table().specs().get(specId);
+ Expression inclusiveExpr =
Projections.inclusive(spec).project(runtimeFilterExpr);
+ Evaluator inclusive = new Evaluator(spec.partitionType(),
inclusiveExpr);
+ evaluatorsBySpecId.put(specId, inclusive);
}
- for (Expression filter : filterExpressions()) {
- scan = scan.filter(filter);
+ LOG.info("Trying to filter {} files using runtime filter {}",
files().size(), runtimeFilterExpr);
+
+ List<FileScanTask> filteredFiles = files().stream()
+ .filter(file -> {
+ Evaluator evaluator = evaluatorsBySpecId.get(file.spec().specId());
+ return evaluator.eval(file.file().partition());
+ })
+ .collect(Collectors.toList());
+
+ LOG.info("{}/{} files matched runtime filter {}", filteredFiles.size(),
files().size(), runtimeFilterExpr);
Review comment:
I agree, but I think that's something that we can possibly add later.
I'm not sure how to make data appear in the Spark UI from data sources.
--
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]
