anoopj commented on code in PR #16958:
URL: https://github.com/apache/iceberg/pull/16958#discussion_r3548168670
##########
core/src/main/java/org/apache/iceberg/Partitioning.java:
##########
@@ -238,9 +238,18 @@ public static StructType groupingKeyType(Schema schema,
Collection<PartitionSpec
* @return the constructed unified partition type
*/
public static StructType partitionType(Table table) {
- Collection<PartitionSpec> specs = table.specs().values();
- return buildPartitionProjectionType(
- "table partition", specs, allActiveFieldIds(table.schema(), specs));
+ return partitionType(table.schema(), table.specs().values());
+ }
+
+ /**
+ * Builds a unified partition type from a schema and its specs, unioning
every partition field
+ * whose source column is present in the schema.
+ *
+ * @param schema the schema used to determine which partition fields are
active
+ * @param specs the partition specs to unify
+ */
+ static StructType partitionType(Schema schema, Collection<PartitionSpec>
specs) {
+ return buildPartitionProjectionType("table partition", specs,
allActiveFieldIds(schema, specs));
Review Comment:
Thanks for pointing this out - I didn't think about this scenario. We can
fix it by dropping the filter on active fields, but there might be another
problem: partition specs don't store the type, and we derive it based on the
type of the source column. So if the source column doesn't exist in the schema
anymore, we can't derive the partition type. I think this might be working in
v3 because of the way the reader is implemented: the reader gets the schema
from the Avro schema, and uses it instead of the schema from the metadata.
Maybe the Parquet reader can also work that way (can verify), but is this by
design? Does the other Iceberg implementations actually work in this scenario
in v3 if this detail is not in the spec?
--
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]
