Skip to content
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

Query: Allow FromSql to accept parameter of type DbParameter in subquery #8885

Merged
merged 1 commit into from
Jun 22, 2017

Conversation

smitpatel
Copy link
Contributor

@smitpatel smitpatel commented Jun 16, 2017

Resolves #8721

TODO:

  • Doc comments to be updated
  • Better tests with variations.

@anpete - Creating PR to get initial feedback on the approach.

if (parameterValues.TryGetValue(parameter.InvariantName, out var parameterValue))
if (parameter is RawRelationalParameter)
{
parameter.AddDbParameter(command, value: null);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since IRelationalParameter.AddDbParameter` has value parameter, passing null in this case.

Copy link
Contributor

Choose a reason for hiding this comment

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

Change to polymorphic call on parameter

@smitpatel smitpatel requested a review from anpete June 19, 2017 16:49

substitutions = new string[argumentValues.Length];

for (var i = 0; i < argumentValues.Length; i++)
{
var value = argumentValues[i];
if (value is DbParameter dbParameter)
{
substitutions[i] = dbParameter.ParameterName;
Copy link
Contributor

Choose a reason for hiding this comment

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

So this is to support {0} syntax with raw parameter?

{
substitutions[i] = dbParameter.ParameterName;
_relationalCommandBuilder.AddRawParameter(dbParameter.ParameterName, dbParameter);
continue;
Copy link
Contributor

Choose a reason for hiding this comment

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

Just use else instead?

@smitpatel smitpatel force-pushed the fromsqllostparameters8721 branch 2 times, most recently from f7a4e48 to 0bb07a9 Compare June 21, 2017 22:49
@smitpatel
Copy link
Contributor Author

@anpete - Ready for review

Test patterns covered

  • No format item with SqlParameter
  • Format item with SqlParameter without name (generates parameter name)
  • Format item with SqlParameter with name (reuse already given name)
  • Format item mixed with SqlParameter without format item
  • Format item missing 0 index with SqlParameter providing value

cc: @divega - If you want to check out.

@smitpatel smitpatel changed the title Query: Add RawRelationalParameter to flow user defined DbParameter to DbCommand Query: Allow FromSql to accept parameter of type DbParameter in subquery Jun 21, 2017
@smitpatel smitpatel force-pushed the fromsqllostparameters8721 branch from 0bb07a9 to 779da18 Compare June 21, 2017 23:06
/// </summary>
public void AddDbParameter(DbCommand command, object value)
{
if (value == null)
Copy link
Contributor

Choose a reason for hiding this comment

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

This ever false?

@smitpatel smitpatel force-pushed the fromsqllostparameters8721 branch 2 times, most recently from 65f366e to 78fceb9 Compare June 22, 2017 02:12
Add RawRelationalParameter to flow user defined DbParameter to DbCommand
Add method on IRelationalParameter which takes command and parameterValues
Add RelationalParameterBase class
FromSql when taking DbParameter arg
- Use format item to substitute if available
- Use named arg if no format item available
- Generate a name if DbParameter.ParameterName is unspecified
@smitpatel smitpatel force-pushed the fromsqllostparameters8721 branch from 78fceb9 to 4c94662 Compare June 22, 2017 02:17
@smitpatel
Copy link
Contributor Author

Updated.

Copy link
Contributor

@anpete anpete left a comment

Choose a reason for hiding this comment

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

Nice!

@smitpatel smitpatel merged commit 4c94662 into dev Jun 22, 2017
@smitpatel smitpatel deleted the fromsqllostparameters8721 branch June 22, 2017 16:42
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.

3 participants