Skip to content

Conversation

@alinaliBQ
Copy link
Contributor

@alinaliBQ alinaliBQ commented Nov 4, 2025

Rationale for this change

Add tests for SQL_HANDLE_DESC descriptor handle allocation, and diagnostics test

What changes are included in this PR?

  • tests for SQL_HANDLE_DESC
  • We use error associated returned from Driver manager to ensure users can get errors from descriptor handle

Are these changes tested?

Tested on local MSVC Windows

Are there any user-facing changes?

N/A

@github-actions
Copy link

github-actions bot commented Nov 4, 2025

⚠️ GitHub issue #47707 has been automatically assigned in GitHub to PR creator.

// GH-47706 TODO: Add tests for SQLAllocStmt, pre-requisite requires
// SQLDriverConnect implementation

// GH-47707 TODO: Add tests for SQL_HANDLE_DESC implementation for
Copy link
Contributor Author

@alinaliBQ alinaliBQ Nov 10, 2025

Choose a reason for hiding this comment

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

TODO - add /brief doxygen doc for RevertAppDescriptor in odbc_statement.h

@github-actions github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Nov 10, 2025
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This PR needs to be rebased onto main after PR #47764 is merged

@alinaliBQ alinaliBQ force-pushed the gh-47707-descriptor-handle branch from 65a1708 to 5041f73 Compare November 11, 2025 00:05
@alinaliBQ alinaliBQ changed the title GH-47707: [C++][FlightRPC] WIP: Add tests for descriptor handle allocation GH-47707: [C++][FlightRPC] Add tests for descriptor handle allocation Nov 11, 2025
Copy link
Contributor Author

@alinaliBQ alinaliBQ Nov 11, 2025

Choose a reason for hiding this comment

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

Similar here, would need to rebase after #47762 merged to get rid of duplicate changes, as this PR depends on it

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