Core: Fix query failure when using projection on top of partitions metadata table#4720
Conversation
singhpk234
force-pushed
the
fix/ICEBERG-4718
branch
from
6f74a52 to
3fd3889
Compare
singhpk234
changed the title
singhpk234
changed the title
| LoadingCache<Integer, ManifestEvaluator> evalCache = Caffeine.newBuilder().build(specId -> { | ||
| PartitionSpec spec = table.specs().get(specId); | ||
| PartitionSpec transformedSpec = transformSpec(scan.schema(), spec); | ||
| PartitionSpec transformedSpec = transformSpec(scan.tableSchema(), spec); |
There was a problem hiding this comment.
So in this case "Schema" is the projected schema which is missing all the partition columns and "tableSchema" has the full partitionSpec. LGTM
| Assert.assertEquals("Metadata table should return one data file", 1, actualDataFiles.size()); | ||
| TestHelpers.assertEqualsSafe(filesTableSchema.asStruct(), expectedDataFiles.get(0), actualDataFiles.get(0)); | ||
|
|
||
| List<Row> actualPartitionsWithProjection = |
There was a problem hiding this comment.
While I like this test I believe we should also add one to https://github.com/apache/iceberg/blob/4ae2002bd46bf8e1c20db03cffc6319237e4d74a/core/src/test/java/org/apache/iceberg/TestMetadataTableScans.java
To make sure we have one in the core module.
There was a problem hiding this comment.
+1 added a UT for the same, thanks @RussellSpitzer !!!
There was a problem hiding this comment.
Thanks! Otherwise we wouldn't have an automated test running on this with "Core" only changes.
|
Thanks @singhpk234 for a quick turn around, appreciate it! |
|
Thanks @singhpk234 for the PR and @abmo-x for review! |
|
Arg, sorry about the initial bug, thanks @singhpk234 for the fix and all for review |
…ions metadata table (apache#4720)
…tadata table (apache#4720) (apache#619) Co-authored-by: Prashant Singh <[email protected]>
4 participants
Closes #4718
As per my understanding, we should use scan.tableSchema() rather than scan.schema() for transformSpec when planFiles() for partitions metaData table.
Hence we faced :
as present in reported ticket
Though in master we will get :
Testing Done :
Added UT fails without the change
cc @szehon-ho @RussellSpitzer