Makes SqlException.errorContext non-nullable, resolving Issue #616.

[vgapeyev]

Relevant Issues

• Closes Issue Change errorContextArg of SqlException be to non-nullable. #616

Description

This is a simple PR: There was an in-code TODO at SqlException, pointing at Issue #616.
The suggestion was to change the field to non-nullable, switching from null to an empty PropertyValueMap as the declared default. It turned out that many use sites have already provided non-null actuals, so even a default is not necessary in a few places. The only subclass that had use sites with null actuals (coming through the default) was EvaluationException, so its default did get changed rather than removed.

Other Information

• Updated Unreleased Section in CHANGELOG: [NO]
  • This is an internal change.
• Any backward-incompatible changes? [NO]
• Any new external dependencies? [NO]

License Information

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[codecov-commenter]

Codecov Report

Base: 79.70% // Head: 79.69% // Decreases project coverage by -0.01% ⚠️

Coverage data is based on head (89dacc2) compared to base (8255a07).
Patch coverage: 31.25% of modified lines in pull request are covered.

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #847      +/-   ##
============================================
- Coverage     79.70%   79.69%   -0.02%     
+ Complexity     2658     2657       -1     
============================================
  Files           269      269              
  Lines         21825    21823       -2     
  Branches       3936     3936              
============================================
- Hits          17396    17391       -5     
- Misses         3231     3234       +3     
  Partials       1198     1198              

Flag	Coverage Δ
CLI	22.56% <ø> (ø)
EXAMPLES	78.63% <ø> (ø)
EXTENSIONS	69.23% <ø> (ø)
LANG	81.36% <31.25%> (-0.02%)	⬇️
PARTIQL_PTS	∅ <ø> (∅)
PARTIQL_TEST_SCRIPT	80.90% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...c/org/partiql/lang/ast/passes/SemanticException.kt	100.00% <ø> (ø)
...ng/src/org/partiql/lang/eval/EvaluatingCompiler.kt	80.76% <0.00%> (-0.35%)	⬇️
...iql/lang/eval/physical/PhysicalPlanCompilerImpl.kt	83.14% <0.00%> (ø)
...l/physical/operators/SortOperatorFactoryDefault.kt	83.33% <0.00%> (+3.33%)	⬆️
lang/src/org/partiql/lang/eval/Exceptions.kt	77.96% <66.66%> (ø)
lang/src/org/partiql/lang/SqlException.kt	75.00% <100.00%> (-0.87%)	⬇️
...partiql/lang/eval/builtins/MakeDateExprFunction.kt	60.00% <100.00%> (ø)
...partiql/lang/eval/builtins/MakeTimeExprFunction.kt	96.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[RCHowell]

Looks good to me, but I'm curious why the partiql-tests git module was included? Perhaps you can do git submodule update --init and update the PR?

[vgapeyev]

@RCHowell, Not sure what is going on with the submodule. If it has been introduced recently, I must have missed the memo on how to deal with it.

Here is what happened on my side, if anyone can do the forensics: I started this PR off main, before 55de098 came in (which had a change in test/partiql-tests). After that, I pulled myself up on main, branched and committed. Looking at the history, this commit reverted what 55de098 did in test/partiql-tests. Why? -- someone with better understanding of git is needed. With "regular" files the above workflow did not give me grief before.

At this point, the suggested git submodule update --init did not have a detectable effect.
I think I'll just revert the revert that I see in test/partiql-tests and hope for the better.

[vgapeyev]

Well git restore --source 55de0988 test/partiql-tests did not have a visible effect either.

@alancai98 do you have any suggestions?

Otherwise, not understanding what is gong on, I'd just cancel this PR and do a new one with the same delta.

[alancai98]

Well git restore --source 55de0988 test/partiql-tests did not have a visible effect either.
@alancai98 do you have any suggestions?
Otherwise, not understanding what is gong on, I'd just cancel this PR and do a new one with the same delta.

I'm not sure why Robert's suggested command doesn't work to update the git submodule. An alternative is to manually go to the submodule and pull in the latest changes: i.e.

1. cd test/partiql-tests
2. git pull

Alternatively from the root directory, I think you can run git submodule foreach git pull origin main to have each submodule update with what's in main.

After the submodule is updated, can go about the usual git commit commands.

[vgapeyev]

@RCHowell , @alancai98 thanks for the catch and for the assistance. The diff looks as expected now.