Skip to content

Add verification for has/hasnot to ensure input is a collection #317

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 1, 2025

Conversation

knassre-bodo
Copy link
Contributor

Resolves #316 . Ensures the argument is a collection, and provides a clean error message if it isn't.

… a collection, and checking it is a collection, as well as other operator tests [RUN CI]
Comment on lines -151 to +156
HAS = ExpressionFunctionOperator("HAS", True, AllowAny(), ConstantType(BooleanType()))
HAS = ExpressionFunctionOperator(
"HAS", True, RequireCollection(), ConstantType(BooleanType())
)
HASNOT = ExpressionFunctionOperator(
"HASNOT", True, AllowAny(), ConstantType(BooleanType())
"HASNOT", True, RequireCollection(), ConstantType(BooleanType())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These verifier objects are assigned to each operator, and provide much higher quality error messages when the operands have an issue. Right now, most operators have weak/nonexistent verifiers, but eventually we will add proper verifiers for everything.

Comment on lines +92 to +105
pytest.param(
TableCollectionInfo("Customers")
** WhereInfo(
[SubCollectionInfo("orders")],
FunctionInfo(
"HAS",
[
ChildReferenceExpressionInfo("order_date", 0),
],
),
),
"Invalid operator invocation 'HAS(orders.order_date)': Expected a collection as an argument, received an expression",
id="has_on_expression",
),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test is the the equivalent of Customers.WHERE(HAS(orders.order_date)), and shows the error message we would receive in this case. The XxxInfo classes are helpers that exist for creating QDAG nodes (one of the IR datastructures). The ** operator is used to chain these qdag nodes together when they are built (e.g. X ** WhereInfo(...) means build X, then build & return a WHERE clause with X as its predecessor)

Copy link
Contributor

@vineetg3 vineetg3 left a comment

Choose a reason for hiding this comment

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

LGTM!

@knassre-bodo knassre-bodo merged commit 716390a into main Apr 1, 2025
5 checks passed
@knassre-bodo knassre-bodo deleted the kian/has_verifier branch April 1, 2025 18:12
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.

Ensure HAS/HASNOT have a proper error message when the argument is not a collection
3 participants