From 0602692413c59ff476cbc98d2e5c0ba5fd804633 Mon Sep 17 00:00:00 2001 From: lukaseder Date: Tue, 27 Feb 2018 15:24:30 +0100 Subject: [PATCH] [#7224] Do not emulate nested set operators if not strictly needed - [#7222] IN (SELECT .. UNION SELECT ..) doesn't work on Derby - [#7224] Do not emulate nested set operators if not strictly needed - [#6431] Recursive CTE doesn't work on MySQL because of automatic UNION nesting --- .../java/org/jooq/impl/AbstractContext.java | 33 ++++++- .../jooq/impl/CommonTableExpressionImpl.java | 10 +-- .../main/java/org/jooq/impl/DerivedTable.java | 4 +- .../java/org/jooq/impl/ExistsCondition.java | 4 +- .../java/org/jooq/impl/IsDistinctFrom.java | 8 +- .../org/jooq/impl/QuantifiedSelectImpl.java | 31 +++---- .../org/jooq/impl/RowSubqueryCondition.java | 6 +- .../java/org/jooq/impl/ScalarSubquery.java | 4 +- .../java/org/jooq/impl/SelectQueryImpl.java | 86 ++++++++++++++----- jOOQ/src/main/java/org/jooq/impl/Tools.java | 30 +++++++ jOOQ/src/main/java/org/jooq/impl/Values.java | 4 +- 11 files changed, 148 insertions(+), 72 deletions(-) diff --git a/jOOQ/src/main/java/org/jooq/impl/AbstractContext.java b/jOOQ/src/main/java/org/jooq/impl/AbstractContext.java index 6d8c36c880..ba4fbe0ce1 100644 --- a/jOOQ/src/main/java/org/jooq/impl/AbstractContext.java +++ b/jOOQ/src/main/java/org/jooq/impl/AbstractContext.java @@ -37,6 +37,7 @@ */ package org.jooq.impl; +import static java.lang.Boolean.TRUE; // ... // ... // ... @@ -44,11 +45,13 @@ package org.jooq.impl; import static org.jooq.conf.ParamType.INDEXED; import static org.jooq.impl.Tools.EMPTY_CLAUSE; import static org.jooq.impl.Tools.EMPTY_QUERYPART; +import static org.jooq.impl.Tools.DataKey.DATA_NESTED_SET_OPERATIONS; import static org.jooq.impl.Tools.DataKey.DATA_OMIT_CLAUSE_EVENT_EMISSION; import java.sql.PreparedStatement; import java.util.ArrayDeque; import java.util.ArrayList; +import java.util.BitSet; import java.util.Collections; import java.util.Deque; import java.util.EnumSet; @@ -97,7 +100,8 @@ abstract class AbstractContext> extends AbstractScope imple boolean declareAliases; boolean declareWindows; boolean declareCTE; - boolean subquery; + int subquery; + BitSet subqueryScopedNestedSetOperations; int stringLiteral; String stringLiteralEscapedApos = "'"; int index; @@ -505,12 +509,35 @@ abstract class AbstractContext> extends AbstractScope imple @Override public final boolean subquery() { - return subquery; + return subquery > 0; } @Override public final C subquery(boolean s) { - this.subquery = s; + if (s) { + subquery++; + + // [#7222] If present the nested set operations flag needs to be reset whenever we're + // entering a subquery (and restored when leaving it). + // [#2791] This works differently from the scope marking mechanism, which wraps a + // [#7152] naming scope for aliases and other object names. + if (TRUE.equals(data(DATA_NESTED_SET_OPERATIONS))) { + data().remove(DATA_NESTED_SET_OPERATIONS); + + if (subqueryScopedNestedSetOperations == null) + subqueryScopedNestedSetOperations = new BitSet(); + + subqueryScopedNestedSetOperations.set(subquery); + } + } + else { + if (subqueryScopedNestedSetOperations != null && + subqueryScopedNestedSetOperations.get(subquery)) + data(DATA_NESTED_SET_OPERATIONS, true); + + subquery--; + } + return (C) this; } diff --git a/jOOQ/src/main/java/org/jooq/impl/CommonTableExpressionImpl.java b/jOOQ/src/main/java/org/jooq/impl/CommonTableExpressionImpl.java index 72cb964679..1ade45cc04 100644 --- a/jOOQ/src/main/java/org/jooq/impl/CommonTableExpressionImpl.java +++ b/jOOQ/src/main/java/org/jooq/impl/CommonTableExpressionImpl.java @@ -93,9 +93,7 @@ final class CommonTableExpressionImpl extends AbstractTable @Override public final void accept(Context ctx) { - if (ctx.declareCTE()) { - boolean subquery = ctx.subquery(); - + if (ctx.declareCTE()) ctx.visit(name) .sql(' ') .visit(K_AS) @@ -106,12 +104,10 @@ final class CommonTableExpressionImpl extends AbstractTable .visit(select) .formatIndentEnd() .formatNewLine() - .subquery(subquery) + .subquery(false) .sql(')'); - } - else { + else ctx.visit(DSL.name(name.name)); - } } @Override diff --git a/jOOQ/src/main/java/org/jooq/impl/DerivedTable.java b/jOOQ/src/main/java/org/jooq/impl/DerivedTable.java index 3076c3ac51..561c031dac 100644 --- a/jOOQ/src/main/java/org/jooq/impl/DerivedTable.java +++ b/jOOQ/src/main/java/org/jooq/impl/DerivedTable.java @@ -86,15 +86,13 @@ final class DerivedTable extends AbstractTable { @Override public final void accept(Context ctx) { - boolean subquery = ctx.subquery(); - ctx.subquery(true) .formatIndentStart() .formatNewLine() .visit(query) .formatIndentEnd() .formatNewLine() - .subquery(subquery); + .subquery(false); } @Override diff --git a/jOOQ/src/main/java/org/jooq/impl/ExistsCondition.java b/jOOQ/src/main/java/org/jooq/impl/ExistsCondition.java index 7baae21576..60e6d26ff2 100644 --- a/jOOQ/src/main/java/org/jooq/impl/ExistsCondition.java +++ b/jOOQ/src/main/java/org/jooq/impl/ExistsCondition.java @@ -67,8 +67,6 @@ final class ExistsCondition extends AbstractCondition { @Override public final void accept(Context ctx) { - boolean subquery = ctx.subquery(); - ctx.visit(exists ? K_EXISTS : K_NOT_EXISTS) .sql(" (") .subquery(true) @@ -77,7 +75,7 @@ final class ExistsCondition extends AbstractCondition { .visit(query) .formatIndentEnd() .formatNewLine() - .subquery(subquery) + .subquery(false) .sql(')'); } diff --git a/jOOQ/src/main/java/org/jooq/impl/IsDistinctFrom.java b/jOOQ/src/main/java/org/jooq/impl/IsDistinctFrom.java index 2cb3396abe..b452d3d5cf 100644 --- a/jOOQ/src/main/java/org/jooq/impl/IsDistinctFrom.java +++ b/jOOQ/src/main/java/org/jooq/impl/IsDistinctFrom.java @@ -114,11 +114,13 @@ final class IsDistinctFrom extends AbstractCondition { */ private final QueryPartInternal delegate(Configuration configuration) { - // [#3511] These dialects need to emulate the IS DISTINCT FROM predicate, optimally using INTERSECT... + // [#3511] These dialects need to emulate the IS DISTINCT FROM predicate, + // optimally using INTERSECT... + // [#7222] [#7224] Make sure the columns are aliased if (EMULATE_DISTINCT_PREDICATE.contains(configuration.family())) { return (comparator == IS_DISTINCT_FROM) - ? (QueryPartInternal) notExists(select(lhs).intersect(select(rhs))) - : (QueryPartInternal) exists(select(lhs).intersect(select(rhs))); + ? (QueryPartInternal) notExists(select(lhs.as("x")).intersect(select(rhs.as("x")))) + : (QueryPartInternal) exists(select(lhs.as("x")).intersect(select(rhs.as("x")))); } // MySQL knows the <=> operator diff --git a/jOOQ/src/main/java/org/jooq/impl/QuantifiedSelectImpl.java b/jOOQ/src/main/java/org/jooq/impl/QuantifiedSelectImpl.java index dc933cef62..4bf0e2b8a1 100644 --- a/jOOQ/src/main/java/org/jooq/impl/QuantifiedSelectImpl.java +++ b/jOOQ/src/main/java/org/jooq/impl/QuantifiedSelectImpl.java @@ -90,27 +90,16 @@ final class QuantifiedSelectImpl extends AbstractQueryPart imp ) ; - // If this is already a subquery, proceed - if (ctx.subquery()) - ctx.visit(quantifier.toKeyword()) - .sql(extraParentheses ? " ((" : " (") - .formatIndentStart() - .formatNewLine() - .visit(delegate(ctx.configuration())) - .formatIndentEnd() - .formatNewLine() - .sql(extraParentheses ? "))" : ")"); - else - ctx.visit(quantifier.toKeyword()) - .sql(extraParentheses ? " ((" : " (") - .subquery(true) - .formatIndentStart() - .formatNewLine() - .visit(delegate(ctx.configuration())) - .formatIndentEnd() - .formatNewLine() - .subquery(false) - .sql(extraParentheses ? "))" : ")"); + ctx.visit(quantifier.toKeyword()) + .sql(extraParentheses ? " ((" : " (") + .subquery(true) + .formatIndentStart() + .formatNewLine() + .visit(delegate(ctx.configuration())) + .formatIndentEnd() + .formatNewLine() + .subquery(false) + .sql(extraParentheses ? "))" : ")"); } @Override diff --git a/jOOQ/src/main/java/org/jooq/impl/RowSubqueryCondition.java b/jOOQ/src/main/java/org/jooq/impl/RowSubqueryCondition.java index 5919d66d20..c7184fe188 100644 --- a/jOOQ/src/main/java/org/jooq/impl/RowSubqueryCondition.java +++ b/jOOQ/src/main/java/org/jooq/impl/RowSubqueryCondition.java @@ -211,8 +211,6 @@ final class RowSubqueryCondition extends AbstractCondition { @Override public final void accept(Context ctx) { - boolean subquery = ctx.subquery(); - ctx.visit(left) .sql(' ') .visit(comparator.toKeyword()) @@ -235,7 +233,7 @@ final class RowSubqueryCondition extends AbstractCondition { .visit(right) .formatIndentEnd() .formatNewLine() - .subquery(subquery); + .subquery(false); ctx.data(DATA_ROW_VALUE_EXPRESSION_PREDICATE_SUBQUERY, null); ctx.sql(extraParentheses ? "))" : ")"); } @@ -245,7 +243,7 @@ final class RowSubqueryCondition extends AbstractCondition { ctx.data(DATA_ROW_VALUE_EXPRESSION_PREDICATE_SUBQUERY, true); ctx.subquery(true) .visit(rightQuantified) - .subquery(subquery); + .subquery(false); ctx.data(DATA_ROW_VALUE_EXPRESSION_PREDICATE_SUBQUERY, null); } } diff --git a/jOOQ/src/main/java/org/jooq/impl/ScalarSubquery.java b/jOOQ/src/main/java/org/jooq/impl/ScalarSubquery.java index 11f52fb2e4..a5ee220bc0 100644 --- a/jOOQ/src/main/java/org/jooq/impl/ScalarSubquery.java +++ b/jOOQ/src/main/java/org/jooq/impl/ScalarSubquery.java @@ -66,8 +66,6 @@ final class ScalarSubquery extends AbstractField { @Override public final void accept(Context ctx) { - boolean subquery = ctx.subquery(); - ctx.sql('(') .subquery(true) .formatIndentStart() @@ -75,7 +73,7 @@ final class ScalarSubquery extends AbstractField { .visit(query) .formatIndentEnd() .formatNewLine() - .subquery(subquery) + .subquery(false) .sql(')'); } } diff --git a/jOOQ/src/main/java/org/jooq/impl/SelectQueryImpl.java b/jOOQ/src/main/java/org/jooq/impl/SelectQueryImpl.java index c0c3bc030a..a087c403ea 100644 --- a/jOOQ/src/main/java/org/jooq/impl/SelectQueryImpl.java +++ b/jOOQ/src/main/java/org/jooq/impl/SelectQueryImpl.java @@ -120,11 +120,13 @@ import static org.jooq.impl.Keywords.K_WINDOW; import static org.jooq.impl.Keywords.K_WITH_CHECK_OPTION; import static org.jooq.impl.Keywords.K_WITH_LOCK; import static org.jooq.impl.Keywords.K_WITH_READ_ONLY; +import static org.jooq.impl.Tools.EMPTY_FIELD; import static org.jooq.impl.Tools.fieldArray; import static org.jooq.impl.Tools.hasAmbiguousNames; import static org.jooq.impl.Tools.DataKey.DATA_COLLECTED_SEMI_ANTI_JOIN; import static org.jooq.impl.Tools.DataKey.DATA_COLLECT_SEMI_ANTI_JOIN; import static org.jooq.impl.Tools.DataKey.DATA_INSERT_SELECT_WITHOUT_INSERT_COLUMN_LIST; +import static org.jooq.impl.Tools.DataKey.DATA_NESTED_SET_OPERATIONS; import static org.jooq.impl.Tools.DataKey.DATA_OMIT_INTO_CLAUSE; import static org.jooq.impl.Tools.DataKey.DATA_OVERRIDE_ALIASES_IN_ORDER_BY; import static org.jooq.impl.Tools.DataKey.DATA_RENDER_TRAILING_LIMIT_IF_APPLICABLE; @@ -876,8 +878,6 @@ 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); - boolean subquery = ctx.subquery(); - ctx.visit(K_SELECT).sql(' ') .declareFields(true) .visit(new SelectFieldList>(unaliasedFields)) @@ -890,7 +890,7 @@ final class SelectQueryImpl extends AbstractResultQuery imp toSQLReference0(ctx, originalFields, alternativeFields); - ctx.subquery(subquery) + ctx.subquery(false) .formatIndentEnd() .formatNewLine() .sql(") ") @@ -1017,6 +1017,7 @@ final class SelectQueryImpl extends AbstractResultQuery imp SQLDialect family = dialect.family(); int unionOpSize = unionOp.size(); + boolean unionOpNesting = false; // The SQL standard specifies: // @@ -1038,10 +1039,11 @@ final class SelectQueryImpl extends AbstractResultQuery imp - // [#2995] Prevent the generation of wrapping parentheses around the - // INSERT .. SELECT statement's SELECT because they would be - // interpreted as the (missing) INSERT column list's parens. - || (context.data(DATA_INSERT_SELECT_WITHOUT_INSERT_COLUMN_LIST) != null && unionOpSize > 0); +// // [#2995] Prevent the generation of wrapping parentheses around the +// // INSERT .. SELECT statement's SELECT because they would be +// // interpreted as the (missing) INSERT column list's parens. +// || (context.data(DATA_INSERT_SELECT_WITHOUT_INSERT_COLUMN_LIST) != null && unionOpSize > 0) + ; if (wrapQueryExpressionInDerivedTable) context.visit(K_SELECT).sql(" *") @@ -1087,6 +1089,9 @@ final class SelectQueryImpl extends AbstractResultQuery imp // [#1658] jOOQ applies left-associativity to set operators. In order to enforce that across // all databases, we need to wrap relevant subqueries in parentheses. if (unionOpSize > 0) { + if (!TRUE.equals(context.data(DATA_NESTED_SET_OPERATIONS))) + context.data(DATA_NESTED_SET_OPERATIONS, unionOpNesting = unionOpNesting()); + for (int i = unionOpSize - 1; i >= 0; i--) { switch (unionOp.get(i)) { case EXCEPT: context.start(SELECT_EXCEPT); break; @@ -1097,7 +1102,7 @@ final class SelectQueryImpl extends AbstractResultQuery imp case UNION_ALL: context.start(SELECT_UNION_ALL); break; } - unionParenthesis(context, "("); + unionParenthesis(context, '(', getSelect().toArray(EMPTY_FIELD)); } } @@ -1376,7 +1381,7 @@ final class SelectQueryImpl extends AbstractResultQuery imp // SET operations like UNION, EXCEPT, INTERSECT // -------------------------------------------- if (unionOpSize > 0) { - unionParenthesis(context, ")"); + unionParenthesis(context, ')', null); for (int i = 0; i < unionOpSize; i++) { CombineOperator op = unionOp.get(i); @@ -1386,14 +1391,14 @@ final class SelectQueryImpl extends AbstractResultQuery imp .visit(op.toKeyword(dialect)) .sql(' '); - unionParenthesis(context, "("); + unionParenthesis(context, '(', other.getSelect().toArray(EMPTY_FIELD)); context.visit(other); - unionParenthesis(context, ")"); + unionParenthesis(context, ')', null); } // [#1658] Close parentheses opened previously if (i < unionOpSize - 1) - unionParenthesis(context, ")"); + unionParenthesis(context, ')', null); switch (unionOp.get(i)) { case EXCEPT: context.end(SELECT_EXCEPT); break; @@ -1404,6 +1409,9 @@ final class SelectQueryImpl extends AbstractResultQuery imp case UNION_ALL: context.end(SELECT_UNION_ALL); break; } } + + if (unionOpNesting) + context.data().remove(DATA_NESTED_SET_OPERATIONS); } @@ -1575,21 +1583,55 @@ final class SelectQueryImpl extends AbstractResultQuery imp private static final EnumSet UNION_PARENTHESIS = EnumSet.of(DERBY, MARIADB, MYSQL, SQLITE); - private final void unionParenthesis(Context ctx, String parenthesis) { - if (")".equals(parenthesis)) { + private final boolean unionOpNesting() { + if (unionOp.size() > 1) + return true; + + for (QueryPartList> s1 : union) + for (Select s2 : s1) + if (s2 instanceof SelectQueryImpl + && ((SelectQueryImpl) s2).unionOp.size() > 0) + return true; + else if (s2 instanceof SelectImpl + && ((SelectImpl) s2).getDelegate() instanceof SelectQueryImpl + && ((SelectQueryImpl) ((SelectImpl) s2).getDelegate()).unionOp.size() > 0) + return true; + + return false; + } + + private final void unionParenthesis(Context ctx, char parenthesis, Field[] fields) { + boolean derivedTable = + (TRUE.equals(ctx.data(DATA_NESTED_SET_OPERATIONS)) && UNION_PARENTHESIS.contains(ctx.family())) + + || ctx.data(DATA_INSERT_SELECT_WITHOUT_INSERT_COLUMN_LIST) != null + + // [#7222] Workaround for https://issues.apache.org/jira/browse/DERBY-6984 + || (ctx.family() == DERBY && ctx.subquery()); + + if (')' == parenthesis) { ctx.formatIndentEnd() .formatNewLine(); } // [#3579] Nested set operators aren't supported in some databases. Emulate them via derived tables... - else if ("(".equals(parenthesis)) { - if (UNION_PARENTHESIS.contains(ctx.family())) + // [#7222] Do this only in the presence of actual nested set operators + else if ('(' == parenthesis) { + if (derivedTable) { ctx.formatNewLine() - .visit(K_SELECT) - .sql(" *") - .formatSeparator() + .visit(K_SELECT).sql(' '); + + // [#7222] Workaround for https://issues.apache.org/jira/browse/DERBY-6983 + if (ctx.family() == DERBY) + ctx.visit(new SelectFieldList>(Tools.unqualified(fields))); + else + ctx.sql('*'); + + + ctx.formatSeparator() .visit(K_FROM) .sql(' '); + } } // [#3579] ... but don't use derived tables to emulate nested set operators for Firebird, as that @@ -1607,13 +1649,13 @@ final class SelectQueryImpl extends AbstractResultQuery imp break; } - if ("(".equals(parenthesis)) { + if ('(' == parenthesis) { ctx.formatIndentStart() .formatNewLine(); } - else if (")".equals(parenthesis)) { - if (UNION_PARENTHESIS.contains(ctx.family())) + else if (')'== parenthesis) { + if (derivedTable) ctx.sql(" x"); } } diff --git a/jOOQ/src/main/java/org/jooq/impl/Tools.java b/jOOQ/src/main/java/org/jooq/impl/Tools.java index a91bd63a2c..32e2adabda 100644 --- a/jOOQ/src/main/java/org/jooq/impl/Tools.java +++ b/jOOQ/src/main/java/org/jooq/impl/Tools.java @@ -473,6 +473,12 @@ final class Tools { * The level of anonymous block nesting, in case we're generating a block. */ DATA_BLOCK_NESTING, + + /** + * [#3579] [#6431] [#7222] There are nested set operations in the current + * {@link Select} scope. + */ + DATA_NESTED_SET_OPERATIONS } /** @@ -1024,6 +1030,30 @@ final class Tools { return result; } + static final Field[] unqualified(Field[] fields) { + if (fields == null) + return null; + + Field[] result = new Field[fields.length]; + + for (int i = 0; i < fields.length; i++) + result[i] = DSL.field(fields[i].getUnqualifiedName(), fields[i].getDataType()); + + return result; + } + + static final Name[] unqualifiedNames(Field[] fields) { + if (fields == null) + return null; + + Name[] result = new Name[fields.length]; + + for (int i = 0; i < fields.length; i++) + result[i] = fields[i].getUnqualifiedName(); + + return result; + } + static final Field[] aliasedFields(Field[] fields, Name[] aliases) { if (fields == null) return null; diff --git a/jOOQ/src/main/java/org/jooq/impl/Values.java b/jOOQ/src/main/java/org/jooq/impl/Values.java index 0e7156da46..badd9f4a8b 100644 --- a/jOOQ/src/main/java/org/jooq/impl/Values.java +++ b/jOOQ/src/main/java/org/jooq/impl/Values.java @@ -110,8 +110,6 @@ final class Values extends AbstractTable { case MARIADB: case MYSQL: { Select selects = null; - boolean subquery = ctx.subquery(); - for (Row row : rows) { Select select = create().select(row.fields()); @@ -127,7 +125,7 @@ final class Values extends AbstractTable { .formatNewLine() .subquery(true) .visit(selects) - .subquery(subquery) + .subquery(false) .formatIndentEnd() .formatNewLine(); break;