rdblue commented on code in PR #16958:
URL: https://github.com/apache/iceberg/pull/16958#discussion_r3552954793
##########
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:
> So if the source column doesn't exist in the schema anymore, we can't
derive the partition type.
This is a good point to think about, but I think it will not affect what we
do in practice. It's possible to remove old partition specs, but in practice
this hardly ever happens. In addition, the only cases where we can't determine
the _output_ type of the transform are identity and truncate transforms.
Truncation isn't very common and identity columns are unlikely to be removed
from the table.
Because there are good reasons to think this case is unlikely, I think we
should move forward assuming that the partition specs will be there. If we want
to be more careful, we can require a metadata check to remove old specs to
avoid breaking anything. Also, we can always go back and get the type from
existing metadata files at read time if we can't determine the type.
I've also thought about storing the output type of transforms when we add
column ranges for partition output values, like we do for UDF result types. We
may just need to solve this by storing the output 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: [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]
