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 outputs from UDF #250

Merged
merged 2 commits into from
Oct 20, 2021
Merged

fix string outputs from UDF #250

merged 2 commits into from
Oct 20, 2021

Conversation

aplavin
Copy link
Contributor

@aplavin aplavin commented Jul 31, 2021

Not sure why "+1" was there, but it lead to an extra nul byte at the end.

@ImreSamu
Copy link

Maybe related issue: #147 ?

@aplavin
Copy link
Contributor Author

aplavin commented Jul 31, 2021

Yes, I believe that issue is exactly what I fix here. The +1 lead to trailing 00 as seen in your hex outputs.

@codecov
Copy link

codecov bot commented Aug 1, 2021

Codecov Report

Merging #250 (957403f) into master (b4d80a9) will increase coverage by 0.55%.
The diff coverage is n/a.

❗ Current head 957403f differs from pull request most recent head 00fc933. Consider uploading reports for the commit 00fc933 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #250      +/-   ##
==========================================
+ Coverage   83.19%   83.74%   +0.55%     
==========================================
  Files           5        5              
  Lines         607      603       -4     
==========================================
  Hits          505      505              
+ Misses        102       98       -4     
Impacted Files Coverage Δ
src/api.jl 41.66% <ø> (+1.17%) ⬆️
src/consts.jl 85.71% <0.00%> (-3.18%) ⬇️
src/SQLite.jl 96.62% <0.00%> (+0.40%) ⬆️
src/UDF.jl 85.71% <0.00%> (+0.79%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b4d80a9...00fc933. Read the comment docs.

Copy link
Member

@quinnj quinnj left a comment

Choose a reason for hiding this comment

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

Thanks for the fix! Looks like the same only issue as the other PR.

@quinnj quinnj closed this Aug 1, 2021
@quinnj quinnj reopened this Aug 1, 2021
@quinnj
Copy link
Member

quinnj commented Aug 1, 2021

Ah shoot, looks like only wasn't introduced until Julia 1.4, so we still have the error.

@quinnj quinnj merged commit 25f3702 into JuliaDatabases:master Oct 20, 2021
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