Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assorted clang-tidy cleanups in writer.cpp #344

Merged
merged 1 commit into from
Sep 27, 2024
Merged

Assorted clang-tidy cleanups in writer.cpp #344

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 43 additions & 34 deletions src/pantab/writer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@
static auto GetHyperTypeFromArrowSchema(struct ArrowSchema *schema,
ArrowError *error)
-> hyperapi::SqlType {
struct ArrowSchemaView schema_view;
struct ArrowSchemaView schema_view {};
if (ArrowSchemaViewInit(&schema_view, schema, error) != 0) {
throw std::runtime_error("Issue converting to hyper type: " +
std::string(error->message));
std::string(&error->message[0]));
}

switch (schema_view.type) {
Expand Down Expand Up @@ -79,19 +79,24 @@ class InsertHelper {
if (ArrowArrayViewInitFromSchema(array_view_.get(), child_schema, error_) !=
0) {
throw std::runtime_error("Could not construct insert helper: " +
std::string{error_->message});
std::string{&error_->message[0]});
}

if (ArrowArrayViewSetArray(array_view_.get(),
chunk_->children[column_position_],
error_) != 0) {
throw std::runtime_error("Could not set array view: " +
std::string{error_->message});
std::string{&error_->message[0]});
}
}

InsertHelper(const InsertHelper &) = delete;
InsertHelper &operator=(const InsertHelper &) = delete;
InsertHelper(InsertHelper &&) = delete;
InsertHelper &operator=(InsertHelper &&) = delete;

virtual ~InsertHelper() = default;
virtual void InsertValueAtIndex(size_t) {}
virtual void InsertValueAtIndex(int64_t) {}

