Repository navigation
Conversation
|
@copilot continue |
Addressed in |
|
@copilot the inital comment indicates that some tasks are not completed. Can you please confirm? |
The original checklist is stale. The regression test, SELECT LET support, targeted validation, secret scan, and final review/security checks were completed in |
| try { | ||
| if (query.wherefn(scope, query.params, alasql)) { | ||
| // If there is a GROUP BY then pipe to grouping function | ||
| if (query.groupfn) { | ||
| query.groupfn(scope, query.params, alasql); | ||
| } else { | ||
| query.data.push(query.selectfn(scope, query.params, alasql)); | ||
| } | ||
| } | ||
| } finally { | ||
| query.params = originalParams; |
There was a problem hiding this comment.
Are we expecting the commands to error? What is the concern? There is a performance hit on this BUT worse, it seems like we dont trust our data structure of our logic.
There was a problem hiding this comment.
Updated in ee4e5e7: LET now restores only the touched variables after evaluation instead of cloning/swapping the whole params object.
| .map(function (assignment) { | ||
| return ( | ||
| assignment.method + assignment.variable + ' = ' + assignment.expression.toString() | ||
| ); | ||
| }) |
There was a problem hiding this comment.
Is this the most efficient way?
There was a problem hiding this comment.
Tightened in ee4e5e7 to a small shared formatter helper.
There was a problem hiding this comment.
We need many more exampled and edgecases covered.
There was a problem hiding this comment.
Expanded in ee4e5e7 with coverage for filtering/selection, restoring existing named params, GROUP BY/HAVING, and cleanup of temporary @ variables.
Co-authored-by: mathiasrw <1063454+mathiasrw@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.