Skip to content
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

Case statements for the RANGE type. #415

Merged
merged 4 commits into from
May 21, 2024

Conversation

vlulla
Copy link
Contributor

@vlulla vlulla commented May 20, 2024

Slight modification for the new RANGE type.

Copy link

google-cla bot commented May 20, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@afleisc
Copy link
Collaborator

afleisc commented May 21, 2024

Hi @vlulla thanks for your submission !

For the CLA check - please visit https://cla.developers.google.com/. Once you've signed, follow the "New Contributors" link at the bottom of that page to update this check.

Do you mind also adding a few test cases in the test_cases file for the new type? It should follow the format for the other types and you can add yours below.

When you're finished, the bigquery-utils-push-to-pr check should succeed, and I can run the rest of the checks. Let me know if you need anything else

@danieldeleo
Copy link
Collaborator

Error thrown: Column 1 in UNION ALL has incompatible types: RANGE, RANGE, RANGE at [5:3].

You'll have to separate the test cases out so each RANGE type has its own generate_udf function

Comment on lines 349 to 363
generate_udf_test("typeof", [
{
inputs: [`RANGE<DATE> "[null, 2024-04-25)"`],
expected_output: `"RANGE<DATE>"`
},
{
inputs: [`RANGE<DATETIME> "[2021-01-01, 2024-04-25)"`],
expected_output: `"RANGE<DATETIME>"`
},
{
inputs: [`RANGE(current_timestamp() - interval 5 day, current_timestamp())`],
expected_output: `"RANGE<TIMESTAMP>"`
},

]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
generate_udf_test("typeof", [
{
inputs: [`RANGE<DATE> "[null, 2024-04-25)"`],
expected_output: `"RANGE<DATE>"`
},
{
inputs: [`RANGE<DATETIME> "[2021-01-01, 2024-04-25)"`],
expected_output: `"RANGE<DATETIME>"`
},
{
inputs: [`RANGE(current_timestamp() - interval 5 day, current_timestamp())`],
expected_output: `"RANGE<TIMESTAMP>"`
},
]);
generate_udf_test("typeof", [
{
inputs: [`RANGE<DATE> "[null, 2024-04-25)"`],
expected_output: `"RANGE<DATE>"`
},
]);
generate_udf_test("typeof", [
{
inputs: [`RANGE<DATETIME> "[2021-01-01, 2024-04-25)"`],
expected_output: `"RANGE<DATETIME>"`
},
]);
generate_udf_test("typeof", [
{
inputs: [`RANGE(current_timestamp() - interval 5 day, current_timestamp())`],
expected_output: `"RANGE<TIMESTAMP>"`
},
]);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! Thank you @danieldeleo! Let's see if this takes...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My tests pass, i think, but now getting another failure. In kruskal_wallis? What am i doing wrong?

@vlulla
Copy link
Contributor Author

vlulla commented May 21, 2024

I think my tests pass but the build step still fails. The error I get is bigquery error: Access Denied: BigQuery BigQuery: Error getting metadata for external code resource, please verify you have provided a valid path and/or that you have access to the resource: gs://bqutil-lib-test/bq_js_libs/jstat-v1.9.4.min.js How can i fix this?

@danieldeleo
Copy link
Collaborator

I'll take a look now

@danieldeleo
Copy link
Collaborator

/gcbrun

@afleisc afleisc self-requested a review May 21, 2024 19:33
@afleisc afleisc merged commit c1c515c into GoogleCloudPlatform:master May 21, 2024
45 checks passed
@afleisc afleisc self-assigned this May 29, 2024
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.

3 participants