-
Notifications
You must be signed in to change notification settings - Fork 991
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
Segfault in dcast because of type mismatch in fun.aggregate #2394
Comments
Latest master doesn't segfault:
It looks like this might be an improvement from |
Yep, seems to be that recent R itself errors more gracefully; 3.1.0 still segfaults
So, not sure if we should try and build a patch to this while we still depend on 3.1.0, or slap a |
No point in raising dep. If someone is affected and then can upgrade R. Otherwise we kill all non affected environments running older R. |
i.e. wontfix then? |
we could eventually improve error message to mention that, now it is not very useful |
I came across a related problem having to do with NA vs NA_real_. It doesn't generate a Segfault or other error, but it does put a garbage number where NA should appear. Nearly the same example as above, but with numeric value.var instead of character:
Note that if I set fill = NA, I get the result I expect, and that is my current workaround.
|
@bpolacco can you try running this on current master? i'm actually not getting any error |
Not sure how (it's a big commit) but this commit fixed this issue as far as I can tell; I'll add a test & file to close: |
@MichaelChirico Yes, latest master doesn't break for me. Sorry for not trying that before commenting. To my naive eyes, I'd say the changed if statement in file fcast.c line 27 (was 29) is the 'how' of the fix. Thanks!
|
I ran this test in v1.12.8 where it failed as described, but returns correct result in v1.13.0. So yes it does look like 4aadde8 in v1.13.0 fixed it. |
With the latest release and development data.table versions, the following example results in a segfault with no warning:
Careless use of
NA
instead ofNA_character_
in the aggregation function is my fault. But I hope such errors could be handled more gracefully.The text was updated successfully, but these errors were encountered: