Copilot commented on code in PR #17357:
URL: https://github.com/apache/pinot/pull/17357#discussion_r3062924802
##########
pinot-core/src/main/java/org/apache/pinot/core/query/executor/sql/SqlQueryExecutor.java:
##########
@@ -121,11 +167,251 @@ public BrokerResponse
executeDMLStatement(SqlNodeAndOptions sqlNodeAndOptions,
return result;
}
+ /**
+ * Execute metadata (SHOW) statements.
+ */
+ public BrokerResponse executeMetadataStatement(SqlNodeAndOptions
sqlNodeAndOptions,
+ @Nullable Map<String, String> headers) {
+ BrokerResponseNative result = new BrokerResponseNative();
+ Map<String, String> requestHeaders = copyHeaders(headers);
+ try {
+ ResultTable resultTable =
+ buildMetadataResult(sqlNodeAndOptions.getSqlNode(),
sqlNodeAndOptions.getOptions(), requestHeaders);
+ result.setResultTable(resultTable);
+ } catch (Exception e) {
+ result.addException(
+ new QueryProcessingException(QueryErrorCode.QUERY_EXECUTION,
e.getClass().getSimpleName() + ": "
+ + (e.getMessage() != null ? e.getMessage() : "(no message)")));
+ }
+ return result;
+ }
+
private MinionClient getMinionClient() {
// NOTE: using null auth provider here as auth headers injected by caller
in "executeDMLStatement()"
if (_helixManager != null) {
return new MinionClient(getControllerBaseUrl(_helixManager), null);
}
return new MinionClient(_controllerUrl, null);
}
+
+ private ResultTable buildMetadataResult(SqlNode sqlNode, Map<String, String>
options, Map<String, String> headers)
+ throws Exception {
+ if (sqlNode instanceof SqlShowDatabases) {
+ return toSingleStringColumnResult("databaseName",
fetchDatabases(headers));
+ }
+ if (sqlNode instanceof SqlShowTables) {
+ SqlShowTables showTables = (SqlShowTables) sqlNode;
+ String database = resolveDatabase(showTables.getDatabaseName(), options,
headers);
+ List<String> tables = stripDatabasePrefix(fetchTables(database,
headers), database);
+ return toSingleStringColumnResult("tableName", tables);
+ }
+ if (sqlNode instanceof SqlShowSchemas) {
+ SqlShowSchemas showSchemas = (SqlShowSchemas) sqlNode;
+ String database = resolveDatabase(null, options, headers);
+ List<String> schemas = stripDatabasePrefix(fetchSchemas(database,
headers), database);
+ String likePattern = getLikePattern(showSchemas.getLikePattern());
+ schemas = applyLikeFilter(schemas, likePattern);
+ return toSingleStringColumnResult("schemaName", schemas);
+ }
+ throw new UnsupportedOperationException("Unsupported METADATA SqlKind - "
+ sqlNode.getKind());
+ }
+
+ private ResultTable toSingleStringColumnResult(String columnName,
List<String> values) {
+ DataSchema dataSchema = new DataSchema(new String[]{columnName}, new
ColumnDataType[]{ColumnDataType.STRING});
+ List<Object[]> rows = new ArrayList<>(values.size());
+ for (String value : values) {
+ rows.add(new Object[]{value});
+ }
+ return new ResultTable(dataSchema, rows);
+ }
+
+ private String resolveDatabase(@Nullable SqlIdentifier explicitDatabase,
Map<String, String> options,
+ Map<String, String> headers)
+ throws DatabaseConflictException {
+ String databaseFromSql = explicitDatabase != null ?
explicitDatabase.toString() : null;
+ String databaseFromOptions = options.get(CommonConstants.DATABASE);
+ String databaseFromHeaders = getHeaderValue(headers,
CommonConstants.DATABASE);
+
+ if (databaseFromSql != null) {
+ if (databaseFromOptions != null &&
!databaseFromSql.equalsIgnoreCase(databaseFromOptions)) {
+ StringBuilder message = new StringBuilder("Database name
'").append(databaseFromSql)
+ .append("' from statement does not match database name
'").append(databaseFromOptions)
+ .append("' from query options");
+ if (databaseFromHeaders != null) {
+ message.append(" (request header database name:
'").append(databaseFromHeaders).append("')");
+ }
+ throw new DatabaseConflictException(message.toString());
+ }
+ if (databaseFromHeaders != null &&
!databaseFromSql.equalsIgnoreCase(databaseFromHeaders)) {
+ StringBuilder message = new StringBuilder("Database name
'").append(databaseFromSql)
+ .append("' from statement does not match database name
'").append(databaseFromHeaders)
+ .append("' from request header");
+ if (databaseFromOptions != null) {
+ message.append(" (query options database name:
'").append(databaseFromOptions).append("')");
+ }
+ throw new DatabaseConflictException(message.toString());
+ }
+ return databaseFromSql;
+ }
+
+ if (databaseFromOptions != null && databaseFromHeaders != null
+ && !databaseFromOptions.equalsIgnoreCase(databaseFromHeaders)) {
+ throw new DatabaseConflictException(
+ "Database name mismatch between query options ('" +
databaseFromOptions + "') and request header ('"
+ + databaseFromHeaders + "')");
+ }
+
+ return databaseFromOptions != null ? databaseFromOptions
+ : databaseFromHeaders != null ? databaseFromHeaders :
CommonConstants.DEFAULT_DATABASE;
+ }
+
+ private List<String> stripDatabasePrefix(List<String> names, String
database) {
+ if (names.isEmpty()) {
+ return names;
+ }
+ List<String> result = new ArrayList<>(names.size());
+ for (String name : names) {
+ result.add(DatabaseUtils.isPartOfDatabase(name, database)
+ ? DatabaseUtils.removeDatabasePrefix(name, database) : name);
+ }
+ return result;
+ }
+
+ private List<String> applyLikeFilter(List<String> values, @Nullable String
likePattern) {
+ if (StringUtils.isEmpty(likePattern)) {
+ return values;
+ }
+ Pattern pattern = buildLikeRegex(likePattern);
+ List<String> result = new ArrayList<>();
+ for (String value : values) {
+ if (pattern.matcher(value).matches()) {
+ result.add(value);
+ }
+ }
+ return result;
+ }
+
+ private Pattern buildLikeRegex(String likePattern) {
+ StringBuilder regex = new StringBuilder();
+ boolean escaped = false;
+ for (char c : likePattern.toCharArray()) {
+ if (escaped) {
+ regex.append(Pattern.quote(String.valueOf(c)));
+ escaped = false;
+ } else if (c == '\\') {
+ escaped = true;
+ } else if (c == '%') {
Review Comment:
`buildLikeRegex` treats `\` as an escape prefix, but if the LIKE pattern
ends with a trailing backslash (e.g. `'foo\'`), `escaped` remains `true` at
end-of-loop and the backslash is silently dropped. This makes the filter match
strings as if the final `\` wasn’t present. Consider handling the
trailing-escape case explicitly (e.g., treat a terminal `\` as a literal
backslash or throw a parsing/execution error) to align with documented
`\`-escape semantics.
##########
pinot-common/src/main/codegen/includes/parserImpls.ftl:
##########
@@ -109,3 +109,49 @@ SqlNode SqlPhysicalExplain() :
nDynamicParams);
}
}
+
+SqlNode SqlShowDatabases() :
+{
+ SqlParserPos pos;
+}
+{
+ <SHOW> { pos = getPos(); }
+ <DATABASES>
+ {
+ return new SqlShowDatabases(pos);
+ }
+}
+
+SqlNode SqlShowTables() :
+{
+ SqlParserPos pos;
+ SqlIdentifier dbName = null;
+}
+{
+ <SHOW> { pos = getPos(); }
+ <TABLES>
+ [
+ <FROM>
+ dbName = CompoundIdentifier()
Review Comment:
The grammar for `SHOW TABLES FROM <databaseName>` currently parses the
database name via `CompoundIdentifier()`, which allows multi-part identifiers
like `db1.db2`. Since the supported syntax in the PR description is a single
database name, consider using `SimpleIdentifier()` here to prevent ambiguous
inputs and accidental header values like `db1.db2` being treated as a database
name.
```suggestion
dbName = SimpleIdentifier()
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
