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

Fix string passing at interface C++ to JS #1905

Merged
merged 4 commits into from
Oct 30, 2024

Conversation

carlopi
Copy link
Collaborator

@carlopi carlopi commented Oct 30, 2024

Thanks to @jraymakers that identified the problem.

There are still a few places where we pass table names or DB names, those ought to be shorter, but I will address and remove those in a separate PR, for now keeping old and new interface in, but using only the new one where we pass only malloc-ed memory around.

@carlopi carlopi force-pushed the fix_strings_at_interface branch 3 times, most recently from 8d15ff6 to dda7bde Compare October 30, 2024 13:08
@carlopi carlopi force-pushed the fix_strings_at_interface branch from dda7bde to feb1aba Compare October 30, 2024 13:09
@jraymakers
Copy link
Contributor

Should createPrepared get the same treatment? It also takes a query string.

@carlopi
Copy link
Collaborator Author

carlopi commented Oct 30, 2024

Should createPrepared get the same treatment? It also takes a query string.

Yes, thanks. I will address those in a separate PR, I think this still makes sense as-is.

@jraymakers
Copy link
Contributor

Should createPrepared get the same treatment? It also takes a query string.

Yes, thanks. I will address those in a separate PR, I think this still makes sense as-is.

Separate PR is fine. But note that createPrepared is also an important call for us.

@jraymakers
Copy link
Contributor

Less important for us, but also needing attention, is getTableNames.

@jraymakers
Copy link
Contributor

This is minor, but a couple of the C functions have unused "script" arguments:

  • duckdb_web_pending_query_poll
  • duckdb_web_pending_query_cancel

@carlopi carlopi merged commit 932a6cb into duckdb:main Oct 30, 2024
15 checks passed
@carlopi carlopi deleted the fix_strings_at_interface branch October 30, 2024 16:39
@carlopi
Copy link
Collaborator Author

carlopi commented Oct 30, 2024

Follow up is here: #1906

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