Skip to content
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

error handling (especially for 07001) #92

Open
GitMensch opened this issue Jul 22, 2022 · 10 comments
Open

error handling (especially for 07001) #92

GitMensch opened this issue Jul 22, 2022 · 10 comments

Comments

@GitMensch
Copy link
Contributor

While this should "of course" never happen it did (not for the first time). While the reason this time was a changed interface (the 1.0.17 announcement possibly should have hinted at "you need to re-compile everything that uses prepared statements).

There is a correct sqlstate of 07001 but the error number is -108 and the error message "Generic GixSQL error".

Could this be adjusted to have a reasonable message? Even the internal name of the define would be much better than "generic".

And - not sure on this one - maybe it would be reasonable to use the same return codes as ecpg does?
https://www.postgresql.org/docs/9.2/ecpg-errors.html

-201 (ECPG_TOO_MANY_ARGUMENTS)
This means that the command specified more host variables than the command expected. (SQLSTATE 07001 or 07002)
-202 (ECPG_TOO_FEW_ARGUMENTS)
This means that the command specified fewer host variables than the command expected. (SQLSTATE 07001 or 07002)

Currently there is a return of -108 in both cases (while SQLSTATE is set correctly).

@mridoni
Copy link
Owner

mridoni commented Jul 22, 2022

the 1.0.17 announcement possibly should have hinted at "you need to re-compile everything that uses prepared statements).

You are right. The interface changes were inevitable, but there should have been a warning.

Could this be adjusted to have a reasonable message? Even the internal name of the define would be much better than "generic".

And - not sure on this one - maybe it would be reasonable to use the same return codes as ecpg does? https://www.postgresql.org/docs/9.2/ecpg-errors.html

Will work on that

@mridoni
Copy link
Owner

mridoni commented Jul 29, 2022

The fix for #94, when released, should also fix this. I am afraid that using the same codes as PosgreSQL wouldn't make much sense if using a different DB driver.

@GitMensch
Copy link
Contributor Author

The fix for #94, when released, should also fix this.

Agreed. We may close this one as duplicate then.

I am afraid that using the same codes as PosgreSQL wouldn't make much sense if using a different DB driver.

Agreed. And as the messages come from the gixsql "frontend" library this would leave three choices (apart from "don't care"):

  • document known status differences and leave as-is
  • ask the driver library for the correct status (in this case using a different driver means to either check multiple status values in the COBOL program - likely not relevant for this specific issues in any case...)
  • allow this to be configured, similar to FR: allow mapping of "NoRecCode" #95 (but that's a "very common error" so much more relevant than these two which should never happen)

I think I'd say "document common error codes of GixSQL and - where known their values in other environments)". [it would be good to not have to look at the code to get more info on some numbers]

@mridoni
Copy link
Owner

mridoni commented Jul 29, 2022

I think I'd say "document common error codes of GixSQL and - where known their values in other environments)". [it would be good to not have to look at the code to get more info on some numbers]

I agree.

@mridoni
Copy link
Owner

mridoni commented Jul 30, 2022

A table with the error codes and a brief explanation has been added to the documentation (in the internal repository). I am reproducing it below:


GixSQL-specific error codes

When an error occurs, in the runtime libraries or in the DBMS, GixSQL does its best to return standard-compliant SQLSTATE and SQLCODE error codes and messages. There are a few instances where an operation fails due to "internal" issues (logic errors, consistency checks, unsupported features, possible driver bugs, etc.). In these case GixSQL will use a custom error code and message for SQLCODE and SQLERRM. The table below details the internal error codes that are currently use and a brief explanation for each of them (error messages in SQLERRM are slightly different due to space limitation in the field).

When one of these errors occur and there isn't a self-evident explanation (e.g. your program did not properly initialize a data field used for a prepared statement) you can use the logging system (see above) to try and diagnose the problem.

ID Number Description
DBERR_NO_ERROR 0 No error occurred
DBERR_CONNECTION_FAILED -100 Connection to the database has failed
DBERR_BEGIN_TX_FAILED -101 A transaction could not be started
DBERR_END_TX_FAILED -102 A transaction could not be ended
DBERR_CONN_NOT_FOUND -103 Connection ID not found
DBERR_CONN_RESET_FAILED -104 Connection close failed
DBERR_EMPTY_QUERY -105 Empty query
DBERR_SQL_ERROR -106 Generic SQL/driver error
DBERR_TOO_MANY_ARGUMENTS -107 Too many arguments for a given function
DBERR_TOO_FEW_ARGUMENTS -108 Too few arguments for a given function
DBERR_NO_PARAMETERS -109 Parameters were expected but not supplied
DBERR_CURSOR_EXISTS -110 The cursor already exists
DBERR_NO_SUCH_CURSOR -111 There is no such cursor
DBERR_CLOSE_CURSOR_FAILED -112 Cursor could not be closed
DBERR_DISCONNECT_FAILED -113 Could not disconnect from the DB
DBERR_OUT_OF_MEMORY -114 Out of memory
DBERR_DECLARE_CURSOR_FAILED -115 Cursor declaration failed
DBERR_OPEN_CURSOR_FAILED -116 Cursor could not be opened
DBERR_FETCH_ROW_FAILED -117 Could not fetch a row from the cursor
DBERR_INVALID_COLUMN_DATA -118 Column data is not valid
DBERR_CURSOR_CLOSED -119 Cursor is closed
DBERR_MOVE_TO_FIRST_FAILED -120 Cannot move to first row in a resultset
DBERR_FIELD_COUNT_MISMATCH -121 Result field count does not match with the one in the query
DBERR_NO_DATA -122 No data rows when data rows were expected
DBERR_TOO_MUCH_DATA -123 Received more data rows than expected
DBERR_PREPARE_FAILED -124 Prepare statement failed
DBERR_CONN_INIT_ERROR -201 Connection initialization error
DBERR_CONN_INVALID_DBTYPE -202 Invalid DB type

@mridoni mridoni added the ready label Jul 30, 2022
mridoni added a commit that referenced this issue Aug 15, 2022
This is a maintenance pre-release for GixSQL. It fixes a few issues and adds two new databases drivers (Oracle and SQLite). The next "standard" release (presumably v1.0.18) will have feature parity for all database drivers.

- Added new Oracle driver, based on ODPI
- Added new SQLite driver
- Solution for "PG: issue with prepared statements" (#99)
- Solution for "PCursors cannot be re-opened after close" (#98)
- Solution for "libgixpp: setStatus is called for errors without DBI parm passed - sets SQLERRM" (#94)
- Solution for "error handling (especially for 07001)" (#92)
- Solution for "show-stopper bug in pgsql_prepare" (#91)
- Solution for "PREPARE does not work with VARLENGTH groups (ocesql compat)" (#79)
- Partial solution for "PREPARE does not work with VARLENGTH groups (ocesql compat)" (#68)
- Solution for "The PostgreSQL driver needs START TRANSACTION before using cursors" (#14)
- Solution for "FR: support EXEC SQL VAR" (#21)
- Fixed a bug in "problems with "codegen / logic issue for "GIXSQLCursorDeclareParams" (#88)
- Fixed COMP-3 handling in drivers other than PostgreSQL
- Rewrote the test suite (still MSTest-based) to dynamically generate a matrix of test to be run on the various platforms/database drivers
@GitMensch
Copy link
Contributor Author

GitMensch commented Aug 17, 2022

DBERR_CONN_INIT_ERROR and DBERR_CONN_INVALID_DBTYPE both result in HV000; I think it would be reasonable to give them a separate code, maybe one of IM002, IM003 (Microsoft ODBC docs for driver issues) or one of 08002, 08001. 01S09, HY090, HY110 and may get a different message than "General Error" (maybe: "GixSQL Driver Error")?

@mridoni
Copy link
Owner

mridoni commented Aug 19, 2022

DBERR_CONN_INIT_ERROR and DBERR_CONN_INVALID_DBTYPE both result in HV000;

This is because these two error codes are not handled (as they should) as special cases, so they fall back to HV000 that is the "generic" error. I have already modified this and it will be in v1.0.18.

mridoni added a commit that referenced this issue Aug 23, 2022
- Added new Oracle driver, based on ODPI
- Added new SQLite driver
- All the drivers have been updated and now implement the complete set of supported features
- Solution for "PG: issue with prepared statements" (#99)
- Solution for "PCursors cannot be re-opened after close" (#98)
- Solution for "libgixpp: setStatus is called for errors without DBI parm passed - sets SQLERRM" (#94)
- Solution for "error handling (especially for 07001)" (#92)
- Solution for "show-stopper bug in pgsql_prepare" (#91)
- Solution for "PREPARE does not work with VARLENGTH groups (ocesql compat)" (#79)
- Partial solution for "PREPARE does not work with VARLENGTH groups (ocesql compat)" (#68)
- Solution for "The PostgreSQL driver needs START TRANSACTION before using cursors" (#14)
- Solution for "FR: support EXEC SQL VAR" (#21)
- Fixed a bug in "problems with "codegen / logic issue for "GIXSQLCursorDeclareParams" (#88)
- Solution for "FR: allow mapping of "NoRecCode"' (#95) - added --no-rec-code parameter to gixpp
- Tokens in the parser have been labeled to improve diagnostics (pulled PR #96 by @GitMensch)
- Fixed COMP-3 handling in drivers other than PostgreSQL
- Rewrote the test suite (still MSTest-based) to dynamically generate a matrix of test to be run on the various platforms/database drivers
- Added options for parameter generation in gixpp (-a was removed)
- Added new GIXSQL_FIXUP_PARAMS option for runtime, to automatically convert parameter format in prepared statments
- "Native" cursors are now the default for the PostgreSQL driver
- "Smart" cursor initialization is now the default for all cursors, including those declared in WORKING-STORAGE (-L was removed from gixpp), should fix #101
- Removed dynamic cursor emulation from the ODBC driver when using PostgreSQL
@mridoni mridoni removed the ready label Aug 24, 2022
@GitMensch
Copy link
Contributor Author

According to the note above and the release notes finished.

@GitMensch
Copy link
Contributor Author

Not sure, but this change possibly created a new issue (or it was in since the initial implementation).

  • _pgsql_exec_params(), which is called by GIXSQLCursorOpen via dbi->cursor_open may result in an error
  • the result state including the last error is resolved from the PGResultSetData and stored in the db interface
  • it is used for error logging
  • if there is *no error, then the PGResultSetData would also be stored, either in the cursor (was be in my debugged case) or in the db interface's "current_resultset_data"
  • after calling dbi->cursor_open, GIXSQLCursorOpen issues a set_status via FAIL_ON_ERROR
  • to get the sql message set_status does, because it has a dbi object this time a call to DbInterfacePGSQL::get_error_message()
  • DbInterfacePGSQL::get_error_message() sees a current_resultset_data and gets the error from there

... but as followed above - this was never set, because there was an error...

Current issues:

  • the error handling does not take into account that there is a cursor which has the resultset data
  • upon errors: either the current_resultset_data should be always set or it should be freed, if the rest of the logic should stay a is
  • maybe always read the last error from the last_error in dbi->get_error_message()? That would be in par with get_error_code and get_state - and may make a bunch of calls to resolve the error message unnecessary.

@GitMensch GitMensch reopened this Aug 29, 2022
@mridoni
Copy link
Owner

mridoni commented Aug 29, 2022

Ok, I will check this

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

No branches or pull requests

2 participants