Skip to content

Odbc static cursors - #660

Open
Shfdis wants to merge 11 commits into
odbc-driver-featurefrom
odbc-static-cursors
Open

Odbc static cursors#660
Shfdis wants to merge 11 commits into
odbc-driver-featurefrom
odbc-static-cursors

Conversation

@Shfdis

@Shfdis Shfdis commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

ODBC integration report

Open the interactive Allure report · 📦 Download the report bundle

Commit: 2b4b30149135cc3db9349a7a34f93af39b96f60c; result: success; updated by each run.

github-actions Bot added a commit that referenced this pull request Aug 21, 2026
github-actions Bot added a commit that referenced this pull request Aug 21, 2026
github-actions Bot added a commit that referenced this pull request Aug 24, 2026
github-actions Bot added a commit that referenced this pull request Aug 24, 2026
github-actions Bot added a commit that referenced this pull request Aug 24, 2026
@Shfdis

Shfdis commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 17df722cad

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread odbc/src/utils/cursor.cpp Outdated
Comment thread odbc/src/metadata.cpp
github-actions Bot added a commit that referenced this pull request Aug 24, 2026

@robot-vibe-db robot-vibe-db Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AI Review Summary

Verdict: ✅ No critical issues found

The cursor window implementation correctly follows the ODBC spec for all fetch orientations (NEXT, PRIOR, FIRST, LAST, ABSOLUTE, RELATIVE), including the overlapped-start SQLSTATE 01S06. The NegativeMagnitude helper correctly handles SQLLEN_MIN without signed overflow. The PrimitiveText refactoring in convert.cpp is safe — both old and new paths ultimately call WriteText for string types, so the only change is avoiding a string copy. The FillBoundColumns error-handling change (removing the LastFetchRc_ == SQL_SUCCESS guard) is an improvement: the old code could leave LastConvertSqlState unconsumed when a second column failed after the first, leaking state to the next GetData call.

Critical issues

No critical issues found.

Other findings

  • Minor | Medium: TStaticCursor::EnsureRow throws generic std::runtime_error through the ODBC API boundary — odbc/src/utils/cursor.cpp:133
  • Minor | Low: TForwardCursor bypasses the inherited TCursorWindow entirely (overrides Fetch/GetRowNumber without delegation), leaving the base-class window at totalRows=0odbc/src/utils/cursor.cpp:55
  • Nit | Low: PrimitiveText returns std::string_view into parser-owned state; lifetime is correct in all current call sites but the contract is implicit — odbc/src/utils/convert.cpp:542

This review was generated automatically. Critical issues require attention; other findings are advisory.
If this comment was useful, please give it a 👍 — it helps us improve the review bot.

Comment thread odbc/src/utils/cursor.cpp Outdated
Comment thread odbc/src/utils/cursor.cpp
Comment thread odbc/src/utils/convert.cpp
@robot-vibe-db

robot-vibe-db Bot commented Aug 24, 2026

Copy link
Copy Markdown

Full analysis log

Analysis performed by claude, claude-opus-4-6.

github-actions Bot added a commit that referenced this pull request Aug 24, 2026
github-actions Bot added a commit that referenced this pull request Aug 24, 2026
@Shfdis

Shfdis commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 70fa75ebb2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread odbc/src/statement.cpp
Comment thread odbc/src/statement.cpp Outdated
github-actions Bot added a commit that referenced this pull request Aug 24, 2026

@robot-vibe-db robot-vibe-db Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AI Review Summary

Verdict: ✅ No critical issues found

Critical issues

No critical issues found.

Other findings

  • Nit | High: SQLSTATE inconsistency — setCursorType uses HY011 but SQL_ATTR_CONCURRENCY uses 24000 for the same "cursor is open" guard — odbc/src/statement.cpp:875 / odbc/src/statement.cpp:910

This review was generated automatically. Critical issues require attention; other findings are advisory.
If this comment was useful, please give it a 👍 — it helps us improve the review bot.

Comment thread odbc/src/statement.cpp
@robot-vibe-db

robot-vibe-db Bot commented Aug 24, 2026

Copy link
Copy Markdown

Full analysis log

Analysis performed by claude, claude-opus-4-6.

github-actions Bot added a commit that referenced this pull request Aug 24, 2026
github-actions Bot added a commit that referenced this pull request Aug 24, 2026
@Shfdis

Shfdis commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ca764629e5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread odbc/src/utils/cursor.cpp Outdated
github-actions Bot added a commit that referenced this pull request Aug 24, 2026
@Shfdis

Shfdis commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2b4b301491

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread odbc/src/utils/cursor.cpp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant