CQL Long by cmoesel · Pull Request #363 · cqframework/cql-execution

cmoesel · 2026-05-11T12:47:06Z

This PR introduces support for CQL Long, represented using BigInt in JavaScript. Since Integer and Decimal are represented using Number, we can easily distinguish CQL Longs from Integers and Decimals by their type (typeof var === 'bigint'). This actually made implementation easier (and more correct) compared to my initial implementation using Number (which I thought would be easier since it was more of an incremental change, but maybe not).

You can test this by building and running a CQL library that uses Long types -- or you can just review the unit tests and trust that they are properly exercising the capability.

Submitter:

This pull request describes why these changes were made
Code diff has been done and been reviewed (it does not contain: additional white space, not applicable code changes, debug statements, etc.)
Tests are included and test edge cases
Tests have been run locally and pass
Code coverage has not gone down and all code touched or added is covered.
Code passes lint and prettier (hint: use npm run test:plus to run tests, lint, and prettier)
All dependent libraries are appropriately updated or have a corresponding PR related to this change (N/A)
cql4browsers.js built with npm run build:browserify if source changed.

Reviewer:

Name:

Code is maintainable and reusable, reuses existing code and infrastructure where appropriate, and accomplishes the task’s purpose
The tests appropriately test the new code, including edge cases
You have tried to break the code

codecov-commenter · 2026-05-11T12:48:48Z

Codecov Report

❌ Patch coverage is 84.12017% with 37 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.06%. Comparing base (5bc96b7) to head (bb6e72c).
⚠️ Report is 3 commits behind head on master.

Files with missing lines	Patch %	Lines
src/elm/type.ts	64.44%	13 Missing and 3 partials ⚠️
src/util/math.ts	89.09%	6 Missing ⚠️
src/elm/interval.ts	83.33%	3 Missing and 2 partials ⚠️
src/datatypes/interval.ts	85.00%	0 Missing and 3 partials ⚠️
src/elm/arithmetic.ts	88.88%	2 Missing and 1 partial ⚠️
src/runtime/context.ts	25.00%	2 Missing and 1 partial ⚠️
src/elm/literal.ts	85.71%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #363      +/-   ##
==========================================
+ Coverage   87.58%   88.06%   +0.48%     
==========================================
  Files          52       54       +2     
  Lines        4606     4760     +154     
  Branches     1297     1327      +30     
==========================================
+ Hits         4034     4192     +158     
- Misses        359      372      +13     
+ Partials      213      196      -17

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

lmd59

Looking good! The tests are very thorough and helpful for confirming successful implementation. I left a couple of small comments on tests. Most are just nitpick/optional, but I think a couple more aggregate tests wouldn't hurt.

I also wanted to ask about support for Long in the interval datatype
https://cql.hl7.org/04-logicalspecification.html#interval states that "An interval must be defined using a point type that supports comparison, as well as Successor and Predecessor operations, and Minimum and Maximum Value operations." Long seems like it would fit in this category. Is there any reason not to support Long as part of an interval?

cmoesel · 2026-05-19T17:05:18Z

Thanks for the excellent review! You uncovered some gaps in my implementation and also helped me to find more gaps on my own. But I will also note: be careful what you wish for! I just added a lot more changes and tests.

I added support for Interval<Long> in 79c0fb8, including support for multiple operations that work on intervals.
I added tests for sum and product overflow/underflow and in the process discovered we weren't checking for overflow/underflow at all, so I also fixed the implementation. See: 6eaf011.
This led me to realize that we could improve our type handling in general by using ELM result types when they are available, so I did that and fixed a bunch of related stuff in 4b5e62d. That one's kind of big.
And finally, I found a couple more spots where we weren't explicitly accounting for long/bigint, so I fixed those in 9620bb8.

Thanks again for the great review. Sorry for adding a bunch more work for your review!

- Add support for Long literal, ToLong conversion, min/max long values - Use JavaScript Number to represent Long (NOTE: this means that values are imprecise outside of the safe integer range in JS) - Add tests for literals, conversion, and other operations that accept Long arguments - Improve underflow/overflow tests to test boundaries more carefully - Added .skip to tests that fail due to Number imprecision for high values - Unskipped Long tests in the spec tests that now pass

Update support for Long to use BigInt so we can distinguish between decimal/integer (JS Number) and long (JS BigInt).

- support for <, <=, >=, > for long/bigint types - support for ConvertsToLong and Convert operator with Long

Co-authored-by: lmd59 <lmd59@cornell.edu>

- Add optional resultTypeName property to base Expression class - Use resultTypeName when possible to distinguish between decimal and integer - Use constants for ELM type strings (e.g., {urn:hl7-org:elm-types:r1}Decimal) - Update spec-test-data generator CQL-to-ELM to produce resultTypes - Unskip tests that are now passing - Fix tests that were unintentionally incorrect based on previous incomplete type support

