Support multi-dimensional filters

[navis]

Work for #2187

I didn't know that there is already a PR(#2210) on that till this tomorrow. But things done and I'm making PR to be used as reference purpose.

[fjy]

Hi @navis, can you and @sirpkt consolidate #2210 and #2217 into a single PR?

[navis]

@fjy Each are implemented in (much) different manner. Might be hard to consolidate.

[fjy]

@navis it'll be challenging to review both if they overlap in functionality :), do you guys want to have an internal discussion about which one people should review?

[navis]

@fjy We should do ;). Could you wait for sometime before starting the reviewing?

[fjy]

@navis I'm guessing this is the PR to review right?

[navis]

@fjy I've talked with @sirpkt and concluded we need both of implementation in druid because this(#2217) has different semantic with current implementation on multi-valued dimensions, but which might be useful in some cases.

[fjy]

@navis okay. Will @sirpkt reopen his PR? Just as an FYI, the JS implementations of features will be much slower than native Java implementations.

[navis]

merged #2210

[xvrl]

I would prefer if we did not introduce a second javascript filter and instead merge this into the existing one.
We should maintain backwards compatibility for the old syntax, but use the new one by default.

[navis]

Yes, but each filter has different semantic and hard to merge into single one.

[fjy]

I think it is better to have a flag on javascript filter that utimately leads to two code paths, one that uses indexes, and one that doesnt rather htan having two separate implementations. Two impls will confuse users.

[navis]

@fjy You are right. How couldn't I thought of that?

[fjy]

if this goes into 0.9.0 (which I think it should), we can make the backwards incompatible API change

[xvrl]

we need to maintain backwards compatibility to allow for rolling upgrades.

[fjy]

@xvrl is correct. I didn't think about rolling upgrades. But we can remove "dimension" from the docs and let new people use "dimensions" everywhere

[fjy]

@navis let's remove "dimension" from the docs but keep it in the code

[himanshug]

+1 for removing dimension from docs. also, let us explicitly call it deprecated in 0.9.0 release notes so that we can remove it in later release from the code.

[fjy]

is this needed?

[navis]

will be removed.

[navis]

Test failed by regression of #2006

[fjy]

@navis I think the issues you brought up in #2006 should now be addressed

[fjy]

minor nit, but can we move static functions to the top of the class? similar to other druid files

[navis]

I remember someone has told me static class should be at the bottom of the class in other PR. Then methods to top and classes to bottom. Right?

[navis]

moved up

[fjy]

@navis we can move this to 0.9.1 as I think it might take a little bit more work to finish this one up.

[cheddar]

What is the difference between this method and makeValueMatcher?

[cheddar]

A part of me wonders if this wouldn't be simpler if we were to add the method

public ValueMatcher makeValueMatcher(ColumnSelectorFactory metricFactory);

to the ValueMatcherFactory interface?

[navis]

@fjy We have a plan to use this but currently it's not yet applied. So no need to rush to include this in near release versions.
@cheddar It's an internal interface only used internally. We can do anything with that.

[b-slim]

this is assuming that user has to call hasNext() before calling next() which i think is not part of the Iterator API, or i am missing something ?

[navis]

done

[fjy]

@navis any chance we can finish this one up? I think @cheddar's comment is reasonable

[fjy]

@navis ?

[navis]

@fjy Forgot for a long time. I'm on it.

[navis]

Rebased on master. Because there has been so many changes in here about, I've make code barely compile. let's see the test results.

[navis]

cannot sustain this, sorry.