Unit Test for Query Planner Regression

[jimsimon-wk]

Description

We noticed a regression in the query planner between versions 0.288 and 0.292 and this is a test to demonstrate that. The query passes the parser but throws an exception in the planner

QualifiedObjectName should have exactly 3 parts

Here's a truncated stacktrace

Caused by: com.facebook.presto.jdbc.internal.client.FailureInfo$FailureException: QualifiedObjectName should have exactly 3 parts
 at com.facebook.presto.common.QualifiedObjectName.valueOf(QualifiedObjectName.java:45)
 at com.facebook.presto.common.type.TypeSignatureBase.of(TypeSignatureBase.java:43)
 at com.facebook.presto.common.type.TypeSignature.<init>(TypeSignature.java:101)
 at com.facebook.presto.common.type.TypeSignature.parseTypeSignature(TypeSignature.java:209)
 at com.facebook.presto.common.type.TypeSignature.parseTypeSignature(TypeSignature.java:195)
 at com.facebook.presto.sql.analyzer.ExpressionTreeUtils.tryResolveEnumLiteralType(ExpressionTreeUtils.java:149)
 at com.facebook.presto.sql.analyzer.ExpressionAnalyzer$Visitor.visitDereferenceExpression(ExpressionAnalyzer.java:536)
 at com.facebook.presto.sql.analyzer.ExpressionAnalyzer$Visitor.visitDereferenceExpression(ExpressionAnalyzer.java:389)
 at com.facebook.presto.sql.tree.DereferenceExpression.accept(DereferenceExpression.java:54)
 at com.facebook.presto.sql.tree.StackableAstVisitor.process(StackableAstVisitor.java:26)
 at com.facebook.presto.sql.analyzer.ExpressionAnalyzer$Visitor.process(ExpressionAnalyzer.java:412)
 at com.facebook.presto.sql.analyzer.ExpressionAnalyzer$Visitor.getOperator(ExpressionAnalyzer.java:1603)
 at com.facebook.presto.sql.analyzer.ExpressionAnalyzer$Visitor.visitComparisonExpression(ExpressionAnalyzer.java:608)
 at com.facebook.presto.sql.analyzer.ExpressionAnalyzer$Visitor.visitComparisonExpression(ExpressionAnalyzer.java:389)
 at com.facebook.presto.sql.tree.ComparisonExpression.accept(ComparisonExpression.java:71)
 at com.facebook.presto.sql.tree.StackableAstVisitor.process(StackableAstVisitor.java:26)
 at com.facebook.presto.sql.analyzer.ExpressionAnalyzer$Visitor.process(ExpressionAnalyzer.java:412)
 at com.facebook.presto.sql.analyzer.ExpressionAnalyzer.analyze(ExpressionAnalyzer.java:350)
 at com.facebook.presto.sql.analyzer.ExpressionAnalyzer.analyzeExpression(ExpressionAnalyzer.java:1941)
 at com.facebook.presto.sql.planner.TranslateExpressionsUtil.toRowExpression(TranslateExpressionsUtil.java:57)
 at com.facebook.presto.sql.planner.QueryPlanner.rowExpression(QueryPlanner.java:1327)
 at com.facebook.presto.sql.planner.QueryPlanner.filter(QueryPlanner.java:478)
 at com.facebook.presto.sql.planner.QueryPlanner.plan(QueryPlanner.java:208)
 at com.facebook.presto.sql.planner.RelationPlanner.visitQuerySpecification(RelationPlanner.java:776)
 at com.facebook.presto.sql.planner.RelationPlanner.visitQuerySpecification(RelationPlanner.java:137)
 at com.facebook.presto.sql.tree.QuerySpecification.accept(QuerySpecification.java:138)
 at com.facebook.presto.sql.tree.AstVisitor.process(AstVisitor.java:27)
 at com.facebook.presto.sql.planner.RelationPlanner.process(RelationPlanner.java:173)
 at com.facebook.presto.sql.planner.QueryPlanner.planQueryBody(QueryPlanner.java:411)
 at com.facebook.presto.sql.planner.QueryPlanner.plan(QueryPlanner.java:188)
 at com.facebook.presto.sql.planner.RelationPlanner.visitQuery(RelationPlanner.java:769)
 at com.facebook.presto.sql.planner.RelationPlanner.visitQuery(RelationPlanner.java:137)
 at com.facebook.presto.sql.tree.Query.accept(Query.java:105)
 at com.facebook.presto.sql.tree.AstVisitor.process(AstVisitor.java:27)
 at com.facebook.presto.sql.planner.RelationPlanner.process(RelationPlanner.java:173)
 at com.facebook.presto.sql.planner.RelationPlanner.process(RelationPlanner.java:137)
 at com.facebook.presto.sql.tree.DefaultTraversalVisitor.visitSubqueryExpression(DefaultTraversalVisitor.java:320)
 at com.facebook.presto.sql.tree.SubqueryExpression.accept(SubqueryExpression.java:51)
 at com.facebook.presto.sql.tree.AstVisitor.process(AstVisitor.java:27)
 at com.facebook.presto.sql.planner.RelationPlanner.process(RelationPlanner.java:173)
 at com.facebook.presto.sql.planner.SubqueryPlanner.createPlanBuilder(SubqueryPlanner.java:506)
 at com.facebook.presto.sql.planner.SubqueryPlanner.appendInPredicateApplyNode(SubqueryPlanner.java:209)
 at com.facebook.presto.sql.planner.SubqueryPlanner.appendInPredicateApplyNodes(SubqueryPlanner.java:190)
 at com.facebook.presto.sql.planner.SubqueryPlanner.handleSubqueries(SubqueryPlanner.java:148)
 at com.facebook.presto.sql.planner.SubqueryPlanner.handleSubqueries(SubqueryPlanner.java:142)

Motivation and Context

Impact

Test Plan

Contributor checklist

• Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
• PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
• Documented new properties (with its default value), SQL syntax, functions, or other functionality.
• If release notes are required, they follow the release notes guidelines.
• Adequate tests were added if applicable.
• CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== NO RELEASE NOTE ==

[ZacBlanco]

@jimsimon-wk Thanks for raising this. I believe this was found earlier and patched recently in #24981

Can you check if applying this patch fixes the issue? If this is a big blocker for you, we can consider a 292.1 patch release, however 293 is around the corner and this fix will be included in the next release.

[jimsimon-wk]

Thanks for the quick response @ZacBlanco - I should have first thought to test against master before raising the issue. I can confirm that this test passes on master. Is there a timeline on 0.293?

[ZacBlanco]

No worries. One thing the fix PR is missing is a test which reproduced this issue, so I appreciate you coming up with this change. I'd be happy to merge this test case if you can fix the failing maven checks and release note action. Do you think you could come up with a slightly more descriptive method name for the test case too?

I don't think we have a hard date for the release, but I believe it will be around the end of the month.

[jimsimon-wk]

Yep, I can brush this up and get it ready to merge!

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: jimsimon-wk / name: Jim Simon (1250164)

[ZacBlanco]

Just one minor thing. Looks good otherwise

[ZacBlanco]

LGTM!

[ZacBlanco]

@jimsimon-wk Can you update the release note section in the PR description to just have == NO RELEASE NOTE ==

[hantangwangd]

Thank you @jimsimon-wk for adding this test, lgtm.