Skip to content
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

Mssql docs #724

Merged
merged 17 commits into from
Dec 31, 2023
Merged

Mssql docs #724

merged 17 commits into from
Dec 31, 2023

Conversation

gittgott
Copy link
Contributor

@gittgott gittgott commented Oct 6, 2023

closes #713

I couldn't get deno or bun to work with the dialect. Deno was having issues connecting to the database with the message Error: Failed to connect to localhost:1433 - tlssock._start is not a function. Bun was having issues with tarn with the message Failed to connect to localhost:1433 - socket must be an instance of net.Socket.

Since this wasn't working for me, I stated that bun and deno didn't support the mssql dialect in the docs. Just want to make sure that they aren't currently known to work.

Thanks!

@vercel
Copy link

vercel bot commented Oct 6, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
kysely ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 31, 2023 0:20am

Copy link
Member

@igalklebanov igalklebanov left a comment

Choose a reason for hiding this comment

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

Hey 👋

Thanks! 💪

Let some initial comments..

site/docs/dialects.md Outdated Show resolved Hide resolved
site/docs/getting-started/Dialects.tsx Outdated Show resolved Hide resolved
site/docs/getting-started/Summary.tsx Outdated Show resolved Hide resolved
site/docs/getting-started/shared.tsx Outdated Show resolved Hide resolved
site/docs/getting-started/shared.tsx Outdated Show resolved Hide resolved
@igalklebanov igalklebanov added documentation Improvements or additions to documentation mssql Related to MS SQL Server (MSSQL) labels Oct 6, 2023
@igalklebanov
Copy link
Member

We need to somehow mention that it also uses Tarn for connection pool management and provide a link to tarn's documentation.
https://github.com/kysely-org/kysely/pull/724/files#diff-f8ce7b5d9b471415c6e85f9db24baf58b16c5a55101d354bf66c0dc497340671R105-R109

@igalklebanov
Copy link
Member

Regarding Deno compatibility, here's an official issue that seems related to the problem (and someone claimed it still doesn't work 2 weeks ago despite being closed).

@igalklebanov
Copy link
Member

Regarding Bun compatibility, here's an issue that seems related to the problem (all commenters use libraries that use tedious under the hood).

…MSSQL pretty dialect name more clear

Co-authored-by: Igal Klebanov <[email protected]>
@gittgott
Copy link
Contributor Author

gittgott commented Oct 7, 2023

Hey @igalklebanov
I should have everything covered that you brought up.

Thanks for all your work maintaining this library by the way. You all do an amazing job.
I'm quite ecstatic that mssql is becoming part of the core dialects.

Copy link
Member

@igalklebanov igalklebanov left a comment

Choose a reason for hiding this comment

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

Looks good!

Just a small correction..

site/docs/getting-started/Dialects.tsx Outdated Show resolved Hide resolved
@gittgott gittgott marked this pull request as ready for review October 7, 2023 22:06
Copy link
Member

@igalklebanov igalklebanov left a comment

Choose a reason for hiding this comment

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

image
something's missing in "getting started -> querying".

@sofanati-nour
Copy link

image something's missing in "getting started -> querying".

it's the dialectSpecificCodeSnippets missing a mssql entry in getting-started/Querying.tsx

@gittgott
Copy link
Contributor Author

gittgott commented Oct 9, 2023

Hey @igalklebanov,

Sorry about that. I ran into some issues getting that one to work and forgot about it (and didn't run typecheck 🤦‍♂️)

I meant to ask: what would be a preferable solution for inserting a new row and returning data from that inserted row while #687 is still open?
The only thought I had was to use that existing findPeople function and grab the last value of the array that's returned. Here's the snippet for that:

export async function createPerson(person: NewPerson) {
  await db.insertInto('person')
    .values(person)
    .executeTakeFirstOrThrow()

  const newPerson = (await findPeople(person)).at(-1)
  return newPerson
}

export async function deletePerson(id: number) {
  const person = await findPersonById(id)

  if (person) {
    await db.deleteFrom('person').where('id', '=', id).execute()
  }

  return person
}

I think that the data will be sorted by id, so this would grab the most recently inserted row, but there would be issues if something was inserted to the table with identical values between the execution of the insertInto line and newPerson line.

Am I missing something or would there be no better option until the output clause is supported?

@igalklebanov
Copy link
Member

@gittgott I'd prefer using scope identity in a transaction.

@igalklebanov igalklebanov merged commit 52f4c48 into kysely-org:master Dec 31, 2023
5 checks passed
@gittgott gittgott deleted the mssql-docs branch January 11, 2024 19:03
thecodrr pushed a commit to thecodrr/kysely that referenced this pull request Sep 3, 2024
Co-authored-by: Igal Klebanov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation mssql Related to MS SQL Server (MSSQL)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add mssql to docs.
3 participants