mridoni / gixsql

GixSQL is an ESQL preprocessor and a series of runtime libraries to enable GnuCOBOL to access PostgreSQL, ODBC, MySQL, Oracle and SQLite databases.
GNU General Public License v3.0
16 stars 8 forks source link

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

Closed GitMensch closed 2 years ago

GitMensch commented 2 years ago

I've recognized this in GIXSQLExecSelectIntoOne https://github.com/mridoni/gixsql/blob/081674b3c03e1b97b0a2edf854a152d765331bee/runtime/libgixsql/gixsql.cpp#L943 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 commented 2 years ago

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: https://github.com/mridoni/gixsql/blob/081674b3c03e1b97b0a2edf854a152d765331bee/runtime/libgixsql/gixsql.cpp#L907-L911

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.

GitMensch commented 2 years ago

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 commented 2 years ago

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

mridoni commented 2 years ago

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.

GitMensch commented 2 years ago

seems to be fine in 1.0.18dev2