Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 2, 2025

Description

Fixes an issue where RECORDSET OF SELECT t.*, 1 as rn FROM ? t only returned explicit columns (like rn) in the columns array, missing the columns expanded from the wildcard (a, b).

Root Cause

When SELECT * is used with dynamic data sources (parameterized queries), the compileSelectStar function sets query.dirtyColumns = true but doesn't add columns to query.columns because column names aren't known at compile time. The dirtyColumns flag indicates that column metadata needs to be dynamically determined from the data itself rather than being known at compile time. Later, when explicit columns like 1 as rn are processed, they ARE added to query.columns. This resulted in only the explicit columns being included in the final columns array.

Changes Made

  • Modified src/40select.js in the modify function to check if query.dirtyColumns is true
  • When true, the function now:
    1. Scans the result data to extract all column names
    2. Merges these with any existing columns in query.columns (from explicit column expressions)
    3. Avoids duplicates by tracking which columns are already defined
  • Added comprehensive test suite in test/test2070.js with 5 test cases covering various scenarios
  • Applied prettier code formatting to maintain consistency with project standards

Code Quality Improvements

  • Restructured conditional logic to check for null/undefined first for better readability
  • Changed all equality comparisons to use strict equality (===) for consistency
  • Applied prettier formatting for consistent code style across all modified files
  • Refactored test assertions to use assert.deepEqual for complete output validation instead of multiple individual assertions

Testing

  • ✅ Added test2070.js (renamed from test2201.js to match issue number) with 5 comprehensive test cases:
    1. SELECT t.*, additional_column - the original issue scenario
    2. SELECT *, additional_column - without table alias
    3. SELECT t.* - wildcard only (regression test)
    4. Explicit columns without wildcard (regression test)
    5. Multiple additional columns with wildcard
  • ✅ All new tests pass with assert.deepEqual validating complete expected output
  • ✅ All existing RECORDSET tests pass (test 349, 380)
  • ✅ Full test suite passes with 564 tests (no new failures)
  • ✅ Code review completed and feedback addressed
  • ✅ Security scan completed (CodeQL) - 0 vulnerabilities found
  • ✅ Code formatting applied with prettier - all files pass formatting checks
Original prompt

This section details on the original issue you should resolve

<issue_title>Incorrect result returned when using "RECORDSET OF" to retrieve columns</issue_title>
<issue_description>When adding other computer column after t.*, the columns in t.* cannot be obtained correctly
Example

var data = [{a:1,b:10}, {a:2,b:20}, {a:1,b:30}];
var res = alasql('RECORDSET OF SELECT t.*, 1 as rn FROM ? t',[data]);   

Result

{
  "columns": [
    {
      "columnid": "rn"
    }
  ],
  "data": [
    {
      "rn": 1,
      "a": 1,
      "b": 10
    },
    {
      "rn": 1,
      "a": 2,
      "b": 20
    },
    {
      "rn": 1,
      "a": 1,
      "b": 30
    }
  ]
}

columns only contains "rn" and "a, b" is missing</issue_description>

<agent_instructions>I think this has been solved in another PR. Can you please verify?</agent_instructions>

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


💡 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
Copy link
Member

@copilot Continue

Copy link
Contributor Author

Copilot AI commented Dec 2, 2025

@copilot Continue

The fix is complete and ready for review. All tasks in the checklist have been finished:

✅ Issue reproduced and root cause identified
✅ Fix implemented in src/40select.js
✅ Comprehensive test suite added (test/test2201.js with 5 test cases)
✅ All existing tests pass (564 passing, no new failures)
✅ Code review feedback addressed
✅ Security scan completed (0 vulnerabilities)

The fix correctly handles the case where RECORDSET OF SELECT t.*, 1 as rn FROM ? t now returns all columns (a, b, rn) in the columns array instead of just (rn).

Copilot AI changed the title [WIP] Fix incorrect result returned for RECORDSET OF query Fix incorrect result returned for RECORDSET OF query Dec 2, 2025
Copilot finished work on behalf of mathiasrw December 2, 2025 23:26
@mathiasrw mathiasrw marked this pull request as ready for review December 2, 2025 23:38
@mathiasrw
Copy link
Member

@copilot

Run yarn test-format-all
yarn run v1.22.22
$ prettier --list-different '{.,src,test}/*.{js,ts,json}' || (echo 'Please correct file formatting using yarn format-all and try again.' && exit 1)
src/40select.js
test/test2201.js
Please correct file formatting using yarn format-all and try again.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
Error: Process completed with exit code 1.

Copy link
Contributor Author

Copilot AI commented Dec 2, 2025

@copilot

Run yarn test-format-all
...

Code formatting applied with prettier in commit 24508ac. All files now pass the formatting check.

Copilot finished work on behalf of mathiasrw December 2, 2025 23:45
Copilot AI requested a review from mathiasrw December 2, 2025 23:45
@mathiasrw
Copy link
Member

@copilot continue

Copy link
Contributor Author

Copilot AI commented Dec 4, 2025

@copilot continue

Completed the requested changes in commit 5cf3dc5:

  • Renamed test file from test/test2201.js to test/test2070.js to match issue number
  • Refactored all test cases to use assert.deepEqual with complete expected output instead of multiple individual assertions
  • All tests pass and formatting is correct

Copilot finished work on behalf of mathiasrw December 4, 2025 04:04
Copilot AI requested a review from mathiasrw December 6, 2025 13:11
Copilot finished work on behalf of mathiasrw December 6, 2025 13:11
@mathiasrw mathiasrw merged commit 7020237 into develop Dec 6, 2025
10 checks passed
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.

Incorrect result returned when using "RECORDSET OF" to retrieve columns

2 participants