Skip to content

fix(adapter-d1): throws Error when string column contains any ISO date#25483

Merged
aqrln merged 9 commits intoprisma:mainfrom
Takur0:fix/d1AdapterDateBug
Oct 28, 2024
Merged

fix(adapter-d1): throws Error when string column contains any ISO date#25483
aqrln merged 9 commits intoprisma:mainfrom
Takur0:fix/d1AdapterDateBug

Conversation

@Takur0
Copy link
Copy Markdown
Contributor

@Takur0 Takur0 commented Oct 22, 2024 •
edited
Loading

This PR closes #25404

@Takur0 Takur0 marked this pull request as ready for review October 22, 2024 18:02
@aqrln
Copy link
Copy Markdown
Member

aqrln commented Oct 24, 2024

Hey @Takur0, thanks so much for the PR, the change looks good to me. Would you mind adding a test as well?

@aqrln aqrln modified the milestones: 5.21.0, 5.22.0 Oct 24, 2024
@jkomyno jkomyno self-assigned this Oct 24, 2024
@Takur0
Copy link
Copy Markdown
Contributor Author

Takur0 commented Oct 25, 2024

Hi @aqrln, thank you for reviewing my PR! I'm happy to add a test for this change. I'll update the PR soon.

@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Oct 25, 2024
@Takur0 Takur0 marked this pull request as draft October 25, 2024 20:52
@Takur0 Takur0 marked this pull request as ready for review October 25, 2024 20:52
@Takur0
Copy link
Copy Markdown
Contributor Author

Takur0 commented Oct 25, 2024

Hi @aqrln, I've added the test as requested. Please let me know if there's anything else you'd like me to adjust.

@aqrln
Copy link
Copy Markdown
Member

aqrln commented Oct 26, 2024 •
edited
Loading

Thanks @Takur0, this looks great. My only comment is that even though it's a regression test for a specific issue that happened with D1 adapter, I think there's still value in running it with all providers and adapters. Maybe some hypothetical future adapter could introduce a similar issue, and we'll be able to catch it early with the test already in place.

@Takur0
Copy link
Copy Markdown
Contributor Author

Takur0 commented Oct 28, 2024

Thank you for your detailed comment, @aqrln ! I've updated the test code according to your suggestions. Please take a look!

aqrln
aqrln approved these changes Oct 28, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@aqrln aqrln 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 to me, thanks!

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Oct 28, 2024
@aqrln aqrln merged commit 9567ac0 into prisma:main Oct 28, 2024
@Takur0 Takur0 deleted the fix/d1AdapterDateBug branch October 29, 2024 00:45
jkomyno added a commit that referenced this pull request Oct 31, 2024
@jkomyno @tmm1
@Takur0 @hinaloe
feat(cli): [BREAKING] remove custom yarn logic (#25457)
b0ca0f7 
* feat(cli): [BREAKING] remove custom yarn logic

* test(internals): update snapshots

* chore(internals): get rid of unused fixtures

* fix(fetch-engine): use rename syscall to ensure atomic moves (#25129)

* fix(adapter-d1): throws Error when string column contains any ISO date (#25483)

* fix(cli): `prisma version --json` suppress dotenv message (#25535)

- fix #25478

`prisma version --json` should not contain non-json string.

* chore: removed unused file

* chore: restore yarn detection logic for Windows

---------

Co-authored-by: Aman Karmani <aman@tmm1.net>
Co-authored-by: Yamashiro Takuro <39460271+Takur0@users.noreply.github.com>
Co-authored-by: Hinaloe <hina@hinaloe.net>
@jkomyno jkomyno mentioned this pull request Nov 14, 2024
Merged
@acidjazz
Copy link
Copy Markdown

acidjazz commented Dec 27, 2024

is this in a version I can use it? I'm not sure how to work around this

@jkomyno jkomyno mentioned this pull request Dec 30, 2024
Closed
@jkomyno
Copy link
Copy Markdown
Contributor

jkomyno commented Dec 30, 2024

is this in a version I can use it? I'm not sure how to work around this

Hi, this issue was fixed in Prisma 5.22.0.

@Yesterday17 Yesterday17 mentioned this pull request May 11, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aqrln aqrln aqrln approved these changes

+1 more reviewer

@xPapla xPapla xPapla approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jkomyno jkomyno

Labels

lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

5.22.0

Development

Successfully merging this pull request may close these issues.

SQL Injection bug - D1 adaptor throws "Conversion failed: expected a datetime string in column" when string column contains any ISO date

5 participants

@Takur0 @aqrln @acidjazz @jkomyno @xPapla