This is an automated email from the ASF dual-hosted git repository.
xiangfu 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 4aecd77fd5 Changes to support private columns (#11484)
4aecd77fd5 is described below
commit 4aecd77fd5d98d3b69da895a5550daf5b9d11a14
Author: Gonzalo Ortiz Jaureguizar <[email protected]>
AuthorDate: Mon Sep 4 17:30:13 2023 +0200
Changes to support private columns (#11484)
* Changes to support private columns
* Fix code style and add javadoc
* Create some tests to verify correct select star projection
---
.../pinot/integration/tests/ClusterTest.java | 38 ++++++++++++++++++++++
.../tests/MultiStageEngineIntegrationTest.java | 26 +++++++++++++++
.../org/apache/pinot/query/validate/Validator.java | 24 ++++++++++++++
3 files changed, 88 insertions(+)
diff --git
a/pinot-integration-test-base/src/test/java/org/apache/pinot/integration/tests/ClusterTest.java
b/pinot-integration-test-base/src/test/java/org/apache/pinot/integration/tests/ClusterTest.java
index 2a278166c7..9845318f30 100644
---
a/pinot-integration-test-base/src/test/java/org/apache/pinot/integration/tests/ClusterTest.java
+++
b/pinot-integration-test-base/src/test/java/org/apache/pinot/integration/tests/ClusterTest.java
@@ -30,6 +30,7 @@ import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
+import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Random;
@@ -76,6 +77,7 @@ import org.apache.pinot.spi.utils.JsonUtils;
import org.apache.pinot.spi.utils.NetUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import org.testng.Assert;
import org.testng.annotations.DataProvider;
import static
org.apache.pinot.integration.tests.ClusterIntegrationTestUtils.getBrokerQueryApiUrl;
@@ -513,6 +515,42 @@ public abstract class ClusterTest extends ControllerTest {
sendPostRequest(controllerBaseApiUrl + "/sql",
JsonUtils.objectToString(payload), headers));
}
+ public List<String> getColumns(JsonNode response) {
+ JsonNode resultTableJson = response.get("resultTable");
+ Assert.assertNotNull(resultTableJson, "'resultTable' is null");
+ JsonNode dataSchemaJson = resultTableJson.get("dataSchema");
+ Assert.assertNotNull(resultTableJson, "'resultTable.dataSchema' is null");
+ JsonNode colNamesJson = dataSchemaJson.get("columnNames");
+ Assert.assertNotNull(resultTableJson,
"'resultTable.dataSchema.columnNames' is null");
+
+ List<String> cols = new ArrayList<>();
+ int i = 0;
+ for (JsonNode jsonNode : colNamesJson) {
+ String colName = jsonNode.textValue();
+ Assert.assertNotNull(colName, "Column at index " + i + " is not a
string");
+ cols.add(colName);
+ i++;
+ }
+ return cols;
+ }
+
+ public void assertNoError(JsonNode response) {
+ JsonNode exceptionsJson = response.get("exceptions");
+ Iterator<JsonNode> exIterator = exceptionsJson.iterator();
+ if (exIterator.hasNext()) {
+ Assert.fail("There is at least one exception: " + exIterator.next());
+ }
+ }
+
+ @DataProvider(name = "systemColumns")
+ public Object[][] systemColumns() {
+ return new Object[][] {
+ {"$docId"},
+ {"$hostName"},
+ {"$segmentName"}
+ };
+ }
+
@DataProvider(name = "useBothQueryEngines")
public Object[][] useBothQueryEngines() {
return new Object[][]{
diff --git
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineIntegrationTest.java
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineIntegrationTest.java
index 28ba7be826..f6a2345a62 100644
---
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineIntegrationTest.java
+++
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineIntegrationTest.java
@@ -674,6 +674,32 @@ public class MultiStageEngineIntegrationTest extends
BaseClusterIntegrationTestS
Assert.assertEquals(jsonNode.get("resultTable").get("dataSchema").get("columnDataTypes").get(0).asText(),
"DOUBLE");
}
+ @Test
+ public void selectStarDoesNotProjectSystemColumns()
+ throws Exception {
+ JsonNode jsonNode = postQuery("select * from mytable limit 0");
+ List<String> columns = getColumns(jsonNode);
+ for (int i = 0; i < columns.size(); i++) {
+ String colName = columns.get(i);
+ Assert.assertFalse(colName.startsWith("$"), "Column " + colName + "
(found at index " + i + " is a system column "
+ + "and shouldn't be included in select *");
+ }
+ }
+
+ @Test(dataProvider = "systemColumns")
+ public void systemColumnsCanBeSelected(String systemColumn)
+ throws Exception {
+ JsonNode jsonNode = postQuery("select " + systemColumn + " from mytable
limit 0");
+ assertNoError(jsonNode);
+ }
+
+ @Test(dataProvider = "systemColumns")
+ public void systemColumnsCanBeUsedInWhere(String systemColumn)
+ throws Exception {
+ JsonNode jsonNode = postQuery("select 1 from mytable where " +
systemColumn + " is not null limit 0");
+ assertNoError(jsonNode);
+ }
+
@AfterClass
public void tearDown()
throws Exception {
diff --git
a/pinot-query-planner/src/main/java/org/apache/pinot/query/validate/Validator.java
b/pinot-query-planner/src/main/java/org/apache/pinot/query/validate/Validator.java
index 14a0797957..6a3aa25d1a 100644
---
a/pinot-query-planner/src/main/java/org/apache/pinot/query/validate/Validator.java
+++
b/pinot-query-planner/src/main/java/org/apache/pinot/query/validate/Validator.java
@@ -20,14 +20,19 @@ package org.apache.pinot.query.validate;
import java.util.ArrayList;
import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.rel.type.RelDataTypeFactory;
import org.apache.calcite.rel.type.RelDataTypeField;
import org.apache.calcite.rel.type.RelRecordType;
import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlKind;
import org.apache.calcite.sql.SqlNode;
import org.apache.calcite.sql.SqlNodeList;
import org.apache.calcite.sql.SqlOperatorTable;
import org.apache.calcite.sql.SqlSelect;
+import org.apache.calcite.sql.validate.SelectScope;
import org.apache.calcite.sql.validate.SqlConformanceEnum;
import org.apache.calcite.sql.validate.SqlValidatorCatalogReader;
import org.apache.calcite.sql.validate.SqlValidatorImpl;
@@ -124,4 +129,23 @@ public class Validator extends SqlValidatorImpl {
List<String> names = identifier.names;
return names.get(names.size() - 1);
}
+
+ /**
+ * This method is only called by Calcite when adding rows to select as a
result of doing {@code select *}.
+ *
+ * This is just a patch knowing the internals of Calcite, but there is no
officially supported solution right now.
+ * See <a
href="https://lists.apache.org/thread/c71fkv329001hjjq7bp4hm56q6yx6dvw";>this
question in the dev email
+ * list</a>
+ */
+ @Override
+ protected void addToSelectList(List<SqlNode> list, Set<String> aliases,
+ List<Map.Entry<String, RelDataType>> fieldList, SqlNode exp, SelectScope
scope, boolean includeSystemVars) {
+ if (exp.getKind() == SqlKind.IDENTIFIER) {
+ SqlIdentifier sqlIdentifier = (SqlIdentifier) exp;
+ if (sqlIdentifier.names.stream().anyMatch(s -> s.startsWith("$"))) {
+ return;
+ }
+ }
+ super.addToSelectList(list, aliases, fieldList, exp, scope,
includeSystemVars);
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
