Github user hsuanyi commented on the pull request:
https://github.com/apache/drill/pull/439#issuecomment-200919168
In addition to @jinfengni's point:
@sudheeshkatkam did a proposal to add a new SqlAvgAggFunction (the
inference mechanism of the original is not ideal for Drill). It was declined by
Calcite's community, and the argument was adding unnecessary duplicates to the
code. I think this argument is fair. Otherwise, there might be tens of variants
for a single operator.
Regarding adding a new converlet:
Initially, I though having three wrappers (operator, function, agg
function) are sufficient for all scenarios. However, at one method during
conversion, the type SqlBetweenOperator is being expected. Thus, I decided to
add a new wrapper which extends SqlBetweenOperator, so we do not need to do
anything in conversion.
Certainly, we might be able to attain by not adding a new wrapper but
adding a new converlet. I would not prefer this approach since
1. I tried not to spread out the code complexity (introduced by inference
and operand type check) to too many places.
2. Adding our own converlet also introduces code complexity. And it
probably is more complicated than a wrapper.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
