-
Notifications
You must be signed in to change notification settings - Fork 394
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
[add]: Added try/catches to primitive parsing methods #127
base: develop
Are you sure you want to change the base?
Conversation
I am looking at the patch and I think it's a good idea, but the main concern I have is that in case of an error it returns a default value of 0, I think it should be NULL. |
I absolutely agree with you and that would be my initial intention, however these are primitives, not objects and returning null is not possible... |
Yeah, I am looking at the code and it's a cascade of changes... to make it an option I have to change the objectinspector caching, plus all the objectinspectors... |
I will try to do it myself, time permitting. |
I am actually working on it too :) |
Oh cool, thank you :) |
So I spent a good amount of time looking at this and I think it can't be easily done. |
Handle invalid primitives in json:
Problem we experience quite often:
Looking at the code and exception stack trace,
and relevant codes
MapOperator.java:process
my understanding is that currently there is no way to catch these exceptions via configuration options (e.g. mapreduce.job.skiprecords).
The only thing that seems viable is to catch these kind of problems using try/catch within methods of ParsePrimitiveUtils.
I understand this is probably the ugliest solution and a better pattern might be to pre-validate json, but unfortunately this doesn't seem to be a feasible situation in our case.
I will be pleased if you merge this PR, however I understand that this is not the ideal situation and you might not accept it.
In any case, I am open to your suggestions regarding this.