[7.x] revert PR #33430 #33488
Closed
[7.x] revert PR #33430 #33488
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Please target this to 6.x, not 7.x. |
|
Our standard process is to merge upwards, and not to backport. |
|
Thanks @GrahamCampbell . Sent PR #33496 target to branch 6.x |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As noted by issue #33485 PR #33430 breaks a Laravel app when using the freeTDS driver to connect to SQL Server.
PR #33430 was sent to address issue #32883, but when implementing it I didn't test it against freeTDS drivers. I only tested against Microsoft official drivers ( https://github.com/Microsoft/msphpsql ). It seems freeTDS ise widely used , specially with older versions of SQL Server.
After the issue was reported I started investigating a solution that could solve both issues #33485 and #32883 but I could not find one.
So I am sending this PR to revert the changes made. Issue #32883 will still need to be addressed after reverting it.
I shared some of my findings on a comment on PR #33430 discussion and also presented some alternatives. Please refer to comment #33430 (comment) to have more details about it.
There I said I would send PR for both 6.x and 7.x branches, but I will wait on feedback on this one before sending the second one as maintainers could have a different workflow on back porting features and fixes.
If someone has a better suggestion on how to address this I would be glad to help.