[
https://issues.apache.org/jira/browse/CALCITE-5839?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17742430#comment-17742430
]
Ruben Q L edited comment on CALCITE-5839 at 7/12/23 1:43 PM:
-------------------------------------------------------------
Credits to [~thomas.rebele] for finding this.
[~zabetak], sorry to ping you directly, but it would seem it was you who
introduced the {{StaticFieldDetector}}. Is the analysis in the ticket
description correct (i.e. this is a real bug)? Or am I missing something?
It would seem the code should rather be:
{code}
@Override public Void visit(final FieldDeclaration fieldDeclaration) {
containsStaticField |= (fieldDeclaration.modifier & Modifier.STATIC) != 0;
return containsStaticField ? null : super.visit(fieldDeclaration);
}
{code}
or alternatively maybe:
{code}
@Override public Void visit(final FieldDeclaration fieldDeclaration) {
if (containsStaticField) {
return null;
}
containsStaticField = (fieldDeclaration.modifier & Modifier.STATIC) != 0;
return containsStaticField ? null : super.visit(fieldDeclaration);
}
{code}
to ensure the correct result?
was (Author: rubenql):
Credits to [~thomas.rebele] for finding this.
[~zabetak], sorry to ping you directly, but it would seem it was you who
introduced the {{StaticFieldDetector}}. Is the analysis in the ticket
description correct (i.e. this is a real bug)? Or am I missing something?
It would seem the code should rather be:
{code}
@Override public Void visit(final FieldDeclaration fieldDeclaration) {
containsStaticField |= (fieldDeclaration.modifier & Modifier.STATIC) != 0;
return containsStaticField ? null : super.visit(fieldDeclaration);
}
{code}
or alternatively maybe:
{code}
@Override public Void visit(final FieldDeclaration fieldDeclaration) {
if (containsStaticField) {
return null;
}
containsStaticField = (fieldDeclaration.modifier & Modifier.STATIC) != 0;
return containsStaticField ? null : super.visit(fieldDeclaration);
}
{code}
oo ensure the correct result?
> EnumerableInterpretable#StaticFieldDetector can overwrite its flag and return
> an incorrect result
> -------------------------------------------------------------------------------------------------
>
> Key: CALCITE-5839
> URL: https://issues.apache.org/jira/browse/CALCITE-5839
> Project: Calcite
> Issue Type: Bug
> Components: core
> Reporter: Ruben Q L
> Assignee: Ruben Q L
> Priority: Major
>
> 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):
> {code}
> 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);
> }
> }
> {code}
> 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).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
