[freetds] SQLMoreResults() and dbresults() sometimesreturnextraphantom resultset

ZIGLIO, Frediano, VF-IT Frediano.Ziglio at vodafone.com
Fri Jul 7 11:19:07 EDT 2006


 

> > From: ZIGLIO, Frediano, VF-IT
> > Sent: Thursday, July 06, 2006 3:15 AM
> > 
> > Another question is... 0.64 behave correctly?
> 
> Sadly, no.  I relinked the dblib rpc.c to the 0.64 libsybdb.  It fails
> the same way.  
> 
> > > SQLMoreResults() and dbresults() both sometimes return SUCCEED
> > 
> > Don't mix SQLMoreResults and dbresults, they are quite different
> 
> Maybe they should be less different?  
> 
> 	"dbresults Sets up the results of the next query."
> 	"SQLMoreResults determines whether more results are available
> ... and, if so, initializes processing for those results."
> 
> Both functions set up the metadata and return SUCCESS if 
> there are rows
> to be read.  
> 

What I was trying to say is that they have very different implementation
so is hard to compare them in these situations.

> > Note that all that questions are very longstanding with lot of
> > small fixes but, it seems, not real fix... the problem is that
> > libraries have sligtly different behavior that should be tested
> > with care... 
> 
> That's one problem, surely.  But I think the real problem is 
> more basic,
> and I wonder if you agree.  
> 
> I don't understand why there are two main processing loops in libtds:
> 
> 	tds_process_default_tokens
> and	tds_process_tokens
> 
> These functions are very redundant.  As just one example,
> TDS_PARAM_TOKEN is handled in both.  
> 

Well... tds_process_default_tokens is static while tds_process_tokens
not... tds_process_default_tokens is used internally to tokens when
other functions don't know what to do. Yes, there are some duplications
that should be removed...

> IMHO there should be only *one* place in libtds that reads a 
> token.  It
> should read the token ("marker") and, for packets that have them, the
> length.  Then it should call the appropriate process_* 
> function, passing
> the token and length.  Each process_* function would read 
> only 'length'
> bytes from the stream and return; it would *never* read another token.
> If it fails to read 'length' bytes, the single read loop would be the
> place to handle that error and inform the caller.  
> 

I think this has to do only with token.c file and nothing with done
processing. I agree that each token should have a procedure that read
the token. I don't know where is the best place to read length. Reading
it from caller means that caller read length just before calling token
handler (which knows exactly length size...).

> The token reader ("tds_read_token()") would be passed an "intention",
> something to indicate what the caller expects.  Like a good librarian,
> It would read as many tokens as it could looking for the requested
> packet.  Examples:
> 
> expecting login token, got  e3(ENVCHANGE)
> expecting login token, got  ab(INFO)
> expecting login token, got  ad(LOGINACK)	<-- expected
> expecting login token, got  fd(DONE)
> 

Mmm... so here you pass "I want loginack" and tds_read_token stop at
loginack skipping (or handling) any tokens inside. And if I want a row
should it skip all DONE/PARAMS tokens?? I don't think this way it too
much flexible... Currently tokens are classified and you can decide to
handle/stop before/stop after for each class. Complicated but more
flexible.

> 	and
> 
> expecting row token, got d1(ROW)	<-- expected
> expecting row token, got a8(ALTFMT)
> expecting row token, got d3(ALTROW)
> expecting row token, got 79(RETURNSTATUS)
> expecting row token, got ac(PARAM)
> expecting row token, got fe(DONEPROC)
> 
> All libraries have a "get next row" function.  Always, the 
> last row can
> be followed in the stream by output parameters and/or return status.
> Always, the application may ignore compute rows and other 
> "not regular"
> results.  When reading a row, the token reader should not 
> stop until it
> gets a ROW or DONE/DONEPROC.  (And the results should not be 
> freed until
> the next metadata arrive or next packet is sent.)  
> 

I agree libTDS should not free results before next query however you
have to consider library states. For instance after getting last row it
should be allowed to read row number and at the same time under odbc
creating another statement and issue a new query while first statement
still "contains" row number and metadata informations. And by "at the
same time" I mean even another thread running in another processor...
The problem here is that libTDS store number of row (and row
informations) in TDSSOCKET, set connection status to IDLE and than
return. I think that libTDS should handle server state update so it
should return number of rows like it does with done status... 

> I know you've been working on this "read ahead" idea; I think you
> understood the problem earlier.  But I now want to simplify things by:
> 
> 1.  One read loop.
> 2.  Read all compute rows at one time, too, and keep list of 
> structures
> in TDSRESULTINFO.  
> 3.  Free results as late as possible.  
> 

1. I don't know however how to make all libraries happy...
2. Does by "compute rows" you mean "compute rows" or even "normal" rows
??
3. if libTDS receive another query is has to free results.

> libtds began as a set of service routines to support db-lib.  
> db-lib was
> the "driver": it made 
> requests of libtds, which handled the low-level details.  
> libtds was the
> passive servant.  We have to make it more active, to read as far ahead
> as possible.  
> 

Mmm... but any library handle data in a different way...

One problem with ODBC is that charset support is very poor.. I think a
day I'll set charset fixed to iso8859-1. This cause iODBC and unixODBC
make this supposition... However ODBC supports wide characters so I
think one day it will have to store characters in a different way..

> We should also create a "public" libtds API, functions the client
> libraries will call.  I think we can identify a set of functions that
> all libraries need e.g.:
> 
> 	connect/disconnect
> 	"ioctl" (get/set options)
> 	install callbacks
> 	status
> 	execute/cancel
> 	rpc
> 	read metadata
> 	read row
> 	bcp
> 
> Not too much more, eh?  I could work on a document showing the related
> client API functions.  
> 
> Do you think this idea is:
> 
> 1.  feasible?
> 2.  good?
> 3.  good to do now?
> 

I think is a very big idea... that is good, feasible but very
complicated and far from fixing done processing... your change is... a
lot of change...
- token handling, separate each token handling with different function
- token processing, put more processing on libTDS
- library change, change all libTDS interface
But if you are able to work on a document you already think enough :)

> > I think that dblib is the former library so probably
> > fixing it will help to understand the real behavior. 
> 
> Agreed. 
> 

I'll try to improve done_handling output.. 

> > I think I'll start trying all dblib tests under windows
> 
> I think that will be useful going forward.  Microsoft promises its
> version will be very stable (they stopped distributing it).  But of
> course their db-lib is 4.2.  
> 

Mmmm... I have to say that ms dblib is not that happy :(

t0001 ok
t0002 ko
t0003 ok
t0004 ok
t0005 ko
t0006 ok
t0007 ok
t0008 ko
t0009 ok
t0011 ok
t0012 ok
t0013 ko
t0014 ok
t0015 ok
t0016 ko
t0017 ko
t0018 ok
t0019 ko
t0020 ok
t0021 ko (sybase extension)
t0022 ok
t0023 ok
rpc ko
dbmorecmds ok
bcp ko
thread ok (pthread)
text_buffer ko
done_handling ko

But I also think that many are related to done processing (and It seems
I did something wrong with bcp... perhaps ms dblib don't like unix files
:) ).

freddy77




More information about the FreeTDS mailing list