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

libgixpp: setStatus is called for errors without DBI parm passed - sets SQLERRM #94

Closed
GitMensch opened this issue Jul 26, 2022 · 5 comments
Labels
bug Something isn't working ready

Comments

@GitMensch
Copy link
Contributor

GitMensch commented Jul 26, 2022

I've recognized this in GIXSQLExecSelectIntoOne

setStatus(st, NULL, DBERR_NO_DATA);
but there are lots of other calls of setStatus that pass NULL - even with a valid dbi available - and in many other cases I do wonder if it shouldn't be requested from the valid conn we often have.

Because of this missing call the return is not:

01 SQLCA
  05 SQLCAID : "SQLCA   "
  05 SQLCABC : +0000000136
  05 SQLCODE : +0000000100
  05 SQLERRM
    49 SQLERRML : +00000
    49 SQLERRMC : ' ' <repeats 70 times>
  05 SQLERRP : "        "
  05 SQLERRD (1) :
  05 SQLWARN
    10 SQLWARN0 : " "
    10 SQLWARN1 : " "
    10 SQLWARN2 : " "
    10 SQLWARN3 : " "
    10 SQLWARN4 : " "
    10 SQLWARN5 : " "
    10 SQLWARN6 : " "
    10 SQLWARN7 : " "
  05 SQLSTATE : "02000"

but

01 SQLCA
  05 SQLCAID : "SQLCA   "
  05 SQLCABC : +0000000136
  05 SQLCODE : +0000000100
  05 SQLERRM
    49 SQLERRML : +00027
    49 SQLERRMC : "-122 : Generic GIXSQL error\0o0", ' ' <repeats 42 times>
  05 SQLERRP : "        "
  05 SQLERRD (1) :
  05 SQLWARN
    10 SQLWARN0 : " "
    10 SQLWARN1 : " "
    10 SQLWARN2 : " "
    10 SQLWARN3 : " "
    10 SQLWARN4 : " "
    10 SQLWARN5 : " "
    10 SQLWARN6 : " "
    10 SQLWARN7 : " "
  05 SQLSTATE : "02000"

Because of performance reasons (not sure if this makes an actual difference) you may consider that for a bunch of known return status (like the 02000 above) you directly set SQLERRM to 0+space, but that's your's to consider :-)

@mridoni
Copy link
Owner

mridoni commented Jul 28, 2022

In some of these cases (notably not the one above) the error does not originate in the driver/DBMS (which is never hit) but by some logic/consistency issue. For instance:

if (_query == NULL || strlen(_query) == 0) {
spdlog::error("Empty query/cursor");
setStatus(st, NULL, DBERR_EMPTY_QUERY);
return RESULT_FAILED;
}

The logic is (bugs like in your case aside) that if dbi is NULL, we do not try to retrieve an error code/message from the DB driver, because it would be useless, we just build one from the supplied error code (e.g. DBERR_EMPTY_QUERY). Unfortunately, in many of these cases SQLERRM is simply not set.

@mridoni mridoni added the bug Something isn't working label Jul 28, 2022
@GitMensch
Copy link
Contributor Author

I totally agree - this has likely to be checked for each invocation - and possibly a comment be left in the code that explains the logic (more or less copy+paste +delete from your comment above).

@GitMensch
Copy link
Contributor Author

Another place to definitely pass dbi: in line 960 - error for prepare (actually any place that uses DBERR_SQL_ERRROR.

@mridoni
Copy link
Owner

mridoni commented Jul 29, 2022

Another place to definitely pass dbi: in line 960 - error for prepare (actually any place that uses DBERR_SQL_ERRROR.

Noted (and done). Also: SQLERRM - in my internal repository - should now be correctly set for "internal" error codes (those emitted without a corresponding error from the DBMS/driver), I want make another "pass" to be sure that all error messages are covered.

@mridoni mridoni added the ready label Aug 2, 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

seems to be fine in 1.0.18dev2

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ready
Projects
None yet
Development

No branches or pull requests

2 participants