Skip to content

Coerce boolean type for filter expression#18551

Merged
martint merged 1 commit intotrinodb:masterfrom
takezoe:coerce-type-for-filter-expression
Aug 9, 2023
Merged

Coerce boolean type for filter expression#18551
martint merged 1 commit intotrinodb:masterfrom
takezoe:coerce-type-for-filter-expression

Conversation

@takezoe
Copy link
Member

@takezoe takezoe commented Aug 5, 2023

Description

The following query fails with INTERNAL_ERROR:

select count(*) filter(where NULL) from system.runtime.queries;

Full stacktrace:

java.lang.IllegalArgumentException: Predicate must be an expression of boolean type: null
	at com.google.common.base.Preconditions.checkArgument(Preconditions.java:218)
	at io.trino.sql.planner.plan.FilterNode.<init>(FilterNode.java:48)
	at io.trino.sql.planner.optimizations.PredicatePushDown$Rewriter.visitTableScan(PredicatePushDown.java:1579)
	at io.trino.sql.planner.optimizations.PredicatePushDown$Rewriter.visitTableScan(PredicatePushDown.java:168)
	at io.trino.sql.planner.plan.TableScanNode.accept(TableScanNode.java:222)
	at io.trino.sql.planner.plan.SimplePlanRewriter$RewriteContext.rewrite(SimplePlanRewriter.java:81)
	at io.trino.sql.planner.plan.SimplePlanRewriter$RewriteContext.lambda$defaultRewrite$0(SimplePlanRewriter.java:72)
	at com.google.common.collect.ImmutableList.forEach(ImmutableList.java:422)
	at io.trino.sql.planner.plan.SimplePlanRewriter$RewriteContext.defaultRewrite(SimplePlanRewriter.java:72)
	at io.trino.sql.planner.optimizations.PredicatePushDown$Rewriter.visitProject(PredicatePushDown.java:309)
	at io.trino.sql.planner.optimizations.PredicatePushDown$Rewriter.visitProject(PredicatePushDown.java:168)
	at io.trino.sql.planner.plan.ProjectNode.accept(ProjectNode.java:81)
	at io.trino.sql.planner.plan.SimplePlanRewriter$RewriteContext.rewrite(SimplePlanRewriter.java:81)
	at io.trino.sql.planner.optimizations.PredicatePushDown$Rewriter.visitFilter(PredicatePushDown.java:414)
	at io.trino.sql.planner.optimizations.PredicatePushDown$Rewriter.visitFilter(PredicatePushDown.java:168)
	at io.trino.sql.planner.plan.FilterNode.accept(FilterNode.java:79)
	at io.trino.sql.planner.plan.SimplePlanRewriter$RewriteContext.rewrite(SimplePlanRewriter.java:81)
	at io.trino.sql.planner.plan.SimplePlanRewriter$RewriteContext.lambda$defaultRewrite$0(SimplePlanRewriter.java:72)
	at com.google.common.collect.ImmutableList.forEach(ImmutableList.java:422)
	at io.trino.sql.planner.plan.SimplePlanRewriter$RewriteContext.defaultRewrite(SimplePlanRewriter.java:72)
	at io.trino.sql.planner.optimizations.PredicatePushDown$Rewriter.visitPlan(PredicatePushDown.java:213)
	at io.trino.sql.planner.optimizations.PredicatePushDown$Rewriter.visitAggregation(PredicatePushDown.java:1459)
	at io.trino.sql.planner.optimizations.PredicatePushDown$Rewriter.visitAggregation(PredicatePushDown.java:168)
	at io.trino.sql.planner.plan.AggregationNode.accept(AggregationNode.java:221)
	at io.trino.sql.planner.plan.SimplePlanRewriter$RewriteContext.rewrite(SimplePlanRewriter.java:81)
	at io.trino.sql.planner.plan.SimplePlanRewriter$RewriteContext.lambda$defaultRewrite$0(SimplePlanRewriter.java:72)
	at com.google.common.collect.ImmutableList.forEach(ImmutableList.java:422)
	at io.trino.sql.planner.plan.SimplePlanRewriter$RewriteContext.defaultRewrite(SimplePlanRewriter.java:72)
	at io.trino.sql.planner.optimizations.PredicatePushDown$Rewriter.visitPlan(PredicatePushDown.java:213)
	at io.trino.sql.planner.optimizations.PredicatePushDown$Rewriter.visitPlan(PredicatePushDown.java:168)
	at io.trino.sql.planner.plan.PlanVisitor.visitOutput(PlanVisitor.java:49)
	at io.trino.sql.planner.plan.OutputNode.accept(OutputNode.java:82)
	at io.trino.sql.planner.plan.SimplePlanRewriter.rewriteWith(SimplePlanRewriter.java:31)
	at io.trino.sql.planner.optimizations.PredicatePushDown.optimize(PredicatePushDown.java:162)
	at io.trino.sql.planner.optimizations.StatsRecordingPlanOptimizer.optimize(StatsRecordingPlanOptimizer.java:56)
	at io.trino.sql.planner.LogicalPlanner.runOptimizer(LogicalPlanner.java:299)
	at io.trino.sql.planner.LogicalPlanner.plan(LogicalPlanner.java:269)
	at io.trino.sql.planner.LogicalPlanner.plan(LogicalPlanner.java:238)
	at io.trino.sql.planner.LogicalPlanner.plan(LogicalPlanner.java:233)
	at io.trino.execution.SqlQueryExecution.doPlanQuery(SqlQueryExecution.java:482)
	at io.trino.execution.SqlQueryExecution.planQuery(SqlQueryExecution.java:462)
	at io.trino.execution.SqlQueryExecution.start(SqlQueryExecution.java:400)
	at io.trino.execution.SqlQueryManager.createQuery(SqlQueryManager.java:256)
	at io.trino.dispatcher.LocalDispatchQuery.startExecution(LocalDispatchQuery.java:145)
	at io.trino.dispatcher.LocalDispatchQuery.lambda$waitForMinimumWorkers$2(LocalDispatchQuery.java:129)
	at io.airlift.concurrent.MoreFutures.lambda$addSuccessCallback$12(MoreFutures.java:568)
	at io.airlift.concurrent.MoreFutures$3.onSuccess(MoreFutures.java:543)
	at com.google.common.util.concurrent.Futures$CallbackListener.run(Futures.java:1133)
	at io.trino.$gen.Trino_dev____20230805_065421_2.run(Unknown Source)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
	at java.base/java.lang.Thread.run(Thread.java:833)

I made a related fix to reject non-boolean filter expression in #18460 recently but it seems better to coerce boolean type for filter expression there to cover this case.

Additional context and related issues

Release notes

(x) This is not user-visible or is docs only, and no release notes are required.

@cla-bot cla-bot bot added the cla-signed label Aug 5, 2023
@takezoe takezoe force-pushed the coerce-type-for-filter-expression branch from 5579026 to 7f6bc97 Compare August 5, 2023 07:43
@takezoe takezoe requested a review from martint August 8, 2023 15:45
@martint martint merged commit aee6bc8 into trinodb:master Aug 9, 2023
@github-actions github-actions bot added this to the 423 milestone Aug 9, 2023
@mosabua
Copy link
Member

mosabua commented Aug 10, 2023

This does not need a release note entry @martint ?

@martint
Copy link
Member

martint commented Aug 10, 2023

No, it just changes how an error surfaces to the user

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants