You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@calcite.apache.org by "Vladimir Sitnikov (Jira)" <ji...@apache.org> on 2020/12/04 11:22:00 UTC
[jira] [Comment Edited] (CALCITE-4421) AssertionError is thrown
with empty message
[ https://issues.apache.org/jira/browse/CALCITE-4421?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17243942#comment-17243942 ]
Vladimir Sitnikov edited comment on CALCITE-4421 at 12/4/20, 11:21 AM:
-----------------------------------------------------------------------
{quote}Julian: Ask yourself: who is the user? If that person is a Calcite developer, then then they should be looking in the code{quote}
{{project = CALCITE AND text ~ AssertionError and resolution=unresolved}} search yields 74 results.
Issues with meaningful messages are much easier to understand.
It looks like most part of the issues does already have a non-empty assertion mesage.
For instance, https://issues.apache.org/jira/browse/CALCITE-2448 has the following:
{noformat}
java.lang.AssertionError: Cannot add expression of different type to set:
set type is RecordType(CHAR(1) CHARACTER SET "ISO-8859-1" COLLATE "ISO-8859-1$en_US$primary" NOT NULL EXPR$0, INTEGER NOT NULL EXPR$1) NOT NULL
expression type is RecordType(CHAR(1) CHARACTER SET "ISO-8859-1" COLLATE "ISO-8859-1$en_US$primary" NOT NULL EXPR$0, CHAR(1) CHARACTER SET "ISO-8859-1" COLLATE "ISO-8859-1$en_US$primary" NOT NULL EXPR$1) NOT NULL
set is rel#9:LogicalAggregate(input=HepRelVertex#8,group={0, 1})
expression is LogicalProject#11
at org.apache.calcite.plan.RelOptUtil.verifyTypeEquivalence(RelOptUtil.java:380)
{noformat}
Even though it is quite verbose, the message enables us to see the nature of the failure.
On the other hand, https://issues.apache.org/jira/browse/CALCITE-4323 provides no clue:
{noformat}
java.lang.AssertionError
at org.apache.calcite.sql2rel.SqlToRelConverter.requiredCollation(SqlToRelConverter.java:634)
at org.apache.calcite.sql2rel.SqlToRelConverter.requiredCollation(SqlToRelConverter.java:629)
at org.apache.calcite.sql2rel.SqlToRelConverter.convertQuery(SqlToRelConverter.java:578)
at org.apache.calcite.prepare.CalcitePrepareImpl$CalcitePreparingStmt.expandView(CalcitePrepareImpl.java:1072)
at org.apache.calcite.plan.ViewExpanders$1.expandView(ViewExpanders.java:52)
at org.apache.calcite.schema.impl.ViewTable.expandView(ViewTable.java:127)
at org.apache.calcite.schema.impl.ViewTable.toRel(ViewTable.java:120){noformat}
Another case is https://issues.apache.org/jira/browse/CALCITE-2315
{quote}
java.lang.AssertionError
at org.apache.calcite.adapter.elasticsearch.ElasticsearchProject.<init>(ElasticsearchProject.java:44)
at org.apache.calcite.adapter.elasticsearch.ElasticsearchProject.copy(ElasticsearchProject.java:49)
{quote}
{quote} If so, it's an internal error. If it's outside of Calcite code, then the public API needs to give an error that is meaningful in terms of that public API{quote}
I agree it would be great if public API raised human-friendly messages.
However, even internal errors should have reasonable messages, error codes, etc.
For instance, Oracle Database has {{ORA-00600: internal error code}}. However, most of the "internal" errors have extra message parameters that enable users to lookup the errors and analyze the nature of the failures.
Just in case, I guess Pair.zip is a public method, and I can imagine the method might be used even outside of Calcite.
was (Author: vladimirsitnikov):
{quote}Julian: Ask yourself: who is the user? If that person is a Calcite developer, then then they should be looking in the code{quote}
{{project = CALCITE AND text ~ AssertionError and resolution=unresolved}} search yields 74 results.
Issues with meaningful messages are much easier to understand.
It looks like most part of the issues does already have a non-empty assertion mesage.
For instance, https://issues.apache.org/jira/browse/CALCITE-2448 has the following:
{noformat}
java.lang.AssertionError: Cannot add expression of different type to set:
set type is RecordType(CHAR(1) CHARACTER SET "ISO-8859-1" COLLATE "ISO-8859-1$en_US$primary" NOT NULL EXPR$0, INTEGER NOT NULL EXPR$1) NOT NULL
expression type is RecordType(CHAR(1) CHARACTER SET "ISO-8859-1" COLLATE "ISO-8859-1$en_US$primary" NOT NULL EXPR$0, CHAR(1) CHARACTER SET "ISO-8859-1" COLLATE "ISO-8859-1$en_US$primary" NOT NULL EXPR$1) NOT NULL
set is rel#9:LogicalAggregate(input=HepRelVertex#8,group={0, 1})
expression is LogicalProject#11
at org.apache.calcite.plan.RelOptUtil.verifyTypeEquivalence(RelOptUtil.java:380)
{noformat}
Even though it is quite verbose, the message enables to see the nature of the failure.
On the other hand, https://issues.apache.org/jira/browse/CALCITE-4323 provides no clue:
{noformat}
java.lang.AssertionError
at org.apache.calcite.sql2rel.SqlToRelConverter.requiredCollation(SqlToRelConverter.java:634)
at org.apache.calcite.sql2rel.SqlToRelConverter.requiredCollation(SqlToRelConverter.java:629)
at org.apache.calcite.sql2rel.SqlToRelConverter.convertQuery(SqlToRelConverter.java:578)
at org.apache.calcite.prepare.CalcitePrepareImpl$CalcitePreparingStmt.expandView(CalcitePrepareImpl.java:1072)
at org.apache.calcite.plan.ViewExpanders$1.expandView(ViewExpanders.java:52)
at org.apache.calcite.schema.impl.ViewTable.expandView(ViewTable.java:127)
at org.apache.calcite.schema.impl.ViewTable.toRel(ViewTable.java:120){noformat}
Another case is https://issues.apache.org/jira/browse/CALCITE-2315
{quote}
java.lang.AssertionError
at org.apache.calcite.adapter.elasticsearch.ElasticsearchProject.<init>(ElasticsearchProject.java:44)
at org.apache.calcite.adapter.elasticsearch.ElasticsearchProject.copy(ElasticsearchProject.java:49)
{quote}
{quote} If so, it's an internal error. If it's outside of Calcite code, then the public API needs to give an error that is meaningful in terms of that public API{quote}
I agree it would be great if public API raised human-friendly messages.
However, even internal errors should have reasonable messages, error codes, etc.
For instance, Oracle Database has {{ORA-00600: internal error code}}. However, most of the "internal" errors have extra message parameters that enable users to lookup the errors and analyze the nature of the failures.
Just in case, I guess Pair.zip is a public method, and I can imagine the method might be used even outside of Calcite.
> 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
> Labels: pull-request-available
> Time Spent: 50m
> Remaining Estimate: 0h
>
> 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)