Skip to content

Fix unsafe cast in RowExpressionFormatter#16550

Merged
shixuan-fan merged 1 commit intoprestodb:masterfrom
lightseba:master
Aug 3, 2021
Merged

Fix unsafe cast in RowExpressionFormatter#16550
shixuan-fan merged 1 commit intoprestodb:masterfrom
lightseba:master

Conversation

@lightseba
Copy link
Contributor

@lightseba lightseba commented Aug 2, 2021

Fixed the ClassCastException that occurs when the RowExpression containing a compiled LIKE predicate (RowExpressionInterpreter.tryHandleLike()) is printed with RowExpressionFormatter.

Test plan: unit tests

== RELEASE NOTES ==

General Changes
* Fixed a casting error that sometimes occurred for EXPLAIN queries when the query plan contains LIKE predicates

Copy link
Contributor

@kaikalur kaikalur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need a test that can reproduce the issue.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this belong inside the if? Looks like it's unrelated

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, those are created here.

@NikhilCollooru NikhilCollooru marked this pull request as ready for review August 2, 2021 19:08
@lightseba
Copy link
Contributor Author

We need a test that can reproduce the issue.

@kaikalur added tests that reproduce the issue

@lightseba lightseba requested a review from kaikalur August 2, 2021 19:20
Copy link
Contributor

@kaikalur kaikalur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rschlussel
Copy link
Contributor

Can you make sure your commit message follows the guidelines here: https://chris.beams.io/posts/git-commit/. Also, add a description to the commit message body about what was fixed (what previously failed and what error we would see).

Also, this should have a release note about what was fixed.

@lightseba
Copy link
Contributor Author

@rschlussel added more info in the commit message + added release note

@rschlussel
Copy link
Contributor

Thanks! can you also change the commit message title from "Fixed..." to "Fix..." as per the commit guidelines

The pattern argument of a LIKE CallExpression
is changed from a CallExpression describing the
pattern into a joni.Regex object when the RowExpression
is optimized by RowExpressionInterperter. When
this occured on a LIKE CallExpression in a query
plan before the plan was output with PlanPrinter,
the following cast error occured:

class com.facebook.presto.spi.relation.ConstantExpression cannot be cast to class com.facebook.presto.spi.relation.CallExpression

This occurred due to an unchecked cast in
RowExpressionFormatter. A type check was added before
the cast, and tests were added to RowExpressionFormatter
that reproduce the behaviour of printing an
optimized LIKE CallExpression.
Copy link
Contributor

@shixuan-fan shixuan-fan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also fix the PR title to be consistent with commit title?

@NikhilCollooru NikhilCollooru changed the title Fixed unsafe cast in RowExpressionFormatter Fix unsafe cast in RowExpressionFormatter Aug 3, 2021
@shixuan-fan shixuan-fan merged commit 334397c into prestodb:master Aug 3, 2021
@shixuan-fan
Copy link
Contributor

Merged since PR title could be fixed afterwards :)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants