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

PLP/PLE ERROR: org.postgresql.util.PSQLException: ERROR: syntax error at or near "@" #1755

Closed
KSadomtsev opened this issue Jan 22, 2021 · 7 comments · Fixed by #1778
Closed
Assignees
Labels
Milestone

Comments

@KSadomtsev
Copy link

WebApi 2.8.1

Steps:
1)Run any of PLP/PLE analysis on postgresql

Actual:
It should be replaced @results_database_schema by real schema, but it doesn't

Error:
org.postgresql.util.PSQLException: ERROR: syntax error at or near "@"
Position: 13

SQL:
delete from @results_database_schema.cohort_censor_stats where cohort_definition_id = 157

@KSadomtsev KSadomtsev added the bug label Jan 22, 2021
@KSadomtsev KSadomtsev added this to the V2.8.1 milestone Jan 22, 2021
ssuvorov-fls added a commit to odysseusinc/SkeletonComparativeEffectStudy that referenced this issue Jan 26, 2021
added results database schema parameter to pass to sql render
ssuvorov-fls added a commit to odysseusinc/SkeletonPredictionStudy that referenced this issue Jan 26, 2021
added results database schema parameter to pass to sql render
@anthonysena
Copy link
Collaborator

@ssuvorov-fls - it would appear that the cohort SQL is adding some extra code to compute inclusion stats which is not required for PLE/PLP. Could you share the design json that exhibited this problem so I can take a look? I'd like to run this through Hydra and inspect the resulting package to see where things may be off.

@ssuvorov-fls
Copy link
Contributor

ssuvorov-fls commented Jan 26, 2021

@anthonysena
estimation.json.zip
Other analyses also throws this exception

@anthonysena
Copy link
Collaborator

@ssuvorov-fls - I'm having a bit of trouble reproducing this issue. To test this, I use the instructions from @schuemie posted on https://ohdsi.github.io/Hydra/articles/HydratingPackages.html.

