Skip to content
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
8acf405
Refactor
WanjohiSammy Apr 1, 2025
065d26c
Remove unnecessary changes
WanjohiSammy Apr 1, 2025
425b42c
Remove unnecessary whitespaces
WanjohiSammy Apr 1, 2025
082a1ed
Remove whitespaces
WanjohiSammy Apr 1, 2025
6162161
Remove unnecessary whitespaces
WanjohiSammy Apr 1, 2025
b267f82
Remove the private method and just pass the functionName as parameter
WanjohiSammy Jul 9, 2025
421cbbf
Merge branch 'main' into fix/impossible-cast-with-isof-exception
WanjohiSammy Jul 9, 2025
0d37d6c
nit
WanjohiSammy Jul 9, 2025
41a440f
Merge branch 'fix/impossible-cast-with-isof-exception' of https://git…
WanjohiSammy Jul 9, 2025
3e33ff6
Refactor
WanjohiSammy Apr 1, 2025
f431035
Remove unnecessary changes
WanjohiSammy Apr 1, 2025
c69fd1c
Remove unnecessary whitespaces
WanjohiSammy Apr 1, 2025
7cf4fa7
Remove whitespaces
WanjohiSammy Apr 1, 2025
53e2015
Remove unnecessary whitespaces
WanjohiSammy Apr 1, 2025
6bdf585
Remove the private method and just pass the functionName as parameter
WanjohiSammy Jul 9, 2025
049fae6
nit
WanjohiSammy Jul 9, 2025
0679cdf
Update src/Microsoft.OData.Core/UriParser/Parsers/FunctionCallParser.cs
WanjohiSammy Aug 6, 2025
5f1c503
refactor and merge with origin main
WanjohiSammy Aug 6, 2025
51b1da4
Merge remote-tracking branch 'origin' into fix/impossible-cast-with-i…
WanjohiSammy Aug 6, 2025
7b2a775
Use of NormalizedModelElementsCache
WanjohiSammy Aug 6, 2025
c5ba104
using index
WanjohiSammy Aug 6, 2025
7ebd963
nit
WanjohiSammy Aug 6, 2025
d7e5211
using SchemaElements
WanjohiSammy Aug 6, 2025
8644a6e
rename tests correctly
WanjohiSammy Aug 7, 2025
04f2f61
Add NormalizedModelElementsCache singleton for EdmCoreModel and use i…
WanjohiSammy Aug 7, 2025
1b7e1c6
FindSchemaTypes can return null
WanjohiSammy Aug 7, 2025
f219ef3
Use a lightweight stack struct
WanjohiSammy Aug 7, 2025
106199f
Make StackStruct internal to avoid exposing it to the Public API
WanjohiSammy Aug 7, 2025
76e644d
move StackStructOfT to Microsoft.OData namespace
WanjohiSammy Aug 7, 2025
a6e59a8
use explicit types
WanjohiSammy Aug 8, 2025
a8d7bce
Update src/Microsoft.OData.Core/UriParser/Resolver/NormalizedModelEle…
WanjohiSammy Aug 8, 2025
785ac02
Use regular Stack
WanjohiSammy Aug 8, 2025
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
9 changes: 9 additions & 0 deletions src/Microsoft.OData.Core/SRResources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/Microsoft.OData.Core/SRResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -2462,6 +2462,9 @@
<data name="ExceptionUtils_ArgumentStringNullOrEmpty" xml:space="preserve">
<value>Value cannot be null or empty.</value>
</data>
<data name="ExceptionUtils_IsNullOrEmpty" xml:space="preserve">
<value>{0} is null or empty.</value>
</data>
<data name="ExpressionToken_OnlyRefAllowWithStarInExpand" xml:space="preserve">
<value>Only $ref is allowed with star in $expand option.</value>
</data>
Expand Down
175 changes: 175 additions & 0 deletions src/Microsoft.OData.Core/StackStructOfT.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
//---------------------------------------------------------------------
// <copyright file="StackStructOfT.cs" company="Microsoft">
// Copyright (C) Microsoft Corporation. All rights reserved. See License.txt in the project root for license information.
// </copyright>
//---------------------------------------------------------------------

using System;
using System.Collections.Generic;
using Microsoft.OData.Core;

namespace Microsoft.OData;

/// <summary>
/// A lightweight, struct-based generic stack implementation for value and reference types.
/// Provides efficient push, pop, and peek operations with dynamic resizing.
/// </summary>
/// <typeparam name="T">The type of elements in the stack.</typeparam>
internal struct StackStruct<T>
Copy link
Contributor

