Skip to content

Commit

Permalink
Fix read_table compilation on windows (#455)
Browse files Browse the repository at this point in the history
<!--
Thanks for sending a pull request!  Here are some tips for you:
1. If this is your first time, please read our contributor guidelines:
https://github.com/delta-incubator/delta-kernel-rs/blob/main/CONTRIBUTING.md
2. Run `cargo t --all-features --all-targets` to get started testing,
and run `cargo fmt`.
  3. Ensure you have added or run the appropriate tests for your PR.
4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]
Your PR title ...'.
  5. Be sure to keep the PR description updated to reflect all changes.
-->

## What changes are proposed in this pull request?
<!--
Please clarify what changes you are proposing and why the changes are
needed.
The purpose of this section is to outline the changes, why they are
needed, and how this PR fixes the issue.
If the reason for the change is already explained clearly in an issue,
then it does not need to be restated here.
1. If you propose a new API or feature, clarify the use case for a new
API or feature.
  2. If you fix a bug, you can clarify why it is a bug.
-->
Make `read_table` actually compile on windows.

1. Add a `strndup` implementation for windows (thanks @hntd187)
2. Fix some types that the windows compiler doesn't like
3. Update `CMakeLists.txt` with correct things (thanks @hntd187 again)
4. Update the README with some minimal instructions.

<!--
Uncomment this section if there are any changes affecting public APIs:
### This PR affects the following public APIs

If there are breaking changes, please ensure the `breaking-changes`
label gets added by CI, and describe why the changes are needed.

Note that _new_ public APIs are not considered breaking.
-->


## How was this change tested?
<!--
Please make sure to add test cases that check the changes thoroughly
including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please
clarify how you tested, ideally via a reproducible test documented in
the PR description.
-->

Built and run on windows (without `PRINT_DATA`)
  • Loading branch information
nicklan authored Nov 6, 2024
1 parent bd2ea9f commit 0214a96
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 9 deletions.
4 changes: 2 additions & 2 deletions ffi/examples/read-table/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ add_test(NAME read_and_print_basic_partitioned COMMAND ${TestRunner} ${DatPath}/

if(WIN32)
set(CMAKE_C_FLAGS_DEBUG "/MT")
target_link_libraries(read_table PUBLIC ws2_32 userenv bcrypt ntdll)
target_link_libraries(read_table PUBLIC ws2_32 userenv bcrypt ncrypt crypt32 secur32 ntdll RuntimeObject)
endif(WIN32)

if(MSVC)
target_compile_options(read_table PRIVATE /W4 /WX)
target_compile_options(read_table PRIVATE /W3 /WX)
else()
# no-strict-prototypes because arrow headers have fn defs without prototypes
target_compile_options(read_table PRIVATE -Wall -Wextra -Wpedantic -Werror -Wno-strict-prototypes -g -fsanitize=address)
Expand Down
42 changes: 41 additions & 1 deletion ffi/examples/read-table/README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,44 @@
read table
==========

Simple example to show how to read and dump the data of a table using kernel's cffi, and arrow-glib
Simple example to show how to read and dump the data of a table using kernel's cffi, and arrow-glib.

# Building

This example is built with [cmake]. Instructions below assume you start in the directory containing this README.

## Linux / MacOS

Most likely something like this should work:
```
$ mkdir build
$ cd build
$ cmake ..
$ make
$ ./read_table [path/to/table]
```

## Windows

For windows, assuming you already have a working cmake + c toolchain:
```
PS mkdir build
PS cd build
PS cmake -G "Visual Studio 17 2022" ..
PS cmake --build .
PS .\Debug\read_table.exe [path\to\table]
```

If running on windows you should also run `chcp.exe 65001` to set the codepage to utf-8, or things
won't print out correctly.

## Arrow GLib
This example uses the `arrow-glib (c)` component from arrow to print out data. This requires
_installing_ that component which can be non-trivial. Please see
[here](https://arrow.apache.org/install/) to find installation instructions for your system.

If you don't want to have to install this, you can run `ccmake ..` (`cmake-gui.exe ..` on windows)
from the `build` directory, and turn `OFF`/uncheckmark `PRINT_DATA`. Then "configure" and
"generate" and follow the above instructions again.

[cmake]: https://cmake.org/
2 changes: 1 addition & 1 deletion ffi/examples/read-table/arrow.c
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ static GArrowRecordBatch* add_partition_columns(
gint64 cols = garrow_record_batch_get_n_columns(record_batch);
GArrowRecordBatch* cur_record_batch = record_batch;
GError* error = NULL;
for (int i = 0; i < partition_cols->len; i++) {
for (uintptr_t i = 0; i < partition_cols->len; i++) {
char* col = partition_cols->cols[i];
guint pos = cols + i;
KernelStringSlice key = { col, strlen(col) };
Expand Down
20 changes: 16 additions & 4 deletions ffi/examples/read-table/read_table.c
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ void print_selection_vector(const char* indent, const KernelBoolSlice* selection
void print_partition_info(struct EngineContext* context, const CStringMap* partition_values)
{
#ifdef VERBOSE
for (int i = 0; i < context->partition_cols->len; i++) {
for (uintptr_t i = 0; i < context->partition_cols->len; i++) {
char* col = context->partition_cols->cols[i];
KernelStringSlice key = { col, strlen(col) };
char* partition_val = get_from_map(partition_values, key, allocate_string);
Expand Down Expand Up @@ -79,6 +79,18 @@ EngineError* allocate_error(KernelError etype, const KernelStringSlice msg)
return (EngineError*)error;
}

#ifdef WIN32 // windows doesn't have strndup
char *strndup(const char *s, size_t n) {
size_t len = strnlen(s, n);
char *p = malloc(len + 1);
if (p) {
memcpy(p, s, len);
p[len] = '\0';
}
return p;
}
#endif

// utility to turn a slice into a char*
void* allocate_string(const KernelStringSlice slice)
{
Expand Down Expand Up @@ -165,7 +177,7 @@ void visit_partition(void* context, const KernelStringSlice partition)
PartitionList* get_partition_list(SharedGlobalScanState* state)
{
print_diag("Building list of partition columns\n");
int count = get_partition_column_count(state);
uintptr_t count = get_partition_column_count(state);
PartitionList* list = malloc(sizeof(PartitionList));
// We set the `len` to 0 here and use it to track how many items we've added to the list
list->len = 0;
Expand All @@ -184,7 +196,7 @@ PartitionList* get_partition_list(SharedGlobalScanState* state)
}
if (list->len > 0) {
print_diag("Partition columns are:\n");
for (int i = 0; i < list->len; i++) {
for (uintptr_t i = 0; i < list->len; i++) {
print_diag(" - %s\n", list->cols[i]);
}
} else {
Expand All @@ -195,7 +207,7 @@ PartitionList* get_partition_list(SharedGlobalScanState* state)
}

void free_partition_list(PartitionList* list) {
for (int i = 0; i < list->len; i++) {
for (uintptr_t i = 0; i < list->len; i++) {
free(list->cols[i]);
}
free(list->cols);
Expand Down
2 changes: 1 addition & 1 deletion ffi/examples/read-table/read_table.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
// A list of partition column names
typedef struct PartitionList
{
int len;
uintptr_t len;
char** cols;
} PartitionList;

Expand Down

0 comments on commit 0214a96

Please sign in to comment.