fix(capi): preserve error message on unsuccessful QueryResult (#754) - #762
Open
adsharma wants to merge 1 commit into
Open
fix(capi): preserve error message on unsuccessful QueryResult (#754)#762adsharma wants to merge 1 commit into
adsharma wants to merge 1 commit into
Conversation
Set the last-error message on five error paths that previously returned LbugError without populating the error string: - setQueryResult (connection.cpp) - lbug_connection_query (connection.cpp) - lbug_connection_execute (connection.cpp) - lbug_query_result_get_arrow_schema (query_result.cpp) - lbug_query_result_get_next_arrow_chunk (query_result.cpp) Add three tests: - GetLastErrorQueryFailure: query against nonexistent table - GetLastErrorExecuteFailure: execute prepared statement failing at runtime - GetLastErrorArrowSchemaFailure: get Arrow schema from JSON column All 145 existing C API tests continue to pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #754
Several C API entry points returned
LbugErrorwhile leaving the last-error message empty, so bindings (notably the JVM) could only report a generic string. The fix is to populate the last-error message on every error path, matching the convention already used bylbug_connection_get_pushed_sql.Changes (5 sites, all in
src/c_api/):connection.cppsetQueryResult— set last error beforereturn LbugError;on the unsuccessful-result pathconnection.cpplbug_connection_query— sameconnection.cpplbug_connection_execute— samequery_result.cpplbug_query_result_get_arrow_schema— callsetLastCAPIErrorMessage(e.what())in thecatchblockquery_result.cpplbug_query_result_get_next_arrow_chunk— sameTests added (3, all pass):
CApiConnectionTest.GetLastErrorQueryFailure—MATCH (a:NoSuchTable) RETURN areturnsLbugErrorandlbug_get_last_error()carries the binder exception textCApiConnectionTest.GetLastErrorExecuteFailure— same forlbug_connection_executeCApiQueryResultTest.GetLastErrorArrowSchemaFailure—lbug_query_result_get_arrow_schemaon a JSON column reports the underlying exception throughlbug_get_last_error()All 145 existing C API tests continue to pass. The bindings will now surface real diagnostic messages instead of the generic fallback.