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

Fix places where the parameterPlaceholder is hardcoded to a ? #616

Merged
merged 2 commits into from
Sep 28, 2022

Conversation

toburger
Copy link
Contributor

Accidentally closed the old PR #592

@ahmad-moussawi
Copy link
Contributor

Merged now thanks!

@toburger
Copy link
Contributor Author

When do you plan to release a new NuGet package?

@kamisoft-fr
Copy link

Hi guys, I tested the new code and unfortunately we still have a last hardcoded question mark in SqlResult

        {
            var deepParameters = Helper.Flatten(Bindings).ToList();

            return Helper.ReplaceAll(RawSql, "?", i =>
            {
                if (i >= deepParameters.Count)
                {
                    throw new Exception(
                        $"Failed to retrieve a binding at index {i}, the total bindings count is {Bindings.Count}");
                }

                var value = deepParameters[i];
                return ChangeToSqlValue(value);
            });
        }

Apparently the compiler is not saved in any of the properties or fields of SqlResult, otherwise I would have created a PR :)

@ahmad-moussawi
Copy link
Contributor

@kamisoft-fr thanks for reporting, oops yes this still missing, we should find a way to instruct the SqlResult to copy the placeholder from the compiler, do you have any suggestion?

@flookami
Copy link

Hi @ahmad-moussawi, do you have the time to take a look at PR #592 ?
it's been months I've made the pull request in order to change the default parameter placeholder & use JSON queries in my code base :)

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.

4 participants