protected:
hyperapi::Inserter &inserter_;
Expand All @@ -106,7 +111,7 @@ template <typename T> class IntegralInsertHelper : public InsertHelper {
public:
using InsertHelper::InsertHelper;

void InsertValueAtIndex(size_t idx) override {
void InsertValueAtIndex(int64_t idx) override {
if (ArrowArrayViewIsNull(array_view_.get(), idx)) {
// MSVC on cibuildwheel doesn't like this templated optional
// inserter_->add(std::optional<T>{std::nullopt});
Expand All @@ -124,7 +129,7 @@ class UInt32InsertHelper : public InsertHelper {
public:
using InsertHelper::InsertHelper;

void InsertValueAtIndex(size_t idx) override {
void InsertValueAtIndex(int64_t idx) override {
if (ArrowArrayViewIsNull(array_view_.get(), idx)) {
// MSVC on cibuildwheel doesn't like this templated optional
// inserter_->add(std::optional<T>{std::nullopt});
Expand All @@ -142,7 +147,7 @@ template <typename T> class FloatingInsertHelper : public InsertHelper {
public:
using InsertHelper::InsertHelper;

void InsertValueAtIndex(size_t idx) override {
void InsertValueAtIndex(int64_t idx) override {
if (ArrowArrayViewIsNull(array_view_.get(), idx)) {
// MSVC on cibuildwheel doesn't like this templated optional
// inserter_->add(std::optional<T>{std::nullopt});
Expand All @@ -160,7 +165,7 @@ class BinaryInsertHelper : public InsertHelper {
public:
using InsertHelper::InsertHelper;

void InsertValueAtIndex(size_t idx) override {
void InsertValueAtIndex(int64_t idx) override {
if (ArrowArrayViewIsNull(array_view_.get(), idx)) {
// MSVC on cibuildwheel doesn't like this templated optional
// inserter_->add(std::optional<std:::string_view>{std::nullopt});
Expand All @@ -180,7 +185,7 @@ template <typename OffsetT> class Utf8InsertHelper : public InsertHelper {
public:
using InsertHelper::InsertHelper;

void InsertValueAtIndex(size_t idx) override {
void InsertValueAtIndex(int64_t idx) override {
if (ArrowArrayViewIsNull(array_view_.get(), idx)) {
// MSVC on cibuildwheel doesn't like this templated optional
// inserter_->add(std::optional<std:::string_view>{std::nullopt});
Expand All @@ -205,7 +210,7 @@ class Date32InsertHelper : public InsertHelper {
public:
using InsertHelper::InsertHelper;

void InsertValueAtIndex(size_t idx) override {
void InsertValueAtIndex(int64_t idx) override {
constexpr size_t elem_size = sizeof(int32_t);
if (ArrowArrayViewIsNull(array_view_.get(), idx)) {
// MSVC on cibuildwheel doesn't like this templated optional
Expand All @@ -214,7 +219,7 @@ class Date32InsertHelper : public InsertHelper {
return;
}

int32_t value;
int32_t value{};
memcpy(&value,
array_view_->buffer_views[1].data.as_uint8 + (idx * elem_size),
elem_size);
Expand All @@ -235,11 +240,15 @@ class Date32InsertHelper : public InsertHelper {
}
};

static constexpr int64_t MicrosecondsPerSecond = 1'000'000;
static constexpr int64_t MicrosecondsPerMillisecond = 1'000;
static constexpr int64_t NanosecondsPerMicrosecond = 1'000;

template <enum ArrowTimeUnit TU> class TimeInsertHelper : public InsertHelper {
public:
using InsertHelper::InsertHelper;

void InsertValueAtIndex(size_t idx) override {
void InsertValueAtIndex(int64_t idx) override {
if (ArrowArrayViewIsNull(array_view_.get(), idx)) {
// MSVC on cibuildwheel doesn't like this templated optional
// inserter_->add(std::optional<T>{std::nullopt});
Expand All @@ -250,11 +259,11 @@ template <enum ArrowTimeUnit TU> class TimeInsertHelper : public InsertHelper {
int64_t value = ArrowArrayViewGetIntUnsafe(array_view_.get(), idx);
// TODO: check for overflow in these branches
if constexpr (TU == NANOARROW_TIME_UNIT_SECOND) {
value *= 1'000'000;
value *= MicrosecondsPerSecond;
} else if constexpr (TU == NANOARROW_TIME_UNIT_MILLI) {
value *= 1000;
value *= MicrosecondsPerMillisecond;
} else if constexpr (TU == NANOARROW_TIME_UNIT_NANO) {
value /= 1000;
value /= NanosecondsPerMicrosecond;
}
hyperapi::internal::ValueInserter{inserter_}.addValue(value);
}
Expand All @@ -265,7 +274,7 @@ class TimestampInsertHelper : public InsertHelper {
public:
using InsertHelper::InsertHelper;

void InsertValueAtIndex(size_t idx) override {
void InsertValueAtIndex(int64_t idx) override {
constexpr size_t elem_size = sizeof(int64_t);
if (ArrowArrayViewIsNull(array_view_.get(), idx)) {
// MSVC on cibuildwheel doesn't like this templated optional
Expand All @@ -274,18 +283,18 @@ class TimestampInsertHelper : public InsertHelper {
return;
}

int64_t value;
int64_t value{};
memcpy(&value,
array_view_->buffer_views[1].data.as_uint8 + (idx * elem_size),
elem_size);

// TODO: need overflow checks here
if constexpr (TU == NANOARROW_TIME_UNIT_SECOND) {
value *= 1000000;
value *= MicrosecondsPerSecond;
} else if constexpr (TU == NANOARROW_TIME_UNIT_MILLI) {
value *= 1000;
value *= MicrosecondsPerMillisecond;
} else if constexpr (TU == NANOARROW_TIME_UNIT_NANO) {
value /= 1000;
value /= NanosecondsPerMicrosecond;
}

constexpr int64_t USEC_TABLEAU_TO_UNIX_EPOCH = 210866803200000000LL;
Expand All @@ -305,15 +314,15 @@ class IntervalInsertHelper : public InsertHelper {
public:
using InsertHelper::InsertHelper;

void InsertValueAtIndex(size_t idx) override {
void InsertValueAtIndex(int64_t idx) override {
if (ArrowArrayViewIsNull(array_view_.get(), idx)) {
// MSVC on cibuildwheel doesn't like this templated optional
// inserter_->add(std::optional<timestamp_t>{std::nullopt});
hyperapi::internal::ValueInserter{inserter_}.addNull();
return;
}

struct ArrowInterval arrow_interval;
struct ArrowInterval arrow_interval {};
ArrowIntervalInit(&arrow_interval, NANOARROW_TYPE_INTERVAL_MONTH_DAY_NANO);
ArrowArrayViewGetIntervalUnsafe(array_view_.get(), idx, &arrow_interval);
const auto usec = static_cast<int32_t>(arrow_interval.ns / 1000);
Expand All @@ -338,7 +347,7 @@ class DecimalInsertHelper : public InsertHelper {
: InsertHelper(inserter, chunk, schema, error, column_position),
precision_(precision), scale_(scale) {}

void InsertValueAtIndex(size_t idx) override {
void InsertValueAtIndex(int64_t idx) override {
if (ArrowArrayViewIsNull(array_view_.get(), idx)) {
// MSVC on cibuildwheel doesn't like this templated optional
// inserter_->add(std::optional<timestamp_t>{std::nullopt});
Expand All @@ -347,11 +356,11 @@ class DecimalInsertHelper : public InsertHelper {
}

constexpr int32_t bitwidth = 128;
struct ArrowDecimal decimal;
struct ArrowDecimal decimal {};
ArrowDecimalInit(&decimal, bitwidth, precision_, scale_);
ArrowArrayViewGetDecimalUnsafe(array_view_.get(), idx, &decimal);

struct ArrowBuffer buffer;
struct ArrowBuffer buffer {};
ArrowBufferInit(&buffer);
if (ArrowDecimalAppendDigitsToBuffer(&decimal, &buffer)) {
throw std::runtime_error("could not create buffer from decmial value");
Expand Down Expand Up @@ -416,11 +425,11 @@ static auto MakeInsertHelper(hyperapi::Inserter &inserter,
// right now we pass false as the template paramter to the
// PrimitiveInsertHelper as that is all pandas generates; other libraries may
// need the true variant
struct ArrowSchemaView schema_view;
struct ArrowSchemaView schema_view {};
if (ArrowSchemaViewInit(&schema_view, schema->children[column_position],
error) != 0) {
throw std::runtime_error("Issue generating insert helper: " +
std::string(error->message));
std::string(&error->message[0]));
}

switch (schema_view.type) {
Expand Down Expand Up @@ -644,7 +653,7 @@ void write_to_hyper(
throw std::runtime_error("Could not read from arrow schema:" + error_msg);
}

struct ArrowError error;
struct ArrowError error {};
std::vector<hyperapi::TableDefinition::Column> hyper_columns;
std::vector<hyperapi::Inserter::ColumnMapping> column_mappings;
// subtley different from hyper_columns with geo
Expand Down Expand Up @@ -698,11 +707,11 @@ void write_to_hyper(
"Unexpected code path hit - contact a developer");
}
} else {
struct ArrowSchemaView schema_view;
struct ArrowSchemaView schema_view {};
if (ArrowSchemaViewInit(&schema_view, schema->children[i], &error)) {
throw std::runtime_error(
"Could not init schema view from child schema " +
std::to_string(i) + ": " + std::string(error.message));
std::to_string(i) + ": " + std::string(&error.message[0]));
}

if (schema_view.type == NANOARROW_TYPE_DECIMAL128) {
Expand Down Expand Up @@ -756,12 +765,12 @@ void write_to_hyper(
auto inserter = hyperapi::Inserter(connection, table_def, column_mappings,
inserter_defs);

struct ArrowArray c_chunk;
int errcode;
struct ArrowArray c_chunk {};
int errcode{};
while ((errcode = stream->get_next(stream.get(), &c_chunk) == 0) &&
c_chunk.release != nullptr) {
nanoarrow::UniqueArray chunk{&c_chunk};
const int nrows = chunk->length;
const auto nrows = chunk->length;
if (nrows < 0) {
throw std::runtime_error("Unexpected array length < 0");
}
Expand Down
Loading