Skip to content

Conversation

@rdblue
Copy link
Contributor

@rdblue rdblue commented Nov 23, 2022

This adds a simple expression parser based on pyparsing.

assert And(NotNull("x"), Or(LessThan("x", 5), GreaterThan("x", 10))) == parser.parse("x is not null and (x < 5 or 10 < x)")
assert Or(IsNull("x"), And(GreaterThanOrEqual("x", 5), Not(LessThan("x", 10)))) == parser.parse(
"(x is null) or (5 <= x) and not(x < 10)"
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I verified that the result of these complex cases matches Spark.

@rdblue rdblue requested a review from Fokko November 26, 2022 21:29
Copy link
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

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

Some cosmetic changes, but apart from that it looks great. I think this is a great addition to the library

return hash(self.value)

def __eq__(self, other: Any) -> bool:
if not isinstance(other, Literal):
Copy link
Contributor

Choose a reason for hiding this comment

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

If we get something else, we could wrap it in a literal and compare it. This would allow literal(1) == 1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think that we want to do that. Then equality would not be symmetric.

return GreaterThanOrEqual(result.column, result.literal)
if result.op in ("=", "=="):
return EqualTo(result.column, result.literal)
if result.op in ("!=", "<>"):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if result.op in ("!=", "<>"):
elif result.op in ("!=", "<>"):

@Fokko Fokko added this to the Python 0.2.0 release milestone Nov 28, 2022
rdblue and others added 4 commits November 28, 2022 12:20
Co-authored-by: Fokko Driesprong <fokko@apache.org>
Co-authored-by: Fokko Driesprong <fokko@apache.org>
Co-authored-by: Fokko Driesprong <fokko@apache.org>
@rdblue rdblue merged commit 51b5f08 into apache:master Nov 28, 2022
@rdblue
Copy link
Contributor Author

rdblue commented Nov 28, 2022

Merged. Thanks for reviewing, @Fokko!

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants