aokolnychyi commented on code in PR #6163: URL: https://github.com/apache/iceberg/pull/6163#discussion_r1019734173
########## core/src/main/java/org/apache/iceberg/Partitioning.java: ########## @@ -195,41 +198,68 @@ public Void alwaysNull(int fieldId, String sourceName, int sourceId) { } /** - * Builds a common partition type for all specs in a table. + * Builds an intersection of all partition types in a table. * - * <p>Whenever a table has multiple specs, the partition type is a struct containing all columns - * that have ever been a part of any spec in the table. + * <p>Whenever a table has multiple specs, the common partition type is a struct containing only + * fields that are present in every spec of the table. In other words, the struct fields represent + * an intersection of all partition types. * - * @param table a table with one or many specs + * @param specs one or many specs * @return the constructed common partition type */ + public static StructType commonPartitionType(Collection<PartitionSpec> specs) { + return buildPartitionType(specs, true /* only common fields */); + } + + /** + * Builds a unified partition type considering all specs in a table. + * + * <p>Whenever a table has multiple specs, the partition type is a struct containing all fields + * that have ever been a part of any spec in the table. In other words, the struct fields + * represent a union of all known partition fields. + * + * @param table a table with one or many specs + * @return the constructed unified partition type + */ public static StructType partitionType(Table table) { + return buildPartitionType(table.specs().values(), false /* only common fields */); + } + + private static StructType buildPartitionType( + Collection<PartitionSpec> specs, boolean onlyCommonFields) { + // we currently don't know the output type of unknown transforms - List<Transform<?, ?>> unknownTransforms = collectUnknownTransforms(table); + List<Transform<?, ?>> unknownTransforms = collectUnknownTransforms(specs); ValidationException.check( unknownTransforms.isEmpty(), - "Cannot build table partition type, unknown transforms: %s", + "Cannot build partition type, unknown transforms: %s", unknownTransforms); - if (table.specs().size() == 1) { - return table.spec().partitionType(); + if (specs.size() == 1) { + PartitionSpec spec = Iterables.getOnlyElement(specs); + return spec.partitionType(); } Map<Integer, PartitionField> fieldMap = Maps.newHashMap(); Map<Integer, Type> typeMap = Maps.newHashMap(); Map<Integer, String> nameMap = Maps.newHashMap(); - // sort the spec IDs in descending order to pick up the most recent field names - List<Integer> specIds = - table.specs().keySet().stream() - .sorted(Collections.reverseOrder()) + // sort specs by ID in descending order to pick up the most recent field names + List<PartitionSpec> sortedSpecs = + specs.stream() + .sorted(Comparator.comparingLong(PartitionSpec::specId).reversed()) .collect(Collectors.toList()); - for (Integer specId : specIds) { - PartitionSpec spec = table.specs().get(specId); + Set<Integer> projectedFieldIds = onlyCommonFields ? commonPartitionFieldIds(specs) : null; Review Comment: Changed. -- 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-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org