-
Notifications
You must be signed in to change notification settings - Fork 3.6k
HHH-19396 cannot select the same column twice with different aliases while using cte #10158
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
base: main
Are you sure you want to change the base?
Conversation
5126bf6
to
8b3f32e
Compare
Previous change was always setting |
8b3f32e
to
38b7ede
Compare
@@ -491,7 +491,7 @@ private static CacheableSqmInterpretation buildCacheableSqmInterpretation( | |||
executionContext.getQueryParameterBindings(), | |||
executionContext.getSession().getLoadQueryInfluencers(), | |||
sessionFactory.getSqlTranslationEngine(), | |||
true | |||
sqm.getCteStatements().isEmpty() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't do this for the entire query, only because there are CTEs involved. IIUC, this only matters when rendering the CTE itself. We should rather change the BaseSqmToSqlAstConverter#deduplicateSelectionItems
to false
before visiting the CTEs themselves (e.g. in org.hibernate.query.sqm.sql.BaseSqmToSqlAstConverter#visitCteContainer
) like we do for subqueries.
String alias = aliases == null ? null : aliases.get( i ); | ||
if ( alias == null ) { | ||
alias = component.getAlias(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think you need this double-check: you're already getting the selectable node's alias in extractAliases()
.
protected static List<String> extractAliases(SqmSelectQuery<?> subQuery) { | ||
final SqmSelectClause selectClause = subQuery.getQueryPart() | ||
.getFirstQuerySpec() | ||
.getSelectClause(); | ||
final var aliases = new ArrayList<String>(); | ||
for (final var selection : selectClause.getSelections()) { | ||
final var alias = selection.getAlias(); | ||
selection.getSelectableNode().visitSubSelectableNodes( node -> | ||
aliases.add( alias == null ? node.getAlias() : alias ) | ||
); | ||
} | ||
return aliases; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm guessing we need this to get the SqmSelection#getAlias
value, instead of the SqmSelectableNode
's one. Edit: have you perhaps looked into why that's needed? Would be nice to add a clarification comment here for future reference.
We're already iterating through selections when calling getSelectionItems()
though, maybe it would be worth consolidating the creation of the lists within the extractSqmExpressibles
and createStatementTable
methods. Also, small nitpick, but I would either get both as arrays or both as lists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One SqmSelection
can contain more than one SqmSelectableNode
. I've tried that what you are saying in first "iteration", but that caused some twenty tests to fail. See, for example, org.hibernate.orm.test.jpa.criteria.subquery.SubqueryTests#testNoFromClauseInSubquery
.
I'll do some refactoring into new commit so, please, check what you (don't) like.
…wice in same (sub)query
…mporary tables and subqueries Selection items representing same column in (sub)query will be represented with single instance, this will hide real alias
Temporary deduplication disabling moved from org.hibernate.query.sqm.internal.ConcreteSqmSelectQueryPlan.buildCacheableSqmInterpretation into org.hibernate.query.sqm.sql.BaseSqmToSqlAstConverter.visitCteContainer
6a83ba0
to
9a986df
Compare
Jira issue HHH-19396
When creating
SqlAstQueryPartProcessingStateImpl
in methodorg.hibernate.query.sqm.sql.BaseSqmToSqlAstConverter#visitQuerySpec
parameterdeduplicateSelectionItems
must be set tofalse
(see comment in moved code block)When
selectStatement
contains same column more than once in methodorg.hibernate.query.sqm.tree.cte.SqmCteTable#createStatementTable
,SqmCteTable
constructor will be unable to properly resolve aliases. To prevent this alias in select clause selection should be used if exist, one from selectable node should be used otherwise.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.
https://hibernate.atlassian.net/browse/HHH-19396