Add disconnected RowSet class with typed Row access#46
Conversation
|
It will be difficult to review and merge if a PR includes things present in others not yet merged. |
Agreed. I mistakenly assumed it would be easier for you to review the three PRs sequentially, and that the other two would be trivial. I’ll rebase this one out on its own. P.S.: Please let me know if you’d like me to do the same for #45 as well. |
|
Rebased onto latest |
|
2nd attempt. Row — shared, non-owning typed view
This eliminates ~900 lines of duplicated reading/conversion logic from RowSet — disconnected row buffer
API// Lightweight non-owning typed view of a single row
class Row final
{
public:
Row(const std::byte* message, const std::vector<Descriptor>& descriptors,
NumericConverter& numericConverter, CalendarConverter& calendarConverter);
bool isNull(unsigned index);
std::optional<bool> getBool(unsigned index);
std::optional<std::int32_t> getInt32(unsigned index);
std::optional<std::string> getString(unsigned index);
// ... all other typed accessors from Statement ...
template <typename T> T get(unsigned index);
template <Aggregate T> T get();
template <TupleLike T> T get();
template <VariantLike V> V get(unsigned index);
};
// Disconnected row buffer with typed access
class RowSet final
{
public:
explicit RowSet(Statement& statement, unsigned maxRows);
unsigned getCount() const noexcept;
unsigned getMessageLength() const noexcept;
Row getRow(unsigned index); // typed access
const std::byte* getRawRow(unsigned index); // raw access
const std::vector<std::byte>& getBuffer() const noexcept;
};
// Statement gains currentRow()
class Statement
{
public:
Row currentRow(); // view of current output buffer
// existing getters now delegate to currentRow()
};ODBC driver usage (N-row prefetch with typed access)Statement select{attachment, transaction, sql, options};
select.execute(transaction);
RowSet batch{select, 64};
for (unsigned i = 0; i < batch.getCount(); ++i)
{
auto row = batch.getRow(i);
auto name = row.getString(0);
auto age = row.getInt32(1);
}Changes
Impact
|
|
Rebased onto latest |
|
Rebased onto latest |
|
@asfernandes! The work on the Firebird ODBC driver is moving along nicely. I think we’re getting close to the point where we’ll need this PR in order to fully use Do you have any thoughts, suggestions, or concerns about this? P.S. If you’re short on time and can’t review this yet, that’s totally fine. 👍 |
14a42fe to
c7fa972
Compare
|
@fdcastel I did a general architecture refactor in https://github.com/asfernandes/fb-cpp/tree/issue-42-req3 in top of your branch. Please check it and if ok, I'd like to commit it to your PR to continue the review there. |
|
I Can't review it now, but, Sure! Be my guest! You can add new commits here. Or open a new branch / PR. I'll review tomorrow morning. |
|
I pushed the changes here. |
Looks great! Two considerations:
I can take care of both if you’d like (with some guidance, of course 😉) |
|
For reference, I'm thinking something along the lines of: // ODBC driver bulk fetch pattern
Statement select{attachment, transaction, sql, options};
select.execute(transaction);
while (true)
{
RowSet batch{select, rowArraySize};
if (batch.getCount() == 0)
break;
for (unsigned i = 0; i < batch.getCount(); ++i)
{
// Option A: typed access via Row
auto row = batch.getRow(i);
auto val = row.getInt32(0);
// Option B: zero-copy raw access for ODBC buffer filling
auto rawRow = batch.getRawRow(i);
std::memcpy(odbcBuffer + i * rowSize, rawRow.data(), rawRow.size());
}
}This eliminates the per-row Suggestions welcome! |
|
@asfernandes I went ahead and updated the PR description to reflect the latest changes. Feel free to comment. |
?
Please. |
Forget it, brain fart. I messed up the branches when comparing 🤦🏻♂️. Lack of morning coffee and all.
Working on it. |
|
Done!
|
|
Released as 0.0.4. |
Addresses REQ-3 from #42 per @asfernandes feedback.
The original REQ-3 proposed a
fetchNext(void* buffer)overload. This approach was rejected and instead requested a "disconnected RowSet (a buffer of rows, disconnected after fetched) class."The RowSet feature is critical for the ODBC driver's N-row prefetch pattern (
SQL_ATTR_ROW_ARRAY_SIZE > 1), which eliminates per-rowfetchNext()+memcpyoverhead.After several iterations based on reviewer feedback, the current design introduces two new classes:
Row— a lightweight, non-owning view of a single row's message buffer with typed accessors.RowSet— a disconnected buffer of rows fetched from a Statement's result set.Statementis refactored to useRowinternally, eliminating ~700 lines of duplicated reading/conversion logic.Design
Row — lightweight non-owning view
Rowis a value-type view over a raw message buffer. It holds four pointers:message,descriptors,numericConverter, andcalendarConverter. All typed read logic (isNull,getBool,getInt32,getString,get<T>, etc.) lives inRow.Rowis used in two places:Statementas a privateoutRowmember, pointing to the statement's ownoutMessagebuffer. All ~30 public getters inStatementnow delegate tooutRow. The pointer is stable becauseoutMessageis sized once during construction and never reallocated.RowSet::getRow(i)as a transient view over the i-th row in the fetched buffer.This design satisfies the constraint of eliminating duplicated non-trivial getters while keeping
Rowindependent of bothStatementandRowSetas classes.RowSet — disconnected row buffer
RowSetis a move-only class that:Statement&with an open result set and amaxRowscount. CallsIResultSet::fetchNext()directly into a contiguous internal buffer, up tomaxRowstimes.Statementcan be freed or reused without affecting theRowSet.getRow(index)returns aRowfor typed column reading.getRawRow(index)returns astd::span<const std::byte>for zero-copy scenarios.API
ODBC driver usage (N-row prefetch)
Statement select{attachment, transaction, sql, options}; select.execute(transaction); while (true) { RowSet batch{select, rowArraySize}; if (batch.getCount() == 0) break; for (unsigned i = 0; i < batch.getCount(); ++i) { // Typed access auto row = batch.getRow(i); auto name = row.getString(0); auto age = row.getInt32(1); // Zero-copy raw access for direct ODBC buffer filling auto raw = batch.getRawRow(i); std::memcpy(odbcRow, raw.data(), raw.size()); } }Impact
Statementgetters continue to work identically.Row.RowSeteliminates the per-rowfetchNext()+memcpypattern for the ODBC driver.