…ifier

cmoesel · 2026-05-27T13:12:21Z

FYI: I rebased this branch on master to take in the latest test-server changes. There was minimal impact to the changes in this branch (just some small changes in tsconfig.json).

lmd59

Excellent updates, and addressing a lot of new Long functionality and improved type handling!
I added some comments, but most of them are small!

lmd59 · 2026-05-31T19:31:05Z

+define longs_at_min_value_product: Product({-1L, 2L, 4611686018427387904L}) // -9223372036854775808
+define longs_below_min_value_product: Product({-1L, 3L, 3074457345618258603L}) // -9223372036854775809
+define decimal_product: Product({1.0, 2.0, 3.0, 4.0})
+define decimals_at_max_value_product: Product({99999999999999999999.99999999})


Suggested change

define decimals_at_max_value_product: Product({99999999999999999999.99999999})

define decimals_at_max_value_product: Product({99999999999999999999.99999999, 1.0})

Optional: Not really necessary, but might make the test a little clearer

Yeah, it's probably better to have at least two arguments. Good catch.

lmd59 · 2026-05-31T19:31:27Z

+define decimal_product: Product({1.0, 2.0, 3.0, 4.0})
+define decimals_at_max_value_product: Product({99999999999999999999.99999999})
+define decimals_above_max_value_product: Product({99999999999999999999.99999999, 2.0})
+define decimals_at_min_value_product: Product({-99999999999999999999.99999999})


Suggested change

define decimals_at_min_value_product: Product({-99999999999999999999.99999999})

define decimals_at_min_value_product: Product({-99999999999999999999.99999999, 1.0})

Optional

lmd59 · 2026-05-31T19:31:54Z

+define decimals_at_min_value_product: Product({-99999999999999999999.99999999})
+define decimals_below_min_value_product: Product({-99999999999999999999.99999999, 2.0})
 define quantity_product: Product({1.0 'g', 2.0 'g', 3.0 'g', 4.0 'g'})
+define quantities_at_max_value_product: Product({99999999999999999999.99999999 'g'})


Suggested change

define quantities_at_max_value_product: Product({99999999999999999999.99999999 'g'})

define quantities_at_max_value_product: Product({99999999999999999999.99999999 'g', 1.0 'g'})

Optional

lmd59 · 2026-05-31T19:41:48Z

