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

Added the 'GetExpectedParameterTypes' method to the PreparedStatement… #5792

Merged
merged 2 commits into from
Jan 12, 2023
Merged

Added the 'GetExpectedParameterTypes' method to the PreparedStatement… #5792

merged 2 commits into from
Jan 12, 2023

Conversation

AlexR2D2
Copy link
Contributor

This method allows to get the expected types of parameters of SQL query.

To be short.
DuckDB amalgamation can be used in the Erlang NIF. In the Erlang the parameters of SQL query are passed as terms. Term - a piece of data of any data type. We can use special Erlang NIF functions like enif_is_number(term), enif_is_binary(term) to detect the type of the term we passed into query and convert this term parameter to the duckdb::Value parameter of this detected type.

Basically, it works, but may be it would be more correct if we could get the expected types of parsed SQL query parameters from the duckdb::PreparedStatement not from the terms itself, and try convert Erlang terms to the duckdb::Value's of expected types.

… to return the map of parameter index to the expected type of parameter.
@Tishj
Copy link
Contributor

Tishj commented Dec 27, 2022

Can we add a D_ASSERT(it.first >= 1);?
To cover the assumption made when doing it.first - 1

And just to be safe, also D_ASSERT((it.first - 1) < expected_types.size());

@AlexR2D2
Copy link
Contributor Author

AlexR2D2 commented Dec 28, 2022

Sure, updated. If you want, you can make any changes, I don't mind at all). I just wish the method would continue to return expected types for query parameters.

@hannes
Copy link
Member

hannes commented Dec 30, 2022

We might want to think about how this could be extended to named parameters.

@AlexR2D2
Copy link
Contributor Author

AlexR2D2 commented Jan 9, 2023

We might want to think about how this could be extended to named parameters.

Maybe then replace this:
vector<LogicalType> PreparedStatement::GetExpectedParameterTypes()
to this?
unordered_map<string, LogicalType> PreparedStatement::GetExpectedParameterTypes()

For example, for SQL parameters like this $1, $2, $name, $city it will returns unordered_map like this {"1" -> LogicalType, "2" -> LogicalType, "name" -> LogicalType, "city" -> LogicalType}

So, until you made named parameters it will return a map where indexes of parameters are in the form of a string {"1" -> LogicalType, "2" -> LogicalType,... etc}

@Mytherin
Copy link
Collaborator

I would say the named parameters should be separate from the positional parameters - this looks fine to me for the positional parameters.

@Mytherin Mytherin merged commit 123b6e7 into duckdb:master Jan 12, 2023
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.

None yet

4 participants