[ 
https://issues.apache.org/jira/browse/SOLR-1131?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12789330#action_12789330
 ] 

Chris A. Mattmann commented on SOLR-1131:
-----------------------------------------

Hi Grant:

bq.The array is sorted and array access is much faster and we often have to 
loop over it to look it up.

OK, fair enough.

bq. I think comma makes sense. 

OK, so you think it makes sense -- why? Because it's an N-dimensional array and 
spaces are less "user friendly" as Yonik put it? 

bq. As for the optimization stuff, I agree w/ Yonik, this is code that will be 
called a lot.

I'm wondering what there is to agree with since "optimization" was never 
defined. Are you talking speed? Are you talking memory efficiency? Code 
readability? Maintainability? Some combination of all of those? There are 
tradeoffs in everything. You could rewrite some of the provided java runtime 
methods to squeeze out extra performance, but what's the point of libraries or 
reusable functions then? The prior code that was in there basically rewrote 
exactly what split() and trim() do, so why not reuse what's there? If you throw 
up the performance flag, I would push back on readability and maintainability. 

bq. It is logged, but for the poly fields, if the dyn field is already defined, 
that's just fine.

Where is it logged? It wasn't in the most up-to-date patch, provided on 
2009-12-10 04:34 PM. Here was the code snipped that was there:

{code}
+    //For each poly field, go through and add the appropriate Dynamic field
+      for (FieldType fieldType : polyFieldTypes.values()) {
+        if (fieldType instanceof DelegatingFieldType){
+          List<FieldType> subs = ((DelegatingFieldType) 
fieldType).getSubTypes();
+          if (subs.isEmpty() == false){
+            //add a new dynamic field for each sub field type
+            for (FieldType type : subs) {
+              log.debug("dynamic field creation for sub type: " + 
type.typeName);
+              SchemaField df = SchemaField.create("*" + 
FieldType.POLY_FIELD_SEPARATOR + type.typeName,
+                      type, type.args);//TODO: is type.args right?
+              if (isDuplicateDynField(dFields, df) == false){
+                addDynamicFieldNoDupCheck(dFields, df);
+              }
               // NOTE: there is no else here, so I added an else and a log 
message
+            }
+          }
            // NOTE: there is no else here, so I added an else and a log message
+        }
+      }
+
{code}

Here's what I added:

{code}
+    //For each poly field, go through and add the appropriate Dynamic field
+      for (FieldType fieldType : polyFieldTypes.values()) {
+        if (fieldType instanceof DelegatingFieldType){
+          List<FieldType> subs = ((DelegatingFieldType) 
fieldType).getSubTypes();
+          if (!subs.isEmpty()){
+            //add a new dynamic field for each sub field type
+            for (FieldType type : subs) {
+              log.debug("dynamic field creation for sub type: " + 
type.typeName);
+              SchemaField df = SchemaField.create("*" + 
FieldType.POLY_FIELD_SEPARATOR + type.typeName,
+                      type, type.args);//TODO: is type.args right?
+              if (!isDuplicateDynField(dFields, df)){
+                addDynamicFieldNoDupCheck(dFields, df);
+              }
+              else{
+                log.debug("dynamic field creation avoided: dynamic field: 
["+df.getName()+"] " +
+                               "already defined in the schema!");
+              }
+              
+            }
+          }
+          else{
+            log.debug("field type: ["+fieldType.getTypeName()+"]: no sub 
fields defined");
+          }
+        }
+      }
+
{code}

Also, I get that it's fine for the poly fields if the dyn field is already 
defined (in fact, based on my mailing lists comments 
http://old.nabble.com/SOLR-1131:-disconnect-between-fields-created-by-poly-fields-td26736431.html
 I think this should _always_ be the case), but whether it's fine or not, it's 
still worthy to log to provide someone more information. 



> Allow a single field type to index multiple fields
> --------------------------------------------------
>
>                 Key: SOLR-1131
>                 URL: https://issues.apache.org/jira/browse/SOLR-1131
>             Project: Solr
>          Issue Type: New Feature
>          Components: Schema and Analysis
>            Reporter: Ryan McKinley
>            Assignee: Grant Ingersoll
>             Fix For: 1.5
>
>         Attachments: SOLR-1131-IndexMultipleFields.patch, 
> SOLR-1131.Mattmann.121009.patch.txt, SOLR-1131.patch, SOLR-1131.patch, 
> SOLR-1131.patch, SOLR-1131.patch, SOLR-1131.patch, SOLR-1131.patch
>
>
> In a few special cases, it makes sense for a single "field" (the concept) to 
> be indexed as a set of Fields (lucene Field).  Consider SOLR-773.  The 
> concept "point" may be best indexed in a variety of ways:
>  * geohash (sincle lucene field)
>  * lat field, lon field (two double fields)
>  * cartesian tiers (a series of fields with tokens to say if it exists within 
> that region)

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to