slim bouguerra, I see your pull request https://github.com/apache/calcite/pull/444. However, there seems to be a bigger problem that we don't check whether Druid can handle a particular expression before we create a DruidFilter. Can we solve that once and for all?

Julian Hyde You are right this patch fixes that specific case.
Fixing the general issue might be more invasive change.
The way i will do it i will parse the filter tree at FilterRule class and try to generate the Druid Json and push the filter only and only if i have a valid Druid filter like we can be 100% sure we can push it.
As you can see this approach is totally different from current way of doing things where we use isValidFilter not sure if that is the way how calcite community like thing to be done ?
What do you think about the approach ?
How about fix this small issue and tackle the bigger one in a different scope ?

I was thinking of a simpler change, that is just in Calcite. There are 2 pieces of logic in Calcite: (1) Can we push this expression to Druid?, (2) Generate Druid JSON for this expression. We need to unify those pieces of logic to make sure they are consistent.

DruidRules.splitFilters is the method responsible for (1) but AFAICT it doesn't look too closely at the expression. In this case, it lets through "false". But I suspect it would also let through, say, "substring($1, 2, 4) = 'Foo'" even though Druid doesn't support the substring operator.

I think what i proposed is just in calcite and i think it joins what your are describing above, when i said generate the Json Filter and push only if the filter json is a valid Druid filter means use the code generating the json string to examine the rel nodes, like that if we get a null string we know we can not push the filter.
are we in the same page ?

Yes, we're on the same page.

Most adapters have to do something similar to this. But they tend to do it in an ad hoc way too. CalcRelSplitter could potentially be used to split up expressions into pieces that can and cannot be pushed.

I'll commit your patch, but please log a case for the rest of the work.

Fixed in http://git-wip-us.apache.org/repos/asf/calcite/commit/520c0ccc. Thanks for the PR, slim bouguerra!

Julian Hyde, substring should not be pushed, as it is not part of the supported operators in isValidFilter.

slim bouguerra, in this specific case, translation was failing because 'false' is a constant and we are assuming that constants can be translated in every case, but actually we cannot translate them if they are at the top level of a predicate, e.g., true, or as an input to some other expressions, e.g., a = 5 OR true. This patch has fixed the 'false' constant issue, but we might face same problem for these other cases (probably difficult to reproduce because of constant folding). IMO the right approach was to fix this in isValidFilter to skip the pushdown translation in these cases, till we have a more robust unified logic like you propose above (which seems a better solution altogether).

Resolved in release 1.13.0 (2017-06-26).