-
Notifications
You must be signed in to change notification settings - Fork 283
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.
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
SQL Server support #38
Comments
Thank you! I don't know how popular SQL Server is compared to MySQL and PostgreSQL. If this gets enough upvotes I'll add the dialect. The problem with SQL Server is that it's so different from all the others. There's so many non-standard keywords, statements and other stuff that we'd need to add a huge amount of SQL Server specific methods to the query builders. And to be honest, I have very little experience in using SQL Server. |
Yeah, I know the pain. My current attempt for create a SQL Server dialect that currently serve my purpose. It need more update but currently enough to compile the queries of my use case. class MssqlQueryCompiler extends DefaultQueryCompiler {
protected override getCurrentParameterPlaceholder() {
return '@' + this.numParameters;
}
protected override getLeftIdentifierWrapper(): string {
return '"';
}
protected override getRightIdentifierWrapper(): string {
return '"';
}
protected override visitOffset(node: OffsetNode): void {
if (this.parentNode != null && isSelectQueryNode(this.parentNode) && this.parentNode.limit != null) return; // will be handle when visitLimit
this.append(' OFFSET ');
this.visitNode(node.offset);
this.append(' ROWS ');
}
protected override visitLimit(node: LimitNode): void {
if (this.parentNode != null && isSelectQueryNode(this.parentNode)) {
if (this.parentNode.offset != null) {
this.append(' OFFSET ');
this.visitNode(this.parentNode.offset.offset);
this.append(' ROWS ');
} else this.append(' OFFSET 0 ROWS ');
}
this.append(' FETCH NEXT ');
this.visitNode(node.limit);
this.append(' ROWS ONLY ');
}
}
function isSelectQueryNode(node: OperationNode): node is SelectQueryNode { return node.kind == "SelectQueryNode" } |
Not sure about node.js / deno ecosystem, but according to some "popularity" statistics [1], the standings are:
This issue is open since 12.2021 and only has 4 upvotes, so my opinion leans towards "this should be a 3rd party package". |
DB-Engines uses Google trends, # tweets, # jobs, # questions on Stack Overflow... Here some numbers from the 2022 Stack Overflow survey:
JetBrains 2022 survey: Edit: it seems to me that Microsoft SQL Server is on a slow decline when comparing with previous surveys |
Thank you! Sadly we can't even tell how many responders are using typescript. Gut feeling tells me its mostly C# & Java codebases. |
Our use cases are mostly corporate revamps that requires incremental adoption of Node.js, large companies usually don't have the luxury and agility to completely move away from existing stacks. We are lucky to strike deals with 90% of edge compatible stacks, but they usually ends up requiring at least one connection to their existing infrastructure such as SQL Server. We really want to use Kysely in these projects for it's small bundle size and performance when compared with TypeORM. |
Many legacy systems still uses Microsoft SQL Server. I too would very much like Kysely support. I will try to see how far I get with @rexpan´s attempt above |
I'm in a similar situation to vicary above. Kysely truly seems like the best option for our use case. @LarsBuur Do you have a repository we can check out and contribute to? |
We're investigating adding mssql support, seeing if the experience will be good enough and what gaps we need to tackle. No promises. |
Really wanted to use |
Just curious, is it possible for the new dialect to interact with |
@vicary no idea. try it and let us know. |
@igalklebanov I want to give it a try. Do I clone and build, then |
@vicary that'd work. you can also add a file, import straight from src, and run it with |
@igalklebanov It works! Some notes for future users and/or upcoming docs, please correct me if I'm wrong.
This won't workawait sql`SELECT * INTO #tempTable FROM table1`.execute(db);
await sql`SELECT * FROM #tempTable`.execute(db); // Invalid object name '#tempTable'. Therefore using the normal query builder API is close to impossible. await db.insertInto("#tempTable")
.columns(...)
.expression(
db.selectFrom("table1").select(...)
)
.execute();
await sql`SELECT * FROM #tempTable`.execute(db); // Invalid object name '#tempTable'. Forcing Wrapping it in a transaction is not enough to force it to use the same session. await db.transaction().execute(async (manager) => {
await sql`SELECT * INTO #tempTable FROM table1`.execute(manager);
return await sql`SELECT * FROM #tempTable`.execute(manager);
}); // Invalid object name '#tempTable'. This will workawait sql`
SELECT * INTO #tempTable FROM table1;
SELECT * FROM #tempTable;
`.execute(db); |
Sorry I don't know how to derive the commit hash from the diff URL. I pulled the latest changes from PR instead, and the following code fails the same way. // tarn: { min: 1, max: 1 }
await db.transaction().execute(async (manager) => {
await sql`SELECT TOP 1 * INTO #tempTable FROM Table1`.execute(manager);
return await sql`SELECT * FROM #tempTable`.execute(manager); // Invalid object name '#tempTable'.
}); Running multiple statements in the same request still works, and that's the only way it works. await sql`
SELECT TOP 1 * INTO #tempTable FROM Table1;
SELECT * FROM #tempTable;
`.execute(db); Although I want the first one to work, I can live without it. At least there is a way now. |
@igalklebanov Got it.
We usually use local temp objects ( |
Could simply suffixing the table name with a random id or a request/trace/user id work here? |
@igalklebanov Yeah that would work. |
@igalklebanov I wonder how does new driver handles columns with whitespaces.
Is it left for future PRs, or is there another way to use it? |
Kysely automatically inserts IMHO, we should not change the API for a bad practice and hurt everyone's compile performance. @koskimas wdyt? I think a plugin could help here. Types would have underscores instead of whitespaces. The plugin would replace underscores with whitespaces and back. This'll improve your DX in TypeScript land, not having to |
@igalklebanov The generated statement from SELECT actually works fine, double quotes are placed perfectly even with whitespaces AND alias. e.g. Frankly I'm not particularly fond of I think the only issue is that order by parser does a simple split and checks the second part, kysely/src/parser/order-by-parser.ts Lines 81 to 82 in 2c3603e
If it is possible to let it slip, theoretically we should have no performance hit. I am not familiar with the plugin API yet, is it capable of doing this?
Sadly, not my choice. On the bright side, you may call it a migration plan, away from those naming conventions. |
Check out CamelCasePlugin. The whitespace plugin you would implement is quite similar. |
I am really impress by Kysely and your work. Any plan for SQL Server dialect?
The text was updated successfully, but these errors were encountered: