Jackie-Jiang commented on code in PR #16721:
URL: https://github.com/apache/pinot/pull/16721#discussion_r2310880597


##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/BaseClusterIntegrationTestSet.java:
##########
@@ -445,11 +447,57 @@ public void testVirtualColumnQueries() {
     }
 
     // Check that the virtual columns work as expected (throws no exceptions)
-    getPinotConnection().execute("select $docId, $segmentName, $hostName from 
mytable");
-    getPinotConnection().execute("select $docId, $segmentName, $hostName from 
mytable where $docId < 5 limit 50");
-    getPinotConnection().execute("select $docId, $segmentName, $hostName from 
mytable where $docId = 5 limit 50");
-    getPinotConnection().execute("select $docId, $segmentName, $hostName from 
mytable where $docId > 19998 limit 50");
-    getPinotConnection().execute("select max($docId) from mytable group by 
$segmentName");
+    getPinotConnection().execute("select $docId, $segmentName, $hostName, 
$partitionId from mytable");
+    getPinotConnection().execute(
+        "select $docId, $segmentName, $hostName, $partitionId from mytable 
where $docId < 5 limit 50");
+    getPinotConnection().execute(
+        "select $docId, $segmentName, $hostName, $partitionId from mytable 
where $docId = 5 limit 50");
+    getPinotConnection().execute(
+        "select $docId, $segmentName, $hostName, $partitionId from mytable 
where $docId > 19998 limit 50");
+    testPartitionIdVirtualColumn();
+  }
+
+  /**
+   * Test that the $partitionId virtual column returns valid partition IDs.
+   * Realtime segments (LLCSegmentName or UploadedRealtimeSegmentName) should 
return partition IDs extracted from
+   * segment names,
+   * while offline segments should return hash-based partition IDs (0-9999).

Review Comment:
   Does this still apply?



##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/virtualcolumn/VirtualColumnProviderFactory.java:
##########
@@ -58,5 +59,15 @@ public static void 
addBuiltInVirtualColumnsToSegmentSchema(Schema schema, String
       schema.addField(new DimensionFieldSpec(BuiltInVirtualColumn.SEGMENTNAME, 
FieldSpec.DataType.STRING, true,
           DefaultNullValueVirtualColumnProvider.class, segmentName));
     }
+
+    if (!schema.hasColumn(BuiltInVirtualColumn.PARTITIONID)) {
+      // Try to extract partition ID from segment name, fall back to -1 as 
default for offline segments
+      Integer partitionId = 
SegmentUtils.getPartitionIdFromSegmentName(segmentName);

Review Comment:
   This is not very useful because this info can also be extracted from the 
`$segmentName`. Can you try to pass in the segment metadata and use the actual 
partition id stored in the metadata?



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