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

Update source parsing for query filter parsing #280

Merged
merged 9 commits into from
Oct 18, 2023
+92 −36
Merged

Update source parsing for query filter parsing #280

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8f41f92
update source parsing to not be restricted to immediate retrieves
sarahmcdougall Sep 28, 2023
5191be6
retrieve datatype from resultTypeSpecifier
sarahmcdougall Sep 28, 2023
69d01d5
clean up sourceInfo
sarahmcdougall Sep 28, 2023
cb9da71
use cql-exec typespecifier types instead of custom types
sarahmcdougall Sep 28, 2023
c58cf50
prettier
sarahmcdougall Sep 29, 2023
0e7843d
update docstrings, add TODOs for checking for matching types
sarahmcdougall Oct 11, 2023
7556acc
update inner query reassignment, addFiltersToDataRequirements, etc.
sarahmcdougall Oct 16, 2023
1e1afa5
update docstring
sarahmcdougall Oct 18, 2023
bead5ad
update codeFilter array initialization
sarahmcdougall Oct 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 33 additions & 26 deletions src/gaps/GapsReportBuilder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -614,35 +614,42 @@
withErrors: GracefulError[]
) {
if (q.queryInfo) {
const detailedFilters = flattenFilters(q.queryInfo.filter);

detailedFilters.forEach(df => {
if (df.type === 'equals' || df.type === 'in') {
const cf = generateDetailedCodeFilter(df as EqualsFilter | InFilter, q.dataType);

if (cf !== null) {
dataRequirement.codeFilter?.push(cf);
}
} else if (df.type === 'during') {
const dateFilter = generateDetailedDateFilter(df as DuringFilter);
if (dataRequirement.dateFilter) {
dataRequirement.dateFilter.push(dateFilter);
} else {
dataRequirement.dateFilter = [dateFilter];
}
} else {
const valueFilter = generateDetailedValueFilter(df);
if (didEncounterDetailedValueFilterErrors(valueFilter)) {
withErrors.push(valueFilter);
} else if (valueFilter) {
if (dataRequirement.extension) {
dataRequirement.extension.push(valueFilter);
const relevantSource = q.queryInfo.sources.find(source => source.resourceType === q.dataType);
// if a source cannot be found that matches, exit the function
if (relevantSource) {
const detailedFilters = flattenFilters(q.queryInfo.filter);

detailedFilters.forEach(df => {
// DuringFilter, etc. inherit from attribute filter (and have alias on them)
if (relevantSource.alias === (df as AttributeFilter).alias) {
if (df.type === 'equals' || df.type === 'in') {
const cf = generateDetailedCodeFilter(df as EqualsFilter | InFilter, q.dataType);

if (cf !== null) {
dataRequirement.codeFilter?.push(cf);

Check warning on line 629 in src/gaps/GapsReportBuilder.ts

View workflow job for this annotation

GitHub Actions / Coverage annotations (🧪 jest-coverage-report-action)

🌿 Branch is not covered 

Warning! Not covered branch
}
} else if (df.type === 'during') {
const dateFilter = generateDetailedDateFilter(df as DuringFilter);
if (dataRequirement.dateFilter) {
dataRequirement.dateFilter.push(dateFilter);

Check warning on line 634 in src/gaps/GapsReportBuilder.ts

View workflow job for this annotation

GitHub Actions / Coverage annotations (🧪 jest-coverage-report-action)

🧾 Statement is not covered 

Warning! Not covered statement
} else {
dataRequirement.dateFilter = [dateFilter];
}

Check warning on line 637 in src/gaps/GapsReportBuilder.ts

View workflow job for this annotation

GitHub Actions / Coverage annotations (🧪 jest-coverage-report-action)

🌿 Branch is not covered 

Warning! Not covered branch
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this code was already there but I am a little confused by it. So for the first if statement, are we only pushing cf to dataRequirement.codeFilter if dataRequirement.codeFilter already exists? If not, then can we consolidate the code in the else if to do the same thing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like in DataRequirementHelpers.ts this function gets called after calling generateDataRequirement on the retrieve. The generateDataRequirement function already creates the codeFilter array if retrieve.valueSet or retrieve.code exists. So it seems that in most cases dataRequirement.codeFilter will already exist by the time that we get here, unless the retrieve does not have a code or valueSet. In that case, it seems that the cf here would not get added since codeFilter does not exist. Perhaps this is the expected behavior? @hossenlopp does this reasoning seem right?

Copy link
Contributor

Choose a reason for hiding this comment

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

While this reasoning sounds good. It is possible that they may have not used a filter on the retrieve itself and may be filtering in the where part of the query. So codeFilter could definitely use similar logic to the dateFilter to see if the list is there first, and create it if it is not.

} else {
dataRequirement.extension = [valueFilter];
const valueFilter = generateDetailedValueFilter(df);
if (didEncounterDetailedValueFilterErrors(valueFilter)) {
withErrors.push(valueFilter);

Check warning on line 641 in src/gaps/GapsReportBuilder.ts

View workflow job for this annotation

GitHub Actions / Coverage annotations (🧪 jest-coverage-report-action)

🧾 Statement is not covered 

Warning! Not covered statement
} else if (valueFilter) {
if (dataRequirement.extension) {
dataRequirement.extension.push(valueFilter);

Check warning on line 644 in src/gaps/GapsReportBuilder.ts

View workflow job for this annotation

GitHub Actions / Coverage annotations (🧪 jest-coverage-report-action)

🧾 Statement is not covered 

Warning! Not covered statement
} else {
dataRequirement.extension = [valueFilter];
}

Check warning on line 647 in src/gaps/GapsReportBuilder.ts

View workflow job for this annotation

GitHub Actions / Coverage annotations (🧪 jest-coverage-report-action)

🌿 Branch is not covered 

Warning! Not covered branch
}

Check warning on line 648 in src/gaps/GapsReportBuilder.ts

View workflow job for this annotation

GitHub Actions / Coverage annotations (🧪 jest-coverage-report-action)

🌿 Branch is not covered 

Warning! Not covered branch
}
}
}
});
});
}
}
}

Expand Down
59 changes: 50 additions & 9 deletions src/gaps/QueryFilterParser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
import { Interval, Expression, PatientContext, Library, DateTime } from 'cql-execution';
import {
Interval,
Expression,
PatientContext,
Library,
DateTime,
NamedTypeSpecifier,
ListTypeSpecifier
} from 'cql-execution';
import { CQLPatient } from '../types/CQLPatient';
import {
ELM,
Expand Down Expand Up @@ -147,8 +155,8 @@ export async function parseQueryInfo(
patient
);

// use sources from inner query
queryInfo.sources = innerQueryInfo.sources;
// use first source from inner query (only replaces the first source)
queryInfo.sources = [...innerQueryInfo.sources.slice(0, 1), ...queryInfo.sources.slice(1)];

// replace the filters for this query to match the inner source
replaceAliasesInFilters(queryInfo.filter, query.source[0].alias, innerQuery.source[0].alias);
Expand Down Expand Up @@ -216,14 +224,17 @@ function replaceAliasesInFilters(filter: AnyFilter, match: string, replace: stri
}

/**
* Parse information about the sources in a given query.
* Parse information about the sources in a given query. Treat relationships as sources.
*
* @param query The Query expression to parse.
* @returns Information about each source. This is usually an array of one.
* @param query The Query to parse. The query source can consist of aliased query sources or relationship clauses.
* @returns Information about each source. This is usually an array of one, except for when we are working with
* multi-source queries or relationships.
*/
function parseSources(query: ELMQuery): SourceInfo[] {
const sources: SourceInfo[] = [];
query.source.forEach(source => {
const querySources = [...query.source, ...query.relationship];

querySources.forEach(source => {
if (source.expression.type == 'Retrieve') {
const sourceInfo: SourceInfo = {
sourceLocalId: source.localId,
Expand All @@ -232,8 +243,17 @@ function parseSources(query: ELMQuery): SourceInfo[] {
resourceType: parseDataType(source.expression as ELMRetrieve)
};
sources.push(sourceInfo);
// use the resultTypeSpecifier as a fallback if the expression is not a Retrieve
} else if (source.expression.resultTypeSpecifier) {
const sourceInfo: SourceInfo = {
sourceLocalId: source.localId,
alias: source.alias,
resourceType: parseElementType(source.expression)
};
sources.push(sourceInfo);
}
});

return sources;
}

Expand All @@ -247,6 +267,27 @@ function parseDataType(retrieve: ELMRetrieve): string {
return retrieve.dataType.replace(/^(\{http:\/\/hl7.org\/fhir\})?/, '');
}

/**
* Pulls out the resource type of a result type. Used when a source expression type is *not* a
* Retrieve but the source expression contains a type specified for the result of the expression that
* we can use to parse the resource type of the query source.
*
* @param expression The ELM expression to pull out resource type from.
* @returns FHIR ResourceType name.
*/
function parseElementType(expression: ELMExpression): string {
const resultTypeSpecifier = expression.resultTypeSpecifier;
if (
resultTypeSpecifier?.type === 'ListTypeSpecifier' &&
(resultTypeSpecifier as ListTypeSpecifier).elementType.type === 'NamedTypeSpecifier'
) {
const elementType = (resultTypeSpecifier as ListTypeSpecifier).elementType as NamedTypeSpecifier;
return elementType.name.replace(/^(\{http:\/\/hl7.org\/fhir\})?/, '');
}
console.warn(`Resource type cannot be found for ELM Expression with localId ${expression.localId}`);
return '';
}

/**
* Interprets an expression into a filter tree. This is the central point where the interpreting occurs. This function
* determines the expression type and sends it to the correct place to be parsed.
Expand Down Expand Up @@ -713,7 +754,7 @@ export function interpretIncludedIn(
parameters: any
): DuringFilter | UnknownFilter {
let propRef: ELMProperty | GracefulError | null = null;
let withError: GracefulError = { message: 'An unknown error occured' };
let withError: GracefulError = { message: 'An unknown error occurred' };
if (includedIn.operand[0].type == 'FunctionRef') {
propRef = interpretFunctionRef(includedIn.operand[0] as ELMFunctionRef, library);
} else if (includedIn.operand[0].type == 'Property') {
Expand Down Expand Up @@ -779,7 +820,7 @@ export async function interpretIn(
parameters: any
): Promise<InFilter | DuringFilter | UnknownFilter> {
let propRef: ELMProperty | GracefulError | null = null;
let withError: GracefulError = { message: 'An unknown error occured' };
let withError: GracefulError = { message: 'An unknown error occurred' };
if (inExpr.operand[0].type == 'FunctionRef') {
propRef = interpretFunctionRef(inExpr.operand[0] as ELMFunctionRef, library);
} else if (inExpr.operand[0].type == 'Property') {
Expand Down
6 changes: 5 additions & 1 deletion src/types/ELMTypes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { AnyTypeSpecifier } from 'cql-execution';

/** Top level of an ELM JSON. */
export interface ELM {
/** ELM Library definition. */
Expand Down Expand Up @@ -64,7 +66,7 @@ export interface ELMInclude {
locator?: string;
/** Local identifier that will be used to reference this library in the logic. */
localIdentifier: string;
/** The id of the refereced library. */
/** The id of the referenced library. */
path: string;
/** The version of the referenced library. */
version: string;
Expand Down Expand Up @@ -167,6 +169,8 @@ export interface ELMExpression {
localId?: string;
/** Locator in the original CQL file. Only exists if compiled with this info. */
locator?: string;
/** Type specifier for the result of the expression. This field may or may not be included in translator output. */
resultTypeSpecifier?: AnyTypeSpecifier;
hossenlopp marked this conversation as resolved.
Show resolved Hide resolved
}

export type AnyELMExpression =
Expand Down
Loading