imply-cheddar commented on code in PR #13085:
URL: https://github.com/apache/druid/pull/13085#discussion_r997618452
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/querykit/BaseLeafFrameProcessor.java:
##########
@@ -146,19 +147,31 @@ protected SegmentReference mapSegment(final Segment
segment)
private boolean initializeSegmentMapFn(final IntSet readableInputs)
{
+ final AtomicLong cpuAccumulator = new AtomicLong();
if (segmentMapFn != null) {
return true;
} else if (broadcastJoinHelper == null) {
segmentMapFn = Function.identity();
return true;
} else {
- final boolean retVal =
broadcastJoinHelper.buildBroadcastTablesIncrementally(readableInputs);
-
- if (retVal) {
- segmentMapFn = broadcastJoinHelper.makeSegmentMapFn(query);
+ if (query.getDataSource() instanceof InputNumberDataSource) {
+ final boolean retVal =
broadcastJoinHelper.buildBroadcastTablesIncrementally(readableInputs);
+ if (retVal) {
+ InputNumberDataSource inputNumberDataSource =
(InputNumberDataSource) query.getDataSource();
+ // The InputNumberDataSource requires a BroadcastJoinHelper to be
able to create its
+ // segment map function. It would be a lot better if the
InputNumberDataSource actually
+ // had a way to get that injected into it on its own, but the
relationship between these objects
+ // was figured out during a refactor and using a setter here seemed
like the least-bad way to
+ // make progress on the refactor without breaking functionality.
Hopefully, some future
+ // developer will move this away from a setter.
+ inputNumberDataSource.setBroadcastJoinHelper(broadcastJoinHelper);
+ segmentMapFn = inputNumberDataSource.createSegmentMapFunction(query,
cpuAccumulator);
+ }
+ return retVal;
+ } else {
+ segmentMapFn = Function.identity();
Review Comment:
What I'm trying to say is, if there's not a test that can exercise the
corner that is being mentioned, then the corner doesn't exist. If the corner
does exist, there should be a test that exercises it which is broken. So, we
are in 1 of 3 states:
1) There should be a test that's breaking, but there isn't yet. In that
case, let's create the test and then talk about solutions.
2) There is a test that exercises the corner, but it's passing. In this
case, the corner doesn't actually exist and we do not need further code changes.
3) There is no test that generates the highlighted state because something
upstream from this code is actually preventing that state from ever existing.
So, it is impossible to actually get to the bad state. In this case, the
corner doesn't actually exist, no code changes required.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
