You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/11/29 16:57:54 UTC
[GitHub] [iceberg] aokolnychyi opened a new pull request, #6309: Spark 3.3: Consume arbitrary scans in SparkBatchQueryScan
aokolnychyi opened a new pull request, #6309:
URL: https://github.com/apache/iceberg/pull/6309
This PR allows using passing arbitrary scans in `SparkBatchQueryScan`. This change is needed to support tables that only implement the `BatchScan` interface. In addition, it migrates to `ScanTaskGroup` API, which is required for storage-partitioned joins.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a diff in pull request #6309: Spark 3.3: Consume arbitrary scans in SparkBatchQueryScan
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on code in PR #6309:
URL: https://github.com/apache/iceberg/pull/6309#discussion_r1035244113
##########
spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/source/SparkBatchQueryScan.java:
##########
@@ -63,7 +64,7 @@ class SparkBatchQueryScan extends SparkScan implements SupportsRuntimeFiltering
private static final Logger LOG = LoggerFactory.getLogger(SparkBatchQueryScan.class);
- private final TableScan scan;
+ private final Scan<?, ?, ?> scan;
Review Comment:
It's verbose, but it is a bit less scary to me than just ?,? ,?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #6309: Spark 3.3: Consume arbitrary scans in SparkBatchQueryScan
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #6309:
URL: https://github.com/apache/iceberg/pull/6309#discussion_r1035231519
##########
spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/source/SparkScanBuilder.java:
##########
@@ -213,40 +215,53 @@ public Scan build() {
Schema expectedSchema = schemaWithMetadataColumns();
- TableScan scan =
- table
- .newScan()
- .caseSensitive(caseSensitive)
- .filter(filterExpression())
- .project(expectedSchema);
+ if (startSnapshotId == null) {
+ BatchScan scan =
Review Comment:
Funny enough, I did that and then wasn't sure whether I overcomplicate. I can give it another try.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a diff in pull request #6309: Spark 3.3: Consume arbitrary scans in SparkBatchQueryScan
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on code in PR #6309:
URL: https://github.com/apache/iceberg/pull/6309#discussion_r1035247127
##########
spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/source/SparkScanBuilder.java:
##########
@@ -213,40 +215,53 @@ public Scan build() {
Schema expectedSchema = schemaWithMetadataColumns();
- TableScan scan =
- table
- .newScan()
- .caseSensitive(caseSensitive)
- .filter(filterExpression())
- .project(expectedSchema);
+ if (startSnapshotId == null) {
+ BatchScan scan =
Review Comment:
We can always hold off till we get a third scan type
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #6309: Spark 3.3: Consume arbitrary scans in SparkBatchQueryScan
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #6309:
URL: https://github.com/apache/iceberg/pull/6309#discussion_r1035037531
##########
spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/source/SparkScanBuilder.java:
##########
@@ -213,40 +215,53 @@ public Scan build() {
Schema expectedSchema = schemaWithMetadataColumns();
- TableScan scan =
- table
- .newScan()
- .caseSensitive(caseSensitive)
- .filter(filterExpression())
- .project(expectedSchema);
+ if (startSnapshotId == null) {
+ BatchScan scan =
Review Comment:
@szehon-ho, this should be all your need for the position deletes metadata table, apart from readers.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi merged pull request #6309: Spark 3.3: Consume arbitrary scans in SparkBatchQueryScan
Posted by GitBox <gi...@apache.org>.
aokolnychyi merged PR #6309:
URL: https://github.com/apache/iceberg/pull/6309
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #6309: Spark 3.3: Consume arbitrary scans in SparkBatchQueryScan
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #6309:
URL: https://github.com/apache/iceberg/pull/6309#discussion_r1035029464
##########
spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/source/SparkBatchQueryScan.java:
##########
@@ -109,40 +110,49 @@ Long snapshotId() {
private Set<Integer> specIds() {
if (specIds == null) {
Set<Integer> specIdSet = Sets.newHashSet();
- for (FileScanTask file : files()) {
- specIdSet.add(file.spec().specId());
+ for (PartitionScanTask task : tasks()) {
+ specIdSet.add(task.spec().specId());
}
this.specIds = specIdSet;
}
return specIds;
}
- private List<FileScanTask> files() {
- if (files == null) {
- try (CloseableIterable<FileScanTask> filesIterable = scan.planFiles()) {
- this.files = Lists.newArrayList(filesIterable);
+ private List<PartitionScanTask> tasks() {
+ if (tasks == null) {
+ try (CloseableIterable<? extends ScanTask> taskIterable = scan.planFiles()) {
+ List<PartitionScanTask> partitionScanTasks = Lists.newArrayList();
+ for (ScanTask task : taskIterable) {
+ ValidationException.check(
+ task instanceof PartitionScanTask,
Review Comment:
`PartitionScanTask` is a very abstract class extended by all tasks right now. I can't just use arbitrary tasks as this scan supports runtime filtering and needs to know the partition spec and partition of each task.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #6309: Spark 3.3: Consume arbitrary scans in SparkBatchQueryScan
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #6309:
URL: https://github.com/apache/iceberg/pull/6309#discussion_r1035023768
##########
spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/source/SparkBatch.java:
##########
@@ -95,43 +96,61 @@ public PartitionReaderFactory createReaderFactory() {
}
private int batchSize() {
- if (parquetOnly() && parquetBatchReadsEnabled()) {
+ if (useParquetBatchReads()) {
return readConf.parquetBatchSize();
- } else if (orcOnly() && orcBatchReadsEnabled()) {
+ } else if (useOrcBatchReads()) {
return readConf.orcBatchSize();
} else {
return 0;
}
}
- private boolean parquetOnly() {
- return taskGroups.stream()
- .allMatch(task -> !task.isDataTask() && onlyFileFormat(task, FileFormat.PARQUET));
- }
-
- private boolean parquetBatchReadsEnabled() {
+ // conditions for using Parquet batch reads:
+ // - Parquet vectorization is enabled
+ // - at least one column is projected
+ // - only primitives are projected
+ // - all tasks are of FileScanTask type and read only Parquet files
+ private boolean useParquetBatchReads() {
return readConf.parquetVectorizationEnabled()
- && // vectorization enabled
Review Comment:
Spotless formatted these comments in a weird way so moved them into the comment on the method.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #6309: Spark 3.3: Consume arbitrary scans in SparkBatchQueryScan
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #6309:
URL: https://github.com/apache/iceberg/pull/6309#discussion_r1035022903
##########
spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/source/SparkBatch.java:
##########
@@ -51,7 +52,7 @@ class SparkBatch implements Batch {
JavaSparkContext sparkContext,
Table table,
SparkReadConf readConf,
- List<CombinedScanTask> taskGroups,
+ List<? extends ScanTaskGroup<? extends ScanTask>> taskGroups,
Review Comment:
Methods for planning task groups that preserve grouping work with `ScanTaskGroup` directly.
Also, `BatchScan` returns `ScanTaskGroup`. `CombinedScanTask` extends `ScanTaskGroup `.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #6309: Spark 3.3: Consume arbitrary scans in SparkBatchQueryScan
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #6309:
URL: https://github.com/apache/iceberg/pull/6309#discussion_r1037410284
##########
spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/source/SparkScanBuilder.java:
##########
@@ -213,40 +215,53 @@ public Scan build() {
Schema expectedSchema = schemaWithMetadataColumns();
- TableScan scan =
- table
- .newScan()
- .caseSensitive(caseSensitive)
- .filter(filterExpression())
- .project(expectedSchema);
+ if (startSnapshotId == null) {
+ BatchScan scan =
Review Comment:
Went ahead and added separate methods in this PR.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #6309: Spark 3.3: Consume arbitrary scans in SparkBatchQueryScan
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #6309:
URL: https://github.com/apache/iceberg/pull/6309#discussion_r1035030706
##########
spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/source/SparkScanBuilder.java:
##########
@@ -213,40 +215,53 @@ public Scan build() {
Schema expectedSchema = schemaWithMetadataColumns();
- TableScan scan =
- table
- .newScan()
- .caseSensitive(caseSensitive)
- .filter(filterExpression())
- .project(expectedSchema);
+ if (startSnapshotId == null) {
+ BatchScan scan =
Review Comment:
Instead of `TableScan`, we use `BatchScan` for regular queries and `IncrementalAppendScan` for incremental appends.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on pull request #6309: Spark 3.3: Consume arbitrary scans in SparkBatchQueryScan
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on PR #6309:
URL: https://github.com/apache/iceberg/pull/6309#issuecomment-1330988871
cc @flyrain @RussellSpitzer @szehon-ho @rdblue
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a diff in pull request #6309: Spark 3.3: Consume arbitrary scans in SparkBatchQueryScan
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on code in PR #6309:
URL: https://github.com/apache/iceberg/pull/6309#discussion_r1037424678
##########
spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/source/SparkScanBuilder.java:
##########
@@ -211,11 +213,19 @@ public Scan build() {
SparkReadOptions.END_SNAPSHOT_ID,
SparkReadOptions.START_SNAPSHOT_ID);
+ if (startSnapshotId != null) {
+ return buildIncrementalAppendScan(startSnapshotId, endSnapshotId);
Review Comment:
I do think this is cleaner to me as a code reviewer 👍
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on pull request #6309: Spark 3.3: Consume arbitrary scans in SparkBatchQueryScan
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on PR #6309:
URL: https://github.com/apache/iceberg/pull/6309#issuecomment-1334137970
I tried adding boundaries but it was mostly useless as we need to support arbitrary types. The current approach seems most reasonable at the moment.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on pull request #6309: Spark 3.3: Consume arbitrary scans in SparkBatchQueryScan
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on PR #6309:
URL: https://github.com/apache/iceberg/pull/6309#issuecomment-1334290414
Thanks, @RussellSpitzer!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on pull request #6309: Spark 3.3: Consume arbitrary scans in SparkBatchQueryScan
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on PR #6309:
URL: https://github.com/apache/iceberg/pull/6309#issuecomment-1331642485
Let me actually take a step back and see if we can have some reasonable type boundaries.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a diff in pull request #6309: Spark 3.3: Consume arbitrary scans in SparkBatchQueryScan
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on code in PR #6309:
URL: https://github.com/apache/iceberg/pull/6309#discussion_r1035200849
##########
spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/source/SparkScanBuilder.java:
##########
@@ -213,40 +215,53 @@ public Scan build() {
Schema expectedSchema = schemaWithMetadataColumns();
- TableScan scan =
- table
- .newScan()
- .caseSensitive(caseSensitive)
- .filter(filterExpression())
- .project(expectedSchema);
+ if (startSnapshotId == null) {
+ BatchScan scan =
Review Comment:
Maybe this doesn't have to happen yet, but does it make sense to break out some of this into some private helpers
```java
if (startingSnapshot) {
buildIncrementalScan()
return new SparkBatchQueryScan()
} else {
buildBatchScan()
return new SparkBatchQueryScan()
}
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a diff in pull request #6309: Spark 3.3: Consume arbitrary scans in SparkBatchQueryScan
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on code in PR #6309:
URL: https://github.com/apache/iceberg/pull/6309#discussion_r1035190450
##########
spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/source/SparkBatchQueryScan.java:
##########
@@ -63,7 +64,7 @@ class SparkBatchQueryScan extends SparkScan implements SupportsRuntimeFiltering
private static final Logger LOG = LoggerFactory.getLogger(SparkBatchQueryScan.class);
- private final TableScan scan;
+ private final Scan<?, ?, ?> scan;
Review Comment:
it would be nice if Java had type macros :(
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #6309: Spark 3.3: Consume arbitrary scans in SparkBatchQueryScan
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #6309:
URL: https://github.com/apache/iceberg/pull/6309#discussion_r1035234966
##########
spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/source/SparkBatchQueryScan.java:
##########
@@ -63,7 +64,7 @@ class SparkBatchQueryScan extends SparkScan implements SupportsRuntimeFiltering
private static final Logger LOG = LoggerFactory.getLogger(SparkBatchQueryScan.class);
- private final TableScan scan;
+ private final Scan<?, ?, ?> scan;
Review Comment:
We could do something like this (if it is any better):
```
private final Scan<?, ? extends ScanTask, ?> scan
private final Scan<?, ? extends ScanTask, ? extends ScanTaskGroup<?>> scan
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #6309: Spark 3.3: Consume arbitrary scans in SparkBatchQueryScan
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #6309:
URL: https://github.com/apache/iceberg/pull/6309#discussion_r1035032134
##########
spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/source/SparkScanBuilder.java:
##########
@@ -213,40 +215,53 @@ public Scan build() {
Schema expectedSchema = schemaWithMetadataColumns();
- TableScan scan =
- table
- .newScan()
- .caseSensitive(caseSensitive)
- .filter(filterExpression())
- .project(expectedSchema);
+ if (startSnapshotId == null) {
+ BatchScan scan =
+ table
+ .newBatchScan()
+ .caseSensitive(caseSensitive)
+ .filter(filterExpression())
+ .project(expectedSchema);
- if (snapshotId != null) {
- scan = scan.useSnapshot(snapshotId);
- }
+ if (snapshotId != null) {
+ scan = scan.useSnapshot(snapshotId);
+ }
- if (asOfTimestamp != null) {
- scan = scan.asOfTime(asOfTimestamp);
- }
+ if (asOfTimestamp != null) {
+ scan = scan.asOfTime(asOfTimestamp);
+ }
- if (branch != null) {
- scan = scan.useRef(branch);
- }
+ if (branch != null) {
+ scan = scan.useRef(branch);
+ }
- if (tag != null) {
- scan = scan.useRef(tag);
- }
+ if (tag != null) {
+ scan = scan.useRef(tag);
+ }
+
+ scan = configureSplitPlanning(scan);
+
+ return new SparkBatchQueryScan(
+ spark, table, scan, readConf, expectedSchema, filterExpressions);
+
+ } else {
+ IncrementalAppendScan scan =
+ table
+ .newIncrementalAppendScan()
+ .fromSnapshotExclusive(startSnapshotId)
Review Comment:
I believe the `startSnapshotId` option is always exclusive.
```
// Start snapshot ID used in incremental scans (exclusive)
public static final String START_SNAPSHOT_ID = "start-snapshot-id";
```
We may want to add a new boolean option to make it configurable.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #6309: Spark 3.3: Consume arbitrary scans in SparkBatchQueryScan
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #6309:
URL: https://github.com/apache/iceberg/pull/6309#discussion_r1035023768
##########
spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/source/SparkBatch.java:
##########
@@ -95,43 +96,61 @@ public PartitionReaderFactory createReaderFactory() {
}
private int batchSize() {
- if (parquetOnly() && parquetBatchReadsEnabled()) {
+ if (useParquetBatchReads()) {
return readConf.parquetBatchSize();
- } else if (orcOnly() && orcBatchReadsEnabled()) {
+ } else if (useOrcBatchReads()) {
return readConf.orcBatchSize();
} else {
return 0;
}
}
- private boolean parquetOnly() {
- return taskGroups.stream()
- .allMatch(task -> !task.isDataTask() && onlyFileFormat(task, FileFormat.PARQUET));
- }
-
- private boolean parquetBatchReadsEnabled() {
+ // conditions for using Parquet batch reads:
+ // - Parquet vectorization is enabled
+ // - at least one column is projected
+ // - only primitives are projected
+ // - all tasks are of FileScanTask type and read only Parquet files
+ private boolean useParquetBatchReads() {
return readConf.parquetVectorizationEnabled()
- && // vectorization enabled
Review Comment:
Spotless formatted these comments in a weird way so moved them into the comment.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #6309: Spark 3.3: Consume arbitrary scans in SparkBatchQueryScan
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #6309:
URL: https://github.com/apache/iceberg/pull/6309#discussion_r1035026607
##########
spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/source/SparkBatchQueryScan.java:
##########
@@ -63,7 +64,7 @@ class SparkBatchQueryScan extends SparkScan implements SupportsRuntimeFiltering
private static final Logger LOG = LoggerFactory.getLogger(SparkBatchQueryScan.class);
- private final TableScan scan;
+ private final Scan<?, ?, ?> scan;
Review Comment:
I considered defining this variable type as below:
```
private final Scan<?, ? extends ScanTask, ? extends ScanTaskGroup<? extends ScanTask>> scan;
```
But it seemed like a very hard to read definition.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #6309: Spark 3.3: Consume arbitrary scans in SparkBatchQueryScan
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #6309:
URL: https://github.com/apache/iceberg/pull/6309#discussion_r1037409951
##########
spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/source/SparkBatchQueryScan.java:
##########
@@ -63,7 +64,7 @@ class SparkBatchQueryScan extends SparkScan implements SupportsRuntimeFiltering
private static final Logger LOG = LoggerFactory.getLogger(SparkBatchQueryScan.class);
- private final TableScan scan;
+ private final Scan<?, ?, ?> scan;
Review Comment:
Changed.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #6309: Spark 3.3: Consume arbitrary scans in SparkBatchQueryScan
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #6309:
URL: https://github.com/apache/iceberg/pull/6309#discussion_r1035235633
##########
spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/source/SparkBatchQueryScan.java:
##########
@@ -63,7 +64,7 @@ class SparkBatchQueryScan extends SparkScan implements SupportsRuntimeFiltering
private static final Logger LOG = LoggerFactory.getLogger(SparkBatchQueryScan.class);
- private final TableScan scan;
+ private final Scan<?, ?, ?> scan;
Review Comment:
The last seems reasonable. Unless there is a totally different idea.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org