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

Replaced the @ sign with the : sign for the primary key value referen… #39

Closed
wants to merge 1 commit into from

Conversation

jonast92
Copy link

…ce in the BuildPrimaryKeySql method. It was using the @ identifier to assign the value for which primary key to reference. This caused the UpdateAsync(poco) method to not work for Oracle applications because the OracleCommand.Parameters Property requires a : sign, according to the official msdn documentation (https://msdn.microsoft.com/en-us/library/system.data.oracleclient.oraclecommand.parameters.aspx).

…ce in the BuildPrimaryKeySql method. It was using the @ identifier to assign the value for which primary key to reference. This caused the UpdateAsync(poco) method to not work for Oracle applications because the OracleCommand.Parameters Property requires a : sign, according to the official msdn documentation (https://msdn.microsoft.com/en-us/library/system.data.oracleclient.oraclecommand.parameters.aspx).
@lajjts
Copy link

lajjts commented Feb 28, 2018

I hope all is well, @tmenier. Is it possible for you to check the impact on the code for your DBRM of choice that this PR would have? It appears to be crucial for Oracle statements as explained. I have to use a utility method within my projects to construct the Sql from scratch from the pogos to include ':' instead of '@' for the primary key value reference, it's not very feasible in the long run.

@tmenier
Copy link
Owner

tmenier commented Mar 11, 2018

I can't accept this because it causes all database tests to fail on all non-Oracle platforms. It's an Oracle-specific bug that needs an Oracle-specific solution. I'll look into it.

@tmenier tmenier closed this Mar 11, 2018
tmenier added a commit that referenced this pull request Mar 11, 2018
@tmenier
Copy link
Owner

tmenier commented Mar 16, 2018

@jonast92 @lajjts A fix for this is now available:

https://www.nuget.org/packages/AsyncPoco/1.2.1

Please let me know if this resolves the issue for you.

@tmenier tmenier reopened this Mar 16, 2018
@tmenier tmenier closed this Mar 16, 2018
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