This is an automated email from the ASF dual-hosted git repository.
FrankChen021 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new fed2713f717 fix: address CodeQL formatting warnings (#19826)
fed2713f717 is described below
commit fed2713f7179b5898b3d64ae19bda88036680287
Author: Frank Chen <[email protected]>
AuthorDate: Wed Aug 5 10:26:18 2026 +0800
fix: address CodeQL formatting warnings (#19826)
* Fix CodeQL formatting warnings
* test: cover SQL lexer state transitions
* test: cover ForkingTaskRunner directory failures
* test: migrate parser token tests to JUnit 5
---
.../druid/indexing/overlord/ForkingTaskRunner.java | 5 +-
.../FilteringCloseableInputRowIteratorTest.java | 4 +-
.../indexing/overlord/ForkingTaskRunnerTest.java | 34 +++++
.../msq/test/MSQTestOverlordServiceClient.java | 2 +-
.../server/log/DefaultRequestLogEventTest.java | 2 +-
sql/src/main/codegen/templates/Parser.jj | 50 +++++--
.../parser/DruidSqlParserImplTokenManagerTest.java | 144 +++++++++++++++++++++
7 files changed, 222 insertions(+), 19 deletions(-)
diff --git
a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/ForkingTaskRunner.java
b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/ForkingTaskRunner.java
index 1c9b13a1175..2ec1faf77b8 100644
---
a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/ForkingTaskRunner.java
+++
b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/ForkingTaskRunner.java
@@ -934,7 +934,7 @@ public class ForkingTaskRunner
FileUtils.mkdirp(attemptDir);
}
catch (IOException e) {
- throw new ISE("Error creating directory", e);
+ throw new ISE(e, "Error creating directory[%s]", attemptDir);
}
int maxAttempt =
Arrays.stream(attemptDir.listFiles(File::isDirectory))
@@ -946,7 +946,7 @@ public class ForkingTaskRunner
FileUtils.mkdirp(attempt);
}
catch (IOException e) {
- throw new ISE("Error creating directory", e);
+ throw new ISE(e, "Error creating directory[%s]", attempt);
}
return maxAttempt + 1;
}
@@ -996,4 +996,3 @@ public class ForkingTaskRunner
}
}
-
diff --git
a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/FilteringCloseableInputRowIteratorTest.java
b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/FilteringCloseableInputRowIteratorTest.java
index e037d9adf72..e28ec5a154c 100644
---
a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/FilteringCloseableInputRowIteratorTest.java
+++
b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/FilteringCloseableInputRowIteratorTest.java
@@ -115,7 +115,7 @@ public class FilteringCloseableInputRowIteratorTest
if (currentIdx % 2 == 0) {
return ROWS.get(currentIdx / 2);
} else {
- throw new ParseException(null, "Parse exception at ", currentIdx);
+ throw new ParseException(null, "Parse exception at [%d]",
currentIdx);
}
}
@@ -198,7 +198,7 @@ public class FilteringCloseableInputRowIteratorTest
if (currentIndex % 2 == 0) {
return currentIndex < numRowsToIterate;
} else {
- throw new ParseException(null, "Parse exception at ", currentIndex);
+ throw new ParseException(null, "Parse exception at [%d]",
currentIndex);
}
}
diff --git
a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/ForkingTaskRunnerTest.java
b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/ForkingTaskRunnerTest.java
index a3e6cec6bd2..aff352ce31e 100644
---
a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/ForkingTaskRunnerTest.java
+++
b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/ForkingTaskRunnerTest.java
@@ -42,6 +42,7 @@ import
org.apache.druid.indexing.seekablestream.supervisor.SeekableStreamSupervi
import org.apache.druid.indexing.worker.config.WorkerConfig;
import org.apache.druid.jackson.DefaultObjectMapper;
import org.apache.druid.java.util.common.FileUtils;
+import org.apache.druid.java.util.common.ISE;
import org.apache.druid.java.util.common.Pair;
import org.apache.druid.java.util.common.RE;
import org.apache.druid.java.util.common.granularity.AllGranularity;
@@ -432,6 +433,39 @@ public class ForkingTaskRunnerTest
);
}
+ @Test
+ public void
testGettingTheNextAttemptDirFailsIfAttemptDirectoryCannotBeCreated() throws
IOException
+ {
+ final File taskDir = temporaryFolder.newFile();
+ final File attemptDir = new File(taskDir, "attempt");
+
+ final ISE exception = Assert.assertThrows(
+ ISE.class,
+ () -> ForkingTaskRunner.getNextAttemptID(taskDir)
+ );
+
+ Assert.assertEquals("Error creating directory[" + attemptDir + "]",
exception.getMessage());
+ Assert.assertTrue(exception.getCause() instanceof IOException);
+ }
+
+ @Test
+ public void testGettingTheNextAttemptDirFailsIfAttemptCannotBeCreated()
throws IOException
+ {
+ final File taskDir = temporaryFolder.newFolder();
+ final File attemptDir = new File(taskDir, "attempt");
+ FileUtils.mkdirp(attemptDir);
+ final File attempt = new File(attemptDir, "1");
+ Assert.assertTrue(attempt.createNewFile());
+
+ final ISE exception = Assert.assertThrows(
+ ISE.class,
+ () -> ForkingTaskRunner.getNextAttemptID(taskDir)
+ );
+
+ Assert.assertEquals("Error creating directory[" + attempt + "]",
exception.getMessage());
+ Assert.assertTrue(exception.getCause() instanceof IOException);
+ }
+
@Test
public void testJavaOptsAndJavaOptsArrayOverride() throws
ExecutionException, InterruptedException,
JsonProcessingException
diff --git
a/multi-stage-query/src/test/java/org/apache/druid/msq/test/MSQTestOverlordServiceClient.java
b/multi-stage-query/src/test/java/org/apache/druid/msq/test/MSQTestOverlordServiceClient.java
index f7b8d31def4..a03298114a6 100644
---
a/multi-stage-query/src/test/java/org/apache/druid/msq/test/MSQTestOverlordServiceClient.java
+++
b/multi-stage-query/src/test/java/org/apache/druid/msq/test/MSQTestOverlordServiceClient.java
@@ -250,7 +250,7 @@ public class MSQTestOverlordServiceClient extends
NoopOverlordClient
{
MSQTestTaskDetails old = taskDetailsByQueryId.get(queryId);
if (old != null) {
- throw DruidException.defensive("There is an existing queryId {}!",
queryId);
+ throw DruidException.defensive("There is an existing queryId [%s]!",
queryId);
}
taskDetailsByQueryId.put(queryId, msqTestTaskDetails);
}
diff --git
a/server/src/test/java/org/apache/druid/server/log/DefaultRequestLogEventTest.java
b/server/src/test/java/org/apache/druid/server/log/DefaultRequestLogEventTest.java
index 9b4807f9ab1..01c01410b22 100644
---
a/server/src/test/java/org/apache/druid/server/log/DefaultRequestLogEventTest.java
+++
b/server/src/test/java/org/apache/druid/server/log/DefaultRequestLogEventTest.java
@@ -180,7 +180,7 @@ public class DefaultRequestLogEventTest
Assert.assertEquals(expected, observedEventMap);
Assert.assertEquals(
StringUtils.format(
-
"{\"feed\":\"test\",\"timestamp\":\"2019-12-12T03:01:00.000Z\",\"service\":\"druid-service\",\"host\":\"127.0.0.1\",\"remoteAddr\":\"127.0.0.1\",\"queryStats\":{\"sqlQuery/time\":13,\"sqlQuery/planningTimeMs\":1,\"sqlQuery/bytes\":10,\"success\":true,\"identity\":\"allowAll\"},\"sqlQueryContext\":{},\"sql\":\"select
* from foo where x =
?\",\"sqlParameters\":[{\"type\":\"BIGINT\",\"value\":1234}]}",
+
"{\"feed\":\"test\",\"timestamp\":\"%s\",\"service\":\"druid-service\",\"host\":\"127.0.0.1\",\"remoteAddr\":\"127.0.0.1\",\"queryStats\":{\"sqlQuery/time\":13,\"sqlQuery/planningTimeMs\":1,\"sqlQuery/bytes\":10,\"success\":true,\"identity\":\"allowAll\"},\"sqlQueryContext\":{},\"sql\":\"select
* from foo where x =
?\",\"sqlParameters\":[{\"type\":\"BIGINT\",\"value\":1234}]}",
timestamp
),
new DefaultObjectMapper().writeValueAsString(observedEventMap)
diff --git a/sql/src/main/codegen/templates/Parser.jj
b/sql/src/main/codegen/templates/Parser.jj
index 3eb4f041c94..3f39b1f050e 100644
--- a/sql/src/main/codegen/templates/Parser.jj
+++ b/sql/src/main/codegen/templates/Parser.jj
@@ -22,6 +22,7 @@ options {
STATIC = false;
IGNORE_CASE = true;
UNICODE_INPUT = true;
+ COMMON_TOKEN_ACTION = true;
}
@@ -8310,13 +8311,13 @@ SqlPostfixOperator PostfixRowOperator() :
| < DEFINED: "DEFINED" >
| < DEFINER: "DEFINER" >
| < DEGREE: "DEGREE" >
-| < DELETE: "DELETE" > { beforeTableName(); }
+| < DELETE: "DELETE" >
| < DENSE_RANK: "DENSE_RANK" >
| < DEPTH: "DEPTH" >
| < DEREF: "DEREF" >
| < DERIVED: "DERIVED" >
| < DESC: "DESC" >
-| < DESCRIBE: "DESCRIBE" > { beforeTableName(); }
+| < DESCRIBE: "DESCRIBE" >
| < DESCRIPTION: "DESCRIPTION" >
| < DESCRIPTOR: "DESCRIPTOR" >
| < DETERMINISTIC: "DETERMINISTIC" >
@@ -8378,7 +8379,7 @@ SqlPostfixOperator PostfixRowOperator() :
| < FRAME_ROW: "FRAME_ROW" >
| < FREE: "FREE" >
| < FRIDAY: "FRIDAY" >
-| < FROM: "FROM" > { beforeTableName(); }
+| < FROM: "FROM" >
| < FULL: "FULL" >
| < FUNCTION: "FUNCTION" >
| < FUSION: "FUSION" >
@@ -8420,7 +8421,7 @@ SqlPostfixOperator PostfixRowOperator() :
| < INOUT: "INOUT" >
| < INPUT: "INPUT" >
| < INSENSITIVE: "INSENSITIVE" >
-| < INSERT: "INSERT" > { beforeTableName(); }
+| < INSERT: "INSERT" >
| < INSTANCE: "INSTANCE" >
| < INSTANTIABLE: "INSTANTIABLE" >
| < INT: "INT" >
@@ -8435,7 +8436,7 @@ SqlPostfixOperator PostfixRowOperator() :
| < ISOYEAR: "ISOYEAR" >
| < ISOLATION: "ISOLATION" >
| < JAVA: "JAVA" >
-| < JOIN: "JOIN" > { beforeTableName(); }
+| < JOIN: "JOIN" >
| < JSON: "JSON" >
| < JSON_ARRAY: "JSON_ARRAY">
| < JSON_ARRAYAGG: "JSON_ARRAYAGG">
@@ -8484,7 +8485,7 @@ SqlPostfixOperator PostfixRowOperator() :
| < MEASURE: "MEASURE" >
| < MEASURES: "MEASURES" >
| < MEMBER: "MEMBER" >
-| < MERGE: "MERGE" > { beforeTableName(); }
+| < MERGE: "MERGE" >
| < MESSAGE_LENGTH: "MESSAGE_LENGTH" >
| < MESSAGE_OCTET_LENGTH: "MESSAGE_OCTET_LENGTH" >
| < MESSAGE_TEXT: "MESSAGE_TEXT" >
@@ -8669,7 +8670,7 @@ SqlPostfixOperator PostfixRowOperator() :
| < SECTION: "SECTION" >
| < SECURITY: "SECURITY" >
| < SEEK: "SEEK" >
-| < SELECT: "SELECT" > { afterTableName(); }
+| < SELECT: "SELECT" >
| < SELF: "SELF" >
| < SENSITIVE: "SENSITIVE" >
| < SEPARATOR: "SEPARATOR" >
@@ -8679,7 +8680,7 @@ SqlPostfixOperator PostfixRowOperator() :
| < SERVER_NAME: "SERVER_NAME" >
| < SESSION: "SESSION" >
| < SESSION_USER: "SESSION_USER" >
-| < SET: "SET" > { afterTableName(); }
+| < SET: "SET" >
| < SETS: "SETS" >
| < SET_MINUS: "MINUS">
| < SHOW: "SHOW" >
@@ -8771,7 +8772,7 @@ SqlPostfixOperator PostfixRowOperator() :
| < SYSTEM: "SYSTEM" >
| < SYSTEM_TIME: "SYSTEM_TIME" >
| < SYSTEM_USER: "SYSTEM_USER" >
-| < TABLE: "TABLE" > { beforeTableName(); }
+| < TABLE: "TABLE" >
| < TABLE_NAME: "TABLE_NAME" >
| < TABLESAMPLE: "TABLESAMPLE" >
| < TEMPORARY: "TEMPORARY" >
@@ -8826,7 +8827,7 @@ SqlPostfixOperator PostfixRowOperator() :
| < UNNAMED: "UNNAMED" >
| < UNNEST: "UNNEST" >
| < UNSIGNED: "UNSIGNED" >
-| < UPDATE: "UPDATE" > { beforeTableName(); }
+| < UPDATE: "UPDATE" >
| < UPPER: "UPPER" >
| < UPSERT: "UPSERT" >
| < USAGE: "USAGE" >
@@ -8841,7 +8842,7 @@ SqlPostfixOperator PostfixRowOperator() :
| < UTF32: "UTF32" >
| < UUID: "UUID" >
| < VALUE: "VALUE" >
-| < VALUES: "VALUES" > { afterTableName(); }
+| < VALUES: "VALUES" >
| < VALUE_OF: "VALUE_OF" >
| < VAR_POP: "VAR_POP" >
| < VAR_SAMP: "VAR_SAMP" >
@@ -9106,6 +9107,31 @@ TOKEN_MGR_DECLS : {
}
}
+ void CommonTokenAction(final Token token) {
+ switch (token.kind) {
+ case DELETE:
+ case DESCRIBE:
+ case FROM:
+ case INSERT:
+ case JOIN:
+ case MERGE:
+ case TABLE:
+ case UPDATE:
+ beforeTableName();
+ break;
+ case SELECT:
+ case SET:
+ case VALUES:
+ afterTableName();
+ break;
+ case HYPHENATED_IDENTIFIER:
+ popState();
+ break;
+ default:
+ break;
+ }
+ }
+
<#if
(parser.includeAdditionalDeclarations!default.parser.includeAdditionalDeclarations)>
<#include "/@includes/tokenManagerDeclarations.ftl" />
</#if>
@@ -9254,7 +9280,7 @@ MORE :
// Per BigQuery: "Project IDs must contain 6-63 lowercase letters, digits,
// or dashes. IDs must start with a letter and may not end with a dash."
// We do not restrict length, or prevent identifiers from ending in a dash.
- < HYPHENATED_IDENTIFIER: <LETTER> (<LETTER>|<DIGIT>|"-")* > { popState(); }
+ < HYPHENATED_IDENTIFIER: <LETTER> (<LETTER>|<DIGIT>|"-")* >
}
<DEFAULT, DQID, BTID, BQID> TOKEN :
diff --git
a/sql/src/test/java/org/apache/druid/sql/calcite/parser/DruidSqlParserImplTokenManagerTest.java
b/sql/src/test/java/org/apache/druid/sql/calcite/parser/DruidSqlParserImplTokenManagerTest.java
new file mode 100644
index 00000000000..ecb2a9e2013
--- /dev/null
+++
b/sql/src/test/java/org/apache/druid/sql/calcite/parser/DruidSqlParserImplTokenManagerTest.java
@@ -0,0 +1,144 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.sql.calcite.parser;
+
+import org.apache.calcite.avatica.util.Quoting;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlSelect;
+import org.apache.calcite.sql.parser.SqlParseException;
+import org.apache.calcite.sql.parser.SqlParser;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import java.io.StringReader;
+
+public class DruidSqlParserImplTokenManagerTest
+{
+ @Test
+ public void testBeforeTableNameActionsAllowHyphenatedIdentifier()
+ {
+ final String[] keywords = {
+ "DELETE",
+ "DESCRIBE",
+ "FROM",
+ "INSERT",
+ "JOIN",
+ "MERGE",
+ "TABLE",
+ "UPDATE"
+ };
+ final int[] tokenKinds = {
+ DruidSqlParserImplConstants.DELETE,
+ DruidSqlParserImplConstants.DESCRIBE,
+ DruidSqlParserImplConstants.FROM,
+ DruidSqlParserImplConstants.INSERT,
+ DruidSqlParserImplConstants.JOIN,
+ DruidSqlParserImplConstants.MERGE,
+ DruidSqlParserImplConstants.TABLE,
+ DruidSqlParserImplConstants.UPDATE
+ };
+
+ for (int i = 0; i < keywords.length; i++) {
+ final DruidSqlParserImplTokenManager tokenManager =
createBigQueryTokenManager(
+ keywords[i] + " foo-bar abcxyz"
+ );
+
+ assertNextToken(tokenManager, tokenKinds[i],
DruidSqlParserImplConstants.BQHID);
+ assertNextToken(
+ tokenManager,
+ DruidSqlParserImplConstants.HYPHENATED_IDENTIFIER,
+ DruidSqlParserImplConstants.BQID
+ );
+ assertNextToken(tokenManager, DruidSqlParserImplConstants.IDENTIFIER,
DruidSqlParserImplConstants.BQID);
+ assertNextToken(tokenManager, DruidSqlParserImplConstants.EOF,
DruidSqlParserImplConstants.BQID);
+ Assertions.assertTrue(tokenManager.lexicalStateStack.isEmpty());
+ }
+ }
+
+ @Test
+ public void testAfterTableNameActionsRestoreBigQueryIdentifierState()
+ {
+ final String[] keywords = {"SELECT", "SET", "VALUES"};
+ final int[] tokenKinds = {
+ DruidSqlParserImplConstants.SELECT,
+ DruidSqlParserImplConstants.SET,
+ DruidSqlParserImplConstants.VALUES
+ };
+
+ for (int i = 0; i < keywords.length; i++) {
+ final DruidSqlParserImplTokenManager tokenManager =
createBigQueryTokenManager(
+ "FROM " + keywords[i] + " abcxyz"
+ );
+
+ assertNextToken(tokenManager, DruidSqlParserImplConstants.FROM,
DruidSqlParserImplConstants.BQHID);
+ assertNextToken(tokenManager, tokenKinds[i],
DruidSqlParserImplConstants.BQID);
+ assertNextToken(tokenManager, DruidSqlParserImplConstants.IDENTIFIER,
DruidSqlParserImplConstants.BQID);
+ Assertions.assertTrue(tokenManager.lexicalStateStack.isEmpty());
+ }
+ }
+
+ @Test
+ public void testCommentPreservesTableNameState()
+ {
+ final DruidSqlParserImplTokenManager tokenManager =
createBigQueryTokenManager(
+ "FROM /* comment */ foo-bar"
+ );
+
+ assertNextToken(tokenManager, DruidSqlParserImplConstants.FROM,
DruidSqlParserImplConstants.BQHID);
+ final Token tableName = assertNextToken(
+ tokenManager,
+ DruidSqlParserImplConstants.HYPHENATED_IDENTIFIER,
+ DruidSqlParserImplConstants.BQID
+ );
+
+ Assertions.assertNotNull(tableName.specialToken);
+ Assertions.assertEquals(DruidSqlParserImplConstants.MULTI_LINE_COMMENT,
tableName.specialToken.kind);
+ Assertions.assertTrue(tokenManager.lexicalStateStack.isEmpty());
+ }
+
+ @Test
+ public void testBigQueryHyphenatedTableNameParses() throws SqlParseException
+ {
+ final SqlParser.Config parserConfig =
DruidSqlParser.PARSER_CONFIG.withQuoting(Quoting.BACK_TICK_BACKSLASH);
+ final SqlSelect select = (SqlSelect) SqlParser.create("SELECT * FROM
foo-bar", parserConfig).parseStmt();
+
+ Assertions.assertEquals("foo-bar", ((SqlIdentifier)
select.getFrom()).getSimple());
+ }
+
+ private static DruidSqlParserImplTokenManager
createBigQueryTokenManager(final String input)
+ {
+ return new DruidSqlParserImplTokenManager(
+ new SimpleCharStream(new StringReader(input)),
+ DruidSqlParserImplConstants.BQID
+ );
+ }
+
+ private static Token assertNextToken(
+ final DruidSqlParserImplTokenManager tokenManager,
+ final int expectedKind,
+ final int expectedLexicalState
+ )
+ {
+ final Token token = tokenManager.getNextToken();
+ Assertions.assertEquals(expectedKind, token.kind);
+ Assertions.assertEquals(expectedLexicalState, tokenManager.curLexState);
+ return token;
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
