Re: [Gnumed-devel] gmClinical.sql and general comments

[Richard Terry]

Will digest the stuff below.

However in regard to naming conventions I'd suggest the following
gm + schema + table + whatever.

eg GmClinical_Vaccinations_Static.sql, which would contain all the vaccination 
stuff

Though I'm not sure I like the nomenclature for the Static etc, at least now 
you have explained it I understand why you have separated it into static and 
dynamic.

Also with table definitions, because of  the inconsistancies in your table 
names they dont' sit together often in the database structure.

I would keep the stems all similar whereas I note using vaccinations as an 
example you have tables called vacc_whatever, vaccination, link_vacc etc 
instead of 
vaccinations_data
vaccination_link_bla bla
vaccination_lu_vaccines (lu=lookup)
vaccination_lu_schedules

I can give some thought to this, the above is only off the cuff examples, but 
whatever, one should aim to make things more readable.

Given that postgres supports longer names you should make use of this, again 
because anyone coming after you (pray you don't get suddenly killed(), won't 
be able to understand easily what you have done.

Same with shema's schema clinical is much better than clin, or demographics, 
instead of dem, or configuration instead of cfg or administration instead of 
admin. There is no reason to use short names.

Regards

Ricahrd



vaccinations
vacciaations_schedules.
On Thursday 12 January 2006 09:17, Karsten Hilbert wrote:
> On Thu, Jan 12, 2006 at 08:44:45AM +1100, Richard wrote:
> > ========
> > gmClinical.sql
> > ========
> > Apologies first if this is a bit garbled, but I've been jotting notes as
> > I go.
>
> No problem.
>
> > I'm getting really stuck on this one Karsten, it seems to  me this file 
> > is a maintenance nightmare.
>
> It is.
>
> > It is a huge file, and covers many many areas of clinical things all
> > stuffed in together.
>
> Yes. The reason is historical. I have been getting away from
> that with the newer stuff.
>
> > Is there any reason that it can't be split up into logical units eg
> > gmVaccinations.sql, gmAllergies.sql etc?.
>
> No reason. However, if so do maintain the separation of
> static (tables) vs dynamic (pretty much everything else)
> stuff. The reason is that the dynamic stuff can be *rerun*
> on an existing database without putting the actual data in
> jeopardy at all.
>
> > Some of the areas causing me problems seem to be around statements
> > containing lines like:
> >
> > alter table clin.vacc_def  (from gmClinicalViews.sql)
> >     add constraint numbered_shot_xor_booster
> >             check (
> >                     ((is_booster is true) and (seq_no is null)) or
> >                     ((is_booster is false) and (seq_no > 0))
> >             );
> >
> > this won't compile, stops and asks to input  a parameter
>
> Very strange. I tend to think pgAdmin is not sophisticated
> enough. The above is perfectly valid SQL and it bootstraps
> just fine. You might have to contact the pgAdmin people.
>
> > Also it  crashes and burns with syntax errors in some statements  eg:
> >
> > -- crashes and burns (syntax error at char 19) --- start *****
> > create or replace trigger tr_ins_booster_must_have_base_immunity
>
>          ^^^^^^^^^^
> This DOES NOT EXIST in CVS. It would indeed be invalid.
>
> >     before insert on clin.vacc_def
> >     for each row execute procedure
> > clin.f_ins_booster_must_have_base_immunity(); -- crashes and burns
> > (syntax error at char 19) --- end *****
> >
> > I  will try and dissect this file into smaller subunits myself and let
> > you know the result in a more specific manner, but if you could give
> > thought to putting it into smaller files that would be appreciated.
>
> Sure.
>
> > Another couple of questions  - in the code you often have on error
> > start/stop statements when creating a function or table. Is there any
> > reason you do this rather than using
>
> The reason is this:
>
> The script is *supposed* to work in its entirety. If it
> doesn't it contains a bug. That's why we do "\set
> ON_ERROR_STOP 1" at the top. This means any error will stop
> the script execution. Unfortunately, some things are treated
> as errors by PostgreSQL which aren't really errors, for example:
>
> - to create a view it must not yet exist
> - there is no way to *test* whether it exists
> - hence we need to first DROP the view
> - however, if the view actually didn't exist dropping it would be an error
> - hence we temporarily allow the dropping to fail
>
> Now, one could just as well allow the creation to fail and
> continue but that would not allow to differentiate the
> *reason* for that failure while the above procedure
> guarantuees that the failure is NOT due to the view already
> existing.
>
> > Create or replace function/table?
>
> "create or replace" only works for functions, any functions
> not yet using "create or replace" just has not been
> converted yet
>
> > Lastly sql file naming conventions - It. would be nice to have same
> > capitalisation, hyphenation schemata for the files.
>
> The convention is:
>
> - start with "gm"
> - end with ".sql"
> - name it for the content it handles (yes, gmClinial.sql sucks)
> - add -dynamic/-static/-data to differentiate
>
> Hence:
>
>  gmProviderInbox-static.sql
>  gmProviderInbox-dynamic.sql
>
> Any files not following this convention just haven't been
> converted yet. If you think it is *really* important I'll
> rename them ASAP.
>
> Karsten