Copilot commented on code in PR #13940:
URL: https://github.com/apache/skywalking/pull/13940#discussion_r3536190395
##########
oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/mqe/rt/MQEVisitor.java:
##########
@@ -220,11 +235,49 @@ public ExpressionResult
visitMetric(MQEParser.MetricContext ctx) {
}
}
+ /**
+ * Reject a top_n attribute condition whose key is not a queryable column
of the metric, before the
+ * request reaches storage. Attribute columns (attr0..attrN) exist only on
decorated OAL metrics and
+ * the MAL meter base; metrics such as relations or database/cache/mq
access carry none, so passing the
+ * condition to the storage engine surfaces a raw backend exception
instead of a query result. Failing
+ * here turns that into a descriptive MQE error via {@link
#getErrorResult(String)}.
+ *
+ * @param metricName the queried metric name
+ * @param attrConditions the attribute conditions parsed from the top_n
function
+ * @throws IllegalExpressionException if an attribute key is not a
queryable column of the metric
+ */
+ void checkAttributes(String metricName,
+ List<AttrCondition> attrConditions) throws
IllegalExpressionException {
+ if (attrConditions == null || attrConditions.isEmpty()) {
+ return;
+ }
+ Optional<Model> model = getModelManager().allModels().stream()
+ .filter(m ->
m.getName().equals(metricName))
+ .findFirst();
+ if (model.isEmpty()) {
+ // The metric existence is already validated via
ValueColumnMetadata; a missing model here is
+ // unexpected, so leave the condition untouched rather than reject
a possibly-valid query.
+ return;
+ }
+ Set<String> queryableColumns = model.get().getColumns().stream()
+ .filter(ModelColumn::shouldIndex)
+ .map(column ->
column.getColumnName().getName())
+ .collect(Collectors.toSet());
+ for (AttrCondition attr : attrConditions) {
+ if (!queryableColumns.contains(attr.getKey())) {
+ throw new IllegalExpressionException(
+ "The attribute [" + attr.getKey() + "] is not a queryable
column of metric [" + metricName
+ + "], so it can not be used as a top_n condition.");
Review Comment:
The exception message uses "can not"; standard English is "cannot". Fixing
this improves the user-facing MQE error text.
--
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]
