This is an automated email from the ASF dual-hosted git repository.
xiangfu0 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 081a013e4f1 Add safe SQL quoting for typed table names (#19327)
081a013e4f1 is described below
commit 081a013e4f1d3745b3f725f8f3e269cd50408986
Author: Xiang Fu <[email protected]>
AuthorDate: Sat Aug 22 20:50:48 2026 -0700
Add safe SQL quoting for typed table names (#19327)
---
.../pinot/sql/parsers/CalciteSqlCompilerTest.java | 14 ++++++++
.../java/org/apache/pinot/spi/utils/SqlUtils.java} | 31 +++++++---------
.../pinot/spi/utils/TimestampIndexUtils.java | 2 +-
.../pinot/spi/utils/builder/TableNameBuilder.java | 42 ++++++++++++++++++++++
...TableNameBuilderTest.java => SqlUtilsTest.java} | 24 ++++++-------
.../pinot/spi/utils/TimestampIndexUtilsTest.java | 17 +++++++++
.../spi/utils/builder/TableNameBuilderTest.java | 41 +++++++++++++++++++++
.../pinot/sql/ddl/reverse/SqlIdentifiers.java | 3 +-
8 files changed, 139 insertions(+), 35 deletions(-)
diff --git
a/pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java
b/pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java
index dcc242e8e31..f3ee5d141b8 100644
---
a/pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java
+++
b/pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java
@@ -34,6 +34,7 @@ import org.apache.pinot.common.request.JoinType;
import org.apache.pinot.common.request.Literal;
import org.apache.pinot.common.request.PinotQuery;
import org.apache.pinot.segment.spi.AggregationFunctionType;
+import org.apache.pinot.spi.utils.builder.TableNameBuilder;
import org.apache.pinot.sql.FilterKind;
import org.apache.pinot.sql.parsers.parser.ParseException;
import org.apache.pinot.sql.parsers.parser.SqlInsertFromFile;
@@ -3326,6 +3327,19 @@ public class CalciteSqlCompilerTest {
Assert.assertEquals(defaultCatalogQuery.getDataSource().getTableName(),
"default.foo");
}
+ @Test
+ public void testQuotedTypedTableNameRoundTrip() {
+ List<String> tableNamesWithType = List.of(
+ "events_OFFLINE",
+ "analytics.events_REALTIME",
+ "db\"name.events\";DROP_TABLE--_OFFLINE");
+ for (String tableNameWithType : tableNamesWithType) {
+ PinotQuery pinotQuery = compileToPinotQuery(
+ "SELECT * FROM " +
TableNameBuilder.quoteTableNameWithType(tableNameWithType));
+ Assert.assertEquals(pinotQuery.getDataSource().getTableName(),
tableNameWithType);
+ }
+ }
+
@Test
public void testInvalidQueryWithSemicolon() {
Assert.expectThrows(SqlCompilationException.class, () ->
compileToPinotQuery(";"));
diff --git
a/pinot-spi/src/test/java/org/apache/pinot/spi/utils/builder/TableNameBuilderTest.java
b/pinot-spi/src/main/java/org/apache/pinot/spi/utils/SqlUtils.java
similarity index 52%
copy from
pinot-spi/src/test/java/org/apache/pinot/spi/utils/builder/TableNameBuilderTest.java
copy to pinot-spi/src/main/java/org/apache/pinot/spi/utils/SqlUtils.java
index c3204cce84e..4fb0f2c40d3 100644
---
a/pinot-spi/src/test/java/org/apache/pinot/spi/utils/builder/TableNameBuilderTest.java
+++ b/pinot-spi/src/main/java/org/apache/pinot/spi/utils/SqlUtils.java
@@ -16,27 +16,20 @@
* specific language governing permissions and limitations
* under the License.
*/
+package org.apache.pinot.spi.utils;
-package org.apache.pinot.spi.utils.builder;
-import com.google.common.collect.ImmutableSet;
-import org.testng.annotations.Test;
-
-import static org.testng.Assert.assertEquals;
-
-
-public class TableNameBuilderTest {
-
- @Test
- public void testGetTableNameVariations() {
-
- assertEquals(TableNameBuilder.getTableNameVariations("tableAbc"),
- ImmutableSet.of("tableAbc", "tableAbc_REALTIME", "tableAbc_OFFLINE"));
-
- assertEquals(TableNameBuilder.getTableNameVariations("tableAbc_REALTIME"),
- ImmutableSet.of("tableAbc", "tableAbc_REALTIME", "tableAbc_OFFLINE"));
+/// Utilities for constructing SQL fragments. This stateless utility is
thread-safe.
+public class SqlUtils {
+ private SqlUtils() {
+ }
- assertEquals(TableNameBuilder.getTableNameVariations("tableAbc_OFFLINE"),
- ImmutableSet.of("tableAbc", "tableAbc_REALTIME", "tableAbc_OFFLINE"));
+ /// Quotes one SQL identifier component with double quotes, escaping
embedded double quotes by doubling them.
+ /// Qualified names must be split by the caller so that each component is
quoted separately.
+ ///
+ /// @param identifier SQL identifier component
+ /// @return Identifier quoted for use in a SQL statement
+ public static String quoteIdentifier(String identifier) {
+ return "\"" + identifier.replace("\"", "\"\"") + "\"";
}
}
diff --git
a/pinot-spi/src/main/java/org/apache/pinot/spi/utils/TimestampIndexUtils.java
b/pinot-spi/src/main/java/org/apache/pinot/spi/utils/TimestampIndexUtils.java
index 66e221a933b..d095d02b6f5 100644
---
a/pinot-spi/src/main/java/org/apache/pinot/spi/utils/TimestampIndexUtils.java
+++
b/pinot-spi/src/main/java/org/apache/pinot/spi/utils/TimestampIndexUtils.java
@@ -190,6 +190,6 @@ public class TimestampIndexUtils {
}
private static String getTransformExpression(String timestampColumn,
TimestampIndexGranularity granularity) {
- return "dateTrunc('" + granularity + "',\"" + timestampColumn + "\")";
+ return "dateTrunc('" + granularity + "'," +
SqlUtils.quoteIdentifier(timestampColumn) + ")";
}
}
diff --git
a/pinot-spi/src/main/java/org/apache/pinot/spi/utils/builder/TableNameBuilder.java
b/pinot-spi/src/main/java/org/apache/pinot/spi/utils/builder/TableNameBuilder.java
index e2f44ca00d2..a9dd0fd5ff6 100644
---
a/pinot-spi/src/main/java/org/apache/pinot/spi/utils/builder/TableNameBuilder.java
+++
b/pinot-spi/src/main/java/org/apache/pinot/spi/utils/builder/TableNameBuilder.java
@@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableSet;
import java.util.Set;
import javax.annotation.Nullable;
import org.apache.pinot.spi.config.table.TableType;
+import org.apache.pinot.spi.utils.SqlUtils;
public class TableNameBuilder {
@@ -117,6 +118,47 @@ public class TableNameBuilder {
return REALTIME.tableHasTypeSuffix(resourceName);
}
+ /// Quotes a typed table name for use as a SQL identifier. When the table
name is database-qualified, the database
+ /// and table components are quoted separately. Embedded double quotes are
escaped by doubling them.
+ ///
+ /// @param tableNameWithType Table name ending in `_OFFLINE` or `_REALTIME`,
optionally prefixed with a database name
+ /// @return Table name quoted for use in a SQL statement
+ /// @throws IllegalArgumentException If the table name is not a valid typed
table resource
+ public static String quoteTableNameWithType(String tableNameWithType) {
+ if (tableNameWithType == null || containsWhitespace(tableNameWithType)) {
+ throw new IllegalArgumentException("Invalid table name with type");
+ }
+
+ int separatorIndex = tableNameWithType.indexOf('.');
+ if (separatorIndex < 0) {
+ validateTableNameWithType(tableNameWithType);
+ return SqlUtils.quoteIdentifier(tableNameWithType);
+ }
+ if (separatorIndex == 0 || separatorIndex !=
tableNameWithType.lastIndexOf('.')) {
+ throw new IllegalArgumentException("Invalid table name with type");
+ }
+
+ String tableNameWithTypeWithoutDatabase =
tableNameWithType.substring(separatorIndex + 1);
+ validateTableNameWithType(tableNameWithTypeWithoutDatabase);
+ return SqlUtils.quoteIdentifier(tableNameWithType.substring(0,
separatorIndex)) + "."
+ + SqlUtils.quoteIdentifier(tableNameWithTypeWithoutDatabase);
+ }
+
+ private static void validateTableNameWithType(String tableNameWithType) {
+ if (!isTableResource(tableNameWithType)) {
+ throw new IllegalArgumentException("Invalid table name with type");
+ }
+ }
+
+ private static boolean containsWhitespace(String value) {
+ for (int i = 0; i < value.length(); i++) {
+ if (Character.isWhitespace(value.charAt(i))) {
+ return true;
+ }
+ }
+ return false;
+ }
+
public static Set<String> getTableNameVariations(String tableName) {
String rawTableName = extractRawTableName(tableName);
String offlineTableName = OFFLINE.tableNameWithType(rawTableName);
diff --git
a/pinot-spi/src/test/java/org/apache/pinot/spi/utils/builder/TableNameBuilderTest.java
b/pinot-spi/src/test/java/org/apache/pinot/spi/utils/SqlUtilsTest.java
similarity index 58%
copy from
pinot-spi/src/test/java/org/apache/pinot/spi/utils/builder/TableNameBuilderTest.java
copy to pinot-spi/src/test/java/org/apache/pinot/spi/utils/SqlUtilsTest.java
index c3204cce84e..9af3cf51fd3 100644
---
a/pinot-spi/src/test/java/org/apache/pinot/spi/utils/builder/TableNameBuilderTest.java
+++ b/pinot-spi/src/test/java/org/apache/pinot/spi/utils/SqlUtilsTest.java
@@ -16,27 +16,23 @@
* specific language governing permissions and limitations
* under the License.
*/
+package org.apache.pinot.spi.utils;
-package org.apache.pinot.spi.utils.builder;
-
-import com.google.common.collect.ImmutableSet;
import org.testng.annotations.Test;
import static org.testng.Assert.assertEquals;
-public class TableNameBuilderTest {
+/// Tests for [SqlUtils].
+public class SqlUtilsTest {
@Test
- public void testGetTableNameVariations() {
-
- assertEquals(TableNameBuilder.getTableNameVariations("tableAbc"),
- ImmutableSet.of("tableAbc", "tableAbc_REALTIME", "tableAbc_OFFLINE"));
-
- assertEquals(TableNameBuilder.getTableNameVariations("tableAbc_REALTIME"),
- ImmutableSet.of("tableAbc", "tableAbc_REALTIME", "tableAbc_OFFLINE"));
-
- assertEquals(TableNameBuilder.getTableNameVariations("tableAbc_OFFLINE"),
- ImmutableSet.of("tableAbc", "tableAbc_REALTIME", "tableAbc_OFFLINE"));
+ public void testQuoteIdentifier() {
+ assertEquals(SqlUtils.quoteIdentifier("column"), "\"column\"");
+ assertEquals(SqlUtils.quoteIdentifier("column with space"), "\"column with
space\"");
+ assertEquals(SqlUtils.quoteIdentifier("database.table"),
"\"database.table\"");
+ assertEquals(SqlUtils.quoteIdentifier("column\"name"),
"\"column\"\"name\"");
+ assertEquals(SqlUtils.quoteIdentifier("column\"; DROP TABLE events; --"),
+ "\"column\"\"; DROP TABLE events; --\"");
}
}
diff --git
a/pinot-spi/src/test/java/org/apache/pinot/spi/utils/TimestampIndexUtilsTest.java
b/pinot-spi/src/test/java/org/apache/pinot/spi/utils/TimestampIndexUtilsTest.java
index c7f4ec4fef3..2adb2ea2e29 100644
---
a/pinot-spi/src/test/java/org/apache/pinot/spi/utils/TimestampIndexUtilsTest.java
+++
b/pinot-spi/src/test/java/org/apache/pinot/spi/utils/TimestampIndexUtilsTest.java
@@ -127,4 +127,21 @@ public class TimestampIndexUtilsTest {
assertEquals(new HashSet<>(rangeIndexColumns), transformColumns);
}
}
+
+ @Test
+ public void testApplyTimestampIndexEscapesQuotedColumn() {
+ String timestampColumn = "ts\"name";
+ TableConfig tableConfig = new
TableConfigBuilder(TableType.OFFLINE).setTableName("testTable").setFieldConfigList(
+ List.of(new FieldConfig.Builder(timestampColumn)
+ .withTimestampConfig(new
TimestampConfig(List.of(TimestampIndexGranularity.DAY))).build())).build();
+ Schema schema = new Schema.SchemaBuilder().setSchemaName("testTable")
+ .addDateTime(timestampColumn, DataType.TIMESTAMP, "TIMESTAMP",
"1:MILLISECONDS").build();
+
+ TimestampIndexUtils.applyTimestampIndex(tableConfig, schema);
+
+ List<TransformConfig> transformConfigs =
tableConfig.getIngestionConfig().getTransformConfigs();
+ assertEquals(transformConfigs.size(), 1);
+ assertEquals(transformConfigs.get(0).getColumnName(), "$ts\"name$DAY");
+ assertEquals(transformConfigs.get(0).getTransformFunction(),
"dateTrunc('DAY',\"ts\"\"name\")");
+ }
}
diff --git
a/pinot-spi/src/test/java/org/apache/pinot/spi/utils/builder/TableNameBuilderTest.java
b/pinot-spi/src/test/java/org/apache/pinot/spi/utils/builder/TableNameBuilderTest.java
index c3204cce84e..dcb0dee00d6 100644
---
a/pinot-spi/src/test/java/org/apache/pinot/spi/utils/builder/TableNameBuilderTest.java
+++
b/pinot-spi/src/test/java/org/apache/pinot/spi/utils/builder/TableNameBuilderTest.java
@@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableSet;
import org.testng.annotations.Test;
import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertThrows;
public class TableNameBuilderTest {
@@ -39,4 +40,44 @@ public class TableNameBuilderTest {
assertEquals(TableNameBuilder.getTableNameVariations("tableAbc_OFFLINE"),
ImmutableSet.of("tableAbc", "tableAbc_REALTIME", "tableAbc_OFFLINE"));
}
+
+ @Test
+ public void testQuoteTableNameWithType() {
+ assertEquals(TableNameBuilder.quoteTableNameWithType("events_OFFLINE"),
"\"events_OFFLINE\"");
+ assertEquals(TableNameBuilder.quoteTableNameWithType("events_REALTIME"),
"\"events_REALTIME\"");
+
assertEquals(TableNameBuilder.quoteTableNameWithType("analytics.events_OFFLINE"),
+ "\"analytics\".\"events_OFFLINE\"");
+
assertEquals(TableNameBuilder.quoteTableNameWithType("analytics.my-table$1_REALTIME"),
+ "\"analytics\".\"my-table$1_REALTIME\"");
+
assertEquals(TableNameBuilder.quoteTableNameWithType("\u6570\u636E\u5E93.\u4E8B\u4EF6_REALTIME"),
+ "\"\u6570\u636E\u5E93\".\"\u4E8B\u4EF6_REALTIME\"");
+ assertEquals(TableNameBuilder.quoteTableNameWithType("_OFFLINE"),
"\"_OFFLINE\"");
+
assertEquals(TableNameBuilder.quoteTableNameWithType("analytics._REALTIME"),
+ "\"analytics\".\"_REALTIME\"");
+ }
+
+ @Test
+ public void testQuoteTableNameWithTypeEscapesEmbeddedQuotes() {
+
assertEquals(TableNameBuilder.quoteTableNameWithType("db\"name.events\";DROP_TABLE--_OFFLINE"),
+ "\"db\"\"name\".\"events\"\";DROP_TABLE--_OFFLINE\"");
+ }
+
+ @Test
+ public void testQuoteTableNameWithTypeRejectsInvalidNames() {
+ String[] invalidTableNames = {
+ null,
+ "",
+ "events",
+ "events_offline",
+ ".events_OFFLINE",
+ "analytics.",
+ "analytics.events_OFFLINE.extra",
+ "analytics..events_REALTIME",
+ "events name_OFFLINE",
+ "events_OFFLINE;SELECT"
+ };
+ for (String invalidTableName : invalidTableNames) {
+ assertThrows(IllegalArgumentException.class, () ->
TableNameBuilder.quoteTableNameWithType(invalidTableName));
+ }
+ }
}
diff --git
a/pinot-sql-ddl/src/main/java/org/apache/pinot/sql/ddl/reverse/SqlIdentifiers.java
b/pinot-sql-ddl/src/main/java/org/apache/pinot/sql/ddl/reverse/SqlIdentifiers.java
index e8cacf01062..4ccd1159608 100644
---
a/pinot-sql-ddl/src/main/java/org/apache/pinot/sql/ddl/reverse/SqlIdentifiers.java
+++
b/pinot-sql-ddl/src/main/java/org/apache/pinot/sql/ddl/reverse/SqlIdentifiers.java
@@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableSet;
import java.util.Locale;
import java.util.Set;
import java.util.regex.Pattern;
+import org.apache.pinot.spi.utils.SqlUtils;
/// Quoting helpers used by the canonical DDL emitter.
@@ -62,7 +63,7 @@ final class SqlIdentifiers {
/// bare otherwise. Embedded double quotes are escaped per SQL convention
(`"` → `""`).
static String quote(String identifier) {
if (mustQuote(identifier)) {
- return "\"" + identifier.replace("\"", "\"\"") + "\"";
+ return SqlUtils.quoteIdentifier(identifier);
}
return identifier;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
