[bug] WIP fix to deduplicate drilldowns

[AnthonyHewins]

This is definitely not done, but if you want to check if this is anywhere in the expected ballpark, please go ahead

[jspeis]

@hwchen what do you think so far?

The only thing I might suggest adding so far would be a test case for the Ok case. Nice work so far @AnthonyHewins !

[hwchen]

Thanks again @AnthonyHewins for your work, really sorry I've been slow to respond, I'm a bit busy at the moment.

[AnthonyHewins]

Thanks for the comments, I just updated my PR. Added logic to check that measures and drilldowns both exist. Also made the assertions more verbose to make sure it was picking up on the right error.

Apologize if I'm lacking a bit of knowledge in the OLAP domain...I learned about them very briefly about 3-4 years ago and it wasn't in-depth. I will put aside some time to get up to speed, it's a realm I'm going to encounter anyways at some point.

[jspeis]

@AnthonyHewins this is looking very good! I will take a closer look tomorrow and test it out

[jspeis]

Looks good to me so far. @hwchen in terms of hooking this up, were you thinking that this would get called in do_aggregate?

[hwchen]

yup @jspeis that sounds good.

[jspeis]

@AnthonyHewins would you feel comfortable hooking up your validate function in the do_aggregate functions? (both in the regular and streaming modes?)

We're happy to help if you have any questions!

[AnthonyHewins]

Absolutely, this weekend is probably when I can get around to doing that. I may have some questions along the way but I'll just post them after I make a commit

[jspeis]

Great! And feel free to let us know if you have any questions

…

On Thu, Oct 10, 2019, 12:00 PM adm ***@***.***> wrote: Absolutely, this weekend is probably when I can get around to doing that. I may have some questions along the way but I'll just post them after I make a commit — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#195?email_source=notifications&email_token=AAAVMHN7BKKXVUEIM6T7YKTQN5GRRA5CNFSM4IWYYS3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEA434NI#issuecomment-540655157>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAVMHKR3EFZEVIE4FIFMKTQN5GRRANCNFSM4IWYYS3A> .

[AnthonyHewins]

Sorry it's taken some time, things got a little crazy the past two weeks, but I have gotten around to implementing part of it. I want to spend some time with actix so I know how to properly send the right error with FutureResponse before I push up though, once I get a chance to reach that part in the tutorial I will push up and hopefully this drawn-out PR will come to a close lol

[jspeis]

@AnthonyHewins thanks for sharing!

EDIT: sorry misread originally. let us know whenever is good! Thanks again for your contributions!