Skip to content

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.

Sign up for GitHub

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

Open
daxian-dbw wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix to unwrap PSObject when setting data to DataRow and DataRowView #19071

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3de2df4
Fix to unwrap PSObject when setting data to DataRow and DataRowView
daxian-dbw Jan 31, 2023
7babf1d
Update comments
daxian-dbw Feb 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/System.Management.Automation/engine/CoreAdapter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5755,11 +5755,12 @@ protected override object PropertyGet(PSProperty property)
protected override void PropertySet(PSProperty property, object setValue, bool convertIfPossible)
{
DataRow dataRow = (DataRow)property.baseObject;
dataRow[(string)property.adapterData] = setValue;
dataRow[(string)property.adapterData] = PSObject.Base(setValue);
return;
}
#endregion virtual
}

/// <summary>
/// Deals with DataRowView objects.
/// </summary>
Expand Down Expand Up @@ -5878,7 +5879,7 @@ protected override object PropertyGet(PSProperty property)
protected override void PropertySet(PSProperty property, object setValue, bool convertIfPossible)
{
DataRowView dataRowView = (DataRowView)property.baseObject;
dataRowView[(string)property.adapterData] = setValue;
dataRowView[(string)property.adapterData] = PSObject.Base(setValue);
Copy link
Collaborator

@iSazonov iSazonov Feb 1, 2023

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.

Copy link
Member Author

@daxian-dbw daxian-dbw Feb 3, 2023

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.

return;
}
#endregion virtual
Expand Down
23 changes: 18 additions & 5 deletions src/System.Management.Automation/engine/runtime/Binding/Binders.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2038,7 +2038,7 @@ internal static bool IsValueTypeMutable(Type type)
// First, check for enums/primitives and compiler-defined attributes.
if (type.IsPrimitive
|| type.IsEnum
|| type.IsDefined(typeof(System.Runtime.CompilerServices.IsReadOnlyAttribute), inherit: false))
|| type.IsDefined(typeof(IsReadOnlyAttribute), inherit: false))
{
return false;
}
Expand Down Expand Up @@ -4622,7 +4622,7 @@ public override DynamicMetaObject FallbackSetIndex(
var defaultMember = target.LimitType.GetCustomAttributes<DefaultMemberAttribute>(true).FirstOrDefault();
if (defaultMember != null)
{
return (InvokeIndexer(target, indexes, value, errorSuggestion, defaultMember.MemberName)).WriteToDebugLog(this);
return InvokeIndexer(target, indexes, value, errorSuggestion, defaultMember.MemberName).WriteToDebugLog(this);
}

return errorSuggestion ?? CannotIndexTarget(target, indexes, value).WriteToDebugLog(this);
Expand Down Expand Up @@ -4676,15 +4676,28 @@ private DynamicMetaObject InvokeIndexer(
target.PSGetTypeRestriction());
}

// For set-index operation on 'DataRow' and 'DataRowView', 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.
Type targetType = setter.DeclaringType;
bool unwrapArgIfNeed = targetType == typeof(DataRow) || targetType == typeof(DataRowView);

Expression[] indexExprs = new Expression[paramLength];
for (int i = 0; i < paramLength; ++i)
{
var parameterType = setterParams[i].ParameterType;
var argument = (i == paramLength - 1) ? value : indexes[i];

indexExprs[i] = parameterType.IsByRefLike
? PSConvertBinder.ConvertToByRefLikeTypeViaCasting(argument, parameterType)
: PSGetIndexBinder.ConvertIndex(argument, parameterType);
if (unwrapArgIfNeed && parameterType == typeof(object) && argument.LimitType == typeof(PSObject))
{
indexExprs[i] = Expression.Call(CachedReflectionInfo.PSObject_Base, argument.Expression.Cast(typeof(PSObject)));
}
else
{
indexExprs[i] = parameterType.IsByRefLike
? PSConvertBinder.ConvertToByRefLikeTypeViaCasting(argument, parameterType)
: PSGetIndexBinder.ConvertIndex(argument, parameterType);
}

if (indexExprs[i] == null)
{
Expand Down
31 changes: 31 additions & 0 deletions test/powershell/Language/Scripting/Scripting.Followup.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,4 +134,35 @@ public class NullStringTest {
$result = & $powershell -noprofile -c '[System.Text.Encoding]::GetEncoding("IBM437").WebName'
$result | Should -BeExactly "ibm437"
}

It 'Can set DataRow with PSObject through adapter and indexer' {
$dataTable = [Data.DataTable]::new()
$null = $dataTable.Columns.Add('Date', [DateTime])
$row = $dataTable.Rows.Add((Get-Date))

$date1 = Get-Date
$row.Date = $date1
$row.Date.Ticks | Should -Be $date1.Ticks

$date2 = Get-Date
$row['Date'] = $date2
$row.Date.Ticks | Should -Be $date2.Ticks
}

It 'Can set DataRowView with PSObject through adapter and indexer' {
$dataTable = [Data.DataTable]::new()
$null = $dataTable.Columns.Add('Date', [DateTime])
$dataTable.Rows.Add((Get-Date))

$dataView = [System.Data.DataView]::new($dataTable)
$rowView = $dataView[0]

$date1 = Get-Date
$rowView.Date = $date1
$rowView.Date.Ticks | Should -Be $date1.Ticks

$date2 = Get-Date
$rowView['Date'] = $date2
$rowView.Date.Ticks | Should -Be $date2.Ticks
}
}