clintropolis commented on a change in pull request #6974: sql support for
dynamic parameters
URL: https://github.com/apache/druid/pull/6974#discussion_r370594559
##########
File path: sql/src/main/java/org/apache/druid/sql/avatica/DruidStatement.java
##########
@@ -183,11 +202,10 @@ public DruidStatement execute()
{
synchronized (lock) {
ensure(State.PREPARED);
-
try {
- final Sequence<Object[]> baseSequence =
yielderOpenCloseExecutor.submit(
- sqlLifecycle::execute
- ).get();
+ sqlLifecycle.setParameters(parameters);
+ sqlLifecycle.planAndAuthorize(authenticationResult);
Review comment:
Ah, so this PR sort of fundamentally changes what 'prepare' means for Druid
prepared statements. To try to elaborate on what the PR description talks
about, previously prepare of a druid statement would actually plan the query,
and it would be re-used on subsequent executions, which is one of the
traditional reasons to use a prepared statement as I understand.
Planning in druid SQL entails using the calcite planner to convert to the
relational algebra constructs, and then converting those into native druid
queries, but when parameters are involved, that latter part is impossible
because native druid queries do not have this construct, nor does it
particularly make sense in that context. This is why the PR introduces an
explicit prepare to the druid planner, that is used to collect parameter
information by doing the first part of planning and examining the expression,
and later, ensure those get set by the time it is actually planned into a
native query.
I suppose there is room for improvement.. if we detect no parameters after
preparing, we could go ahead and plan as well, but I think I would prefer to do
this change in a follow-up if there is no strong feeling about this.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
