Skip to content

Commit

Permalink
review comments, formatting, linting
Browse files Browse the repository at this point in the history
  • Loading branch information
bkietz committed Aug 15, 2024
1 parent 7e5aaf5 commit 7556e29
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 55 deletions.
9 changes: 7 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,13 @@ if(NANOARROW_IPC AND (NANOARROW_BUILD_INTEGRATION_TESTS OR NANOARROW_BUILD_TESTS
PUBLIC $<BUILD_INTERFACE:${NANOARROW_BUILD_INCLUDE_DIR}>
$<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}/src>
$<INSTALL_INTERFACE:include>)
target_link_libraries(nanoarrow_ipc_integration PRIVATE nanoarrow_testing
nanoarrow_ipc flatccrt gtest gmock)
target_link_libraries(nanoarrow_ipc_integration
PRIVATE nanoarrow_testing
nanoarrow_ipc
flatccrt
gtest
gmock
nanoarrow_coverage_config)
endif()

if(NANOARROW_DEVICE)
Expand Down
1 change: 1 addition & 0 deletions src/nanoarrow/common/array.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <errno.h>
#include <inttypes.h>
#include <stdarg.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

Expand Down
4 changes: 1 addition & 3 deletions src/nanoarrow/integration/ipc_integration.cc
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,7 @@ struct File {

size_t bytes_read = 0;
while (bytes_read < contents.size()) {
bytes_read +=
fread(contents.data() + bytes_read, 1, contents.size() - bytes_read, file_);
bytes_read += fread(&contents[bytes_read], 1, contents.size() - bytes_read, file_);
}
return contents;
}
Expand Down Expand Up @@ -370,4 +369,3 @@ TEST(Integration, ErrorMessages) {
testing::HasSubstr("Expected file of more than 8 bytes, got 3"));
}
}

