CritasWang commented on code in PR #17387:
URL: https://github.com/apache/iotdb/pull/17387#discussion_r3071200614
##########
external-service-impl/rest/src/main/java/org/apache/iotdb/rest/protocol/table/v1/impl/RestApiServiceImpl.java:
##########
@@ -287,7 +286,8 @@ private Statement createStatement(
}
clientSession.setSqlDialect(IClientSession.SqlDialect.TABLE);
- return relationSqlParser.createStatement(sql.getSql(),
ZoneId.systemDefault(), clientSession);
+ return relationSqlParser.createStatement(
Review Comment:
The table REST API (RestApiServiceImpl in table/v1) was also updated to
use session ZoneId, but there are no integration tests for the table model
endpoints with the Time-Zone header.
##########
external-service-impl/rest/src/main/java/org/apache/iotdb/rest/protocol/v1/impl/GrafanaApiServiceImpl.java:
##########
@@ -91,8 +91,8 @@ public Response variables(SQL sql, SecurityContext
securityContext) {
try {
RequestValidationHandler.validateSQL(sql);
- Statement statement =
- StatementGenerator.createStatement(sql.getSql(),
ZoneId.systemDefault());
+ ZoneId zoneId = SESSION_MANAGER.getCurrSession().getZoneId();
Review Comment:
Every service method now does:
ZoneId zoneId = SESSION_MANAGER.getCurrSession().getZoneId();
If SESSION_MANAGER.getCurrSession() returns null (e.g., on the /ping
endpoint which bypasses auth, or an edge case in session lifecycle), this will
throw a NullPointerException. The
old code used ZoneId.systemDefault() which was always safe.
Recommendation: Add a null guard:
IClientSession session = SESSION_MANAGER.getCurrSession();
ZoneId zoneId = (session != null) ? session.getZoneId() :
ZoneId.systemDefault();
--
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]
