[jira] [Commented] (CALCITE-4421) AssertionError is thrown with empty message

["Chunwei Lei (Jira)" <ji...@apache.org>]

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

Chunwei Lei commented on CALCITE-4421:
--------------------------------------

Thank you for your feedback, Julian. As for the example shown in the description, I think:

1) an IllegalArgumentException would be better than AssertionError considering the condition that the size of two lists should be equal is required.

2) It is not an internal error. It's caused by my mistake. But I can not find what goes wrong until I check the code.

So I suggest we can change it to:
{code:java}
if (strict) {
  if (ks.size() != vs.size()) {
    throw new IllegalArgumentException("The size of two lists is required to be equal.");
  }
{code}
 

> AssertionError is thrown with empty message
> -------------------------------------------
>
>                 Key: CALCITE-4421
>                 URL: https://issues.apache.org/jira/browse/CALCITE-4421
>             Project: Calcite
>          Issue Type: Improvement
>            Reporter: Chunwei Lei
>            Assignee: Chunwei Lei
>            Priority: Major
>
> I happened to meet the following error:
> {code:java}
> System internal error - java.lang.AssertionError
> at org.apache.calcite.util.Pair.zip(Pair.java:202)
> at org.apache.calcite.rex.RexUtil.generateCastExpressions(RexUtil.java:134)
> at org.apache.calcite.rex.RexUtil.generateCastExpressions(RexUtil.java:116)
> at org.apache.calcite.plan.RelOptUtil.createCastRel(RelOptUtil.java:728)
> {code}
>  I checked the code and It indeed would throw an exception with an empty message.
> {code:java}
> public static <K, V> List<Pair<K, V>> zip(
>     final List<? extends K> ks,
>     final List<? extends V> vs,
>     boolean strict) {
>   final int size;
>   if (strict) {
>     if (ks.size() != vs.size()) {
>       throw new AssertionError();
>     }
> {code}
> I think a more useful exception message would be much better.
> BTW, there're many other places like this:
>  
> {code:java}
> calcite$ grep 'new AssertionError()' -r ./ -l
> .//core/src/test/java/org/apache/calcite/test/catalog/MockCatalogReaderExtended.java
> .//core/src/main/java/org/apache/calcite/util/mapping/IntPair.java
> .//core/src/main/java/org/apache/calcite/util/NlsString.java
> .//core/src/main/java/org/apache/calcite/util/Pair.java
> .//core/src/main/java/org/apache/calcite/util/PrecedenceClimbingParser.java
> .//core/src/main/java/org/apache/calcite/util/ImmutableBeans.java
> .//core/src/main/java/org/apache/calcite/plan/RelTraitPropagationVisitor.java
> .//core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
> .//core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableTableModify.java
> .//core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcRules.java
> .//core/src/main/java/org/apache/calcite/jdbc/CalciteConnectionImpl.java
> .//core/src/main/java/org/apache/calcite/rex/RexLiteral.java
> .//core/src/main/java/org/apache/calcite/rex/RexInterpreter.java
> .//core/src/main/java/org/apache/calcite/rex/RexBuilder.java
> .//core/src/main/java/org/apache/calcite/rex/RexSimplify.java
> .//core/src/main/java/org/apache/calcite/rel/rules/LoptMultiJoin.java
> .//core/src/main/java/org/apache/calcite/rel/rules/ProjectTableScanRule.java
> .//core/src/main/java/org/apache/calcite/rel/rules/FilterTableScanRule.java
> .//core/src/main/java/org/apache/calcite/rel/metadata/RelMdColumnUniqueness.java
> .//core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
> .//core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java
> .//core/src/main/java/org/apache/calcite/materialize/Lattice.java
> .//core/src/main/java/org/apache/calcite/sql/SqlFilterOperator.java
> .//core/src/main/java/org/apache/calcite/sql/fun/SqlIntervalOperator.java
> .//core/src/main/java/org/apache/calcite/sql/fun/SqlSubstringFunction.java
> .//core/src/main/java/org/apache/calcite/sql/fun/SqlTranslate3Function.java
> .//core/src/main/java/org/apache/calcite/sql/fun/SqlItemOperator.java
> .//core/src/main/java/org/apache/calcite/sql/fun/SqlTrimFunction.java
> .//core/src/main/java/org/apache/calcite/sql/fun/SqlDotOperator.java
> .//core/src/main/java/org/apache/calcite/sql/fun/SqlPositionFunction.java
> .//core/src/main/java/org/apache/calcite/sql/parser/SqlParserUtil.java
> .//core/src/main/java/org/apache/calcite/sql/parser/SqlParserPos.java
> .//core/src/main/java/org/apache/calcite/sql/type/CompositeOperandTypeChecker.java
> .//core/src/main/java/org/apache/calcite/sql/SqlCallBinding.java
> .//core/src/main/java/org/apache/calcite/sql/SqlOperator.java
> .//file/src/test/java/org/apache/calcite/adapter/file/FileAdapterTest.java
> .//example/csv/src/test/java/org/apache/calcite/test/CsvTest.java
> .//mongodb/src/main/java/org/apache/calcite/adapter/mongodb/MongoRules.java
> {code}
> Should we change all of them?
>   



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