zeroshade commented on code in PR #969:
URL: https://github.com/apache/arrow-go/pull/969#discussion_r3641188546
##########
arrow/datatype_extension.go:
##########
@@ -82,6 +83,38 @@ func GetExtensionType(typName string) ExtensionType {
return nil
}
+// FindRegisteredExtensionType returns a registered extension type for
+// which the filter returns true. If no type matches, it returns nil.
+//
+// If multiple types match, the type with the lexicographically smallest
+// registered extension name is returned.
+func FindRegisteredExtensionType(filter func(ExtensionType) bool)
ExtensionType {
+ registry := getExtTypeRegistry()
+
+ // We first sort the types by name to ensure that we return the
+ // type with the lexicographically smallest name. We sort then filter
+ // to ensure that there are no side effects from filtering which would
+ // affect the determinism of the result.
Review Comment:
Minor wording: "no side effects from filtering" reads as if the filter is
required to be pure, but what this actually guarantees is that iteration runs
over a stable, name-sorted snapshot, so the result is deterministic regardless
of sync.Map's Range order. Maybe something like:
// Snapshot and sort the registered names before invoking the filter so
the
// result is deterministic (lexicographically smallest match),
independent of
// sync.Map's unspecified Range order.
##########
parquet/pqarrow/schema.go:
##########
@@ -129,6 +129,13 @@ type ExtensionCustomParquetType interface {
ParquetLogicalType() schema.LogicalType
}
+// ExtensionParquetLogicalType is an interface that Arrow ExtensionTypes may
+// implement to specify how a Parquet LogicalType maps back to an Arrow
+// ExtensionType when converting a Parquet schema to an Arrow schema.
+type ExtensionParquetLogicalType interface {
+ ArrowTypeFromParquet(logical schema.LogicalType, storageType
arrow.DataType) (arrow.ExtensionType, error)
Review Comment:
Optional, doc-only: worth spelling out the return-value contract here, since
third-party types implement this against the exported interface:
- `(nil, nil)` → "not my logical type"; resolution skips this extension and
continues.
- `(nil, err)` → recognized the logical family but couldn't construct a
valid type; the error is deferred and only surfaced if no other extension
matches.
- non-nil type → wins, taking precedence over a deferred error from an
earlier (name-sorted) extension.
The test doubles already follow this — it's just about making the contract
explicit on the exported API.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
