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

Prevent upload of inclusion rule information #78

Closed
anthonysena opened this issue Feb 9, 2023 · 2 comments · Fixed by #156
Closed

Prevent upload of inclusion rule information #78

anthonysena opened this issue Feb 9, 2023 · 2 comments · Fixed by #156
Milestone

Comments

@anthonysena
Copy link
Collaborator

Currently, inclusion rule name insertion will use bulk uploading optionally depending on how a user has their the environment variables set. Here is a reference to the code:

DatabaseConnector::insertTable(
connection = connection,
databaseSchema = cohortDatabaseSchema,
tableName = cohortInclusionTable,
data = inclusionRules,
dropTableIfExists = FALSE,
createTable = FALSE,
camelCaseToSnakeCase = TRUE
)

And the documentation for the insertTable command:

http://ohdsi.github.io/DatabaseConnector/reference/insertTable.html

Generally, the amount of rows to insert for inclusion rules is small so we should disable bulk upload here since may require additional R package dependencies based on the target RDMBS for the insert. Specifically, RedShift requires additional AWS packages when bulk uploading is performed.

@anthonysena anthonysena added this to the v0.10 milestone May 29, 2024
@anthonysena anthonysena changed the title Prevent bulk upload of inclusion rule information Prevent upload of inclusion rule information May 29, 2024
@anthonysena
Copy link
Collaborator Author

Instead of uploading these rules, let's use an in-memory join approach to obtain this information.

@anthonysena anthonysena linked a pull request Jun 5, 2024 that will close this issue
@anthonysena
Copy link
Collaborator Author

The notion of uploading the inclusion rule names should be removed per #158 but since this is a breaking change, we'll wait for v1.x to address this. In the meantime, the exportCohortStatsTables function now takes a cohortDefinitionSet argument to avoid the need to upload inclusion rules per #156.

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 a pull request may close this issue.

1 participant