7 changes: 5 additions & 2 deletions src/nanoarrow/ipc/encoder.c
Original file line number Diff line number Diff line change
Expand Up @@ -662,8 +662,11 @@ ArrowErrorCode ArrowIpcEncoderEncodeFooter(struct ArrowIpcEncoder* encoder,
ns(Footer_recordBatches_extend(builder, n_blocks));
FLATCC_RETURN_IF_NULL(flatcc_RecordBatch_blocks, error);
for (int64_t i = 0; i < n_blocks; i++) {
struct ns(Block) block = {blocks[i].offset, (int32_t)blocks[i].metadata_length,
blocks[i].body_length};
struct ns(Block) block = {
blocks[i].offset,
blocks[i].metadata_length,
blocks[i].body_length,
};
flatcc_RecordBatch_blocks[i] = block;
}
FLATCC_RETURN_UNLESS_0(Footer_recordBatches_end(builder), error);
Expand Down
15 changes: 8 additions & 7 deletions src/nanoarrow/ipc/encoder_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,24 +78,25 @@ TEST(NanoarrowIpcTest, NanoarrowIpcFooterEncoding) {
ASSERT_EQ(ArrowIpcEncoderInit(encoder.get()), NANOARROW_OK);

nanoarrow::ipc::UniqueFooter footer;
ASSERT_EQ(ArrowSchemaInitFromType(&footer->schema, NANOARROW_TYPE_STRUCT), NANOARROW_OK);
ASSERT_EQ(ArrowSchemaInitFromType(&footer->schema, NANOARROW_TYPE_STRUCT),
NANOARROW_OK);

nanoarrow::UniqueBuffer footer_buffer, raw_schema_buffer;
struct ArrowError error;

EXPECT_EQ(ArrowIpcEncoderEncodeFooter(encoder.get(), footer.get(), &error),
NANOARROW_OK)
<< error.message;
EXPECT_EQ(
ArrowIpcEncoderFinalizeBuffer(encoder.get(), /*encapsulate=*/false, footer_buffer.get()),
NANOARROW_OK);
EXPECT_EQ(ArrowIpcEncoderFinalizeBuffer(encoder.get(), /*encapsulate=*/false,
footer_buffer.get()),
NANOARROW_OK);

EXPECT_EQ(ArrowIpcEncoderEncodeSchema(encoder.get(), &footer->schema, &error),
NANOARROW_OK)
<< error.message;
EXPECT_EQ(
ArrowIpcEncoderFinalizeBuffer(encoder.get(), /*encapsulate=*/false, raw_schema_buffer.get()),
NANOARROW_OK);
EXPECT_EQ(ArrowIpcEncoderFinalizeBuffer(encoder.get(), /*encapsulate=*/false,
raw_schema_buffer.get()),
NANOARROW_OK);

EXPECT_GT(footer_buffer->size_bytes, raw_schema_buffer->size_bytes);
}
3 changes: 2 additions & 1 deletion src/nanoarrow/ipc/writer.c
Original file line number Diff line number Diff line change
Expand Up @@ -296,9 +296,10 @@ ArrowErrorCode ArrowIpcWriterWriteArrayView(struct ArrowIpcWriter* writer,
error);

if (private->writing_file) {
_NANOARROW_CHECK_RANGE(private->buffer.size_bytes, 0, INT32_MAX);
struct ArrowIpcFileBlock block = {
.offset = private->bytes_written,
.metadata_length = private->buffer.size_bytes,
.metadata_length = (int32_t) private->buffer.size_bytes,
.body_length = private->body_buffer.size_bytes,
};
NANOARROW_RETURN_NOT_OK_WITH_ERROR(
Expand Down
98 changes: 58 additions & 40 deletions src/nanoarrow/nanoarrow_ipc.h
Original file line number Diff line number Diff line change
Expand Up @@ -417,36 +417,6 @@ ArrowErrorCode ArrowIpcArrayStreamReaderInit(
struct ArrowArrayStream* out, struct ArrowIpcInputStream* input_stream,
struct ArrowIpcArrayStreamReaderOptions* options);

/// \brief The magic which appears at the beginning and end of an IPC file, 0-padded.
#define NANOARROW_IPC_FILE_PADDED_MAGIC "ARROW1\0"

/// \brief Represents a byte range in an IPC file.
struct ArrowIpcFileBlock {
/// \brief offset relative to the first byte of the file.
int64_t offset;
/// \brief length of encapsulated metadata Message (including padding)
int64_t metadata_length;
/// \brief length of contiguous body buffers (including padding)
int64_t body_length;
};

/// \brief A Footer for use in an IPC file
///
/// This structure is intended to be allocated by the caller, initialized using
/// ArrowIpcFooterInit(), and released with ArrowIpcFooterReset().
struct ArrowIpcFooter {
/// \brief the Footer's embedded Schema
struct ArrowSchema schema;
/// \brief all blocks containing RecordBatch Messages
struct ArrowBuffer record_batch_blocks;
};

/// \brief Initialize a Footer
void ArrowIpcFooterInit(struct ArrowIpcFooter* footer);

/// \brief Release all resources attached to an footer
void ArrowIpcFooterReset(struct ArrowIpcFooter* footer);

/// \brief Encoder for Arrow IPC messages
///
/// This structure is intended to be allocated by the caller,
Expand Down Expand Up @@ -492,16 +462,6 @@ ArrowErrorCode ArrowIpcEncoderEncodeSimpleRecordBatch(
struct ArrowIpcEncoder* encoder, const struct ArrowArrayView* array_view,
struct ArrowBuffer* body_buffer, struct ArrowError* error);

/// \brief Encode a Footer for use in an IPC file
///
/// \warning This API is currently only public for use in integration testing;
/// use at your own risk.
///
/// Returns ENOMEM if allocation fails, NANOARROW_OK otherwise.
ArrowErrorCode ArrowIpcEncoderEncodeFooter(struct ArrowIpcEncoder* encoder,
const struct ArrowIpcFooter* footer,
struct ArrowError* error);

/// \brief An user-extensible output data sink
struct ArrowIpcOutputStream {
/// \brief Write up to buf_size_bytes from stream into buf
Expand Down Expand Up @@ -605,6 +565,64 @@ ArrowErrorCode ArrowIpcWriterStartFile(struct ArrowIpcWriter* writer,
ArrowErrorCode ArrowIpcWriterFinalizeFile(struct ArrowIpcWriter* writer,
struct ArrowError* error);
/// @}

// Internal APIs:

/// \brief The magic which appears at the beginning and end of an IPC file, 0-padded.
///
/// \warning This API is currently only public for use in integration testing;
/// use at your own risk.
#define NANOARROW_IPC_FILE_PADDED_MAGIC "ARROW1\0"

/// \brief Represents a byte range in an IPC file.
///
/// \warning This API is currently only public for use in integration testing;
/// use at your own risk.
struct ArrowIpcFileBlock {
/// \brief offset relative to the first byte of the file.
int64_t offset;
/// \brief length of encapsulated metadata Message (including padding)
int32_t metadata_length;
/// \brief length of contiguous body buffers (including padding)
int64_t body_length;
};

/// \brief A Footer for use in an IPC file
///
/// \warning This API is currently only public for use in integration testing;
/// use at your own risk.
///
/// This structure is intended to be allocated by the caller, initialized using
/// ArrowIpcFooterInit(), and released with ArrowIpcFooterReset().
struct ArrowIpcFooter {
/// \brief the Footer's embedded Schema
struct ArrowSchema schema;
/// \brief all blocks containing RecordBatch Messages
struct ArrowBuffer record_batch_blocks;
};

/// \brief Initialize a Footer
///
/// \warning This API is currently only public for use in integration testing;
/// use at your own risk.
void ArrowIpcFooterInit(struct ArrowIpcFooter* footer);

/// \brief Release all resources attached to an footer
///
/// \warning This API is currently only public for use in integration testing;
/// use at your own risk.
void ArrowIpcFooterReset(struct ArrowIpcFooter* footer);

/// \brief Encode a Footer for use in an IPC file
///
/// \warning This API is currently only public for use in integration testing;
/// use at your own risk.
///
/// Returns ENOMEM if allocation fails, NANOARROW_OK otherwise.
ArrowErrorCode ArrowIpcEncoderEncodeFooter(struct ArrowIpcEncoder* encoder,
const struct ArrowIpcFooter* footer,
struct ArrowError* error);

#ifdef __cplusplus
}
#endif
Expand Down

0 comments on commit 7556e29

Please sign in to comment.