Make all params named params #162
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Update the `BatchBuilder` to add all positional parameters as named parameters. This is the same behavior as `CommandBuilder`. Under the hood what is going on is that if there is a mix of positional and named parameters, then npgsql does not edit the query to normalise all parameters. This results in postgres throwing an error (cant mix positional and named parameters). If there were only positional parameters (old behhaviour), the npgsql will rewrite the query to convert all positional params to named params. This is done for maximum compatibility. The easiest way to make things consistently work is to make all positional params as named params from the start, which is what `CommandBuilder`/`CommandBuilderBase` does. This _may_ be a breaking change - but only if people were already using named parameters with the batch builder and only if they were naming their params `p0`, `p1`, etc.
|
@elexisvenator Hey man, sorry, this one is a no go. We very specifically rebuilt BatchBuilder to use positional parameters because that prevents Npgsql from having to do a conversion internally. |
|
Backing out of this |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Needed by JasperFx/marten#3604
Update the
BatchBuilderto add all positional parameters as named parameters. This is the same behavior asCommandBuilder.Under the hood what is going on is that if there is a mix of positional and named parameters, then npgsql does not edit the query to normalise all parameters. This results in postgres throwing an error (cant mix positional and named parameters). If there were only positional parameters (old behaviour), the npgsql will rewrite the query to convert all positional params to named params. This is done for maximum compatibility. The easiest way to make things consistently work is to make all positional params as named params from the start, which is what
CommandBuilder/CommandBuilderBasedoes.This may be a breaking change - but only if people were already using named parameters with the batch builder and only if they were naming their params
p0,p1, etc.