[jira] [Comment Edited] (CALCITE-5460) Types.className should not replace '$' to '.' if name of class contains $

["Dmitry Sysolyatin (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CALCITE-5460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17653321#comment-17653321 ] 

Dmitry Sysolyatin edited comment on CALCITE-5460 at 12/31/22 3:07 PM:
----------------------------------------------------------------------

[~julianhyde] In my understanding the purpose of Types.className is to return correct java classname that can be used inside `Expression` to generate correct java code.

I found this issue when was trying to implement UDF function using scala object method and calcite generated me wrong java code. 
For example:
{code:java}
import org.apache.calcite.linq4j.tree.Expressions
import org.scalatest.funsuite.AnyFunSuite

object FunctionImpls {
  def testFunc(): Boolean = true
}

// JAVA REPRESENTATION OF FunctionImpls object
// public final class FunctionImpls$ {
//  public static final FunctionImpls$ MODULE$ = new FunctionImpls$();
//
//  public boolean testFunc(final String arg) {
//    return true;
//  }
//
//  private FunctionImpls$() {
//  }
//}

class CalciteFuncTest extends AnyFunSuite {
  test("Get object instance expression") {
    val objectExpr = Expressions.field(null, FunctionImpls.getClass, "MODULE$")
    assert(objectExpr.toString == "FunctionImpls$.MODULE$") // without fix it will generate FunctionImpls..MODULE$ instead of FunctionImpls$.MODULE$
}

{code}
Calcite generates `FunctionImpls..MODULE$` instead of FunctionImpls$.MODULE$ without PR's fix.

From git history I found that `return className.replace('$', '.');` was added in order to generate correct class name for inner classes, but it does not take into account that class can contain `$` inside name. Instead of  using className.replace('$', '.'); would be better to use clazz.getCanonicalName() (it returns proper name, how it should be by Java Language Specification)


was (Author: dmsysolyatin):
[~julianhyde] In my understanding the purpose of Types.className is to return correct java classname that can be used inside `Expression` to generate correct java code.

I found this issue when was trying to implement UDF function using scala object method and calcite generated me wrong java code. 
For example:
{code:java}
import org.apache.calcite.linq4j.tree.Expressions
import org.scalatest.funsuite.AnyFunSuite

object FunctionImpls {
  def testFunc(): Boolean = true
}

// JAVA REPRESENTATION OF FunctionImpls object
// public final class FunctionImpls$ {
//  public static final FunctionImpls$ MODULE$ = new FunctionImpls$();
//
//  public boolean testFunc(final String arg) {
//    return true;
//  }
//
//  private FunctionImpls$() {
//  }
//}

class CalciteFuncTest extends AnyFunSuite {
  test("Get object instance expression") {
    val objectExpr = Expressions.field(null, FunctionImpls.getClass, "MODULE$")
    assert(objectExpr.toString == "FunctionImpls$.MODULE$") // without fix it will generate FunctionImpls..MODULE$ instead of FunctionImpls$.MODULE$
}

{code}
Calcite generates `FunctionImpls..MODULE$` instead of FunctionImpls$.MODULE$ without PR's fix.

From git history I found that `return className.replace('$', '.');` was added in order to generate correct class name for inner classes, but it does not handle all cases how you can see from example above. Instead of  using className.replace('$', '.'); would be better to use clazz.getCanonicalName() (it returns proper name, how it should be by Java Language Specification)

> Types.className should not replace '$' to '.' if name of class contains $
> -------------------------------------------------------------------------
>
>                 Key: CALCITE-5460
>                 URL: https://issues.apache.org/jira/browse/CALCITE-5460
>             Project: Calcite
>          Issue Type: Bug
>          Components: linq4j
>    Affects Versions: 1.32.0
>            Reporter: Dmitry Sysolyatin
>            Assignee: Dmitry Sysolyatin
>            Priority: Major
>              Labels: pull-request-available
>
> I faced with this problem when I was trying to use Types.className with scala object.
> Internally, scala object is instance of class "<full_object_name>$". If Types.className is applied to this class then $ will be replaced by '.'. But it is wrong behaviour
> The same behaviour will be for any class names which contain symbol $.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)