FreeTDS Development Group

["Thompson, Bill D (London)" <ThompBil AT exchange.uk.ml.com>]

Thanks James,

> I'd still like to know:  What did you bump into that led you to add the
> discard loop to dbcancel()?  And why shouldn't tds_process_cancel() be
> changed accordingly?  Aren't the other APIs similarly affected ?

Now that I'm asked the question, I am forced into the conclusion that I was
probably guilty of a "scattergun" approach  to solving a number of problems
in a short period of time. Given the (very convincing) arguments submitted
by the group, I'm now convinced that this particular fix is unnecessary. 

I'll get rid of this in the next couple of days, with a successor patch. I
also hope to reach a more elegant solution to the "incompletely processed
result set" issue that I mentioned earlier today.

There is no doubt that the peculiarities of db-library behaviour are very
difficult to marry up with the TDS streams. I agree with your observations
about ct-library and the protocol. It was working with ct-library that
convinced me to change the tds_process_row_tokens and
tds_process_result_tokens functions last year.

Thanks for all the sound criticisms/advice everybody,

Bill

> -----Original Message-----
> From: Lowden, James K [SMTP:LowdenJK AT bernstein.com]
> Sent: Wednesday, March 05, 2003 6:17 PM
> To:   'FreeTDS Development Group'
> Subject:      RE: [freetds] Comment on change
> 
> > From: Thompson, Bill D (London) [mailto:ThompBil AT exchange.uk.ml.com]
> > Sent: March 5, 2003 11:24 AM
> > 
> > > Your version is highly dangerous IMO.  Safer would have been to call
> > > dbcanquery() before tds_send_cancel().  That would have 
> > > discarded genuine
> > > rows, not just anything the server happened to send.  But 
> > > that begs the
> > > question of why tds_send_cancel() isn't good enough.  
> > 
> > except the sybase dblibrary manual says that is exactly what dbcancel
> > does...
> 
> I think you misunderstand a subtle distinction, my friend.  *Any* rows is
> not *all* rows.  After we send the cancel token, we may continue to
> receive
> data from the server.  Such data were "in the pipeline" at the server when
> it received our cancel; the server flushes its buffers (sending us some
> rows, even a partial one) and then acknowledges our cancel request.  
> 
> The results will definitely be truncated unless by chance they were almost
> completely sent at the time the cancel was issued.  With Sybase's library,
> if your result set is 1,000,000 rows and you call dbcancel() on row 1000,
> I
> guarantee the server won't send the other 999,000 rows before
> acknowledging
> the cancel token.  
> 
> The logic should be:
>       send the cancel token
>       read server packets
>               if data, discard
>               if other, honor
>       until cancel acknowledged
> 
> And that's what the old dbcancel() did, or at least was meant to do.  
> 
> Another note on the subject.  Sybase says in Chapter 1:
> 
> If the application calls dbcancel
> while processing the titles rows, then DB-Library discards the titles rows
> and
> the results of all remaining, unprocessed commands in the batch. The
> application does not need to continue calling dbresults after calling
> dbcancel.  
> 
> I don't know that we have to maintain that distinction.  Seems to me an
> application using FreeTDS would be just as happy if, having called
> dbcancel(), the next call to dbresults() returned NO_MORE_RESULTS
> regardless
> of where it was in the results processing.  
> 
> > I don't see the danger myself. An application has the choice 
> > of whether to call dbcanquery or dbcancel.
> > They do different things. 
> 
> Indeed.  A stored procedure may return N result sets.  dbcancel() will
> cancel them all, returning the protocol to an idle state.  dbcanquery()
> cancels only the current result set, queuing up the next one.  
> 
> Doesn't your code attempt the effect of cancelling the current query (by
> reading until TDS_COMPLETED) and then cancelling any successors?  I was
> just
> suggesting we don't need another packet-reading loop in db-lib,
> particuarly
> one that does the same thing AIUI as dbcanquery().  
> 
> Look at tds_process_cancel(), which your patch precedes.  It does the same
> thing your loop does, with more sophisitication (plus some needless
> bitmask
> complexity).  Difference is, the cancel token has already been sent by
> tds_send_cancel(), so its row processing will be limited to the spurious
> pipelined data.  
> 
> I'd still like to know:  What did you bump into that led you to add the
> discard loop to dbcancel()?  And why shouldn't tds_process_cancel() be
> changed accordingly?  Aren't the other APIs similarly affected?  
> 
> Regards, 
> 
> --jkl
> 
> 
> The information contained in this transmission may contain privileged and
> confidential information and is intended only for the use of the person(s)
> named above.  If you are not the intended recipient, or an employee or
> agent
> responsible for delivering this message to the intended recipient, any
> review, dissemination, distribution or duplication of this communication
> is
> strictly prohibited. If you are not the intended recipient, please contact
> the sender immediately by reply e-mail and destroy all copies of the
> original message. Please note that we do not accept account orders and/or
> instructions by e-mail, and therefore will not be responsible for carrying
> out such orders and/or instructions.
> 
> 
> _______________________________________________
> FreeTDS mailing list
> FreeTDS AT lists.ibiblio.org
> http://lists.ibiblio.org/mailman/listinfo/freetds