From: scottmk <scottmk@dcc99617-32d9-48b4-a31d-7c20da2025e4>
Date: Wed, 4 Feb 2009 21:49:06 +0000 (+0000)
Subject: In SELECT(): further rewrote, and festooned with comments, the code
X-Git-Url: https://old-git.evergreen-ils.org/?a=commitdiff_plain;h=34820b106b1bcb96f03739a985ed63a5d5ac8b96;p=evergreen%2Ftadl.git

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.


git-svn-id: svn://svn.open-ils.org/ILS/trunk@12069 dcc99617-32d9-48b4-a31d-7c20da2025e4
---

diff --git a/Open-ILS/src/c-apps/oils_cstore.c b/Open-ILS/src/c-apps/oils_cstore.c
index 3cab483c04..a05029e9c0 100644
--- a/Open-ILS/src/c-apps/oils_cstore.c
+++ b/Open-ILS/src/c-apps/oils_cstore.c
@@ -2399,7 +2399,10 @@ static char* SELECT (
 	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 @@ static char* SELECT (
 		    // round trip through the idl, just to be safe
 			const char* cname = selclass_itr->key;
 			osrfHash* idlClass = osrfHashGet( oilsIDL(), cname );
-		    if (!idlClass) continue;
-
-		    // make sure the target relation is in the join tree
-		    if (strcmp(core_class,cname)) {
-			    if (!join_hash) continue;
+		    if (!idlClass)
+				// No such class.  Skip it.
+				continue;
 
-				unsigned long size;
+		    // 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.
+
+		    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 @@ static char* SELECT (
         // jsonIteratorFree(selclass_itr);
 
 	    if (is_agg) jsonObjectFree(is_agg);
-    } else {
-		OSRF_BUFFER_ADD_CHAR( select_buf, '*' );
     }