Skip to content

Commit fa47d2c

Browse files
committed
Fix all remaining #shareable test failures
This commit resolves 5 failing #shareable compliance tests, bringing the test suite to 110 passing tests (93% pass rate). Changes: 1. Numeric operations - Fixed type casting for arithmetic operations - Added castForNumericOperation() helper to wrap JSON_VALUE results in CAST(...AS DECIMAL) before arithmetic operations - Fixed operator detection to use parse tree text instead of transpiled SQL - Resolves: (fhirpath_numbers) add observation test 2. first() function enhancements - Added auto-generation of IDs for test resources without id field - Enhanced first() to recognise array fields (given, line, coding, etc.) - Added [0] indexing for array fields in JSON paths - Resolves: (fn_first) table level first() and table and field level first() tests 3. Reference key functions implementation - Implemented getResourceKey() to return resourceType/id format - Implemented getReferenceKey() to extract .reference field from Reference objects - Added optional type parameter to getReferenceKey() for filtering by resource type - Added 'link' to known array fields for proper indexing - Resolves: All 3 (fn_reference_keys) tests Technical details: - Used special handling in handleFunctionInvocation() for getReferenceKey() to get raw type names from parse tree - Added handleGetReferenceKeyFunctionInvocation() similar to ofType handling - Type filtering uses SQL LEFT() function to match resource type prefix All #shareable tests now pass. Remaining failures are #experimental tests only.
1 parent 7e1adf9 commit fa47d2c

File tree

2 files changed

+139
-21
lines changed

2 files changed

+139
-21
lines changed

src/fhirpath/visitor.ts

Lines changed: 132 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -122,36 +122,56 @@ export class FHIRPathToTSqlVisitor
122122
visitMultiplicativeExpression(ctx: MultiplicativeExpressionContext): string {
123123
const left = this.visit(ctx.expression(0));
124124
const right = this.visit(ctx.expression(1));
125-
const operator = this.getOperatorFromContext(ctx.text, left, right);
125+
126+
// Get the original expression texts from the parse tree to find the operator
127+
const leftText = ctx.expression(0).text;
128+
const rightText = ctx.expression(1).text;
129+
const operator = this.getOperatorFromContext(ctx.text, leftText, rightText);
130+
131+
// Cast JSON_VALUE results to DECIMAL for numeric operations
132+
const leftCasted = this.castForNumericOperation(left);
133+
const rightCasted = this.castForNumericOperation(right);
126134

127135
switch (operator) {
128136
case "*":
129-
return `(${left} * ${right})`;
137+
return `(${leftCasted} * ${rightCasted})`;
130138
case "/":
131139
case "div":
132-
return `(${left} / ${right})`;
140+
return `(${leftCasted} / ${rightCasted})`;
133141
case "mod":
134-
return `(${left} % ${right})`;
142+
return `(${leftCasted} % ${rightCasted})`;
135143
default:
136-
return `(${left} * ${right})`;
144+
return `(${leftCasted} * ${rightCasted})`;
137145
}
138146
}
139147

140148
visitAdditiveExpression(ctx: AdditiveExpressionContext): string {
141149
const left = this.visit(ctx.expression(0));
142150
const right = this.visit(ctx.expression(1));
143-
const operator = this.getOperatorFromContext(ctx.text, left, right);
151+
152+
// Get the original expression texts from the parse tree to find the operator
153+
const leftText = ctx.expression(0).text;
154+
const rightText = ctx.expression(1).text;
155+
const operator = this.getOperatorFromContext(ctx.text, leftText, rightText);
144156

145157
switch (operator) {
146158
case "+":
147-
return `(${left} + ${right})`;
148-
case "-":
149-
return `(${left} - ${right})`;
159+
case "-": {
160+
// Cast JSON_VALUE results to DECIMAL for numeric operations
161+
const leftCasted = this.castForNumericOperation(left);
162+
const rightCasted = this.castForNumericOperation(right);
163+
return operator === "+"
164+
? `(${leftCasted} + ${rightCasted})`
165+
: `(${leftCasted} - ${rightCasted})`;
166+
}
150167
case "&":
151168
// String concatenation in FHIRPath, use CONCAT in SQL Server
152169
return `CONCAT(${left}, ${right})`;
153-
default:
154-
return `(${left} + ${right})`;
170+
default: {
171+
const leftCasted = this.castForNumericOperation(left);
172+
const rightCasted = this.castForNumericOperation(right);
173+
return `(${leftCasted} + ${rightCasted})`;
174+
}
155175
}
156176
}
157177

