Skip to content

Commit

Permalink
more concise docs
Browse files Browse the repository at this point in the history
  • Loading branch information
ccciudatu committed Dec 15, 2024
1 parent e4fe4ca commit 2738d10
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 20 deletions.
19 changes: 6 additions & 13 deletions datafusion/expr/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,24 +151,17 @@ pub trait SerializerRegistry: Debug + Send + Sync {
)
}

/// Binary representation for custom tables, to be converted to substrait extension tables.
/// Should only return success for table implementations that cannot be found by name
/// in the destination execution context, such as UDTFs or manually registered table providers.
/// Serialized table definition for UDTFs or manually registered table providers that can't be
/// marshaled by reference. Should return some benign error for regular tables that can be
/// found/restored by name in the destination execution context.
fn serialize_custom_table(&self, _table: &dyn TableSource) -> Result<Vec<u8>> {
not_impl_err!("No custom table support")
}

/// Deserialize the custom table with the given name.
/// The name may not be useful as a discriminator if multiple UDTF/TableProvider
/// implementations are expected. This is particularly true for UDTFs in DataFusion,
/// which are always registered under the same name: `tmp_table`, so one should
/// use the binary payload to distinguish between multiple potential table types.
/// A potential future improvement would be to return a (name, bytes) tuple from
/// [SerializerRegistry::serialize_custom_table] to allow the implementors to assign
/// different names to different table provider implementations (e.g. in the case of proto,
/// by using the actual protobuf `type_url`).
/// But this would mean the table names in the restored plan may no longer match
/// the original ones.
/// Note: more often than not, the name can't be used as a discriminator if multiple different
/// `TableSource` and/or `TableProvider` implementations are expected (this is particularly true
/// for UDTFs in DataFusion, which are always registered under the same name: `tmp_table`).
fn deserialize_custom_table(
&self,
name: &str,
Expand Down
12 changes: 5 additions & 7 deletions datafusion/substrait/src/logical_plan/consumer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1001,13 +1001,11 @@ pub async fn from_substrait_rel(
&ext_detail.type_url,
&ext_detail.value,
)?;
let table_name = if let Some((_, name)) =
ext_detail.type_url.rsplit_once('/')
{
name
} else {
&ext_detail.type_url
};
let table_name = ext_detail
.type_url
.rsplit_once('/')
.map(|(_, name)| name)
.unwrap_or(&ext_detail.type_url);
let plan = LogicalPlan::TableScan(TableScan::try_new(
table_name,
source,
Expand Down

0 comments on commit 2738d10

Please sign in to comment.