[ 
https://issues.apache.org/jira/browse/CALCITE-4419?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Ruben Q L updated CALCITE-4419:
-------------------------------
    Description: 
Posix regex operators are special, they are defined as binary ({{public class 
SqlPosixRegexOperator extends SqlBinaryOperator}}), but their actual 
implementation takes three arguments, the third being a caseSensitive boolean 
flag ({{SqlFunctions#posixRegex(String s, String regex, Boolean 
caseSensitive)}}).
 Using them in SQL works fine. The magic happens in 
{{SqlPosixRegexOperator#createCall}}, which adds an extra boolean parameter 
(the caseSensitive flag) into the call:
{code:java}
@Override public SqlCall createCall(SqlLiteral functionQualifier, SqlParserPos 
pos, SqlNode... operands) {
  pos = pos.plusAll(Arrays.asList(operands));
  operands = Arrays.copyOf(operands, operands.length + 1);
  operands[operands.length - 1] = SqlLiteral.createBoolean(caseSensitive, 
SqlParserPos.ZERO);
  return new SqlBasicCall(this, operands, pos, false, functionQualifier);
}
{code}
However, if we try to use them in a plan created by RelBuilder, problems occur.

1) The following plan:
{code:java}
RelBuilder builder = ...
builder
  .scan("s", "emps")
  .filter(
    builder.call(
      SqlStdOperatorTable.POSIX_REGEX_CASE_SENSITIVE,
      builder.field("name"),
      builder.literal("E...")))
...
{code}

Fails because it cannot find the implementor method
{noformat}
Caused by: java.lang.IllegalStateException: Unable to implement 
EnumerableCalc(expr#0..4=[{inputs}], expr#5=['E...'], expr#6=[POSIX REGEX CASE 
SENSITIVE($t2, $t5)]...
...
Caused by: java.lang.NoSuchMethodException: 
org.apache.calcite.runtime.SqlFunctions.posixRegex(java.lang.String, 
java.lang.String)
{noformat}

2) The solution seems to add a (somehow redundant, but necessary due to posix 
regex operators design) third boolean parameter:
{code:java}
  .filter(
    builder.call(
      SqlStdOperatorTable.POSIX_REGEX_CASE_SENSITIVE,
      builder.field("name"),
      builder.literal("E..."),
      builder.literal(true)))
{code}

But it fails validating the operands:
{noformat}
wrong operand count 3 for POSIX REGEX CASE SENSITIVE
java.lang.AssertionError: wrong operand count 3 for POSIX REGEX CASE SENSITIVE
        at org.apache.calcite.util.Litmus$1.fail(Litmus.java:31)
        at 
org.apache.calcite.sql.SqlBinaryOperator.validRexOperands(SqlBinaryOperator.java:200)
{noformat}

