cryptoe commented on code in PR #15495:
URL: https://github.com/apache/druid/pull/15495#discussion_r1428124735
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerImpl.java:
##########
@@ -1268,6 +1296,10 @@ private Int2ObjectMap<List<SegmentIdWithShardSpec>>
makeSegmentGeneratorWorkerFa
{
final Int2ObjectMap<List<SegmentIdWithShardSpec>> retVal = new
Int2ObjectAVLTreeMap<>();
+ if (segmentsToGenerate.isEmpty()) {
Review Comment:
If the segmentsToGenerate is empty, shoudn't we check if `failOnEmptyInsert`
is set. If yes throw an error.
given that the code we would short circuit throwing the exception in the
normal code flow, but logically doesn't it make sense to add a defensive check
here ?
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerImpl.java:
##########
@@ -957,7 +961,8 @@ private List<SegmentIdWithShardSpec>
generateSegmentIdsWithShardSpecs(
final RowSignature signature,
final ClusterBy clusterBy,
final ClusterByPartitions partitionBoundaries,
- final boolean mayHaveMultiValuedClusterByFields
+ final boolean mayHaveMultiValuedClusterByFields,
+ final Boolean isStageOutputEmpty
) throws IOException
{
if (destination.isReplaceTimeChunks()) {
Review Comment:
Since generateSegmentIds would only be used for ingestion/replace, IMHO we
can do a common check in this case but I will leave it upto @abhishekrb19 .
--
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]
