sdf-jkl commented on code in PR #26893:
URL: https://github.com/apache/datafusion/pull/26893#discussion_r4446554488
##########
datafusion/common/src/types/extension.rs:
##########
@@ -63,6 +64,16 @@ pub trait DFExtensionType: Debug + Send + Sync {
/// Returns the serialized metadata.
fn serialize_metadata(&self) -> Option<String>;
+ /// Returns the name of this type shown by schema introspection, such as
`DESCRIBE`,
+ /// `SHOW COLUMNS` and `information_schema.columns`.
+ ///
+ /// The name may depend on the metadata of the type, for example
`VARCHAR(10)`.
+ ///
+ /// If `None` is returned, the storage type is shown.
+ fn display_name(&self) -> Option<String> {
+ None
+ }
Review Comment:
:robot: :speech_balloon: : Could this return `Option<Cow<'_, str>>`? UUID
and JSON already have static names, while a parameterized extension could
borrow a stored name or return an owned computed name. This avoids requiring a
temporary allocation for every name. There is an existing borrowed/owned
display pattern in
[`format_human_display`](https://github.com/apache/datafusion/blob/6b53f287a75832492bd5a442313e15f8525db2ad/datafusion/physical-plan/src/aggregates/mod.rs#L2111).
```suggestion
/// Returns the type's presentation label for schema introspection, such
as
/// `DESCRIBE`, `SHOW COLUMNS` and `information_schema.columns`.
///
/// Names may borrow from this instance or be computed from its metadata.
/// The label is not a unique type identifier or SQL syntax guaranteed
to round-trip.
/// If `None` is returned, the storage type is shown.
fn display_name(&self) -> Option<std::borrow::Cow<'_, str>> {
None
}
```
##########
datafusion/common/src/types/canonical_extensions/json.rs:
##########
@@ -42,4 +42,8 @@ impl DFExtensionType for DFJson {
fn serialize_metadata(&self) -> Option<String> {
self.inner.serialize_metadata()
}
+
+ fn display_name(&self) -> Option<String> {
+ Some("JSON".to_owned())
+ }
Review Comment:
:robot: :speech_balloon: : With the `Cow` return type, this fixed name can
be borrowed directly. The output builder still copies it into its own buffer;
the intermediate String is avoided.
```suggestion
fn display_name(&self) -> Option<std::borrow::Cow<'_, str>> {
Some(std::borrow::Cow::Borrowed("JSON"))
}
```
##########
datafusion/common/src/types/canonical_extensions/uuid.rs:
##########
@@ -43,6 +43,10 @@ impl DFExtensionType for DFUuid {
self.0.serialize_metadata()
}
+ fn display_name(&self) -> Option<String> {
+ Some("UUID".to_owned())
+ }
Review Comment:
same here
```suggestion
fn display_name(&self) -> Option<std::borrow::Cow<'_, str>> {
Some(std::borrow::Cow::Borrowed("UUID"))
}
```
##########
datafusion/expr/src/extension_types/display_name.rs:
##########
@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: Apache-2.0
+// SPDX-FileCopyrightText: See the NOTICE file distributed with this work for
additional information regarding copyright ownership
+// SPDX-FileContributor: Licensed to the Apache Software Foundation (ASF)
under one or more contributor license agreements
+
+use crate::registry::ExtensionTypeRegistry;
+use arrow_schema::Field;
+
+/// Returns the name of the type of `field` shown by schema introspection,
such as `DESCRIBE`,
+/// `SHOW COLUMNS` and `information_schema.columns`.
+///
+/// If the field has a registered extension type that provides a
+/// [`display_name`](datafusion_common::types::DFExtensionType::display_name),
that name is
+/// returned. Otherwise, the storage type is returned. The storage type is
also returned if the
+/// extension type is not registered or cannot be created from the metadata of
the field, so
+/// that the schema of such a table can still be inspected.
+pub fn field_type_display_name(
+ registry: &dyn ExtensionTypeRegistry,
+ field: &Field,
+) -> String {
+ registry
+ .create_extension_type_for_field(field)
+ .ok()
+ .flatten()
+ .and_then(|extension_type| extension_type.display_name())
+ .unwrap_or_else(|| field.data_type().to_string())
+}
Review Comment:
can provide and reuse `StringBuilder` to avoid string allocations
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
