[CALCITE-6383] The class SameOperandTypeChecker is incorrectly documented

• Type: Bug
• Status: Closed
• Priority: Minor
• Resolution: Fixed
• Affects Version/s: 1.36.0
• Fix Version/s: 1.38.0
• Component/s: core
• Labels: pull-request-available
• Assignee: Unassigned
• Reporter: Mihai Budiu
• Created: 23/Apr/24 23:14
• Updated: 17/Oct/24 22:41
• Resolved: 17/Oct/24 22:41

Description

The SameOperandTypeChecker claims that it checks whether operands have the same type (the class name suggests this, as does the JavaDoc).

/**
 * Parameter type-checking strategy where all operand types must be the same.
 */
public class SameOperandTypeChecker implements SqlSingleOperandTypeChecker {

But the code does something this:

    for (int i : operandList) {
      if (prev >= 0) {
        if (!SqlTypeUtil.isComparable(types[i], types[prev])) {

The documentation for isComparable says:

  /**
   * Returns whether two types are comparable. They need to be scalar types of
   * the same family, or struct types whose fields are pairwise comparable.

Thus the class only checks that the operands have the same type family, not the same type.

I am not sure what the right fix is, though, since changing the class name would be a pretty big breaking change. But I suspect this confusion is a source of a few bugs. An instance is CALCITE-6382

Issue Links

• links to: GitHub Pull Request #3774