-    if (typeof pointSize === 'number') {
-      pointSize = new Quantity(pointSize, '1');
+    if (typeof pointSize === 'number' || typeof pointSize === 'bigint') {
+      pointSize = new Quantity(Number(pointSize), '1');


I think ideally we'd want the pointsize for bigint to be 1n. The value and unit for a Quantity both seem to be of type any. Is there a reason not to make the pointsize into a bigint value?

You know, I feel like I had a good reason for this -- like the spec didn't define a Long return type for size. But I'm looking at it again and most of the spec stuff around this uses <T>, so 1n seems appropriate.

Or maybe this was just a straight-up mistake. Anyway, I agree with you.

lmd59 · 2026-05-31T19:43:35Z

      } else if (this.high.isQuantity) {
        pointSize = doSubtraction(successor(this.high), this.high);
      } else {
        pointSize = successor(this.high) - this.high;


Suggested change

pointSize = this.high - predecessor(this.high);

can overflow as is

Woah. This bug is ancient. Git blame shows it was carried over in the conversion from .coffee to .js, which was 6 years ago, but who knows how long it was in the .coffee before that). Excellent find!

lmd59 · 2026-05-31T19:46:35Z

-        if (b.low - a.high <= perWidth.value) {
+        const distance = b.low - a.high;
+        const comparablePerWidth =
+          typeof distance === 'bigint' && Number.isInteger(perWidth.value)


A little confused about this section. What if perWidth is a decimal? Could casting Number(b.low) below produce a precision issue?

Since the perWidth argument is a Quantity, then the perWidth.value is a Decimal (in terms of CQL). But JS doesn't allow us to compare number w/ bigint, so we need to convert the distance and perWidth to like types.

When perWidth is an integer (no decimal component), then it's safest to convert the perWidth to a bigint and compare bigint to bigint -- because integer always converts cleanly to bigint.

But if perWidth has decimal components, then it can't be converted to bigint. The only choice we have is to convert bigint to a number. This could introduce precision issues for Longs that are greater than the max safe integer representation in JS, but there's very little we could do. So we're doing the best we can. I guess the alternative would be to throw an error if we detect that we'll lose precision, but the CQL spec doesn't indicate that collapse can throw, so this very well might take implementers by surprise.

That said, it might be safer to do Number(distance) on line 793 rather than Number(b.low) - Number(a.high) since the difference is likely to be a smaller number than b.low or b.high -- and therefore less likely to run into precision issues. So, yeah, we probably should do that at least.

lmd59 · 2026-05-31T19:49:37Z

+define NegInfBegContainsLong: Interval[null, 5L] contains -7L
+define NegInfBegNotContainsLong: Interval[null, 5L] contains 7L
+define UnknownOpenBegContainsLong: Interval(null, 5L] contains 5L
+define UnknownClosedBegContainsLong: Interval[null, 5L] contains 5L


Isn't this neg infinity closed beginning (rather than unknown)?

Same for all other define statements in this file starting with UnknownClosed

Yeah. Probably a bad copy/paste exacerbated by a series of copy/pastes after.

I think that line 111 should be UnknownBegContainsLong and line 112 can be removed completely since we already have a NegInfContainsLong case. And similar changes should be made in all the other places with was copy/pasted.

lmd59 · 2026-05-31T19:55:28Z

@@ -833,6 +1005,7 @@ define DateTimeIntervalEndsStartsFalse: Interval[DateTime(2012, 1, 5), DateTime(

 // @Test: IntegerIntervalUnion


Superoptional, but since "Integer" test sets (i.e. IntegerIntervalUnion) now include Long tests, is it worth a slight rename (or separating out Long tests) for clarity? Longs are Integers in reality, so also feel free to ignore this comment

We should probably separate the Long tests out.

Co-authored-by: lmd59 <lmd59@cornell.edu>

cmoesel · 2026-06-01T16:31:39Z

Fantastic feedback again, @lmd59. You're making me feel like I'm getting sloppy. Thanks for the thorough review and great finds. I'll fix these soon (but probably not today).

cmoesel mentioned this pull request May 11, 2026

Implement CQL Long #361

Closed

11 tasks

cmoesel marked this pull request as draft May 11, 2026 13:53

cmoesel marked this pull request as ready for review May 11, 2026 15:26

cmoesel force-pushed the cql-long branch from 8ecd7b5 to 9bbb25d Compare May 11, 2026 18:39

elsaperelli requested a review from lmd59 May 12, 2026 14:20

elsaperelli assigned lmd59 May 12, 2026

lmd59 reviewed May 14, 2026

View reviewed changes

Comment thread test/elm/literal/literal-test.ts Outdated

Comment thread test/elm/literal/literal-test.ts Outdated

Comment thread test/elm/arithmetic/data.cql

Comment thread test/elm/aggregate/data.cql

Comment thread test/elm/aggregate/data.cql

cmoesel and others added 9 commits May 27, 2026 09:00

Support for Long as JS BigInt

62f9c44

Update support for Long to use BigInt so we can distinguish between decimal/integer (JS Number) and long (JS BigInt).

Long: comparison and conversion

219d086

- support for <, <=, >=, > for long/bigint types - support for ConvertsToLong and Convert operator with Long

Remove commented out skipped long tests

b6c8069

Update test case descriptions to be more clear

ecaa61d

Co-authored-by: lmd59 <lmd59@cornell.edu>

Support for Interval<Long> and related operations

11a0168

Handle overflow/underflow for Sum and Product

451763e

Improve long/bigint support in ToQuantity and when guessing type spec…

5e4472f

…ifier

cmoesel force-pushed the cql-long branch from 9620bb8 to 5e4472f Compare May 27, 2026 13:09

This was referenced May 29, 2026

Improve handling of boundless and unknown intervals #365

Draft

Fix overlaps for boundless and unknown intervals #366

Merged

lmd59 reviewed May 31, 2026

View reviewed changes

Minor typos from code review

bb6e72c

Co-authored-by: lmd59 <lmd59@cornell.edu>

	define decimals_at_max_value_product: Product({99999999999999999999.99999999})
	define decimals_at_max_value_product: Product({99999999999999999999.99999999, 1.0})

	define decimals_at_min_value_product: Product({-99999999999999999999.99999999})
	define decimals_at_min_value_product: Product({-99999999999999999999.99999999, 1.0})

	define quantities_at_max_value_product: Product({99999999999999999999.99999999 'g'})
	define quantities_at_max_value_product: Product({99999999999999999999.99999999 'g', 1.0 'g'})

		@@ -833,6 +1005,7 @@ define DateTimeIntervalEndsStartsFalse: Interval[DateTime(2012, 1, 5), DateTime(

		// @Test: IntegerIntervalUnion

Conversation

cmoesel commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

lmd59 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cmoesel commented May 19, 2026

Uh oh!

cmoesel commented May 27, 2026

Uh oh!

lmd59 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cmoesel commented Jun 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cmoesel commented May 11, 2026 •

edited

Loading

codecov-commenter commented May 11, 2026 •

edited

Loading