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

Steve Rowe commented on SOLR-5010:
----------------------------------

Grant, a few comments about your most recent patch:

1. This comment in ManagedIndexSchema.addCopyFields() should be changed to 
reflect its context:
{code:java}
// even though fields is volatile, we need to synchronize to avoid two addFields
// happening concurrently (and ending up missing one of them)
{code}
to something like "we need to synchronize to avoid two addCopyFields happening 
concurrently (and ending up missing one of them)"

2. In CopyFieldsCollectionResource.post(), the loop over the copy field 
directives will throw away all but the last directive, because the directives 
are each added to {{oldSchema}}, which is never updated:

{code:java}
ManagedIndexSchema oldSchema = (ManagedIndexSchema) getSchema();
for (Map<String,Object> map : list) {
[...]
  if (splits != null && splits.length > 0){
    for (int i = 0; i < splits.length; i++) {
      destinationSet.add(splits[i].trim());
    }
    IndexSchema newSchema = oldSchema.addCopyFields(fieldName, destinationSet);
    if (newSchema != null) {
      getSolrCore().setLatestSchema(newSchema);
    }
  }
{code}

Maybe ManagedIndexSchema.addCopyFields() could accept a 
{{Map<String,Set<String>>}}, containing all copyField directives, instead of 
the current one at a time?  That would mirror behavior of 
ManagedIndexSchema.addFields(), and would make it so that the schema would only 
need to be modified/persisted once; the above-described problem would then 
disappear.

3. There is no {{else}} after the above {{if}} block to handle the case when 
{{splits}} is null or empty.  It's not an impossible condition - for example, 
{{"dest":",,,"}} would trigger this case.


                
> 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]

Reply via email to