diff --git a/jOOQ/src/main/java/org/jooq/Context.java b/jOOQ/src/main/java/org/jooq/Context.java index 0efcfabea4..d76f08630b 100644 --- a/jOOQ/src/main/java/org/jooq/Context.java +++ b/jOOQ/src/main/java/org/jooq/Context.java @@ -334,6 +334,20 @@ public interface Context> 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 true. diff --git a/jOOQ/src/main/java/org/jooq/impl/AbstractBindContext.java b/jOOQ/src/main/java/org/jooq/impl/AbstractBindContext.java index 284c6b8d22..1d3f57a90a 100644 --- a/jOOQ/src/main/java/org/jooq/impl/AbstractBindContext.java +++ b/jOOQ/src/main/java/org/jooq/impl/AbstractBindContext.java @@ -217,6 +217,16 @@ abstract class AbstractBindContext extends AbstractContext 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; diff --git a/jOOQ/src/main/java/org/jooq/impl/DefaultRenderContext.java b/jOOQ/src/main/java/org/jooq/impl/DefaultRenderContext.java index 4270a3c7f8..1b6e2b8645 100644 --- a/jOOQ/src/main/java/org/jooq/impl/DefaultRenderContext.java +++ b/jOOQ/src/main/java/org/jooq/impl/DefaultRenderContext.java @@ -102,6 +102,7 @@ class DefaultRenderContext extends AbstractContext implements Ren private int alias; private int indent; private Deque indentLock; + private boolean separatorRequired; private boolean separator; private boolean newline; private int skipUpdateCounts; @@ -355,69 +356,77 @@ class DefaultRenderContext extends AbstractContext 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 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 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; } diff --git a/jOOQ/src/main/java/org/jooq/impl/KeywordImpl.java b/jOOQ/src/main/java/org/jooq/impl/KeywordImpl.java index 0520468fac..aed3735013 100644 --- a/jOOQ/src/main/java/org/jooq/impl/KeywordImpl.java +++ b/jOOQ/src/main/java/org/jooq/impl/KeywordImpl.java @@ -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); } diff --git a/jOOQ/src/main/java/org/jooq/impl/QueryPartList.java b/jOOQ/src/main/java/org/jooq/impl/QueryPartList.java index ef9ed421ab..1ca71fd274 100644 --- a/jOOQ/src/main/java/org/jooq/impl/QueryPartList.java +++ b/jOOQ/src/main/java/org/jooq/impl/QueryPartList.java @@ -80,31 +80,40 @@ class QueryPartList 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) diff --git a/jOOQ/src/main/java/org/jooq/impl/SelectQueryImpl.java b/jOOQ/src/main/java/org/jooq/impl/SelectQueryImpl.java index 1884c8c3d7..d33acbc30d 100644 --- a/jOOQ/src/main/java/org/jooq/impl/SelectQueryImpl.java +++ b/jOOQ/src/main/java/org/jooq/impl/SelectQueryImpl.java @@ -1053,7 +1053,7 @@ final class SelectQueryImpl extends AbstractResultQuery 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 extends AbstractResultQuery 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 extends AbstractResultQuery 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 extends AbstractResultQuery imp context.formatSeparator() .visit(K_FROM) - .sql(' ') + .separatorRequired(true) .visit(tablelist); @@ -1499,10 +1498,11 @@ final class SelectQueryImpl extends AbstractResultQuery 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 extends AbstractResultQuery 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 extends AbstractResultQuery 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 extends AbstractResultQuery 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 extends AbstractResultQuery imp if (orderBySiblings) ctx.sql(' ').visit(K_SIBLINGS); - ctx.sql(' ').visit(K_BY) - .sql(' '); + ctx.sql(' ').visit(K_BY).separatorRequired(true); diff --git a/jOOQ/src/main/java/org/jooq/impl/WindowSpecificationImpl.java b/jOOQ/src/main/java/org/jooq/impl/WindowSpecificationImpl.java index 91826b713f..3a6f8ead25 100644 --- a/jOOQ/src/main/java/org/jooq/impl/WindowSpecificationImpl.java +++ b/jOOQ/src/main/java/org/jooq/impl/WindowSpecificationImpl.java @@ -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 constant; + if (requiresOrderBy) { + Field 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) { diff --git a/jOOQ/src/main/java/org/jooq/impl/XMLElement.java b/jOOQ/src/main/java/org/jooq/impl/XMLElement.java index c8fa3be51f..c0eb6b6ca8 100644 --- a/jOOQ/src/main/java/org/jooq/impl/XMLElement.java +++ b/jOOQ/src/main/java/org/jooq/impl/XMLElement.java @@ -75,9 +75,7 @@ final class XMLElement extends AbstractField { 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('('); diff --git a/jOOQ/src/main/java/org/jooq/impl/XMLTable.java b/jOOQ/src/main/java/org/jooq/impl/XMLTable.java index e7b42ecff7..87cc587b36 100644 --- a/jOOQ/src/main/java/org/jooq/impl/XMLTable.java +++ b/jOOQ/src/main/java/org/jooq/impl/XMLTable.java @@ -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()