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.

Sign up for GitHub

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

Jump to bottom

Move Entity/Underlay to EntityFilter #1137

Merged
merged 2 commits into from
Jan 20, 2025
Merged

Move Entity/Underlay to EntityFilter #1137

merged 2 commits into from
Jan 20, 2025

Conversation

dexamundsen
Copy link
Contributor

@dexamundsen dexamundsen commented Jan 20, 2025 •
edited
Loading

  • Move Entity / Underlay to EntityFilter from child classes. this removes repeated definition in child classes
  • add parent class constructor, equals and hashcode functions in EntityFilter, called from child classes
  • replace more wild card imports by individual imports
  • rename ITEntitySearchByAttribute to ITEntitySearchByAttributes (added s at the end) to keep it same as config field
  • update index for aou_test underlay (changes variant_to_person to cb_variant_to_person)
  • update util functions in entity and indexSchema to store AttributeName instead of Attribute for search by attribute (improves performance)

dexamundsen
dexamundsen commented Jan 20, 2025
View reviewed changes
indexer/src/main/java/bio/terra/tanagra/indexing/JobSequencer.java
jobSet.addJob(
new WriteEntitySearchByAttribute(
new WriteEntitySearchByAttributes(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added s at the end (rename)

dexamundsen
dexamundsen commented Jan 20, 2025
View reviewed changes
...xer/src/main/java/bio/terra/tanagra/indexing/job/bigquery/WriteEntitySearchByAttributes.java
@@ -17,18 +17,18 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class WriteEntitySearchByAttribute extends BigQueryJob {
private static final Logger LOGGER = LoggerFactory.getLogger(WriteEntitySearchByAttribute.class);
public class WriteEntitySearchByAttributes extends BigQueryJob {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added s at the end (rename)

dexamundsen
dexamundsen commented Jan 20, 2025
View reviewed changes
indexer/src/main/java/bio/terra/tanagra/indexing/JobSequencer.java
@@ -72,12 +77,14 @@ public static SequencedJobSet getJobSetForEntity(
.getOptimizeSearchByAttributes()
.forEach(
searchTableAttributes -> {
ITEntitySearchByAttribute searchTable =
ITEntitySearchByAttributes searchTable =
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added s at the end (rename)

dexamundsen
dexamundsen commented Jan 20, 2025
View reviewed changes
underlay/src/main/java/bio/terra/tanagra/api/filter/AttributeFilter.java

public class AttributeFilter extends EntityFilter {
private final Underlay underlay;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved to parent class

dexamundsen
dexamundsen commented Jan 20, 2025
View reviewed changes
underlay/src/main/resources/config/service/aouSC2023Q3R2_oneverily_dev.json
@@ -3,15 +3,15 @@
"bigQuery": {
"sourceData": {
"projectId": "vwb-dev-blissful-eggplant-4805",
"datasetId": "aou_test_data_SC2023Q3R2",
"datasetId": "aou_test_data_SC2023Q3R2_011625",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same dataset for source and index

dexamundsen
dexamundsen commented Jan 20, 2025
View reviewed changes
.../main/resources/config/datamapping/aouCT_testonly/entitygroup/variantPerson/rollupCounts.sql
@@ -1,3 +1,3 @@
SELECT vid, ARRAY_LENGTH(person_ids) AS num_persons
/* Wrap variant_to_person table in a SELECT DISTINCT because there is a duplicate row in the test data. */
FROM (SELECT DISTINCT vid, person_ids FROM `${omopDataset}.variant_to_person` WHERE REGEXP_CONTAINS(vid, r"{indexIdRegex}"))
FROM (SELECT DISTINCT vid, person_ids FROM `${omopDataset}.cb_variant_to_person` WHERE REGEXP_CONTAINS(vid, r"{indexIdRegex}"))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed in new source data

dexamundsen
dexamundsen commented Jan 20, 2025
View reviewed changes
underlay/src/main/java/bio/terra/tanagra/utils/GoogleBigQuery.java
@@ -262,13 +262,13 @@ public TableResult runQuery(
JobStatistics.QueryStatistics stats = completedJob.getStatistics();
Long totalBytesProcessed = stats.getTotalBytesProcessed();
String jobId = completedJob.getJobId().getJob();
LOGGER.info("job: {}, query: {}", jobId, sql);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

combine logs

dexamundsen
dexamundsen commented Jan 20, 2025
View reviewed changes
underlay/src/main/java/bio/terra/tanagra/underlay/IndexSchema.java
@@ -66,32 +66,13 @@ public ITEntityLevelDisplayHints getEntityLevelDisplayHints(String entity) {
.orElseThrow();
}

// function used during indexing: when table clustered on all attributes are needed
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

combined to single function

dexamundsen
dexamundsen commented Jan 20, 2025
View reviewed changes
underlay/src/main/java/bio/terra/tanagra/underlay/entitymodel/Entity.java
@@ -42,6 +43,14 @@ public Entity(
this.optimizeSearchByAttributes =
ImmutableList.copyOf(
optimizeSearchByAttributes.stream().map(ImmutableList::copyOf).toList());
this.optimizeSearchByAttributeNames =
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

improves performance in updates util function

@dexamundsen dexamundsen changed the title Move Entity /Underlay to EntityFilter Move Entity/Underlay to EntityFilter Jan 20, 2025
@dexamundsen dexamundsen merged commit 7d0cad7 into main Jan 20, 2025
8 checks passed
@dexamundsen dexamundsen deleted the dexamundsen/efilter branch January 20, 2025 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tjennison-work tjennison-work tjennison-work approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dexamundsen @tjennison-work