diff --git a/dcm/src/main/java/com/vmware/dcm/ViewsWithAnnotations.java b/dcm/src/main/java/com/vmware/dcm/ViewsWithAnnotations.java index 5835664f..c66acf7a 100644 --- a/dcm/src/main/java/com/vmware/dcm/ViewsWithAnnotations.java +++ b/dcm/src/main/java/com/vmware/dcm/ViewsWithAnnotations.java @@ -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; @@ -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() { @@ -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"), @@ -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; } } \ No newline at end of file diff --git a/dcm/src/main/java/com/vmware/dcm/compiler/SyntaxChecking.java b/dcm/src/main/java/com/vmware/dcm/compiler/SyntaxChecking.java index ed27498a..26d4db2c 100644 --- a/dcm/src/main/java/com/vmware/dcm/compiler/SyntaxChecking.java +++ b/dcm/src/main/java/com/vmware/dcm/compiler/SyntaxChecking.java @@ -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; @@ -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 { + 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; } @@ -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); } } \ No newline at end of file diff --git a/dcm/src/test/java/com/vmware/dcm/compiler/SyntaxCheckTest.java b/dcm/src/test/java/com/vmware/dcm/compiler/SyntaxCheckTest.java new file mode 100644 index 00000000..fa5a2796 --- /dev/null +++ b/dcm/src/test/java/com/vmware/dcm/compiler/SyntaxCheckTest.java @@ -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 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 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)}]})")); + } + } +}