Jackie-Jiang commented on code in PR #16626:
URL: https://github.com/apache/pinot/pull/16626#discussion_r2308311630
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotQueryResource.java:
##########
@@ -204,6 +225,50 @@ public QueryErrorCode getErrorCode() {
}
}
+ public static class MultiStageQueryValidationRequest {
+ private final String _sql;
+ private final List<TableConfig> _tableConfigs;
+ private final List<Schema> _schemas;
+ private final List<LogicalTableConfig> _logicalTableConfigs;
+ private final boolean _ignoreCase;
+
+ @JsonCreator
+ public MultiStageQueryValidationRequest(@JsonProperty("sql") String sql,
+ @JsonProperty("tableConfigs") @Nullable List<TableConfig> tableConfigs,
+ @JsonProperty("schemas") @Nullable List<Schema> schemas,
+ @JsonProperty("logicalTableConfigs") @Nullable
List<LogicalTableConfig> logicalTableConfigs,
+ @JsonProperty("ignoreCase") boolean ignoreCase) {
+ _sql = sql;
+ _tableConfigs = tableConfigs;
+ _schemas = schemas;
+ _logicalTableConfigs = logicalTableConfigs;
+ _ignoreCase = ignoreCase;
+ }
+
+ public String getSql() {
+ return _sql;
+ }
+
+ @Nullable
+ public List<TableConfig> getTableConfigs() {
+ return _tableConfigs;
+ }
+
+ @Nullable
+ public List<Schema> getSchemas() {
+ return _schemas;
+ }
+
+ @Nullable
+ public List<LogicalTableConfig> getLogicalTableConfigs() {
+ return _logicalTableConfigs;
+ }
+
+ public boolean getIgnoreCase() {
Review Comment:
```suggestion
public boolean isIgnoreCase() {
```
##########
pinot-common/src/main/java/org/apache/pinot/common/config/provider/TableCache.java:
##########
@@ -38,6 +49,7 @@
* them in sync. It also maintains the table name map and the column name map
for case-insensitive queries.
*/
public interface TableCache extends PinotConfigProvider {
+ Logger LOGGER = LoggerFactory.getLogger(TableCache.class);
Review Comment:
(nit) Add an empty line below
##########
pinot-common/src/main/java/org/apache/pinot/common/config/provider/ZkTableCache.java:
##########
@@ -431,21 +431,7 @@ private void putSchema(ZNRecord znRecord)
_schemaInfoMap.put(schemaName, new SchemaInfo(schema, columnNameMap));
}
- /**
- * Adds the built-in virtual columns to the schema.
- * NOTE: The virtual column provider class is not added.
- */
- private static void addBuiltInVirtualColumns(Schema schema) {
- if (!schema.hasColumn(BuiltInVirtualColumn.DOCID)) {
- schema.addField(new DimensionFieldSpec(BuiltInVirtualColumn.DOCID,
FieldSpec.DataType.INT, true));
- }
- if (!schema.hasColumn(BuiltInVirtualColumn.HOSTNAME)) {
- schema.addField(new DimensionFieldSpec(BuiltInVirtualColumn.HOSTNAME,
FieldSpec.DataType.STRING, true));
- }
- if (!schema.hasColumn(BuiltInVirtualColumn.SEGMENTNAME)) {
- schema.addField(new DimensionFieldSpec(BuiltInVirtualColumn.SEGMENTNAME,
FieldSpec.DataType.STRING, true));
- }
- }
+
Review Comment:
(minor) Remove extra empty lines
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotQueryResource.java:
##########
@@ -153,27 +160,41 @@ public StreamingOutput
handleTimeSeriesQueryRange(@QueryParam("language") String
@POST
@Path("validateMultiStageQuery")
- public MultiStageQueryValidationResponse validateMultiStageQuery(String
requestJsonStr,
+ public MultiStageQueryValidationResponse
validateMultiStageQuery(MultiStageQueryValidationRequest request,
@Context HttpHeaders httpHeaders) {
- JsonNode requestJson;
- try {
- requestJson = JsonUtils.stringToJsonNode(requestJsonStr);
- } catch (Exception e) {
- LOGGER.warn("Caught exception while parsing request {}", e.getMessage());
- return new MultiStageQueryValidationResponse(false, "Failed to parse
request JSON: " + e.getMessage(), null);
- }
- if (!requestJson.has("sql")) {
- return new MultiStageQueryValidationResponse(false, "JSON Payload is
missing the query string field 'sql'", null);
+
+ if (request.getSql() == null || request.getSql().trim().isEmpty()) {
Review Comment:
(minor) We are paying overhead for `trim()`. Consider trim then check:
```
if (request.getSql() == null) {
...
}
String sql = request.getSql().trim();
if (sql.isEmpty()) {
...
}
...
```
##########
pinot-common/src/main/java/org/apache/pinot/common/config/provider/TableCache.java:
##########
@@ -128,4 +140,81 @@ public interface TableCache extends PinotConfigProvider {
@Override
boolean registerLogicalTableConfigChangeListener(
LogicalTableConfigChangeListener logicalTableConfigChangeListener);
+
+ /**
+ * Adds the built-in virtual columns to the schema.
+ * NOTE: The virtual column provider class is not added.
+ */
+ default void addBuiltInVirtualColumns(Schema schema) {
+ if (!schema.hasColumn(BuiltInVirtualColumn.DOCID)) {
+ schema.addField(new DimensionFieldSpec(BuiltInVirtualColumn.DOCID,
FieldSpec.DataType.INT, true));
+ }
+ if (!schema.hasColumn(BuiltInVirtualColumn.HOSTNAME)) {
+ schema.addField(new DimensionFieldSpec(BuiltInVirtualColumn.HOSTNAME,
FieldSpec.DataType.STRING, true));
+ }
+ if (!schema.hasColumn(BuiltInVirtualColumn.SEGMENTNAME)) {
+ schema.addField(new DimensionFieldSpec(BuiltInVirtualColumn.SEGMENTNAME,
FieldSpec.DataType.STRING, true));
+ }
+ }
+
+ static Map<Expression, Expression> createExpressionOverrideMap(String
physicalOrLogicalTableName,
+ QueryConfig queryConfig) {
+ Map<Expression, Expression> expressionOverrideMap = new TreeMap<>();
+ if (queryConfig != null &&
MapUtils.isNotEmpty(queryConfig.getExpressionOverrideMap())) {
+ for (Map.Entry<String, String> entry :
queryConfig.getExpressionOverrideMap().entrySet()) {
+ try {
+ Expression srcExp =
CalciteSqlParser.compileToExpression(entry.getKey());
+ Expression destExp =
CalciteSqlParser.compileToExpression(entry.getValue());
+ expressionOverrideMap.put(srcExp, destExp);
+ } catch (Exception e) {
+ LOGGER.warn("Caught exception while compiling expression override:
{} -> {} for table: {}, skipping it",
+ entry.getKey(), entry.getValue(), physicalOrLogicalTableName);
+ }
+ }
+ int mapSize = expressionOverrideMap.size();
+ if (mapSize == 1) {
+ Map.Entry<Expression, Expression> entry =
expressionOverrideMap.entrySet().iterator().next();
+ return Collections.singletonMap(entry.getKey(), entry.getValue());
+ } else if (mapSize > 1) {
+ return expressionOverrideMap;
+ }
+ }
+ return null;
+ }
+
+ class TableConfigInfo {
+ final TableConfig _tableConfig;
+ final Map<Expression, Expression> _expressionOverrideMap;
+ // All the timestamp with granularity column names
+ final Set<String> _timestampIndexColumns;
+
+ public TableConfigInfo(TableConfig tableConfig) {
+ _tableConfig = tableConfig;
+ _expressionOverrideMap =
+ createExpressionOverrideMap(tableConfig.getTableName(),
tableConfig.getQueryConfig());
+ _timestampIndexColumns =
TimestampIndexUtils.extractColumnsWithGranularity(tableConfig);
+ }
+ }
+
Review Comment:
(nit) Remove extra empty line
--
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]
