Some dimensions are listed as measures.

[vincue]

Have resolved the issue, how to raise a PR if possible?

[adrianmroz]

First we need bug report which describes problem so we can understand the issue, even if it doesn't happen in our case.

Then create pull request that resolves issue in standard-github way (https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/creating-a-pull-request) Bonus points for linking pull request with issue :)

[vincue]

Currently all number type columns are listed as measures, even though they might not necessarily be. So an additional check - if a maker(the formula) for underlying column exists to classify it as measure.

[adrianmroz]

I don't follow. Turnilo works in two separate modes - either it introspects druid metadata and assumes that all number columns are measures or it does exactly what you told it in config file. In what scenario you have formula for column in config but you ask for druid metadata anyway?

[vincue]

In introspecting druid metadata scenario(in file), just another check as mentioned previous comment.

[youngsol]

@adrianmroz-allegro I believe it is fixed in 1.33. I can see whole code structures in model/datacube.ts is changed. I am leaving a comment for users who are using turnilo 1.32

Problems: If you use turnilo config shown below, turnilo will inspect druid and determine columns whether a column is a dimension or measure(metric). Currently all numbers type columns are passed to measures.

config.yaml

clusters:
  - name: druid
    url: "some host name"

https://github.com/allegro/turnilo/blob/5375f9eede5a127b24a9d79f5d7881ce120e830e/src/common/models/data-cube/data-cube.ts#L827-L840

As you can see all number type columns are treated as measures.

To fix this, you have to add something like this. I am not a nodejs person. Feel free to correct me if something is wrong. I fixed it after build ( js file not in typescript (src)). Also in the master branch, data-cube.ts is reformed and I believe this bug will be fixed in 1.33.

Turnilo 1.32

build/common/models/data-cube/data-cube.js

case "NUMBER":
    if (!newAttribute.maker){
        if (!autofillDimensions)
            continue;
        expression = plywood_1.$(name);
        if (this.getDimensionByExpression(expression))
            continue;
        dimensions = dimensions.append(new dimension_1.Dimension({
            name: urlSafeName,
            kind: "number",
            formula: expression.toString()
        }));
        break;
    }
case "NULL":

[adrianmroz-allegro]

@adrianmroz-allegro I believe it is fixed in 1.33. I can see whole code structures in model/datacube.ts is changed.

Not really, old code for introspection has not changed (https://github.com/allegro/turnilo/blob/master/src/common/models/data-cube/queryable-data-cube.ts#L54)

And I don't understand why "fixed"? What is broken here and should be fixed? If user doesn't define if numerical column is a dimension or measure, turnilo can't guess correctly. It just assumes that it is a measure, because not very often we have numerical dimensions. Proper way to fix this is to define config file with dimension type.

As you can see all number type columns are treated as measures.

Yes, because we don't have any information beside type.

To fix this, you have to add something like this.

Turnilo 1.32

build/common/models/data-cube/data-cube.js

case "NUMBER":
    if (!newAttribute.maker){
        if (!autofillDimensions)
            continue;
        expression = plywood_1.$(name);
        if (this.getDimensionByExpression(expression))
            continue;
        dimensions = dimensions.append(new dimension_1.Dimension({
            name: urlSafeName,
            kind: "number",
            formula: expression.toString()
        }));
        break;
    }
case "NULL":

But that will convert all numerical columns to dimensions. I don't see how it is better? You just picked different default.

[youngsol]

@adrianmroz-allegro I see that file name has changed. I was looking at wrong file lol.

Yes, because we don't have any information beside type.

if the column is metric, then it should have some operation.

But that will convert all numerical columns to dimensions. I don't see how it is better? You just picked different default.

yes It will send all numerical columns to dimensions if the column does not have any operation. If a user want it to be metric, I think that should be set in config not by default since default formula is "SUM" the column anyway. "SUM" may not be the what user want.

[adrianmroz-allegro]

I don't follow.

If user set dimension or measure in config file, turnilo will respect your decision after introspection. It won't change it. If user didn't define dimension or measure, all we have is column name and type. And here turnilo guesses, and if it sees number value, it will guess a measure. Columns don't have operations in druid.

[youngsol]

@adrianmroz-allegro

If user set dimension or measure in config file, turnilo will respect your decision after introspection. It won't change it.

That is correct. I am changing the default behavior of turnilo if a user runs turnilo without config. Like I mentioned above

clusters:
  - name: druid
    url: "some host name"

IMO, If a user wants number type column as measures It should set it manually. It the column is metric, it will have some operation. Then it goes to measures.

    if (!newAttribute.maker){

[maorelias]

Hi, any updates about this issue? it's pretty confusing to see the numeric columns listed under measures and pretty frustrating not being able to split or filter based on them :/