r15094 - gnucash/branches/gda-dev - Initial commit of initial gda backend framework. See GDA_STATUS.

[Derek Atkins]

Phil Longstaff <plongstaff at rogers.com> writes:

>> The business ddl should live unser src/business/business-core/gda
>> (along with any other business-specific GDA code).
>
> I agree. I created that ddl file by looking at the xml code and creating
> a ddl file from it.  I haven't even tried to see if mysql or any other
> db will accept the ddl.  Eventually it will be moved (or even moved
> internally into the code in some fashion).

Ahh, I see..  I think it might be a good idea to derive the DDL from
the C code that generates the XML, instead of looking at the XML
itself.  The XML might not show you optional parameters.

>> You should just add a configure test to find libgda and use that.  You
>> could add a --with-gda-prefix= so you could bypass the specific
>> pkgconfig checks.  But you shouldn't hard-code the path in the
>> Makefile.  The configure script should export at least two macros:
>> GDA_CFLAGS and GDA_LIBS (or LIBGDA_, and it could be _INCS instead of
>> _CFLAGS) -- but you get the point.
>
> I agree.  However, I took at look at configure.in to see how to do this
> and saw lots of AC_xxx macros which I didn't understand.  I didn't see
> another --with-xxx-prefix option that I could use as a template.  I did
> what worked for me in the short term, and have it on the TODO list to
> configure it properly.

google is your friend.  (honestly, someone can help you with this
part).  AC_ARG_WITH() is what you need for the --with-xxx-prefix part.
You can look at the --with-qof= for an idea.  And there are plenty of
places where we use PKG_CONFIG to extract the cflags and ldflags for a
package.

>> > +Execution:
>> > +- A basic GDA backend framework now exists.  This framework accepts URLs of
>> > +the form gda://DSN:USERNAME:PASSWORD.  "gda" is required.  "DSN" represents
>> > +a dataset configured in ~/.libgda/config.  USERNAME and PASSWORD are not
>> > +required but can be specified.
>> 
>> How would this work with SQLite?
>
> It will work with SQLite if a DSN is configured.  Another TODO item is
> to switch file:// over to use SQLite which will just take a file name.

Sorry, please assume I know nothing about GDA.  I'm an end user.  I
have no idea what a DSN is, what it means, or how to configure one.
As a user I just want to type in a filename and let gnucash do its
magic; under the covers gnucash should translate that filename into
whatever URL it needs to pass to GDA....

Note that switching file:// over to SQLite is a little more
complicated..  In particular we need to deal with updating from
XML->SQLite, and probably change the File -> New File subsystem to
request a filename (or DSN, whatever that is) for the new file so it
can get created immediately.

> BTW, the file UI is built around files.  Save As, for example, doesn't
> allow me to save as gda://gnucash.

This is a bug that we need to fix.

>> > +- The backend will save commodities and load them on startup.  However, they
>> > +will not have the correct GUIDs.
>> 
>> I'm not convinced that Commodidies need to have (or SHOULD have)
>> GUIDs.  In fact, the XML backend doesn't store the commodity GUID,
>> either.  Everything is referenced PURELY by the Commodity
>> <Namespace,ID> tuple.  The database should mirror this.
>
> The problem is ran into is how to handle it when a commodity's name or
> namespace is changed.  If a guid is used as the commodity ID, I just
> update the commodity.  If I use the tuple, I need to update every
> account, transaction, ... that uses that commodity.  The XML backend
> doesn't have this problem because internally, the name can be changed,
> and the XML backend can write the new tuple with the new namespace or
> ID.

Good point.  Well, the only GUIDs you need to care about are the ones
that are actually used in the database.  But we also need to let
gnucash (the app) update currencies that change over time.  So perhaps
what we want to do is load all the currencies from the DB, then
"merge" them in from the application?  What this DOES mean is that the
app's "create currencies" function would need to be changed to see if
a currency already exists and if so just update it (if necessary) with
the new settings -- and this could change the contents of the DB.  But
the DB really only needs to store the currencies in use by the rest of
the database.

