yashmayya commented on code in PR #16626:
URL: https://github.com/apache/pinot/pull/16626#discussion_r2306221645
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotQueryResource.java:
##########
@@ -153,27 +160,40 @@ 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()) {
+ return new MultiStageQueryValidationResponse(false, "Request is missing
the query string field 'sql'", null);
}
- String sqlQuery = requestJson.get("sql").asText();
+
+ String sqlQuery = request.getSql();
Map<String, String> queryOptionsMap =
RequestUtils.parseQuery(sqlQuery).getOptions();
String database =
DatabaseUtils.extractDatabaseFromQueryRequest(queryOptionsMap, httpHeaders);
- try (QueryEnvironment.CompiledQuery compiledQuery = new
QueryEnvironment(database,
- _pinotHelixResourceManager.getTableCache(), null).compile(sqlQuery)) {
- return new MultiStageQueryValidationResponse(true, null, null);
+
+ try {
+ TableCache tableCache;
+ if (CollectionUtils.isNotEmpty(request.getTableConfigs()) &&
CollectionUtils.isNotEmpty(request.getSchemas())) {
+ tableCache =
+ new StaticTableCache(request.getTableConfigs(),
request.getSchemas(), request.getLogicalTableConfigs(),
+ request.getIgnoreCase());
+ LOGGER.info("Checking MSE validation using static table cache for
query: {}", request.getSql());
+ } else {
+ // Use TableCache from environment if static fields are not specified
+ tableCache = _pinotHelixResourceManager.getTableCache();
+ LOGGER.info("Checking MSE validation using Zk table cache for query:
{}", request.getSql());
+ }
+ try (QueryEnvironment.CompiledQuery compiledQuery = new
QueryEnvironment(database, tableCache, null).compile(
+ sqlQuery)) {
+ return new MultiStageQueryValidationResponse(true, null, null);
+ }
} catch (QueryException e) {
LOGGER.info("Caught exception while compiling multi-stage query: {}",
e.getMessage());
return new MultiStageQueryValidationResponse(false, e.getMessage(),
e.getErrorCode());
+ } catch (Exception e) {
+ LOGGER.error("Caught exception while validating multi-stage query: {}",
e.getMessage());
+ return new MultiStageQueryValidationResponse(false, "Unexpected error: "
+ e.getMessage(),
+ QueryErrorCode.QUERY_VALIDATION);
Review Comment:
nit: this should probably use `UNKNOWN` instead.
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotQueryResource.java:
##########
@@ -153,27 +160,40 @@ 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()) {
+ return new MultiStageQueryValidationResponse(false, "Request is missing
the query string field 'sql'", null);
}
- String sqlQuery = requestJson.get("sql").asText();
+
+ String sqlQuery = request.getSql();
Map<String, String> queryOptionsMap =
RequestUtils.parseQuery(sqlQuery).getOptions();
String database =
DatabaseUtils.extractDatabaseFromQueryRequest(queryOptionsMap, httpHeaders);
- try (QueryEnvironment.CompiledQuery compiledQuery = new
QueryEnvironment(database,
- _pinotHelixResourceManager.getTableCache(), null).compile(sqlQuery)) {
- return new MultiStageQueryValidationResponse(true, null, null);
+
+ try {
+ TableCache tableCache;
+ if (CollectionUtils.isNotEmpty(request.getTableConfigs()) &&
CollectionUtils.isNotEmpty(request.getSchemas())) {
+ tableCache =
+ new StaticTableCache(request.getTableConfigs(),
request.getSchemas(), request.getLogicalTableConfigs(),
+ request.getIgnoreCase());
+ LOGGER.info("Checking MSE validation using static table cache for
query: {}", request.getSql());
Review Comment:
```suggestion
LOGGER.info("Validating multi-stage query compilation using static
table cache for query: {}", request.getSql());
```
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotQueryResource.java:
##########
@@ -204,6 +224,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 != null ? tableConfigs : new ArrayList<>();
Review Comment:
I guess this empty list logic isn't needed anymore?
##########
pinot-controller/src/test/java/org/apache/pinot/controller/api/resources/PinotQueryResourceStaticValidationTest.java:
##########
@@ -0,0 +1,100 @@
+/**
+ * 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.pinot.controller.api.resources;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.common.config.provider.StaticTableCache;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.config.table.TableType;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
+import org.mockito.MockitoAnnotations;
+import org.testng.Assert;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+/**
+ * Unit test for the static table cache functionality in PinotQueryResource.
+ */
+public class PinotQueryResourceStaticValidationTest {
+
+ private ObjectMapper _objectMapper;
+
+ @BeforeClass
+ public void setUp() {
+ MockitoAnnotations.openMocks(this);
+ _objectMapper = new ObjectMapper();
+ }
+
+ @Test
+ public void testStaticTableCacheProvider() {
+ TableConfig tableConfig = new
TableConfigBuilder(TableType.OFFLINE).setTableName("testTable").build();
+
+ Schema schema = new Schema.SchemaBuilder().setSchemaName("testTable")
+ .addSingleValueDimension("dimensionCol", FieldSpec.DataType.STRING)
+ .addMetric("metricCol", FieldSpec.DataType.LONG).build();
+
+ List<TableConfig> tableConfigs = Arrays.asList(tableConfig);
+ List<Schema> schemas = Arrays.asList(schema);
+
+ StaticTableCache provider = new StaticTableCache(tableConfigs, schemas,
null, false);
+
+ Assert.assertFalse(provider.isIgnoreCase());
+ Assert.assertEquals(provider.getActualTableName("testTable_OFFLINE"),
"testTable_OFFLINE");
+ Assert.assertEquals(provider.getActualTableName("testTable"), "testTable");
+ Assert.assertNotNull(provider.getTableConfig("testTable_OFFLINE"));
+ Assert.assertNotNull(provider.getSchema("testTable"));
+ Assert.assertNotNull(provider.getColumnNameMap("testTable"));
+ Assert.assertEquals(provider.getColumnNameMap("testTable").size(), 5); //
2 columns + 3 built-in virtual columns
+
+
Assert.assertTrue(provider.getTableNameMap().containsKey("testTable_OFFLINE"));
+ Assert.assertTrue(provider.getTableNameMap().containsKey("testTable"));
+ }
+
+ @Test
+ public void testRequestSerialization()
Review Comment:
This test can be removed now I think?
--
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]
