Skip to content

Commit c200f9d

Browse files
myrrcclaude
andcommitted
Replace vx_string/vx_binary with non-owning vx_str views
vx_str { ptr, len } is a by-value data view, not a handle: there is nothing to free or clone, closing the gap PR #8658 left for strings. Inputs (field names, paths, utf8 scalars) are copied on the Rust side; outputs (error messages, field names, time zones, string elements) borrow from a live owned handle with a documented lifetime. - vx_data_source_options takes a list of vx_str paths instead of a comma-joined string; commas in paths are now legal - vx_array_get_utf8/get_binary become vx_array_utf8_at/binary_at: canonical-only, O(1), borrowing from the array's buffers - vx_error_get_message becomes vx_str vx_error_message - vx_struct_fields_builder_add_field no longer consumes an allocated name and reports invalid UTF-8 via error_out - the vx_string/vx_binary families are deleted; the header gains a static inline vx_str_from_cstr helper - VX_ABI_VERSION bumped to 2 Signed-off-by: "Mikhail Kot" <mikhail@spiraldb.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 3071fc3 commit c200f9d

25 files changed

Lines changed: 561 additions & 695 deletions

‎vortex-ffi/cbindgen.toml‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,27 @@ typedef struct ArrowArrayStream FFI_ArrowArrayStream;
6565
#endif
6666
"""
6767

68+
trailer = """
69+
#include <string.h>
70+
71+
/**
72+
* Create a vx_str view over a null-terminated C string.
73+
* The view borrows the bytes; it is valid for as long as `str` is.
74+
*/
75+
#ifdef __cplusplus
76+
static inline vx_str vx_str_from_cstr(const char* str) {
77+
vx_str s;
78+
s.ptr = str;
79+
s.len = strlen(str);
80+
return s;
81+
}
82+
#else
83+
static inline vx_str vx_str_from_cstr(const char* str) {
84+
return (vx_str){ str, strlen(str) };
85+
}
86+
#endif
87+
"""
88+
6889
[export.rename]
6990
"f16" = "uint16_t"
7091

‎vortex-ffi/cinclude/vortex.h‎

Lines changed: 84 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ typedef struct ArrowArrayStream FFI_ArrowArrayStream;
6666
* The version of the Vortex C ABI declared by this header.
6767
* Incremented on every backward-incompatible change to the C API.
6868
*/
69-
#define VX_ABI_VERSION 1
69+
#define VX_ABI_VERSION 2
7070

7171
/**
7272
* Maximum size of an inlined binary value.
@@ -497,11 +497,6 @@ typedef struct vx_array_iterator vx_array_iterator;
497497
*/
498498
typedef struct vx_array_sink vx_array_sink;
499499

500-
/**
501-
* Strings for use within Vortex.
502-
*/
503-
typedef struct vx_binary vx_binary;
504-
505500
/**
506501
* A reference to one or more (possibly remote) paths.
507502
* Creating vx_data_source opens the first matched path to read the schema.
@@ -569,11 +564,6 @@ typedef struct vx_scan vx_scan;
569564
*/
570565
typedef struct vx_session vx_session;
571566

572-
/**
573-
* Strings for use within Vortex.
574-
*/
575-
typedef struct vx_string vx_string;
576-
577567
typedef struct vx_struct_column_builder vx_struct_column_builder;
578568

579569
/**
@@ -602,17 +592,38 @@ typedef struct {
602592
const vx_array *array;
603593
} vx_validity;
604594

595+
/**
596+
* A non-owning view over a byte range, used for strings crossing the FFI in both directions.
597+
* Not a handle: there is nothing to `_free`/`_clone`. Inputs are copied by the callee and need
598+
* only stay valid for the call; outputs borrow from the owned handle named in the function's
599+
* doc and live as long as it. Bytes are NOT null-terminated; NULL `ptr` requires `len == 0`.
600+
*/
601+
typedef struct {
602+
/**
603+
* Pointer to the first byte of the string. NULL is allowed only when `len` is 0.
604+
*/
605+
const char *ptr;
606+
/**
607+
* Length of the string in bytes.
608+
*/
609+
size_t len;
610+
} vx_str;
611+
605612
/**
606613
* Options for creating a data source.
607614
*/
608615
typedef struct {
609616
/**
610617
* Required: paths to files, tables, or layout trees.
611-
* May be a glob pattern like "*.vortex".
612-
* If you want to include multiple paths, concat them with a comma:
613-
* "file1.vortex,../file2.vortex".
618+
* Each entry may be a glob pattern like "*.vortex".
619+
* Must point to an array of `paths_len` entries, valid for the duration
620+
* of the call; the bytes are copied.
621+
*/
622+
const vx_str *paths;
623+
/**
624+
* Number of entries in `paths`.
614625
*/
615-
const char *paths;
626+
size_t paths_len;
616627
} vx_data_source_options;
617628

618629
/**
@@ -851,16 +862,18 @@ double vx_array_get_f64(const vx_array *array, size_t index);
851862
double vx_array_get_storage_f64(const vx_array *array, size_t index);
852863

853864
/**
854-
* Return the utf-8 string at `index` in the array. The pointer will be null if the value at `index` is null.
855-
* The caller must free the returned pointer.
865+
* Return the utf-8 string at `index` in a canonical Utf8 array. The view borrows from the
866+
* array's buffers and is valid while the handle is alive; a null element yields `{NULL, 0}`
867+
* without error. Errors on a non-canonical array (call `vx_array_canonicalize` first) or an
868+
* out-of-bounds index.
856869
*/
857-
const vx_string *vx_array_get_utf8(const vx_array *array, uint32_t index);
870+
vx_str vx_array_utf8_at(const vx_array *array, size_t index, vx_error **error_out);
858871

859872
/**
860-
* Return the binary at `index` in the array. The pointer will be null if the value at `index` is null.
861-
* The caller must free the returned pointer.
873+
* Return the binary value at `index` in a canonical Binary array.
874+
* Same contract as `vx_array_utf8_at`.
862875
*/
863-
const vx_binary *vx_array_get_binary(const vx_array *array, uint32_t index);
876+
vx_str vx_array_binary_at(const vx_array *array, size_t index, vx_error **error_out);
864877

865878
/**
866879
* Return the value at `index` in a Bool-typed array.
@@ -910,31 +923,6 @@ void vx_array_iterator_free(const vx_array_iterator *ptr);
910923
*/
911924
const vx_array *vx_array_iterator_next(vx_array_iterator *iter, vx_error **error_out);
912925

913-
/**
914-
* Clone a vx_binary. Returned handle must be release with vx_binary_free
915-
*/
916-
const vx_binary *vx_binary_clone(const vx_binary *ptr);
917-
918-
/**
919-
* Free a vx_binary
920-
*/
921-
void vx_binary_free(const vx_binary *ptr);
922-
923-
/**
924-
* Create a new Vortex UTF-8 string by copying from a pointer and length.
925-
*/
926-
const vx_binary *vx_binary_new(const char *ptr, size_t len);
927-
928-
/**
929-
* Return the length of the string in bytes.
930-
*/
931-
size_t vx_binary_len(const vx_binary *ptr);
932-
933-
/**
934-
* Return the pointer to the string data.
935-
*/
936-
const char *vx_binary_ptr(const vx_binary *ptr);
937-
938926
/**
939927
* Clone a vx_data_source. Returned handle must be release with vx_data_source_free
940928
*/
@@ -1108,9 +1096,12 @@ bool vx_dtype_is_timestamp(const DType *dtype);
11081096
uint8_t vx_dtype_time_unit(const DType *dtype);
11091097

11101098
/**
1111-
* Returns the time zone, assuming the type is time. Caller is responsible for freeing the returned pointer.
1099+
* Returns the time zone, assuming the type is time.
1100+
*
1101+
* The returned view borrows from `dtype` and is valid while that handle is alive.
1102+
* Returns a `{NULL, 0}` view when the timestamp has no time zone.
11121103
*/
1113-
const vx_string *vx_dtype_time_zone(const DType *dtype);
1104+
vx_str vx_dtype_time_zone(const DType *dtype);
11141105

11151106
/**
11161107
* Convert a dtype to ArrowSchema.
@@ -1137,9 +1128,11 @@ const vx_dtype *vx_dtype_from_arrow_schema(FFI_ArrowSchema *schema, vx_error **e
11371128
void vx_error_free(const vx_error *ptr);
11381129

11391130
/**
1140-
* Return an error message for this error
1131+
* Return the error message for this error.
1132+
*
1133+
* The returned view borrows from `error` and is valid while the error handle is alive.
11411134
*/
1142-
const vx_string *vx_error_get_message(const vx_error *error);
1135+
vx_str vx_error_message(const vx_error *error);
11431136

11441137
/**
11451138
* Return the stable category code for this error.
@@ -1217,7 +1210,7 @@ vx_expression *vx_expression_literal(const vx_scalar *scalar, vx_error **err);
12171210
* vx_expression_free(select);
12181211
* vx_expression_free(root);
12191212
*/
1220-
vx_expression *vx_expression_select(const char *const *names, size_t len, const vx_expression *child);
1213+
vx_expression *vx_expression_select(const vx_str *names, size_t len, const vx_expression *child);
12211214

12221215
/**
12231216
* Create an AND expression for multiple child expressions.
@@ -1279,8 +1272,11 @@ vx_expression *vx_expression_is_null(const vx_expression *child);
12791272
*
12801273
* Example: if child is Struct { name=u8, age=u16 } and we do
12811274
* vx_expression_get_item("name", child), output type will be DTYPE_U8
1275+
*
1276+
* The `item` bytes are copied. Returns NULL if `child` is NULL or `item` is
1277+
* not valid UTF-8.
12821278
*/
1283-
vx_expression *vx_expression_get_item(const char *item, const vx_expression *child);
1279+
vx_expression *vx_expression_get_item(vx_str item, const vx_expression *child);
12841280

12851281
/**
12861282
* Create an expression that checks if a value is contained in a list.
@@ -1299,10 +1295,7 @@ const vx_file *vx_file_clone(const vx_file *ptr);
12991295
*/
13001296
void vx_file_free(const vx_file *ptr);
13011297

1302-
void vx_file_write_array(const vx_session *session,
1303-
const char *path,
1304-
const vx_array *array,
1305-
vx_error **error_out);
1298+
void vx_file_write_array(const vx_session *session, vx_str path, const vx_array *array, vx_error **error_out);
13061299

13071300
/**
13081301
* Set the stderr logger to output at the specified level.
@@ -1401,11 +1394,10 @@ vx_scalar *vx_scalar_new_f16_bits(uint16_t bits, bool is_nullable);
14011394
/**
14021395
* Create a UTF-8 scalar.
14031396
*
1404-
* The byte range is copied into the scalar. A NULL data pointer is allowed only
1405-
* for an empty byte range. Invalid UTF-8 returns NULL and writes the error
1406-
* output.
1397+
* The string bytes are copied into the scalar. Invalid UTF-8 returns NULL and
1398+
* writes the error output.
14071399
*/
1408-
vx_scalar *vx_scalar_new_utf8(const char *ptr, size_t len, bool is_nullable, vx_error **err);
1400+
vx_scalar *vx_scalar_new_utf8(vx_str value, bool is_nullable, vx_error **err);
14091401

14101402
/**
14111403
* Create a binary scalar.
@@ -1638,11 +1630,10 @@ vx_session *vx_session_clone(const vx_session *session);
16381630
/**
16391631
* Opens a writable array stream, where sink is used to push values into the stream.
16401632
* To close the stream close the sink with `vx_array_sink_close`.
1633+
* The `path` bytes are copied.
16411634
*/
1642-
vx_array_sink *vx_array_sink_open_file(const vx_session *session,
1643-
const char *path,
1644-
const vx_dtype *dtype,
1645-
vx_error **error_out);
1635+
vx_array_sink *
1636+
vx_array_sink_open_file(const vx_session *session, vx_str path, const vx_dtype *dtype, vx_error **error_out);
16461637

16471638
/**
16481639
* Push an array into a file sink.
@@ -1662,36 +1653,6 @@ void vx_array_sink_close(vx_array_sink *sink, vx_error **error_out);
16621653
*/
16631654
void vx_array_sink_abort(vx_array_sink *sink);
16641655

1665-
/**
1666-
* Clone a vx_string. Returned handle must be release with vx_string_free
1667-
*/
1668-
const vx_string *vx_string_clone(const vx_string *ptr);
1669-
1670-
/**
1671-
* Free a vx_string
1672-
*/
1673-
void vx_string_free(const vx_string *ptr);
1674-
1675-
/**
1676-
* Create a new Vortex UTF-8 string by copying from a pointer and length.
1677-
*/
1678-
const vx_string *vx_string_new(const char *ptr, size_t len);
1679-
1680-
/**
1681-
* Create a new Vortex UTF-8 string by copying from a null-terminated C-style string.
1682-
*/
1683-
const vx_string *vx_string_new_from_cstr(const char *ptr);
1684-
1685-
/**
1686-
* Return the length of the string in bytes.
1687-
*/
1688-
size_t vx_string_len(const vx_string *ptr);
1689-
1690-
/**
1691-
* Return the pointer to the string data.
1692-
*/
1693-
const char *vx_string_ptr(const vx_string *ptr);
1694-
16951656
/**
16961657
* Free an owned [`vx_struct_column_builder`] object.
16971658
*/
@@ -1714,7 +1675,7 @@ vx_struct_column_builder *vx_struct_column_builder_new(const vx_validity *validi
17141675
* deallocate it using vx_struct_column_builder_free.
17151676
*/
17161677
void vx_struct_column_builder_add_field(vx_struct_column_builder *builder,
1717-
const char *name,
1678+
vx_str name,
17181679
const vx_array *field,
17191680
vx_error **error);
17201681

@@ -1754,10 +1715,12 @@ void vx_struct_fields_free(const vx_struct_fields *ptr);
17541715
uint64_t vx_struct_fields_nfields(const vx_struct_fields *dtype);
17551716

17561717
/**
1757-
* Return an owned name of the field at a given index.
1758-
* If index is out of bounds, returns NULL.
1718+
* Return the name of the field at a given index.
1719+
* If index is out of bounds, returns a `{NULL, 0}` view.
1720+
*
1721+
* The returned view borrows from `dtype` and is valid while that handle is alive.
17591722
*/
1760-
const vx_string *vx_struct_fields_field_name(const vx_struct_fields *dtype, size_t idx);
1723+
vx_str vx_struct_fields_field_name(const vx_struct_fields *dtype, size_t idx);
17611724

17621725
/**
17631726
* Return an owned dtype of the field at a given index.
@@ -1778,12 +1741,13 @@ vx_struct_fields_builder *vx_struct_fields_builder_new(void);
17781741
/**
17791742
* Add a field to the struct dtype builder.
17801743
*
1781-
* Takes ownership of both the `name` and `dtype` pointers.
1744+
* The `name` bytes are copied. Takes ownership of the `dtype` pointer.
17821745
* Must either free or finalize the builder.
17831746
*/
17841747
void vx_struct_fields_builder_add_field(vx_struct_fields_builder *builder,
1785-
const vx_string *name,
1786-
const vx_dtype *dtype);
1748+
vx_str name,
1749+
const vx_dtype *dtype,
1750+
vx_error **error_out);
17871751

17881752
/**
17891753
* Finalize the struct dtype builder, returning a new `vx_struct_fields`.
@@ -1795,3 +1759,22 @@ vx_struct_fields *vx_struct_fields_builder_finalize(vx_struct_fields_builder *bu
17951759
#ifdef __cplusplus
17961760
} // extern "C"
17971761
#endif // __cplusplus
1762+
1763+
#include <string.h>
1764+
1765+
/**
1766+
* Create a vx_str view over a null-terminated C string.
1767+
* The view borrows the bytes; it is valid for as long as `str` is.
1768+
*/
1769+
#ifdef __cplusplus
1770+
static inline vx_str vx_str_from_cstr(const char *str) {
1771+
vx_str s;
1772+
s.ptr = str;
1773+
s.len = strlen(str);
1774+
return s;
1775+
}
1776+
#else
1777+
static inline vx_str vx_str_from_cstr(const char *str) {
1778+
return (vx_str) {str, strlen(str)};
1779+
}
1780+
#endif

‎vortex-ffi/examples/dtype.c‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,9 @@ void print_struct_dtype(const vx_dtype *dtype) {
5858
printf("struct(\n");
5959
for (uint64_t i = 0; i < vx_struct_fields_nfields(fields); ++i) {
6060
const vx_dtype *field_dtype = vx_struct_fields_field_dtype(fields, i);
61-
const vx_string *field_name = vx_struct_fields_field_name(fields, i);
62-
printf(" %.*s = ", (int)vx_string_len(field_name), vx_string_ptr(field_name));
61+
const vx_str field_name = vx_struct_fields_field_name(fields, i);
62+
printf(" %.*s = ", (int)field_name.len, field_name.ptr);
6363
print_dtype(field_dtype);
64-
vx_string_free(field_name);
6564
vx_dtype_free(field_dtype);
6665
}
6766
printf(")");
@@ -124,8 +123,8 @@ void print_dtype(const vx_dtype *dtype) {
124123
}
125124

126125
void print_error(const char *what, const vx_error *error) {
127-
const vx_string *str = vx_error_get_message(error);
128-
fprintf(stderr, "%s: %.*s\n", what, (int)vx_string_len(str), vx_string_ptr(str));
126+
const vx_str str = vx_error_message(error);
127+
fprintf(stderr, "%s: %.*s\n", what, (int)str.len, str.ptr);
129128
}
130129

131130
int main(int argc, char **argv) {
@@ -141,7 +140,8 @@ int main(int argc, char **argv) {
141140
return 1;
142141
}
143142

144-
vx_data_source_options ds_options = {.paths = argv[1]};
143+
vx_str path = vx_str_from_cstr(argv[1]);
144+
vx_data_source_options ds_options = {.paths = &path, .paths_len = 1};
145145
const vx_data_source *data_source = vx_data_source_new(session, &ds_options, &error);
146146
if (data_source == NULL) {
147147
print_error("Failed to create data source", error);

0 commit comments

Comments
 (0)