kysely icon indicating copy to clipboard operation
kysely copied to clipboard

SQL Server support

Open rexpan opened this issue 2 years ago • 2 comments

I am really impress by Kysely and your work. Any plan for SQL Server dialect?

rexpan avatar Dec 17 '21 13:12 rexpan

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.

koskimas avatar Dec 18 '21 08:12 koskimas

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" }

rexpan avatar Dec 18 '21 11:12 rexpan

Not sure about node.js / deno ecosystem, but according to some "popularity" statistics [1], the standings are:

  1. Oracle
  2. MySQL.
  3. SQL Server.
  4. Postgres.

This issue is open since 12.2021 and only has 4 upvotes, so my opinion leans towards "this should be a 3rd party package".

igalklebanov avatar Nov 01 '22 12:11 igalklebanov

DB-Engines uses Google trends, # tweets, # jobs, # questions on Stack Overflow...

Here some numbers from the 2022 Stack Overflow survey:

  • https://survey.stackoverflow.co/2022/#most-popular-technologies-database-prof

    image

  • https://survey.stackoverflow.co/2022/#most-loved-dreaded-and-wanted-database-want

    image

JetBrains 2022 survey:

  • https://www.jetbrains.com/lp/devecosystem-2022/databases/

    image

Edit: it seems to me that Microsoft SQL Server is on a slow decline when comparing with previous surveys

tkrotoff avatar Mar 26 '23 16:03 tkrotoff

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.

igalklebanov avatar Mar 27 '23 10:03 igalklebanov

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.

vicary avatar Jun 26 '23 10:06 vicary

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

LarsBuur avatar Jun 26 '23 14:06 LarsBuur

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?

RicardoValdovinos avatar Jul 17 '23 18:07 RicardoValdovinos

We're investigating adding mssql support, seeing if the experience will be good enough and what gaps we need to tackle. No promises.

igalklebanov avatar Jul 18 '23 08:07 igalklebanov

Really wanted to use node-mssql as the underlying driver for the dialect implementation (connection pools, result streaming, js to tedious data type mapping, etc.), but it's not 100% aligned with Kysely's API requirements (no single connection reservation). I've submitted https://github.com/tediousjs/node-mssql/issues/1517.

igalklebanov avatar Jul 27 '23 23:07 igalklebanov

@rexpan @LarsBuur How would I go about implementing the above example?

BStoller avatar Aug 04 '23 14:08 BStoller

@BStoller for now, you can copy the dialect implementation from #595, it's fully functional and ready for review. see the PR description for a list of things that are missing from Kysely.

igalklebanov avatar Aug 13 '23 12:08 igalklebanov

Just curious, is it possible for the new dialect to interact with tempdb tables at it's current state?

vicary avatar Aug 15 '23 18:08 vicary

@vicary no idea. try it and let us know.

igalklebanov avatar Aug 16 '23 13:08 igalklebanov

@igalklebanov I want to give it a try. Do I clone and build, then npm link?

vicary avatar Aug 17 '23 05:08 vicary

@vicary that'd work. you can also add a file, import straight from src, and run it with tsx.

igalklebanov avatar Aug 17 '23 05:08 igalklebanov

@igalklebanov It works!

Some notes for future users and/or upcoming docs, please correct me if I'm wrong.

  1. tempdb allows you to SELECT ... INTO #tempTable without creating a table structure, these tables are inherently dynamic and is difficult to type.
  2. Objects in tempdb will be deleted once the server session is gone. With the current implementation, it seems that they are deleted after each query execution, therefore you kinda have to use raw SQL at the moment.

This won't work

await 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 tarn to use 1 connection via { min: 1, max: 1 } in theory should keep your server session, but it still fails for some reason.

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 work

await sql`
  SELECT * INTO #tempTable FROM table1;
  SELECT * FROM #tempTable;
`.execute(db);

vicary avatar Aug 17 '23 07:08 vicary

@vicary Try this. You have to be in a transaction, or use a single connection for this to work. I couldn't get single # variant to work outside of single multi-query requests - which are a bad idea in general.

igalklebanov avatar Aug 29 '23 10:08 igalklebanov

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.

vicary avatar Aug 29 '23 17:08 vicary

@vicary Try this.

igalklebanov avatar Aug 29 '23 18:08 igalklebanov

@igalklebanov Got it.

  1. [x] Create ##test
  2. [x] Select ##test within the connection callback.
  3. [ ] Select ##test outside the connection callback.
  • Switching create table with select into behaves the same way.
  • Switching ##test with #test requires everything to live inside the same SQL.

We usually use local temp objects (# instead of ##) to prevent naming collision with other users.

vicary avatar Aug 29 '23 19:08 vicary

@vicary

We usually use local temp objects (# instead of ##) to prevent naming collision with other users.

Could simply suffixing the table name with a random id or a request/trace/user id work here?

igalklebanov avatar Sep 22 '23 10:09 igalklebanov

@igalklebanov Yeah that would work.

vicary avatar Sep 22 '23 11:09 vicary

@igalklebanov I wonder how does new driver handles columns with whitespaces.

  1. .select("Full Name") becomes SELECT "Full Name" would be correct
  2. But with table alias, either .select(`alias."Full Name"`) or .select("alias.[Full Name]") fails type checking. .select("alias.Full Name") passes typecheck but it's incorrect syntax in runtime.
  3. .orderBy("Full Name") would also fails an internal check Invalid order by direction: Name

Is it left for future PRs, or is there another way to use it?

vicary avatar Sep 25 '23 16:09 vicary

@vicary

Kysely automatically inserts " around every identifier for mssql. Kysely does not handle whitespaces. Kysely expects an as to come after them (or asc|desc in orderBy).

IMHO, we should not change the API for a bad practice and hurt everyone's compile performance. @koskimas wdyt? Don't use whitespaces in naming. 🤷

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 ["Full Name"] in code everywhere.

igalklebanov avatar Sep 25 '23 16:09 igalklebanov

@igalklebanov The generated statement from SELECT actually works fine, double quotes are placed perfectly even with whitespaces AND alias. e.g. .select("alias.Full Name") becomes SELECT "alias"."Full Name".

Frankly I'm not particularly fond of [Full Name] if "Full Name" also does the job.

I think the only issue is that order by parser does a simple split and checks the second part, https://github.com/kysely-org/kysely/blob/2c3603e18c91b474b03a4a1c71597fbc37593e95/src/parser/order-by-parser.ts#L81-L82

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?

Don't use whitespaces in naming. 🤷

Sadly, not my choice. On the bright side, you may call it a migration plan, away from those naming conventions.

vicary avatar Sep 25 '23 16:09 vicary

@vicary

Check out CamelCasePlugin. The whitespace plugin you would implement is quite similar.

igalklebanov avatar Sep 25 '23 17:09 igalklebanov