Change raw Map type to Map<String,Object> for ValueSource context [LUCENE-5335]

[asfimport]

Just as the title says. Simple refactoring.

---

Migrated from LUCENE-5335 by Ryan Ernst (@rjernst), updated Dec 27 2016
Parent: #6389
Attachments: LUCENE-5335.patch
Linked issues:

• Replace ValueSource context Map with a more concrete data type [LUCENE-6232] #7294
• Fix compiler warnings for ant clean compile [LUCENE-7602] #8653

[asfimport]

Adrien Grand (@jpountz) (migrated from JIRA)

+1

[asfimport]

Yonik Seeley (@yonik) (migrated from JIRA)

Hmmm, but the map is currently an identity hash map, and all the keys are not currently Strings. It may not always be easy to come up with a unique String, and perhaps no good reason to require it?

[asfimport]

Ryan Ernst (@rjernst) (migrated from JIRA)

Patch.

[asfimport]

Ryan Ernst (@rjernst) (migrated from JIRA)

Hmmm, but the map is currently an identity hash map, and all the keys are not currently Strings.

Where is there a non-String key? The patch compiles fine; I would have expected problems if a key existed somewhere that wasn't a String.

[asfimport]

Yonik Seeley (@yonik) (migrated from JIRA)

I would have expected problems if a key existed somewhere that wasn't a String.

On a quick look, you only converted getValues and not createWeight? Perhaps that's why it compiles (but should get an error at runtime if we have good enough test coverage).

SumTotalTermFreqValueSource is one example of where "this" is very conveniently used as a key.

[asfimport]

Yonik Seeley (@yonik) (migrated from JIRA)

That is strange that it even compiles... your patch has this:

+  public FunctionValues getValues(Map<String, Object> context, AtomicReaderContext readerContext) throws IOException {
     return (FunctionValues)context.get(this);
   }

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

I dont think things like SumTotalTermFreqValueSource should be a blocker to fixing this map API, actually I think a cleaner API here is really needed.

Look at DocFreqValueSource for example, it screws this up: if you have 30 segments it will call docFreq for the term 900 times. Same goes with IDFValueSource and many of these other things.

These "top level" valuesources should be implemented in a cleaner and consistent way that is not slow.

[asfimport]

Yonik Seeley (@yonik) (migrated from JIRA)

I dont think things like SumTotalTermFreqValueSource should be a blocker to fixing this map API

The whole point of the current context is so you can do stuff like the current code does, so it's really unclear how you would "fix" it (or why it needs fixing), without first figuring out a different way to implement stuff like SumTotalTermFreqValueSource. In that sense, it certainly does seem like a blocker.