RJLRef: $PH/06f522/asgnt3/asgnt3_ns_moreRJLfdbk061106.txt 1. Thanks for more info on this chgen bug report ($PH/06f522/asgnt3/emptyTableBug_ns061106.txt). 2. Code comments: ------------------------- I read asgnt3.c. Congratulations on an excellent job of organizing your test code and showing how to wrap chgen's API methods into better ones for your purposes. Refactoring this API ought to eliminate the global XXcurr variables updated by find*loop calls wrapped inside your find* methods, because side effects on globals will inevitably interfere with other methods that depend on XXcurr. This refactoring is better done in gencpp not chgen. Your naming conventions and hierarchy of calls are very meaningful and help avoid confusion about what is intended. Others should study them carefully. There are a large number of pr_set_ method calls in asgnt3.c; I do not expect you to convert them to XXGetFname calls in pr_accessors.c. You also create your own test database as well as CK specs and a few CQ queries for them. I expected you to use the SISrev061011.dat table rows by pr_loading them. You can still do this, as long as you load them FIRST, before creating your own example data. If you create your own first, then your pkeys will likely be duplicated in SISrev061011.dat; these duplicates will be IGNORED when pr_loading them, to preserve uniqueness of pkeys. If you pr_load SIS*.dat first, then pr_add assigns the next available unused pkey when you add new rows. 3. Metadata vs. data comments: ------------------------------- Normally, metaschema*.msdat and SIS*.dat are two separate files with non-overlapping VIEWNAMES. Chgen13 itself even has the capability to load file .msdat instead of parsing file .sch. I would like the pr_*.c API to have this capability as well, when chgen and gencpcp are upgraded to remove pr_init and the version# subfield of surrogate pkeys. In chgenv13, pr_init and pr_load do not use metadata, so these two files can be pr_loaded together, after prefixing metaschema*.msdat to SIS*.dat. In effect, Nitin's .msdat.out produces such a merged file. It should be used in repeated regression tests to avoid calling all the same constructors again. Successive revs of the application only need to create new CQ-table-rows and queries; they can pr_load pre-existing ones. Finally, script-language filters can easily create the desired CQ-row values, and add a pkey and one fkey if the start points are given. BBexpand.awk is one example of such a generic filter. (See $CASE/96su523/asgnt4/BBexpand.awk.) This might avoid CQ-row instance creation code altogether. ` 4. Potential bugs: ------------------------- A. In general, you take care of non-error paths but have you verified what happens when a search loop returns a NULL pointer? B. In createCandidatAttribute (or createCA :-) fname is not globally unique - it has table scope and likely to be duplicated. Therefore table_loop is not selective enough. The search needs to do child_loop(TT,TA,TAid,TTid) then if it fails, traverse TA-children of TT-rows for both superclass delegates and enclosing container ancestors bottom-up, looking for the fname in their TA-children. Note the importance of partial ordering here - the fkeys in one TT (is_key = 1 or s) identify the ancestor tables whose TA-attribute fnames are to be searched next. [In general, you take care of non-error paths but have you verified what happens when a search loop returns a NULL pointer?] C. In addTestQueries, addNewQuery("Check 91.522",...) may work but introduces a latent bug if this CQ-row is saved that may corrupt the database when it is pr_loaded again: The cqname field cannot have whitespace inside because pr_parse will split it and corrupt the values of later fields of this record (or worse:-). A way out is to extend chgen to handle a field of type 'qnn' where q means QUOTED string. This is not a big deal, as long as pr_parse is generalized to match up the quote chars. I never found a need for qnn outside the Microsoft world of filenames with spaces which unix avoids (so far). Bob Lechner > From nitin.sonawane@gmail.com Mon Nov 6 20:04:15 2006 > Prof. Lechner, > > It is very easy to reproduce the problem using my code. What > I found was that even after a pr_add() of a table element example, > aa_elt, the pr_add call returned but AA continued to be NULL > (rather than point to the newly created aa_elt). Subsequent calls > to pr_dump() do not work but that is not surprising . > > I even single stepped through gdb to see what might be going > wrong but wasnt able to do much diagnosis. I should be able to > demonstrate this to you tomorrow (perhaps after class?) and we > can step through the debugger. > > Thanks, > Nitin. > > On 11/5/06, Bob Lechner wrote: > > > > I do agree. That's OK Nitin. > > However if you have trouble with pr_*.c files > > you might want to do cvs update and rebuild chgen13. > > I made some changes to gen_*.c files in case/gen/ver_13.. > > and checked them into CVS. > > > > I didn't solve my problems with pr_dump however. > > It may be because of what you discovered - that > > a non=-empty table is needed to start off. > > Now that didn't use to happen so it's probably a bug that > > I introduced. I would like to know details of how you > > discovered it so I can try correcting it at its source. > > > > One thing I tried experimentallay is to avoid versio# > > sensitivity in VMNetDB (and in viewdef files). > > I may have partly updated chgen with some artifacts remaining. > > One possibility is the XX01000n row #s - the 01(hex) is version# 1 > > and pr_dump may be selecting version#0. > > Do you call pr_dump to a .dat file and does it work? > > > > Bob Lechner