vvysotskyi commented on code in PR #2733:
URL: https://github.com/apache/drill/pull/2733#discussion_r1070571396
##
exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceSchemaFactory.java:
##
@@ -403,8 +404,24 @@ private View getView(DotDrillFile f) throws IOException {
return f.getView(mapper);
}
+private String getTemporaryName(String name) {
+ if (isTemporaryWorkspace()) {
+String tableName = DrillStringUtils.removeLeadingSlash(name);
+return schemaConfig.getTemporaryTableName(tableName);
+ }
+ return null;
+}
+
+private boolean isTemporaryWorkspace() {
Review Comment:
I think it is unlikely that it would be reused.
##
exec/java-exec/src/main/java/org/apache/drill/exec/work/metadata/MetadataProvider.java:
##
@@ -607,6 +608,16 @@ public String getQueryUserName() {
@Override public UserCredentials getQueryUserCredentials() {
return session.getCredentials();
}
+
+ @Override
+ public String getTemporaryTableName(String table) {
Review Comment:
I agree it is not good to have such interfaces with unsupported methods.
Ideally, we should split them into several interfaces instead and use broader
ones in places where it is required.
##
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillCalciteCatalogReader.java:
##
@@ -135,14 +112,15 @@ public Prepare.PreparingTable getTable(List
names) {
}
private void checkTemporaryTable(List names) {
-if (allowTemporaryTables) {
+if (allowTemporaryTables || !needsTemporaryTableCheck(names,
session.getDefaultSchemaPath(), drillConfig)) {
return;
}
-String originalTableName =
session.getOriginalTableNameFromTemporaryTable(names.get(names.size() - 1));
+String tableName = names.get(names.size() - 1);
+String originalTableName = session.resolveTemporaryTableName(tableName);
if (originalTableName != null) {
throw UserException
.validationError()
- .message("Temporary tables usage is disallowed. Used temporary table
name: [%s].", originalTableName)
+ .message("Temporary tables usage is disallowed. Used temporary table
name: [%s].", tableName)
Review Comment:
Thanks, replaced it.
##
exec/java-exec/src/main/java/org/apache/drill/exec/work/metadata/MetadataProvider.java:
##
@@ -607,6 +608,16 @@ public String getQueryUserName() {
@Override public UserCredentials getQueryUserCredentials() {
return session.getCredentials();
}
+
+ @Override
+ public String getTemporaryTableName(String table) {
+return session.resolveTemporaryTableName(table);
+ }
+
+ @Override
+ public String getTemporaryWorkspace() {
+return config.getString(ExecConstants.DEFAULT_TEMPORARY_WORKSPACE);
Review Comment:
Yes, config is the only source for this property. But I think it is better
to have an interface that provides only information related to schema config
info rather than allow callers to access config by themselves. The current
approach helps to encapsulate it, so I would prefer to leave it as it is.
--
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: dev-unsubscr...@drill.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
