Join context hints

[adarshsanjeev]

Rebased and addressed some issues of #17406

Currently, using the query context, all joins in a query can be hinted to either broadcast or sort merge join. However, this does not allow this to be set at a join level.

Add support for hints to queries, which can dictate the join type.

Syntax

select /*+ sort_merge */ w1.cityName, w2.countryName
from
(
  select /*+ broadcast */ w3.cityName AS cityName, w4.countryName AS countryName from wikipedia w3 LEFT JOIN wikipedia-set2 w4 ON w3.regionName = w4.regionName
) w1
JOIN wikipedia-set1 w2 ON w1.cityName = w2.cityName
where w1.cityName='New York';

Note
Calcite does not expose if the hint is at a particular join level, and allows inheritance of hints from parent nodes. This causes join hints to to affect inner queries. Additional join hints can be added to inner queries as a workaround.

---

This PR has:

• been self-reviewed.
  • using the concurrency checklist (Remove this item if the PR doesn't have any relation to concurrency.)
• added documentation for new or modified features or behaviors.
• a release note entry in the PR description.
• added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
• added or updated version, license, or notice information in licenses.yaml
• added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
• added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
• added integration tests.
• been tested in a test Druid cluster.

[kgyrtkirk]

I think it would be better to not have this as an enum - and you could probably use as(JoinAlgorithm.class) or asJoinAlgorithm() to obtain the current value

[adarshsanjeev]

There's an issue with that, Calcite only seems to accept certain keywords as hints, and this is sort_merge. Trying to replace the hint name with sortMerge seems to make it not read by calcite.

[kgyrtkirk]

doesn't really mattter - I was just thinking something like:

public abstract class DruidHint {
  abstract T as(Class<T> clazz);

  public static DruidHint fromRelHint(RelHint rh) {
    // factory method
  }

  static abstrat class DruidJoinHint extends DruidHint {}

  static class SortMerge extends DruidJoinHint {
     String name() { "sort_merge"; }
     as() ... { return
  }
}

[adarshsanjeev]

Made some changes to make it more in line with this, please take a look

[kgyrtkirk]

I don't think this should be serializable by jackson; if it gets on the wire it should be a JoinAlgorithm already

[adarshsanjeev]

I think it might have been to deserializer it for the planner context at some point

[kgyrtkirk]

I don't think this JoinHint should be serialized
if its not in use I think it would be better to remove it

[kgyrtkirk]

doesn't really mattter - I was just thinking something like:

public abstract class DruidHint {
  abstract T as(Class<T> clazz);

  public static DruidHint fromRelHint(RelHint rh) {
    // factory method
  }

  static abstrat class DruidJoinHint extends DruidHint {}

  static class SortMerge extends DruidJoinHint {
     String name() { "sort_merge"; }
     as() ... { return
  }
}

[kgyrtkirk]

I don't think this JoinHint should be serialized
if its not in use I think it would be better to remove it

[kgyrtkirk]

is the usage of name() intended here?
because I think that will return BROADCAST the "name" of the enum key and not the id!

[kgyrtkirk]

note: this method returning HintStrategyTable could live inside DruidHint if that's created

[adarshsanjeev]

Moved to DruidHint

[kgyrtkirk]

note: I was thinking to use DruidJoinHint here.. so that you can't even set closesHint to a non-join hint...but its not really important

[adarshsanjeev]

Keeping the RelHint allows keeping track the closestHint's path length, to compare in the for loop. We do have a check to see if it's a join hint in the loop so this should be fine