[open-ils-commits] r12069 - trunk/Open-ILS/src/c-apps

[svn at svn.open-ils.org]

Author: scottmk
Date: 2009-02-04 16:49:06 -0500 (Wed, 04 Feb 2009)
New Revision: 12069

Modified:
   trunk/Open-ILS/src/c-apps/oils_cstore.c
Log:
In SELECT(): further rewrote, and festooned with comments, the code
that verifies that every column in the SELECT clause comes from a
class in the FROM clause.

Also, for readability: reversed an "if" test that treats functions
differently from classes.


Modified: trunk/Open-ILS/src/c-apps/oils_cstore.c
===================================================================
--- trunk/Open-ILS/src/c-apps/oils_cstore.c	2009-02-04 21:28:53 UTC (rev 12068)
+++ trunk/Open-ILS/src/c-apps/oils_cstore.c	2009-02-04 21:49:06 UTC (rev 12069)
@@ -2399,7 +2399,10 @@
 	growing_buffer* group_buf = buffer_init(128);
 	growing_buffer* having_buf = buffer_init(128);
 
-	if(!from_function) {
+	// Build a select list
+	if(from_function)   // From a function we select everything
+		OSRF_BUFFER_ADD_CHAR( select_buf, '*' );
+	else {
 
 		// If we need to build a default list, do so
 		jsonObject* _tmp = jsonObjectGetKey( selhash, core_class );
@@ -2428,26 +2431,43 @@
 		    // round trip through the idl, just to be safe
 			const char* cname = selclass_itr->key;
 			osrfHash* idlClass = osrfHashGet( oilsIDL(), cname );
-		    if (!idlClass) continue;
+		    if (!idlClass)
+				// No such class.  Skip it.
+				continue;
 
-		    // make sure the target relation is in the join tree
-		    if (strcmp(core_class,cname)) {
-			    if (!join_hash) continue;
+		    // Make sure the target relation is in the join tree.
+			
+			// At this point join_hash is a step down from the join_hash we
+			// received as a parameter.  If the original was a JSON_STRING,
+			// then json_hash is now NULL.  If the original was a JSON_HASH,
+			// then json_hash is now the first (and only) entry in it,
+			// denoting the core class.  We've already excluded the
+			// possibility that the original was a JSON_ARRAY, because in
+			// that case from_function would be non-NULL, and we wouldn't
+			// be here.
 
-				unsigned long size;
+		    if ( strcmp( core_class, cname )) {
+			    if (!join_hash) 
+					// There's only one class in the FROM clause,
+					// and this isn't it.  Skip it.
+					continue;
 
 				if (join_hash->type == JSON_STRING) {
 				    string = jsonObjectToSimpleString(join_hash);
-					size = strcmp( string, cname ) ? 0 : 1;
+					int different = strcmp( string, cname );
 				    free(string);
-			    } else {
+					if ( different )
+						// There's only one class in the FROM clause,
+						// and this isn't it.  Skip it.
+						continue;
+				} else {
 				    jsonObject* found = jsonObjectFindPath(join_hash, "//%s", cname);
-					size = found->size;
+					unsigned long size = found->size;
 					jsonObjectFree( found );
-			    }
-
-			    if ( 0 == size )
-				    continue;
+					if ( 0 == size )
+						// No such class anywhere in the join tree.  Skip it.
+						continue;
+				}
 		    }
 
 		    // stitch together the column list ...
@@ -2579,8 +2599,6 @@
         // jsonIteratorFree(selclass_itr);
 
 	    if (is_agg) jsonObjectFree(is_agg);
-    } else {
-		OSRF_BUFFER_ADD_CHAR( select_buf, '*' );
     }