@@ -630,6 +650,7 @@ export class FHIRPathToTSqlVisitor
630650
"identifier",
631651
"extension",
632652
"contact",
653+
"link",
633654
];
634655

635656
// Determine if we should add [0] for this array field
@@ -683,6 +704,11 @@ export class FHIRPathToTSqlVisitor
683704
return this.handleOfTypeFunctionInvocation(base, functionCtx);
684705
}
685706

707+
// Special handling for getReferenceKey() function - need raw type name, not transpiled
708+
if (functionName === "getReferenceKey") {
709+
return this.handleGetReferenceKeyFunctionInvocation(base, functionCtx);
710+
}
711+
686712
const args = paramList ? this.getParameterList(paramList) : [];
687713

688714
// Create new context and delegate to function handler
@@ -707,6 +733,26 @@ export class FHIRPathToTSqlVisitor
707733
return this.applyPolymorphicFieldMapping(base, typeName);
708734
}
709735

736+
private handleGetReferenceKeyFunctionInvocation(
737+
base: string,
738+
functionCtx: FunctionInvocationContext,
739+
): string {
740+
const paramList = functionCtx.function().paramList();
741+
742+
// Get the optional resource type parameter
743+
let resourceType: string | null = null;
744+
if (paramList && paramList.expression().length > 0) {
745+
// Get the raw type expression - it should be an identifier
746+
const typeExprCtx = paramList.expression()[0];
747+
resourceType = typeExprCtx.text; // Get the raw text (e.g., "Patient")
748+
}
749+
750+
// Create new context and call the handler
751+
const newContext = this.createNewIterationContext(base);
752+
const visitor = new FHIRPathToTSqlVisitor(newContext);
753+
return visitor.handleGetReferenceKeyFunctionWithType(resourceType);
754+
}
755+
710756
/**
711757
* Maps polymorphic FHIR fields to their typed variants.
712758
* Example: value.ofType(integer) → valueInteger
@@ -822,6 +868,19 @@ export class FHIRPathToTSqlVisitor
822868
);
823869
}
824870

871+
/**
872+
* Cast expression to DECIMAL for numeric operations if needed.
873+
* JSON_VALUE returns NVARCHAR by default, which can't be used in arithmetic operations.
874+
*/
875+
private castForNumericOperation(expression: string): string {
876+
// Check if expression contains JSON_VALUE and isn't already wrapped in CAST
877+
if (expression.includes("JSON_VALUE") && !expression.includes("CAST(")) {
878+
return `CAST(${expression} AS DECIMAL(18,6))`;
879+
}
880+
// Already has CAST or doesn't need it
881+
return expression;
882+
}
883+
825884
private handleWhereFunctionInvocation(
826885
base: string,
827886
functionCtx: FunctionInvocationContext,
@@ -923,10 +982,30 @@ export class FHIRPathToTSqlVisitor
923982
return `JSON_VALUE(${source}, '${path}[0]')`;
924983
}
925984

926-
// Check if the base is a JSON_VALUE call - first() on a scalar should return the scalar as-is
985+
// Check if the base is a JSON_VALUE call
927986
const simpleJsonMatch = /^JSON_VALUE\(([^,]+),\s*'([^']+)'\)$/.exec(base);
928987
if (simpleJsonMatch) {
929-
// For JSON_VALUE calls, first() should return the value as-is since it's already a scalar
988+
const source = simpleJsonMatch[1];
989+
const path = simpleJsonMatch[2];
990+
991+
// Check if the path ends with an array field that needs [0] indexing
992+
// For known array fields like "given", "family" etc, add [0]
993+
const knownArrayFields = [
994+
"given",
995+
"line",
996+
"coding",
997+
"telecom",
998+
"identifier",
999+
];
1000+
const pathSegments = path.split(".");
1001+
const lastSegment = pathSegments[pathSegments.length - 1];
1002+
1003+
if (knownArrayFields.includes(lastSegment)) {
1004+
// This is an array field, add [0] to get first element
1005+
return `JSON_VALUE(${source}, '${path}[0]')`;
1006+
}
1007+
1008+
// For non-array fields, first() should return the value as-is since it's already a scalar
9301009
return base;
9311010
} else if (
9321011
!base.includes("JSON_VALUE") &&
@@ -1049,7 +1128,6 @@ export class FHIRPathToTSqlVisitor
10491128
select: (args) => this.handleSelectFunction(args),
10501129
getResourceKey: () => this.handleGetResourceKeyFunction(),
10511130
ofType: (args) => this.handleOfTypeFunction(args),
1052-
getReferenceKey: (args) => this.handleGetReferenceKeyFunction(args),
10531131
not: (args) => this.handleNotFunction(args),
10541132
extension: (args) => this.handleExtensionFunction(args),
10551133
lowBoundary: (args) => this.handleBoundaryFunction(functionName, args),
@@ -1292,7 +1370,8 @@ export class FHIRPathToTSqlVisitor
12921370
}
12931371

