Pushdown LIMIT in cross product joins?

[markaddleman]

We have a query of form:

SELECT t1.a FROM t1, t2 LIMIT 10

t1 and t2 come from two different translators.  Both support the LIMIT clause but it appears that Teiid doesn't push the limit either execution.  I think it's safe in this case to produce a plan like this:

LIMIT 10

CROSS JOIN

Access: SELECT t1.a FROM t1 LIMIT 10

Access: SELECT 1 FROM t2 LIMIT 10

Note that the ultimate projected column list does not include anything from but I'm not sure it really matters.

What do you think?

Attached is the actual query and plan.

[shawkins]

Yes that is an enhancement we would perform.  Can you log an issue?

Steve

[markaddleman]

TEIID-2370 created.  This is actually a blocker for us.  Is there any chance of getting a patch quickly?

Thanks!

[shawkins]

Specifically for cross join the change should be pretty narrow.  You could take a stab at in RulePushLimit.  A handler would be needed in canPushLimit similar to SET_OP logic.  Otherwise it would probably just be in the bucket for 8.3 Beta2.

Steve

[markaddleman]

Made the change and it appears to work.  I'm attaching the new RulePushLimit here for your review.

Making the change wasn't nearly as scary as I thought.

[markaddleman]

Don't know how to attach another file.  Relevant change to canPushLimit below:

            if (child.getProperty(Info.JOIN_TYPE).equals(org.teiid.query.sql.lang.JoinType.JOIN_CROSS))
            {
                final List grandChildren = new LinkedList(child.getChildren());
                for (final PlanNode grandChild : grandChildren)
                {
                    final PlanNode newLimit = NodeFactory.getNewNode(NodeConstants.Types.TUPLE_LIMIT);
                    newLimit.setProperty(NodeConstants.Info.MAX_TUPLE_LIMIT,
                                         op(SourceSystemFunctions.ADD_OP,
                                            parentLimit,
                                            parentOffset,
                                            metadata.getFunctionLibrary()));
                    grandChild.addAsParent(newLimit);
                    limitNodes.add(newLimit);
                }
            }
            return false;

[shawkins]

Yes, that code will do exactly what you want.  I'll refine this out to a few more cases and commit it.

Thanks for the optimizer enhancement,

Steve

> Making the change wasn't nearly as scary as I thought.

Just knowning where to look is a big part of making a change.  And I'm more than happy to get you or another dev headed in the right direction.  We did at one point envision allowing pluggable rules (I believe the Farrago project does something similar) - however that opens up even more apis and is a support nightmare...

[markaddleman]

As indicated in the email, we're getting errors in some of the unit tests.  I suspect that those are from an earlier, badly applied patch, rather than from this optimization but I'd appreciate it when you can confirm from your side. 

> Just knowning where to look is a big part of making a change.  And I'm more than happy to get you or another dev headed in the right direction.

We definitely want to participate more in Teiid development.  As we ask for features that you feel can be taken on, please give us a push in the right direction.

> We did at one point envision allowing pluggable rules (I believe the Farrago project does something similar) - however that opens up even more apis and is a support nightmare...

Yeah, it seems like there are finite number of correct optimization rules for SQL.  I'd rather have those come from an authoritative source and vetted by the community than try to role my own.

[shawkins]

> As indicated in the email, we're getting errors in some of the unit tests.  I suspect that those are from an earlier, badly applied patch, rather than from this optimization but I'd appreciate it when you can confirm from your side.

No those are actual failures.  I had first refined things to pass for the initial commit of https://issues.jboss.org/browse/TEIID-2370, but then realised the issue was more general and also addressed https://issues.jboss.org/browse/TEIID-2373

> I'd rather have those come from an authoritative source and vetted by the community than try to role my own.

We definitely want to collect everything that is generally applicable in the mainline.

Steve