First challenge: the JSON file lacks a "packageName" attribute in the JSON that breaks our ability to use this approach. So, that is something we need to fix (I'll note that in a separate issue). Once I manually added that element to the estimation json, I was able to hydrate a new package without issue:

hydraOutput.zip

I inspected the ./inst/sql/sql_server/Dehydration.sql to see where it either contained a reference to @results_database_schema OR cohort_censor_stats. I did not find any references to cohort_censor_stats in these scripts. I did find references to @results_database_schema but these references should disappear when running through SqlRender based on the {0 != 0}?{...} conditional block (line 180 from that code block - see below). So, I'm not sure of the origin of this issue as it does not appear that Hydra is producing anything incorrectly or even where the original code of delete from @results_database_schema.cohort_censor_stats where cohort_definition_id = 157 would appear in the code as I found 0 references to cohort_censor_stats in the resulting package.

You are using Hydra for this, correct?

{0 != 0}?{
-- Find the event that is the 'best match' per person.  
-- the 'best match' is defined as the event that satisfies the most inclusion rules.
-- ties are solved by choosing the event that matches the earliest inclusion rule, and then earliest.

select q.person_id, q.event_id
into #best_events
from #qualified_events Q
join (
	SELECT R.person_id, R.event_id, ROW_NUMBER() OVER (PARTITION BY R.person_id ORDER BY R.rule_count DESC,R.min_rule_id ASC, R.start_date ASC) AS rank_value
	FROM (
		SELECT Q.person_id, Q.event_id, COALESCE(COUNT(DISTINCT I.inclusion_rule_id), 0) AS rule_count, COALESCE(MIN(I.inclusion_rule_id), 0) AS min_rule_id, Q.start_date
		FROM #qualified_events Q
		LEFT JOIN #inclusion_events I ON q.person_id = i.person_id AND q.event_id = i.event_id
		GROUP BY Q.person_id, Q.event_id, Q.start_date
	) R
) ranked on Q.person_id = ranked.person_id and Q.event_id = ranked.event_id
WHERE ranked.rank_value = 1
;

-- modes of generation: (the same tables store the results for the different modes, identified by the mode_id column)
-- 0: all events
-- 1: best event


-- BEGIN: Inclusion Impact Analysis - event
-- calculte matching group counts
delete from @results_database_schema.cohort_inclusion_result where cohort_definition_id = @target_cohort_id and mode_id = 0;
insert into @results_database_schema.cohort_inclusion_result (cohort_definition_id, inclusion_rule_mask, person_count, mode_id)
select @target_cohort_id as cohort_definition_id, inclusion_rule_mask, count_big(*) as person_count, 0 as mode_id
from
(
  select Q.person_id, Q.event_id, CAST(SUM(coalesce(POWER(cast(2 as bigint), I.inclusion_rule_id), 0)) AS bigint) as inclusion_rule_mask
  from #qualified_events Q
  LEFT JOIN #inclusion_events I on q.person_id = i.person_id and q.event_id = i.event_id
  GROUP BY Q.person_id, Q.event_id
) MG -- matching groups
group by inclusion_rule_mask
;

--- TRUNCATED FOR GITHUB
}

@chrisknoll
Copy link
Collaborator

Note on what is in circe 1.9.1:
the cohortCensoredStats is one that I think is a problem: I put it outside the generateStats clause because I figured that we'd want to know how many people were censored out of the cohort due to the 'study window' setting. I realize now that it makes it a dependency on results_dabase_schema even when we don't want to generate stats.

Therefore, I will adjust the query to not write ANY output to cohort_censor_stats when generateStats == false.

Great catch, @anthonysena , this won't impact you in 1.8.4 in your circe reference, but it will impact you when you update to circe 1.9. I'll fix this in 1.9.2.

@ssuvorov-fls
Copy link
Contributor

@chrisknoll Hi, do you have any updates on this issue?

@anthonysena
Copy link
Collaborator

Hi @ssuvorov-fls - can you please use 1.9.2-SNAPSHOT of circe-be in the WebAPI pom.xml? We believe this will fix this issue.

@chrisknoll
Copy link
Collaborator

I've released 1.9.2 of circe, so if can link to the released dependency.

@chrisknoll chrisknoll reopened this Feb 9, 2021
anton-abushkevich added a commit that referenced this issue Feb 17, 2021
* prepare for release 2.8.0

* 2.8.1-SNAPSHOT

* Update CIRCE to 1.9.1

(cherry picked from commit 9fac161)

* Remove trailing ; from DELETE (#1752)

Fixes #1730.

(cherry picked from commit 66f9100)

* PLP/PLE Analysis result file missed analysis executable code #1729 (#1751)

* removed tomcat dependency from hive profile (#1748)

changed default runtime delegate
(#1720)

Co-authored-by: Sergey Suvorov <[email protected]>

* Issue 1733 build error without git dir (#1757)

* git-commit-id-plugin does not throw exception in case of absent .git folder
if git_commit-id-plugin can not get git info parameters then gmaven-plugin creates these parameters with default value ("*")
(#1733)

* removed temporary dependencies
(#1733)

Co-authored-by: Sergey Suvorov <[email protected]>

* Update org.apache.httpcomponents:httpclient version from 4.5.10 to 4.5.13 (#1747)

* eager loading is now used during conversion of entities to dto (#1763)

fixes #1758

Co-authored-by: Sergey Suvorov <[email protected]>
# Conflicts:
#	pom.xml

* GitHub actions (#1690)

* Puts Maven Central as the first repository

This speeds up the build A LOT when none of the dependencies have been
cached yet (e.g. when performing a docker build). Reason: without putting this repository first, all
dependencies are first searched in all of the specified repositories and
only afterwards the central repository is tried. Because most
dependencies are not present in the specified repositories, still
central needs to be queried afterwards. When putting the central
repository first, this reduces time to retrieve dependencies in two
ways:

1. The central repository has a very low latency, much lower than the
other repositories. If a dependency can be fetched from here, it will be
faster than when it is fetched from another one.
2. For most dependencies, it reduces the number of repositories queried
from 7 to 1.

* Enabled github actions

* Use dedicated docker profile to avoid long command line

* Backport github action fixes

* Fix typo

* Remove unsupported platform

* Only use Maven central in the webapi-docker profile

* ancestorAndDescendant:get permission is now added to new sources  (#1766)

* ancestorAndDescendant:get permission is now added to new sources
fixes #1759

* ancestorAndDescendant:get permission is now added to new sources
fixes #1759

* fixed identation
fixes #1759

* ancestorAndDescendant:get permission is not added to new sources #1759 - fix permission name

Co-authored-by: Sergey Suvorov <[email protected]>
Co-authored-by: Anton Abushkevich <[email protected]>

* ir generation results are now exported correctly (#1767)

queryForRowSet function calls isSigned function which is not implemented for hive
fixes #1765

Co-authored-by: Sergey Suvorov <[email protected]>

* Fix sql server migration.
Revert a721214.
Remove IF EXISTS from sql server.

* Update dependency to arachne-commons release 1.17.1 #1774 (#1775)

* Eager loading without redundant calls for fetching collecions works only in 5.4.2.Final version of hibernate(#1758)

# Conflicts:
#	pom.xml

* Reset Impala settings to default(#1758)

* Updated org.ohdsi from 1.9.1 to 1.9.2 to to fix error in script(#1755) (#1778)

Co-authored-by: Sergey Suvorov <[email protected]>

* Updated org.apache.shiro from 1.6.0 to 1.7.1 to eliminate identified vulnerabilities  (#1759) (#1776)

Co-authored-by: Sergey Suvorov <[email protected]>

* Version 2.8.1

Co-authored-by: anton.abushkevich <anton.abushkevich>
Co-authored-by: Chris Knoll <[email protected]>
Co-authored-by: anton-abushkevich <[email protected]>
Co-authored-by: Sergey Suvorov <[email protected]>
Co-authored-by: Joris Borgdorff <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment