HonahX commented on code in PR #288:
URL: https://github.com/apache/iceberg-python/pull/288#discussion_r1462684106
##########
tests/catalog/integration_test_glue.py:
##########
@@ -279,6 +379,20 @@ def test_commit_table_update_schema(
assert test_catalog._parse_metadata_version(table.metadata_location) == 0
assert original_table_metadata.current_schema_id == 0
+ assert athena.get_query_results(f'SELECT * FROM
"{database_name}"."{table_name}"') == [
Review Comment:
Similar to the above comment. Shall we put these in a separate test? That
test can solely focus on the ability to query via Athena after schema evolution
and appending data
##########
pyiceberg/catalog/glue.py:
##########
@@ -84,17 +110,97 @@ def _construct_parameters(
return new_parameters
+GLUE_PRIMITIVE_TYPES = {
+ BooleanType: "boolean",
+ IntegerType: "int",
+ LongType: "bigint",
+ FloatType: "float",
+ DoubleType: "double",
+ DateType: "date",
+ TimeType: "string",
+ StringType: "string",
+ UUIDType: "string",
+ TimestampType: "timestamp",
+ FixedType: "binary",
+ BinaryType: "binary",
+}
+
+
+class SchemaToGlueType(SchemaVisitor[str]):
Review Comment:
```suggestion
class IcebergToGlueType(SchemaVisitor[str]):
```
How about `IcebergToGlueType`? This seems to more clearly indicate the
direction of the conversion.
##########
tests/catalog/integration_test_glue.py:
##########
@@ -64,6 +122,48 @@ def test_create_table(
s3.head_object(Bucket=get_bucket_name(), Key=metadata_location)
assert test_catalog._parse_metadata_version(table.metadata_location) == 0
+ table.append(
Review Comment:
[Minor] Should we consider creating separate tests specifically for table
write operations and Athena integration?
It seems more appropriate for `test_create_table` to concentrate solely on
verifying the correctness of table creation in Glue, such as metadata location,
versioning, and the details within the `storageDescriptor`. I think the
separation can make it easier to isolate bugs in the future development.
--
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
