Conversation
|
Formatting check succeeded! |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1663 +/- ##
============================================
+ Coverage 65.82% 65.88% +0.06%
Complexity 1645 1645
============================================
Files 474 477 +3
Lines 27454 27496 +42
Branches 5463 5467 +4
============================================
+ Hits 18071 18117 +46
+ Misses 7085 7083 -2
+ Partials 2298 2296 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
| cqlEngine: CqlEngine, | ||
| expression: String, | ||
| initialContext: Pair<String, Any>?, | ||
| initialContext: Pair<String, Any?>?, |
There was a problem hiding this comment.
question: This looks incorrect to me. What are the cases where you might have a context specified and set as null?
There was a problem hiding this comment.
Depends on whether we want to allow something like "Unfiltered" to null for the context-value pair. Doing so is equivalent to not providing the pair at all in which case the State.contextValues map won't have the "Unfiltered" key and the lookup will return null anyway.
| context.getCurrentLibrary()?.identifier, | ||
| expression, | ||
| ) | ||
| if (exception.backtrace == null) { |
There was a problem hiding this comment.
question: What stops us from building the exception with the correct backtrace attached from the get-go?
There was a problem hiding this comment.
If it is to be attached always at the very beginning, we'll need to make the engine state or State.stack (the activation frame stack) available everywhere a CqlException or its subclass (InvalidCast, TerminologyProviderException, DataProviderException, etc.) is instantiated. TerminologyProviderException and DataProviderException are part of public provider APIs so they aren't concerned with the engine state. An alternative would be to extend or create a new CqlException when it enters the engine scope and that's what it effectively does currently (and all in one place).
| """ | ||
| .trimIndent(), | ||
| backtrace.toString(), | ||
| ) |
There was a problem hiding this comment.
praise: Nice cleanup / simplification!
TraceFrameclass in both evaluation traces and backtraces.Pair<String, Any?>.