Improvement #8895 : 'invalid request BLR' puts actual error (and procedure where it happened) at the end where it is truncated by ISC API

[hvlad]

Revert order of entires in status-vector so that original error is put on the top, not at the bottom

[dyemanov]

Could you please show the error stack before and after the fix? I'm afraid this gonna break all programmer checks like status[1] == isc_bad_proc_BLR to check whether BLR is known to be invalid.

[hvlad]

Could you please show the error stack before and after the fix?

Before:

SQL> select fn_test3() from rdb$database;
Statement failed, SQLSTATE = 2F000
Error while parsing function FN_TEST3's BLR
-Error while parsing function FN_TEST2's BLR
-Error while parsing function FN_TEST1's BLR
-invalid request BLR at offset 32
-function UDF_SRAND is not defined
-module name or entrypoint could not be found

After:

SQL> select fn_test3() from rdb$database;
Statement failed, SQLSTATE = 39000
invalid request BLR at offset 32
-function UDF_SRAND is not defined
-module name or entrypoint could not be found
-Error while parsing function FN_TEST1's BLR
-Error while parsing function FN_TEST2's BLR
-Error while parsing function FN_TEST3's BLR

Also, note, now this order is the same as in execution error:

SQL> select fn_test3() from rdb$database;

    FN_TEST3
============
Statement failed, SQLSTATE = 39000
function UDF_SRAND is not defined
-module name or entrypoint could not be found
-At function 'FN_TEST1' line: 5, col: 3
At function 'FN_TEST2' line: 5, col: 3
At function 'FN_TEST3' line: 5, col: 3

I'm afraid this gonna break all programmer checks like status[1] == isc_bad_proc_BLR to check whether BLR is known to be invalid.

I would be very surprized if someone look for error code in status[1] only.

[dyemanov]

I would be very surprized if someone look for error code in status[1] only.

It's a common practice in the Firebird codebase :) In RDB, we check for isc_bad_proc_BLR and its friends in gbak in order to restore databases with invalid BLR. Of course, this check can be improved to search through the whole status vector, but it would look inconsistent with another error handlers across the codebase. With the new code, perhaps we could rely on isc_invalid_blr instead, but old checks are still needed for backward compatibility. Sigh.