remove ExtractionFn.apply(Object) method

[navis]

It's really trivial but I think worth to be looked. Afaik it's applied to dimension, which is mostly string type and long type for time dimension. But it takes Object as argument, making me dizzy thinking what this possibly can be when implementing the function. Can we remove that? I think in someday, we would need something like apply(Object value, ValueType type) but for now, it seemed to much.

[jon-wei]

I think we should keep apply(Object), I have a patch WIP that enables long/float dimensions, and it uses that interface:

https://github.com/jon-wei/druid/tree/flex_dims_pt2

If we are to change the ExtractionFn interface, I think it actually makes more sense to collapse all of the apply() functions to apply(Object).

"Stringy" extraction functions like Regex can call .toString() to convert the Object as needed, and other extraction functions like Javascript that are capable of accepting Object should have an interface to do so (JavascriptExtractionFn currently converts Objects to String, but the compiled function is able to accept Object)

[xvrl]

@navis see here for the of the discussion why Object was introduced #1159. The main reason was in anticipation of having other types of dimensions, including float and long, and so that new types of extraction functions would not need to implement every possible type.

[navis]

Yes, it would be better to have multiple methods for each type or only one generic(Object) method. But currently it's not either and that induces confusing. Anyway I've understood current state and thinking of closing this.

One last question. Would it be worse to have,

String apply(Object value, ValueType type);

with

public enum ValueType
{
  FLOAT {
    @Override
    public Float getFloat(Object obj)
    {
      return (Float) obj;
    }
  },
  LONG {
    @Override
    public Long getLong(Object obj)
    {
      return (Long) obj;
    }
  },
  STRING,
  COMPLEX;

  public Float getFloat(Object obj)
  {
    throw new UnsupportedOperationException("getFloat");
  }

  public Long getLong(Object obj)
  {
    throw new UnsupportedOperationException("getLong");
  }

  public String getString(Object obj)
  {
    return String.valueOf(obj);
  }
}

[navis]

With above change, TimeExtractionFunction will have a method like,

@Override
public String apply(Object value, ValueType type)
{
  if (type == ValueType.LONG) {
    return formatter.print(type.getLong(value));
  }
  return formatter.print(new DateTime(type.getString(value)));
}

[stale]

This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@druid.apache.org list. Thank you for your contributions.

[stale]

This pull request has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.