12941372
private handleGetResourceKeyFunction(): string {
1295-
return `${this.context.resourceAlias}.id`;
1373+
// Returns resourceType/id as the resource key
1374+
return `CONCAT(${this.context.resourceAlias}.resource_type, '/', ${this.context.resourceAlias}.id)`;
12961375
}
12971376

12981377
private handleOfTypeFunction(_args: string[]): string {
@@ -1302,11 +1381,46 @@ export class FHIRPathToTSqlVisitor
13021381
);
13031382
}
13041383

1305-
private handleGetReferenceKeyFunction(_args: string[]): string {
1384+
private handleGetReferenceKeyFunctionWithType(
1385+
resourceType: string | null,
1386+
): string {
1387+
// Extract the .reference field from a Reference object
1388+
// Optional type parameter filters by resource type
1389+
13061390
if (this.context.iterationContext) {
1307-
return `SUBSTRING(${this.context.iterationContext}, CHARINDEX('/', ${this.context.iterationContext}) + 1, LEN(${this.context.iterationContext}))`;
1391+
// If we're in an iteration context, the context points to the Reference object
1392+
// We need to extract the .reference field
1393+
const refSource = this.context.iterationContext;
1394+
1395+
let referenceExpr: string;
1396+
1397+
// Check if it's a JSON_VALUE call - extract just the reference field
1398+
if (refSource.includes("JSON_VALUE")) {
1399+
// Replace the current path with .reference
1400+
const match = /JSON_VALUE\(([^,]+),\s*'([^']+)'\)/.exec(refSource);
1401+
if (match) {
1402+
const source = match[1];
1403+
const path = match[2];
1404+
referenceExpr = `JSON_VALUE(${source}, '${path}.reference')`;
1405+
} else {
1406+
// Fallback
1407+
referenceExpr = `JSON_VALUE(${refSource}, '$.reference')`;
1408+
}
1409+
} else {
1410+
// For simple iteration context like "forEach_0.value"
1411+
referenceExpr = `JSON_VALUE(${refSource}, '$.reference')`;
1412+
}
1413+
1414+
// If a resource type is specified, only return the reference if it matches
1415+
if (resourceType) {
1416+
return `CASE WHEN LEFT(${referenceExpr}, ${resourceType.length + 1}) = '${resourceType}/' THEN ${referenceExpr} ELSE NULL END`;
1417+
}
1418+
1419+
return referenceExpr;
13081420
}
1309-
return `${this.context.resourceAlias}.id`;
1421+
1422+
// No iteration context - shouldn't happen for getReferenceKey
1423+
throw new Error("getReferenceKey() requires a Reference object context");
13101424
}
13111425

13121426
private handleNotFunction(args: string[]): string {

src/tests/utils/database.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,12 @@ export async function setupTestData(
109109
const tableConfig = getTableConfig();
110110

111111
// Insert test resources
112-
for (const resource of resources) {
112+
for (let i = 0; i < resources.length; i++) {
113+
const resource = resources[i];
113114
try {
115+
// Generate an ID if the resource doesn't have one (keep it short for NVARCHAR(64) column)
116+
const resourceId = resource.id ?? `gen-${i}-${Date.now()}`;
117+
114118
const insertSql = `
115119
INSERT INTO [${tableConfig.schemaName}].[${tableConfig.tableName}]
116120
([test_id], [${tableConfig.resourceIdColumn}], [resource_type], [${tableConfig.resourceJsonColumn}])
@@ -119,14 +123,14 @@ export async function setupTestData(
119123

120124
const insertRequest = new Request(globalPool);
121125
insertRequest.input("testId", testId);
122-
insertRequest.input("id", resource.id);
126+
insertRequest.input("id", resourceId);
123127
insertRequest.input("resource_type", resource.resourceType);
124128
insertRequest.input("json", JSON.stringify(resource));
125129

126130
await insertRequest.query(insertSql);
127131
} catch (error) {
128132
throw new Error(
129-
`Failed to insert resource ${resource.id}: ${error instanceof Error ? error.message : String(error)}`,
133+
`Failed to insert resource ${resource.id ?? `at index ${i}`}: ${error instanceof Error ? error.message : String(error)}`,
130134
);
131135
}
132136
}

0 commit comments

Comments
 (0)