You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "jeroko (via GitHub)" <gi...@apache.org> on 2025/10/27 09:51:05 UTC
[PR] Remove field id constraint on add files [iceberg-python]
jeroko opened a new pull request, #2662:
URL: https://github.com/apache/iceberg-python/pull/2662
# Rationale for this change
Closes #2131
The PR relaxes the constraint that prevented adding any file with field IDs, and replaces it with a constraint that prevents adding files which contain field IDs that are inconsistent with the field IDs of the table. If the field IDs are compatible, then they can be added safely, if not, they will be rejected.
## Are these changes tested?
Yes
## Are there any user-facing changes?
Yes
--
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] Remove field id constraint on add files [iceberg-python]
Posted by "jeroko (via GitHub)" <gi...@apache.org>.
jeroko commented on code in PR #2662:
URL: https://github.com/apache/iceberg-python/pull/2662#discussion_r2467182058
##########
pyiceberg/io/pyarrow.py:
##########
@@ -2624,8 +2627,41 @@ def _check_pyarrow_schema_compatible(
)
additional_names = set(provided_schema._name_to_id.keys()) - set(requested_schema._name_to_id.keys())
raise ValueError(
- f"PyArrow table contains more columns: {', '.join(sorted(additional_names))}. Update the schema first (hint, use union_by_name)."
+ f"PyArrow table contains more columns: {', '.join(sorted(additional_names))}. "
+ "Update the schema first (hint, use union_by_name)."
) from e
+
+ # If the file has explicit field IDs, validate they match the table schema exactly
+ if has_field_ids:
+ # Build mappings for both schemas (including nested fields)
+ requested_id_to_name = requested_schema._lazy_id_to_name
+ provided_id_to_name = provided_schema._lazy_id_to_name
Review Comment:
@Fokko Right, we should not care about the names if the IDs are provided, and the mapping between the IDs and the types was already checked in the call to `_check_schema_compatible` at the end of this function. In that case I didn't really need to add any extra check, just a new test to verify that files with matching field IDs and incompatible types fail.
--
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] Remove field id constraint on add files [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #2662:
URL: https://github.com/apache/iceberg-python/pull/2662#discussion_r2491745861
##########
pyiceberg/io/pyarrow.py:
##########
Review Comment:
i think we should at least check that the parquet field IDs align with the Iceberg field IDs
##########
mkdocs/docs/api.md:
##########
@@ -1006,9 +1006,11 @@ Expert Iceberg users may choose to commit existing parquet files to the Iceberg
<!-- prettier-ignore-start -->
-!!! note "Name Mapping"
- Because `add_files` uses existing files without writing new parquet files that are aware of the Iceberg's schema, it requires the Iceberg's table to have a [Name Mapping](https://iceberg.apache.org/spec/?h=name+mapping#name-mapping-serialization) (The Name mapping maps the field names within the parquet files to the Iceberg field IDs). Hence, `add_files` requires that there are no field IDs in the parquet file's metadata, and creates a new Name Mapping based on the table's current schema if the table doesn't already have one.
-
+!!! note "Name Mapping and Field IDs"
+ `add_files` can work with Parquet files both with and without field IDs in their metadata:
+ - **Files with field IDs**: When field IDs are present in the Parquet metadata, they must match the corresponding field IDs in the Iceberg table schema. This is common for files generated by tools like Spark or when using or other libraries with explicit field ID metadata.
+ - **Files without field IDs**: When field IDs are absent, the table must have a [Name Mapping](https://iceberg.apache.org/spec/?h=name+mapping#name-mapping-serialization) to map field names to Iceberg field IDs. `add_files` will automatically create a Name Mapping based on the table's current schema if one doesn't already exist.
+ In both cases, a Name Mapping is created if the table doesn't have one, ensuring compatibility with various readers.
Review Comment:
For parquet files with field ID, i dont think we necessary need the name mapping if its aligned with the table schema field IDs
But we can address this separately
--
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] Remove field id constraint on add files [iceberg-python]
Posted by "jeroko (via GitHub)" <gi...@apache.org>.
jeroko commented on PR #2662:
URL: https://github.com/apache/iceberg-python/pull/2662#issuecomment-3552823040
@Fokko @kevinjqliu Are there any other changes or improvements needed in order to merge this PR?
--
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] Remove field id constraint on add files [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #2662:
URL: https://github.com/apache/iceberg-python/pull/2662#discussion_r2466168619
##########
pyiceberg/io/pyarrow.py:
##########
@@ -2624,8 +2627,41 @@ def _check_pyarrow_schema_compatible(
)
additional_names = set(provided_schema._name_to_id.keys()) - set(requested_schema._name_to_id.keys())
raise ValueError(
- f"PyArrow table contains more columns: {', '.join(sorted(additional_names))}. Update the schema first (hint, use union_by_name)."
+ f"PyArrow table contains more columns: {', '.join(sorted(additional_names))}. "
+ "Update the schema first (hint, use union_by_name)."
) from e
+
+ # If the file has explicit field IDs, validate they match the table schema exactly
+ if has_field_ids:
+ # Build mappings for both schemas (including nested fields)
+ requested_id_to_name = requested_schema._lazy_id_to_name
+ provided_id_to_name = provided_schema._lazy_id_to_name
Review Comment:
Hey @jeroko Thanks for working on this, and adding this check.
However, I don't think we really care about the names; it is not a problem when they differ. However, if you add a file with a different schema, we can brick the table because of issues in the types. Should we check if the file contains the expected type for each of the IDs instead?
--
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] Remove field id constraint on add files [iceberg-python]
Posted by "jeroko (via GitHub)" <gi...@apache.org>.
jeroko commented on code in PR #2662:
URL: https://github.com/apache/iceberg-python/pull/2662#discussion_r2531834541
##########
mkdocs/docs/api.md:
##########
@@ -1006,9 +1006,11 @@ Expert Iceberg users may choose to commit existing parquet files to the Iceberg
<!-- prettier-ignore-start -->
-!!! note "Name Mapping"
- Because `add_files` uses existing files without writing new parquet files that are aware of the Iceberg's schema, it requires the Iceberg's table to have a [Name Mapping](https://iceberg.apache.org/spec/?h=name+mapping#name-mapping-serialization) (The Name mapping maps the field names within the parquet files to the Iceberg field IDs). Hence, `add_files` requires that there are no field IDs in the parquet file's metadata, and creates a new Name Mapping based on the table's current schema if the table doesn't already have one.
-
+!!! note "Name Mapping and Field IDs"
+ `add_files` can work with Parquet files both with and without field IDs in their metadata:
+ - **Files with field IDs**: When field IDs are present in the Parquet metadata, they must match the corresponding field IDs in the Iceberg table schema. This is common for files generated by tools like Spark or when using or other libraries with explicit field ID metadata.
+ - **Files without field IDs**: When field IDs are absent, the table must have a [Name Mapping](https://iceberg.apache.org/spec/?h=name+mapping#name-mapping-serialization) to map field names to Iceberg field IDs. `add_files` will automatically create a Name Mapping based on the table's current schema if one doesn't already exist.
+ In both cases, a Name Mapping is created if the table doesn't have one, ensuring compatibility with various readers.
Review Comment:
I remove that last comment
--
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] Remove field id constraint on add files [iceberg-python]
Posted by "jeroko (via GitHub)" <gi...@apache.org>.
jeroko commented on code in PR #2662:
URL: https://github.com/apache/iceberg-python/pull/2662#discussion_r2467182058
##########
pyiceberg/io/pyarrow.py:
##########
@@ -2624,8 +2627,41 @@ def _check_pyarrow_schema_compatible(
)
additional_names = set(provided_schema._name_to_id.keys()) - set(requested_schema._name_to_id.keys())
raise ValueError(
- f"PyArrow table contains more columns: {', '.join(sorted(additional_names))}. Update the schema first (hint, use union_by_name)."
+ f"PyArrow table contains more columns: {', '.join(sorted(additional_names))}. "
+ "Update the schema first (hint, use union_by_name)."
) from e
+
+ # If the file has explicit field IDs, validate they match the table schema exactly
+ if has_field_ids:
+ # Build mappings for both schemas (including nested fields)
+ requested_id_to_name = requested_schema._lazy_id_to_name
+ provided_id_to_name = provided_schema._lazy_id_to_name
Review Comment:
@Fokko Right, we should not care about the names if the IDs are provided, and the mapping between the IDs and the types was already checked in the call to `_check_schema_compatible` at the end of this function. In that case I didn't really need to add any extra check.
--
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] Remove field id constraint on add files [iceberg-python]
Posted by "jeroko (via GitHub)" <gi...@apache.org>.
jeroko commented on code in PR #2662:
URL: https://github.com/apache/iceberg-python/pull/2662#discussion_r2498338500
##########
pyiceberg/io/pyarrow.py:
##########
Review Comment:
Hi @kevinjqliu, what kind of extra ID alignment do you expect that is not already covered by `_check_schema_compatible` ?
--
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] Remove field id constraint on add files [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko merged PR #2662:
URL: https://github.com/apache/iceberg-python/pull/2662
--
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