[
https://issues.apache.org/jira/browse/SOLR-12361?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16505685#comment-16505685
]
David Smiley commented on SOLR-12361:
-------------------------------------
I took your PR and ran with it a bit further; patch attached. I had a stupid
bug that took some time to debug so I wasn't ready till now. I'm running tests
but so far so good.
* Moved/added javadocs to SolrDocumentBase instead of subclass impls RE child
docs. I added a bit more info to them and made getChildDocumentCount
deprecated.
* IngoreLargeDocumentProcessorFactory: ensured the value side of estimate(map)
uses the recursive estimate() instead of the non-recursive primitiveEstimate().
AddUpdateCommand:
* pending nocommit that getLuceneDocument(inPlaceUpdate) is in fact always ==
isInPlaceUpdate() so this parameter is not needed and I think we can get rid of
this overloaded variant.
* rename: getLuceneDocsIfNested (to balance with getLuceneDocument). Added
javadocs.
* this method now returns Iterable<Document> and null if it turns out there is
no nesting. This solves some awkwardness in exposing SolrInputDocument type to
the UpdateHandler which we don't want.
* rename: recUnwrap... to flatten
* moved isUsableForChildDocs check to getLuceneDocsIfNested where (to me) it
seems nicer)
DirectUpdateHandler2:
* updateDocOrDocValues now has a simpler method contract; it's caller doesn't
need to pass the "updateTerm" as it did originally, and improved from the last
patch we return void not the idTerm. doNormalUpdate had "cmd.updateTerm"
(signature dedupe) logic that needed the idTerm & updateTerm which was why
earlier I thought to return it but after some examination I think it's better
if this logic was absorbed into updateDocOrDocValues. I inlined
updateDocument() into updateDocOrDocValues -- ultimately trying to make the
code path and scope of idTerm/updateTerm more clear and I'm pretty happy with
it. Note "inline" is a verb used in coding (and can be found in IDE's for
example); you earlier mistook my suggestion as a suggestion to rename a method.
Looking again at SolrTestCaseJ4.compareSolrInputDocument... maybe this logic
ought to go on SolrInputDocument or better SolrDocumentBase as equals()? If it
were moved there, you wouldn't have even needed to make the change you did, and
thus it'd be simpler and more obvious -- why should one have to know
SolrTestCaseJ4.compareSolrInputDocument exists. Heck, I can argue that
SolrDocumentBase doesn't meet the contract of the Map it implements since a
Map's equals() demands it actually works! (hashcode too)
> Change _childDocuments to Map
> -----------------------------
>
> Key: SOLR-12361
> URL: https://issues.apache.org/jira/browse/SOLR-12361
> Project: Solr
> Issue Type: Sub-task
> Security Level: Public(Default Security Level. Issues are Public)
> Reporter: mosh
> Priority: Major
> Attachments: SOLR-12361.patch, SOLR-12361.patch, SOLR-12361.patch
>
> Time Spent: 10h
> Remaining Estimate: 0h
>
> During the discussion on SOLR-12298, there was a proposal to change
> _childDocuments in SolrDocumentBase to a Map, to incorporate the relationship
> between the parent and its child documents.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
