[
https://issues.apache.org/jira/browse/SOLR-5010?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13702476#comment-13702476
]
Steve Rowe commented on SOLR-5010:
----------------------------------
Grant,
I'm reviewing the commit you did for the first part, that added copyFields
destinations to the addField(s) Java methods and the {{/schema/fields}} REST
API methods.
1. There's an issue in FieldCollectionResource.post() with persisting the
schema after calling registerCopyFields() - you don't call the version of
ManagedIndexSchema.addFields() you added that takes a source->dest+ copyField
map param, and as a result the copyField directives won't be persisted with the
other schema changes:
{code:java}
IndexSchema newSchema = oldSchema.addFields(newFields);
for (Map.Entry<String, String> entry : copyFields.entrySet()) {
//key is the source, value is a comma separated list of targets
String [] splits = entry.getValue().split(",");
if (splits != null && splits.length > 0){
for (int i = 0; i < splits.length; i++) {
newSchema.registerCopyField(entry.getKey(), splits[i].trim());
}
}
}
{code}
By contrast, FieldResource.put() calls the version of the
ManagedIndexSchema.addField() method you added that takes in a collection of
copyField destinations, so it's fine.
2. Ditto of my above comment #3: there is no {{else}} after the above {{if}}
block to handle the case when {{splits}} is null or empty.
3. You changed the return type of ManagedIndexSchema.addFields from
ManagedIndexSchema to IndexSchema see the change
[here|https://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchema.java?r1=1500744&r2=1500743&pathrev=1500744&diff_format=f#l185]
- why did you do this? I intentionally narrowed the return type when
overriding this method. (There may be other instances of this kind of thing, I
didn't check for others.)
4. You changed a shitload of whitespace stuff, most of which I don't care about
but find annoying when evaluating patches. There is one in particular, though,
that bugs me: {{if ( ! boolean) ... }} -> {{if (!boolean) ...}} - I
intentionally add whitespace there to make the negation stand out, and it
doesn't anymore now that you've removed the whitespace. I'd like to put those
back.
> Add REST support for Copy Fields
> --------------------------------
>
> Key: SOLR-5010
> URL: https://issues.apache.org/jira/browse/SOLR-5010
> Project: Solr
> Issue Type: Sub-task
> Components: Schema and Analysis
> Reporter: Grant Ingersoll
> Assignee: Grant Ingersoll
> Fix For: 5.0, 4.4
>
> Attachments: SOLR-5010-copyFields.patch, SOLR-5010.patch,
> SOLR-5010.patch, SOLR-5010.patch
>
>
> Per SOLR-4898, adding copy field support. Should be simply a new parameter
> to the PUT/POST with the name of the target to copy to.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
