Skip to content

Conversation

@jonahgao
Copy link
Member

Which issue does this PR close?

N/A

Rationale for this change

What changes are included in this PR?

The tests for the GCD function exist in both scalar.slt and math.slt; it would be best to combine them.
This PR moves them all into math.slt.

Are these changes tested?

Yes

Are there any user-facing changes?

No

@github-actions github-actions bot added the sqllogictest SQL Logic Tests (.slt) label Jun 28, 2024
;

query II
select gcd(20, 1000), lcm(20, 1000);
Copy link
Member Author

Choose a reason for hiding this comment

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

This test has been split and moved.

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @jonahgao

query error DataFusion error: Arrow error: Compute error: Signed integer overflow in GCD\(0, \-9223372036854775808\)
select gcd(0, -9223372036854775808);

# Test gcd with zero and negative values
Copy link
Contributor

Choose a reason for hiding this comment

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

this was combined with other examples I think

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. gcd(0, 0) was duplicated with another test, and negative tests was moved too.

@alamb alamb merged commit 47db63f into apache:main Jun 28, 2024
@jonahgao jonahgao deleted the gcd_test branch June 28, 2024 15:32
comphead pushed a commit to comphead/arrow-datafusion that referenced this pull request Jul 2, 2024
* minor: consolidate gcd related tests

* fix comment
findepi pushed a commit to findepi/datafusion that referenced this pull request Jul 16, 2024
* minor: consolidate gcd related tests

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

Labels

sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants