[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #9117: Emit a server side metric to identify if MV columns are present as part of the selection order by queries

[GitBox <gi...@apache.org>]

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


##########
pinot-common/src/main/java/org/apache/pinot/common/utils/DataTable.java:
##########
@@ -123,7 +123,12 @@ enum MetadataKey {
     NUM_SEGMENTS_PRUNED_BY_LIMIT("numSegmentsPrunedByLimit", MetadataValueType.INT),
     NUM_SEGMENTS_PRUNED_BY_VALUE("numSegmentsPrunedByValue", MetadataValueType.INT),
     EXPLAIN_PLAN_NUM_EMPTY_FILTER_SEGMENTS("explainPlanNumEmptyFilterSegments", MetadataValueType.INT),
-    EXPLAIN_PLAN_NUM_MATCH_ALL_FILTER_SEGMENTS("explainPlanNumMatchAllFilterSegments", MetadataValueType.INT);
+    EXPLAIN_PLAN_NUM_MATCH_ALL_FILTER_SEGMENTS("explainPlanNumMatchAllFilterSegments", MetadataValueType.INT),
+    // TODO: Add a mechanism to pass generic metadata without adding them to the DataTable's metadata
+    // This metadata field is only required for emitting a metric to identify whether queries using MV columns (as
+    // identifiers or via transform) are present in the Selection Order-By queries. This was the only place where
+    // such a metadata could be added in the existing interfaces even though we don't need this on the Broker side.
+    QUERY_HAS_MV_SELECTION_ORDER_BY("queryHasMVSelectionOrderBy", MetadataValueType.STRING);

Review Comment:
   I'd suggest not adding this because this is a temporary step to catch the bad queries, and we need to remove this later. If we add it as a `DataTable.MetadataKey`, we cannot remove it in the future. See the javadoc for this enum for why it cannot be removed



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org