Have an ffi c example that actually reads data #203

nicklan · 2024-05-13T21:44:07Z

Flesh out what a more full FFI API for data reading would look like. We can certainly iterate on this.

Lots more into the example for using the c-ffi (sorry for another big PR. It's mostly example code at least):

Actually read the data
Include partition columns
Print out the table
Use clang-format to get consistent c style
Add all the gcc warnings, make them errors, and fix them

Unfortunately the glib arrow api doesn't seem to allow passing options to how we print things, so this will print a max of 4 rows out the table. To print more we'd need to write our own printing function.

This also adds engine_funcs.rs which currently only exposes read_parquet_files. This is more of a POC, but seems like potentially the right way forward for exposing the various functions available to rust when it has an Engine.

A couple of example runs:

Reading the basic_partitioned table:

Reading table at file:///home/nick/databricks/delta-kernel-rs/acceptance/tests/dat/out/reader_tests/generated/basic_partitioned/delta/
version: 1

Building schema
Making a list of lenth 3 with id 0
Asked to visit string named letter belonging to list 0.
Asked to visit long named number belonging to list 0.
Asked to visit double named a_float belonging to list 0.
Schema returned in list 0
Done building schema
Schema:
├─ letter: string
├─ number: long
└─ a_float: double

Starting table scan

Building list of partition columns
Done iterating partition columns
Partition columns are:
  - letter

Iterating scan data

Scan iterator found some data to read
  Of this data, here is a selection vector
    sel[0] = 0
    sel[1] = 1
    sel[2] = 1
    sel[3] = 1
Asking kernel to call us back for each scan row (file to read)
Called back to read file: letter=a/part-00000-b17b8597-4c08-4867-b882-2249d604cbc2.c000.snappy.parquet
  No selection vector for this file
  partition 'letter' here: a
  Reading parquet file at file:///home/nick/databricks/delta-kernel-rs/acceptance/tests/dat/out/reader_tests/generated/basic_partitioned/delta/letter=a/part-00000-b17b8597-4c08-4867-b882-2249d604cbc2.c000.snappy.parquet
  Converting read data to arrow
  Adding partition column 'letter' with value 'a' at column 2
  Added batch to arrow context, have 1 batches in context now
  Done reading parquet file
Called back to read file: letter=e/part-00000-90f8c03f-e005-4714-a8ac-55bcf6d94d7d.c000.snappy.parquet
  No selection vector for this file
  partition 'letter' here: e
  Reading parquet file at file:///home/nick/databricks/delta-kernel-rs/acceptance/tests/dat/out/reader_tests/generated/basic_partitioned/delta/letter=e/part-00000-90f8c03f-e005-4714-a8ac-55bcf6d94d7d.c000.snappy.parquet
  Converting read data to arrow
  Adding partition column 'letter' with value 'e' at column 2
  Added batch to arrow context, have 2 batches in context now
  Done reading parquet file
Called back to read file: letter=f/part-00000-4da5e756-3c84-4b40-b483-ebcf3c4ed6ce.c000.snappy.parquet
  No selection vector for this file
  partition 'letter' here: f
  Reading parquet file at file:///home/nick/databricks/delta-kernel-rs/acceptance/tests/dat/out/reader_tests/generated/basic_partitioned/delta/letter=f/part-00000-4da5e756-3c84-4b40-b483-ebcf3c4ed6ce.c000.snappy.parquet
  Converting read data to arrow
  Adding partition column 'letter' with value 'f' at column 2
  Added batch to arrow context, have 3 batches in context now
  Done reading parquet file

Scan iterator found some data to read
  Of this data, here is a selection vector
    sel[0] = 0
    sel[1] = 0
    sel[2] = 0
    sel[3] = 1
    sel[4] = 1
    sel[5] = 1
Asking kernel to call us back for each scan row (file to read)
Called back to read file: letter=a/part-00000-8fe836ee-ec38-4388-91a1-5c3eff176e84.c000.snappy.parquet
  No selection vector for this file
  partition 'letter' here: a
  Reading parquet file at file:///home/nick/databricks/delta-kernel-rs/acceptance/tests/dat/out/reader_tests/generated/basic_partitioned/delta/letter=a/part-00000-8fe836ee-ec38-4388-91a1-5c3eff176e84.c000.snappy.parquet
  Converting read data to arrow
  Adding partition column 'letter' with value 'a' at column 2
  Added batch to arrow context, have 4 batches in context now
  Done reading parquet file
Called back to read file: letter=b/part-00000-2465ef7c-268e-43be-98fe-c9c37a2a6482.c000.snappy.parquet
  No selection vector for this file
  partition 'letter' here: b
  Reading parquet file at file:///home/nick/databricks/delta-kernel-rs/acceptance/tests/dat/out/reader_tests/generated/basic_partitioned/delta/letter=b/part-00000-2465ef7c-268e-43be-98fe-c9c37a2a6482.c000.snappy.parquet
  Converting read data to arrow
  Adding partition column 'letter' with value 'b' at column 2
  Added batch to arrow context, have 5 batches in context now
  Done reading parquet file
Called back to read file: letter=c/part-00000-28c8b160-ee9c-407d-93f5-ff7d8e2a8d3f.c000.snappy.parquet
  No selection vector for this file
  partition 'letter' here: c
  Reading parquet file at file:///home/nick/databricks/delta-kernel-rs/acceptance/tests/dat/out/reader_tests/generated/basic_partitioned/delta/letter=c/part-00000-28c8b160-ee9c-407d-93f5-ff7d8e2a8d3f.c000.snappy.parquet
  Converting read data to arrow
  Adding partition column 'letter' with value 'c' at column 2
  Added batch to arrow context, have 6 batches in context now
  Done reading parquet file
Scan data iterator done
All done reading table data

Table Data:
-----------

number: int64
a_float: double
letter: string
----
number:
  [
    [
      4
    ],
    [
      5
    ],
  ...,
    [
      2
    ],
    [
      3
    ]
  ]
a_float:
  [
    [
      4.4
    ],
    [
      5.5
    ],
  ...,
    [
      2.2
    ],
    [
      3.3
    ]
  ]
letter:
  [
    [
      "a"
    ],
    [
      "e"
    ],
  ...,
    [
      "b"
    ],
    [
      "c"
    ]
  ]

Reading the nested_types table

Reading table at file:///home/nick/databricks/delta-kernel-rs/acceptance/tests/dat/out/reader_tests/generated/nested_types/delta/
version: 0

Building schema
Making a list of lenth 4 with id 0
Asked to visit integer named pk belonging to list 0.
Making a list of lenth 2 with id 1
Asked to visit double named float64 belonging to list 1.
Asked to visit boolean named bool belonging to list 1.
Asked to visit struct named struct belonging to list 0. Children are in 1.
Making a list of lenth 1 with id 2
Asked to visit short named array_element belonging to list 2.
Asked to visit array named array (contains null: true) belonging to list 0. Types are in 2.
Making a list of lenth 2 with id 3
Asked to visit string named map_key belonging to list 3.
Asked to visit integer named map_value belonging to list 3.
Asked to visit map named map (contains null: true) belonging to list 0. Types are in 3.
Schema returned in list 0
Done building schema
Schema:
├─ pk: integer
├─ struct: struct
│  ├─ float64: double
│  └─ bool: boolean
├─ array (contains null: true): array
│  └─ array_element: short
└─ map (contains null: true): map
   ├─ map_key: string
   └─ map_value: integer

Starting table scan

Building list of partition columns
Done iterating partition columns
Table has no partition columns

Iterating scan data

Scan iterator found some data to read
  Of this data, here is a selection vector
    sel[0] = 0
    sel[1] = 0
    sel[2] = 0
    sel[3] = 1
Asking kernel to call us back for each scan row (file to read)
Called back to read file: part-00000-3b8ca3f0-6444-4ed5-8961-c605cba95bf1-c000.snappy.parquet
  No selection vector for this file
  Reading parquet file at file:///home/nick/databricks/delta-kernel-rs/acceptance/tests/dat/out/reader_tests/generated/nested_types/delta/part-00000-3b8ca3f0-6444-4ed5-8961-c605cba95bf1-c000.snappy.parquet
  Converting read data to arrow
  Added batch to arrow context, have 1 batches in context now
  Done reading parquet file
Scan data iterator done
All done reading table data

Table Data:
-----------

pk: int32
struct: struct<float64: double, bool: bool>
  child 0, float64: double
  child 1, bool: bool
array: list<element: int16>
  child 0, element: int16
map: map<string, int32>
  child 0, entries: struct<key: string not null, value: int32> not null
      child 0, key: string not null
      child 1, value: int32
----
pk:
  [
    [
      0,
      1,
      2,
      3,
      4
    ]
  ]
struct:
  [
    -- is_valid: all not null
    -- child 0 type: double
      [
        0,
        1,
        2,
        3,
        4
      ]
    -- child 1 type: bool
      [
        true,
        false,
        true,
        false,
        true
      ]
  ]
array:
  [
    [
      [
        0
      ],
      [
        0,
        1
      ],
      [
        0,
        1,
        2
      ],
      [
        0,
        1,
        2,
        3
      ],
      [
        0,
        1,
        2,
        3,
        4
      ]
    ]
  ]
map:
  [
    [
      keys:
      []
      values:
      [],
      keys:
      [
        "0"
      ]
      values:
      [
        0
      ],
      keys:
      [
        "0",
        "1"
      ]
      values:
      [
        0,
        1
      ],
      keys:
      [
        "0",
        "1",
        "2"
      ]
      values:
      [
        0,
        1,
        2
      ],
      keys:
      [
        "0",
        "1",
        "2",
        "3"
      ]
      values:
      [
        0,
        1,
        2,
        3
      ]
    ]
  ]

with `lang-format --style="{BasedOnStyle: Mozilla, ColumnLimit: 100, BreakBeforeBraces: Attach, AlwaysBreakAfterReturnType: None, AlwaysBreakAfterDefinitionReturnType: None}`

nicklan · 2024-06-07T20:55:13Z

@scovich if you have time, I think this is ready for review (but no worries if not!)

nicklan · 2024-06-07T21:09:58Z

ffi/src/lib.rs

@@ -113,7 +114,7 @@ impl TryFromStringSlice for String {
    ///
    /// The slice must be a valid (non-null) pointer, and must point to the indicated number of
    /// valid utf8 bytes.
-    unsafe fn try_from_slice(slice: KernelStringSlice) -> DeltaResult<String> {
+    unsafe fn try_from_slice(slice: &KernelStringSlice) -> DeltaResult<String> {


@scovich this is obviously a significant change. I can't think of a way that it causes an issue though, since we basically only require the lifetime of the slice pointer to be "this function", which was the same as before. We call slice.into() which allocates, so as soon as this returns, the pointer can become invalid and that's fine.

This is more flexible because we can have a struct with a KernelStringSlice, and then can turn it into a String, without having to consume the whole struct.

Quick drive-by response (will try to look more later): How often do we need to consume a kernel string slice from a struct without consuming the struct? And in such cases, should we just create a new slice object since it's anyway just a borrowed reference to a(n externally owned) string?

However, that got me thinking. Currently, KernelStringSlice is neither Copy nor Clone, in an attempt to encourage at least semi-sane semantics. However, it can still be "moved" from, which includes returning it from a function or assigning it to a heap-allocated struct field, so I'm not sure how much protection we really get? It seems like any copy or clone can only outlive the original if it gets moved-from -- e.g. returned from a function -- but the same is already true for the original instance?? Maybe this is another case where the compiler can't save us and we have to rely on idiomatic code patterns instead?

However, IIRC we moved to passing by value instead of by reference, because it was causing really weird lifetime issues. So I'd be very cautious moving back to references without understanding that previous issue better. I don't remember details any more, sadly...

Update: Thinking more, the lifetime safety issues mentioned above seem orthogonal to whether try_from_slice takes a value or a reference? And if we look at it in isolation, taking a reference is actually the right approach, because creating a String really doesn't consume the backing string the slice references. Similar to how the language has impl From<&str> for String?

Yeah, I think that captures nicely what I was more vaguely thinking. Basically, because try_from_slice doesn't actually consume the slice, it makes sense to take it as a reference.

I've added to the comment of this function that it doesn't consume the slice. LMK if you have further concerns about this change though.