You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "dramaticlly (via GitHub)" <gi...@apache.org> on 2026/05/01 00:47:09 UTC
[PR] [Core] Replace string-based schema projection with selection on field-id [iceberg]
dramaticlly opened a new pull request, #16184:
URL: https://github.com/apache/iceberg/pull/16184
Inspired by https://github.com/apache/iceberg/pull/16077#discussion_r3170215745
Use `TypeUtil.select/selectNot` with field IDs instead of `Schema.select` with string names in FileCleanupStrategy and PartitionsTable.
FYI @RussellSpitzer @amogh-jahagirdar
--
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
Re: [PR] Core: Replace string-based schema projection with selection on field-id [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on PR #16184:
URL: https://github.com/apache/iceberg/pull/16184#issuecomment-4389951106
Ah Amogh beat me
--
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
Re: [PR] Core: Replace string-based schema projection with selection on field-id [iceberg]
Posted by "gaborkaszab (via GitHub)" <gi...@apache.org>.
gaborkaszab commented on code in PR #16184:
URL: https://github.com/apache/iceberg/pull/16184#discussion_r3182512540
##########
core/src/main/java/org/apache/iceberg/PartitionsTable.java:
##########
@@ -102,16 +109,7 @@ public TableScan newScan() {
@Override
public Schema schema() {
if (unpartitionedTable) {
- return schema.select(
- "record_count",
- "file_count",
- "total_data_file_size_in_bytes",
- "position_delete_record_count",
- "position_delete_file_count",
- "equality_delete_record_count",
- "equality_delete_file_count",
- "last_updated_at",
- "last_updated_snapshot_id");
+ return TypeUtil.selectNot(schema, ImmutableSet.of(PARTITION_COLUMN_ID, SPEC_ID_COLUMN_ID));
Review Comment:
While the new version is shorter, I found it more readable to spell out what is included instead of what isn't
##########
core/src/main/java/org/apache/iceberg/PartitionsTable.java:
##########
@@ -37,6 +39,10 @@
/** A {@link Table} implementation that exposes a table's partitions as rows. */
public class PartitionsTable extends BaseMetadataTable {
+ // partition dependent field id
Review Comment:
I find it weird to introduce IDs for only 2 of the schema fields. I see the motivation that these are the ones referenced later, but still seems weird.
Maybe, would introducing each of the fields (not the IDs but the actual fields) as NestedField members of the class and then add these fields to the schema. With this we could get the fieldIds directly from the NestedField similarly to what you do in FileCleanupStrategy.
--
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
Re: [PR] Core: Replace string-based schema projection with selection on field-id [iceberg]
Posted by "dramaticlly (via GitHub)" <gi...@apache.org>.
dramaticlly commented on code in PR #16184:
URL: https://github.com/apache/iceberg/pull/16184#discussion_r3183609665
##########
core/src/main/java/org/apache/iceberg/PartitionsTable.java:
##########
@@ -37,6 +39,10 @@
/** A {@link Table} implementation that exposes a table's partitions as rows. */
public class PartitionsTable extends BaseMetadataTable {
+ // partition dependent field id
Review Comment:
Sounds good, moved all schema to the class members but only kept the field id for partitions as it depends on the table schema.
--
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
Re: [PR] Core: Replace string-based schema projection with selection on field-id [iceberg]
Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar merged PR #16184:
URL: https://github.com/apache/iceberg/pull/16184
--
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
Re: [PR] Core: Replace string-based schema projection with selection on field-id [iceberg]
Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar commented on PR #16184:
URL: https://github.com/apache/iceberg/pull/16184#issuecomment-4389947169
Thanks @dramaticlly and @gaborkaszab @RussellSpitzer for reviewing!
--
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
Re: [PR] Core: Replace string-based schema projection with selection on field-id [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on PR #16184:
URL: https://github.com/apache/iceberg/pull/16184#issuecomment-4389949612
Thanks for cleaning this up @dramaticlly , and thanks @amogh-jahagirdar + @gaborkaszab for reviewing
--
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