Skip to content

ORC: ExpressionToSearchArgument should only convert reference term#8244

Merged
rdblue merged 1 commit into
apache:masterfrom
ConeyLiu:orc-transform-filter
Aug 10, 2023
Merged

ORC: ExpressionToSearchArgument should only convert reference term#8244
rdblue merged 1 commit into
apache:masterfrom
ConeyLiu:orc-transform-filter

Conversation

@ConeyLiu

@ConeyLiu ConeyLiu commented Aug 7, 2023

Copy link
Copy Markdown
Contributor

ExpressionToSearchArgument should only convert those reference terms. Otherwise, we will get incorrect conversions or exceptions. For example, converting equal(year("ts"), 10):

java.lang.ClassCastException: java.lang.Integer cannot be cast to java.lang.Long

	at org.apache.iceberg.orc.ExpressionToSearchArgument.literal(ExpressionToSearchArgument.java:329)
	at org.apache.iceberg.orc.ExpressionToSearchArgument.lambda$eq$13(ExpressionToSearchArgument.java:209)
	at org.apache.iceberg.orc.ExpressionToSearchArgument.convert(ExpressionToSearchArgument.java:52)
	at org.apache.iceberg.orc.TestExpressionToSearchArgument.testExpressionContainsNonReferenceTerm(TestExpressionToSearchArgument.java:492)

@github-actions github-actions Bot added the ORC label Aug 7, 2023
@ConeyLiu ConeyLiu changed the title ORC: ExpressionToSearchArgument only convert reference term ORC: ExpressionToSearchArgument should only convert reference term Aug 7, 2023
@ConeyLiu ConeyLiu mentioned this pull request Aug 7, 2023
Merged
@rdblue

rdblue commented Aug 7, 2023

Copy link
Copy Markdown
Contributor

@dongjoon-hyun, could you also take a look at this to make sure it is correct?

dongjoon-hyun
dongjoon-hyun approved these changes Aug 7, 2023
View reviewed changes

@dongjoon-hyun dongjoon-hyun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This PR looks correct to me too.

@rdblue rdblue merged commit fe522c5 into apache:master Aug 10, 2023
@rdblue

rdblue commented Aug 10, 2023

Copy link
Copy Markdown
Contributor

Thanks, @ConeyLiu! And thanks to @dongjoon-hyun for reviewing.

@ConeyLiu

ConeyLiu commented Aug 11, 2023

Copy link
Copy Markdown
Contributor Author

Thanks @rdblue @dongjoon-hyun

@ConeyLiu ConeyLiu deleted the orc-transform-filter branch August 11, 2023 03:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rdblue rdblue rdblue approved these changes
+1 more reviewer
@dongjoon-hyun dongjoon-hyun dongjoon-hyun approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

ORC

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ConeyLiu @rdblue @dongjoon-hyun