You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/10/06 14:18:03 UTC
[GitHub] [iceberg] Fokko opened a new pull request, #5929: Python: Add partition_type to partition_spec
Fokko opened a new pull request, #5929:
URL: https://github.com/apache/iceberg/pull/5929
We need to construct a StructType from the PartitionSpec for the manifest evaluators
--
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
[GitHub] [iceberg] rdblue commented on a diff in pull request #5929: Python: Add partition_type to partition_spec
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5929:
URL: https://github.com/apache/iceberg/pull/5929#discussion_r989537398
##########
python/tests/table/test_partitioning.py:
##########
@@ -102,3 +109,16 @@ def test_deserialize_partition_spec():
PartitionField(source_id=2, field_id=1001, transform=BucketTransform(num_buckets=25), name="int_bucket"),
),
)
+
+
+def test_partition_type(table_schema_simple: Schema) -> None:
+ spec = PartitionSpec(
+ PartitionField(source_id=1, field_id=1000, transform=TruncateTransform(width=19), name="str_truncate"),
+ PartitionField(source_id=2, field_id=1001, transform=BucketTransform(num_buckets=25), name="int_bucket"),
+ spec_id=3,
+ )
+
+ assert spec.partition_type(table_schema_simple) == StructType(
+ NestedField(field_id=1000, name="str_truncate", field_type=StringType(), required=True),
+ NestedField(field_id=1001, name="int_bucket", field_type=IntegerType(), required=True),
Review Comment:
The partition fields should be optional.
* All partition transforms are required to produce `null` if the input value is `null`, so it can happen when the source column is optional
* Partition fields may be added later, in which case not all files would have the result field and it may be `null`
There is a case where we can guarantee that a partition field in the first and only partition spec that uses a required source column will never be null, but it doesn't seem worth tracking this case.
--
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
[GitHub] [iceberg] Fokko commented on a diff in pull request #5929: Python: Add partition_type to partition_spec
Posted by GitBox <gi...@apache.org>.
Fokko commented on code in PR #5929:
URL: https://github.com/apache/iceberg/pull/5929#discussion_r989722174
##########
python/tests/table/test_partitioning.py:
##########
@@ -102,3 +109,16 @@ def test_deserialize_partition_spec():
PartitionField(source_id=2, field_id=1001, transform=BucketTransform(num_buckets=25), name="int_bucket"),
),
)
+
+
+def test_partition_type(table_schema_simple: Schema) -> None:
+ spec = PartitionSpec(
+ PartitionField(source_id=1, field_id=1000, transform=TruncateTransform(width=19), name="str_truncate"),
+ PartitionField(source_id=2, field_id=1001, transform=BucketTransform(num_buckets=25), name="int_bucket"),
+ spec_id=3,
+ )
+
+ assert spec.partition_type(table_schema_simple) == StructType(
+ NestedField(field_id=1000, name="str_truncate", field_type=StringType(), required=True),
+ NestedField(field_id=1001, name="int_bucket", field_type=IntegerType(), required=True),
Review Comment:
Good catch and makes total sense, also added this to the doc 👍🏻
--
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
[GitHub] [iceberg] rdblue commented on a diff in pull request #5929: Python: Add partition_type to partition_spec
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5929:
URL: https://github.com/apache/iceberg/pull/5929#discussion_r990491254
##########
python/pyiceberg/table/partitioning.py:
##########
@@ -77,9 +78,8 @@ class PartitionSpec(IcebergBaseModel):
PartitionSpec captures the transformation from table data to partition values
Attributes:
- schema(Schema): the schema of data table
spec_id(int): any change to PartitionSpec will produce a new specId
- fields(List[PartitionField): list of partition fields to produce partition values
+ fields(Tuple[PartitionField): list of partition fields to produce partition values
Review Comment:
Missing a closing `]`?
--
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
[GitHub] [iceberg] rdblue merged pull request #5929: Python: Add partition_type to partition_spec
Posted by GitBox <gi...@apache.org>.
rdblue merged PR #5929:
URL: https://github.com/apache/iceberg/pull/5929
--
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
[GitHub] [iceberg] rdblue commented on a diff in pull request #5929: Python: Add partition_type to partition_spec
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5929:
URL: https://github.com/apache/iceberg/pull/5929#discussion_r990491655
##########
python/pyiceberg/table/partitioning.py:
##########
@@ -158,6 +158,29 @@ def compatible_with(self, other: "PartitionSpec") -> bool:
for this_field, that_field in zip(self.fields, other.fields)
)
+ def partition_type(self, schema: Schema) -> StructType:
+ """Produces a struct of the PartitionSpec
+
+ The partition fields should be optional:
+
+ - All partition transforms are required to produce null if the input value is null, so it can
+ happen when the source column is optional
+ - Partition fields may be added later, in which case not all files would have the result field,
+ and it may be null.
+
+ There is a case where we can guarantee that a partition field in the first and only partition spec
Review Comment:
Minor: `we` used in documentation.
--
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