Skip to content

refactor: replace iterator with cursor-based HasNext/Next/Get API#543

Open
liulx20 wants to merge 8 commits into
alibaba:mainfrom
liulx20:refactor/query-result-cursor-api
Open

refactor: replace iterator with cursor-based HasNext/Next/Get API#543
liulx20 wants to merge 8 commits into
alibaba:mainfrom
liulx20:refactor/query-result-cursor-api

Conversation

@liulx20

@liulx20 liulx20 commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Fixes

liulx20 added 5 commits June 12, 2026 10:27
…t/Get API

- Remove RowView class, const_iterator, begin()/end()/cbegin()/cend()
- Add cursor-based traversal: HasNext(), Next(), Reset(), CurrentRowIndex()
- Add typed value accessors using current cursor row: GetInt32(), GetUInt32(),
  GetInt64(), GetUInt64(), GetFloat(), GetDouble(), GetString(), GetBool()
- Add IsNull() and GetValueAsString() for null-checking and generic access
- Add ColumnCount() and ColumnNames() metadata helpers
- Update documentation to reflect new API and usage examples
- All existing interfaces (length, response, Serialize, etc.) preserved
- index.md: replace range-for with HasNext/Next loop
- connection.md: replace range-for with HasNext/Next loop (2 occurrences)
- GetInt64: accepts Int32Array, UInt32Array, BoolArray
- GetUInt64: accepts UInt32Array, BoolArray
- GetFloat: accepts Int32Array, UInt32Array, BoolArray
- GetDouble: accepts all numeric types and BoolArray
- GetString: falls back to string representation for any type
- GetInt32/GetUInt32: accept BoolArray
- No narrowing conversions allowed (e.g. int64 → int32 still throws)
- CursorTraversal: HasNext/Next/Reset/CurrentRowIndex, throw on past-end
- TypedGetters: GetInt32/GetString/GetDouble/GetBool per row
- ImplicitWidening: int32→int64, int32→double, int32→float, bool→int32
- GetStringFallback: non-string columns return string representation
- NarrowingConversionThrows: double→int32, string→int64 throw
- IsNull: validity bitmap null detection
- GetValueAsString: generic string access
- SerializeAndDeserialize: round-trip test
- EmptyResult: zero-row edge case
- ColumnIndexOutOfRange: bounds check
…s all types

- Remove GetValueAsString from header, implementation, docs, and tests
- GetString with fallback to string representation covers the same use case
- Update all doc examples from GetValueAsString to GetString
liulx20 added 3 commits June 12, 2026 10:41
- Add GetColumnIndex(name) private helper for name→index lookup
- Add string overloads: GetInt32(name), GetString(name), IsNull(name), etc.
- All name-based overloads delegate to index-based versions
- Throws on unknown column name with clear error message
- Add tests: GetByColumnName, widening by name, IsNull by name, invalid name
@liulx20 liulx20 requested a review from zhanglei1949 June 12, 2026 06:34
std::cout << record.ToString() << std::endl;
auto& qr = result.value();
while (qr.HasNext()) {
std::cout << qr.GetString(0) << std::endl;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

qr.ToString()?

std::cout << record.ToString() << std::endl;
auto& qr = result.value();
while (qr.HasNext()) {
std::cout << qr.GetString(0) << std::endl;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same as above?

bool IsNull(size_t column_index) const;
bool IsNull(const std::string& column_name) const;

int32_t GetInt32(size_t column_index) const;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What about Date/DateTime/TimeStamp

Comment thread src/main/query_result.cc
}
}

double QueryResult::GetDouble(size_t column_index) const {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we or not allow implicit type case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor QueryResult: replace iterator pattern with cursor-based HasNext/Next/Get API

2 participants