From 334361ba26ee9f04547ae5008e6d061bee843823 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20B=C3=A4r?= Date: Tue, 9 Oct 2018 11:03:38 +0200 Subject: [PATCH 1/6] PMI-95: Support basic inserting with values. --- src/main/java/com/exasol/sql/dml/Insert.java | 30 ++++++++-- .../java/com/exasol/sql/dml/InsertValues.java | 52 ++++++++++++++++ .../com/exasol/sql/dml/InsertVisitor.java | 4 ++ .../sql/dml/rendering/InsertRenderer.java | 17 ++++++ .../com/exasol/sql/dql/ValueExpression.java | 24 -------- .../sql/dql/rendering/SelectRenderer.java | 27 +-------- .../expression/AbstractValueExpression.java | 15 +++++ .../java/com/exasol/sql/expression/Value.java | 31 ++++++++++ .../sql/expression/ValueExpression.java | 7 +++ .../expression/ValueExpressionVisitor.java | 8 +++ .../rendering/AbstractExpressionRenderer.java | 60 +++++++++++++++++++ .../rendering/BooleanExpressionRenderer.java | 44 +------------- .../rendering/ValueExpressionRenderer.java | 26 ++++++++ .../rendering/AbstractFragmentRenderer.java | 18 ++++-- .../dml/rendering/TestInsertRendering.java | 15 +++++ 15 files changed, 277 insertions(+), 101 deletions(-) create mode 100644 src/main/java/com/exasol/sql/dml/InsertValues.java delete mode 100644 src/main/java/com/exasol/sql/dql/ValueExpression.java create mode 100644 src/main/java/com/exasol/sql/expression/AbstractValueExpression.java create mode 100644 src/main/java/com/exasol/sql/expression/Value.java create mode 100644 src/main/java/com/exasol/sql/expression/ValueExpression.java create mode 100644 src/main/java/com/exasol/sql/expression/ValueExpressionVisitor.java create mode 100644 src/main/java/com/exasol/sql/expression/rendering/AbstractExpressionRenderer.java create mode 100644 src/main/java/com/exasol/sql/expression/rendering/ValueExpressionRenderer.java diff --git a/src/main/java/com/exasol/sql/dml/Insert.java b/src/main/java/com/exasol/sql/dml/Insert.java index 2c83517e..8a038d9d 100644 --- a/src/main/java/com/exasol/sql/dml/Insert.java +++ b/src/main/java/com/exasol/sql/dml/Insert.java @@ -8,7 +8,8 @@ */ public class Insert extends AbstractFragment implements SqlStatement, InsertFragment { private final Table table; - private InsertFields fields; + private InsertFields insertFields; + private InsertValues insertValues; /** * Create a new instance of an {@link Insert} statement @@ -27,10 +28,10 @@ public Insert(final String tableName) { * @return this for fluent programming */ public synchronized Insert field(final String... names) { - if (this.fields == null) { - this.fields = new InsertFields(this); + if (this.insertFields == null) { + this.insertFields = new InsertFields(this); } - this.fields.add(names); + this.insertFields.add(names); return this; } @@ -49,8 +50,25 @@ public void accept(final InsertVisitor visitor) { if (this.table != null) { this.table.accept(visitor); } - if (this.fields != null) { - this.fields.accept(visitor); + if (this.insertFields != null) { + this.insertFields.accept(visitor); } + if (this.insertValues != null) { + this.insertValues.accept(visitor); + } + } + + /** + * Insert a list of concrete values + * + * @param values values to be inserted + * @return this for fluent programming + */ + public synchronized Insert values(final Object... values) { + if (this.insertValues == null) { + this.insertValues = new InsertValues(this); + } + this.insertValues.add(values); + return this; } } \ No newline at end of file diff --git a/src/main/java/com/exasol/sql/dml/InsertValues.java b/src/main/java/com/exasol/sql/dml/InsertValues.java new file mode 100644 index 00000000..6a539163 --- /dev/null +++ b/src/main/java/com/exasol/sql/dml/InsertValues.java @@ -0,0 +1,52 @@ +package com.exasol.sql.dml; + +import java.util.ArrayList; +import java.util.List; + +import com.exasol.sql.AbstractFragment; +import com.exasol.sql.Fragment; +import com.exasol.sql.expression.Value; +import com.exasol.sql.expression.ValueExpression; + +/** + * Container class for values to be inserted by an INSERT statement. + */ +public class InsertValues extends AbstractFragment implements InsertFragment { + private final List values = new ArrayList<>(); + + /** + * Create a new instance of {@link InsertValues + * + * @param root root SQL statement + */ + public InsertValues(final Fragment root) { + super(root); + } + + /** + * Add one or more values + * + * @param values values + */ + public void add(final Object... values) { + for (final Object value : values) { + this.getValues().add(new Value(value)); + } + } + + /** + * Get the values + * + * @return value + */ + public List getValues() { + return this.values; + } + + @Override + public void accept(final InsertVisitor visitor) { + visitor.visit(this); + // sub-expression left out intentionally + visitor.leave(this); + } +} \ No newline at end of file diff --git a/src/main/java/com/exasol/sql/dml/InsertVisitor.java b/src/main/java/com/exasol/sql/dml/InsertVisitor.java index 71731729..d4ce8970 100644 --- a/src/main/java/com/exasol/sql/dml/InsertVisitor.java +++ b/src/main/java/com/exasol/sql/dml/InsertVisitor.java @@ -8,4 +8,8 @@ public interface InsertVisitor extends FragmentVisitor { public void visit(InsertFields insertFields); public void leave(InsertFields insertFields); + + public void visit(InsertValues insertValues); + + public void leave(InsertValues insertValues); } \ No newline at end of file diff --git a/src/main/java/com/exasol/sql/dml/rendering/InsertRenderer.java b/src/main/java/com/exasol/sql/dml/rendering/InsertRenderer.java index 02655c53..d498f292 100644 --- a/src/main/java/com/exasol/sql/dml/rendering/InsertRenderer.java +++ b/src/main/java/com/exasol/sql/dml/rendering/InsertRenderer.java @@ -3,6 +3,7 @@ import com.exasol.sql.Field; import com.exasol.sql.Table; import com.exasol.sql.dml.*; +import com.exasol.sql.expression.ValueExpression; import com.exasol.sql.rendering.AbstractFragmentRenderer; import com.exasol.sql.rendering.StringRendererConfig; @@ -38,10 +39,26 @@ public void visit(final Field field) { @Override public void visit(final InsertFields insertFields) { append(" ("); + setLastVisited(insertFields); } @Override public void leave(final InsertFields insertFields) { append(")"); } + + @Override + public void visit(final InsertValues insertValues) { + appendKeyWord(" VALUES "); + for (final ValueExpression expression : insertValues.getValues()) { + appendCommaWhenNeeded(insertValues); + appendRenderedValueExpression(expression); + setLastVisited(insertValues); + } + } + + @Override + public void leave(final InsertValues insertValues) { + // intentionally empty + } } \ No newline at end of file diff --git a/src/main/java/com/exasol/sql/dql/ValueExpression.java b/src/main/java/com/exasol/sql/dql/ValueExpression.java deleted file mode 100644 index 2a6d59db..00000000 --- a/src/main/java/com/exasol/sql/dql/ValueExpression.java +++ /dev/null @@ -1,24 +0,0 @@ -package com.exasol.sql.dql; - -import com.exasol.sql.*; - -/** - * Abstract base class for all types of value expressions - */ -public abstract class ValueExpression extends AbstractFragment implements Fragment { - /** - * Create a new instance of a {@link ValueExpression} - */ - public ValueExpression() { - super(null); - } - - /** - * Create a new instance of a {@link ValueExpression} - * - * @param root root SQL statement this expression belongs to - */ - public ValueExpression(final SqlStatement root) { - super(root); - } -} \ No newline at end of file diff --git a/src/main/java/com/exasol/sql/dql/rendering/SelectRenderer.java b/src/main/java/com/exasol/sql/dql/rendering/SelectRenderer.java index f7bf7601..55dc760d 100644 --- a/src/main/java/com/exasol/sql/dql/rendering/SelectRenderer.java +++ b/src/main/java/com/exasol/sql/dql/rendering/SelectRenderer.java @@ -2,7 +2,8 @@ import java.util.Optional; -import com.exasol.sql.*; +import com.exasol.sql.Field; +import com.exasol.sql.Table; import com.exasol.sql.dql.*; import com.exasol.sql.rendering.AbstractFragmentRenderer; import com.exasol.sql.rendering.StringRendererConfig; @@ -82,28 +83,4 @@ public void visit(final LimitClause limit) { append(limit.getCount()); setLastVisited(limit); } - - /** - * Create a renderer for the given {@link Fragment} and render it. - * - * @param fragment SQL statement fragment to be rendered - * @return rendered statement - */ - public static String render(final Fragment fragment) { - return render(fragment, StringRendererConfig.createDefault()); - } - - /** - * Create a renderer for the given {@link Fragment} and render it. - * - * @param fragment SQL statement fragment to be rendered - * @param config renderer configuration - * @return rendered statement - */ - public static String render(final Fragment fragment, final StringRendererConfig config) { - assert (fragment instanceof SelectFragment); - final SelectRenderer renderer = new SelectRenderer(config); - ((SelectFragment) fragment).accept(renderer); - return renderer.render(); - } } \ No newline at end of file diff --git a/src/main/java/com/exasol/sql/expression/AbstractValueExpression.java b/src/main/java/com/exasol/sql/expression/AbstractValueExpression.java new file mode 100644 index 00000000..fcf1f26f --- /dev/null +++ b/src/main/java/com/exasol/sql/expression/AbstractValueExpression.java @@ -0,0 +1,15 @@ +package com.exasol.sql.expression; + +import com.exasol.util.AbstractBottomUpTreeNode; + +/** + * Abstract base class for all types of value expressions + */ +public abstract class AbstractValueExpression extends AbstractBottomUpTreeNode implements ValueExpression { + /** + * Create a new instance of a {@link AbstractValueExpression} + */ + public AbstractValueExpression() { + super(); + } +} \ No newline at end of file diff --git a/src/main/java/com/exasol/sql/expression/Value.java b/src/main/java/com/exasol/sql/expression/Value.java new file mode 100644 index 00000000..c98e39f1 --- /dev/null +++ b/src/main/java/com/exasol/sql/expression/Value.java @@ -0,0 +1,31 @@ +package com.exasol.sql.expression; + +/** + * This class represents a concrete value link a number or a text. + */ +public class Value extends AbstractValueExpression { + private final Object value; + + /** + * Create a new instance of a {@link Value} + * + * @param value contained value + */ + public Value(final Object value) { + this.value = value; + } + + /** + * Get the value + * + * @return value + */ + public Object get() { + return this.value; + } + + @Override + public void accept(final ValueExpressionVisitor visitor) { + visitor.visit(this); + } +} \ No newline at end of file diff --git a/src/main/java/com/exasol/sql/expression/ValueExpression.java b/src/main/java/com/exasol/sql/expression/ValueExpression.java new file mode 100644 index 00000000..e705c907 --- /dev/null +++ b/src/main/java/com/exasol/sql/expression/ValueExpression.java @@ -0,0 +1,7 @@ +package com.exasol.sql.expression; + +import com.exasol.util.TreeNode; + +public interface ValueExpression extends TreeNode { + void accept(ValueExpressionVisitor visitor); +} diff --git a/src/main/java/com/exasol/sql/expression/ValueExpressionVisitor.java b/src/main/java/com/exasol/sql/expression/ValueExpressionVisitor.java new file mode 100644 index 00000000..5009eeda --- /dev/null +++ b/src/main/java/com/exasol/sql/expression/ValueExpressionVisitor.java @@ -0,0 +1,8 @@ +package com.exasol.sql.expression; + +/** + * Visitor interface for a {@link BooleanTerm} + */ +public interface ValueExpressionVisitor { + void visit(Value value); +} \ No newline at end of file diff --git a/src/main/java/com/exasol/sql/expression/rendering/AbstractExpressionRenderer.java b/src/main/java/com/exasol/sql/expression/rendering/AbstractExpressionRenderer.java new file mode 100644 index 00000000..ab36958f --- /dev/null +++ b/src/main/java/com/exasol/sql/expression/rendering/AbstractExpressionRenderer.java @@ -0,0 +1,60 @@ +package com.exasol.sql.expression.rendering; + +import java.util.Stack; + +import com.exasol.sql.expression.BooleanExpression; +import com.exasol.sql.rendering.StringRendererConfig; + +/** + * Common base class for expression renderers + */ +public class AbstractExpressionRenderer { + protected final StringRendererConfig config; + protected final StringBuilder builder = new StringBuilder(); + protected final Stack connectorStack = new Stack<>(); + + public AbstractExpressionRenderer(final StringRendererConfig config) { + this.config = config; + } + + protected void appendKeyword(final String keyword) { + this.builder.append(this.config.produceLowerCase() ? keyword.toLowerCase() : keyword); + } + + protected void connect(final BooleanExpression expression) { + if (expression.isChild() && !expression.isFirstSibling()) { + appendConnector(); + } + } + + private void appendConnector() { + if (!this.connectorStack.isEmpty()) { + appendKeyword(this.connectorStack.peek()); + } + } + + protected void appendLiteral(final String string) { + this.builder.append(string); + } + + protected void startParenthesis() { + this.builder.append("("); + } + + protected void endParenthesis(final BooleanExpression expression) { + this.builder.append(")"); + } + + /** + * Render expression to a string + * + * @return rendered string + */ + public String render() { + return this.builder.toString(); + } + + protected void append(final String string) { + this.builder.append(string); + } +} \ No newline at end of file diff --git a/src/main/java/com/exasol/sql/expression/rendering/BooleanExpressionRenderer.java b/src/main/java/com/exasol/sql/expression/rendering/BooleanExpressionRenderer.java index 14657429..1588a707 100644 --- a/src/main/java/com/exasol/sql/expression/rendering/BooleanExpressionRenderer.java +++ b/src/main/java/com/exasol/sql/expression/rendering/BooleanExpressionRenderer.java @@ -1,25 +1,15 @@ package com.exasol.sql.expression.rendering; -import java.util.Stack; - import com.exasol.sql.expression.*; import com.exasol.sql.rendering.StringRendererConfig; -public class BooleanExpressionRenderer implements BooleanExpressionVisitor { - private final StringRendererConfig config; - private final StringBuilder builder = new StringBuilder(); - private final Stack connectorStack = new Stack<>(); - +public class BooleanExpressionRenderer extends AbstractExpressionRenderer implements BooleanExpressionVisitor { public BooleanExpressionRenderer(final StringRendererConfig config) { - this.config = config; + super(config); } public BooleanExpressionRenderer() { - this.config = new StringRendererConfig.Builder().build(); - } - - private void appendKeyword(final String keyword) { - this.builder.append(this.config.produceLowerCase() ? keyword.toLowerCase() : keyword); + this(new StringRendererConfig.Builder().build()); } @Override @@ -74,39 +64,11 @@ public void visit(final Literal literal) { appendLiteral(literal.toString()); } - private void connect(final BooleanExpression expression) { - if (expression.isChild() && !expression.isFirstSibling()) { - appendConnector(); - } - } - @Override public void leave(final Literal literal) { // intentionally empty } - private void appendConnector() { - if (!this.connectorStack.isEmpty()) { - appendKeyword(this.connectorStack.peek()); - } - } - - private void appendLiteral(final String string) { - this.builder.append(string); - } - - private void startParenthesis() { - this.builder.append("("); - } - - private void endParenthesis(final BooleanExpression expression) { - this.builder.append(")"); - } - - public String render() { - return this.builder.toString(); - } - @Override public void visit(final Comparison comparison) { connect(comparison); diff --git a/src/main/java/com/exasol/sql/expression/rendering/ValueExpressionRenderer.java b/src/main/java/com/exasol/sql/expression/rendering/ValueExpressionRenderer.java new file mode 100644 index 00000000..d6511737 --- /dev/null +++ b/src/main/java/com/exasol/sql/expression/rendering/ValueExpressionRenderer.java @@ -0,0 +1,26 @@ +package com.exasol.sql.expression.rendering; + +import com.exasol.sql.expression.Value; +import com.exasol.sql.expression.ValueExpressionVisitor; +import com.exasol.sql.rendering.StringRendererConfig; + +/** + * Renderer for common value expressions + */ +public class ValueExpressionRenderer extends AbstractExpressionRenderer implements ValueExpressionVisitor { + public ValueExpressionRenderer(final StringRendererConfig config) { + super(config); + } + + @Override + public void visit(final Value value) { + final Object object = value.get(); + if (object instanceof String) { + append("'"); + append((String) object); + append("'"); + } else { + this.builder.append(value.get().toString()); + } + } +} \ No newline at end of file diff --git a/src/main/java/com/exasol/sql/rendering/AbstractFragmentRenderer.java b/src/main/java/com/exasol/sql/rendering/AbstractFragmentRenderer.java index 4b7f4afe..415e770d 100644 --- a/src/main/java/com/exasol/sql/rendering/AbstractFragmentRenderer.java +++ b/src/main/java/com/exasol/sql/rendering/AbstractFragmentRenderer.java @@ -2,7 +2,9 @@ import com.exasol.sql.Fragment; import com.exasol.sql.expression.BooleanExpression; +import com.exasol.sql.expression.ValueExpression; import com.exasol.sql.expression.rendering.BooleanExpressionRenderer; +import com.exasol.sql.expression.rendering.ValueExpressionRenderer; /** * Abstract base class for SQL fragment renderers @@ -17,11 +19,6 @@ public AbstractFragmentRenderer(final StringRendererConfig config) { this.lastVisited = null; } - @Override - public String render() { - return this.builder.toString(); - } - protected void appendKeyWord(final String keyword) { append(this.config.produceLowerCase() ? keyword.toLowerCase() : keyword); } @@ -53,4 +50,15 @@ protected void appendRenderedExpression(final BooleanExpression expression) { protected void append(final int number) { this.builder.append(number); } + + protected void appendRenderedValueExpression(final ValueExpression expression) { + final ValueExpressionRenderer renderer = new ValueExpressionRenderer(this.config); + expression.accept(renderer); + append(renderer.render()); + } + + @Override + public String render() { + return this.builder.toString(); + } } \ No newline at end of file diff --git a/src/test/java/com/exasol/sql/dml/rendering/TestInsertRendering.java b/src/test/java/com/exasol/sql/dml/rendering/TestInsertRendering.java index b5492f01..53f7ba65 100644 --- a/src/test/java/com/exasol/sql/dml/rendering/TestInsertRendering.java +++ b/src/test/java/com/exasol/sql/dml/rendering/TestInsertRendering.java @@ -1,6 +1,7 @@ package com.exasol.sql.dml.rendering; import static com.exasol.hamcrest.SqlFragmentRenderResultMatcher.rendersTo; +import static com.exasol.hamcrest.SqlFragmentRenderResultMatcher.rendersWithConfigTo; import static org.hamcrest.MatcherAssert.assertThat; import org.junit.jupiter.api.BeforeEach; @@ -8,6 +9,7 @@ import com.exasol.sql.StatementFactory; import com.exasol.sql.dml.Insert; +import com.exasol.sql.rendering.StringRendererConfig; class TestInsertRendering { private static final String PERSON = "person"; @@ -24,9 +26,22 @@ void testInsert() { assertThat(this.insert, rendersTo("INSERT INTO person")); } + // [dsn~rendering.sql.insert~1] + @Test + void testInsertRendersToWithConfig() { + assertThat(this.insert, + rendersWithConfigTo(new StringRendererConfig.Builder().lowerCase(true).build(), "insert into person")); + } + // [dsn~rendering.sql.insert~1] @Test void testInsertFields() { assertThat(this.insert.field("a", "b"), rendersTo("INSERT INTO person (a, b)")); } + + // [dsn~rendering.sql.insert~1] + @Test + void testInsertValues() { + assertThat(this.insert.values(1, "a"), rendersTo("INSERT INTO person VALUES 1, 'a'")); + } } \ No newline at end of file From 5b4eedeb16dbe74b1e3510635b198914b98f1d8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20B=C3=A4r?= Date: Tue, 9 Oct 2018 12:35:56 +0200 Subject: [PATCH 2/6] PMI-95: Added factory methods for renderers. --- .../sql/dml/rendering/InsertRenderer.java | 19 +++++++++++++ .../sql/dql/rendering/SelectRenderer.java | 19 +++++++++++++ .../sql/dml/rendering/TestInsertRenderer.java | 27 +++++++++++++++++++ .../sql/dql/rendering/TestSelectRenderer.java | 27 +++++++++++++++++++ 4 files changed, 92 insertions(+) create mode 100644 src/test/java/com/exasol/sql/dml/rendering/TestInsertRenderer.java create mode 100644 src/test/java/com/exasol/sql/dql/rendering/TestSelectRenderer.java diff --git a/src/main/java/com/exasol/sql/dml/rendering/InsertRenderer.java b/src/main/java/com/exasol/sql/dml/rendering/InsertRenderer.java index d498f292..6a8c8db9 100644 --- a/src/main/java/com/exasol/sql/dml/rendering/InsertRenderer.java +++ b/src/main/java/com/exasol/sql/dml/rendering/InsertRenderer.java @@ -61,4 +61,23 @@ public void visit(final InsertValues insertValues) { public void leave(final InsertValues insertValues) { // intentionally empty } + + /** + * Create an {@link InsertRenderer} using the default renderer configuration + * + * @return insert renderer + */ + public static InsertRenderer create() { + return create(StringRendererConfig.createDefault()); + } + + /** + * Create an {@link InsertRenderer} + * + * @param config renderer configuration + * @return insert renderer + */ + public static InsertRenderer create(final StringRendererConfig config) { + return new InsertRenderer(config); + } } \ No newline at end of file diff --git a/src/main/java/com/exasol/sql/dql/rendering/SelectRenderer.java b/src/main/java/com/exasol/sql/dql/rendering/SelectRenderer.java index 55dc760d..cf83d9fa 100644 --- a/src/main/java/com/exasol/sql/dql/rendering/SelectRenderer.java +++ b/src/main/java/com/exasol/sql/dql/rendering/SelectRenderer.java @@ -83,4 +83,23 @@ public void visit(final LimitClause limit) { append(limit.getCount()); setLastVisited(limit); } + + /** + * Create an {@link SelectRenderer} using the default renderer configuration + * + * @return select renderer + */ + public static SelectRenderer create() { + return create(StringRendererConfig.createDefault()); + } + + /** + * Create an {@link SelectRenderer} + * + * @param config renderer configuration + * @return select renderer + */ + public static SelectRenderer create(final StringRendererConfig config) { + return new SelectRenderer(config); + } } \ No newline at end of file diff --git a/src/test/java/com/exasol/sql/dml/rendering/TestInsertRenderer.java b/src/test/java/com/exasol/sql/dml/rendering/TestInsertRenderer.java new file mode 100644 index 00000000..ec691730 --- /dev/null +++ b/src/test/java/com/exasol/sql/dml/rendering/TestInsertRenderer.java @@ -0,0 +1,27 @@ +package com.exasol.sql.dml.rendering; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.startsWith; + +import org.junit.jupiter.api.Test; + +import com.exasol.sql.StatementFactory; +import com.exasol.sql.dml.Insert; +import com.exasol.sql.rendering.StringRendererConfig; + +class TestInsertRenderer { + @Test + void testCreateWithDefaultConfig() { + assertThat(InsertRenderer.create(), instanceOf(InsertRenderer.class)); + } + + @Test + void testCreateWithConfig() { + final StringRendererConfig config = new StringRendererConfig.Builder().lowerCase(true).build(); + final InsertRenderer renderer = InsertRenderer.create(config); + final Insert insert = StatementFactory.getInstance().insertInto("city"); + insert.accept(renderer); + assertThat(renderer.render(), startsWith("insert")); + } +} \ No newline at end of file diff --git a/src/test/java/com/exasol/sql/dql/rendering/TestSelectRenderer.java b/src/test/java/com/exasol/sql/dql/rendering/TestSelectRenderer.java new file mode 100644 index 00000000..5954bac6 --- /dev/null +++ b/src/test/java/com/exasol/sql/dql/rendering/TestSelectRenderer.java @@ -0,0 +1,27 @@ +package com.exasol.sql.dql.rendering; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.startsWith; + +import org.junit.jupiter.api.Test; + +import com.exasol.sql.StatementFactory; +import com.exasol.sql.dql.Select; +import com.exasol.sql.rendering.StringRendererConfig; + +class TestSelectRenderer { + @Test + void testCreateWithDefaultConfig() { + assertThat(SelectRenderer.create(), instanceOf(SelectRenderer.class)); + } + + @Test + void testCreateWithConfig() { + final StringRendererConfig config = new StringRendererConfig.Builder().lowerCase(true).build(); + final SelectRenderer renderer = SelectRenderer.create(config); + final Select select = StatementFactory.getInstance().select(); + select.accept(renderer); + assertThat(renderer.render(), startsWith("select")); + } +} \ No newline at end of file From d05eccf376655f3d6e1ea1a8bc64d9e4b78ebaf5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20B=C3=A4r?= Date: Wed, 10 Oct 2018 09:36:19 +0200 Subject: [PATCH 3/6] PMI-95: Fixed specification trace and added missing unit tests. --- doc/design.md | 34 ++++++++--- doc/system_requirements.md | 20 +++++- .../com/exasol/sql/UnnamedPlaceholder.java | 10 +++ src/main/java/com/exasol/sql/dml/Insert.java | 61 ++++++++++++++----- .../java/com/exasol/sql/dml/InsertValues.java | 11 ++-- .../sql/dml/rendering/InsertRenderer.java | 4 ++ src/main/java/com/exasol/sql/dql/Select.java | 4 ++ .../sql/dql/rendering/SelectRenderer.java | 1 + .../exasol/sql/expression/BooleanTerm.java | 3 +- .../com/exasol/sql/expression/Comparison.java | 7 ++- .../expression/ValueExpressionVisitor.java | 4 ++ .../rendering/ValueExpressionRenderer.java | 6 ++ .../rendering/AbstractFragmentRenderer.java | 1 + .../dml/rendering/TestInsertRendering.java | 31 ++++++++-- .../java/com/exasol/sql/dql/TestSelect.java | 1 - .../dql/rendering/TestSelectRendering.java | 31 +++++++--- .../sql/expression/TestBooleanTerm.java | 4 +- .../expression/TestComparisonOperator.java | 2 + 18 files changed, 189 insertions(+), 46 deletions(-) create mode 100644 src/main/java/com/exasol/sql/UnnamedPlaceholder.java diff --git a/doc/design.md b/doc/design.md index bf237a2a..4068a95b 100644 --- a/doc/design.md +++ b/doc/design.md @@ -7,17 +7,35 @@ The Data Query Language (DQL) building block is responsible for managing `SELECT` statements. +## Solution Strategy + +### Fluent Programming + +###### Statement Construction With Fluent Programming +`dsn~statement-construction-with-fluent-programming~1` + +All statement builders use the "fluent programming" model, where the return type of each builder step determines the possible next structural elements that can be added. + +Comment: + +This is a design principle that cuts across the whole project. Therefore locating it in a single test or implementation part makes no sense. + +Covers: + +* `req~statement-structure-limited-at-compile-time~1` + ## Runtime View ### Building Select Statements #### Accessing the Clauses That Make Up a SELECT Statement -`dsn~select-statement.accessing-clauses~1` +`dsn~select-statement.out-of-order-clauses~1` -The DQL statement component allows getting the following clauses, provided that they already exist: +`SELECT` commands allow attaching the following clauses in any order: * `FROM` clause * `WHERE` clause +* `LIMIT` clause Covers: @@ -29,6 +47,11 @@ Tags: Select Statement Builder ### Building Boolean Expressions +#### Forwarded Requirements + +* `dsn --> impl, utest: req~boolean-operators~1` +* `dsn --> impl, utest: req~comparison-operations~1` + #### Constructing Boolean Comparison Operations From Operator Strings `dsn~boolean-operation.comparison.constructing-from-strings~1` @@ -57,11 +80,6 @@ Covers: Needs: impl, utest -#### Forwarded Requirements - -* `dsn --> impl, utest : req~comparison-operations~1` -* `dsn --> impl, utest : req~boolean-operators~1` - ### Building INSERT Statements #### Forwarded Requirements @@ -73,6 +91,6 @@ Needs: impl, utest #### Forwarded Requirements -* `dsn --> req~rendering.sql.configurable-case~1` +* `dsn --> impl, utest: req~rendering.sql.configurable-case~1` * `dsn --> impl, utest: req~rendering.sql.select~1` * `dsn --> impl, utest: req~rendering.sql.insert~1` diff --git a/doc/system_requirements.md b/doc/system_requirements.md index 9e7a0562..b8d9ef20 100644 --- a/doc/system_requirements.md +++ b/doc/system_requirements.md @@ -99,7 +99,22 @@ This is necessary since complex statements are usually build as a result of mult Covers: -* [feat~statment-definition~1](#statement-definition) +* [feat~statement-definition~1](#statement-definition) + +Needs: dsn + +#### Statement Structure Limited at Compile-time +`req~statement-structure-limited-at-compile-time~1` + +ESB lets users create only valid statement structures at compile-time. + +Rationale: + +If users can't get illegal structures to compile, they don't need to spend time debugging them later. + +Covers: + +* [feat~compile-time-error-checking~1](#compile-time-error-checking) Needs: dsn @@ -187,7 +202,8 @@ Covers: Needs: dsn -* Upper case / lower case +#### TODO + * One line / pretty #### SELECT Statement Rendering diff --git a/src/main/java/com/exasol/sql/UnnamedPlaceholder.java b/src/main/java/com/exasol/sql/UnnamedPlaceholder.java new file mode 100644 index 00000000..415429d9 --- /dev/null +++ b/src/main/java/com/exasol/sql/UnnamedPlaceholder.java @@ -0,0 +1,10 @@ +package com.exasol.sql; + +import com.exasol.sql.expression.*; + +public class UnnamedPlaceholder extends AbstractValueExpression implements ValueExpression { + @Override + public void accept(final ValueExpressionVisitor visitor) { + visitor.visit(this); + } +} \ No newline at end of file diff --git a/src/main/java/com/exasol/sql/dml/Insert.java b/src/main/java/com/exasol/sql/dml/Insert.java index 8a038d9d..49100441 100644 --- a/src/main/java/com/exasol/sql/dml/Insert.java +++ b/src/main/java/com/exasol/sql/dml/Insert.java @@ -6,6 +6,7 @@ /** * This class implements an SQL {@link Select} statement */ +// [impl->dsn~insert-statements~1] public class Insert extends AbstractFragment implements SqlStatement, InsertFragment { private final Table table; private InsertFields insertFields; @@ -44,26 +45,13 @@ public String getTableName() { return this.table.getName(); } - @Override - public void accept(final InsertVisitor visitor) { - visitor.visit(this); - if (this.table != null) { - this.table.accept(visitor); - } - if (this.insertFields != null) { - this.insertFields.accept(visitor); - } - if (this.insertValues != null) { - this.insertValues.accept(visitor); - } - } - /** * Insert a list of concrete values * * @param values values to be inserted * @return this for fluent programming */ + // [impl->dsn~values-as-insert-source~1] public synchronized Insert values(final Object... values) { if (this.insertValues == null) { this.insertValues = new InsertValues(this); @@ -71,4 +59,49 @@ public synchronized Insert values(final Object... values) { this.insertValues.add(values); return this; } + + /** + * Add an unnamed value placeholder to the value list (this is useful for prepared statements) + * + * @return this for fluent programming + */ + // [impl->dsn~values-as-insert-source~1] + public synchronized Insert valuePlaceholder() { + if (this.insertValues == null) { + this.insertValues = new InsertValues(this); + } + this.insertValues.addPlaceholder(); + return this; + } + + /** + * Add a given number unnamed value placeholder to the value list (this is useful for prepared statements) + * + * @param amount number of placeholders to be added + * @return this for fluent programming + */ + // [impl->dsn~values-as-insert-source~1] + public synchronized Insert valuePlaceholders(final int amount) { + if (this.insertValues == null) { + this.insertValues = new InsertValues(this); + } + for (int i = 0; i < amount; ++i) { + this.insertValues.addPlaceholder(); + } + return this; + } + + @Override + public void accept(final InsertVisitor visitor) { + visitor.visit(this); + if (this.table != null) { + this.table.accept(visitor); + } + if (this.insertFields != null) { + this.insertFields.accept(visitor); + } + if (this.insertValues != null) { + this.insertValues.accept(visitor); + } + } } \ No newline at end of file diff --git a/src/main/java/com/exasol/sql/dml/InsertValues.java b/src/main/java/com/exasol/sql/dml/InsertValues.java index 6a539163..aa5e4456 100644 --- a/src/main/java/com/exasol/sql/dml/InsertValues.java +++ b/src/main/java/com/exasol/sql/dml/InsertValues.java @@ -3,8 +3,7 @@ import java.util.ArrayList; import java.util.List; -import com.exasol.sql.AbstractFragment; -import com.exasol.sql.Fragment; +import com.exasol.sql.*; import com.exasol.sql.expression.Value; import com.exasol.sql.expression.ValueExpression; @@ -25,7 +24,7 @@ public InsertValues(final Fragment root) { /** * Add one or more values - * + * * @param values values */ public void add(final Object... values) { @@ -36,7 +35,7 @@ public void add(final Object... values) { /** * Get the values - * + * * @return value */ public List getValues() { @@ -49,4 +48,8 @@ public void accept(final InsertVisitor visitor) { // sub-expression left out intentionally visitor.leave(this); } + + public void addPlaceholder() { + this.values.add(new UnnamedPlaceholder()); + } } \ No newline at end of file diff --git a/src/main/java/com/exasol/sql/dml/rendering/InsertRenderer.java b/src/main/java/com/exasol/sql/dml/rendering/InsertRenderer.java index 6a8c8db9..1d44aacb 100644 --- a/src/main/java/com/exasol/sql/dml/rendering/InsertRenderer.java +++ b/src/main/java/com/exasol/sql/dml/rendering/InsertRenderer.java @@ -7,6 +7,10 @@ import com.exasol.sql.rendering.AbstractFragmentRenderer; import com.exasol.sql.rendering.StringRendererConfig; +/** + * The {@link InsertRenderer} turns SQL statement structures in to SQL strings. + */ +// [impl->dsn~rendering.sql.insert~1] public class InsertRenderer extends AbstractFragmentRenderer implements InsertVisitor { /** * Create a new {@link InsertRenderer} with custom render settings. diff --git a/src/main/java/com/exasol/sql/dql/Select.java b/src/main/java/com/exasol/sql/dql/Select.java index 3f54de0e..5cf6b9cb 100644 --- a/src/main/java/com/exasol/sql/dql/Select.java +++ b/src/main/java/com/exasol/sql/dql/Select.java @@ -50,6 +50,7 @@ public Select field(final String... names) { * * @return from clause */ + // [impl->dsn~select-statement.out-of-order-clauses~1] public synchronized FromClause from() { if (this.fromClause == null) { this.fromClause = new FromClause(this); @@ -64,6 +65,7 @@ public synchronized FromClause from() { * @return new instance * @throws IllegalStateException if a limit clause already exists */ + // [impl->dsn~select-statement.out-of-order-clauses~1] public synchronized Select limit(final int count) { if (this.limitClause != null) { throw new IllegalStateException( @@ -81,6 +83,7 @@ public synchronized Select limit(final int count) { * @return thisdsn~select-statement.out-of-order-clauses~1] public synchronized Select limit(final int offset, final int count) { if (this.limitClause != null) { throw new IllegalStateException( @@ -96,6 +99,7 @@ public synchronized Select limit(final int offset, final int count) { * @param expression boolean expression that defines the filter criteria * @return new instance */ + // [impl->dsn~select-statement.out-of-order-clauses~1] public synchronized Select where(final BooleanExpression expression) { if (this.whereClause == null) { this.whereClause = new WhereClause(this, expression); diff --git a/src/main/java/com/exasol/sql/dql/rendering/SelectRenderer.java b/src/main/java/com/exasol/sql/dql/rendering/SelectRenderer.java index cf83d9fa..5073f7bb 100644 --- a/src/main/java/com/exasol/sql/dql/rendering/SelectRenderer.java +++ b/src/main/java/com/exasol/sql/dql/rendering/SelectRenderer.java @@ -11,6 +11,7 @@ /** * The {@link SelectRenderer} turns SQL statement structures in to SQL strings. */ +// [impl->dsn~rendering.sql.select~1] public class SelectRenderer extends AbstractFragmentRenderer implements SelectVisitor { /** * Create a new {@link SelectRenderer} with custom render settings. diff --git a/src/main/java/com/exasol/sql/expression/BooleanTerm.java b/src/main/java/com/exasol/sql/expression/BooleanTerm.java index 0ae285e4..a66e81e3 100644 --- a/src/main/java/com/exasol/sql/expression/BooleanTerm.java +++ b/src/main/java/com/exasol/sql/expression/BooleanTerm.java @@ -1,5 +1,6 @@ package com.exasol.sql.expression; +// [impl->dsn~boolean-operators~1] public abstract class BooleanTerm extends AbstractBooleanExpression { private BooleanTerm() { super(); @@ -50,7 +51,7 @@ public static BooleanExpression compare(final String left, final String operator return new Comparison(ComparisonOperator.ofSymbol(operatorSymbol), Literal.of(left), Literal.of(right)); } - // [dsn~boolean-operation.comparison.constructing-from-enum~1] + // [impl->dsn~boolean-operation.comparison.constructing-from-enum~1] public static BooleanExpression compare(final String left, final ComparisonOperator operator, final String right) { return new Comparison(operator, Literal.of(left), Literal.of(right)); } diff --git a/src/main/java/com/exasol/sql/expression/Comparison.java b/src/main/java/com/exasol/sql/expression/Comparison.java index 0a6381af..0fdf2eb3 100644 --- a/src/main/java/com/exasol/sql/expression/Comparison.java +++ b/src/main/java/com/exasol/sql/expression/Comparison.java @@ -6,6 +6,7 @@ public class Comparison extends AbstractBooleanExpression { private final Literal leftOperand; private final Literal rightOperand; + // [impl->dsn~boolean-operation.comparison.constructing-from-enum~1] public Comparison(final ComparisonOperator equal, final Literal leftOperand, final Literal rightOperand) { this.operator = equal; this.leftOperand = leftOperand; @@ -24,7 +25,7 @@ public void dismissConcrete(final BooleanExpressionVisitor visitor) { /** * Get the left-hand side operator of the comparison - * + * * @return left operator */ public AbstractBooleanExpression getLeftOperand() { @@ -33,7 +34,7 @@ public AbstractBooleanExpression getLeftOperand() { /** * Get the right-hand side operator of the comparison - * + * * @return right operator */ public AbstractBooleanExpression getRightOperand() { @@ -42,7 +43,7 @@ public AbstractBooleanExpression getRightOperand() { /** * Get the comparison operator - * + * * @return comparison operator */ public ComparisonOperator getOperator() { diff --git a/src/main/java/com/exasol/sql/expression/ValueExpressionVisitor.java b/src/main/java/com/exasol/sql/expression/ValueExpressionVisitor.java index 5009eeda..f780eb51 100644 --- a/src/main/java/com/exasol/sql/expression/ValueExpressionVisitor.java +++ b/src/main/java/com/exasol/sql/expression/ValueExpressionVisitor.java @@ -1,8 +1,12 @@ package com.exasol.sql.expression; +import com.exasol.sql.UnnamedPlaceholder; + /** * Visitor interface for a {@link BooleanTerm} */ public interface ValueExpressionVisitor { void visit(Value value); + + void visit(UnnamedPlaceholder unnamedPlaceholder); } \ No newline at end of file diff --git a/src/main/java/com/exasol/sql/expression/rendering/ValueExpressionRenderer.java b/src/main/java/com/exasol/sql/expression/rendering/ValueExpressionRenderer.java index d6511737..53eac8d7 100644 --- a/src/main/java/com/exasol/sql/expression/rendering/ValueExpressionRenderer.java +++ b/src/main/java/com/exasol/sql/expression/rendering/ValueExpressionRenderer.java @@ -1,5 +1,6 @@ package com.exasol.sql.expression.rendering; +import com.exasol.sql.UnnamedPlaceholder; import com.exasol.sql.expression.Value; import com.exasol.sql.expression.ValueExpressionVisitor; import com.exasol.sql.rendering.StringRendererConfig; @@ -23,4 +24,9 @@ public void visit(final Value value) { this.builder.append(value.get().toString()); } } + + @Override + public void visit(final UnnamedPlaceholder unnamedPlaceholder) { + append("?"); + } } \ No newline at end of file diff --git a/src/main/java/com/exasol/sql/rendering/AbstractFragmentRenderer.java b/src/main/java/com/exasol/sql/rendering/AbstractFragmentRenderer.java index 415e770d..08abe2a0 100644 --- a/src/main/java/com/exasol/sql/rendering/AbstractFragmentRenderer.java +++ b/src/main/java/com/exasol/sql/rendering/AbstractFragmentRenderer.java @@ -19,6 +19,7 @@ public AbstractFragmentRenderer(final StringRendererConfig config) { this.lastVisited = null; } + // [impl->dsn~rendering.sql.configurable-case~1] protected void appendKeyWord(final String keyword) { append(this.config.produceLowerCase() ? keyword.toLowerCase() : keyword); } diff --git a/src/test/java/com/exasol/sql/dml/rendering/TestInsertRendering.java b/src/test/java/com/exasol/sql/dml/rendering/TestInsertRendering.java index 53f7ba65..40e96e6e 100644 --- a/src/test/java/com/exasol/sql/dml/rendering/TestInsertRendering.java +++ b/src/test/java/com/exasol/sql/dml/rendering/TestInsertRendering.java @@ -20,28 +20,51 @@ void beforeEach() { this.insert = StatementFactory.getInstance().insertInto(PERSON); } - // [dsn~rendering.sql.insert~1] + // [utest->dsn~rendering.sql.insert~1] @Test void testInsert() { assertThat(this.insert, rendersTo("INSERT INTO person")); } - // [dsn~rendering.sql.insert~1] + // [utest->dsn~rendering.sql.configurable-case~1] @Test void testInsertRendersToWithConfig() { assertThat(this.insert, rendersWithConfigTo(new StringRendererConfig.Builder().lowerCase(true).build(), "insert into person")); } - // [dsn~rendering.sql.insert~1] + // [utest->dsn~rendering.sql.insert~1] @Test void testInsertFields() { assertThat(this.insert.field("a", "b"), rendersTo("INSERT INTO person (a, b)")); } - // [dsn~rendering.sql.insert~1] + // [utest->dsn~rendering.sql.insert~1] + // [utest->dsn~values-as-insert-source~1] @Test void testInsertValues() { assertThat(this.insert.values(1, "a"), rendersTo("INSERT INTO person VALUES 1, 'a'")); } + + // [utest->dsn~rendering.sql.insert~1] + // [utest->dsn~values-as-insert-source~1] + @Test + void testInsertValuePlaceholder() { + assertThat(this.insert.valuePlaceholder(), rendersTo("INSERT INTO person VALUES ?")); + } + + // [utest->dsn~rendering.sql.insert~1] + // [utest->dsn~values-as-insert-source~1] + @Test + void testInsertValuePlaceholders() { + assertThat(this.insert.valuePlaceholders(3), rendersTo("INSERT INTO person VALUES ?, ?, ?")); + } + + // [utest->dsn~rendering.sql.insert~1] + // [utest->dsn~values-as-insert-source~1] + @Test + void testInsertMixedValuesAndPlaceholders() { + assertThat(this.insert.values(1).valuePlaceholders(3).values("b", 4), + rendersTo("INSERT INTO person VALUES 1, ?, ?, ?, 'b', 4")); + } } \ No newline at end of file diff --git a/src/test/java/com/exasol/sql/dql/TestSelect.java b/src/test/java/com/exasol/sql/dql/TestSelect.java index 3ec9176d..c1917502 100644 --- a/src/test/java/com/exasol/sql/dql/TestSelect.java +++ b/src/test/java/com/exasol/sql/dql/TestSelect.java @@ -6,7 +6,6 @@ import org.junit.jupiter.api.Test; import com.exasol.sql.StatementFactory; -import com.exasol.sql.dql.Select; class TestSelect { private Select select; diff --git a/src/test/java/com/exasol/sql/dql/rendering/TestSelectRendering.java b/src/test/java/com/exasol/sql/dql/rendering/TestSelectRendering.java index e640566e..f9b6420b 100644 --- a/src/test/java/com/exasol/sql/dql/rendering/TestSelectRendering.java +++ b/src/test/java/com/exasol/sql/dql/rendering/TestSelectRendering.java @@ -1,6 +1,7 @@ package com.exasol.sql.dql.rendering; import static com.exasol.hamcrest.SqlFragmentRenderResultMatcher.rendersTo; +import static com.exasol.hamcrest.SqlFragmentRenderResultMatcher.rendersWithConfigTo; import static org.hamcrest.MatcherAssert.assertThat; import org.junit.jupiter.api.BeforeEach; @@ -8,6 +9,8 @@ import com.exasol.sql.StatementFactory; import com.exasol.sql.dql.Select; +import com.exasol.sql.expression.BooleanTerm; +import com.exasol.sql.rendering.StringRendererConfig; class TestSelectRendering { private Select select; @@ -17,46 +20,60 @@ void beforeEach() { this.select = StatementFactory.getInstance().select(); } - // [dsn~rendering.sql.select~1] + // [utest->dsn~rendering.sql.select~1] @Test void testSelectAll() { assertThat(this.select.all(), rendersTo("SELECT *")); } - // [dsn~rendering.sql.select~1] + // [utest->dsn~rendering.sql.configurable-case~1] + @Test + void testSelectAllLowerCase() { + assertThat(this.select.all(), + rendersWithConfigTo(new StringRendererConfig.Builder().lowerCase(true).build(), "select *")); + } + + // [utest->dsn~rendering.sql.select~1] @Test void testSelectFieldNames() { assertThat(this.select.field("a", "b"), rendersTo("SELECT a, b")); } - // [dsn~rendering.sql.select~1] + // [utest->dsn~rendering.sql.select~1] @Test void testSelectChainOfFieldNames() { assertThat(this.select.field("a", "b").field("c"), rendersTo("SELECT a, b, c")); } - // [dsn~rendering.sql.select~1] + // [utest->dsn~rendering.sql.select~1] @Test void testSelectFromTable() { assertThat(this.select.all().from().table("persons"), rendersTo("SELECT * FROM persons")); } - // [dsn~rendering.sql.select~1] + // [utest->dsn~rendering.sql.select~1] @Test void testSelectFromMultipleTable() { assertThat(this.select.all().from().table("table1").table("table2"), rendersTo("SELECT * FROM table1, table2")); } - // [dsn~rendering.sql.select~1] + // [utest->dsn~rendering.sql.select~1] @Test void testSelectFromTableAs() { assertThat(this.select.all().from().tableAs("table", "t"), rendersTo("SELECT * FROM table AS t")); } - // [dsn~rendering.sql.select~1] + // [utest->dsn~rendering.sql.select~1] @Test void testSelectFromMultipleTableAs() { assertThat(this.select.all().from().tableAs("table1", "t1").tableAs("table2", "t2"), rendersTo("SELECT * FROM table1 AS t1, table2 AS t2")); } + + // [utest->dsn~select-statement.out-of-order-clauses~1] + @Test + void testAddClausesInRandomOrder() { + assertThat(this.select.limit(1).all().where(BooleanTerm.not("foo")).from().join("A", "A.aa = B.bb").table("B"), + rendersTo("SELECT * FROM B JOIN A ON A.aa = B.bb WHERE NOT(foo) LIMIT 1")); + } } \ No newline at end of file diff --git a/src/test/java/com/exasol/sql/expression/TestBooleanTerm.java b/src/test/java/com/exasol/sql/expression/TestBooleanTerm.java index 9c7e77d3..2813da2f 100644 --- a/src/test/java/com/exasol/sql/expression/TestBooleanTerm.java +++ b/src/test/java/com/exasol/sql/expression/TestBooleanTerm.java @@ -76,13 +76,13 @@ void testOperationFromNullOperatorThrowsException() { assertThrows(NullPointerException.class, () -> BooleanTerm.operation(null, not("a"), not("b"))); } - // [impl->dsn~boolean-operation.comparison.constructing-from-strings~1] + // [utest->dsn~boolean-operation.comparison.constructing-from-strings~1] @Test void testOperationFromComparisonOperatorString() { assertThat(BooleanTerm.compare("a", "<>", "b"), instanceOf(Comparison.class)); } - // [impl->dsn~boolean-operation.comparison.constructing-from-strings~1] + // [utest->dsn~boolean-operation.comparison.constructing-from-enum~1] @Test void testOperationFromComparisonOperatorEnum() { assertThat(BooleanTerm.compare("a", ComparisonOperator.NOT_EQUAL, "b"), instanceOf(Comparison.class)); diff --git a/src/test/java/com/exasol/sql/expression/TestComparisonOperator.java b/src/test/java/com/exasol/sql/expression/TestComparisonOperator.java index 819b28d3..e4c501af 100644 --- a/src/test/java/com/exasol/sql/expression/TestComparisonOperator.java +++ b/src/test/java/com/exasol/sql/expression/TestComparisonOperator.java @@ -12,11 +12,13 @@ void testToString() { assertThat(ComparisonOperator.EQUAL.toString(), equalTo("=")); } + // [utest->dsn~boolean-operation.comparison.constructing-from-strings~1] @Test void testOfSymbol() { assertThat(ComparisonOperator.ofSymbol("<>"), equalTo(ComparisonOperator.NOT_EQUAL)); } + // [utest->dsn~boolean-operation.comparison.constructing-from-strings~1] @Test void testOfUnknownSymbolThrowsException() { assertThrows(IllegalArgumentException.class, () -> ComparisonOperator.ofSymbol("ยง")); From 86b618f078977d29cc534b96cf7fb51af73dd00c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20B=C3=A4r?= Date: Wed, 10 Oct 2018 10:41:31 +0200 Subject: [PATCH 4/6] PMI-95: Added auto-quoting for identifiers. --- doc/design.md | 19 ++++++ doc/system_requirements.md | 19 ++++++ .../sql/dml/rendering/InsertRenderer.java | 4 +- .../sql/dql/rendering/SelectRenderer.java | 6 +- .../rendering/AbstractExpressionRenderer.java | 2 +- .../rendering/BooleanExpressionRenderer.java | 2 +- .../rendering/AbstractFragmentRenderer.java | 36 ++++++++++- .../sql/rendering/StringRendererConfig.java | 59 +++++++++++++++---- .../sql/dml/rendering/TestInsertRenderer.java | 2 +- .../dml/rendering/TestInsertRendering.java | 2 +- .../sql/dql/rendering/TestSelectRenderer.java | 2 +- .../dql/rendering/TestSelectRendering.java | 10 +++- .../TestBooleanExpressionRenderer.java | 2 +- 13 files changed, 140 insertions(+), 25 deletions(-) diff --git a/doc/design.md b/doc/design.md index 4068a95b..03f001aa 100644 --- a/doc/design.md +++ b/doc/design.md @@ -94,3 +94,22 @@ Needs: impl, utest * `dsn --> impl, utest: req~rendering.sql.configurable-case~1` * `dsn --> impl, utest: req~rendering.sql.select~1` * `dsn --> impl, utest: req~rendering.sql.insert~1` + +#### Renderer add Double Quotes for Schema, Table and Column Identifiers +`dsn~rendering.add-double-quotes-for-schema-table-and-column-identifiers~1` + +The renderer adds double quotes the following identifiers if configured: + +* Schema identifiers +* Table identifiers +* Column identifiers (except the asterisks) + +Comment: + +Examples are `"my_schema"."my_table"."my_field"`, `"MY_TABLE"."MyField"` and `"MyTable".*` + +Covers: + +* `req~rendering.sql.confiugrable-identifier-quoting~1` + +Needs: impl, utest \ No newline at end of file diff --git a/doc/system_requirements.md b/doc/system_requirements.md index b8d9ef20..109c6efd 100644 --- a/doc/system_requirements.md +++ b/doc/system_requirements.md @@ -202,6 +202,25 @@ Covers: Needs: dsn +###### Configurable Identifier Quoting +`req~rendering.sql.confiugrable-identifier-quoting~1` + +ESB allows users to choose whether the following identifiers should be quoted in the rendered query: + +* Schema identifiers +* Table identifiers +* Column identifiers + +Rationale: + +The Exasol database for example requires identifiers to be enclosed in double quotes in order to enable case sensitivity. + +Covers: + +* [feat~sql-string-rendering~1](#sql-string-rendering) + +Needs: dsn + #### TODO * One line / pretty diff --git a/src/main/java/com/exasol/sql/dml/rendering/InsertRenderer.java b/src/main/java/com/exasol/sql/dml/rendering/InsertRenderer.java index 1d44aacb..f3e89425 100644 --- a/src/main/java/com/exasol/sql/dml/rendering/InsertRenderer.java +++ b/src/main/java/com/exasol/sql/dml/rendering/InsertRenderer.java @@ -29,14 +29,14 @@ public void visit(final Insert insert) { @Override public void visit(final Table table) { - append(table.getName()); + appendAutoQuoted(table.getName()); setLastVisited(table); } @Override public void visit(final Field field) { appendCommaWhenNeeded(field); - append(field.getName()); + appendAutoQuoted(field.getName()); setLastVisited(field); } diff --git a/src/main/java/com/exasol/sql/dql/rendering/SelectRenderer.java b/src/main/java/com/exasol/sql/dql/rendering/SelectRenderer.java index 5073f7bb..93e2dead 100644 --- a/src/main/java/com/exasol/sql/dql/rendering/SelectRenderer.java +++ b/src/main/java/com/exasol/sql/dql/rendering/SelectRenderer.java @@ -31,7 +31,7 @@ public void visit(final Select select) { @Override public void visit(final Field field) { appendCommaWhenNeeded(field); - append(field.getName()); + appendAutoQuoted(field.getName()); setLastVisited(field); } @@ -44,7 +44,7 @@ public void visit(final FromClause fromClause) { @Override public void visit(final Table table) { appendCommaWhenNeeded(table); - append(table.getName()); + appendAutoQuoted(table.getName()); final Optional as = table.getAs(); if (as.isPresent()) { appendKeyWord(" AS "); @@ -61,7 +61,7 @@ public void visit(final Join join) { appendKeyWord(type.toString()); } appendKeyWord(" JOIN "); - append(join.getName()); + appendAutoQuoted(join.getName()); appendKeyWord(" ON "); append(join.getSpecification()); setLastVisited(join); diff --git a/src/main/java/com/exasol/sql/expression/rendering/AbstractExpressionRenderer.java b/src/main/java/com/exasol/sql/expression/rendering/AbstractExpressionRenderer.java index ab36958f..47af626e 100644 --- a/src/main/java/com/exasol/sql/expression/rendering/AbstractExpressionRenderer.java +++ b/src/main/java/com/exasol/sql/expression/rendering/AbstractExpressionRenderer.java @@ -18,7 +18,7 @@ public AbstractExpressionRenderer(final StringRendererConfig config) { } protected void appendKeyword(final String keyword) { - this.builder.append(this.config.produceLowerCase() ? keyword.toLowerCase() : keyword); + this.builder.append(this.config.useLowerCase() ? keyword.toLowerCase() : keyword); } protected void connect(final BooleanExpression expression) { diff --git a/src/main/java/com/exasol/sql/expression/rendering/BooleanExpressionRenderer.java b/src/main/java/com/exasol/sql/expression/rendering/BooleanExpressionRenderer.java index 1588a707..bb112496 100644 --- a/src/main/java/com/exasol/sql/expression/rendering/BooleanExpressionRenderer.java +++ b/src/main/java/com/exasol/sql/expression/rendering/BooleanExpressionRenderer.java @@ -9,7 +9,7 @@ public BooleanExpressionRenderer(final StringRendererConfig config) { } public BooleanExpressionRenderer() { - this(new StringRendererConfig.Builder().build()); + this(StringRendererConfig.builder().build()); } @Override diff --git a/src/main/java/com/exasol/sql/rendering/AbstractFragmentRenderer.java b/src/main/java/com/exasol/sql/rendering/AbstractFragmentRenderer.java index 08abe2a0..ef7c8f02 100644 --- a/src/main/java/com/exasol/sql/rendering/AbstractFragmentRenderer.java +++ b/src/main/java/com/exasol/sql/rendering/AbstractFragmentRenderer.java @@ -21,7 +21,7 @@ public AbstractFragmentRenderer(final StringRendererConfig config) { // [impl->dsn~rendering.sql.configurable-case~1] protected void appendKeyWord(final String keyword) { - append(this.config.produceLowerCase() ? keyword.toLowerCase() : keyword); + append(this.config.useLowerCase() ? keyword.toLowerCase() : keyword); } protected StringBuilder append(final String string) { @@ -58,6 +58,40 @@ protected void appendRenderedValueExpression(final ValueExpression expression) { append(renderer.render()); } + // [impl->dsn~rendering.add-double-quotes-for-schema-table-and-column-identifiers~1] + protected void appendAutoQuoted(final String identifier) { + if (this.config.useQuotes()) { + appendQuoted(identifier); + } else { + append(identifier); + } + } + + private void appendQuoted(final String identifier) { + boolean first = true; + for (final String part : identifier.split("\\.")) { + if (!first) { + append("."); + } + quoteIdentiferPart(part); + first = false; + } + } + + private void quoteIdentiferPart(final String part) { + if ("*".equals(part)) { + append("*"); + } else { + if (!part.startsWith("\"")) { + append("\""); + } + append(part); + if (!part.endsWith("\"")) { + append("\""); + } + } + } + @Override public String render() { return this.builder.toString(); diff --git a/src/main/java/com/exasol/sql/rendering/StringRendererConfig.java b/src/main/java/com/exasol/sql/rendering/StringRendererConfig.java index 07d99554..cbc79551 100644 --- a/src/main/java/com/exasol/sql/rendering/StringRendererConfig.java +++ b/src/main/java/com/exasol/sql/rendering/StringRendererConfig.java @@ -7,9 +7,11 @@ */ public class StringRendererConfig { private final boolean lowerCase; + private final boolean quote; - private StringRendererConfig(final boolean lowerCase) { - this.lowerCase = lowerCase; + private StringRendererConfig(final Builder builder) { + this.lowerCase = builder.lowerCase; + this.quote = builder.quote; } /** @@ -17,15 +19,46 @@ private StringRendererConfig(final boolean lowerCase) { * * @return true if statements are produced in lower case */ - public boolean produceLowerCase() { + public boolean useLowerCase() { return this.lowerCase; } + /** + * Get whether identifiers should be enclosed in double quotation marks. + * + * @return true if should be enclosed in quotes + */ + public boolean useQuotes() { + return this.quote; + } + + /** + * Create the default configuration. + * + * @return default configuration + */ + public static StringRendererConfig createDefault() { + return builder().build(); + } + + /** + * Get a builder for {@link StringRendererConfig} + * + * @return builder + */ + public static Builder builder() { + return new Builder(); + } + /** * Builder for {@link StringRendererConfig} */ public static class Builder { private boolean lowerCase = false; + private boolean quote = false; + + private Builder() { + } /** * Create a new instance of a {@link StringRendererConfig} @@ -33,7 +66,7 @@ public static class Builder { * @return new instance */ public StringRendererConfig build() { - return new StringRendererConfig(this.lowerCase); + return new StringRendererConfig(this); } /** @@ -46,14 +79,16 @@ public Builder lowerCase(final boolean lowerCase) { this.lowerCase = lowerCase; return this; } - } - /** - * Create the default configuration. - * - * @return default configuration - */ - public static StringRendererConfig createDefault() { - return new Builder().build(); + /** + * Define whether schema, table and field identifiers should be enclosed in double quotation marks. + * + * @param quote set to true if identifiers should be enclosed in quotes + * @return this instance for fluent programming + */ + public Builder quoteIdentifiers(final boolean quote) { + this.quote = quote; + return this; + } } } \ No newline at end of file diff --git a/src/test/java/com/exasol/sql/dml/rendering/TestInsertRenderer.java b/src/test/java/com/exasol/sql/dml/rendering/TestInsertRenderer.java index ec691730..0d2b462a 100644 --- a/src/test/java/com/exasol/sql/dml/rendering/TestInsertRenderer.java +++ b/src/test/java/com/exasol/sql/dml/rendering/TestInsertRenderer.java @@ -18,7 +18,7 @@ void testCreateWithDefaultConfig() { @Test void testCreateWithConfig() { - final StringRendererConfig config = new StringRendererConfig.Builder().lowerCase(true).build(); + final StringRendererConfig config = StringRendererConfig.builder().lowerCase(true).build(); final InsertRenderer renderer = InsertRenderer.create(config); final Insert insert = StatementFactory.getInstance().insertInto("city"); insert.accept(renderer); diff --git a/src/test/java/com/exasol/sql/dml/rendering/TestInsertRendering.java b/src/test/java/com/exasol/sql/dml/rendering/TestInsertRendering.java index 40e96e6e..16a07966 100644 --- a/src/test/java/com/exasol/sql/dml/rendering/TestInsertRendering.java +++ b/src/test/java/com/exasol/sql/dml/rendering/TestInsertRendering.java @@ -30,7 +30,7 @@ void testInsert() { @Test void testInsertRendersToWithConfig() { assertThat(this.insert, - rendersWithConfigTo(new StringRendererConfig.Builder().lowerCase(true).build(), "insert into person")); + rendersWithConfigTo(StringRendererConfig.builder().lowerCase(true).build(), "insert into person")); } // [utest->dsn~rendering.sql.insert~1] diff --git a/src/test/java/com/exasol/sql/dql/rendering/TestSelectRenderer.java b/src/test/java/com/exasol/sql/dql/rendering/TestSelectRenderer.java index 5954bac6..f71fbbd9 100644 --- a/src/test/java/com/exasol/sql/dql/rendering/TestSelectRenderer.java +++ b/src/test/java/com/exasol/sql/dql/rendering/TestSelectRenderer.java @@ -18,7 +18,7 @@ void testCreateWithDefaultConfig() { @Test void testCreateWithConfig() { - final StringRendererConfig config = new StringRendererConfig.Builder().lowerCase(true).build(); + final StringRendererConfig config = StringRendererConfig.builder().lowerCase(true).build(); final SelectRenderer renderer = SelectRenderer.create(config); final Select select = StatementFactory.getInstance().select(); select.accept(renderer); diff --git a/src/test/java/com/exasol/sql/dql/rendering/TestSelectRendering.java b/src/test/java/com/exasol/sql/dql/rendering/TestSelectRendering.java index f9b6420b..5d7c1d07 100644 --- a/src/test/java/com/exasol/sql/dql/rendering/TestSelectRendering.java +++ b/src/test/java/com/exasol/sql/dql/rendering/TestSelectRendering.java @@ -30,7 +30,7 @@ void testSelectAll() { @Test void testSelectAllLowerCase() { assertThat(this.select.all(), - rendersWithConfigTo(new StringRendererConfig.Builder().lowerCase(true).build(), "select *")); + rendersWithConfigTo(StringRendererConfig.builder().lowerCase(true).build(), "select *")); } // [utest->dsn~rendering.sql.select~1] @@ -76,4 +76,12 @@ void testAddClausesInRandomOrder() { assertThat(this.select.limit(1).all().where(BooleanTerm.not("foo")).from().join("A", "A.aa = B.bb").table("B"), rendersTo("SELECT * FROM B JOIN A ON A.aa = B.bb WHERE NOT(foo) LIMIT 1")); } + + // [utest->dsn~rendering.add-double-quotes-for-schema-table-and-column-identifiers~1] + @Test + void testSelectWithQuotedIdentifiers() { + final StringRendererConfig config = StringRendererConfig.builder().quoteIdentifiers(true).build(); + assertThat(this.select.field("fieldA", "tableA.fieldB").from().table("schemaA.tableA"), + rendersWithConfigTo(config, "SELECT \"fieldA\", \"tableA\".\"fieldB\" FROM \"schemaA\".\"tableA\"")); + } } \ No newline at end of file diff --git a/src/test/java/com/exasol/sql/expression/rendering/TestBooleanExpressionRenderer.java b/src/test/java/com/exasol/sql/expression/rendering/TestBooleanExpressionRenderer.java index 5e8129a2..6faaa2b1 100644 --- a/src/test/java/com/exasol/sql/expression/rendering/TestBooleanExpressionRenderer.java +++ b/src/test/java/com/exasol/sql/expression/rendering/TestBooleanExpressionRenderer.java @@ -95,7 +95,7 @@ void testAndWhitNestedOr() { @Test void testAndWhitNestedOrInLowercase() { final BooleanExpression expression = and(or(not("a"), "b"), or("c", "d")); - final StringRendererConfig config = new StringRendererConfig.Builder().lowerCase(true).build(); + final StringRendererConfig config = StringRendererConfig.builder().lowerCase(true).build(); assertThat(expression, rendersWithConfigTo(config, "(not(a) or b) and (c or d)")); } From d8b2d74e8a73b02c76995e613b5ce6f9cd91965f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20B=C3=A4r?= Date: Wed, 10 Oct 2018 14:35:37 +0200 Subject: [PATCH 5/6] PMI-95: Fixed insert value list rendering. Added missing unit tests. --- .../com/exasol/sql/dml/rendering/InsertRenderer.java | 4 ++-- .../sql/dml/rendering/TestInsertRendering.java | 8 ++++---- .../sql/dql/rendering/TestSelectRendering.java | 12 ++++++++++-- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/exasol/sql/dml/rendering/InsertRenderer.java b/src/main/java/com/exasol/sql/dml/rendering/InsertRenderer.java index f3e89425..97c2905d 100644 --- a/src/main/java/com/exasol/sql/dml/rendering/InsertRenderer.java +++ b/src/main/java/com/exasol/sql/dml/rendering/InsertRenderer.java @@ -53,7 +53,7 @@ public void leave(final InsertFields insertFields) { @Override public void visit(final InsertValues insertValues) { - appendKeyWord(" VALUES "); + appendKeyWord(" VALUES ("); for (final ValueExpression expression : insertValues.getValues()) { appendCommaWhenNeeded(insertValues); appendRenderedValueExpression(expression); @@ -63,7 +63,7 @@ public void visit(final InsertValues insertValues) { @Override public void leave(final InsertValues insertValues) { - // intentionally empty + append(")"); } /** diff --git a/src/test/java/com/exasol/sql/dml/rendering/TestInsertRendering.java b/src/test/java/com/exasol/sql/dml/rendering/TestInsertRendering.java index 16a07966..387da8f2 100644 --- a/src/test/java/com/exasol/sql/dml/rendering/TestInsertRendering.java +++ b/src/test/java/com/exasol/sql/dml/rendering/TestInsertRendering.java @@ -43,21 +43,21 @@ void testInsertFields() { // [utest->dsn~values-as-insert-source~1] @Test void testInsertValues() { - assertThat(this.insert.values(1, "a"), rendersTo("INSERT INTO person VALUES 1, 'a'")); + assertThat(this.insert.values(1, "a"), rendersTo("INSERT INTO person VALUES (1, 'a')")); } // [utest->dsn~rendering.sql.insert~1] // [utest->dsn~values-as-insert-source~1] @Test void testInsertValuePlaceholder() { - assertThat(this.insert.valuePlaceholder(), rendersTo("INSERT INTO person VALUES ?")); + assertThat(this.insert.valuePlaceholder(), rendersTo("INSERT INTO person VALUES (?)")); } // [utest->dsn~rendering.sql.insert~1] // [utest->dsn~values-as-insert-source~1] @Test void testInsertValuePlaceholders() { - assertThat(this.insert.valuePlaceholders(3), rendersTo("INSERT INTO person VALUES ?, ?, ?")); + assertThat(this.insert.valuePlaceholders(3), rendersTo("INSERT INTO person VALUES (?, ?, ?)")); } // [utest->dsn~rendering.sql.insert~1] @@ -65,6 +65,6 @@ void testInsertValuePlaceholders() { @Test void testInsertMixedValuesAndPlaceholders() { assertThat(this.insert.values(1).valuePlaceholders(3).values("b", 4), - rendersTo("INSERT INTO person VALUES 1, ?, ?, ?, 'b', 4")); + rendersTo("INSERT INTO person VALUES (1, ?, ?, ?, 'b', 4)")); } } \ No newline at end of file diff --git a/src/test/java/com/exasol/sql/dql/rendering/TestSelectRendering.java b/src/test/java/com/exasol/sql/dql/rendering/TestSelectRendering.java index 5d7c1d07..e19c76df 100644 --- a/src/test/java/com/exasol/sql/dql/rendering/TestSelectRendering.java +++ b/src/test/java/com/exasol/sql/dql/rendering/TestSelectRendering.java @@ -81,7 +81,15 @@ void testAddClausesInRandomOrder() { @Test void testSelectWithQuotedIdentifiers() { final StringRendererConfig config = StringRendererConfig.builder().quoteIdentifiers(true).build(); - assertThat(this.select.field("fieldA", "tableA.fieldB").from().table("schemaA.tableA"), - rendersWithConfigTo(config, "SELECT \"fieldA\", \"tableA\".\"fieldB\" FROM \"schemaA\".\"tableA\"")); + assertThat(this.select.field("fieldA", "tableA.fieldB", "tableB.*").from().table("schemaA.tableA"), + rendersWithConfigTo(config, + "SELECT \"fieldA\", \"tableA\".\"fieldB\", \"tableB\".* FROM \"schemaA\".\"tableA\"")); + } + + @Test + void testSelectWithQuotedIdentifiersDoesNotAddExtraQuotes() { + final StringRendererConfig config = StringRendererConfig.builder().quoteIdentifiers(true).build(); + assertThat(this.select.field("\"fieldA\"", "\"tableA\".fieldB"), + rendersWithConfigTo(config, "SELECT \"fieldA\", \"tableA\".\"fieldB\"")); } } \ No newline at end of file From 3b0291947c74904a4fc8ad1f46f4d07d7a31e880 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20B=C3=A4r?= Date: Mon, 15 Oct 2018 10:59:34 +0200 Subject: [PATCH 6/6] PMI-95: Fixed findings from @bobkodex. --- doc/design.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/design.md b/doc/design.md index 03f001aa..aa444da3 100644 --- a/doc/design.md +++ b/doc/design.md @@ -98,7 +98,7 @@ Needs: impl, utest #### Renderer add Double Quotes for Schema, Table and Column Identifiers `dsn~rendering.add-double-quotes-for-schema-table-and-column-identifiers~1` -The renderer adds double quotes the following identifiers if configured: +The renderer sets the following identifiers in double quotes if configured: * Schema identifiers * Table identifiers