From 25772619df4ae0953ed7413271ae96b8fc3abc43 Mon Sep 17 00:00:00 2001 From: Knut Wannheden Date: Wed, 22 Jan 2020 12:09:11 +0100 Subject: [PATCH] [jOOQ/jOOQ#3676] Try avoid rendering parenthesis for set operations By default (`Settings#renderParenthesisAroundSetOperationQueries = false`) jOOQ will attempt to avoid rendering parenthesis pairs around the queries combined with set operators (e.g. `UNION` or `UNION ALL`). In situations where this causes problems, i.e. when the parenthesis pair is required, the setting can be set to `true` in which case the parenthesis pair will always be rendered around each of the combined queries. --- .../src/main/java/org/jooq/conf/Settings.java | 49 ++++++++++++++++ .../java/org/jooq/impl/SelectQueryImpl.java | 58 +++++++++++++++---- .../resources/xsd/jooq-runtime-3.13.0.xsd | 11 ++++ 3 files changed, 107 insertions(+), 11 deletions(-) diff --git a/jOOQ/src/main/java/org/jooq/conf/Settings.java b/jOOQ/src/main/java/org/jooq/conf/Settings.java index acc923a5de..b15fdead55 100644 --- a/jOOQ/src/main/java/org/jooq/conf/Settings.java +++ b/jOOQ/src/main/java/org/jooq/conf/Settings.java @@ -77,6 +77,8 @@ public class Settings protected Boolean renderOrderByRownumberForEmulatedPagination = true; @XmlElement(defaultValue = "true") protected Boolean renderOutputForSQLServerReturningClause = true; + @XmlElement(defaultValue = "false") + protected Boolean renderParenthesisAroundSetOperationQueries = false; @XmlElement(defaultValue = "true") protected Boolean fetchTriggerValuesAfterSQLServerOutput = true; @XmlElement(defaultValue = "false") @@ -641,6 +643,37 @@ public class Settings this.renderOutputForSQLServerReturningClause = value; } + /** + * Whether queries combined with set operators (e.g. UNION and UNION ALL) should always be surrounded by a parenthesis pair. + *

+ * By default (i.e. when this setting is set to false jOOQ will only render parenthesis pairs around queries combined with set operators when required. + * This is for example the case when set operators are nested, when non-associative operators like EXCEPT are used, or when the queries are rendered as derived tables. + *

+ * When this setting is set to true the queries combined with set operators will always be surrounded by a parenthesis pair. + *

+ * For details, see https://github.com/jOOQ/jOOQ/issues/3676 and https://github.com/jOOQ/jOOQ/issues/9751. + * + * @return + * possible object is + * {@link Boolean } + * + */ + public Boolean isRenderParenthesisAroundSetOperationQueries() { + return renderParenthesisAroundSetOperationQueries; + } + + /** + * Sets the value of the renderParenthesisAroundSetOperationQueries property. + * + * @param value + * allowed object is + * {@link Boolean } + * + */ + public void setRenderParenthesisAroundSetOperationQueries(Boolean value) { + this.renderParenthesisAroundSetOperationQueries = value; + } + /** * Fetch trigger values after SQL Server OUTPUT clause. *

@@ -2080,6 +2113,11 @@ public class Settings return this; } + public Settings withRenderParenthesisAroundSetOperationQueries(Boolean value) { + setRenderParenthesisAroundSetOperationQueries(value); + return this; + } + public Settings withFetchTriggerValuesAfterSQLServerOutput(Boolean value) { setFetchTriggerValuesAfterSQLServerOutput(value); return this; @@ -2621,6 +2659,7 @@ public class Settings builder.append("renderScalarSubqueriesForStoredFunctions", renderScalarSubqueriesForStoredFunctions); builder.append("renderOrderByRownumberForEmulatedPagination", renderOrderByRownumberForEmulatedPagination); builder.append("renderOutputForSQLServerReturningClause", renderOutputForSQLServerReturningClause); + builder.append("renderParenthesisAroundSetOperationQueries", renderParenthesisAroundSetOperationQueries); builder.append("fetchTriggerValuesAfterSQLServerOutput", fetchTriggerValuesAfterSQLServerOutput); builder.append("transformTableListsToAnsiJoin", transformTableListsToAnsiJoin); builder.append("backslashEscaping", backslashEscaping); @@ -2859,6 +2898,15 @@ public class Settings return false; } } + if (renderParenthesisAroundSetOperationQueries == null) { + if (other.renderParenthesisAroundSetOperationQueries!= null) { + return false; + } + } else { + if (!renderParenthesisAroundSetOperationQueries.equals(other.renderParenthesisAroundSetOperationQueries)) { + return false; + } + } if (fetchTriggerValuesAfterSQLServerOutput == null) { if (other.fetchTriggerValuesAfterSQLServerOutput!= null) { return false; @@ -3459,6 +3507,7 @@ public class Settings result = ((prime*result)+((renderScalarSubqueriesForStoredFunctions == null)? 0 :renderScalarSubqueriesForStoredFunctions.hashCode())); result = ((prime*result)+((renderOrderByRownumberForEmulatedPagination == null)? 0 :renderOrderByRownumberForEmulatedPagination.hashCode())); result = ((prime*result)+((renderOutputForSQLServerReturningClause == null)? 0 :renderOutputForSQLServerReturningClause.hashCode())); + result = ((prime*result)+((renderParenthesisAroundSetOperationQueries == null)? 0 :renderParenthesisAroundSetOperationQueries.hashCode())); result = ((prime*result)+((fetchTriggerValuesAfterSQLServerOutput == null)? 0 :fetchTriggerValuesAfterSQLServerOutput.hashCode())); result = ((prime*result)+((transformTableListsToAnsiJoin == null)? 0 :transformTableListsToAnsiJoin.hashCode())); result = ((prime*result)+((backslashEscaping == null)? 0 :backslashEscaping.hashCode())); diff --git a/jOOQ/src/main/java/org/jooq/impl/SelectQueryImpl.java b/jOOQ/src/main/java/org/jooq/impl/SelectQueryImpl.java index 67013be90c..bc5c137622 100644 --- a/jOOQ/src/main/java/org/jooq/impl/SelectQueryImpl.java +++ b/jOOQ/src/main/java/org/jooq/impl/SelectQueryImpl.java @@ -1140,6 +1140,7 @@ final class SelectQueryImpl extends AbstractResultQuery imp boolean qualify = context.qualify(); int unionOpSize = unionOp.size(); + boolean unionParensRequired = false; boolean unionOpNesting = false; // The SQL standard specifies: @@ -1232,7 +1233,10 @@ final class SelectQueryImpl extends AbstractResultQuery imp case UNION_ALL: context.start(SELECT_UNION_ALL); break; } - unionParenthesis(context, '(', getSelect().toArray(EMPTY_FIELD)); + // [#3676] There might be cases where nested set operations do not + // imply required parentheses in some dialects, but better + // play safe than sorry + unionParenthesis(context, '(', getSelect().toArray(EMPTY_FIELD), unionParensRequired = unionOpNesting || unionParensRequired(context)); } } @@ -1521,7 +1525,7 @@ final class SelectQueryImpl extends AbstractResultQuery imp // SET operations like UNION, EXCEPT, INTERSECT // -------------------------------------------- if (unionOpSize > 0) { - unionParenthesis(context, ')', null); + unionParenthesis(context, ')', null, unionParensRequired); for (int i = 0; i < unionOpSize; i++) { CombineOperator op = unionOp.get(i); @@ -1531,14 +1535,14 @@ final class SelectQueryImpl extends AbstractResultQuery imp .visit(op.toKeyword(family)) .sql(' '); - unionParenthesis(context, '(', other.getSelect().toArray(EMPTY_FIELD)); + unionParenthesis(context, '(', other.getSelect().toArray(EMPTY_FIELD), unionParensRequired); context.visit(other); - unionParenthesis(context, ')', null); + unionParenthesis(context, ')', null, unionParensRequired); } // [#1658] Close parentheses opened previously if (i < unionOpSize - 1) - unionParenthesis(context, ')', null); + unionParenthesis(context, ')', null, unionParensRequired); switch (unionOp.get(i)) { case EXCEPT: context.end(SELECT_EXCEPT); break; @@ -1773,7 +1777,35 @@ final class SelectQueryImpl extends AbstractResultQuery imp return false; } - private final void unionParenthesis(Context ctx, char parenthesis, Field[] fields) { + private final boolean unionParensRequired(Context context) { + if (unionParensRequired(this) || context.settings().isRenderParenthesisAroundSetOperationQueries()) + return true; + + CombineOperator op = unionOp.get(0); + + // [#3676] EXCEPT and EXCEPT ALL are not associative + if ((op == EXCEPT || op == EXCEPT_ALL) && union.get(0).size() > 1) + return true; + + // [#3676] if a query has an ORDER BY or LIMIT clause parens are required + for (QueryPartList> s1 : union) + for (Select s2 : s1) + if (s2 instanceof SelectQueryImpl + && unionParensRequired((SelectQueryImpl) s2)) + return true; + else if (s2 instanceof SelectImpl + && ((SelectImpl) s2).getDelegate() instanceof SelectQueryImpl + && unionParensRequired((SelectQueryImpl) ((SelectImpl) s2).getDelegate())) + return true; + + return false; + } + + private final boolean unionParensRequired(SelectQueryImpl select) { + return select.orderBy.size() > 0 || select.limit.isApplicable(); + } + + private final void unionParenthesis(Context ctx, char parenthesis, Field[] fields, boolean parensRequired) { boolean derivedTable = // [#3579] [#6431] [#7222] Some databases don't support nested set operations at all @@ -1792,14 +1824,16 @@ final class SelectQueryImpl extends AbstractResultQuery imp || (ctx.subquery() && UNION_PARENTHESIS_IN_DERIVED_TABLES.contains(ctx.family())) ; - if (')' == parenthesis) { + parensRequired |= derivedTable; + + if (parensRequired && ')' == parenthesis) { ctx.formatIndentEnd() .formatNewLine(); } // [#3579] Nested set operators aren't supported in some databases. Emulate them via derived tables... // [#7222] Do this only in the presence of actual nested set operators - else if ('(' == parenthesis) { + else if (parensRequired && '(' == parenthesis) { if (derivedTable) { ctx.formatNewLine() .visit(K_SELECT).sql(' '); @@ -1828,16 +1862,18 @@ final class SelectQueryImpl extends AbstractResultQuery imp break; default: - ctx.sql(parenthesis); + if (parensRequired) + ctx.sql(parenthesis); + break; } - if ('(' == parenthesis) { + if (parensRequired && '(' == parenthesis) { ctx.formatIndentStart() .formatNewLine(); } - else if (')'== parenthesis) { + else if (parensRequired && ')' == parenthesis) { if (derivedTable) ctx.sql(" x"); } diff --git a/jOOQ/src/main/resources/xsd/jooq-runtime-3.13.0.xsd b/jOOQ/src/main/resources/xsd/jooq-runtime-3.13.0.xsd index eedd0fdc36..060fa7b182 100644 --- a/jOOQ/src/main/resources/xsd/jooq-runtime-3.13.0.xsd +++ b/jOOQ/src/main/resources/xsd/jooq-runtime-3.13.0.xsd @@ -155,6 +155,17 @@ be enabled as well. For details, see https://github.com/jOOQ/jOOQ/issues/4498.]]> + + +By default (i.e. when this setting is set to false jOOQ will only render parenthesis pairs around queries combined with set operators when required. +This is for example the case when set operators are nested, when non-associative operators like EXCEPT are used, or when the queries are rendered as derived tables. +

+When this setting is set to true the queries combined with set operators will always be surrounded by a parenthesis pair. +

+For details, see https://github.com/jOOQ/jOOQ/issues/3676 and https://github.com/jOOQ/jOOQ/issues/9751.]]> + + OUTPUT clause.