[CALCITE-5839] EnumerableInterpretable#StaticFieldDetector can overwrite its flag and return an incorrect result

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Fix Version/s: 1.35.0
• Component/s: core
• Labels: pull-request-available
• Assignee: Ruben Q L
• Reporter: Ruben Q L
• Created: 12/Jul/23 13:39
• Updated: 27/Jul/23 10:22
• Resolved: 12/Jul/23 15:36

Description

In EnumerableInterpretable, before using the bindable cache, it is verified whether the class contains static fields (because in that case, the cache shall not be used):

  static Bindable getBindable(ClassDeclaration expr, ...) {
    ...
    if (CalciteSystemProperty.BINDABLE_CACHE_MAX_SIZE.value() != 0) {
      StaticFieldDetector detector = new StaticFieldDetector();
      expr.accept(detector);
      if (!detector.containsStaticField) {
        return BINDABLE_CACHE.get(classBody, () ->  compileToBindable(expr.name, s, compiler));
      }
    }

  ....

  /**
   * A visitor detecting if the Java AST contains static fields.
   */
  static class StaticFieldDetector extends VisitorImpl<Void> {
    boolean containsStaticField = false;

    @Override public Void visit(final FieldDeclaration fieldDeclaration) {
      containsStaticField = (fieldDeclaration.modifier & Modifier.STATIC) != 0;
      return containsStaticField ? null : super.visit(fieldDeclaration);
    }
  }

However, it seem that the containsStaticField flag in {StaticFieldDetector can be overwritten when inspecting the field declarations of the class, so that, if a non-static field is inspected after a static field, the flag will be then (re)set to false; and the final result of the shuttle will be wrong (false, when it should have been true).

Issue Links

• links to: GitHub Pull Request #3312