[
https://issues.apache.org/jira/browse/CALCITE-5148?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17535094#comment-17535094
]
Julian Hyde commented on CALCITE-5148:
--------------------------------------
Well, maybe there's a reason for some of the behavior. But the behavior doesn't
seem consistent, and the consistency isn't documented or enforced. That's what
needs to be fixed.
I would be inclined to canonize instances of SqlCollation the way we do with
types, RelFieldCollation, etc.
> BasicSqlType.generateTypeString() should use equals() to compare collation
> instead of == by reference
> -----------------------------------------------------------------------------------------------------
>
> Key: CALCITE-5148
> URL: https://issues.apache.org/jira/browse/CALCITE-5148
> Project: Calcite
> Issue Type: Bug
> Reporter: Yingyu Wang
> Priority: Major
>
> Currently in {{{}BasicSqlType.{}}}{{{}generateTypeString(StringBuilder sb,
> boolean withDetail){}}} method compares collation field against static
> {{SqlCollation.IMPLICIT}} by reference. I.e.
> {code:java}
> if (collation != null
> && collation != SqlCollation.IMPLICIT && collation !=
> SqlCollation.COERCIBLE) { {code}
> This will cause type that constructed with a collation instance that equals
> to {{SqlCollation.IMPLICIT}} or {{SqlCollation.COERCIBLE}} (e.g. collation
> instance is constructed during deserialization) to generate different type
> string.
> The following test can demonstrate the problem.
> {code:java}
> @Test void testCreateSqlTypeWithCollation() {
> final RelDataTypeFactory typeFactory =
> new SqlTypeFactoryImpl(RelDataTypeSystem.DEFAULT);
> final RelDataType varchar =
> typeFactory.createSqlType(SqlTypeName.VARCHAR);
> final SqlCollation myImplicitCollation = new
> SqlCollation(SqlCollation.Coercibility.IMPLICIT);
> final RelDataType myVarchar = typeFactory.createTypeWithCharsetAndCollation(
> typeFactory.createSqlType(SqlTypeName.VARCHAR),
> typeFactory.getDefaultCharset(),
> myImplicitCollation);
> assertTrue(myImplicitCollation.equals(SqlCollation.IMPLICIT));
> assertTrue(myVarchar.equals(varchar));
> assertEquals("VARCHAR NOT NULL", varchar.getFullTypeString());
> assertEquals("VARCHAR NOT NULL", myVarchar.getFullTypeString());
> }{code}
> Here {{myImplicitCollation}} is constructed with the same coercibility as the
> static instance {{SqlCollation.IMPLICIT}} so {{myImplicitCollation}} equals
> to {{{}SqlCollation.IMPLICIT{}}}.
>
> However due to the issue with {{BasicSqlType.generateTypeString()}} where
> collation is compared by reference, these 2 assert will fail:
> {code:java}
> assertTrue(myVarchar.equals(varchar));
> //myVarchar.digest = VARCHAR COLLATE "ISO-8859-1$en_US$primary" NOT NULL
> //varchar.digest = VARCHAR NOT NULL
> assertEquals("VARCHAR NOT NULL", myVarchar.getFullTypeString());
> //myVarchar.getFullTypeString() = VARCHAR COLLATE "ISO-8859-1$en_US$primary"
> NOT NULL{code}
> The fix would be using equals to compare collation instance in
> BasicSqlType.generateTypeString(). I.e.
> {code:java}
> if (collation != null
> && !collation.equals(SqlCollation.IMPLICIT) &&
> !collation.equals(SqlCollation.COERCIBLE)) { {code}
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