@habbes habbes Aug 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only light-weight in the wrapper StackStruct is a struct and not an object. So it may save one allocation, but you still use a heap allocated array even for the base case. So It's not really much different from a regular Stack and I don't think it's worth creating a custom type.

This is different from the version I proposed which would use an inline array buffer (e.g. using InlineArray attribute, could use fixed array buffer, but the latter is unsafe and limited to simple primitive types).

But I wouldn't want to block this PR because of this. My suggestion would be to remove this altogether (since it doesn't provide much optimization over a standard Stack, then we can think of a lightweight stack helper in a separate PR that can be used across the codebase)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@habbes I have reverted to use the regular stack

{
private T[] _items;
private int _count;
private const int DefaultCapacity = 4;

/// <summary>
/// Initializes a new instance of the <see cref="StackStruct{T}"/> struct with the default capacity.
/// </summary>
public StackStruct()
{
_items = new T[DefaultCapacity];
_count = 0;
}

/// <summary>
/// Initializes a new instance of the <see cref="StackStruct{T}"/> struct with the specified capacity.
/// </summary>
/// <param name="capacity">The initial number of elements the stack can contain.</param>
public StackStruct(int capacity)
{
_items = new T[capacity];
_count = 0;
}

/// <summary>
/// Pushes an item onto the top of the stack.
/// </summary>
/// <param name="item">The item to push onto the stack.</param>
public void Push(T item)
{
EnsureCapacity();
_items[_count++] = item;
}

/// <summary>
/// Removes and returns the item at the top of the stack.
/// </summary>
/// <returns>The item removed from the top of the stack.</returns>
/// <exception cref="InvalidOperationException">Thrown if the stack is empty.</exception>
public T Pop()
{
ThrowIfNullOrEmpty();
return _items[--_count];
}

/// <summary>
/// Returns the item at the top of the stack without removing it.
/// </summary>
/// <returns>The item at the top of the stack.</returns>
/// <exception cref="InvalidOperationException">Thrown if the stack is empty.</exception>
public T Peek()
{
ThrowIfNullOrEmpty();
return _items[_count - 1];
}

/// <summary>
/// Gets a value indicating whether the stack is empty.
/// </summary>
public bool IsEmpty => _count == 0;

/// <summary>
/// Gets the number of elements contained in the stack.
/// </summary>
public int Count => _count;

/// <summary>
/// Determines whether the current stack is equal to another object.
/// </summary>
/// <param name="obj">The object to compare with the current stack.</param>
/// <returns>true if the specified object is a <see cref="StackStruct{T}"/> and is equal to the current stack; otherwise, false.</returns>
public override bool Equals(object obj)
{
if (obj is not StackStruct<T> other)
{
return false;
}

return Equals(other);
}

/// <summary>
/// Determines whether the current stack is equal to another <see cref="StackStruct{T}"/>.
/// Two stacks are equal if they have the same count and all elements are equal in order.
/// </summary>
/// <param name="other">The stack to compare with the current stack.</param>
/// <returns>true if the stacks are equal; otherwise, false.</returns>
public readonly bool Equals(StackStruct<T> other)
{
if (_count != other._count)
return false;

EqualityComparer<T> comparer = EqualityComparer<T>.Default;
for (int i = 0; i < _count; i++)
{
if (!comparer.Equals(_items[i], other._items[i]))
{
return false;
}
}
return true;
}

/// <summary>
/// Returns a hash code for the current stack.
/// </summary>
/// <returns>A hash code for the current stack.</returns>
public override int GetHashCode()
{
EqualityComparer<T> comparer = EqualityComparer<T>.Default;
int hash = 17;
hash = hash * 31 + _count.GetHashCode();
for (int i = 0; i < _count; i++)
{
hash = hash * 31 + comparer.GetHashCode(_items[i]);
}
return hash;
}

/// <summary>
/// Determines whether two <see cref="StackStruct{T}"/> instances are equal.
/// </summary>
/// <param name="left">The first stack to compare.</param>
/// <param name="right">The second stack to compare.</param>
/// <returns>true if the stacks are equal; otherwise, false.</returns>
public static bool operator ==(StackStruct<T> left, StackStruct<T> right)
{
return left.Equals(right);
}

/// <summary>
/// Determines whether two <see cref="StackStruct{T}"/> instances are not equal.
/// </summary>
/// <param name="left">The first stack to compare.</param>
/// <param name="right">The second stack to compare.</param>
/// <returns>true if the stacks are not equal; otherwise, false.</returns>
public static bool operator !=(StackStruct<T> left, StackStruct<T> right)
{
return !(left == right);
}

private void EnsureCapacity()
{
if (_count == _items.Length)
{
Array.Resize(ref _items, _items.Length * 2);
}
}

private readonly void ThrowIfNullOrEmpty()
{
if (_count == 0)
{
throw new InvalidOperationException(Error.Format(SRResources.ExceptionUtils_IsNullOrEmpty, "StackStruct"));
}
}
}
15 changes: 11 additions & 4 deletions src/Microsoft.OData.Core/UriParser/Binders/FunctionCallBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -720,12 +720,19 @@ private static IEdmTypeReference ValidateIsOfOrCast(BindingState state, bool isC
Error.Format(SRResources.MetadataBinder_CastOrIsOfExpressionWithWrongNumberOfOperands, args.Count));
}