>> > +	PRIMARY KEY(slot_id)
>> 
>> Actually, I think the primary key would be the <obj_guid,name> tuple.
>
> Could be, since as far as I can tell, the slot info really belongs to
> the owning object.

Yes, the slot info really does belong to the owning object.

[snip]
>> > +#ifndef HAVE_STRPTIME
>> > +# include "strptime.h"
>> > +#endif
>> 
>> Are you sure you need all these headers?  Or did you just copy it?
>
> Just copied.

Okay, you might want to trim down the header includes ;)

>> For example, I could see how you could define the following mappings:
>> 
>>   column -> QofParam
>>   QofType -> GdaType
>> 
>> and then use this abstraction to say something like:
>> 
>> AccountColumns[] = { QOF_PARAM_GUID, ACC_TYPE, ACC_CMDTY, ... };
>> 
>> Then you can use the QofParam Getters/Setters...
>
> This was partly to experiment with Qof and with Gda and GValues and ...
> to see how things actually work.  Unfortunately, not all db fields can
> be handled via a mapping (e.g. Account parent GUID).

Note that the mapping would work back into QOF.  The Account parent GUID
already has a qof mapping, which would use qofAccountSetParent() which
just takes a QofEntity..  But keep in mind that QOF knows that the
parent is of type ID_ACCOUNT, so you would know how to convert between
ID_ACCOUNT and the row content.  E.g., you would know that to STORE
the data you would use:

    guid_to_string(get_guid(qof_getter(obj)))

and to LOAD this you would use:

    qof_setter(lookup_by_guid(ID_ACCOUNT,string_to_guid(row_data)))

This is all abstracted because you know how to store an ID_ACCOUNT QOF
Type from the QOF Param Tables and use the QOF Getters/Setters.

I still believe that all db fields can be handled via a mapping.  You
just might need to add additional handling for certain QOF types...
In particular QOF Types that are compsite objects are referenced by ID
so you would just need a special handler.

>> > +	pAccount = xaccMallocAccount( pBook );
>> 
>> You should begin_edit here...
>
> except that all I want to do is create a new account structure.  I
> really don't want any background stuff.  I certainly don't want to
> commit (and I've already added a flag turning off commits while doing an
> initial load).

Hmm... True...

>> [snip]
>> > +			sprintf( cmdbuf, "UPDATE accounts set name='%s',account_type_id=%d,commodity_guid='%s',parent_guid='%s',code='%s',description='%s' WHERE guid='%s';\n",
>> > +				name, type, commodity_guid_buf, parent_guid_buf, code, description,
>> > +				guid_buf );
>> 
>> This could easily overwrite the buffer size.  Also, what happens if
>> the user puts parens into the data?  E.g., what happens if I have an
>> account name of something like "Foo's Account" -- that would cause a
>> SQL error.  Worse, this could cause a user to be able to potentially
>> execute arbitrary SQL.
>> 
>> Doesn't GDA have a feature to pull variables from an argument array
>> instead of printing them into standard SQL String?  E.g., something like:
>> 
>>    gda_build_query("UPDATE accounts set name=%1,account_id=%2,...",
>>                    name, type, ...);
[snip]
>> Again, you've got a huge potential for buffer overrun here.   We should
>> probably use g_strdup_printf() here, again, or at least be amenable to
>> arbitrary string sizes.   Also, notice this SEGV waiting again?  ;)
>
> Again, the segv is a FIXME.  I need to look more at how to handle error
> conditions.

There's also the string-quoting issue...

> Thanks for the comments.

You're welcome.  I hope the (constructive) criticism has been
useful and helpful.

> Phil

-derek

-- 
       Derek Atkins, SB '93 MIT EE, SM '95 MIT Media Laboratory
       Member, MIT Student Information Processing Board  (SIPB)
       URL: http://web.mit.edu/warlord/    PP-ASEL-IA     N1NWH
       warlord at MIT.EDU                        PGP key available