Copilot commented on code in PR #17350:
URL: https://github.com/apache/iotdb/pull/17350#discussion_r2980159600
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/execution/IQueryExecution.java:
##########
@@ -70,6 +70,8 @@ public interface IQueryExecution {
*/
long getTotalExecutionTime();
Review Comment:
The new `getTimeout()` method has no contract documented (unit, meaning of
0/negative/Long.MAX_VALUE, whether it’s per-query or session-level, etc.).
Since this value is now surfaced via `SHOW QUERIES`/information_schema, please
add Javadoc specifying at least the unit (ms) and semantics so callers don’t
misinterpret it.
```suggestion
/**
* Returns the timeout for this query execution in milliseconds.
*
* <p>The returned value applies to this query execution instance only
(per-query timeout).
*
* <p>Semantics of special values:
* <ul>
* <li>{@code timeout > 0}: maximum allowed execution time in
milliseconds for this query.</li>
* <li>{@code timeout <= 0}: no timeout is configured for this query
(unlimited).</li>
* <li>{@code Long.MAX_VALUE}: effectively treated as no timeout.</li>
* </ul>
*
* @return the configured timeout in milliseconds for this query
*/
```
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/schema/column/ColumnHeaderConstant.java:
##########
@@ -645,10 +648,12 @@ private ColumnHeaderConstant() {
public static final List<ColumnHeader> showQueriesColumnHeaders =
ImmutableList.of(
- new ColumnHeader(QUERY_ID, TSDataType.TEXT),
+ new ColumnHeader(QUERY_ID, TSDataType.STRING),
new ColumnHeader(DATA_NODE_ID, TSDataType.INT32),
new ColumnHeader(ELAPSED_TIME, TSDataType.FLOAT),
- new ColumnHeader(STATEMENT, TSDataType.TEXT));
+ new ColumnHeader(STATEMENT, TSDataType.STRING),
+ new ColumnHeader(CLIENT_IP_TREE_MODEL, TSDataType.STRING),
Review Comment:
`showQueriesColumnHeaders` switches string columns (e.g.,
QueryId/Statement/ClientIp) from `TSDataType.TEXT` to `TSDataType.STRING`. This
changes the result schema of the TREE-model `SHOW QUERIES` statement and can
break compatibility with older clients/components that only expect `TEXT` in
show-statement outputs. Consider keeping existing columns as `TEXT` and only
adding the new columns, also as `TEXT`, unless there is a hard requirement to
return `STRING` here.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/execution/operator/source/ShowQueriesOperator.java:
##########
@@ -150,6 +150,8 @@ private TsBlock buildTsBlock() {
(float) (currTime - queryExecution.getStartExecutionTime()) /
1000);
columnBuilders[3].writeBinary(
BytesUtils.valueOf(queryExecution.getExecuteSQL().orElse("UNKNOWN")));
+
columnBuilders[4].writeBinary(BytesUtils.valueOf(queryExecution.getClientHostname()));
+ columnBuilders[5].writeLong(queryExecution.getTimeout());
Review Comment:
This populates the new `ClientIp` column using `getClientHostname()`. If the
value isn’t guaranteed to be an IP address (e.g., could be a hostname),
consider normalizing it to an IP (or renaming the column/accessor) to avoid
misleading output from `SHOW QUERIES`.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/execution/operator/source/relational/InformationSchemaContentSupplierFactory.java:
##########
@@ -283,6 +283,8 @@ protected void constructLine() {
columnBuilders[4].writeBinary(
BytesUtils.valueOf(queryExecution.getExecuteSQL().orElse("UNKNOWN")));
columnBuilders[5].writeBinary(BytesUtils.valueOf(queryExecution.getUser()));
+
columnBuilders[6].writeBinary(BytesUtils.valueOf(queryExecution.getClientHostname()));
+ columnBuilders[7].writeLong(queryExecution.getTimeout());
Review Comment:
The new `client_ip` / `ClientIp` columns are populated from
`queryExecution.getClientHostname()`. The naming suggests an IP address, but
the API returns a “hostname”/`cliHostname`; if this can be a DNS name (or
includes port), the column name is misleading. Either ensure the stored value
is a normalized IP address (and ideally rename the accessor accordingly) or
consider exposing it as `client_host`/`client_hostname` instead.
--
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]
