remove visit_snapshot_schema, add logical_schema fn, do cleanup

ffi/examples/read-table/read_table.c

@@ -321,7 +321,7 @@ int main(int argc, char* argv[])
 
   free_kernel_scan_data(data_iter);
   free_scan(scan);
-  free_global_read_schema(read_schema);
+  free_schema(read_schema);
   free_global_scan_state(global_state);
   free_snapshot(snapshot);
   free_engine(engine);

ffi/examples/read-table/schema.h

@@ -273,7 +273,8 @@ void print_schema(SharedSnapshot* snapshot)
     .visit_timestamp = visit_timestamp,
     .visit_timestamp_ntz = visit_timestamp_ntz,
   };
-  uintptr_t schema_list_id = visit_snapshot_schema(snapshot, &visitor);
+  SharedSchema* schema = logical_schema(snapshot);
+  uintptr_t schema_list_id = visit_schema(schema, &visitor);
 #ifdef VERBOSE
   printf("Schema returned in list %" PRIxPTR "\n", schema_list_id);
 #endif

ffi/src/engine_funcs.rs

@@ -8,8 +8,8 @@ use tracing::debug;
 use url::Url;
 
 use crate::{
-    scan::SharedSchema, ExclusiveEngineData, ExternEngine, ExternResult, IntoExternResult,
-    KernelStringSlice, NullableCvoid, SharedExternEngine, TryFromStringSlice,
+    ExclusiveEngineData, ExternEngine, ExternResult, IntoExternResult, KernelStringSlice,
+    NullableCvoid, SharedExternEngine, SharedSchema, TryFromStringSlice,
 };
 
 use super::handle::Handle;

ffi/src/lib.rs

@@ -11,6 +11,7 @@ use std::sync::Arc;
 use tracing::debug;
 use url::Url;
 
+use delta_kernel::schema::Schema;
 use delta_kernel::snapshot::Snapshot;
 use delta_kernel::{DeltaResult, Engine, EngineData, Table};
 use delta_kernel_ffi_macros::handle_descriptor;
@@ -561,6 +562,9 @@ pub unsafe extern "C" fn free_engine(engine: Handle<SharedExternEngine>) {
     engine.drop_handle();
 }
 
+#[handle_descriptor(target=Schema, mutable=false, sized=true)]
+pub struct SharedSchema;
+
 #[handle_descriptor(target=Snapshot, mutable=false, sized=true)]
 pub struct SharedSnapshot;
 
@@ -607,6 +611,26 @@ pub unsafe extern "C" fn version(snapshot: Handle<SharedSnapshot>) -> u64 {
     snapshot.version()
 }
 
+/// Get the logical schema of the specified snapshot
+///
+/// # Safety
+///
+/// Caller is responsible for passing a valid snapshot handle.
+#[no_mangle]
+pub unsafe extern "C" fn logical_schema(snapshot: Handle<SharedSnapshot>) -> Handle<SharedSchema> {
+    let snapshot = unsafe { snapshot.as_ref() };
+    Arc::new(snapshot.schema().clone()).into()
+}
+
+/// Free a schema
+///
+/// # Safety
+/// Engine is responsible for providing a valid schema obtained via [`get_global_read_schema`]
+#[no_mangle]
+pub unsafe extern "C" fn free_schema(schema: Handle<SharedSchema>) {
+    schema.drop_handle();
+}
+
 /// Get the resolved root of the table. This should be used in any future calls that require
 /// constructing a path
 ///

ffi/src/scan.rs

@@ -5,7 +5,6 @@ use std::sync::{Arc, Mutex};
 
 use delta_kernel::scan::state::{visit_scan_files, DvInfo, GlobalScanState};
 use delta_kernel::scan::{Scan, ScanData};
-use delta_kernel::schema::Schema;
 use delta_kernel::snapshot::Snapshot;
 use delta_kernel::{DeltaResult, Error, ExpressionRef};
 use delta_kernel_ffi_macros::handle_descriptor;
@@ -18,7 +17,8 @@ use crate::expressions::engine::{
 use crate::{
     kernel_string_slice, AllocateStringFn, ExclusiveEngineData, ExternEngine, ExternResult,
     IntoExternResult, KernelBoolSlice, KernelRowIndexArray, KernelStringSlice, NullableCvoid,
-    SharedExternEngine, SharedSnapshot, StringIter, StringSliceIterator, TryFromStringSlice,
+    SharedExternEngine, SharedSchema, SharedSnapshot, StringIter, StringSliceIterator,
+    TryFromStringSlice,
 };
 
 use super::handle::Handle;
@@ -70,8 +70,6 @@ fn scan_impl(
 
 #[handle_descriptor(target=GlobalScanState, mutable=false, sized=true)]
 pub struct SharedGlobalScanState;
-#[handle_descriptor(target=Schema, mutable=false, sized=true)]
-pub struct SharedSchema;
 
 /// Get the global state for a scan. See the docs for [`delta_kernel::scan::state::GlobalScanState`]
 /// for more information.
@@ -99,15 +97,6 @@ pub unsafe extern "C" fn get_global_read_schema(
     state.physical_schema.clone().into()
 }
 
-/// Free a global read schema
-///
-/// # Safety
-/// Engine is responsible for providing a valid schema obtained via [`get_global_read_schema`]
-#[no_mangle]
-pub unsafe extern "C" fn free_global_read_schema(schema: Handle<SharedSchema>) {
-    schema.drop_handle();
-}
-
 /// Get a count of the number of partition columns for this scan
 ///
 /// # Safety

ffi/src/schema.rs

@@ -1,7 +1,8 @@
 use std::os::raw::c_void;
 
-use crate::scan::{CStringMap, SharedSchema};
-use crate::{handle::Handle, kernel_string_slice, KernelStringSlice, SharedSnapshot};
+use crate::scan::CStringMap;
+use crate::SharedSchema;
+use crate::{handle::Handle, kernel_string_slice, KernelStringSlice};
 use delta_kernel::schema::{ArrayType, DataType, MapType, PrimitiveType, StructType};
 
 /// The `EngineSchemaVisitor` defines a visitor system to allow engines to build their own
@@ -192,23 +193,6 @@ pub struct EngineSchemaVisitor {
     ),
 }
 
-/// Visit the schema of the passed `SnapshotHandle`, using the provided `visitor`. See the
-/// documentation of [`EngineSchemaVisitor`] for a description of how this visitor works.
-///
-/// This method returns the id of the list allocated to hold the top level schema columns.
-///
-/// # Safety
-///
-/// Caller is responsible for passing a valid snapshot handle and schema visitor.
-#[no_mangle]
-pub unsafe extern "C" fn visit_snapshot_schema(
-    snapshot: Handle<SharedSnapshot>,
-    visitor: &mut EngineSchemaVisitor,
-) -> usize {
-    let snapshot = unsafe { snapshot.as_ref() };
-    visit_schema_impl(snapshot.schema(), visitor)
-}
-
 /// Visit the given `schema` using the provided `visitor`. See the documentation of
 /// [`EngineSchemaVisitor`] for a description of how this visitor works.
 ///

kernel/src/snapshot.rs

@@ -98,6 +98,7 @@ impl Snapshot {
     }
 
     /// Table [`Schema`] at this `Snapshot`s version.
+    // TODO should this return SchemaRef?
     pub fn schema(&self) -> &Schema {
         self.table_configuration.schema()
     }