-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
chore(placeholder): update error message and add tests #9073
chore(placeholder): update error message and add tests #9073
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @appletreeisyellow -- I think this code is an improvement in that it documents what the current behavior is.
However, I am not sure if this resolves #8819, because I am not sure what @simonvandel is trying to do.
I am trying to clarify here #8819 (comment)
datafusion/core/tests/sql/select.rs
Outdated
@@ -251,3 +251,63 @@ async fn test_parameter_invalid_types() -> Result<()> { | |||
); | |||
Ok(()) | |||
} | |||
|
|||
#[tokio::test] | |||
async fn test_parameter_empty_table_with_param_values() -> Result<()> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given these tests aren't SQL but use the DataFrame API they probably belong in https://github.com/apache/arrow-datafusion/tree/main/datafusion/core/tests/dataframe instead
chore: add positive test and remove println
7858e01
to
3ad62dc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @appletreeisyellow
Which issue does this PR close?
Relate to #8819
Rationale for this change
To help users use placeholder and parameter values
What changes are included in this PR?
No code change, only added tests
Are these changes tested?
Yes
Are there any user-facing changes?
No