[spark] Fix Fluss Spark read paths - #4237
Open
fkzhao wants to merge 1 commit into
Open
Conversation
Fix Spark reads for Iceberg sort-merge tables and append log scans. Handle empty and progress-only log scan batches without reporting a false end of data, and preserve consumed offsets when reading full projections. Use independent rows and the correct timestamp representation in Iceberg sort-merge reads to avoid projection and timestamp failures. Add regression coverage for the affected Iceberg and log-change reader paths.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new Iceberg key comparator has confirmed ordering bugs for TIMESTAMP_NANO precision handling and signed byte comparisons that can mis-order primary keys.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR fixes several Spark read-path edge cases in Fluss integrations: bounded append log scans that can return empty/progress-only batches, and Iceberg sort-merge reads that must preserve correct primary-key ordering and avoid mutable row reuse when projecting full rows.
Changes:
- Update
FlussAppendPartitionReaderto handle empty/progress-only polls and only advance offsets after finishing a batch. - Make
LogChangesIteratorrobust to empty inputs and add a regression test. - Add an Iceberg
SortedRecordReaderimplementation for sort-merge union reads and extend Iceberg lake-source tests.
File summaries
| File | Description |
|---|---|
| fluss-spark/fluss-spark-ut/src/test/scala/org/apache/fluss/spark/utils/LogChangesIteratorTest.scala | Adds regression coverage for empty log-change inputs. |
| fluss-spark/fluss-spark-common/src/main/scala/org/apache/fluss/spark/utils/LogChangesIterator.scala | Avoids .head on empty record sets by using an empty iterator. |
| fluss-spark/fluss-spark-common/src/main/scala/org/apache/fluss/spark/read/FlussAppendPartitionReader.scala | Handles empty/progress-only scan batches and defers applying consumed offsets until the batch is drained. |
| fluss-lake/fluss-lake-iceberg/src/test/java/org/apache/fluss/lake/iceberg/source/IcebergLakeSourceTest.java | Adds coverage for sorted readers with missing splits and PK projections. |
| fluss-lake/fluss-lake-iceberg/src/main/java/org/apache/fluss/lake/iceberg/source/IcebergSortedRecordReader.java | Introduces a sorted Iceberg reader for primary-key sort/merge. |
| fluss-lake/fluss-lake-iceberg/src/main/java/org/apache/fluss/lake/iceberg/source/IcebergRecordReader.java | Ensures per-record row objects are independent to prevent projection/mutation issues. |
| fluss-lake/fluss-lake-iceberg/src/main/java/org/apache/fluss/lake/iceberg/source/IcebergLakeSource.java | Returns the sorted reader when the reader context requires sorted records. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+146
to
+167
| case TIMESTAMP: | ||
| return compareTimestamp( | ||
| row1, row2, position, ((Types.TimestampType) type).shouldAdjustToUTC()); | ||
| case TIMESTAMP_NANO: | ||
| return compareTimestamp( | ||
| row1, row2, position, ((Types.TimestampNanoType) type).shouldAdjustToUTC()); | ||
| case BINARY: | ||
| case FIXED: | ||
| return compareBytes(row1.getBytes(position), row2.getBytes(position)); | ||
| default: | ||
| throw new UnsupportedOperationException( | ||
| "Unsupported Iceberg identifier type: " + type.typeId()); | ||
| } | ||
| } | ||
|
|
||
| private int compareTimestamp( | ||
| InternalRow row1, InternalRow row2, int position, boolean shouldAdjustToUTC) { | ||
| if (shouldAdjustToUTC) { | ||
| return row1.getTimestampLtz(position, 6).compareTo(row2.getTimestampLtz(position, 6)); | ||
| } | ||
| return row1.getTimestampNtz(position, 6).compareTo(row2.getTimestampNtz(position, 6)); | ||
| } |
Comment on lines
+171
to
+176
| for (int i = 0; i < length; i++) { | ||
| int result = Byte.compare(bytes1[i], bytes2[i]); | ||
| if (result != 0) { | ||
| return result; | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix Spark reads for Iceberg sort-merge tables and append log scans.
Handle empty and progress-only log scan batches without reporting a false end of data, and preserve consumed offsets when reading full projections. Use independent rows and the correct timestamp representation in Iceberg sort-merge reads to avoid projection and timestamp failures.
Add regression coverage for the affected Iceberg and log-change reader paths.
Purpose
Fix Spark read failures when querying Fluss tables with full-column projections such as
SELECT *.The Spark append reader previously treated an empty
ScanRecordsresult as end-of-data. However, a scan result may be empty while the scanner has already advanced its consumed offset, or may simply be an empty poll caused by a timeout. This caused Spark tasks to fail with an incorrectNo more data from fluss serverexception.This change also fixes Iceberg sort-merge reading and empty log-change iterator handling.
Brief change log
FlussAppendPartitionReader.consumedUpToOffsetafter the current batch is consumed.LogChangesIterator.Tests
IcebergLakeSourceTest: 5 tests passed.Spark common module compilation passed.
Spark 3.5 connector packaging passed with:
mvn -pl fluss-spark/fluss-spark-3.5 \ -am \ -DskipTests \ -Dcheckstyle.skip=true \ -Drat.skip=true \ package