3) So, it would seem {{SqlPosixRegexOperator}} should override 
{{validRexOperands}} to support 2 *and 3* parameters (which in fact would seem 
correct and aligned to the current overridden behavior of 
{{SqlPosixRegexOperator#getOperandCountRange}} and 
{{SqlPosixRegexOperator#checkOperandTypes}}:
{code:java}
  @Override public boolean validRexOperands(int count, Litmus litmus) {
    if (count != 2 && count != 3) {
      return litmus.fail("wrong operand count {} for {}", count, this);
    }
    return litmus.succeed();
  }
{code}

But now the test fails with almost the same message as the very first one:
{noformat}
Caused by: java.lang.IllegalStateException: Unable to implement 
EnumerableCalc(expr#0..4=[{inputs}], expr#5=['E...'], expr#6=[true], 
expr#7=[POSIX REGEX CASE SENSITIVE($t2, $t5, $t6)]...
...
Caused by: java.lang.NoSuchMethodException: 
org.apache.calcite.runtime.SqlFunctions.posixRegex(java.lang.String, 
java.lang.String, boolean)
{noformat}

4) The problem seems to be a mismatch between our third argument (boolean) and 
the implementor's third argument (Boolean). Looking at the code, it seems clear 
that a primitive boolean should / must be used since having a null Boolean 
object makes no sense (and will case a NPE anyway). So if we change it:
{code:java}
// BuiltInMethod.java
...
POSIX_REGEX(SqlFunctions.class, "posixRegex", String.class, String.class, 
Boolean.class), // instead of Boolean.class

// ---------

// SqlFunctions.java
...
public static boolean posixRegex(String s, String regex, boolean caseSensitive) 
// instead of Boolean
{ ... }
{code}

Now our test finally succeeds.
Therefore, I propose to implement these sequence of changes in order to solve 
this issue.

  was:
Posix regex operators are special, they are defined as binary ({{public class 
SqlPosixRegexOperator extends SqlBinaryOperator}}), but their actual 
implementation takes three arguments, the third being a caseSensitive boolean 
flag ({{SqlFunctions#posixRegex(String s, String regex, Boolean 
caseSensitive)}}).
 Using them in SQL works fine. The magic happens in 
{{SqlPosixRegexOperator#createCall}}, which adds an extra boolean parameter 
(the caseSensitive flag) into the call:
{code:java}
@Override public SqlCall createCall(SqlLiteral functionQualifier, SqlParserPos 
pos, SqlNode... operands) {
  pos = pos.plusAll(Arrays.asList(operands));
  operands = Arrays.copyOf(operands, operands.length + 1);
  operands[operands.length - 1] = SqlLiteral.createBoolean(caseSensitive, 
SqlParserPos.ZERO);
  return new SqlBasicCall(this, operands, pos, false, functionQualifier);
}
{code}
However, if we try to use them in a plan created by RelBuilder, problems occur.

1) The following plan:
{code:java}
RelBuilder builder = ...
builder
  .scan("s", "emps")
  .filter(
    builder.call(
      SqlStdOperatorTable.POSIX_REGEX_CASE_SENSITIVE,
      builder.field("name"),
      builder.literal("E...")))
...
{code}

Fails because it cannot find the implementor method
{noformat}
Caused by: java.lang.IllegalStateException: Unable to implement 
EnumerableCalc(expr#0..4=[{inputs}], expr#5=['E...'], expr#6=[POSIX REGEX CASE 
SENSITIVE($t2, $t5)]...
...
Caused by: java.lang.NoSuchMethodException: 
org.apache.calcite.runtime.SqlFunctions.posixRegex(java.lang.String, 
java.lang.String)
{noformat}

2) The solution seems add a (somehow redundant, but necessary due to posix 
regex operators design) third boolean parameter:
{code:java}
  .filter(
    builder.call(
      SqlStdOperatorTable.POSIX_REGEX_CASE_SENSITIVE,
      builder.field("name"),
      builder.literal("E..."),
      builder.literal(true)))
{code}

But it fails validating the operands:
{noformat}
wrong operand count 3 for POSIX REGEX CASE SENSITIVE
java.lang.AssertionError: wrong operand count 3 for POSIX REGEX CASE SENSITIVE
        at org.apache.calcite.util.Litmus$1.fail(Litmus.java:31)
        at 
org.apache.calcite.sql.SqlBinaryOperator.validRexOperands(SqlBinaryOperator.java:200)
{noformat}

3) So, it would seem {{SqlPosixRegexOperator}} should override 
{{validRexOperands}} to support 2 *and 3* parameters (which in fact would seem 
correct and aligned to the current overridden behavior of 
{{SqlPosixRegexOperator#getOperandCountRange}} and 
{{SqlPosixRegexOperator#checkOperandTypes}}:
{code:java}
  @Override public boolean validRexOperands(int count, Litmus litmus) {
    if (count != 2 && count != 3) {
      return litmus.fail("wrong operand count {} for {}", count, this);
    }
    return litmus.succeed();
  }
{code}

But now the test fails with almost the same message as the very first one:
{noformat}
Caused by: java.lang.IllegalStateException: Unable to implement 
EnumerableCalc(expr#0..4=[{inputs}], expr#5=['E...'], expr#6=[true], 
expr#7=[POSIX REGEX CASE SENSITIVE($t2, $t5, $t6)]...
...
Caused by: java.lang.NoSuchMethodException: 
org.apache.calcite.runtime.SqlFunctions.posixRegex(java.lang.String, 
java.lang.String, boolean)
{noformat}

4) The problem seems to be a mismatch between our third argument (boolean) and 
the implementor's third argument (Boolean). Looking at the code, it seems clear 
that a primitive boolean should / must be used since having a null Boolean 
object makes no sense (and will case a NPE anyway). So if we change it:
{code:java}
// BuiltInMethod.java
...
POSIX_REGEX(SqlFunctions.class, "posixRegex", String.class, String.class, 
Boolean.class), // instead of Boolean.class

// ---------

// SqlFunctions.java
...
public static boolean posixRegex(String s, String regex, boolean caseSensitive) 
// instead of Boolean
{ ... }
{code}

Now our test finally succeeds.
Therefore, I propose to implement these sequence of changes in order to solve 
this issue.


> Posix regex operators cannot be used within RelBuilder
> ------------------------------------------------------
>
>                 Key: CALCITE-4419
>                 URL: https://issues.apache.org/jira/browse/CALCITE-4419
>             Project: Calcite
>          Issue Type: Bug
>          Components: core
>            Reporter: Ruben Q L
>            Assignee: Ruben Q L
>            Priority: Major
>             Fix For: 1.27.0
>
>
> Posix regex operators are special, they are defined as binary ({{public class 
> SqlPosixRegexOperator extends SqlBinaryOperator}}), but their actual 
> implementation takes three arguments, the third being a caseSensitive boolean 
> flag ({{SqlFunctions#posixRegex(String s, String regex, Boolean 
> caseSensitive)}}).
>  Using them in SQL works fine. The magic happens in 
> {{SqlPosixRegexOperator#createCall}}, which adds an extra boolean parameter 
> (the caseSensitive flag) into the call:
> {code:java}
> @Override public SqlCall createCall(SqlLiteral functionQualifier, 
> SqlParserPos pos, SqlNode... operands) {
>   pos = pos.plusAll(Arrays.asList(operands));
>   operands = Arrays.copyOf(operands, operands.length + 1);
>   operands[operands.length - 1] = SqlLiteral.createBoolean(caseSensitive, 
> SqlParserPos.ZERO);
>   return new SqlBasicCall(this, operands, pos, false, functionQualifier);
> }
> {code}
> However, if we try to use them in a plan created by RelBuilder, problems 
> occur.
> 1) The following plan:
> {code:java}
> RelBuilder builder = ...
> builder
>   .scan("s", "emps")
>   .filter(
>     builder.call(
>       SqlStdOperatorTable.POSIX_REGEX_CASE_SENSITIVE,
>       builder.field("name"),
>       builder.literal("E...")))
> ...
> {code}
> Fails because it cannot find the implementor method
> {noformat}
> Caused by: java.lang.IllegalStateException: Unable to implement 
> EnumerableCalc(expr#0..4=[{inputs}], expr#5=['E...'], expr#6=[POSIX REGEX 
> CASE SENSITIVE($t2, $t5)]...
> ...
> Caused by: java.lang.NoSuchMethodException: 
> org.apache.calcite.runtime.SqlFunctions.posixRegex(java.lang.String, 
> java.lang.String)
> {noformat}
> 2) The solution seems to add a (somehow redundant, but necessary due to posix 
> regex operators design) third boolean parameter:
> {code:java}
>   .filter(
>     builder.call(
>       SqlStdOperatorTable.POSIX_REGEX_CASE_SENSITIVE,
>       builder.field("name"),
>       builder.literal("E..."),
>       builder.literal(true)))
> {code}
> But it fails validating the operands:
> {noformat}
> wrong operand count 3 for POSIX REGEX CASE SENSITIVE
> java.lang.AssertionError: wrong operand count 3 for POSIX REGEX CASE SENSITIVE
>       at org.apache.calcite.util.Litmus$1.fail(Litmus.java:31)
>       at 
> org.apache.calcite.sql.SqlBinaryOperator.validRexOperands(SqlBinaryOperator.java:200)
> {noformat}
> 3) So, it would seem {{SqlPosixRegexOperator}} should override 
> {{validRexOperands}} to support 2 *and 3* parameters (which in fact would 
> seem correct and aligned to the current overridden behavior of 
> {{SqlPosixRegexOperator#getOperandCountRange}} and 
> {{SqlPosixRegexOperator#checkOperandTypes}}:
> {code:java}
>   @Override public boolean validRexOperands(int count, Litmus litmus) {
>     if (count != 2 && count != 3) {
>       return litmus.fail("wrong operand count {} for {}", count, this);
>     }
>     return litmus.succeed();
>   }
> {code}
> But now the test fails with almost the same message as the very first one:
> {noformat}
> Caused by: java.lang.IllegalStateException: Unable to implement 
> EnumerableCalc(expr#0..4=[{inputs}], expr#5=['E...'], expr#6=[true], 
> expr#7=[POSIX REGEX CASE SENSITIVE($t2, $t5, $t6)]...
> ...
> Caused by: java.lang.NoSuchMethodException: 
> org.apache.calcite.runtime.SqlFunctions.posixRegex(java.lang.String, 
> java.lang.String, boolean)
> {noformat}
> 4) The problem seems to be a mismatch between our third argument (boolean) 
> and the implementor's third argument (Boolean). Looking at the code, it seems 
> clear that a primitive boolean should / must be used since having a null 
> Boolean object makes no sense (and will case a NPE anyway). So if we change 
> it:
> {code:java}
> // BuiltInMethod.java
> ...
> POSIX_REGEX(SqlFunctions.class, "posixRegex", String.class, String.class, 
> Boolean.class), // instead of Boolean.class
> // ---------
> // SqlFunctions.java
> ...
> public static boolean posixRegex(String s, String regex, boolean 
> caseSensitive) // instead of Boolean
> { ... }
> {code}
> Now our test finally succeeds.
> Therefore, I propose to implement these sequence of changes in order to solve 
> this issue.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to