Skip to content

Commit

Permalink
Use node reference comparison to dermine if table is delete target
Browse files Browse the repository at this point in the history
  • Loading branch information
@losipiuk
losipiuk committed Dec 14, 2020
1 parent 694900e commit b51b6da
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 14 deletions.
39 changes: 32 additions & 7 deletions presto-main/src/main/java/io/prestosql/sql/analyzer/Analysis.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,7 @@ public class Analysis
private final Statement root;
private final Map<NodeRef<Parameter>, Expression> parameters;
private String updateType;
// TODO Reference by NodeRef instead name so we can distinguish actual target from other references to same table in the query
private Optional<QualifiedObjectName> target = Optional.empty();
private Optional<UpdateTarget> target = Optional.empty();
private boolean skipMaterializedViewRefresh;

private final Map<NodeRef<Table>, Query> namedQueries = new LinkedHashMap<>();
Expand Down Expand Up @@ -205,13 +204,16 @@ public String getUpdateType()

public Optional<Output> getTarget()
{
return target.map(table -> new Output(table.getCatalogName(), table.getSchemaName(), table.getObjectName()));
return target.map(target -> {
QualifiedObjectName name = target.getName();
return new Output(name.getCatalogName(), name.getSchemaName(), name.getObjectName());
});
}

public void setUpdateType(String updateType, QualifiedObjectName target)
public void setUpdateType(String updateType, QualifiedObjectName targetName, Optional<Table> targetTable)
{
this.updateType = updateType;
this.target = Optional.of(target);
this.target = Optional.of(new UpdateTarget(targetName, targetTable));
}

public void resetUpdateType()
Expand All @@ -222,8 +224,9 @@ public void resetUpdateType()

public boolean isDeleteTarget(Table table)
{
QualifiedObjectName name = tables.get(NodeRef.of(table)).getName();
return "DELETE".equals(updateType) && Optional.of(name).equals(target);
return "DELETE".equals(updateType) &&
target.orElseThrow(() -> new IllegalStateException("Update target not set"))
.getTable().orElseThrow(() -> new IllegalStateException("Table reference not set in update target")) == table; // intentional comparison by reference
}

public boolean isSkipMaterializedViewRefresh()
Expand Down Expand Up @@ -1389,4 +1392,26 @@ public String getAuthorization()
return authorization;
}
}

private static class UpdateTarget
{
private final QualifiedObjectName name;
private final Optional<Table> table;

public UpdateTarget(QualifiedObjectName name, Optional<Table> table)
{
this.name = requireNonNull(name, "name is null");
this.table = requireNonNull(table, "table is null");
}

public QualifiedObjectName getName()
{
return name;
}

public Optional<Table> getTable()
{
return table;
}
}
}
14 changes: 7 additions & 7 deletions presto-main/src/main/java/io/prestosql/sql/analyzer/StatementAnalyzer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ protected Scope visitInsert(Insert insert, Optional<Scope> scope)
// analyze the query that creates the data
Scope queryScope = analyze(insert.getQuery(), createScope(scope));

analysis.setUpdateType("INSERT", targetTable);
analysis.setUpdateType("INSERT", targetTable, Optional.empty());

// verify the insert destination columns match the query
Optional<TableHandle> targetTableHandle = metadata.getTableHandle(session, targetTable);
Expand Down Expand Up @@ -485,7 +485,7 @@ protected Scope visitRefreshMaterializedView(RefreshMaterializedView refreshMate
Query query = parseView(optionalView.get().getOriginalSql(), name, refreshMaterializedView);
Scope queryScope = process(query, scope);

analysis.setUpdateType("INSERT", targetTable);
analysis.setUpdateType("INSERT", targetTable, Optional.empty());

// verify the insert destination columns match the query
Optional<TableHandle> targetTableHandle = metadata.getTableHandle(session, targetTable);
Expand Down Expand Up @@ -625,7 +625,7 @@ protected Scope visitDelete(Delete node, Optional<Scope> scope)
Scope tableScope = analyzer.analyzeForUpdate(table, scope);
node.getWhere().ifPresent(where -> analyzeWhere(node, tableScope, where));

analysis.setUpdateType("DELETE", tableName);
analysis.setUpdateType("DELETE", tableName, Optional.of(table));

return createAndAssignScope(node, scope, Field.newUnqualified("rows", BIGINT));
}
Expand All @@ -634,7 +634,7 @@ protected Scope visitDelete(Delete node, Optional<Scope> scope)
protected Scope visitAnalyze(Analyze node, Optional<Scope> scope)
{
QualifiedObjectName tableName = createQualifiedObjectName(session, node, node.getTableName());
analysis.setUpdateType("ANALYZE", tableName);
analysis.setUpdateType("ANALYZE", tableName, Optional.empty());

// verify the target table exists and it's not a view
if (metadata.getView(session, tableName).isPresent()) {
Expand Down Expand Up @@ -679,7 +679,7 @@ protected Scope visitCreateTableAsSelect(CreateTableAsSelect node, Optional<Scop
{
// turn this into a query that has a new table writer node on top.
QualifiedObjectName targetTable = createQualifiedObjectName(session, node, node.getName());
analysis.setUpdateType("CREATE TABLE", targetTable);
analysis.setUpdateType("CREATE TABLE", targetTable, Optional.empty());

Optional<TableHandle> targetTableHandle = metadata.getTableHandle(session, targetTable);
if (targetTableHandle.isPresent()) {
Expand Down Expand Up @@ -773,7 +773,7 @@ protected Scope visitCreateTableAsSelect(CreateTableAsSelect node, Optional<Scop
protected Scope visitCreateView(CreateView node, Optional<Scope> scope)
{
QualifiedObjectName viewName = createQualifiedObjectName(session, node, node.getName());
analysis.setUpdateType("CREATE VIEW", viewName);
analysis.setUpdateType("CREATE VIEW", viewName, Optional.empty());

// analyze the query that creates the view
StatementAnalyzer analyzer = new StatementAnalyzer(analysis, metadata, sqlParser, groupProvider, accessControl, session, warningCollector, CorrelationSupport.ALLOWED);
Expand Down Expand Up @@ -958,7 +958,7 @@ protected Scope visitCall(Call node, Optional<Scope> scope)
protected Scope visitCreateMaterializedView(CreateMaterializedView node, Optional<Scope> scope)
{
QualifiedObjectName viewName = createQualifiedObjectName(session, node, node.getName());
analysis.setUpdateType("CREATE MATERIALIZED VIEW", viewName);
analysis.setUpdateType("CREATE MATERIALIZED VIEW", viewName, Optional.empty());

if (node.isReplace() && node.isNotExists()) {
throw semanticException(NOT_SUPPORTED, node, "'CREATE OR REPLACE' and 'IF NOT EXISTS' clauses can not be used together");
Expand Down

0 comments on commit b51b6da

Please sign in to comment.