-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Fix to unwrap PSObject when setting data to DataRow and DataRowView
#19071
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
base: master
Are you sure you want to change the base?
Conversation
| { | ||
| DataRowView dataRowView = (DataRowView)property.baseObject; | ||
| dataRowView[(string)property.adapterData] = setValue; | ||
| dataRowView[(string)property.adapterData] = PSObject.Base(setValue); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we make PSObject more flexible/smart with IConvertable?
The error is raised in https://github.com/dotnet/runtime/blob/1c442fcb36e73eb3d8be10423a45389ce2a468b9/src/libraries/System.Data.Common/src/System/Data/Common/DateTimeStorage.cs#L165
The pattern looks as commonly used. So perhaps we could benefit from implementation IConvertable in PSObject.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sounds like a good idea, better than special-casing the DataRow and DataRowView in the binder code.
SeeminglyScience
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
|
By chance I encountered a similar error today that exist even in 5.1 $MyObject = [PSCustomObject]@{
Param = 1
}
$Select = $MyObject | Select-Object -Property ('Param')
$SelectExpression = 1 | Select-Object -Property (
@{ Name = 'Param'; Expression = { 1 } }
)
$Dt = [System.Data.DataTable]::new()
$Dt.Columns.Add('Param', [int]) | Out-Null
$Row = $Dt.Rows.Add()
$Row['Param'] = $MyObject.Param # 👍
$Row['Param'] = $Select.Param # 👍
$Row['Param'] = $SelectExpression.Param # Error
# Unable to cast object of type 'System.Management.Automation.PSObject' to type 'System.IConvertible'.Couldn't store <1> in Param Column. Expected type is Int32. |
|
Another one I see now with multiple items $MyObject = [PSCustomObject]@{
Param = 1
Param2 = 'Foo'
}
$Select = $MyObject | Select-Object -Property ('Param', 'Param2')
$SelectExpression = 1 | Select-Object -Property (
@{ Name = 'Param'; Expression = { 1 } },
@{ Name = 'Param2'; Expression = { 'Foo' } }
)
$Dt = [System.Data.DataTable]::new()
$Dt.Columns.Add('Param', [int]) | Out-Null
$Dt.Columns.Add('Param2', [string]) | Out-Null
$Row = $Dt.Rows.Add()
$Row['Param'] = $MyObject.Param # 👍
$Row['Param'] = $Select.Param # 👍
$Row['Param'] = $SelectExpression.Param # Error
$Row = $Dt.Rows.Add($SelectExpression.Param, $SelectExpression.Param2) # Individual 👍
$Row = $Dt.Rows.Add($MyObject.PSObject.Properties.Value) # Array exact types 👍
$Row = $Dt.Rows.Add(('1', 'Foo')) # Array string casted automatically 👍
$Row = $Dt.Rows.Add($SelectExpression.PSObject.Properties.Value) # Array PSObject error, not casted
|
|
The idea to implement |
|
This PR has Quantification details
Why proper sizing of changes matters
Optimal pull request sizes drive a better predictable PR flow as they strike a
What can I do to optimize my changes
How to interpret the change counts in git diff output
Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email) |
|
This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days. |
|
Engine WG discussed this again and we still think it's a good idea. |
PR Summary
Fix #17627
Fix to unwrap
PSObjectwhen setting data toDataRowandDataRowView.For set-index operation on
DataRowandDataRowView, we unwrap its argument value if it's appropriate.We treat those 2 types specially because their indexers accept 'object' arguments in the signatures, but actually check the data type internally and will fail if the argument is wrapped in PSObject.
This fixes a regression introduced in PowerShell v7.2.0, see #17627 (comment) for details.
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.