Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Commit

Permalink
dcm: show the exact location of syntax errors
Browse files Browse the repository at this point in the history 
Signed-off-by: Lalith Suresh <[email protected]>
  • Loading branch information
@lalithsuresh
lalithsuresh committed Jul 2, 2021
1 parent d300584 commit 5c0b29e
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 32 deletions.
38 changes: 22 additions & 16 deletions dcm/src/main/java/com/vmware/dcm/ViewsWithAnnotations.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

package com.vmware.dcm;

import com.facebook.presto.sql.SqlFormatter;
import com.facebook.presto.sql.parser.ParsingOptions;
import com.facebook.presto.sql.parser.SqlParser;
import com.facebook.presto.sql.tree.CreateView;
Expand All @@ -28,18 +27,33 @@ public class ViewsWithAnnotations {
private static final ParsingOptions OPTIONS = ParsingOptions.builder().build();
private final CreateView createView;
private final boolean isObjective;
private final String inputView;
@Nullable private final String inputConstraint;
@Nullable private final Expression constraint;

private ViewsWithAnnotations(final CreateView createView) {
private ViewsWithAnnotations(final CreateView createView, final String inputView) {
this.createView = createView;
this.constraint = null;
this.inputConstraint = null;
this.isObjective = false;
this.inputView = inputView;
}

private ViewsWithAnnotations(final CreateView createView, final Expression constraint, final boolean isObjective) {
private ViewsWithAnnotations(final CreateView createView, final Expression constraint, final boolean isObjective,
final String inputView, final String inputConstraint) {
this.createView = createView;
this.isObjective = isObjective;
this.constraint = constraint;
this.inputView = inputView;
this.inputConstraint = inputConstraint;
}

public String getInputView() {
return inputView;
}

public String getInputConstraint() {
return inputConstraint;
}

public CreateView getCreateView() {
Expand Down Expand Up @@ -80,7 +94,8 @@ public static ViewsWithAnnotations fromString(final String view) {
OPTIONS);
final String checkExpressionStr = splitQueryOnCheck.get(1);
final Expression checkExpression = PARSER.createExpression(checkExpressionStr, OPTIONS);
return new ViewsWithAnnotations(createViewStatement, checkExpression, false);
return new ViewsWithAnnotations(createViewStatement, checkExpression, false,
relationWithoutConstraint, checkExpressionStr);
}
else if (splitQueryOnMaximize.size() == 2) {
Preconditions.checkState(!view.toLowerCase(Locale.ROOT).contains("check"),
Expand All @@ -90,24 +105,15 @@ else if (splitQueryOnMaximize.size() == 2) {
OPTIONS);
final String maximizeExpressionStr = splitQueryOnMaximize.get(1);
final Expression maximizeExpression = PARSER.createExpression(maximizeExpressionStr, OPTIONS);
return new ViewsWithAnnotations(createViewStatement, maximizeExpression, true);
return new ViewsWithAnnotations(createViewStatement, maximizeExpression, true,
relationWithoutConstraint, maximizeExpressionStr);
} else {
Preconditions.checkState(!view.toLowerCase(Locale.ROOT).contains("maximize") &&
!view.toLowerCase(Locale.ROOT).contains("check"),
"Unexpected query: non-constraint view cannot have a check clause or a maximize " +
"annotation: " + view);
final CreateView createViewStatement = (CreateView) PARSER.createStatement(view, OPTIONS);
return new ViewsWithAnnotations(createViewStatement);
}
}

@Override
public String toString() {
final String createViewStr = SqlFormatter.formatSql(createView, Optional.empty());
if (constraint != null) {
final String constraintStr = SqlFormatter.formatSql(constraint, Optional.empty());
return createViewStr + (isObjective ? "MAXIMIZE " + constraintStr : "CHECK " + constraintStr);
return new ViewsWithAnnotations(createViewStatement, view);
}
return createViewStr;
}
}
40 changes: 24 additions & 16 deletions dcm/src/main/java/com/vmware/dcm/compiler/SyntaxChecking.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

package com.vmware.dcm.compiler;

import com.facebook.presto.sql.SqlFormatter;
import com.facebook.presto.sql.tree.AliasedRelation;
import com.facebook.presto.sql.tree.AllColumns;
import com.facebook.presto.sql.tree.ArithmeticBinaryExpression;
Expand Down Expand Up @@ -39,24 +38,41 @@
import com.vmware.dcm.ViewsWithAnnotations;

import javax.annotation.Nullable;
import java.util.Optional;

/*
* Checks if the parsed AST only uses the supported subset of SQL to specify policies
*/
public class SyntaxChecking extends AstVisitor<Boolean, Void> {
private final ViewsWithAnnotations view;
private final String componentType;
@Nullable Node lastTraversedNode = null;

SyntaxChecking(final ViewsWithAnnotations view, final String componentType) {
this.view = view;
this.componentType = componentType;
}

@Override
public Boolean process(final Node node, @Nullable final Void context) {
this.lastTraversedNode = node;
final Boolean ret = super.process(node, context);
if (ret == null || !ret) {
final NodeLocation nodeLocation = lastTraversedNode.getLocation().get();
final String err = String.format("Unexpected AST type %s (%s) at line number %s and column number %s",
lastTraversedNode.getClass(), lastTraversedNode, nodeLocation.getLineNumber(),
nodeLocation.getColumnNumber());
throw new ModelException(err);
final String[] lines = componentType.equals("CREATE VIEW") ? view.getInputView().split("\n")
: view.getInputConstraint().split("\n");
final int lineNo = nodeLocation.getLineNumber() - 1;
final int colNo = nodeLocation.getColumnNumber();
final String faultyLine = lines[lineNo];
final String error = String.format("---> Unexpected AST type %s (%s)",
lastTraversedNode.getClass(), lastTraversedNode);
final String withUnderline = faultyLine.substring(0, colNo - 1) + "\033[4m" +
faultyLine.substring(colNo - 1) +
String.format("%13s", "") + "\033[0m" + String.format("%10s", error);
lines[lineNo] = withUnderline;
final String inputStringWithUnderlineAndError = String.join("\n", lines);
final String full = String.format("Unsupported SQL syntax in view \"%s\":" +
"%n%s", view.getCreateView().getName(), inputStringWithUnderlineAndError);
throw new ModelException(full);
}
return ret;
}
Expand Down Expand Up @@ -213,15 +229,7 @@ public static void apply(final ViewsWithAnnotations view) {
}

private static void check(final ViewsWithAnnotations view, final Node part, final String partType) {
try {
final SyntaxChecking validQuery = new SyntaxChecking();
validQuery.process(view.getCreateView());
} catch (final ModelException inner) {
final String err = String.format("Unsupported SQL syntax in view \"%s\"." +
"%nIn %s component %s" +
"%s", view.getCreateView().getName(), partType,
SqlFormatter.formatSql(part, Optional.empty()), inner.getMessage());
throw new ModelException(err);
}
final SyntaxChecking validQuery = new SyntaxChecking(view, partType);
validQuery.process(part);
}
}
59 changes: 59 additions & 0 deletions dcm/src/test/java/com/vmware/dcm/compiler/SyntaxCheckTest.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/*
* Copyright 2018-2021 VMware, Inc. All Rights Reserved.
* SPDX-License-Identifier: BSD-2
*/

package com.vmware.dcm.compiler;

import com.vmware.dcm.Model;
import com.vmware.dcm.ModelException;
import org.jooq.DSLContext;
import org.jooq.impl.DSL;
import org.junit.jupiter.api.Test;

import java.util.List;

import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;

public class SyntaxCheckTest {

@Test
public void testUnsupportedCheckSyntax() {
final DSLContext conn = DSL.using("jdbc:h2:mem:");
conn.execute("create table t1(c1 varchar(100), controllable__dummy integer)");
final List<String> views = List.of("CREATE VIEW constraint_with_like AS " +
"SELECT * FROM t1 " +
"CHECK c1 LIKE 'node'");
try {
Model.build(conn, views);
fail();
} catch (final ModelException err) {
assertTrue(err.getMessage()
.contains("---> Unexpected AST type class "));
assertTrue(err.getMessage()
.contains("com.facebook.presto.sql.tree.LikePredicate ((c1 LIKE 'node'))"));
}
}

@Test
public void testUnsupportedJoinSyntax() {
final DSLContext conn = DSL.using("jdbc:h2:mem:");
conn.execute("create table t1(c1 varchar(100), controllable__dummy integer)");
conn.execute("create table t2(c2 varchar(100))");

final List<String> views = List.of("CREATE VIEW constraint_with_left_join AS\n" +
"SELECT * FROM t1 LEFT JOIN t2 on c1 = c2 " +
"CHECK c1 LIKE 'node'");
try {
Model.build(conn, views);
fail();
} catch (final ModelException err) {
assertTrue(err.getMessage()
.contains("---> Unexpected AST type class "));
assertTrue(err.getMessage()
.contains("com.facebook.presto.sql.tree.Join " +
"(Join{type=LEFT, left=Table{t1}, right=Table{t2}, criteria=Optional[JoinOn{(c1 = c2)}]})"));
}
}
}

0 comments on commit 5c0b29e

Please sign in to comment.