- Notifications
You must be signed in to change notification settings - Fork126
Named parameters support#200
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Named parameters support#200
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Signed-off-by: Matthew Kim <11141331+mattdeekay@users.noreply.github.com>
Signed-off-by: Matthew Kim <11141331+mattdeekay@users.noreply.github.com>
Signed-off-by: Matthew Kim <11141331+mattdeekay@users.noreply.github.com>
Signed-off-by: Matthew Kim <11141331+mattdeekay@users.noreply.github.com>
Signed-off-by: Matthew Kim <11141331+mattdeekay@users.noreply.github.com>
Signed-off-by: Matthew Kim <11141331+mattdeekay@users.noreply.github.com>
Signed-off-by: Matthew Kim <11141331+mattdeekay@users.noreply.github.com>
andrefurlan-db left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
please make sure that parameterized query support is consistent across all drivers/connectors.@rcypher-databricks needs to review and make sure it is. Specially data type handling stuff
| ) | ||
| defexecute( | ||
| self,operation:str,parameters:Optional[Dict[str,str]]=None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
If we allowed users to do this I think we should continue to support it, but instead of injecting the parameter values in the driver we should convert to the format expected by the backend and pass it on.
| returnspark_params | ||
| @staticmethod | ||
| defget_type_and_value(value:Any)-> (str,TSparkParameterValue): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We need to have more comprehensive handling of types. There needs to be a way for a user to specify the desired sql type of the parameter value.
susodapop commentedNov 1, 2023
Closing as this was implemented in#217 |
Supports server-side named query parameterization. Client will need to call execute like this:
This removes client-side query injection that was a facade for query parameterization while the server-side support was still under development.