Skip to content

Fix incorrect default param types for mssql, oracle, and sqlite #87

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rathboma wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix incorrect default param types for mssql, oracle, and sqlite #87

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
071946f
Fix incorrect default param types for mssql, oracle, and sqlite
rathboma Feb 26, 2026
9475da5
Add :name as a supported parameter style for mssql
rathboma Feb 26, 2026
d093958
Fix lint errors: rename short var and reformat single-element array
rathboma Feb 26, 2026
42e753b
Exclude DECLARE'd local variables from parameters
claude May 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1112,8 +1112,13 @@ export function defaultParamTypesFor(dialect: Dialect): ParamTypes {
numbered: ['$'],
};
case 'mssql':
return {
named: ['@', ':'],
};
case 'oracle':
return {
named: [':'],
numbered: [':'],
};
case 'bigquery':
return {
Expand All @@ -1125,7 +1130,7 @@ export function defaultParamTypesFor(dialect: Dialect): ParamTypes {
return {
positional: true,
numbered: ['?'],
named: [':', '@'],
named: [':', '@', '$'],
};
default:
return {
Expand Down
94 changes: 87 additions & 7 deletions test/identifier/single-statement.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -791,7 +791,7 @@ describe('identifier', () => {
text: query,
type: 'CREATE_FUNCTION',
executionType: 'MODIFICATION',
parameters: [],
parameters: ['@DATE', '@ISOweek'],

@MasterOdin MasterOdin Mar 5, 2026

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.

This seems like a bug to me, since they're not parameters that the user would need to fill in. We can at least detect variables within a function/body by checking if the preceding token is DECLARE, though I'm a bit of a loss for dealing with function parameters.

tables: [],
},
];
Expand Down Expand Up @@ -1445,18 +1445,18 @@ describe('identifier', () => {
});

it('Should extract named Parameters', () => {
const actual = identify('SELECT * FROM Persons where x = :one and y = :two and a = :one', {
const actual = identify('SELECT * FROM Persons where x = @one and y = @two and a = @one', {
dialect: 'mssql',
strict: true,
});
const expected = [
{
start: 0,
end: 61,
text: 'SELECT * FROM Persons where x = :one and y = :two and a = :one',
text: 'SELECT * FROM Persons where x = @one and y = @two and a = @one',
type: 'SELECT',
executionType: 'LISTING',
parameters: [':one', ':two'],
parameters: ['@one', '@two'],
tables: [],
},
];
Expand All @@ -1465,18 +1465,98 @@ describe('identifier', () => {
});

it('Should extract named Parameters with trailing commas', () => {
const actual = identify('SELECT * FROM Persons where x in (:one, :two, :three)', {
const actual = identify('SELECT * FROM Persons where x in (@one, @two, @three)', {
dialect: 'mssql',
strict: true,
});
const expected = [
{
start: 0,
end: 52,
text: 'SELECT * FROM Persons where x in (:one, :two, :three)',
text: 'SELECT * FROM Persons where x in (@one, @two, @three)',
type: 'SELECT',
executionType: 'LISTING',
parameters: ['@one', '@two', '@three'],
tables: [],
},
];

expect(actual).to.eql(expected);
});

it('Should extract mssql colon-prefixed named parameters', () => {
const actual = identify('SELECT * FROM Persons where x = :one and y = :two and a = :one', {
dialect: 'mssql',
strict: true,
});
const expected = [
{
start: 0,
end: 61,
text: 'SELECT * FROM Persons where x = :one and y = :two and a = :one',
type: 'SELECT',
executionType: 'LISTING',
parameters: [':one', ':two'],
tables: [],
},
];

expect(actual).to.eql(expected);
});

it('Should extract oracle named parameters', () => {
const actual = identify('SELECT * FROM persons WHERE id = :one AND status = :two', {
dialect: 'oracle',
strict: true,
});
const expected = [
{
start: 0,
end: 54,
text: 'SELECT * FROM persons WHERE id = :one AND status = :two',
type: 'SELECT',
executionType: 'LISTING',
parameters: [':one', ':two'],
tables: [],
},
];

expect(actual).to.eql(expected);
});

it('Should extract oracle numbered parameters', () => {
const actual = identify('SELECT * FROM persons WHERE id = :1 AND status = :2', {
dialect: 'oracle',
strict: true,
});
const expected = [
{
start: 0,
end: 50,
text: 'SELECT * FROM persons WHERE id = :1 AND status = :2',
type: 'SELECT',
executionType: 'LISTING',
parameters: [':1', ':2'],
tables: [],
},
];

expect(actual).to.eql(expected);
});

it('Should extract sqlite $name parameters', () => {
const actual = identify('SELECT * FROM persons WHERE id = $myid', {
dialect: 'sqlite',
strict: true,
});
const expected = [
{
start: 0,
end: 37,
text: 'SELECT * FROM persons WHERE id = $myid',
type: 'SELECT',
executionType: 'LISTING',
parameters: [':one', ':two', ':three'],
parameters: ['$myid'],
tables: [],
},
];
Expand Down
3 changes: 2 additions & 1 deletion test/index.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,8 @@ describe('Regression tests', () => {
{ strict: false, dialect: 'mssql' as Dialect },
);
result.forEach((res) => {
expect(res.parameters.length).to.equal(0);
// :: cast syntax should not produce colon-prefixed parameters
expect(res.parameters.every((param) => !param.startsWith(':'))).to.equal(true);

@MasterOdin MasterOdin Mar 5, 2026

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.

Feels like can rework the this test to then only do identify('SET @g = geometry::STGeomFromText('POLYGON((0 0, 2 0, 2 2, 0 2, 0 0))', 0);') if just checking that cast isn't parsed to a parameter.

However, similar to above, would be good to think about how we should handle these parameters here.

});
});

Expand Down
14 changes: 7 additions & 7 deletions test/parser/single-statements.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -804,7 +804,7 @@ describe('parser', () => {

it('should extract mssql parameters', () => {
const actual = parse(
'select x from a where x = :foo',
'select x from a where x = @foo',
true,
'mssql',
false,
Expand All @@ -826,13 +826,13 @@ describe('parser', () => {
},
{
type: 'parameter',
value: ':foo',
value: '@foo',
start: 26,
end: 29,
},
];
expect(actual.tokens).to.eql(expected);
expect(actual.body[0].parameters).to.eql([':foo']);
expect(actual.body[0].parameters).to.eql(['@foo']);
});

it('should not identify params in a comment', () => {
Expand Down Expand Up @@ -875,7 +875,7 @@ describe('parser', () => {

it('should extract multiple mssql parameters', () => {
const actual = parse(
'select x from a where x = :foo and y = :bar',
'select x from a where x = @foo and y = @bar',
true,
'mssql',
false,
Expand All @@ -897,7 +897,7 @@ describe('parser', () => {
},
{
type: 'parameter',
value: ':foo',
value: '@foo',
start: 26,
end: 29,
},
Expand All @@ -909,13 +909,13 @@ describe('parser', () => {
},
{
type: 'parameter',
value: ':bar',
value: '@bar',
start: 39,
end: 42,
},
];
expect(actual.tokens).to.eql(expected);
expect(actual.body[0].parameters).to.eql([':foo', ':bar']);
expect(actual.body[0].parameters).to.eql(['@foo', '@bar']);
});
});
});
Expand Down
52 changes: 47 additions & 5 deletions test/tokenizer/index.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ describe('scan', () => {
['?', 'generic'],
['?', 'mysql'],
['?', 'sqlite'],
[':', 'mssql'],
['@', 'mssql'],
].forEach(([ch, dialect]) => {
it(`scans just ${ch} as parameter for ${dialect}`, () => {
const input = `${ch}`;
Expand Down Expand Up @@ -322,10 +322,7 @@ describe('scan', () => {
expect(actual).to.eql(expected);
});
});
[
['$', 'psql'],
[':', 'mssql'],
].forEach(([ch, dialect]) => {
[['$', 'psql']].forEach(([ch, dialect]) => {
it(`should scan ${ch}1 for ${dialect}`, () => {
const input = `${ch}1`;
const actual = scanToken(
Expand Down Expand Up @@ -369,6 +366,24 @@ describe('scan', () => {
it('should not include trailing non-alphanumerics for mssql', () => {
const paramTypes = defaultParamTypesFor('mssql');
[
{
actual: scanToken(initState('@one,'), 'mssql', paramTypes),
expected: {
type: 'parameter',
value: '@one',
start: 0,
end: 3,
},
},
{
actual: scanToken(initState('@two)'), 'mssql', paramTypes),
expected: {
type: 'parameter',
value: '@two',
start: 0,
end: 3,
},
},
{
actual: scanToken(initState(':one,'), 'mssql', paramTypes),
expected: {
Expand All @@ -390,6 +405,33 @@ describe('scan', () => {
].forEach(({ actual, expected }) => expect(actual).to.eql(expected));
});

it('should not include trailing non-alphanumerics for oracle', () => {
const paramTypes = defaultParamTypesFor('oracle');
[
{
actual: scanToken(initState(':one,'), 'oracle', paramTypes),
expected: {
type: 'parameter',
value: ':one',
start: 0,
end: 3,
},
},
].forEach(({ actual, expected }) => expect(actual).to.eql(expected));
});

it('should recognize $name for sqlite', () => {
const paramTypes = defaultParamTypesFor('sqlite');
const actual = scanToken(initState('$myvar'), 'sqlite', paramTypes);
const expected = {
type: 'parameter',
value: '$myvar',
start: 0,
end: 5,
};
expect(actual).to.eql(expected);
});

describe('custom parameters', () => {
describe('positional parameters', () => {
const paramTypes = {
Expand Down