Fix using undefined DruidMetrics for limit_spec

[bolkedebruin]

in case a form did not specify a metric (e.g. mapbox). If it is
not available it now defaults to the first dimension instead.

This fixes issue #3604

@mistercrunch @Fokko @amoussoubaruch

It probably warrants a unittest, I am still figuring out how those work in Superset (ie. I know where to find them, but trying to wrap my mind around what the right adjustment is)

[mistercrunch]

This part of the codebase is growing a bit out of control (not your fault! it's been slipping out of control for a long time...). . Sidenote here is that the future way forward with Druid (according to Druid core committers) is with the SQL interface. For now the SQL interface vs JSON still has pros/cons and I'd assume many people are using earlier versions of Druid where JSON is the only way, so we'll have to support the JSON connector for a while.

Coworker @betodealmeida at Lyft just rolled out https://github.com/betodealmeida/druid-dbapi/ which has a DBAPI driver and SQLAlchemy dialect for Druid, meaning we'll be able to use Druid through the SQLAlchemy interface.

---

So is it fair to say that this bug occurs with metrics that have groupby but no metrics?

Now if it's a bug with Mapbox viz, it seems like is_timeseries should be False and we shouldn't be in the section of the code under if timeseries_limit and is_timeseries: (unless this bug occurs in another context?)

[bolkedebruin]

Ah that looks great! I’m aware of the effort to create a sql interface to Druid. That should put adoption on steroids I guess, but it will take awhile before that is deemed stable.

So I think you’re right on how the issue is triggered from MapBox, due to the absence of a defined metric in the UI superset defaults to a set of pre-defined ones which 1) are not in the query and thus not known to Druid and 2) not normalize (as done at other places) to just it’s value hence being unserializable.

The pr uses the dimension as per Druid spec if a metric is absent.

So setting ‘is_timeseries’ to false probably fixes the symptom but will hide the bug.

[bolkedebruin]

@mistercrunch anything holding this back? It is an error in the Druid interface (and will not just affect MapBox).

[mistercrunch]

Ooops, that one had slipped in a crack, mergin'