Update metadata tables for unpartitioned tables #285
Conversation
| Schema schema = new Schema(DataFile.getType(table.spec().partitionType()).fields()); | ||
| if (table.spec().fields().size() < 1) { | ||
| // avoid returning an empty struct, which is not always supported. instead, drop the partition field (id 102) | ||
| return TypeUtil.selectNot(schema, Sets.newHashSet(102)); |
There was a problem hiding this comment.
Should we enumerate these fields rather than using magic numbers (i.e. 102)?
There was a problem hiding this comment.
These IDs are part of the spec, so I think it is better to use the IDs than to use names. That's why I added a comment to explain which field is being removed.
|
|
||
| public static class FilesTableScan extends BaseTableScan { | ||
| private static final long TARGET_SPLIT_SIZE = 32 * 1024 * 1024; // 32 MB | ||
| private final Schema fileSchema; |
There was a problem hiding this comment.
Seems like we're just double capturing a field from BaseTableScan here. Seem like we should just expose a way to get at the original schema or use the refined schema.
There was a problem hiding this comment.
Right now, data tasks aren't responsible for projection because that's done easily by the engines. So we don't want to use the refined schema.
We could use the base table's schema without passing it through by making this a non-static inner class, but doing that seems odd with the refinement pattern to me. I thought it would be better to explicitly pass this through.
There was a problem hiding this comment.
I was thinking more of exposing the ability get the original, unrefined schema from the base table, which wouldn't require making this an inner static class.
I'll leave it up to you though as I don't have a strong opinion about it.
|
+1 LGTM |
2 participants
This removes the
partitioncolumn from thefilesandentriesmetadata tables when the underlying table is not partitioned. For unpartitioned tables, this is an empty struct and cannot be projected in Spark.In support of suppressing the
partitioncolumn, this also updatesManifestReaderandFilteredManifestto support schema-based projection. Because this is already updating theFilterableAPI, this fixes #145 and adds case sensitivity methods.