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

test(nodejs): add test_all_types.test.ts #7740

Merged
merged 6 commits into from
Jun 15, 2023

Conversation

Mause
Copy link
Member

@Mause Mause commented May 30, 2023

This is mostly a port from the python test_all_types, with the addition of switching to using the BigInt class for the larger integer types. Worth discussing whether we make this breaking change, or put it behind a setting

tools/nodejs/src/statement.cpp Outdated Show resolved Hide resolved
const all_types = await get_all_types();

for (const cur_type of all_types) {
// FIXME: these currently have too high a precision to be tested
Copy link
Member Author

Choose a reason for hiding this comment

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

Node doesn't have a builtin Decimal class, so while these are returned, the results are way off. Not sure if we should write a simple wrapper around the BigInt class and use that, return them as strings, or just live with the inaccuracy

@Mause Mause force-pushed the feature/nodejs-test-all-types branch from 49f446a to e60cda3 Compare June 1, 2023 10:41
@Mause Mause requested a review from Tishj June 6, 2023 06:18
@Mytherin Mytherin merged commit 4f8ae89 into duckdb:feature Jun 15, 2023
@Mytherin
Copy link
Collaborator

Thanks!

@Mause Mause deleted the feature/nodejs-test-all-types branch July 5, 2023 14:09
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