FreeTDS Development Group

["ZIGLIO, Frediano, VF-IT" <Frediano.Ziglio AT vodafone.com>]

> 
> Error handling has been a longstanding weakness in FreeTDS, 
> partly because
> it's complex and partly because Sybase made it complex.  I'm speaking
> mostly of dberrhandle().  It's hard enough to install a 
> handler, a little
> harder to return the right value, and very hard to interpret 
> that value
> correctly.  I'm pretty sure when Nick worked on it, he found 
> that Sybase
> didn't adhere to its own documentation. 
> 

Never noted it was so weak...

...
> 
> Inter-library
> -------------
> 
> Sometimes libtds has to call back to the client's installed 
> handler.  We
> need very clear semantics between libtds and the client API, 
> because the
> handler may be written according to several specifications, 
> depending on
> API and vendor.  We don't have them.  
> 
> Inbound, the error numbers that libtds emits are db-lib based, and the
> parameters include some ct-lib (I think) state.  ct-lib's 
> error numbering
> scheme is much saner and thus inconsistent with db-lib.  My 
> guess is ODBC
> uses another set altogether.  My idea is to have libtds 
> standardize on the
> ct-lib numbers, and let the others map to that.  
> 

ODBC support both native errors and "sql" errors. Native errors are
errors specific to database (MySql, SQL Server or whatever) so it use
error code provided by tds_client_msg. sql errors (also called sql
state) are an iso standard used by ODBC itself and also ctlib. I don't
know if ctlib use same errors used by dblib and ODBC but I think so...

> Outbound, tds_client_msg() gets back a db-lib SUCCEED or FAIL, which
> thankfully map to CS_SUCCEED and CS_FAIL, and their TDS 
> bretheren.  But
> woe to he who would like his handler to insist on re-trying a timeout:
> 
>       case INT_TIMEOUT:
>               /* XXX do something clever */
>               return SUCCEED;
> 
> If we're ever going to get timeouts to work according to 
> spec, we're going
> to have to let the handler have its say.  We need to synthesize the
> meanings of all the handler conventions to convey clear 
> information back
> to libtds.  
> 

Mmmm... perhaps we should call tds_cancel/exit inside error handler...
only dblib require INT_CANCEL/INT_EXIT.

> (We've talked about this before.  The reason I'm interested 
> now is that
> we've done enough work on timeouts that error handling has 
> become the weak
> link in the chain.)  
> 
> We can keep tds_client_msg(), but we can't have it call
> _dblib_handle_err_message(), because _dblib_handle_err_message(): 
> 
> 1.  expects db-lib message numbers, where libtds should use 
> libtds error
> numbers.
> 2.  returns db-lib codes, because it's a db-lib function widely called
> from withing db-lib.  It can't know it's been called from libtds.  
> 
> For errors that originate in libtds, we'll have 
> tds_client_msg() call a
> new db-lib function, maybe "_dblib_handle_tds_message()".  It 
> will convert
> the msgno, call _dblib_err(), and convert the return code to 
> fit libtds's
> specification.  
> 
> The good news is that all of this can be done within db-lib, 
> initially.
> Once the new functions have been written, we can convert the 
> libtds error
> message numbers and begin calling the new db-lib functions.  
> 
> I skipped a step, of course.  libtds has 15 calls like this:
> 
> tds_client_msg(tds->tds_ctx, tds, 20014, 9, 0, 0, "Login incorrect.");
> 
> It's wrong to map that text to that number, and it's wrong to 
> send that
> number to a ct-lib or ODBC handler.  What we need instead:
> 

As said ODBC require native errors and I think 20014 is the right
number.

>       tds_client_msg(tds, routine & layer & origin);
> 
> And make sure _dblib_handle_err_message() can deal.  
> Fortunately, 15 isn't
> that many to do.  
> 

I noted you remove TDSCONTEXT* parameter.

$ grep tds_client_msg `find -name \*.c`
./apps/tsql.c:                          tds_client_msg(tds->tds_ctx,
tds, line, line, line, line, message);
./apps/tsql.c:                  tds_client_msg(tds->tds_ctx, tds, 1, 1,
1, 1, message);
./dblib/dblib.c: * \return Propogates return code of tds_client_msg().
./dblib/dblib.c: * \sa tds_client_msg().
./dblib/dblib.c:        return tds_client_msg(g_dblib_ctx.tds_ctx, tds,
dberr, severity, -1, -1, dberrstr);
./dblib/dbutil.c:               tds_client_msg(tds_ctx, tds, SYBESMSG,
EXSERVER, -1, -1,
./tds/iconv.c:                          tds_client_msg(tds->tds_ctx,
tds, 2404, 16, 0, 0,
./tds/iconv.c:                          tds_client_msg(tds->tds_ctx,
tds, 2403, 16, 0, 0,
./tds/iconv.c:                  tds_client_msg(tds->tds_ctx, tds, 2402,
16, 0, 0,
./tds/iconv.c:          tds_client_msg(tds->tds_ctx, tds, 2401, 16,
*inbytesleft, 0,
./tds/iconv.c:          tds_client_msg(tds->tds_ctx, tds, 2400, 16,
*inbytesleft, 0,
./tds/login.c:          tds_client_msg(tds->tds_ctx, tds, 20014, 9, 0,
0, "Login incorrect.");
./tds/net.c:            tds_client_msg(tds->tds_ctx, tds, 20009, 9, 0,
0, "Server is unavailable or does not exist.");
./tds/net.c:                    tds_client_msg(tds->tds_ctx, tds, 20004,
9, 0, 0, "Read from SQL server failed.");
./tds/net.c:                    tds_client_msg(tds->tds_ctx, tds, 20006,
9, 0, 0, "Write to SQL Server failed.");
./tds/token.c:          tds_client_msg(tds->tds_ctx, tds, 20020, 9, 0,
0, "Unknown marker");
./tds/token.c: * tds_client_msg() sends a message to the client
application from the CLI or
./tds/token.c:tds_client_msg(const TDSCONTEXT * tds_ctx, TDSSOCKET *
tds, int msgnum, int level, int state, int line, const char *msg_text)
./tds/token.c:  tdsdump_log(TDS_DBG_FUNC, "tds_client_msg: #%d: \"%s\".
Connection state is now %d.  \n", msgnum, msg_text, tds ?
(int)tds->state : -1);
./tds/util.c:                   tds_client_msg(tds->tds_ctx, tds, 20006,
9, 0, 0, "Write to SQL Server failed.");
./tds/util.c:                   tds_client_msg(tds->tds_ctx, tds, 20019,
7, 0, 1,

>From dblib.c

int
_dblib_client_msg(DBPROCESS * dbproc, int dberr, int severity, const
char *dberrstr)
{
        TDSSOCKET *tds = NULL;

        if (dbproc)
                tds = dbproc->tds_socket;
        return tds_client_msg(g_dblib_ctx.tds_ctx, tds, dberr, severity,
-1, -1, dberrstr);
}

$ grep _dblib_client_msg `find -name \*.c`
./dblib/bcp.c:                  erc = _dblib_client_msg(dbproc,
bcp_errno, EXRESOURCE, p);
./dblib/bcp.c:  return _dblib_client_msg(dbproc, bcp_errno, severity,
errmsg);
./dblib/dblib.c:                _dblib_client_msg(NULL, SYBEMEM,
EXRESOURCE, "Unable to allocate sufficient memory.");
./dblib/dblib.c:                _dblib_client_msg(NULL, SYBEMEM,
EXRESOURCE, "Unable to allocate sufficient memory.");
./dblib/dblib.c:                _dblib_client_msg(NULL, SYBEMEM,
EXRESOURCE, "Unable to allocate sufficient memory.");
./dblib/dblib.c:                _dblib_client_msg(NULL, SYBEMEM,
EXRESOURCE, "Unable to allocate sufficient memory.");
...

So it's wrong to remove TDSCONTEXT* parameter, used for error without a
connection.

> I haven't looked at the ct-lib side.  It will be affected to, 
> at least for
> those 15 calls.  But, since we're walking libtds in ct-lib's 
> direction,
> the impact creator should be smaller.  
> 
> Lastly, I have a question about TDSMESSAGE::priv_msg_type.  
> It's written
> to 5 times (once in mem.c, 4 in token.c).  It's never read.  
> Anyone have
> any idea what it's meant to do?  Also, I'd like to 

It useful to distingue from information to error but it's not used
(level is used instead)

> s/msg_level/severity/
> and s/msg_number/msgno/ and s/msg_state/state/ if there are 
> no objections.

I agree

>  Those are the terms used in the docs.  
> 
> typedef struct tds_message
> {
>       TDS_SMALLINT priv_msg_type;
>       TDS_SMALLINT line_number;
>       TDS_UINT msg_number;
>       TDS_SMALLINT msg_state;
>       TDS_SMALLINT msg_level;
>       TDS_CHAR *server;
>       TDS_CHAR *message;
>       TDS_CHAR *proc_name;
>       TDS_CHAR *sql_state;
> } TDSMESSAGE;
> 
> Here's to better errors.  
> 

bye
  freddy77