[ 
https://issues.apache.org/jira/browse/SOLR-10503?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Steve Rowe updated SOLR-10503:
------------------------------
    Attachment: SOLR-10503.patch

Patch folding in [~hossman]'s latest feedback.  

I was pulling out my hair trying to figure out why Solrj example tests were 
failing when performing an atomic update on a currency field, e.g.:

{noformat}
  [junit4]   2> NOTE: reproduce with: ant test  -Dtestcase=SolrExampleXMLTest 
-Dtests.method=testUpdateField -Dtests.seed=82002888280BD61E -Dtests.slow=true 
-Dtests.locale=lv -Dtests.timezone=Asia/Kamchatka -Dtests.asserts=true 
-Dtests.file.encoding=US-ASCII
   [junit4] ERROR   0.56s J10 | SolrExampleXMLTest.testUpdateField <<<
   [junit4]    > Throwable #1: 
org.apache.solr.client.solrj.impl.HttpSolrClient$RemoteSolrException: Error 
from server at http://127.0.0.1:50840/solr/collection1: ERROR: [doc=unique] 
multiple values encountered for non multiValued field price_c____l: [10000, 100]
{noformat}

I eventually traced this to {{stored="true"}} used on the dynamic sub-fields in 
{{CurrencyFieldType}}-based field types in example schemas; this causes Atomic 
Update indexing to fail; see 
[https://lucene.apache.org/solr/guide/6_6/updating-parts-of-documents.html#UpdatingPartsofDocuments-FieldStorage].
  I added information about this to the example schemas and to the ref guide.

Comments inline below:

bq. now that CurrencyField extends CurrencyFieldType, can't we move 
CurrencyValue and FileExchangeRateProvider back into CurrencyFieldType.java (or 
make them static inner classes of CurrencyFieldType) ?

I made them static inner classes, but as a result had to add exceptional 
classloader logic to load  CurrencyFieldType inner classes (currently only 
FileExchangeRateProvider).

bq. you removed the error checking from ValueSourceParser.java? ... if someone 
tries to use the currency() function on a non CurrencyFieldType it should still 
through a clean error, not a ClassCastException

Fixed.

bq. do we need to bother parameterizing fieldTypeClass in 
CurrencyFieldTypeTest? isn't it enough to just assert that it's an instanceof 
CurrencyFieldType? ... the test methods really shouldn't be affected at all by 
this distinction

Agreed, removed the fieldTypeClass parameter.

bq. If we're going to consolidate the existing tests like this, it seems better 
to parameterize the expected provider class, and add a getter to 
CurrencyFieldType for that. then we could also fix the assume in 
testAsymmetricPointQuery so it isn't dependent on us remembering to keep the 
list of field names accurate.

Done. (CurrencyFieldType already had getProvider().)

bq. I think these (2) comments are suppose to refer to subclass (not 
superclass) ??... // Don't initialize if superclass already has done so

Right, fixed.

bq. FWIW: I'm still -0 to CurrencyFieldType having hardcoded defalts for \[the 
field suffixes\] ...I think it would be a lot better if the suffixes were 
mandatory in this new class

I've made this change.  I think the user experience would be better with 
defaults for the majority of folks who I'm guessing won't care about sub-field 
details.  But I'm ok with not providing them.

bq. The existence of the dynamicFieldDocValuesArg() method feels more awkward 
and clunky then just making createDynamicCurrencyField() a protected method 
that the subclass can override....of course: if I can convince you to eliminte 
the DEFAULT_FIELD_SUFFIX_AMOUNT_* logic from the CurrencyFieldType parent 
class, that gets much simpler: only the CurrencyField subclass needs to bother 
implementing/calling createDynamicCurrencyField()

Since only CurrencyField uses createDynamicCurrencyField(), I moved it there.


> CurrencyField should be changed from TrieLongField to LongPointField for 
> underlying raw-polyfield
> -------------------------------------------------------------------------------------------------
>
>                 Key: SOLR-10503
>                 URL: https://issues.apache.org/jira/browse/SOLR-10503
>             Project: Solr
>          Issue Type: Sub-task
>      Security Level: Public(Default Security Level. Issues are Public) 
>            Reporter: Hoss Man
>         Attachments: SOLR-10503.patch, SOLR-10503.patch, SOLR-10503.patch
>
>




--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

Reply via email to