ConstantNode typeArgument = args.Last() as ConstantNode;
QueryNode queryNode = args[^1];

string typeArgumentFullName = null;
IEdmTypeReference returnType = null;
if (typeArgument != null)
if (queryNode is SingleResourceCastNode singleResourceCastNode)
{
returnType = TryGetTypeReference(state.Model, typeArgument.Value as string, state.Configuration.Resolver);
returnType = singleResourceCastNode.TypeReference;
typeArgumentFullName = returnType.FullName();
}
else if (queryNode is ConstantNode constantNode)
{
typeArgumentFullName = constantNode.Value as string;
returnType = TryGetTypeReference(state.Model, typeArgumentFullName, state.Configuration.Resolver);
}

if (returnType == null)
Expand Down Expand Up @@ -758,7 +765,7 @@ private static IEdmTypeReference ValidateIsOfOrCast(BindingState state, bool isC
{
// throw if cast enum to not-string :
if ((args[0].GetEdmTypeReference() is IEdmEnumTypeReference)
&& !string.Equals(typeArgument.Value as string, Microsoft.OData.Metadata.EdmConstants.EdmStringTypeName, StringComparison.Ordinal))
&& !string.Equals(typeArgumentFullName, Microsoft.OData.Metadata.EdmConstants.EdmStringTypeName, state.Configuration.Resolver?.EnableCaseInsensitive == true ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal))
{
throw new ODataException(SRResources.CastBinder_EnumOnlyCastToOrFromString);
}
Expand Down
66 changes: 59 additions & 7 deletions src/Microsoft.OData.Core/UriParser/Parsers/FunctionCallParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ namespace Microsoft.OData.UriParser
using System.Diagnostics;
using System.Linq;
using Microsoft.OData.Core;
using Microsoft.OData.Edm;

/// <summary>
/// Implementation of IFunctionCallParser that allows functions calls and parses arguments with a provided method.
Expand Down Expand Up @@ -101,7 +102,7 @@ public bool TryParseIdentifierAsFunction(QueryToken parent, out QueryToken resul
this.Lexer.NextToken();
}

FunctionParameterToken[] arguments = this.ParseArgumentListOrEntityKeyList(() => lexer.RestorePosition(position));
FunctionParameterToken[] arguments = this.ParseArgumentListOrEntityKeyList(() => lexer.RestorePosition(position), functionName);
if (arguments != null)
{
result = new FunctionCallToken(functionName.ToString(), arguments, parent);
Expand All @@ -114,8 +115,9 @@ public bool TryParseIdentifierAsFunction(QueryToken parent, out QueryToken resul
/// Parses argument lists or entity key value list.
/// </summary>
/// <param name="restoreAction">Action invoked for restoring a state during failure.</param>
/// <param name="functionName">The name of the function being called. Default is an empty span.</param>
/// <returns>The lexical tokens representing the arguments.</returns>
public FunctionParameterToken[] ParseArgumentListOrEntityKeyList(Action restoreAction = null)
public FunctionParameterToken[] ParseArgumentListOrEntityKeyList(Action restoreAction = null, ReadOnlySpan<char> functionName = default)
{
if (this.Lexer.CurrentToken.Kind != ExpressionTokenKind.OpenParen)
{
Expand All @@ -136,7 +138,7 @@ public FunctionParameterToken[] ParseArgumentListOrEntityKeyList(Action restoreA
}
else
{
arguments = this.ParseArguments();
arguments = this.ParseArguments(functionName);
}

if (this.Lexer.CurrentToken.Kind != ExpressionTokenKind.CloseParen)
Expand All @@ -161,33 +163,53 @@ public FunctionParameterToken[] ParseArgumentListOrEntityKeyList(Action restoreA
/// Arguments can either be of the form a=1,b=2,c=3 or 1,2,3.
/// They cannot be mixed between those two styles.
/// </remarks>
/// <param name="functionName">The name of the function being called. Default is an empty span.</param>
/// <returns>The lexical tokens representing the arguments.</returns>
public FunctionParameterToken[] ParseArguments()
public FunctionParameterToken[] ParseArguments(ReadOnlySpan<char> functionName = default)
{
ICollection<FunctionParameterToken> argList;
if (this.TryReadArgumentsAsNamedValues(out argList))
{
return argList.ToArray();
}

return this.ReadArgumentsAsPositionalValues().ToArray();
return this.ReadArgumentsAsPositionalValues(functionName).ToArray();
}

/// <summary>
/// Read the list of arguments as a set of positional values
/// </summary>
/// <param name="functionName">The name of the function being called. Default is an empty span.</param>
/// <returns>A list of FunctionParameterTokens representing each argument</returns>
private List<FunctionParameterToken> ReadArgumentsAsPositionalValues()
private List<FunctionParameterToken> ReadArgumentsAsPositionalValues(ReadOnlySpan<char> functionName = default)
{
// Store the parent expression of the current argument.
StackStruct<QueryToken> expressionParents = new StackStruct<QueryToken>();
bool isFunctionCallNameCastOrIsOf = functionName.Length > 0 &&
(functionName.SequenceEqual(ExpressionConstants.UnboundFunctionCast.AsSpan()) || functionName.SequenceEqual(ExpressionConstants.UnboundFunctionIsOf.AsSpan()));
List<FunctionParameterToken> argList = new List<FunctionParameterToken>();
while (true)
{
argList.Add(new FunctionParameterToken(null, this.parser.ParseExpression()));
// If we have a parent expression, we need to set the parent of the next argument to the current argument.
QueryToken parentExpression = expressionParents.Count > 0 ? expressionParents.Pop() : null;
QueryToken parameterToken = this.parser.ParseExpression();

// If the function call is cast or isof, set the parent of the next argument to the current argument.
if (parentExpression != null && isFunctionCallNameCastOrIsOf)
{
parameterToken = SetParentForCurrentParameterToken(parentExpression, parameterToken);
}

argList.Add(new FunctionParameterToken(null, parameterToken));
if (this.Lexer.CurrentToken.Kind != ExpressionTokenKind.Comma)
{
break;
}

// In case of comma, we need to parse the next argument
// but first we need to set the parent of the next argument to the current argument.
expressionParents.Push(parameterToken);

this.Lexer.NextToken();
}

Expand All @@ -213,5 +235,35 @@ private bool TryReadArgumentsAsNamedValues(out ICollection<FunctionParameterToke

return false;
}

/// <summary>
/// Set the parent of the next argument to the current argument.
/// For example, in the following query:
/// cast(Location, NS.HomeAddress) where Location is the parentExpression and NS.HomeAddress is the parameterToken.
/// isof(Location, NS.HomeAddress) where Location is the parentExpression and NS.HomeAddress is the parameterToken.
/// </summary>
/// <param name="parentExpression">The previous parameter token.</param>
/// <param name="parameterToken">The current parameter token.</param>
/// <returns>The updated parameter token.</returns>
private static QueryToken SetParentForCurrentParameterToken(QueryToken parentExpression, QueryToken parameterToken)
{
if (parameterToken is not DottedIdentifierToken dottedIdentifierToken || dottedIdentifierToken?.NextToken is not null)
{
return parameterToken;
}

// Check if the identifier is a primitive type
IEdmSchemaType schemaType = NormalizedModelElementsCache.EdmCoreModelCacheInstance.FindSchemaTypes(dottedIdentifierToken.Identifier)?.FirstOrDefault();

// If the identifier is a primitive type
if (schemaType is IEdmPrimitiveType)
{
return parameterToken;
}

// Set the parent of the next argument to the current argument
dottedIdentifierToken.NextToken = parentExpression;
return dottedIdentifierToken;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ namespace Microsoft.OData.Edm
/// </summary>
internal sealed class NormalizedModelElementsCache
{
/// <summary>
/// Provides a shared, normalized case-insensitive cache of model elements for the EDM core model.
/// </summary>
public static NormalizedModelElementsCache EdmCoreModelCacheInstance = new NormalizedModelElementsCache(EdmCoreModel.Instance);

// We create different caches for different types of schema elements because all current usage request schema elements
// of specific types. If we were to use a single dictionary <string, ISchemaElement> we would need
// to do additional work (and allocations) during lookups to filter the results to the subset that matches the request type.
Expand Down
Loading