Skip to content

fix: treat SELECT ... INTO as a modification#95

Open
rathboma wants to merge 1 commit into
mainfrom
fix/select-into-execution-type
Open

fix: treat SELECT ... INTO as a modification#95
rathboma wants to merge 1 commit into
mainfrom
fix/select-into-execution-type

Conversation

@rathboma
Copy link
Copy Markdown
Contributor

Problem

identify("SELECT * INTO public.\"MyTable1\" FROM public.\"MyTable2\"") reports executionType: "LISTING", but SELECT ... INTO creates and populates a new table — it modifies the database.

The parser derives executionType purely from the statement type, and SELECT maps to LISTING.

Fixes #81.

Fix

When a SELECT statement contains an INTO clause, set its executionType to MODIFICATION. This is gated to the dialects where SELECT INTO builds a table — generic, mssql, psql. Oracle and MySQL use SELECT INTO to assign values to variables rather than create a table, so they keep the LISTING classification (and the existing Oracle PL/SQL block tests are unaffected).

Tests

Added a SELECT ... INTO describe block to test/identifier/single-statement.spec.ts:

  • SELECT ... INTOMODIFICATION (generic — the exact case from SELECT INTO statement executionType is LISTING #81)
  • SELECT ... INTOMODIFICATION (mssql)
  • SELECT ... INTOMODIFICATION (psql)
  • a plain SELECT still resolves to LISTING
  • an Oracle SELECT ... INTO (variable assignment) stays LISTING

The three MODIFICATION assertions fail without the parser change; the full suite passes with it.

`SELECT ... INTO target` creates and populates a new table, so it
modifies the database. The parser reported executionType `LISTING` for
it because the statement type is `SELECT`.

Flag any `SELECT` statement that contains an `INTO` clause as
`MODIFICATION` for the dialects where `SELECT INTO` builds a table
(generic, mssql, psql). Oracle and MySQL use `SELECT INTO` to assign
variables, so they keep the `LISTING` classification.

Closes #81
Comment on lines +117 to +121
it('should still identify a plain "SELECT" as a listing', () => {
const actual = identify('SELECT * FROM Persons');

expect(actual[0].executionType).to.eql('LISTING');
});
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think we need this test as it's adequately covered by a bunch of other tests, if not explicitly so.

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.

SELECT INTO statement executionType is LISTING

2 participants