Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unnecessary stack elements when SelectQueryImpl doesn't have both semi join and where clause predicates #10548

Closed
lukaseder opened this issue Aug 27, 2020 · 2 comments

Comments

@lukaseder
Copy link
Member

I've noticed some repeated occurrence of ConditionProviderImpl on the stack trace of a query execution, see indented code:

RowSubqueryCondition.accept(Context<?>) line: 135	
DefaultRenderContext.visit0(QueryPartInternal) line: 609	
DefaultRenderContext(AbstractContext<C>).visit(QueryPart) line: 264	
CompareCondition.accept(Context<?>) line: 127	
DefaultRenderContext.visit0(QueryPartInternal) line: 609	
DefaultRenderContext(AbstractContext<C>).visit(QueryPart) line: 264	
CombinedCondition.accept(Context<?>) line: 184	
DefaultRenderContext.visit0(QueryPartInternal) line: 609	
DefaultRenderContext(AbstractContext<C>).visit(QueryPart) line: 264	
    ConditionProviderImpl.accept(Context<?>) line: 124	
    DefaultRenderContext.visit0(QueryPartInternal) line: 609	
    DefaultRenderContext(AbstractContext<C>).visit(QueryPart) line: 264	
    ConditionProviderImpl.accept(Context<?>) line: 124
    DefaultRenderContext.visit0(QueryPartInternal) line: 609	
    DefaultRenderContext(AbstractContext<C>).visit(QueryPart) line: 264	
SelectQueryImpl<R>.toSQLReference0(Context<?>, Field<?>[], Field<?>[]) line: 1730	
SelectQueryImpl<R>.toSQLReferenceLimitDefault(Context<?>, Field<?>[], Field<?>[]) line: 1249	
SelectQueryImpl<R>.accept0(Context<?>) line: 1141	
SelectQueryImpl<R>.accept(Context<?>) line: 916	
DefaultRenderContext.visit0(QueryPartInternal) line: 609	
DefaultRenderContext(AbstractContext<C>).visit(QueryPart) line: 264	
SelectQueryImpl<R>(AbstractQuery).getSQL0(ExecuteContext) line: 524	
SelectQueryImpl<R>(AbstractQuery).execute() line: 326	
SelectQueryImpl<R>(AbstractResultQuery<R>).fetchLazy(int) line: 481	
SelectQueryImpl<R>(AbstractResultQuery<R>).fetchLazy() line: 450	
SelectQueryImpl<R>(AbstractResultQuery<R>).fetchLazyNonAutoClosing() line: 464	
SelectQueryImpl<R>(AbstractResultQuery<R>).fetchOne() line: 641	
SelectImpl<R,T1,T2,T3,T4,T5,T6,T7,T8,T9,T10,T11,T12,T13,T14,T15,T16,T17,T18,T19,T20,T21,T22>.fetchOne() line: 3074	

This is because of the following logic in SelectQueryImpl:

ConditionProviderImpl actual = new ConditionProviderImpl();

if (semiAntiJoinPredicates != null)
    actual.addConditions(semiAntiJoinPredicates);

if (where.hasWhere())
    actual.addConditions(where);

context.formatSeparator()
       .visit(K_WHERE)
       .sql(' ')
       .visit(actual);

We don't have to wrap the where clause in another ConditionProviderImpl, unless we also have a semi join predicate. Not a huge improvement, but still, almost all queries are affected.

@lukaseder
Copy link
Member Author

The fix is simply:

if (where.hasWhere())
    actual.addConditions(where.getWhere());

lukaseder added a commit that referenced this issue Aug 27, 2020
@lukaseder
Copy link
Member Author

Fixed in jOOQ 3.14.0. Backport scheduled for 3.13.5 (#10549)

3.14 Other improvements automation moved this from To do to Done Aug 27, 2020
lukaseder added a commit that referenced this issue Aug 27, 2020
doesn't have both semi join and where clause predicates
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests

1 participant