Fix UNION ALL with subquery in NOT IN clause

[Copilot]

Fix for UNION ALL with Subquery Issue

This PR fixes the issue where UNION ALL fails when the second SELECT contains a subquery in the NOT IN clause.

Changes Completed

• Confirm the issue exists - UNION ALL fails with subquery in NOT IN clause
• Investigate the root cause in the source code
• Implement parser-based fix for the issue
• Refactor to use cleaner stack pattern based on code review feedback
• Create proper test case following repository conventions (test2420.js)
• Verify the fix resolves the problem
• Run existing tests to ensure no regressions
• Run code review and security checks

Issue Summary

Original Issue: UNION ALL fails when the second SELECT contains a subquery in the NOT IN clause.

Status: ✅ FIXED

Both test cases from the original issue now work correctly:

let d = [{d:1}];

// Now works (baseline test)
alasql("select d from test where d in (select 1) union all select d from test where d not in (2)");
// Result: [{d: 1}, {d: 1}] ✅

// Now works (was failing)
alasql("select d from test where d in (select 1) union all select d from test where d not in (select 2)");
// Result: [{d: 1}, {d: 1}] ✅

Root Cause

The parser wasn't properly distributing the queries array to nested SELECTs in UNION ALL constructs:

1. During parsing, subqueries are added to a global yy.queries array
2. The AStatement production attaches this array to the top-level SELECT
3. Nested SELECTs in UNION ALL didn't receive their portion of the queries
4. During execution, this.queriesfn[queriesidx] fails because queriesfn is undefined

Solution Implemented

Modified the parser grammar (src/alasqlparser.jison) to properly handle queries in UNION constructs using a stack pattern:

1. UnionOp Production: When encountering UNION/EXCEPT/INTERSECT operators:

   • Push current yy.queries array onto yy.queriesStack
   • Reset yy.queries to empty array for the nested SELECT

2. UnionableSelect Production: After parsing the nested SELECT:

   • Assign collected queries to the nested SELECT
   • Restore outer SELECT's queries by popping from the stack

This ensures each SELECT in a UNION construct maintains its own queries array, allowing subqueries in both parts of the UNION to work correctly.

Refactoring Based on Code Review

The initial implementation used yy.savedQueriesForUnion with logic spread across multiple productions. After code review feedback, the solution was refactored to:

• Use clearer naming: yy.queriesStack instead of yy.savedQueriesForUnion
• Consolidate query handling in just two productions (UnionOp and UnionableSelect)
• Remove complex logic from UnionClause
• Add clear comments explaining the stack-based approach

Test Coverage

Created test/test2420.js with 4 comprehensive test cases:

• ✅ UNION ALL with literal in NOT IN (baseline)
• ✅ UNION ALL with subquery in NOT IN (main fix)
• ✅ UNION ALL with multiple subqueries
• ✅ UNION with subquery (not UNION ALL)

All existing UNION tests continue to pass (test017, test198, test1684).

Code Quality

• ✅ Code review completed - refactored based on feedback
• ✅ Security scan completed - 0 alerts
• ✅ All tests passing (4/4 new tests, all existing UNION tests)

Original prompt

---

This section details on the original issue you should resolve

<issue_title>UNION ALL does not work with sub query</issue_title>
<issue_description>I have issues with UNION ALL. I have condensed an example

let d = [{d:1}];

// This works
alasql("create table test");
alasql("select * into test from ?", [d]);
alasql("select d from test where d in (select 1) union all select d from test where d not in (2)");

// This does not work
alasql("create table test");
alasql("select * into test from ?", [d]);
alasql("select d from test where d in (select 1) union all select d from test where d not in (select 2)");

</issue_description>

<agent_instructions>I think this is resolved by now. Can you please confirm?</agent_instructions>

Comments on the Issue (you are @copilot in this section)

• Fixes UNION ALL does not work with sub query #1146

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[mathiasrw]

This seems to be a duplicate of #2417

Pausing for now.

[mathiasrw]

@copilot Continue

[Copilot]

@copilot Continue

I've investigated the issue and confirmed it still exists after the merge with PR #2417. The problem is that when UNION ALL is used with a subquery in the NOT IN clause of the second SELECT, the parser doesn't properly distribute the queries array to the nested SELECT, causing the compiled query to fail with "Cannot read properties of undefined".

I've attempted several approaches to fix this in the parser grammar, but due to the timing of when grammar actions execute versus when subqueries are added to the global yy.queries array, a clean fix requires significant refactoring of how queries are tracked and distributed during parsing.

Would you like me to:

1. Continue working on a parser-based fix (may require extensive changes)
2. Look for a simpler runtime-based workaround in the query execution code
3. Close this as a known limitation that requires a larger refactoring effort

The issue is confirmed and reproducible - it's not yet resolved despite PR #2417.

[mathiasrw]

@copilot Please continue impelemnting it the right way i the parser.

I propose you have a look at how c4b46f3 added the ability to use parenthesized selects and order by.

[Copilot]

@copilot Please continue impelemnting it the right way i the parser.

I propose you have a look at how c4b46f3 added the ability to use parenthesized selects and or...

Fixed in commit 1bd25f6. I implemented a parser-based solution by modifying the grammar to save and restore the yy.queries array when encountering UNION operators. This ensures each SELECT in a UNION construct maintains its own queries array.

The fix handles all UNION variants (UNION, UNION ALL, EXCEPT, INTERSECT) and includes comprehensive test coverage in test/test2420.js. All 4 new tests pass and existing UNION tests continue to work.

[mathiasrw]

This seems like a very scetchy way of solving this. Can you please review and see if we can find a more elegang way to incooporate this into the grammar? is there someting we need to fix at a higher level of abstraction to not have all the `if yy.savedqueriesFotUnion) kind of notation?

[Copilot]

Refactored in commit 7d3b342. The solution now uses a cleaner stack pattern:

• Renamed to yy.queriesStack (more descriptive)
• Consolidated logic in just 2 productions (UnionOp and UnionableSelect)
• Removed complex conditional logic from UnionClause
• Added clear comments explaining the approach

The repetition in UnionOp is unavoidable in parser grammars where each production needs its action code, but the overall pattern is now much cleaner and follows the same stack-based approach used elsewhere in the grammar.