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

refactor(c/driver/postgresql): Cleanups for result_helper signatures #2261

Merged
merged 2 commits into from
Nov 5, 2024
Merged

refactor(c/driver/postgresql): Cleanups for result_helper signatures #2261

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2c02c97
refactor(c/driver/postgresql): Cleanups for result_helper signatures
WillAyd Oct 18, 2024
ae6e171
Remove noexcept
WillAyd Oct 21, 2024
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
21 changes: 11 additions & 10 deletions c/driver/postgresql/result_helper.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ namespace adbcpq {

PqResultHelper::~PqResultHelper() { ClearResult(); }

Status PqResultHelper::PrepareInternal(int n_params, const Oid* param_oids) {
Status PqResultHelper::PrepareInternal(int n_params,
const Oid* param_oids) const noexcept {
// TODO: make stmtName a unique identifier?
PGresult* result =
PQprepare(conn_, /*stmtName=*/"", query_.c_str(), n_params, param_oids);
Expand All @@ -43,13 +44,13 @@ Status PqResultHelper::PrepareInternal(int n_params, const Oid* param_oids) {
return Status::Ok();
}

Status PqResultHelper::Prepare() { return PrepareInternal(0, nullptr); }
Status PqResultHelper::Prepare() const noexcept { return PrepareInternal(0, nullptr); }

Status PqResultHelper::Prepare(const std::vector<Oid>& param_oids) {
Status PqResultHelper::Prepare(const std::vector<Oid>& param_oids) const noexcept {
return PrepareInternal(param_oids.size(), param_oids.data());
}

Status PqResultHelper::DescribePrepared() {
Status PqResultHelper::DescribePrepared() noexcept {
ClearResult();
result_ = PQdescribePrepared(conn_, /*stmtName=*/"");
if (PQresultStatus(result_) != PGRES_COMMAND_OK) {
Expand All @@ -64,7 +65,7 @@ Status PqResultHelper::DescribePrepared() {
}

Status PqResultHelper::Execute(const std::vector<std::string>& params,
PostgresType* param_types) {
PostgresType* param_types) noexcept {
if (params.size() == 0 && param_types == nullptr && output_format_ == Format::kText) {
ClearResult();
result_ = PQexec(conn_, query_.c_str());
Expand Down Expand Up @@ -104,7 +105,7 @@ Status PqResultHelper::Execute(const std::vector<std::string>& params,
return Status::Ok();
}

Status PqResultHelper::ExecuteCopy() {
Status PqResultHelper::ExecuteCopy() noexcept {
// Remove trailing semicolon(s) from the query before feeding it into COPY
while (!query_.empty() && query_.back() == ';') {
query_.pop_back();
Expand All @@ -130,7 +131,7 @@ Status PqResultHelper::ExecuteCopy() {
}

Status PqResultHelper::ResolveParamTypes(PostgresTypeResolver& type_resolver,
PostgresType* param_types) {
PostgresType* param_types) noexcept {
struct ArrowError na_error;
ArrowErrorInit(&na_error);

Expand All @@ -156,7 +157,7 @@ Status PqResultHelper::ResolveParamTypes(PostgresTypeResolver& type_resolver,
}

Status PqResultHelper::ResolveOutputTypes(PostgresTypeResolver& type_resolver,
PostgresType* result_types) {
PostgresType* result_types) noexcept {
struct ArrowError na_error;
ArrowErrorInit(&na_error);

Expand All @@ -181,13 +182,13 @@ Status PqResultHelper::ResolveOutputTypes(PostgresTypeResolver& type_resolver,
return Status::Ok();
}

PGresult* PqResultHelper::ReleaseResult() {
PGresult* PqResultHelper::ReleaseResult() noexcept {
PGresult* out = result_;
result_ = nullptr;
return out;
}

int64_t PqResultHelper::AffectedRows() {
int64_t PqResultHelper::AffectedRows() const noexcept {
if (result_ == nullptr) {
return -1;
}
Expand Down
68 changes: 37 additions & 31 deletions c/driver/postgresql/result_helper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ struct PqRecord {
}
}

Result<std::vector<std::string>> ParseTextArray() {
Result<std::vector<std::string>> ParseTextArray() const {
std::string text_array(data, len);
text_array.erase(0, 1);
text_array.erase(text_array.size() - 1);
Expand All @@ -89,7 +89,7 @@ class PqResultRow {
PqResultRow() : result_(nullptr), row_num_(-1) {}
PqResultRow(PGresult* result, int row_num) : result_(result), row_num_(row_num) {}

PqRecord operator[](const int& col_num) const {
PqRecord operator[](int col_num) const noexcept {
Copy link
Contributor Author

@WillAyd WillAyd Oct 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note the change in the paramater declaration from const int& to int

It may not matter, but I believe the const reference type just adds an unnecessary layer of indirection

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, AFAIK there is little reason to use const int&

assert(col_num < PQnfields(result_));
const char* data = PQgetvalue(result_, row_num_, col_num);
const int len = PQgetlength(result_, row_num_, col_num);
Expand All @@ -98,9 +98,11 @@ class PqResultRow {
return PqRecord{data, len, is_null};
}

bool IsValid() { return result_ && row_num_ >= 0 && row_num_ < PQntuples(result_); }
bool IsValid() const noexcept {
return result_ && row_num_ >= 0 && row_num_ < PQntuples(result_);
}

PqResultRow Next() { return PqResultRow(result_, row_num_ + 1); }
PqResultRow Next() const noexcept { return PqResultRow(result_, row_num_ + 1); }

private:
PGresult* result_ = nullptr;
Expand Down Expand Up @@ -129,45 +131,47 @@ class PqResultHelper {

~PqResultHelper();

void set_param_format(Format format) { param_format_ = format; }
void set_output_format(Format format) { output_format_ = format; }
void set_param_format(Format format) noexcept { param_format_ = format; }
void set_output_format(Format format) noexcept { output_format_ = format; }

Status Prepare();
Status Prepare(const std::vector<Oid>& param_oids);
Status DescribePrepared();
Status Prepare() const noexcept;
Status Prepare(const std::vector<Oid>& param_oids) const noexcept;
Status DescribePrepared() noexcept;
Status Execute(const std::vector<std::string>& params = {},
PostgresType* param_types = nullptr);
Status ExecuteCopy();
PostgresType* param_types = nullptr) noexcept;
Status ExecuteCopy() noexcept;
Status ResolveParamTypes(PostgresTypeResolver& type_resolver,
PostgresType* param_types);
PostgresType* param_types) noexcept;
Status ResolveOutputTypes(PostgresTypeResolver& type_resolver,
PostgresType* result_types);
PostgresType* result_types) noexcept;

bool HasResult() { return result_ != nullptr; }
bool HasResult() const noexcept { return result_ != nullptr; }

void SetResult(PGresult* result) {
void SetResult(PGresult* result) noexcept {
ClearResult();
result_ = result;
}

PGresult* ReleaseResult();
PGresult* ReleaseResult() noexcept;

void ClearResult() {
void ClearResult() noexcept {
PQclear(result_);
result_ = nullptr;
}

int64_t AffectedRows();
int64_t AffectedRows() const noexcept;

int NumRows() const { return PQntuples(result_); }
int NumRows() const noexcept { return PQntuples(result_); }

int NumColumns() const { return PQnfields(result_); }
int NumColumns() const noexcept { return PQnfields(result_); }

const char* FieldName(int column_number) const {
const char* FieldName(int column_number) const noexcept {
return PQfname(result_, column_number);
}
Oid FieldType(int column_number) const { return PQftype(result_, column_number); }
PqResultRow Row(int i) { return PqResultRow(result_, i); }
Oid FieldType(int column_number) const noexcept {
return PQftype(result_, column_number);
}
PqResultRow Row(int i) const noexcept { return PqResultRow(result_, i); }

class iterator {
const PqResultHelper& outer_;
Expand All @@ -176,29 +180,31 @@ class PqResultHelper {
public:
explicit iterator(const PqResultHelper& outer, int curr_row = 0)
: outer_(outer), curr_row_(curr_row) {}
iterator& operator++() {
iterator& operator++() noexcept {
curr_row_++;
return *this;
}
iterator operator++(int) {
iterator operator++(int) noexcept {
iterator retval = *this;
++(*this);
return retval;
}
bool operator==(iterator other) const {
bool operator==(iterator other) const noexcept {
return outer_.result_ == other.outer_.result_ && curr_row_ == other.curr_row_;
}
bool operator!=(iterator other) const { return !(*this == other); }
PqResultRow operator*() { return PqResultRow(outer_.result_, curr_row_); }
bool operator!=(iterator other) const noexcept { return !(*this == other); }
PqResultRow operator*() const noexcept {
return PqResultRow(outer_.result_, curr_row_);
}
using iterator_category = std::forward_iterator_tag;
using difference_type = std::ptrdiff_t;
using value_type = std::vector<PqResultRow>;
using pointer = const std::vector<PqResultRow>*;
using reference = const std::vector<PqResultRow>&;
};

iterator begin() { return iterator(*this); }
iterator end() { return iterator(*this, NumRows()); }
iterator begin() const noexcept { return iterator(*this); }
iterator end() const noexcept { return iterator(*this, NumRows()); }

private:
PGresult* result_ = nullptr;
Expand All @@ -207,7 +213,7 @@ class PqResultHelper {
Format param_format_ = Format::kText;
Format output_format_ = Format::kText;

Status PrepareInternal(int n_params, const Oid* param_oids);
Status PrepareInternal(int n_params, const Oid* param_oids) const noexcept;
};

} // namespace adbcpq
Loading