[ 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.