You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@calcite.apache.org by "Yingyu Wang (Jira)" <ji...@apache.org> on 2022/05/10 18:14:00 UTC
[jira] [Updated] (CALCITE-5148) BasicSqlType.generateTypeString() should use equals() to compare collation instead of == by reference
[ https://issues.apache.org/jira/browse/CALCITE-5148?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Yingyu Wang updated CALCITE-5148:
---------------------------------
Description:
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}
was:
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}
> 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)