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

Update parser/deparser weirdness with "function" (postgres context, but expected everywhere) #94

Open
christopheleroy opened this issue Oct 25, 2018 · 3 comments

Comments

@christopheleroy
Copy link

The below simple request fails its Parsing and can never be execute:

update shipment_request set is_retired=1, retired_ts = now(), retired_uid = ?v WHERE reqid = ?n and is_retired=0

It fails with a NullPointerException, on 8.6.1 it is at :
"at com.novartis.opensource.yada.Parser.visit(Parser.java:275)", "at net.sf.jsqlparser.statement.update.Update.accept(Update.java:60)", "at com.novartis.opensource.yada.Parser.deparse(Parser.java:467)", "at com.novartis.opensource.yada.Parser.parseDeparse(Parser.java:393)",

The reason seems to be that the function now() has no parameter. The Parser.java code expects an list of parameters, but somewhat the empty list is coming as Null (a little before line 275?)

Please check for Nulls.

This query works with:
update shipment_request set is_retired=1, retired_uid = ?v WHERE reqid = ?n and is_retired=0

but that is not really valid.

An even better version of the query is:

update shipment_request set 
 is_retired=1, 
 retired_uid = ?v, 
 retired_ts = case is_retired when 0 then now() else retired_ts end
WHERE reqid = ?n and is_retired=0

and since the parser seems to be less precise on the parsing of the case statement, it suddenly works again.

lol though.

@varontron
Copy link
Collaborator

varontron commented Oct 27, 2018

Can you determine if this Is this a limitation of JSQLParser? From your description, I'm guessing, regrettably, it isn't.

@christopheleroy
Copy link
Author

Hello - I think JSQLParser may be may be parsing correctly, however, Yada Parser may recognize now() as a function and perhaps JSQLParser is providing the function.getParameterList() as a null instead of an empty list.

but here is a new case:

I get the same nullpointer exception with:

insert into person(user521, fullname, email)
select x.user521, x.fullname, x.email
from (select ?v as user521, ?v as fullname, ?v as email ) as x
where x.user521 not in (select user521 from person)

alternative form below, gets the same exception:

insert into person(user521, fullname, email)
select x.user521, x.fullname, x.email
from (values(?v, ?v, ?v) as x(user521, fullname, email)
where x.user521 not in (select user521 from person)

@christopheleroy
Copy link
Author

christopheleroy commented Dec 5, 2018

Well - what do you know. .. !
I think I've spent 5 hours going around this NullPointerException, for no good reason.
I think the queries I've pasted are were not failing, it is the next query - it fails before mentioning it in the logs probably.

Now I have the 2nd query in the JP batch that fails when it has been working fine all along. So in fact, the failure is on the 3rd query -- which must be:

UPDATE STUDY_PERSON_ROLE
set is_retired=1, retired_uid = ?v, retired_ts = now()
WHERE is_retired=0 and studyId = ?v and userId = ?v

The unreliability of the parser combined the opacity of this kind of error probably made me lost 3 days in the last 3 weeks --
that would not happen if I was always writing new queries, but I take long pauses ...
this is getting hard and unwieldy... My SQL is perhaps beyond the average simple UI stuff ..

I bet this version of the query will work like a charm:

UPDATE STUDY_PERSON_ROLE
set is_retired=1, retired_uid = ?v, retired_ts = CASE is_retired when 0 then now() else retired_ts END
WHERE is_retired=0 and studyId = ?v and userId = ?v

Bingo - it parse alright. now, after finagling for 5 hours on this, I forgot the retired_uid in my payload.
It would be another 30 minutes chasing the missing column unless I had implemented a local fix for issue #99.
(the local fix is throws a RuntimeException with the actual message "could not find column retired_uid in payload" - but this is dirty fix, as it should be its own Yada exception somehow?)
without this fix, I'd get another super cryptic NullPointerException or worse.

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

No branches or pull requests

2 participants