[hibernate-dev] SQLServer getLimitString() refactoring

[Steve Ebersole]

So frustrating that we could do this so much cleaner if we were dealing 
with syntax trees instead of strings.  The dialect could just position 
the limit parameter nodes correctly in the tree and that would be that.

Minor, but I'd like to see something other than 
bindLimitParametersPreQuery and bindLimitParametersPostQuery.  The 
parameters are still part of the query so its not really before the 
query or after it.  bindLimitParametersAtStartOfQuery and 
bindLimitParametersAtEndOfQuery?

Also (and the old code did this as well) I notice a lot of calling to 
the dialect just to prepare the arguments that you pass in to 
bindLimitParametersPreQuery and bindLimitParametersPostQuery.  I'd like 
to see that cleaned up.  Mainly its the interpretation of 
org.hibernate.engine.spi.RowSelection.  For example, both 
interpretFirstRow and getMaxOrLimit are basically calls to the dialect. 
 Then your new code in Loader can go from

col += dialect.bindLimitParametersPostQuery(
        sql,
        st,
        col,
        interpretFirstRow( getFirstRow( selection ) ),
        getMaxOrLimit( selection, dialect )
);

to just:

col += dialect.bindLimitParametersPostQuery(
        sql,
        st,
        col,
        selection
);

Also, I wonder if it might be better to introduce a delegate for "limit 
support" into the Dialect.  I have been thinking of this approach for a 
few Dialect-related things.  Basically right now the Dialect is asked 
to do a few things in steps all specific to managing limits on the SQL 
query.  First it is asked to modify the actual query statement; then 
Loader does some other stuff; then it is asked to handle binding 
parameters representing to the limit values.  Based on the code I see 
for SQL Server there is some duplication in that last step to figure 
out what it did in the first step.

if ( useLimit ) {
	sql = dialect.getLimitString(
			sql.trim(), //use of trim() here is ugly?
			useLimitOffset ? getFirstRow(selection) : 0,
			getMaxOrLimit(selection, dialect)
		);
}

What if, instead, we did something like:

final LimitHandler limitHandler = useLimit
        ? dialect.buildLimitHandler( sql.trim(), selection )
        : NoopLimitHandler.INSTANCE;

sql = limitHandler.getProcessedSql();
...
col += limitHandler.bindLimitParametersAtStartOfQuery( st, col );
...
col += limitHandler.bindLimitParametersAtEndOfQuery( st, col );


On Wed 13 Jun 2012 01:58:30 PM CDT, Łukasz Antoniak wrote:
> Hello Guenther,
>
> I totally agree that user could adapt his query to use "AS" keyword, while we focus on keeping getLimitString() most fast and
> simple as possible.
>
> I think that my modifications are more or less ready and you can view them here:
> https://github.com/lukasz-antoniak/hibernate-core/compare/SQLServerLimitString
> TOP(100)PERCENT and ORDER BY issue fixed plus introduced limit parameters binding in Dialect class.
>
> Query "select id, description descr, (select max(id) from Product2) maximum from Product2" fails for me (even without paging):
> 19:50:29,046 ERROR ErrorCounter:54 - line 1:24: unexpected token: descr
> 19:50:29,046 ERROR ErrorCounter:50 - line 1:24: unexpected token: descr
> line 1:24: unexpected token: descr
> 	at org.hibernate.hql.internal.antlr.HqlBaseParser.identPrimary(HqlBaseParser.java:4016)
> 	at org.hibernate.hql.internal.antlr.HqlBaseParser.primaryExpression(HqlBaseParser.java:859)
> 	at org.hibernate.hql.internal.antlr.HqlBaseParser.atom(HqlBaseParser.java:3390)
> 	at org.hibernate.hql.internal.antlr.HqlBaseParser.unaryExpression(HqlBaseParser.java:3168)
> 	at org.hibernate.hql.internal.antlr.HqlBaseParser.multiplyExpression(HqlBaseParser.java:3040)
> 	at org.hibernate.hql.internal.antlr.HqlBaseParser.additiveExpression(HqlBaseParser.java:2750)
> 	at org.hibernate.hql.internal.antlr.HqlBaseParser.concatenation(HqlBaseParser.java:568)
> 	at org.hibernate.hql.internal.antlr.HqlBaseParser.relationalExpression(HqlBaseParser.java:2518)
> 	at org.hibernate.hql.internal.antlr.HqlBaseParser.equalityExpression(HqlBaseParser.java:2379)
> 	at org.hibernate.hql.internal.antlr.HqlBaseParser.negatedExpression(HqlBaseParser.java:2343)
> 	at org.hibernate.hql.internal.antlr.HqlBaseParser.logicalAndExpression(HqlBaseParser.java:2259)
> 	at org.hibernate.hql.internal.antlr.HqlBaseParser.logicalOrExpression(HqlBaseParser.java:2224)
> 	at org.hibernate.hql.internal.antlr.HqlBaseParser.expression(HqlBaseParser.java:2010)
> 	at org.hibernate.hql.internal.antlr.HqlBaseParser.aliasedExpression(HqlBaseParser.java:2177)
> 	at org.hibernate.hql.internal.antlr.HqlBaseParser.selectedPropertiesList(HqlBaseParser.java:1390)
> 	at org.hibernate.hql.internal.antlr.HqlBaseParser.selectClause(HqlBaseParser.java:1293)
> 	at org.hibernate.hql.internal.antlr.HqlBaseParser.selectFrom(HqlBaseParser.java:1030)
> 	at org.hibernate.hql.internal.antlr.HqlBaseParser.queryRule(HqlBaseParser.java:700)
> 	at org.hibernate.hql.internal.antlr.HqlBaseParser.selectStatement(HqlBaseParser.java:294)
> 	at org.hibernate.hql.internal.antlr.HqlBaseParser.statement(HqlBaseParser.java:157)
> 	at org.hibernate.hql.internal.ast.QueryTranslatorImpl.parse(QueryTranslatorImpl.java:266)
> 	at org.hibernate.hql.internal.ast.QueryTranslatorImpl.doCompile(QueryTranslatorImpl.java:180)
> 	at org.hibernate.hql.internal.ast.QueryTranslatorImpl.compile(QueryTranslatorImpl.java:136)
> 	at org.hibernate.engine.query.spi.HQLQueryPlan.<init>(HQLQueryPlan.java:105)
> 	at org.hibernate.engine.query.spi.HQLQueryPlan.<init>(HQLQueryPlan.java:80)
> 	at org.hibernate.engine.query.spi.QueryPlanCache.getHQLQueryPlan(QueryPlanCache.java:168)
> 	at org.hibernate.internal.AbstractSessionImpl.getHQLQueryPlan(AbstractSessionImpl.java:219)
> 	at org.hibernate.internal.AbstractSessionImpl.createQuery(AbstractSessionImpl.java:197)
> 	at org.hibernate.internal.SessionImpl.createQuery(SessionImpl.java:1736)
> ...
> 19:50:29,062  WARN HqlParser:259 - HHH000203: processEqualityExpression() : No expression to process!
> ...
>
> Best Regards,
> Lukasz Antoniak
> _______________________________________________
> hibernate-dev mailing list
> hibernate-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hibernate-dev

--
steve at hibernate.org
http://hibernate.org