This is an automated email from the ASF dual-hosted git repository.
arina pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/drill.git
commit 6fef0820d427aca92df42ca5c578e932d7a0b36b
Author: Volodymyr Vysotskyi <vvo...@gmail.com>
AuthorDate: Tue Jan 21 20:16:36 2020 +0200
DRILL-7527: DROP METADATA doesn't work with table name starting with '/'
inside workspace
closes #1958
---
.../apache/drill/common/util/DrillStringUtils.java | 37 +++++++++++-----------
.../sql/conversion/DrillCalciteCatalogReader.java | 4 +--
.../planner/sql/handlers/CreateTableHandler.java | 6 ++--
.../planner/sql/handlers/DropTableHandler.java | 4 +--
.../exec/planner/sql/handlers/ViewHandler.java | 6 ++--
.../drill/exec/planner/sql/parser/SqlSchema.java | 4 +--
.../apache/drill/exec/store/dfs/FileSelection.java | 12 ++-----
.../exec/store/dfs/WorkspaceSchemaFactory.java | 3 +-
.../drill/exec/sql/TestMetastoreCommands.java | 30 ++++++++++++++++++
.../drill/exec/store/dfs/TestFileSelection.java | 5 +--
.../apache/drill/metastore/metadata/TableInfo.java | 4 ++-
11 files changed, 70 insertions(+), 45 deletions(-)
diff --git
a/common/src/main/java/org/apache/drill/common/util/DrillStringUtils.java
b/common/src/main/java/org/apache/drill/common/util/DrillStringUtils.java
index 0fb6f80..de65ff8 100644
--- a/common/src/main/java/org/apache/drill/common/util/DrillStringUtils.java
+++ b/common/src/main/java/org/apache/drill/common/util/DrillStringUtils.java
@@ -17,13 +17,13 @@
*/
package org.apache.drill.common.util;
-import org.apache.drill.shaded.guava.com.google.common.base.Joiner;
import io.netty.buffer.ByteBuf;
import org.apache.commons.lang3.StringEscapeUtils;
import org.apache.commons.lang3.StringUtils;
-import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.stream.Collectors;
public class DrillStringUtils {
@@ -51,7 +51,7 @@ public class DrillStringUtils {
* @param input the {@code String} to unescape, may be null
* @return a new unescaped {@code String}, {@code null} if null string input
*/
- public static final String unescapeJava(String input) {
+ public static String unescapeJava(String input) {
return StringEscapeUtils.unescapeJava(input);
}
@@ -72,11 +72,11 @@ public class DrillStringUtils {
* @param input String to escape values in, may be null
* @return String with escaped values, {@code null} if null string input
*/
- public static final String escapeJava(String input) {
+ public static String escapeJava(String input) {
return StringEscapeUtils.escapeJava(input);
}
- public static final String escapeNewLines(String input) {
+ public static String escapeNewLines(String input) {
if (input == null) {
return null;
}
@@ -209,21 +209,20 @@ public class DrillStringUtils {
* @return The sanitized CSV string
*/
public static String sanitizeCSV(String csv) {
- // tokenize
String[] tokens = csv.split(",");
- ArrayList<String> sanitizedTokens = new ArrayList<String>(tokens.length);
- // check for empties
- for (String s : tokens) {
- String trimmedToken = s.trim();
- if (trimmedToken.length() != 0) {
- sanitizedTokens.add(trimmedToken);
- }
- }
- String result = "";
- if (sanitizedTokens.size() != 0) {
- result = Joiner.on(",").join(sanitizedTokens);
- }
- return result;
+ return Arrays.stream(tokens)
+ .map(String::trim)
+ .filter(StringUtils::isNotEmpty)
+ .collect(Collectors.joining(","));
}
+ /**
+ * Removes all leading slash characters from specified string.
+ *
+ * @param path string to remove all leading slash characters
+ * @return string with removed leading slash characters
+ */
+ public static String removeLeadingSlash(String path) {
+ return StringUtils.stripStart(path, "/");
+ }
}
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillCalciteCatalogReader.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillCalciteCatalogReader.java
index 4c8b254..b6fdcc5 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillCalciteCatalogReader.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillCalciteCatalogReader.java
@@ -35,11 +35,11 @@ import org.apache.calcite.sql.validate.SqlValidatorUtil;
import org.apache.calcite.util.Util;
import org.apache.drill.common.config.DrillConfig;
import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.common.util.DrillStringUtils;
import org.apache.drill.exec.metastore.MetadataProviderManager;
import org.apache.drill.exec.planner.logical.DrillTable;
import org.apache.drill.exec.planner.sql.SchemaUtilites;
import org.apache.drill.exec.rpc.user.UserSession;
-import org.apache.drill.exec.store.dfs.FileSelection;
import org.apache.drill.shaded.guava.com.google.common.cache.CacheBuilder;
import org.apache.drill.shaded.guava.com.google.common.cache.CacheLoader;
import org.apache.drill.shaded.guava.com.google.common.cache.LoadingCache;
@@ -101,7 +101,7 @@ class DrillCalciteCatalogReader extends
CalciteCatalogReader {
List<String> getTemporaryNames(List<String> names) {
if (needsTemporaryTableCheck(names, session.getDefaultSchemaPath(),
drillConfig)) {
- String tableName =
FileSelection.removeLeadingSlash(names.get(names.size() - 1));
+ String tableName =
DrillStringUtils.removeLeadingSlash(names.get(names.size() - 1));
String temporaryTableName = session.resolveTemporaryTableName(tableName);
if (temporaryTableName != null) {
List<String> temporaryNames = new
ArrayList<>(SchemaUtilites.getSchemaPathAsList(temporarySchema));
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java
index 82a111a..082fe27 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java
@@ -37,6 +37,7 @@ import org.apache.calcite.tools.RelConversionException;
import org.apache.calcite.tools.ValidationException;
import org.apache.drill.common.config.DrillConfig;
import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.common.util.DrillStringUtils;
import org.apache.drill.exec.ExecConstants;
import org.apache.drill.exec.physical.PhysicalPlan;
import org.apache.drill.exec.physical.base.PhysicalOperator;
@@ -55,7 +56,6 @@ import org.apache.drill.exec.planner.sql.DrillSqlOperator;
import org.apache.drill.exec.planner.sql.SchemaUtilites;
import org.apache.drill.exec.planner.sql.parser.SqlCreateTable;
import org.apache.drill.exec.store.AbstractSchema;
-import org.apache.drill.exec.store.dfs.FileSelection;
import org.apache.drill.exec.util.Pointer;
import org.apache.drill.exec.work.foreman.ForemanSetupException;
import org.apache.drill.exec.work.foreman.SqlUnsupportedException;
@@ -73,7 +73,7 @@ public class CreateTableHandler extends DefaultSqlHandler {
@Override
public PhysicalPlan getPlan(SqlNode sqlNode) throws ValidationException,
RelConversionException, IOException, ForemanSetupException {
final SqlCreateTable sqlCreateTable = unwrap(sqlNode,
SqlCreateTable.class);
- final String originalTableName =
FileSelection.removeLeadingSlash(sqlCreateTable.getName());
+ final String originalTableName =
DrillStringUtils.removeLeadingSlash(sqlCreateTable.getName());
final ConvertedRelNode convertedRelNode =
validateAndConvert(sqlCreateTable.getQuery());
final RelDataType validatedRowType =
convertedRelNode.getValidatedRowType();
@@ -329,4 +329,4 @@ public class CreateTableHandler extends DefaultSqlHandler {
}
return true;
}
-}
\ No newline at end of file
+}
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DropTableHandler.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DropTableHandler.java
index 665bddd..ab0427a 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DropTableHandler.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DropTableHandler.java
@@ -25,13 +25,13 @@ import org.apache.calcite.schema.Table;
import org.apache.calcite.sql.SqlNode;
import org.apache.drill.common.config.DrillConfig;
import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.common.util.DrillStringUtils;
import org.apache.drill.exec.physical.PhysicalPlan;
import org.apache.drill.exec.planner.sql.DirectPlan;
import org.apache.drill.exec.planner.sql.SchemaUtilites;
import org.apache.drill.exec.planner.sql.parser.SqlDropTable;
import org.apache.drill.exec.rpc.user.UserSession;
import org.apache.drill.exec.store.AbstractSchema;
-import org.apache.drill.exec.store.dfs.FileSelection;
// SqlHandler for dropping a table.
public class DropTableHandler extends DefaultSqlHandler {
@@ -54,7 +54,7 @@ public class DropTableHandler extends DefaultSqlHandler {
@Override
public PhysicalPlan getPlan(SqlNode sqlNode) {
SqlDropTable dropTableNode = ((SqlDropTable) sqlNode);
- String originalTableName =
FileSelection.removeLeadingSlash(dropTableNode.getName());
+ String originalTableName =
DrillStringUtils.removeLeadingSlash(dropTableNode.getName());
SchemaPlus defaultSchema = config.getConverter().getDefaultSchema();
List<String> tableSchema = dropTableNode.getSchema();
DrillConfig drillConfig = context.getConfig();
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java
index d524d2d..7812d84 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java
@@ -27,6 +27,7 @@ import org.apache.calcite.tools.RelConversionException;
import org.apache.calcite.tools.ValidationException;
import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.common.util.DrillStringUtils;
import org.apache.drill.exec.dotdrill.View;
import org.apache.drill.exec.ops.QueryContext;
import org.apache.drill.exec.physical.PhysicalPlan;
@@ -35,7 +36,6 @@ import org.apache.drill.exec.planner.sql.SchemaUtilites;
import org.apache.drill.exec.planner.sql.parser.SqlCreateView;
import org.apache.drill.exec.planner.sql.parser.SqlDropView;
import org.apache.drill.exec.store.AbstractSchema;
-import org.apache.drill.exec.store.dfs.FileSelection;
import org.apache.drill.exec.work.foreman.ForemanSetupException;
import org.apache.calcite.rel.RelNode;
import org.apache.calcite.sql.SqlNode;
@@ -61,7 +61,7 @@ public abstract class ViewHandler extends DefaultSqlHandler {
public PhysicalPlan getPlan(SqlNode sqlNode) throws ValidationException,
RelConversionException, IOException, ForemanSetupException {
SqlCreateView createView = unwrap(sqlNode, SqlCreateView.class);
- final String newViewName =
FileSelection.removeLeadingSlash(createView.getName());
+ final String newViewName =
DrillStringUtils.removeLeadingSlash(createView.getName());
// Disallow temporary tables usage in view definition
config.getConverter().disallowTemporaryTables();
@@ -157,7 +157,7 @@ public abstract class ViewHandler extends DefaultSqlHandler
{
@Override
public PhysicalPlan getPlan(SqlNode sqlNode) throws IOException,
ForemanSetupException {
SqlDropView dropView = unwrap(sqlNode, SqlDropView.class);
- final String viewName =
FileSelection.removeLeadingSlash(dropView.getName());
+ final String viewName =
DrillStringUtils.removeLeadingSlash(dropView.getName());
final AbstractSchema drillSchema =
SchemaUtilites.resolveToMutableDrillSchema(context.getNewDefaultSchema(),
dropView.getSchemaPath());
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlSchema.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlSchema.java
index 44a3adb..97f52a3 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlSchema.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlSchema.java
@@ -29,11 +29,11 @@ import org.apache.calcite.sql.SqlSpecialOperator;
import org.apache.calcite.sql.SqlWriter;
import org.apache.calcite.sql.parser.SqlParserPos;
import org.apache.calcite.sql.util.SqlBasicVisitor;
+import org.apache.drill.common.util.DrillStringUtils;
import org.apache.drill.exec.planner.sql.handlers.AbstractSqlHandler;
import org.apache.drill.exec.planner.sql.handlers.SqlHandlerConfig;
import org.apache.drill.exec.planner.sql.handlers.SchemaHandler;
import org.apache.drill.exec.planner.sql.handlers.SqlHandlerUtil;
-import org.apache.drill.exec.store.dfs.FileSelection;
import java.util.Arrays;
import java.util.Collections;
@@ -88,7 +88,7 @@ public abstract class SqlSchema extends DrillSqlCall {
public String getTableName() {
if (hasTable()) {
String tableName = table.isSimple() ? table.getSimple() :
table.names.get(table.names.size() - 1);
- return FileSelection.removeLeadingSlash(tableName);
+ return DrillStringUtils.removeLeadingSlash(tableName);
}
return null;
}
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSelection.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSelection.java
index a1fccb7..6700114 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSelection.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSelection.java
@@ -18,6 +18,7 @@
package org.apache.drill.exec.store.dfs;
import org.apache.drill.common.exceptions.DrillRuntimeException;
+import org.apache.drill.common.util.DrillStringUtils;
import org.apache.drill.exec.util.DrillFileSystemUtil;
import org.apache.drill.shaded.guava.com.google.common.base.Preconditions;
import org.apache.drill.shaded.guava.com.google.common.base.Stopwatch;
@@ -269,7 +270,7 @@ public class FileSelection {
Stopwatch timer = logger.isDebugEnabled() ? Stopwatch.createStarted() :
null;
boolean hasWildcard = path.contains(WILD_CARD);
- Path combined = new Path(parent, removeLeadingSlash(path));
+ Path combined = new Path(parent,
DrillStringUtils.removeLeadingSlash(path));
if (!allowAccessOutsideWorkspace) {
checkBackPaths(new Path(parent).toUri().getPath(),
combined.toUri().getPath(), path);
}
@@ -365,15 +366,6 @@ public class FileSelection {
}
}
- public static String removeLeadingSlash(String path) {
- if (!path.isEmpty() && path.charAt(0) == '/') {
- String newPath = path.substring(1);
- return removeLeadingSlash(newPath);
- } else {
- return path;
- }
- }
-
/**
* Check if the path is a valid sub path under the parent after removing
backpaths. Throw an exception if
* it is not. We pass subpath in as a parameter only for the error message
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceSchemaFactory.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceSchemaFactory.java
index 87b5592..7d9d9dd 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceSchemaFactory.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceSchemaFactory.java
@@ -46,6 +46,7 @@ import
org.apache.drill.common.exceptions.ExecutionSetupException;
import org.apache.drill.common.exceptions.UserException;
import org.apache.drill.common.logical.FormatPluginConfig;
import org.apache.drill.common.scanner.persistence.ScanResult;
+import org.apache.drill.common.util.DrillStringUtils;
import org.apache.drill.exec.ExecConstants;
import org.apache.drill.exec.dotdrill.DotDrillFile;
import org.apache.drill.exec.dotdrill.DotDrillType;
@@ -410,7 +411,7 @@ public class WorkspaceSchemaFactory {
try {
try {
files = DotDrillUtil.getDotDrills(getFS(), new
Path(config.getLocation()),
- FileSelection.removeLeadingSlash(tableName), DotDrillType.VIEW);
+ DrillStringUtils.removeLeadingSlash(tableName),
DotDrillType.VIEW);
} catch (AccessControlException e) {
if (!schemaConfig.getIgnoreAuthErrors()) {
logger.debug(e.getMessage());
diff --git
a/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestMetastoreCommands.java
b/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestMetastoreCommands.java
index ccee4ed..b31002e 100644
---
a/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestMetastoreCommands.java
+++
b/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestMetastoreCommands.java
@@ -3131,6 +3131,36 @@ public class TestMetastoreCommands extends ClusterTest {
}
}
+ @Test
+ public void testAnalyzeWithLeadingSlash() throws Exception {
+ String tableName = "tableWithLeadingSlash";
+ TableInfo tableInfo = getTableInfo("/" + tableName, "tmp");
+ try {
+ run("create table dfs.tmp.`%s` as\n" +
+ "select * from cp.`tpch/region.parquet`", tableName);
+
+ testBuilder()
+ .sqlQuery("analyze table dfs.tmp.`%s` REFRESH METADATA", tableName)
+ .unOrdered()
+ .baselineColumns("ok", "summary")
+ .baselineValues(true, String.format("Collected / refreshed metadata
for table [dfs.tmp.%s]", tableName))
+ .go();
+
+ MetastoreTableInfo metastoreTableInfo = cluster.drillbit().getContext()
+ .getMetastoreRegistry()
+ .get()
+ .tables()
+ .basicRequests()
+ .metastoreTableInfo(tableInfo);
+
+ assertTrue("table metadata wasn't found", metastoreTableInfo.isExists());
+ } finally {
+ run("analyze table dfs.tmp.`%s` drop metadata", tableName);
+ run("drop table if exists dfs.tmp.`%s`", tableName);
+ client.resetSession(ExecConstants.METASTORE_ENABLED);
+ }
+ }
+
private static <T> ColumnStatistics<T> getColumnStatistics(T minValue, T
maxValue,
long rowCount, TypeProtos.MinorType minorType) {
return new ColumnStatistics<>(
diff --git
a/exec/java-exec/src/test/java/org/apache/drill/exec/store/dfs/TestFileSelection.java
b/exec/java-exec/src/test/java/org/apache/drill/exec/store/dfs/TestFileSelection.java
index b853b9b..9ab4aea 100644
---
a/exec/java-exec/src/test/java/org/apache/drill/exec/store/dfs/TestFileSelection.java
+++
b/exec/java-exec/src/test/java/org/apache/drill/exec/store/dfs/TestFileSelection.java
@@ -22,6 +22,7 @@ import static org.junit.Assert.assertNull;
import java.util.List;
+import org.apache.drill.common.util.DrillStringUtils;
import org.apache.drill.exec.util.DrillFileSystemUtil;
import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableList;
import org.apache.drill.test.BaseTestQuery;
@@ -62,7 +63,7 @@ public class TestFileSelection extends BaseTestQuery {
boolean isPathGood = true;
try {
String parent = badPaths[i][0];
- String subPath = FileSelection.removeLeadingSlash(badPaths[i][1]);
+ String subPath = DrillStringUtils.removeLeadingSlash(badPaths[i][1]);
String path = new Path(parent, subPath).toString();
FileSelection.checkBackPaths(parent, path, subPath);
} catch (IllegalArgumentException e) {
@@ -88,7 +89,7 @@ public class TestFileSelection extends BaseTestQuery {
for (int i = 0; i < goodPaths.length; i++) {
try {
String parent = goodPaths[i][0];
- String subPath = FileSelection.removeLeadingSlash(goodPaths[i][1]);
+ String subPath = DrillStringUtils.removeLeadingSlash(goodPaths[i][1]);
String path = new Path(parent, subPath).toString();
FileSelection.checkBackPaths(parent, path, subPath);
} catch (IllegalArgumentException e) {
diff --git
a/metastore/metastore-api/src/main/java/org/apache/drill/metastore/metadata/TableInfo.java
b/metastore/metastore-api/src/main/java/org/apache/drill/metastore/metadata/TableInfo.java
index 538c4a2..f9fb3f4 100644
---
a/metastore/metastore-api/src/main/java/org/apache/drill/metastore/metadata/TableInfo.java
+++
b/metastore/metastore-api/src/main/java/org/apache/drill/metastore/metadata/TableInfo.java
@@ -22,6 +22,7 @@ import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonTypeName;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.annotation.JsonPOJOBuilder;
+import org.apache.drill.common.util.DrillStringUtils;
import org.apache.drill.metastore.components.tables.TableMetadataUnit;
import org.apache.drill.metastore.expressions.FilterExpression;
@@ -156,7 +157,8 @@ public class TableInfo {
}
public TableInfoBuilder name(String name) {
- this.name = name;
+ // removes leading slash characters since such table names are equivalent
+ this.name = DrillStringUtils.removeLeadingSlash(name);
return this;
}
