From ae3058789750c52574caada7a1c0eecefb344b11 Mon Sep 17 00:00:00 2001 From: Lukas Eder Date: Thu, 1 Jul 2021 14:31:26 +0200 Subject: [PATCH] [jOOQ/jOOQ#12092] MySQL queries running GROUP_CONCAT() or JSON_ARRAYAGG() should auto-increase the default group_concat_max_len --- jOOQ/src/main/java/org/jooq/Context.java | 8 ++- .../src/main/java/org/jooq/conf/Settings.java | 49 +++++++++++++++++++ .../java/org/jooq/impl/AbstractContext.java | 7 ++- .../org/jooq/impl/DefaultRenderContext.java | 14 ++++-- jOOQ/src/main/java/org/jooq/impl/ListAgg.java | 23 ++++++--- jOOQ/src/main/java/org/jooq/impl/Tools.java | 35 +++++++++++-- .../resources/xsd/jooq-runtime-3.15.0.xsd | 11 +++++ 7 files changed, 130 insertions(+), 17 deletions(-) diff --git a/jOOQ/src/main/java/org/jooq/Context.java b/jOOQ/src/main/java/org/jooq/Context.java index 7cb3098f6e..6583b16485 100644 --- a/jOOQ/src/main/java/org/jooq/Context.java +++ b/jOOQ/src/main/java/org/jooq/Context.java @@ -343,11 +343,17 @@ public interface Context> extends Scope { int peekIndex(); /** - * Skip an update count produced by this query. + * Skip an additional update count produced by this query. */ @NotNull C skipUpdateCount(); + /** + * Skip a number of additional update counts produced by this query. + */ + @NotNull + C skipUpdateCounts(int skip); + /** * The number of update counts to be skipped by this query. */ diff --git a/jOOQ/src/main/java/org/jooq/conf/Settings.java b/jOOQ/src/main/java/org/jooq/conf/Settings.java index ccdb7a1f79..16c60ba89b 100644 --- a/jOOQ/src/main/java/org/jooq/conf/Settings.java +++ b/jOOQ/src/main/java/org/jooq/conf/Settings.java @@ -96,6 +96,8 @@ public class Settings protected Boolean renderOrderByRownumberForEmulatedPagination = true; @XmlElement(defaultValue = "true") protected Boolean renderOutputForSQLServerReturningClause = true; + @XmlElement(defaultValue = "true") + protected Boolean renderGroupConcatMaxLenSessionVariable = true; @XmlElement(defaultValue = "false") protected Boolean renderParenthesisAroundSetOperationQueries = false; @XmlElement(defaultValue = ".") @@ -866,6 +868,37 @@ public class Settings this.renderOutputForSQLServerReturningClause = value; } + /** + * Whether the jOOQ GROUP_CONCAT function should be overflow-protected by setting the @@group_concat_max_len session variable in MySQL style database systems. + *

+ * MySQL truncates GROUP_CONCAT results after a certain length, which may be way + * too small for jOOQ's usage, especially when using the MULTISET emulation. By + * default, jOOQ sets a session variable to the highest possible value prior to executing a + * query containing GROUP_CONCAT. This flag can be used to opt out of this. + *

+ * For details, see https://github.com/jOOQ/jOOQ/issues/12092. + * + * @return + * possible object is + * {@link Boolean } + * + */ + public Boolean isRenderGroupConcatMaxLenSessionVariable() { + return renderGroupConcatMaxLenSessionVariable; + } + + /** + * Sets the value of the renderGroupConcatMaxLenSessionVariable property. + * + * @param value + * allowed object is + * {@link Boolean } + * + */ + public void setRenderGroupConcatMaxLenSessionVariable(Boolean value) { + this.renderGroupConcatMaxLenSessionVariable = value; + } + /** * Whether queries combined with set operators (e.g. UNION and UNION ALL) should always be surrounded by a parenthesis pair. *

@@ -3139,6 +3172,11 @@ public class Settings return this; } + public Settings withRenderGroupConcatMaxLenSessionVariable(Boolean value) { + setRenderGroupConcatMaxLenSessionVariable(value); + return this; + } + public Settings withRenderParenthesisAroundSetOperationQueries(Boolean value) { setRenderParenthesisAroundSetOperationQueries(value); return this; @@ -3972,6 +4010,7 @@ public class Settings builder.append("renderCoalesceToEmptyStringInConcat", renderCoalesceToEmptyStringInConcat); builder.append("renderOrderByRownumberForEmulatedPagination", renderOrderByRownumberForEmulatedPagination); builder.append("renderOutputForSQLServerReturningClause", renderOutputForSQLServerReturningClause); + builder.append("renderGroupConcatMaxLenSessionVariable", renderGroupConcatMaxLenSessionVariable); builder.append("renderParenthesisAroundSetOperationQueries", renderParenthesisAroundSetOperationQueries); builder.append("namePathSeparator", namePathSeparator); builder.append("bindOffsetDateTimeType", bindOffsetDateTimeType); @@ -4305,6 +4344,15 @@ public class Settings return false; } } + if (renderGroupConcatMaxLenSessionVariable == null) { + if (other.renderGroupConcatMaxLenSessionVariable!= null) { + return false; + } + } else { + if (!renderGroupConcatMaxLenSessionVariable.equals(other.renderGroupConcatMaxLenSessionVariable)) { + return false; + } + } if (renderParenthesisAroundSetOperationQueries == null) { if (other.renderParenthesisAroundSetOperationQueries!= null) { return false; @@ -5200,6 +5248,7 @@ public class Settings result = ((prime*result)+((renderCoalesceToEmptyStringInConcat == null)? 0 :renderCoalesceToEmptyStringInConcat.hashCode())); result = ((prime*result)+((renderOrderByRownumberForEmulatedPagination == null)? 0 :renderOrderByRownumberForEmulatedPagination.hashCode())); result = ((prime*result)+((renderOutputForSQLServerReturningClause == null)? 0 :renderOutputForSQLServerReturningClause.hashCode())); + result = ((prime*result)+((renderGroupConcatMaxLenSessionVariable == null)? 0 :renderGroupConcatMaxLenSessionVariable.hashCode())); result = ((prime*result)+((renderParenthesisAroundSetOperationQueries == null)? 0 :renderParenthesisAroundSetOperationQueries.hashCode())); result = ((prime*result)+((namePathSeparator == null)? 0 :namePathSeparator.hashCode())); result = ((prime*result)+((bindOffsetDateTimeType == null)? 0 :bindOffsetDateTimeType.hashCode())); diff --git a/jOOQ/src/main/java/org/jooq/impl/AbstractContext.java b/jOOQ/src/main/java/org/jooq/impl/AbstractContext.java index dc20bbe1a6..83698accac 100644 --- a/jOOQ/src/main/java/org/jooq/impl/AbstractContext.java +++ b/jOOQ/src/main/java/org/jooq/impl/AbstractContext.java @@ -785,7 +785,12 @@ abstract class AbstractContext> extends AbstractScope imple @Override public final C skipUpdateCount() { - skipUpdateCounts++; + return skipUpdateCounts(1); + } + + @Override + public final C skipUpdateCounts(int skip) { + skipUpdateCounts += skip; return (C) this; } diff --git a/jOOQ/src/main/java/org/jooq/impl/DefaultRenderContext.java b/jOOQ/src/main/java/org/jooq/impl/DefaultRenderContext.java index e5ad943e95..ca4dcff6a5 100644 --- a/jOOQ/src/main/java/org/jooq/impl/DefaultRenderContext.java +++ b/jOOQ/src/main/java/org/jooq/impl/DefaultRenderContext.java @@ -47,6 +47,7 @@ import static org.jooq.impl.Identifiers.QUOTE_END_DELIMITER; import static org.jooq.impl.Identifiers.QUOTE_END_DELIMITER_ESCAPED; import static org.jooq.impl.Identifiers.QUOTE_START_DELIMITER; import static org.jooq.impl.Tools.BooleanDataKey.DATA_COUNT_BIND_VALUES; +import static org.jooq.impl.Tools.DataKey.DATA_APPEND_SQL; import static org.jooq.impl.Tools.DataKey.DATA_PREPEND_SQL; import java.util.ArrayDeque; @@ -373,12 +374,19 @@ class DefaultRenderContext extends AbstractContext implements Ren @Override public final String render() { String prepend = (String) data(DATA_PREPEND_SQL); + String append = (String) data(DATA_APPEND_SQL); + String result = sql.toString(); - return prepend == null + + return prepend == null && append == null ? result : format() - ? prepend + (prepend.endsWith(cachedNewline) ? "" : cachedNewline) + result - : prepend + (prepend.endsWith(" ") ? "" : " ") + result; + ? (prepend != null ? prepend + (prepend.endsWith(cachedNewline) ? "" : cachedNewline) : "") + + result + + (append != null ? ";" + (append.endsWith(cachedNewline) ? "" : cachedNewline) + append : "") + : (prepend != null ? prepend + (prepend.endsWith(" ") ? "" : " ") : "") + + result + + (append != null ? ";" + (append.endsWith(" ") ? "" : " ") + append : ""); } @Override diff --git a/jOOQ/src/main/java/org/jooq/impl/ListAgg.java b/jOOQ/src/main/java/org/jooq/impl/ListAgg.java index 66b27a2d30..f4c245c866 100644 --- a/jOOQ/src/main/java/org/jooq/impl/ListAgg.java +++ b/jOOQ/src/main/java/org/jooq/impl/ListAgg.java @@ -37,6 +37,7 @@ */ package org.jooq.impl; +import static java.lang.Boolean.FALSE; // ... // ... // ... @@ -57,23 +58,21 @@ import static org.jooq.SQLDialect.SQLITE; // ... // ... // ... +import static org.jooq.impl.DSL.query; import static org.jooq.impl.DSL.sql; -import static org.jooq.impl.Keywords.K_AS; -import static org.jooq.impl.Keywords.K_CONTENT; import static org.jooq.impl.Keywords.K_DISTINCT; import static org.jooq.impl.Keywords.K_SEPARATOR; -import static org.jooq.impl.Names.N_CONCAT; import static org.jooq.impl.Names.N_GROUP_CONCAT; import static org.jooq.impl.Names.N_LIST; import static org.jooq.impl.Names.N_LISTAGG; import static org.jooq.impl.Names.N_STRING_AGG; -import static org.jooq.impl.Names.N_SUBSTR; -import static org.jooq.impl.Names.N_XMLAGG; import static org.jooq.impl.Names.N_XMLSERIALIZE; import static org.jooq.impl.Names.N_XMLTEXT; import static org.jooq.impl.SQLDataType.VARCHAR; import static org.jooq.impl.SQLDataType.XML; +import static org.jooq.impl.Tools.appendSQL; import static org.jooq.impl.Tools.castIfNeeded; +import static org.jooq.impl.Tools.prependSQL; import java.util.Set; @@ -81,13 +80,12 @@ import org.jooq.Context; import org.jooq.Field; // ... import org.jooq.SQLDialect; -import org.jooq.XML; -// ... /** * @author Lukas Eder */ final class ListAgg extends DefaultAggregateFunction { + private static final Set SET_GROUP_CONCAT_MAX_LEN = SQLDialect.supportedBy(MARIADB, MYSQL); private static final Set SUPPORT_GROUP_CONCAT = SQLDialect.supportedBy(CUBRID, H2, HSQLDB, MARIADB, MYSQL, SQLITE); private static final Set SUPPORT_STRING_AGG = SQLDialect.supportedBy(POSTGRES); @@ -110,7 +108,16 @@ final class ListAgg extends DefaultAggregateFunction { @Override public final void accept(Context ctx) { if (SUPPORT_GROUP_CONCAT.contains(ctx.dialect())) { - acceptGroupConcat(ctx); + if (SET_GROUP_CONCAT_MAX_LEN.contains(ctx.dialect()) && !FALSE.equals(ctx.settings().isRenderGroupConcatMaxLenSessionVariable())) { + prependSQL(ctx.skipUpdateCounts(2), + query("{set} @t = @@group_concat_max_len"), + query("{set} @@group_concat_max_len = 4294967295") + ); + acceptGroupConcat(ctx); + appendSQL(ctx, query("{set} @@group_concat_max_len = @t")); + } + else + acceptGroupConcat(ctx); } else if (SUPPORT_STRING_AGG.contains(ctx.dialect())) { acceptStringAgg(ctx); diff --git a/jOOQ/src/main/java/org/jooq/impl/Tools.java b/jOOQ/src/main/java/org/jooq/impl/Tools.java index 908386587c..652f2ba667 100644 --- a/jOOQ/src/main/java/org/jooq/impl/Tools.java +++ b/jOOQ/src/main/java/org/jooq/impl/Tools.java @@ -586,9 +586,11 @@ final class Tools { DATA_COLLECTED_SEMI_ANTI_JOIN, /** - * [#5764] Sometimes, it is necessary to prepend some SQL to the generated SQL. + * [#5764] Sometimes, it is necessary to prepend some SQL to the + * generated SQL. *

- * This needs to be done e.g. to emulate inline table valued parameters in SQL Server: + * This needs to be done e.g. to emulate inline table valued parameters + * in SQL Server: *

* 

          * -- With TVP bind variable:
@@ -602,6 +604,23 @@ final class Tools {
          */
         DATA_PREPEND_SQL,
 
+        /**
+         * [#12092] Sometimes, it is necessary to append some SQL to the
+         * generated SQL.
+         * 

+         * This needs to be done e.g. to make sure the
+         * MySQL @@group_concat_max_len setting is set to an appropriate value,
+         * and reset to the previous value again.
+         * 

+         * 
+         * SET @t = @@group_concat_max_len;
+         * SET @@group_concat_max_len = 4294967295;
+         * SELECT group_concat(...);
+         * SET @@group_concat_max_len = @t;
+         * 

+         */
+        DATA_APPEND_SQL,
+
 
 
 
@@ -5126,8 +5145,16 @@ final class Tools {
         return VARCHAR(length).nullability(type.nullability()).defaultValue((Field) type.defaultValue());
     }
 
-    static > C prependSQL(C ctx, Query... queries) {
-        ctx.data().compute(DataKey.DATA_PREPEND_SQL, (k, v) -> {
+    static final > C prependSQL(C ctx, Query... queries) {
+        return preOrAppendSQL(DataKey.DATA_PREPEND_SQL, ctx, queries);
+    }
+
+    static final > C appendSQL(C ctx, Query... queries) {
+        return preOrAppendSQL(DataKey.DATA_APPEND_SQL, ctx, queries);
+    }
+
+    private static final > C preOrAppendSQL(DataKey key, C ctx, Query... queries) {
+        ctx.data().compute(key, (k, v) -> {
             String sql = ctx.dsl().renderInlined(ctx.dsl().queries(queries));
 
             if (v == null)
diff --git a/jOOQ/src/main/resources/xsd/jooq-runtime-3.15.0.xsd b/jOOQ/src/main/resources/xsd/jooq-runtime-3.15.0.xsd
index 6bbe0d6fb6..e365da33fe 100644
--- a/jOOQ/src/main/resources/xsd/jooq-runtime-3.15.0.xsd
+++ b/jOOQ/src/main/resources/xsd/jooq-runtime-3.15.0.xsd
@@ -188,6 +188,17 @@ be enabled as well.
 For details, see https://github.com/jOOQ/jOOQ/issues/4498.]]>
       
       
+      
+        GROUP_CONCAT function should be overflow-protected by setting the @@group_concat_max_len session variable in MySQL style database systems.
+

+MySQL truncates GROUP_CONCAT results after a certain length, which may be way
+too small for jOOQ's usage, especially when using the MULTISET emulation. By
+default, jOOQ sets a session variable to the highest possible value prior to executing a
+query containing GROUP_CONCAT. This flag can be used to opt out of this.
+

+For details, see https://github.com/jOOQ/jOOQ/issues/12092.]]>
+      
+