[
https://issues.apache.org/jira/browse/DRILL-8380?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17677017#comment-17677017
]
ASF GitHub Bot commented on DRILL-8380:
---------------------------------------
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<String>
names) {
}
private void checkTemporaryTable(List<String> 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.
> Remove customised SqlValidatorImpl.deriveAlias
> ----------------------------------------------
>
> Key: DRILL-8380
> URL: https://issues.apache.org/jira/browse/DRILL-8380
> Project: Apache Drill
> Issue Type: Sub-task
> Reporter: Vova Vysotskyi
> Assignee: Vova Vysotskyi
> Priority: Major
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
