Add analysisTypes to SegmentMetadataQuery cache key

[jon-wei]

Addresses issue #1779

[fjy]

is this needed?

[jon-wei]

I saw that ListColumnIncluderator also puts a prefix byte before its list in the cache key, so I added that to be consistent

[fjy]

ah, in that case, there's different types for the last column includerator and the cache key is required to distinguish those types. In this case, analysis types is an enum, which is a little bit different. I think what needs to happen is that the analysis_types_cache_prefix cannot be a static constant, but should instead reflect what type of analysis type it is (you can use the enum value).

[fjy]

actually u already have typeBytes, so u can remove the analysis_types_cache_prefix entirely

[jon-wei]

@fjy removed the prefix byte for analysistypes

[fjy]

👍

[gianm]

this could be called getCacheBytes to make it clearer what it's for

[nishantmonu51]

or probably getCacheKey in order to be consistent with other places where we create cacheKey

[jon-wei]

@gianm @nishantmonu51 Renamed getBytes() to getCacheKey()

[gianm]

👍 lgtm

[drcrallen]

Does there need to be anything between the individual cache keys? Is it possible two different analysisTypes could have cache keys that yield the same cache key?

[gianm]

I don't think we need a separator. Right now the keys are just single bytes (and probably forever… how many flags could there be!) so this is not an imminent risk. If we do find that we need a ton of types then we can use the high bit for a variable length encoding.

[jon-wei]

I don't think that would happen in this case since each analysisType's byte[] has the same fixed length