Skip to content

Add support for anonymous data tables in UPDATE, DELETE, and INSERT#2349

Merged
mathiasrw merged 13 commits intodevelopfrom
copilot/add-support-anonymous-data-tables
Dec 15, 2025
Merged

Add support for anonymous data tables in UPDATE, DELETE, and INSERT#2349
mathiasrw merged 13 commits intodevelopfrom
copilot/add-support-anonymous-data-tables

Conversation

Copy link
Contributor

Copilot AI commented Dec 13, 2025

  • Explore the codebase to understand how UPDATE, DELETE, and INSERT currently work
  • Understand how SELECT uses ParamValue for anonymous data
  • Modify parser to accept ParamValue in UPDATE statements
  • Modify parser to accept ParamValue in DELETE statements
  • Modify parser to accept ParamValue in INSERT statements
  • Implement runtime-only approach for ParamValue handling
  • Extract ParamValue logic into shared helper function
  • Align implementation with codebase patterns
  • Create comprehensive tests for all operations
  • Verify existing tests still pass
  • Follow AlaSQL callback conventions
Original prompt

This section details on the original issue you should resolve

<issue_title>Deal with anonymous data tables</issue_title>
<issue_description>Just like how we can use normal internal named tables for anything (created by create table abc) I would like to be able to operate on anonymous tables (provided via ?) for all the same operations.

This means I can select, insert, update all the things like alasql("Update ? set status = 'on' where type = 4", [mydata])</issue_description>

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


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 3 commits December 13, 2025 04:27
Co-authored-by: mathiasrw <1063454+mathiasrw@users.noreply.github.com>
Co-authored-by: mathiasrw <1063454+mathiasrw@users.noreply.github.com>
Co-authored-by: mathiasrw <1063454+mathiasrw@users.noreply.github.com>
Copilot AI changed the title [WIP] Add support for operations on anonymous data tables Add support for anonymous data tables in UPDATE, DELETE, and INSERT Dec 13, 2025
Copilot AI requested a review from mathiasrw December 13, 2025 04:37
Copy link
Member

@mathiasrw mathiasrw left a comment

Choose a reason for hiding this comment

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

This is really a lot of code to add. Are you sure this is the best way to solve this?

What if we simply, in these scenarios, move the data into a table and then just let it run, like making a shortcut basically creating a new internal table, yanking the data in, let it run like normal, as if you had imported it first and then delete the tmp table again?

Instead of adding complex logic to each statement's compile function,
use a simpler approach: create a temporary table, assign the param array
by reference, execute the statement on the temp table, and sync changes back.

This reduces code complexity significantly (~145 lines vs ~300+ lines)
while maintaining the same functionality.

Co-authored-by: mathiasrw <1063454+mathiasrw@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 13, 2025

This is really a lot of code to add. Are you sure this is the best way to solve this?

What if we simply, in these scenarios, move the data into a table and then just let it run, like making a shortcu...

Great suggestion! I've simplified the implementation to use the temporary table approach. Instead of adding complex logic to each statement's compile function, I now:

  1. Create a temporary table with a unique name
  2. Assign the parameter array directly to the table's data (by reference)
  3. Execute the statement on the temp table normally
  4. Sync any changes back to the original array (for DELETE which replaces the array)
  5. Clean up the temp table

This reduced the added code from ~300+ lines to ~145 lines while maintaining all functionality. The approach leverages existing table operations instead of reimplementing them for anonymous data.

Commit: d161bfd

Co-authored-by: mathiasrw <1063454+mathiasrw@users.noreply.github.com>
Instead of creating new statement objects and recursively calling compile(),
simply swap the table reference at the start of compile() and let the normal
logic run. Wrap the returned statement function to setup/cleanup temp table.

This eliminates the recursion and makes the code much cleaner and easier to
understand.

Co-authored-by: mathiasrw <1063454+mathiasrw@users.noreply.github.com>
- Created wrapParamValueStatement helper to eliminate code duplication
- Renamed test942.js to test2348.js to match issue number
- Reduced wrapper code from ~30 lines x 3 files to single function call
- All tests passing

Co-authored-by: mathiasrw <1063454+mathiasrw@users.noreply.github.com>
Added catch blocks that re-throw errors after cleanup to ensure proper
error handling flow. While try-finally works without catch, having an
explicit catch makes the error handling intent clearer and follows
best practices for resource cleanup patterns.

Co-authored-by: mathiasrw <1063454+mathiasrw@users.noreply.github.com>
- Added callback error handling in catch blocks for all three operations
- Updated all test assertions to use deepEqual for complete output comparison
- This ensures proper error propagation to callbacks when provided
- Tests now validate the entire resulting data structure

Co-authored-by: mathiasrw <1063454+mathiasrw@users.noreply.github.com>
Changed error callback from cb(err) to cb(null, err) with return,
following the AlaSQL callback convention where the first parameter
is data and the second is error. This matches the pattern used
throughout the codebase.

Co-authored-by: mathiasrw <1063454+mathiasrw@users.noreply.github.com>
@mathiasrw
Copy link
Member

@copilot

Please review this solution andsee if we can do a better implementation using les code and if we are adapting the library that is aligned with the nature of the rest of the code.

Replaced complex wrapper approach with simpler runtime-only solution:
- Removed 45-line wrapParamValueStatement helper (was duplicated 3x)
- Handle ParamValue directly in compile() with early return
- Create temp table only at runtime, not compile time
- Reduced from ~240 added lines to ~120 lines
- More aligned with codebase patterns (similar to FROM ParamValue)
- No compile-time table creation overhead
- Cleaner, more maintainable code

Co-authored-by: mathiasrw <1063454+mathiasrw@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 14, 2025

@copilot

Please review this solution andsee if we can do a better implementation using les code and if we are adapting the library that is aligned with the nature of the rest of the code.

Simplified the implementation significantly:

Previous approach (removed):

  • 45-line wrapParamValueStatement helper function duplicated 3 times
  • Created temp tables at compile time
  • Complex wrapper with multiple responsibilities
  • ~240 lines of added code

New approach:

  • Direct ParamValue handling in compile() with early return
  • Temp tables created only at runtime when needed
  • Simple, focused implementation (40 lines per file)
  • ~120 lines of added code (50% reduction)
  • Better aligned with codebase patterns (similar to how FROM handles ParamValue)

The new implementation is cleaner, more maintainable, and follows the library's existing patterns.

Commit: 4592c3c

Eliminated code duplication across INSERT, DELETE, and UPDATE:
- Added yy.compileParamValue helper function in 28yy.js
- Reduced each file from ~40 lines to 1 line for ParamValue handling
- Total reduction: ~115 lines of duplicated code removed
- All tests passing

Co-authored-by: mathiasrw <1063454+mathiasrw@users.noreply.github.com>
@mathiasrw mathiasrw marked this pull request as ready for review December 15, 2025 04:29
@mathiasrw mathiasrw merged commit e01badd into develop Dec 15, 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.

Deal with anonymous data tables

2 participants