Added values only to discovery count.#3530
Conversation
hoper-38709
force-pushed
the
task/add-sum-count-to-discovery
branch
from
0707bd6 to
076cbf5
Compare
| fields.add(this.makeField("VALUE", markings, "", 0L, thing.getTerm())); | ||
| /** | ||
| * Added query model to alias FIELD | ||
| * Added query model to alias FIELD, if DiscoveredThing::field both not NULL and not empty. |
There was a problem hiding this comment.
apply similar logic to DATE and DATATYPE. We only want to show the term value and the combined vis.
| TermEntry termEntry = new TermEntry(key, iterator.getTopValue()) { | ||
| // Only use term and visibility for equality. | ||
| @Override | ||
| public boolean equals(Object o) { |
There was a problem hiding this comment.
Move these to a TermOnlyEntry?
There was a problem hiding this comment.
Keep as TermEntry so that, downstream, we can re-use aggregate function for both "term only" path and "summation path" . DiscoveredThing aggregate(Collection termEntries)
There was a problem hiding this comment.
I assumed we would make inheritance appropriate between them such that we could still use the aggregate
| // @formatter:off | ||
| return (valuesOnly) ? new DiscoveredThing(dt.getTerm(), | ||
| "", | ||
| "", | ||
| "", | ||
| dt.getColumnVisibility(), | ||
| 0L, | ||
| new MapWritable()) | ||
| : dt; | ||
| // @formatter:on |
There was a problem hiding this comment.
Few comments here. Do not use a ternary operator and do not disable the formatter to put constructor args on different lines like this.
Use a simple guard statement with a simpler constructor like this:
if (valuesOnly ) {
return new DiscoveredThing(dt.getTerm, dt.getColumnVisibility())
}
return dt;
| // Find all matching entries and parse term entries from them. | ||
| while (iterator.hasTop() && start.equals((key = iterator.getTopKey()), PartialKey.ROW_COLFAM) && dateMatchingFunction.apply(start, key)) { | ||
| //@formatter:off | ||
| while (Optional.ofNullable(iterator).isPresent() && |
There was a problem hiding this comment.
In the event that the iterator is not set, or is later set to null a NPE is appropriate. This code change will suppress a failure state that we want to know about. Please reset this.
There was a problem hiding this comment.
I'll re-set. Another commenter recommended a NULL check.
|
New direction: Turns out we don't need to aggregate the vis. We only need to return the first instance of each unique rowid. So, if valuesOnly, break when you get the first valid TermEntry. Each next iteration will have just a single DiscoveredThing. Do the logic to only set the term and vis. In setTop, precedent is already set to modify the column family to advance to the next column family. In the values only case we will want to skip to the next row. Note: appending a null byte to the end of the current row won't work, so recommend the start key be the row and last possible column family (something like \uffff) |
5 participants
Add values only option to return from a DiscoveryLogic/ DiscoveryIterator.