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/05/07 08:13:21 UTC
[GitHub] [iceberg] singhpk234 opened a new pull request, #4720: [Core] : Fix query failure when using projection on top of partitions table
singhpk234 opened a new pull request, #4720:
URL: https://github.com/apache/iceberg/pull/4720
Closes https://github.com/apache/iceberg/issues/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 :
> 22/05/06 16:12:25 ERROR SparkSQLDriver: Failed in [select file_count from spark_catalog.monitoring.test.partitions]
java.lang.IllegalArgumentException: Cannot find source column: partition.date
as present in reported ticket
Though in master we will get :
>Cannot find source column: 1000
java.lang.NullPointerException: Cannot find source column: 1000
---
Testing Done :
Added UT fails without the change
cc @szehon-ho @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] RussellSpitzer commented on a diff in pull request #4720: Core: Fix query failure when using projection on top of partitions metadata table
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on code in PR #4720:
URL: https://github.com/apache/iceberg/pull/4720#discussion_r867345949
##########
spark/v3.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMetadataTables.java:
##########
@@ -171,6 +171,13 @@ public void testPartitionedTable() throws Exception {
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 =
Review 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.
--
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 pull request #4720: Core: Fix query failure when using projection on top of partitions metadata table
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on PR #4720:
URL: https://github.com/apache/iceberg/pull/4720#issuecomment-1121361571
Thanks @singhpk234 for the PR and @abmo-x for review!
--
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 #4720: Core: Fix query failure when using projection on top of partitions metadata table
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on code in PR #4720:
URL: https://github.com/apache/iceberg/pull/4720#discussion_r868233958
##########
spark/v3.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMetadataTables.java:
##########
@@ -171,6 +171,13 @@ public void testPartitionedTable() throws Exception {
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 =
Review Comment:
Thanks! Otherwise we wouldn't have an automated test running on this with "Core" only changes.
--
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 #4720: Core: Fix query failure when using projection on top of partitions metadata table
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on code in PR #4720:
URL: https://github.com/apache/iceberg/pull/4720#discussion_r867345558
##########
core/src/main/java/org/apache/iceberg/PartitionsTable.java:
##########
@@ -152,7 +152,7 @@ static CloseableIterable<FileScanTask> planFiles(StaticTableScan scan) {
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);
Review Comment:
So in this case "Schema" is the projected schema which is missing all the partition columns and "tableSchema" has the full partitionSpec. LGTM
--
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 #4720: Core: Fix query failure when using projection on top of partitions metadata table
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on code in PR #4720:
URL: https://github.com/apache/iceberg/pull/4720#discussion_r867345977
##########
spark/v3.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMetadataTables.java:
##########
@@ -171,6 +171,13 @@ public void testPartitionedTable() throws Exception {
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 =
Review Comment:
@singhpk234 ^
--
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] singhpk234 commented on a diff in pull request #4720: Core: Fix query failure when using projection on top of partitions metadata table
Posted by GitBox <gi...@apache.org>.
singhpk234 commented on code in PR #4720:
URL: https://github.com/apache/iceberg/pull/4720#discussion_r867348816
##########
spark/v3.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMetadataTables.java:
##########
@@ -171,6 +171,13 @@ public void testPartitionedTable() throws Exception {
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 =
Review Comment:
+1 added a UT for the same, 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] szehon-ho commented on pull request #4720: Core: Fix query failure when using projection on top of partitions metadata table
Posted by GitBox <gi...@apache.org>.
szehon-ho commented on PR #4720:
URL: https://github.com/apache/iceberg/pull/4720#issuecomment-1121385349
Arg, sorry about the initial bug, thanks @singhpk234 for the fix and all for review
--
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] abmo-x commented on pull request #4720: Core: Fix query failure when using projection on top of partitions metadata table
Posted by GitBox <gi...@apache.org>.
abmo-x commented on PR #4720:
URL: https://github.com/apache/iceberg/pull/4720#issuecomment-1120293063
Thanks @singhpk234 for a quick turn around, appreciate it!
--
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 merged pull request #4720: Core: Fix query failure when using projection on top of partitions metadata table
Posted by GitBox <gi...@apache.org>.
RussellSpitzer merged PR #4720:
URL: https://github.com/apache/iceberg/pull/4720
--
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