Skip to content

4.x-identity-insert #829

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

Merged
merged 2 commits into from
Mar 27, 2025

Conversation

jhandel
Copy link
Contributor

@jhandel jhandel commented Mar 24, 2025

added an implementation of insert and bulkInsert to the SqlServerAdapter.php.

In the new implementations they check options for 'identity_insert' = true.. if so it wraps the generated sql in the identity insert sql logic.

I also refactored AbstractAdapter, pulling the SQL generation out into protected functions that the copies pushed down to SqlServerAdapter didn't need the SQL generation logic as well.

…t have to copy too much code to SqlServerAdapter. (could be dryer if we wanted)
@jhandel
Copy link
Contributor Author

jhandel commented Mar 24, 2025

the PR does pass the original problem test.. I am not sure why its failing the other tests.. those failures aren't even close to the changes I made..

@jhandel
Copy link
Contributor Author

jhandel commented Mar 25, 2025

ok before I go way down the rabbit hole on why these output comparison tests are failing..

the first one I looked into(testNotEmptySnapshot in ). the difference is that the result is missing '<?php' on the first line otherwise it absolutely would have passed..

is this a known issue or something I am tripping over while working on something else (because this is not at all related to my changes to insert or bulkInsert.

happy to look into them but don't want to confuse the issues as those are not what this is about.

@markstory
Copy link
Member

ok before I go way down the rabbit hole on why these output comparison tests are failing..

You don't have to worry about those failures, they are fixed on 4.x already.

@markstory markstory merged commit bebcee8 into cakephp:4.x-identity-insert Mar 27, 2025
20 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants