Skip to content

Empty Stub Improvements #124

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.

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

Merged
merged 8 commits into from
May 14, 2025

Conversation

maiadegraaf
Copy link
Collaborator

This PR improves the implementation of the empty stub functions to either set an empty response or a diagnostic record.

Empty Results:

  • SQLPrimaryKeys
  • SQLForeignKeys
  • SQLProcedureColumns
  • SQLProcedures
  • SQLColumnPrivileges
  • SQLTablePrivileges
  • SQLSpecialColumns
  • SQLStatistics

Returns Error with diagnostics

  • SQLNativeSql
  • SQLBrowseConnect
  • SQLBulkOperations
  • SQLSetPos

SQLSpecialColumns and SQLStatistics were previously implemented with empty results and moved to statement.cpp but have now been pushed back to empty_stubs.cpp to keep all the functions with empty results together.

@maiadegraaf maiadegraaf requested a review from staticlibs May 7, 2025 08:35
# Conflicts:
#	src/common/odbc_utils.cpp
#	src/empty_stubs.cpp
#	src/statement/statement.cpp
#	test/CMakeLists.txt
@maiadegraaf maiadegraaf force-pushed the empty_stubs_improvements branch from 8ea5fe0 to abc637f Compare May 7, 2025 13:39
Copy link
Collaborator

@staticlibs staticlibs left a comment

Choose a reason for hiding this comment

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

Nice cleanup of this part and great test coverage! I've added a few minor things in inline comments.

}

return SetNotImplemented(hdl, "SQLBrowseConnect");
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add a SQLBrowseConnectW counterpart to this function? It is included in the list of Unicode ODBC functions. We are not planning to implement it, so this is just for consistency.

Copy link
Collaborator Author

@maiadegraaf maiadegraaf May 13, 2025

Choose a reason for hiding this comment

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

Good catch. I've added it as well as a test for it.

@maiadegraaf maiadegraaf force-pushed the empty_stubs_improvements branch from 77d0529 to d7eda0b Compare May 13, 2025 16:37
@maiadegraaf maiadegraaf merged commit 88e79d7 into duckdb:main May 14, 2025
7 checks passed
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.

2 participants