FrankChen021 commented on code in PR #5281:
URL: https://github.com/apache/calcite/pull/5281#discussion_r4335735472
##########
core/src/test/java/org/apache/calcite/sql/type/SqlTypeFactoryTest.java:
##########
@@ -36,16 +39,57 @@
import static org.hamcrest.CoreMatchers.notNullValue;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.hasToString;
+import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotSame;
import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
+import static java.util.Objects.requireNonNull;
+
/**
* Test for {@link SqlTypeFactoryImpl}.
*/
class SqlTypeFactoryTest {
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7807";>[CALCITE-7807]
+ * Reuse unchanged SQL types during least-restrictive inference</a>. */
+ @Test void testUnchangedCharsetAndCollationReturnsSameType() {
+ final BasicSqlType type = (BasicSqlType) new SqlTypeFixture().sqlVarchar;
+ final Charset charset = requireNonNull(type.getCharset());
+ final SqlCollation collation = requireNonNull(type.getCollation());
+
+ assertSame(type,
+ type.createWithCharsetAndCollation(charset, collation),
+ "createWithCharsetAndCollation(" + charset + ", " + collation + ")");
+ }
+
+ @Test void testDifferentCharsetIsAppliedToNewType() {
+ final BasicSqlType type = (BasicSqlType) new SqlTypeFixture().sqlVarchar;
+ final BasicSqlType utf16Type =
+ type.createWithCharsetAndCollation(StandardCharsets.UTF_16,
SqlCollation.IMPLICIT);
+
+ assertNotSame(type, utf16Type, "a different charset must produce a new
type");
Review Comment:
Addressed. I removed both redundant assertNotSame assertions; the charset
and collation assertions verify the behavior directly.
##########
core/src/test/java/org/apache/calcite/sql/type/SqlTypeFactoryTest.java:
##########
@@ -36,16 +39,57 @@
import static org.hamcrest.CoreMatchers.notNullValue;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.hasToString;
+import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotSame;
import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
+import static java.util.Objects.requireNonNull;
+
/**
* Test for {@link SqlTypeFactoryImpl}.
*/
class SqlTypeFactoryTest {
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7807";>[CALCITE-7807]
+ * Reuse unchanged SQL types during least-restrictive inference</a>. */
+ @Test void testUnchangedCharsetAndCollationReturnsSameType() {
+ final BasicSqlType type = (BasicSqlType) new SqlTypeFixture().sqlVarchar;
+ final Charset charset = requireNonNull(type.getCharset());
+ final SqlCollation collation = requireNonNull(type.getCollation());
+
+ assertSame(type,
+ type.createWithCharsetAndCollation(charset, collation),
+ "createWithCharsetAndCollation(" + charset + ", " + collation + ")");
+ }
+
+ @Test void testDifferentCharsetIsAppliedToNewType() {
+ final BasicSqlType type = (BasicSqlType) new SqlTypeFixture().sqlVarchar;
+ final BasicSqlType utf16Type =
+ type.createWithCharsetAndCollation(StandardCharsets.UTF_16,
SqlCollation.IMPLICIT);
+
+ assertNotSame(type, utf16Type, "a different charset must produce a new
type");
+ assertEquals(StandardCharsets.UTF_16, utf16Type.getCharset());
+ assertSame(SqlCollation.IMPLICIT, utf16Type.getCollation());
+ }
+
+ /** {@link SqlCollation#equals} ignores coercibility, so COERCIBLE equals
+ * IMPLICIT; the type must still be rebuilt. */
+ @Test void testCollationDifferingOnlyInCoercibilityIsNotReused() {
+ final BasicSqlType type = (BasicSqlType) new SqlTypeFixture().sqlVarchar;
Review Comment:
Addressed. The test now constructs the IMPLICIT base type directly outside
DATATYPE_CACHE, so its precondition is deterministic.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
