Skip to content

Add detection for outer DECLARE statements and update documentation#40

Open
max-ostapenko wants to merge 1 commit intomainfrom
friendly-peafowl
Open

Add detection for outer DECLARE statements and update documentation#40
max-ostapenko wants to merge 1 commit intomainfrom
friendly-peafowl

Conversation

@max-ostapenko
Copy link
Contributor

  • Implemented hasOuterDeclare function to check for outer level DECLARE statements in SQL.
  • Updated autoAssignActions to skip operations with outer DECLARE.
  • Enhanced README and Copilot instructions to clarify limitations of automated assignment.
  • Added tests for various scenarios involving outer DECLARE statements.

- Implemented `hasOuterDeclare` function to check for outer level DECLARE statements in SQL.
- Updated `autoAssignActions` to skip operations with outer DECLARE.
- Enhanced README and Copilot instructions to clarify limitations of automated assignment.
- Added tests for various scenarios involving outer DECLARE statements.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds detection for outer-level DECLARE statements in SQL operations to prevent syntax errors when prepending reservation SET statements. BigQuery requires DECLARE to be the first statement in a script, so prepending SET @@reservation before an outer DECLARE would cause the query to fail. The PR implements automatic detection and skipping of such operations.

Changes:

  • Added hasOuterDeclare() function to detect DECLARE statements at the outer level of SQL queries, stripping comments to find the first real statement
  • Updated applyReservationToAction() to skip reservation assignment for operations with outer DECLARE
  • Added comprehensive test coverage for various DECLARE scenarios (mixed case, inside BEGIN...END blocks, inside EXECUTE IMMEDIATE, after comments, in arrays)
  • Updated README and Copilot instructions to document this limitation of automated assignment

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
index.js Implemented hasOuterDeclare() function and integrated DECLARE detection into reservation assignment logic
test/index.test.js Added 5 test cases covering DECLARE detection in various scenarios
README.md Added "Limitations of Automated Assignment" section documenting DECLARE restriction
.github/copilot-instructions.md Added section 5 documenting outer DECLARE detection logic
Comments suppressed due to low confidence (1)

index.js:204

  • The DECLARE detection doesn't handle cases where queries is passed as a function that returns SQL with an outer DECLARE statement. The check on line 191 only evaluates the function object itself (which will stringify to something like "function (ctx) { ... }"), not the SQL it returns.

When the function is executed later (lines 196-204), the result could contain an outer DECLARE, which would cause a syntax error since the reservation SET statement would be prepended before it. Consider either checking the function's return value at execution time, or documenting this as a known limitation requiring manual assignment.

        // Check for outer DECLARE before wrapping
        if (hasOuterDeclare(queries)) {
          return originalQueriesFn.apply(this, [queries])
        }

        if (typeof queries === 'function') {
          queriesArray = (ctx) => {
            const result = queries(ctx)
            if (typeof result === 'string') {
              return [statement, result]
            } else if (Array.isArray(result)) {
              return [statement, ...result]
            }
            return result
          }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants