[
https://issues.apache.org/jira/browse/SOLR-2933?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13161861#comment-13161861
]
James Dyer commented on SOLR-2933:
----------------------------------
{quote}
Why not add randomization for choosing map class
{quote}
createMap() is but a trifle to facilitate testing. Personally I think
randomizing on various Map impl's here is confusing and it would not be
intuitive why it was done. What is important is the order of the fields being
sent to the MockDataSource. In this case, there are 2 fields: "id" and
"desc". Having some rows put "id" first and the others put "desc" first is
adequate to test all the possible variants. I also think it is a good idea to
always use a Map that preserves order because when writing these tests, it is
not intuitive (for me anyhow) that the Map you're creating is going to iterate
in an order different than the one you specify. That is why I changed the
Abstract class to always use a LinkedHashMap.
{quote}
"assuming that first column is a primary key" is a really user-"hostile"
feature.
{quote}
I didn't investigate whether this is just a function of the test or if DIH "in
real life" behaves this way. In any case, if this is truly a DIH feature, I
wouldn't consider it hostile. The primary key comes first most of the time. In
any case, I wanted to solve the bug that this JIRA issue address and no more.
If we have questions about default behaviors and want to change how the
features work that is probably left to a separate non-"bug" issue.
{quote}
test methods names withWhereClause() and withKeyAndLookup() at
TestCachedSqlEntityProcessor should be swapped each other.
{quote}
I think you're right, but then again I didn't write these tests, so I'm not
sure why they were named this way. Also, changing the names is not directly
related to fixing the bug here. Personally, I would love to see the DIH tests
get revamped someday so you could just glance through them and understand what
they do and how.
In any case, now that I understand the bug and how to fix it, I would rather
create a lean patch that someone can quickly evaluate and commit. If we add
bloat or make it hard for a committer to understand it might just languish and
remain unfixed for a long time.
> DIHCacheSupport ignores left side of where="xid=x.id" attribute
> ---------------------------------------------------------------
>
> Key: SOLR-2933
> URL: https://issues.apache.org/jira/browse/SOLR-2933
> Project: Solr
> Issue Type: Sub-task
> Components: contrib - DataImportHandler
> Affects Versions: 4.0
> Reporter: Mikhail Khludnev
> Priority: Minor
> Labels: noob, random
> Attachments:
> AbstractDataImportHandlerTestCase.java-choose-map-randomly.patch,
> SOLR-2933.patch
>
> Original Estimate: 1h
> Remaining Estimate: 1h
>
> DIHCacheSupport introduced at SOLR-2382 uses new config attributes cachePk
> and cacheLookup. But support old one where="xid=x.id" is broken by
> [DIHCacheSupport.<init>|http://svn.apache.org/viewvc/lucene/dev/trunk/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DIHCacheSupport.java?view=markup]
> - it never put where="" sides into the context, but it revealed by
> [SortedMapBackedCache.<init>|http://svn.apache.org/viewvc/lucene/dev/trunk/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/SortedMapBackedCache.java?view=markup],
> which takes just first column as a primary key. That's why all tests are
> green.
> To reproduce the issue I need just reorder entry at [line
> 219|http://svn.apache.org/viewvc/lucene/dev/trunk/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestCachedSqlEntityProcessor.java?revision=1201659&view=markup]
> and make desc first and picked up as a primary key.
> To do that I propose to chose concrete map class randomly for all DIH test
> cases at
> [createMap()|http://svn.apache.org/viewvc/lucene/dev/trunk/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/AbstractDataImportHandlerTestCase.java?revision=1149600&view=markup].
>
> I'm attaching test breaking patch and seed.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
