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

Split DELETE statement after translate. #1752

Merged
merged 2 commits into from
Jan 20, 2021
Merged

Split DELETE statement after translate. #1752

merged 2 commits into from
Jan 20, 2021

Conversation

chrisknoll
Copy link
Collaborator

Fixes #1730.

@chrisknoll
Copy link
Collaborator Author

@ssuvorov-fls , could you check this against your Impala/Hive cdm source? This should address your issue.

@chrisknoll chrisknoll merged commit 66f9100 into master Jan 20, 2021
@delete-merged-branch delete-merged-branch bot deleted the issue-1730 branch January 20, 2021 18:29
chrisknoll added a commit that referenced this pull request Jan 20, 2021
Fixes #1730.

(cherry picked from commit 66f9100)
@ssuvorov-fls
Copy link
Contributor

@chrisknoll
Sorry for late answer
There's a problem with Impala - it has a translation for delete statement in SqlRender but it needs a trailing semicolon to be translated. I tried to modify translation rule for this case but I got "Error in search pattern: pattern cannot start or end with a non-regex variable"
May be we should remove trailing semicolon after translation?

@chrisknoll
Copy link
Collaborator Author

Or we go back to my original approach which is to split it and then batch execute it.

@ssuvorov-fls
Copy link
Contributor

ssuvorov-fls commented Feb 16, 2021

@chrisknoll
I don't understand what you want to split. There's one single statement before translation (DELETE FROM %s.cohort_inclusion WHERE cohort_definition_id = %d;) and one after (INSERT OVERWRITE TABLE %s.cohort_inclusion SELECT * FROM %s.cohort_inclusion WHERE NOT(cohort_definition_id = %d); for Impala)

@chrisknoll
Copy link
Collaborator Author

chrisknoll commented Feb 16, 2021

The split handles trailing semicolons in a reasonable manner. You're suggesting that we add a semicolon to it (so that it can be translated via impala) and then strip the semicolon after (because impala can't handle trailing semicolons)? So, you both want and do not want a semicolon. Splitting it will solve both problems.

If you have a better approach, go ahead and submit the PR, I'm fine with whatever works.

anton-abushkevich added a commit that referenced this pull request 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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ORA-00933: SQL command not properly ended
3 participants