Re: Ada DB bindings and APQ

[Brian May <bam@snoopy.apana.org.au>]

>>>>> "Warren" == Warren W Gay VE3WWG <ve3wwg@NoSpam.cogeco.ca> writes:

Hello Warren,

After reading some of this code, I can begin to understand some of the
issues better. Some comments below.

    Warren> When I started with the first version, I didn't want
    Warren> coupling between the Connection_Type and the Query_Type,
    Warren> because one or the other could go out of scope first.  As
    Warren> long as they are kept separately, you avoid this problem,
    Warren> because to invoke some operations like "Execute", you must
    Warren> provide them both. Hence you cannot accidentally have the
    Warren> Connection_Type destructed for Execute (though the
    Warren> connection object may be in an unconnected state - but
    Warren> this is detectable ;-) This keeps the application
    Warren> programmer more honest.

Unfortunately, you seem to have lost this benefit already, Query_Type
*is* coupled to Connection_Type (or it looks that way to me):

      Q : Root_Query_Type'Class := New_Query(C1);

begin

...
      Execute_Checked(Q,C2);
...


I get the impression that New_Query(C1) constrains Q so it is only
guaranteed to work with C1.  As such C2 needs to be C1 (or am I
mistaken)?

Worst case, what happens if C1 is a Postgresql connection, but C2 is a
Mysql connection, and the programmer got the two confused?

If this is the case, then one of the parameters is redundant, likely
to cause errors, and should be removed...

As the query string is specific to a connection anyway, I think the
reference in New_Query should stay.

Same goes with the encoding routines.

    >> 3. Anyway, Execute should take only one parameter: Execute (Q);
    >> -- Query knows its connection

    Warren> See the above. From a design point of view, I didn't like
    Warren> this, because the application writer could mistakenly
    Warren> allow the connection to fall out of scope, and hence
    Warren> create "undefined behavior" at this point (because of a
    Warren> hidden connection reference). This form:

This is an issue. Obviously the connection needs to last longer then
the query.

I believe an issue, especially for the case of Execute, is that it may
change the results of the connection, e.g. to error status. You don't
want it to change values that have no obvious connection with the
parameters.

Possible solution (rough):

Split Connection_Type into two types, a Connection_Type and a
Internal_Type.  The user only sees the Connection_Type. Internal_Type
(or whatever you want to call it) is the actual database connection
(similar to the current Connection_Type). Connection_Type and
Query_Type both refer to Internal_Type, and Internal_Type has
reference counting (so it isn't closed while a reference still
exists).

As far as the user is concerned the types work just the same as
before, with one important difference: If an error occurs when calling
Execute(Q), the state of the connection is not effected. Rather the
error information is stored in the Query_Type object instead.

If the user closes the Connection_Type, then it closes the connection,
but doesn't free Internal_Type if Queries are still referring to it
(as known by reference counting). If a query tries to use the
connection after it is closed, then an exception is generated.

Obviously this is rough, and I haven't considered certain issues yet
(most obvious question for me at the moment: would we still need a
child class of Connection_Type for every database? If we don't, then
every connection has to have the same API; need to think about this
more).

I hope some of this makes sense to others, and I hope it makes sense
to me after I wake up tomorrow ;-).


On the matter of coding style, I often notice loops like the following
in APQ code and manual of the form:

      loop
         begin
            Fetch(Q);
         exception
            when No_Tuple =>
               exit;
         end;

         ...

      end loop;

I don't like this, an exception should only be for unexpected events,
but the end of the query is completely expected.

The documentation has an alternative loop structure:

while not End_Of_Query(Q) loop
      Fetch(Q);
      ...
end loop;

This, IMHO, is better as no exceptions occur unless something
unexpected really does occur. It also looks neater and easier to read.
-- 
Brian May <bam@snoopy.apana.org.au>