abhishekrb19 commented on code in PR #16141:
URL: https://github.com/apache/druid/pull/16141#discussion_r1534875745


##########
server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataQuery.java:
##########
@@ -312,53 +327,91 @@ public int markSegments(final Collection<SegmentId> 
segmentIds, final boolean us
   }
 
   /**
-   * Marks all used segments that are *fully contained by* a particular 
interval as unused.
+   * Marks all used segments that are <b>fully contained by</b> a particular 
interval as unused.
    *
-   * @return the number of segments actually modified.
+   * @return Number of segments updated.
    */
   public int markSegmentsUnused(final String dataSource, final Interval 
interval)
+  {
+    return markSegmentsUnused(dataSource, interval, null);
+  }
+
+  /**
+   * Marks all used segments that are <b>fully contained by</b> a particular 
interval filtered by an optional list of versions
+   * as unused.
+   *
+   * @return Number of segments updated.
+   */
+  public int markSegmentsUnused(final String dataSource, final Interval 
interval, @Nullable final List<String> versions)
   {
     if (Intervals.isEternity(interval)) {
-      return handle
-          .createStatement(
-              StringUtils.format(
-                  "UPDATE %s SET used=:used, used_status_last_updated = 
:used_status_last_updated "
-                  + "WHERE dataSource = :dataSource AND used = true",
-                  dbTables.getSegmentsTable()
-              )
+      final StringBuilder sb = new StringBuilder();
+      sb.append(
+          StringUtils.format(
+              "UPDATE %s SET used=:used, used_status_last_updated = 
:used_status_last_updated "
+              + "WHERE dataSource = :dataSource AND used = true",
+              dbTables.getSegmentsTable()
           )
+      );
+
+      final boolean hasVersions = !CollectionUtils.isNullOrEmpty(versions);

Review Comment:
   Yeah, good point. It makes sense to treat them differently at the DB level 
since it's also used by other internal calls, not just by the user-facing APIs. 
I quickly checked the usages, and it's either null or a non-empty set, so we 
should be good as-is. I will address your comment in a follow-up change and 
call out any difference in the javadocs as needed.



-- 
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]

Reply via email to