From 23a0ae44e0aacf01519935965e7b9033f8727cf0 Mon Sep 17 00:00:00 2001 From: Lukas Eder Date: Thu, 22 Aug 2024 17:25:14 +0200 Subject: [PATCH] [jOOQ/jOOQ#17131] Improve formatting of functions containing CASE arguments - WIP --- .../java/org/jooq/impl/AbstractFunction.java | 9 ++- .../main/java/org/jooq/impl/CaseSearched.java | 1 + .../main/java/org/jooq/impl/CaseSimple.java | 1 + .../org/jooq/impl/ComplexCheckQueryPart.java | 66 +++++++++++++++++++ .../java/org/jooq/impl/ComplexQueryPart.java | 52 +++++++++++++++ .../java/org/jooq/impl/JSONEntryImpl.java | 14 +++- .../jooq/impl/QueryPartCollectionView.java | 24 ++++--- jOOQ/src/main/java/org/jooq/impl/Tools.java | 5 ++ 8 files changed, 160 insertions(+), 12 deletions(-) create mode 100644 jOOQ/src/main/java/org/jooq/impl/ComplexCheckQueryPart.java create mode 100644 jOOQ/src/main/java/org/jooq/impl/ComplexQueryPart.java diff --git a/jOOQ/src/main/java/org/jooq/impl/AbstractFunction.java b/jOOQ/src/main/java/org/jooq/impl/AbstractFunction.java index 50bbd146a1..8c43fabce9 100644 --- a/jOOQ/src/main/java/org/jooq/impl/AbstractFunction.java +++ b/jOOQ/src/main/java/org/jooq/impl/AbstractFunction.java @@ -39,6 +39,7 @@ package org.jooq.impl; import static org.jooq.impl.Tools.camelCase; import static org.jooq.impl.Tools.getMappedSchema; +import static org.jooq.impl.Tools.isComplex; import org.jooq.Context; import org.jooq.DataType; @@ -62,6 +63,8 @@ abstract class AbstractFunction extends AbstractField implements QOM.Funct @Override public final void accept(Context ctx) { + QueryPart args = arguments(); + switch (ctx.family()) { @@ -72,7 +75,11 @@ abstract class AbstractFunction extends AbstractField implements QOM.Funct default: { acceptFunctionName(ctx, applySchemaMapping, getQualifiedName()); - ctx.sql('(').visit(arguments()).sql(')'); + + if (ctx.format() && isComplex(ctx, args)) + ctx.sqlIndentStart('(').visit(args).sqlIndentEnd(')'); + else + ctx.sql('(').visit(args).sql(')'); break; } diff --git a/jOOQ/src/main/java/org/jooq/impl/CaseSearched.java b/jOOQ/src/main/java/org/jooq/impl/CaseSearched.java index 94d738b7ff..c4cde146cc 100644 --- a/jOOQ/src/main/java/org/jooq/impl/CaseSearched.java +++ b/jOOQ/src/main/java/org/jooq/impl/CaseSearched.java @@ -73,6 +73,7 @@ extends AbstractField implements CaseConditionStep, + ComplexQueryPart, QOM.CaseSearched { diff --git a/jOOQ/src/main/java/org/jooq/impl/CaseSimple.java b/jOOQ/src/main/java/org/jooq/impl/CaseSimple.java index 7120b15004..75b90ce654 100644 --- a/jOOQ/src/main/java/org/jooq/impl/CaseSimple.java +++ b/jOOQ/src/main/java/org/jooq/impl/CaseSimple.java @@ -67,6 +67,7 @@ extends AbstractCaseSimple> implements CaseWhenStep, + ComplexQueryPart, QOM.CaseSimple { diff --git a/jOOQ/src/main/java/org/jooq/impl/ComplexCheckQueryPart.java b/jOOQ/src/main/java/org/jooq/impl/ComplexCheckQueryPart.java new file mode 100644 index 0000000000..1172b04a17 --- /dev/null +++ b/jOOQ/src/main/java/org/jooq/impl/ComplexCheckQueryPart.java @@ -0,0 +1,66 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * Other licenses: + * ----------------------------------------------------------------------------- + * Commercial licenses for this work are available. These replace the above + * Apache-2.0 license and offer limited warranties, support, maintenance, and + * commercial database integrations. + * + * For more information, please visit: https://www.jooq.org/legal/licensing + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + */ +package org.jooq.impl; + +import org.jooq.Context; +import org.jooq.OrderField; +import org.jooq.QueryPart; +import org.jooq.QueryPartInternal; + +/** + * A marker interface for all query parts that are incapable of generating + * "simple" SQL. This information is used mainly for formatting decisions. + *

+ * Unlike {@link ComplexQueryPart}, which marks a {@link QueryPart} that is + * unconditionally complex, this allows for checking whether a {@link QueryPart} + * is {@link #isComplex(Context)}. + * + * @author Lukas Eder + */ +interface ComplexCheckQueryPart extends QueryPartInternal { + + /** + * Whether the {@link QueryPart} really is complex. + *

+ * e.g. an {@link OrderField} can be complex if any of its contents are also + * complex. + */ + default boolean isComplex(Context ctx) { + return true; + } +} diff --git a/jOOQ/src/main/java/org/jooq/impl/ComplexQueryPart.java b/jOOQ/src/main/java/org/jooq/impl/ComplexQueryPart.java new file mode 100644 index 0000000000..69e78ea0b1 --- /dev/null +++ b/jOOQ/src/main/java/org/jooq/impl/ComplexQueryPart.java @@ -0,0 +1,52 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * Other licenses: + * ----------------------------------------------------------------------------- + * Commercial licenses for this work are available. These replace the above + * Apache-2.0 license and offer limited warranties, support, maintenance, and + * commercial database integrations. + * + * For more information, please visit: https://www.jooq.org/legal/licensing + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + */ +package org.jooq.impl; + +import org.jooq.QueryPartInternal; + +/** + * A marker interface for all query parts that are incapable of generating + * "simple" SQL. This information is used mainly for formatting decisions. + *

+ * Unlike {@link ComplexCheckQueryPart}, this is always complex. + * + * @author Lukas Eder + */ +interface ComplexQueryPart extends QueryPartInternal { + +} diff --git a/jOOQ/src/main/java/org/jooq/impl/JSONEntryImpl.java b/jOOQ/src/main/java/org/jooq/impl/JSONEntryImpl.java index 4c932e884e..f7a139486e 100644 --- a/jOOQ/src/main/java/org/jooq/impl/JSONEntryImpl.java +++ b/jOOQ/src/main/java/org/jooq/impl/JSONEntryImpl.java @@ -107,7 +107,14 @@ import org.jooq.conf.NestedCollectionEmulation; * * @author Lukas Eder */ -final class JSONEntryImpl extends AbstractQueryPart implements JSONEntry, JSONEntryValueStep { +final class JSONEntryImpl +extends + AbstractQueryPart +implements + JSONEntry, + JSONEntryValueStep, + ComplexCheckQueryPart +{ static final Set SUPPORT_JSON_MERGE_PRESERVE = SQLDialect.supportedBy(MARIADB, MYSQL); @@ -123,6 +130,11 @@ final class JSONEntryImpl extends AbstractQueryPart implements JSONEntry, this.value = value; } + @Override + public final boolean isComplex(Context ctx) { + return Tools.isComplex(ctx, value); + } + @Override public final Field key() { return key; diff --git a/jOOQ/src/main/java/org/jooq/impl/QueryPartCollectionView.java b/jOOQ/src/main/java/org/jooq/impl/QueryPartCollectionView.java index 1531150e50..36066ef34a 100644 --- a/jOOQ/src/main/java/org/jooq/impl/QueryPartCollectionView.java +++ b/jOOQ/src/main/java/org/jooq/impl/QueryPartCollectionView.java @@ -46,29 +46,21 @@ import static org.jooq.impl.Tools.isRendersSeparator; import static org.jooq.impl.Tools.last; import static org.jooq.impl.Tools.BooleanDataKey.DATA_LIST_ALREADY_INDENTED; -import java.util.ArrayList; import java.util.BitSet; import java.util.Collection; import java.util.Collections; import java.util.Iterator; import java.util.List; -import java.util.Objects; -import java.util.Set; import java.util.function.Function; -import java.util.function.Predicate; import org.jooq.Condition; import org.jooq.Context; -import org.jooq.Function0; -import org.jooq.Function1; import org.jooq.QueryPart; import org.jooq.QueryPartInternal; // ... // ... import org.jooq.impl.QOM.UnmodifiableCollection; -import org.jetbrains.annotations.NotNull; - /** * A {@link List} view, delegating all calls to a wrapped list, but acting like * a {@link QueryPart}. @@ -128,6 +120,10 @@ implements return wrapped; } + private final boolean isComplex(Context ctx) { + return anyMatch(this, e -> Tools.isComplex(ctx, e)); + } + @Override public boolean isSimple(Context ctx) { return allMatch(this, e -> Tools.isSimple(ctx, e)); @@ -146,6 +142,14 @@ implements return !isEmpty(); } + final boolean format(Context ctx, int size) { + return ctx.format() && ( + size == 1 && isComplex(ctx) + || size >= 2 && !isSimple(ctx) + || size > 4 + ); + } + @Override public /* non-final */ void accept(Context ctx) { BitSet rendersContent = new BitSet(size()); @@ -155,7 +159,7 @@ implements rendersContent.set(i++, ((QueryPartInternal) e).rendersContent(ctx)); int size = rendersContent.cardinality(); - boolean format = ctx.format() && (size >= 2 && !isSimple(ctx) || size > 4); + boolean format = format(ctx, size); boolean previousQualify = ctx.qualify(); boolean previousAlreadyIndented = TRUE.equals(ctx.data(DATA_LIST_ALREADY_INDENTED)); boolean indent = format && !previousAlreadyIndented; @@ -213,7 +217,7 @@ implements ctx.data( DATA_LIST_ALREADY_INDENTED, - t instanceof QueryPartCollectionView && ((QueryPartCollectionView) t).size() > 1, + t instanceof QueryPartCollectionView && ((QueryPartCollectionView) t).format(ctx, ((QueryPartCollectionView) t).size()), c -> acceptElement(c, t) ); } diff --git a/jOOQ/src/main/java/org/jooq/impl/Tools.java b/jOOQ/src/main/java/org/jooq/impl/Tools.java index 2da98c18a9..0e678a0a5b 100644 --- a/jOOQ/src/main/java/org/jooq/impl/Tools.java +++ b/jOOQ/src/main/java/org/jooq/impl/Tools.java @@ -4097,6 +4097,11 @@ final class Tools { return part instanceof AbstractWindowFunction && ((AbstractWindowFunction) part).isWindow(); } + static final boolean isComplex(Context ctx, QueryPart part) { + return part instanceof ComplexQueryPart + || part instanceof ComplexCheckQueryPart && ((ComplexCheckQueryPart) part).isComplex(ctx); + } + static final boolean isSimple(Context ctx, QueryPart part) { return part instanceof SimpleQueryPart || part instanceof SimpleCheckQueryPart && ((SimpleCheckQueryPart) part).isSimple(ctx);