github/jooq
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[jOOQ/jOOQ#15636] Wrong SQL generated when specifying explicit path

Browse Source 
joins in UPDATE .. FROM and DELETE .. USING
This commit is contained in:
Lukas Eder 2024-05-29 16:30:16 +02:00
parent 475a94fe26
commit 9d83a9730e
6 changed files with 81 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
jOOQ/src/main/java/org/jooq/impl/AbstractContext.java
View File

@ -49,6 +49,7 @@ import static org.jooq.JoinType.LEFT_OUTER_JOIN;
import static org.jooq.conf.InvocationOrder.REVERSE;
import static org.jooq.conf.ParamType.INDEXED;
import static org.jooq.impl.JoinTable.onKey0;
import static org.jooq.impl.TableFieldImpl.implicitJoinAsScalarSubquery;
import static org.jooq.impl.Tools.DATAKEY_RESET_IN_SUBQUERY_SCOPE;
import static org.jooq.impl.Tools.EMPTY_CLAUSE;
import static org.jooq.impl.Tools.EMPTY_QUERYPART;

2
jOOQ/src/main/java/org/jooq/impl/DefaultRenderContext.java
View File

@ -234,7 +234,7 @@ class DefaultRenderContext extends AbstractContext<RenderContext> implements Ren
List<TableImpl<?>> tables = new ArrayList<>();
// [#14985] Traverse paths only when referencing paths (!forceNew),
// not when declaring them in FROM (forceNew)
// not when declaring them in FROM or USING (forceNew)
if (!forceNew) {
while ((child = TableImpl.path(root)) != null) {

33
jOOQ/src/main/java/org/jooq/impl/DeleteQueryImpl.java
View File

@ -87,12 +87,10 @@ import static org.jooq.impl.ConditionProviderImpl.extractCondition;
import static org.jooq.impl.DSL.field;
import static org.jooq.impl.DSL.mergeInto;
import static org.jooq.impl.DSL.name;
import static org.jooq.impl.DSL.noCondition;
import static org.jooq.impl.DSL.row;
import static org.jooq.impl.DSL.select;
import static org.jooq.impl.DSL.systemName;
import static org.jooq.impl.DSL.trueCondition;
import static org.jooq.impl.DeleteQueryImpl.keyFieldsCondition;
import static org.jooq.impl.InlineDerivedTable.hasInlineDerivedTables;
import static org.jooq.impl.InlineDerivedTable.transformInlineDerivedTables;
import static org.jooq.impl.InlineDerivedTable.transformInlineDerivedTables0;
@ -104,6 +102,8 @@ import static org.jooq.impl.Keywords.K_ORDER_BY;
import static org.jooq.impl.Keywords.K_ROWS;
import static org.jooq.impl.Keywords.K_USING;
import static org.jooq.impl.Keywords.K_WHERE;
import static org.jooq.impl.SelectQueryImpl.addPathConditions;
import static org.jooq.impl.SelectQueryImpl.prependPathJoins;
import static org.jooq.impl.Tools.containsDeclaredTable;
import static org.jooq.impl.Tools.fieldName;
import static org.jooq.impl.Tools.map;
@ -112,13 +112,11 @@ import static org.jooq.impl.Tools.BooleanDataKey.DATA_UNQUALIFY_LOCAL_SCOPE;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Consumer;
import java.util.function.Function;
import org.jooq.Clause;
import org.jooq.Condition;
@ -138,7 +136,6 @@ import org.jooq.SQLDialect;
import org.jooq.Scope;
import org.jooq.SortField;
import org.jooq.Table;
import org.jooq.TableField;
import org.jooq.TableLike;
// ...
import org.jooq.conf.ParamType;
@ -407,7 +404,7 @@ implements
TableList u0 = u;
TableList u0 = traverseJoinsAndAddPathConditions(ctx, where0, u);
ctx.formatSeparator()
.visit(K_USING)
.sql(' ')
@ -474,6 +471,21 @@ implements
ctx.end(DELETE_RETURNING);
}
static final TableList traverseJoinsAndAddPathConditions(Context<?> ctx, ConditionProviderImpl where0, TableList u) {
// [#15636] Allow for explicit path joins in USING
traverseJoins(u, x -> {
if (x instanceof TableImpl)
ctx.scopeRegister(x, true);
});
// [#14985] [#15755] Add skipped join segments from path joins
u = prependPathJoins(ctx, where0, u);
addPathConditions(ctx, where0, u);
return u;
}
private final boolean limitEmulation(Context<?> ctx) {
if (limit != null) {
if (NO_SUPPORT_LIMIT.contains(ctx.dialect()))
@ -500,7 +512,14 @@ implements
SortFieldList orderBy,
Field<? extends Number> limit
) {
if (orderBy.isEmpty() && limit == null && tables.size() == 1 && tables.get(0) instanceof TableImpl) {
// [#15636] Cannot use the simplified emulation when there are join paths in the table list (USING or FROM clause)
if (orderBy.isEmpty()
&& limit == null
&& tables.size() == 1
&& tables.get(0) instanceof TableImpl
&& TableImpl.path(tables.get(0)) == null
) {
return new MergeUsing(tables.get(0), emptyMap());
}

47
jOOQ/src/main/java/org/jooq/impl/SelectQueryImpl.java
View File

@ -2896,7 +2896,7 @@ final class SelectQueryImpl<R extends Record> extends AbstractResultQuery<R> imp
private static final TableList prependPathJoins(Context<?> ctx, ConditionProviderImpl where, TableList tablelist) {
static final TableList prependPathJoins(Context<?> ctx, ConditionProviderImpl where, TableList tablelist) {
TableList result = new TableList(tablelist);
for (int i = 0; i < result.size(); i++) {
@ -4375,9 +4375,9 @@ final class SelectQueryImpl<R extends Record> extends AbstractResultQuery<R> imp
return getWhere(ctx, tablelist, new ConditionProviderImpl());
}
final ConditionProviderImpl getWhere(Context<?> ctx, TableList tablelist, ConditionProviderImpl result) {
final ConditionProviderImpl getWhere(Context<?> ctx, TableList tablelist, ConditionProviderImpl where0) {
if (condition.hasWhere())
result.addConditions(condition.getWhere());
where0.addConditions(condition.getWhere());
// Apply SEEK predicates in the WHERE clause only if:
// - There is an ORDER BY clause (SEEK is non-deterministic)
@ -4386,23 +4386,9 @@ final class SelectQueryImpl<R extends Record> extends AbstractResultQuery<R> imp
// and SEEK predicate is applied outside). See [#7459]
// [#15820] We're not grouping
if (!isGrouping() && !getOrderBy().isEmpty() && !getSeek().isEmpty() && unionOp.isEmpty())
result.addConditions(getSeekCondition(ctx));
where0.addConditions(getSeekCondition(ctx));
// [#13639] Add JOIN predicates generated by path expressions in FROM clause.
for (Table<?> t : tablelist)
addPathConditions(ctx, result, t);
traverseJoins(
tablelist, null, null,
t -> {
if (t instanceof CrossJoin j) {
addPathConditions(ctx, result, j.lhs);
addPathConditions(ctx, result, j.rhs);
}
return true;
},
null, null, null
);
addPathConditions(ctx, where0, tablelist);
@ -4411,9 +4397,28 @@ final class SelectQueryImpl<R extends Record> extends AbstractResultQuery<R> imp
if (NO_SUPPORT_LIMIT_ZERO.contains(ctx.dialect()) && limit.limitZero() && !isGrouping())
result.addConditions(falseCondition());
where0.addConditions(falseCondition());
return result;
return where0;
}
static final void addPathConditions(Context<?> ctx, ConditionProviderImpl where0, TableList tablelist) {
// [#13639] Add JOIN predicates generated by path expressions in FROM clause.
for (Table<?> t : tablelist)
addPathConditions(ctx, where0, t);
traverseJoins(
tablelist, null, null,
t -> {
if (t instanceof CrossJoin j) {
addPathConditions(ctx, where0, j.lhs);
addPathConditions(ctx, where0, j.rhs);
}
return true;
},
null, null, null
);
}
private static final void addPathConditions(Context<?> ctx, ConditionProviderImpl result, Table<?> t) {

12
jOOQ/src/main/java/org/jooq/impl/TableFieldImpl.java
View File

@ -206,24 +206,20 @@ implements
accept1(ctx);
}
private final boolean implicitJoinAsScalarSubquery(Context<?> ctx, TableImpl<?> t, Table<?> root) {
static final boolean implicitJoinAsScalarSubquery(Context<?> ctx, TableImpl<?> t, Table<?> root) {
return
// [#15755] Never apply the scalar subquery rendering when we're already rendering an implicit
// join tree. Otherwise, we'd get useless subqueries in the ON clauses.
!TRUE.equals(ctx.data(DATA_RENDER_IMPLICIT_JOIN))
&& !ctx.inScope(t)
&& (
// [#15754] Explicit scalar subqueries can be configured for implicit to-one paths
(t.childPath != null
&& ctx.settings().getRenderImplicitJoinType() == SCALAR_SUBQUERY
&& !ctx.inScope(t))
(t.childPath != null && ctx.settings().getRenderImplicitJoinType() == SCALAR_SUBQUERY)
// [#15755] The default is to render scalar subqueries for implicit to-many paths
|| (t.parentPath != null
&& (ctx.settings().getRenderImplicitJoinToManyType() == SCALAR_SUBQUERY)
&& !ctx.inScope(t))
|| (t.parentPath != null && (ctx.settings().getRenderImplicitJoinToManyType() == SCALAR_SUBQUERY))
// [#7508] Implicit join path references inside of DML queries have to
// be emulated differently

31
jOOQ/src/main/java/org/jooq/impl/UpdateQueryImpl.java
View File

@ -93,6 +93,7 @@ import static org.jooq.impl.DSL.selectFrom;
import static org.jooq.impl.DSL.trueCondition;
import static org.jooq.impl.DeleteQueryImpl.keyFieldsCondition;
import static org.jooq.impl.DeleteQueryImpl.mergeUsing;
import static org.jooq.impl.DeleteQueryImpl.traverseJoinsAndAddPathConditions;
import static org.jooq.impl.InlineDerivedTable.hasInlineDerivedTables;
import static org.jooq.impl.InlineDerivedTable.transformInlineDerivedTables;
import static org.jooq.impl.InlineDerivedTable.transformInlineDerivedTables0;
@ -103,9 +104,12 @@ import static org.jooq.impl.Keywords.K_SET;
import static org.jooq.impl.Keywords.K_UPDATE;
import static org.jooq.impl.Keywords.K_WHERE;
import static org.jooq.impl.SQLDataType.INTEGER;
import static org.jooq.impl.SelectQueryImpl.addPathConditions;
import static org.jooq.impl.SelectQueryImpl.prependPathJoins;
import static org.jooq.impl.Tools.anyMatch;
import static org.jooq.impl.Tools.containsDeclaredTable;
import static org.jooq.impl.Tools.findAny;
import static org.jooq.impl.Tools.traverseJoins;
import static org.jooq.impl.Tools.BooleanDataKey.DATA_UNQUALIFY_LOCAL_SCOPE;
import java.util.Arrays;
@ -749,6 +753,9 @@ implements
.declareTables(declareTables)
.end(UPDATE_UPDATE);
ConditionProviderImpl where0 = new ConditionProviderImpl();
TableList f = getFrom(ctx, where0);
@ -771,9 +778,8 @@ implements
acceptFrom(ctx);
acceptFrom(ctx, where0, f);
ConditionProviderImpl where0 = new ConditionProviderImpl();
if (limitEmulation(ctx)) {
// [#16632] Push down USING table list here
@ -839,12 +845,11 @@ implements
return false;
}
private final void acceptFrom(Context<?> ctx) {
ctx.start(UPDATE_FROM);
private final TableList getFrom(Context<?> ctx, ConditionProviderImpl where0) {
TableList f = new TableList();
// [#16634] Prevent unnecessary FROM clause in some dialects, e.g. HANA
if (!NO_SUPPORT_FROM.contains(ctx.dialect())) {
TableList f = new TableList();
@ -865,11 +870,21 @@ implements
if (!f.isEmpty())
ctx.formatSeparator()
.visit(K_FROM).sql(' ')
.declareTables(true, c -> c.visit(f));
f = traverseJoinsAndAddPathConditions(ctx, where0, f);
}
return f;
}
private final void acceptFrom(Context<?> ctx, ConditionProviderImpl where0, TableList f) {
ctx.start(UPDATE_FROM);
// [#16634] Prevent unnecessary FROM clause in some dialects, e.g. HANA
if (!f.isEmpty())
ctx.formatSeparator()
.visit(K_FROM).sql(' ')
.declareTables(true, c -> c.visit(f));
ctx.end(UPDATE_FROM);
}