github/jooq
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[jOOQ/jOOQ#9764] Rework formatting of SQL

Browse Source 
One big formatting problem is in QueryPartList. 1) it adds unnecessary
whitespace at the end of lines, because it hardcodes a ", " separator.
That is easy to fix. Also, it needs to decide whether at the beginning
of a list, an extra separator is required. That's something that a
container QueryPart needs to decide for the QueryPartList, whereas the
QueryPartList gets to decide if that separator is a newline or
whitespace.

A few other formatting issues are fixed with this one, including:

- [jOOQ/jOOQ#1962] Formatting of SQL rendered by window functions
- [jOOQ/jOOQ#3479] Awkward formatting of "short" functions
- [jOOQ/jOOQ#5488] Excess newline in formatted SELECT w/out FROM clause
- [jOOQ/jOOQ#9764] This issue
This commit is contained in:
Lukas Eder 2020-03-24 11:00:38 +01:00
parent 787878ed80
commit e8ee1bf94d
9 changed files with 183 additions and 107 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
jOOQ/src/main/java/org/jooq/Context.java
View File

@ -334,6 +334,20 @@ public interface Context<C extends Context<C>> extends Scope {
*/
C formatSeparator();
/**
* Specify that a separator will be required before the next
* {@link #visit(QueryPart)} call, but leave the decision whether to
* generate a {@link #formatSeparator()} or just a whitespace to that next
* {@link QueryPart}.
*/
C separatorRequired(boolean separatorRequired);
/**
* Whether some sort of separator is required before rendering the next
* {@link QueryPart}.
*/
boolean separatorRequired();
/**
* Start indenting subsequent SQL by one level (two characters), if
* {@link Settings#isRenderFormatted()} is set to <code>true</code>.

10
jOOQ/src/main/java/org/jooq/impl/AbstractBindContext.java
View File

@ -217,6 +217,16 @@ abstract class AbstractBindContext extends AbstractContext<BindContext> implemen
return this;
}
@Override
public final BindContext separatorRequired(boolean separatorRequired) {
return this;
}
@Override
public final boolean separatorRequired() {
return false;
}
@Override
public final BindContext formatIndentStart() {
return this;

63
jOOQ/src/main/java/org/jooq/impl/DefaultRenderContext.java
View File

@ -102,6 +102,7 @@ class DefaultRenderContext extends AbstractContext<RenderContext> implements Ren
private int alias;
private int indent;
private Deque<Integer> indentLock;
private boolean separatorRequired;
private boolean separator;
private boolean newline;
private int skipUpdateCounts;
@ -355,69 +356,77 @@ class DefaultRenderContext extends AbstractContext<RenderContext> implements Ren
if (stringLiteral())
s = StringUtils.replace(s, "'", stringLiteralEscapedApos);
applyNewLine();
sql.append(s);
separator = false;
newline = false;
resetSeparatorFlags();
return this;
}
@Override
public final RenderContext sql(char c) {
applyNewLine();
sql.append(c);
if (c == '\'' && stringLiteral())
sql.append(c);
separator = false;
newline = false;
resetSeparatorFlags();
return this;
}
@Override
public final RenderContext sql(int i) {
applyNewLine();
sql.append(i);
separator = false;
newline = false;
resetSeparatorFlags();
return this;
}
@Override
public final RenderContext sql(long l) {
applyNewLine();
sql.append(l);
separator = false;
newline = false;
resetSeparatorFlags();
return this;
}
@Override
public final RenderContext sql(float f) {
applyNewLine();
sql.append(f);
separator = false;
newline = false;
resetSeparatorFlags();
return this;
}
@Override
public final RenderContext sql(double d) {
applyNewLine();
sql.append(d);
resetSeparatorFlags();
return this;
}
private final void resetSeparatorFlags() {
separatorRequired = false;
separator = false;
newline = false;
return this;
}
@Override
public final RenderContext formatNewLine() {
if (cachedRenderFormatted) {
sql(cachedNewline, true);
sql(indentation(), true);
if (cachedRenderFormatted)
newline = true;
}
return this;
}
private final void applyNewLine() {
if (newline) {
sql.append(cachedNewline);
sql.append(indentation());
}
}
@Override
public final RenderContext formatNewLineAfterPrintMargin() {
if (cachedRenderFormatted && cachedPrintMargin > 0)
@ -456,6 +465,17 @@ class DefaultRenderContext extends AbstractContext<RenderContext> implements Ren
return this;
}
@Override
public final RenderContext separatorRequired(boolean separatorRequired) {
this.separatorRequired = separatorRequired;
return this;
}
@Override
public final boolean separatorRequired() {
return separatorRequired;
}
@Override
public final RenderContext formatIndentStart() {
return formatIndentStart(cachedIndentWidth);
@ -468,13 +488,12 @@ class DefaultRenderContext extends AbstractContext<RenderContext> implements Ren
@Override
public final RenderContext formatIndentStart(int i) {
if (cachedRenderFormatted) {
if (cachedRenderFormatted)
indent += i;
// [#9193] If we've already generated the separator (and indentation)
if (newline)
sql.append(cachedIndentation);
}
// // [#9193] If we've already generated the separator (and indentation)
// if (newline)
// sql.append(cachedIndentation);
return this;
}

3
jOOQ/src/main/java/org/jooq/impl/KeywordImpl.java
View File

@ -66,6 +66,9 @@ final class KeywordImpl extends AbstractQueryPart implements Keyword {
@Override
public final void accept(Context<?> ctx) {
if (ctx.separatorRequired())
ctx.sql(' ');
ctx.sql(render(ctx), true);
}

31
jOOQ/src/main/java/org/jooq/impl/QueryPartList.java
View File

@ -80,31 +80,40 @@ class QueryPartList<T extends QueryPart> extends AbstractQueryPart implements Li
@Override
public /* non-final */ void accept(Context<?> ctx) {
int size = size();
if (ctx.separatorRequired())
if (size > 1)
ctx.formatSeparator();
else
ctx.sql(' ');
// Some lists render different SQL when empty
if (isEmpty()) {
if (size == 0) {
toSQLEmptyList(ctx);
}
else {
String separator = "";
boolean indent = (size() > 1) && !TRUE.equals(ctx.data(DATA_LIST_ALREADY_INDENTED));
boolean indent = (size > 1) && !TRUE.equals(ctx.data(DATA_LIST_ALREADY_INDENTED));
if (indent)
ctx.formatIndentStart();
for (int i = 0; i < size(); i++) {
ctx.sql(separator);
for (int i = 0; i < size; i++) {
T part = get(i);
if (i > 0 || indent)
if (i > 0) {
// [#3607] Procedures and functions are not separated by comma
if (!(part instanceof Statement))
ctx.sql(',');
ctx.formatSeparator();
}
else if (indent)
ctx.formatNewLine();
T part = get(i);
ctx.visit(part);
// [#3607] Procedures and functions are not separated by comma
if (!(part instanceof Statement))
separator = ", ";
}
if (indent)

37
jOOQ/src/main/java/org/jooq/impl/SelectQueryImpl.java
View File

@ -1053,7 +1053,7 @@ final class SelectQueryImpl<R extends Record> extends AbstractResultQuery<R> imp
// v1 as ID, v2 as ID, v3 as TITLE
final Field<?>[] unaliasedFields = Tools.aliasedFields(Tools.fields(originalFields.length), originalNames);
ctx.visit(K_SELECT).sql(' ')
ctx.visit(K_SELECT).separatorRequired(true)
.declareFields(true)
.visit(new SelectFieldList<>(unaliasedFields))
.declareFields(false)
@ -1302,12 +1302,11 @@ final class SelectQueryImpl<R extends Record> extends AbstractResultQuery<R> imp
// SELECT clause
// -------------
context.start(SELECT_SELECT)
.visit(K_SELECT)
.sql(' ');
.visit(K_SELECT).separatorRequired(true);
// [#1493] Oracle hints come directly after the SELECT keyword
if (!StringUtils.isBlank(hint))
context.sql(hint).sql(' ');
context.sql(' ').sql(hint).separatorRequired(true);
@ -1316,9 +1315,9 @@ final class SelectQueryImpl<R extends Record> extends AbstractResultQuery<R> imp
if (Tools.isNotEmpty(distinctOn))
context.visit(K_DISTINCT_ON).sql(" (").visit(distinctOn).sql(") ");
context.visit(K_DISTINCT_ON).sql(" (").visit(distinctOn).sql(')').separatorRequired(true);
else if (distinct)
context.visit(K_DISTINCT).sql(' ');
context.visit(K_DISTINCT).separatorRequired(true);
@ -1413,7 +1412,7 @@ final class SelectQueryImpl<R extends Record> extends AbstractResultQuery<R> imp
context.formatSeparator()
.visit(K_FROM)
.sql(' ')
.separatorRequired(true)
.visit(tablelist);
@ -1499,10 +1498,11 @@ final class SelectQueryImpl<R extends Record> extends AbstractResultQuery<R> imp
if (grouping) {
context.formatSeparator()
.visit(K_GROUP_BY)
.sql(' ');
.separatorRequired(true);
// [#1665] Empty GROUP BY () clauses need parentheses
if (Tools.isEmpty(groupBy))
if (Tools.isEmpty(groupBy)) {
context.sql(' ');
// [#4292] Some dialects accept constant expressions in GROUP BY
// Note that dialects may consider constants as indexed field
@ -1527,6 +1527,7 @@ final class SelectQueryImpl<R extends Record> extends AbstractResultQuery<R> imp
// Few dialects support the SQL standard "grand total" (i.e. empty grouping set)
else
context.sql("()");
}
else
context.visit(groupBy);
}
@ -1558,14 +1559,13 @@ final class SelectQueryImpl<R extends Record> extends AbstractResultQuery<R> imp
// -------------
context.start(SELECT_WINDOW);
if (Tools.isNotEmpty(window) && SUPPORT_WINDOW_CLAUSE.contains(context.dialect())) {
if (Tools.isNotEmpty(window) && SUPPORT_WINDOW_CLAUSE.contains(context.dialect()))
context.formatSeparator()
.visit(K_WINDOW)
.sql(' ')
.separatorRequired(true)
.declareWindows(true)
.visit(window)
.declareWindows(false);
}
context.end(SELECT_WINDOW);
@ -1588,12 +1588,14 @@ final class SelectQueryImpl<R extends Record> extends AbstractResultQuery<R> imp
for (Select<?> other : union.get(i)) {
context.formatSeparator()
.visit(op.toKeyword(family))
.sql(' ');
.visit(op.toKeyword(family));
if (!unionParenthesis(context, '(', other.getSelect().toArray(EMPTY_FIELD), unionParensRequired) && !unionParensRequired)
context.formatNewLine();
if (unionParensRequired)
context.sql(' ');
else
context.formatSeparator();
unionParenthesis(context, '(', other.getSelect().toArray(EMPTY_FIELD), unionParensRequired);
context.visit(other);
unionParenthesis(context, ')', null, unionParensRequired);
}
@ -1768,8 +1770,7 @@ final class SelectQueryImpl<R extends Record> extends AbstractResultQuery<R> imp
if (orderBySiblings)
ctx.sql(' ').visit(K_SIBLINGS);
ctx.sql(' ').visit(K_BY)
.sql(' ');
ctx.sql(' ').visit(K_BY).separatorRequired(true);

126
jOOQ/src/main/java/org/jooq/impl/WindowSpecificationImpl.java
View File

@ -146,43 +146,13 @@ final class WindowSpecificationImpl extends AbstractQueryPart implements
@Override
public final void accept(Context<?> ctx) {
String glue = "";
SortFieldList o = orderBy;
if (windowDefinition != null) {
boolean declareWindows = ctx.declareWindows();
ctx.sql(glue)
.declareWindows(false)
.visit(windowDefinition)
.declareWindows(declareWindows);
glue = " ";
}
if (!partitionBy.isEmpty()) {
// Ignore PARTITION BY 1 clause. These databases erroneously map the
// 1 literal onto the column index (CUBRID, Sybase), or do not support
// constant expressions in the PARTITION BY clause (HANA)
if (partitionByOne && OMIT_PARTITION_BY_ONE.contains(ctx.family())) {
}
else {
ctx.sql(glue)
.visit(K_PARTITION_BY).sql(' ')
.visit(partitionBy);
glue = " ";
}
}
if (!orderBy.isEmpty()) {
ctx.sql(glue)
.visit(K_ORDER_BY).sql(' ')
.visit(orderBy);
glue = " ";
}
// [#8414] [#8593] Some RDBMS require ORDER BY in some window functions
if (o.isEmpty()) {
boolean requiresOrderBy =
TRUE.equals(ctx.data(BooleanDataKey.DATA_ORDERED_WINDOW_FUNCTION))
&& REQUIRES_ORDER_BY_IN_RANKING.contains(ctx.family());
@ -190,12 +160,8 @@ final class WindowSpecificationImpl extends AbstractQueryPart implements
else if (TRUE.equals(ctx.data(BooleanDataKey.DATA_ORDERED_WINDOW_FUNCTION)) && REQUIRES_ORDER_BY_IN_RANKING.contains(ctx.family())) {
Field<Integer> constant;
if (requiresOrderBy) {
Field<Integer> constant;
@ -204,15 +170,69 @@ final class WindowSpecificationImpl extends AbstractQueryPart implements
constant = field(select(one()));
ctx.sql(glue)
.visit(K_ORDER_BY).sql(' ')
.visit(constant);
glue = " ";
o = new SortFieldList();
o.add(constant.sortDefault());
}
}
if (frameStart != null) {
ctx.sql(glue);
boolean hasWindowDefinitions = windowDefinition != null;
boolean hasPartitionBy = !partitionBy.isEmpty();
boolean hasOrderBy = !o.isEmpty();
boolean hasFrame = frameStart != null;
int clauses = 0;
if (hasWindowDefinitions)
clauses++;
if (hasPartitionBy)
clauses++;
if (hasOrderBy)
clauses++;
if (hasFrame)
clauses++;
boolean indent = clauses > 1;
if (indent)
ctx.formatIndentStart()
.formatNewLine();
if (windowDefinition != null) {
boolean declareWindows = ctx.declareWindows();
ctx.declareWindows(false)
.visit(windowDefinition)
.declareWindows(declareWindows);
}
if (hasPartitionBy) {
// Ignore PARTITION BY 1 clause. These databases erroneously map the
// 1 literal onto the column index (CUBRID, Sybase), or do not support
// constant expressions in the PARTITION BY clause (HANA)
if (partitionByOne && OMIT_PARTITION_BY_ONE.contains(ctx.family())) {
}
else {
if (hasWindowDefinitions)
ctx.formatSeparator();
ctx.visit(K_PARTITION_BY).separatorRequired(true)
.visit(partitionBy);
}
}
if (hasOrderBy) {
if (hasWindowDefinitions || hasPartitionBy)
ctx.formatSeparator();
ctx.visit(K_ORDER_BY).separatorRequired(true)
.visit(o);
}
if (hasFrame) {
if (hasWindowDefinitions || hasPartitionBy || hasOrderBy)
ctx.formatSeparator();
ctx.visit(frameUnits.keyword).sql(' ');
if (frameEnd != null) {
@ -226,11 +246,13 @@ final class WindowSpecificationImpl extends AbstractQueryPart implements
toSQLRows(ctx, frameStart);
}
glue = " ";
if (exclude != null)
ctx.sql(glue).visit(K_EXCLUDE).sql(' ').visit(exclude.keyword);
ctx.sql(' ').visit(K_EXCLUDE).sql(' ').visit(exclude.keyword);
}
if (indent)
ctx.formatIndentEnd()
.formatNewLine();
}
private final void toSQLRows(Context<?> ctx, Integer rows) {

4
jOOQ/src/main/java/org/jooq/impl/XMLElement.java
View File

@ -75,9 +75,7 @@ final class XMLElement extends AbstractField<XML> {
public final void accept(Context<?> ctx) {
boolean hasAttributes = attributes != null && !((XMLAttributesImpl) attributes).attributes.isEmpty();
boolean hasContent = !content.isEmpty();
boolean format =
hasAttributes && ((XMLAttributesImpl) attributes).attributes.size() > 1
|| hasContent && content.size() > 1;
boolean format = hasAttributes || hasContent;
Object previous = ctx.data(DATA_LIST_ALREADY_INDENTED);
ctx.visit(N_XMLELEMENT).sql('(');

2
jOOQ/src/main/java/org/jooq/impl/XMLTable.java
View File

@ -226,7 +226,7 @@ implements
acceptPassing(ctx, passing, passingMechanism);
ctx.formatSeparator()
.visit(K_COLUMNS).sql(' ').visit(columns);
.visit(K_COLUMNS).separatorRequired(true).visit(columns);
ctx.formatIndentEnd()
.formatNewLine()