Init snapshot #1
Conversation
| // Begin by getting references to the column sources from the input table to process later. | ||
| this.timeSource = source.getColumnSource(timestampColumnName).reinterpret(long.class); | ||
| this.timeSource = ReinterpretUtils.instantToLongSource(source.getColumnSource(timestampColumnName)); |
There was a problem hiding this comment.
Wrong reinterpret here. Use timeToEpochLongSource instead. The source is probably, but not necessarily an Instant source, so you need to be more forgiving
There was a problem hiding this comment.
You mean zonedDateTimeToLongSource? That seems less generic I think?
| * @return the Map of grouping keys to BookState | ||
| */ | ||
| final Map<Object, BookState> processInitBook(final Table t, String... groupings) { | ||
| final Map<Object, BookState> initStates = new HashMap<>(); |
There was a problem hiding this comment.
You'll want to assert that t is not live, otherwise the calculation below will be suspect.
There was a problem hiding this comment.
Would this do it?
assert !t.isRefreshing();
| * @return a new table representing the current state of the book. This table will update as the source table updates. | ||
| */ | ||
| @SuppressWarnings("unused") | ||
| public static QueryTable build(@NotNull Table source, |
There was a problem hiding this comment.
There are enough arguments here to warrant switching to a Builder pattern. We could use Immutables to generate the boilerplate builder itself and this method would simply take the result object.
Something like
public static QueryTable of(@NotNull final BookConfig config)and used like this
Pricebok.of(BookConfig.newBuilder()
.thing1()
.thing2()
.build())I realize that there are other factors at play so you might not have time to do this in this review, but we should definitely do this as a follow on.
…k-builder into init_snapshot
No description provided.