[
https://issues.apache.org/jira/browse/CALCITE-7893?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
ASF GitHub Bot updated CALCITE-7893:
------------------------------------
Labels: pull-request-available (was: )
> Type mapping rules can be corrupted when identity hash codes of SqlTypeName
> values collide
> ------------------------------------------------------------------------------------------
>
> Key: CALCITE-7893
> URL: https://issues.apache.org/jira/browse/CALCITE-7893
> Project: Calcite
> Issue Type: Bug
> Components: core
> Reporter: Vladimir Sitnikov
> Priority: Major
> Labels: pull-request-available
>
> {{SqlTypeAssignmentRule}} and {{SqlTypeCoercionRule}} build their type tables
> once per JVM. If two different sets of {{SqlTypeName}} passed to the builder
> have the same hash code, the type added later receives the source set of the
> type added earlier. {{SqlTypeName}} is an enum, so its hash code is the
> identity hash code, and a set's hash code is the sum of its elements' hash
> codes. Identity hash codes change from one JVM run to the next, so the
> corruption is probabilistic: a query can pass in one run and fail in the
> next, and the wrong table stays in effect until the JVM exits.
> With a corrupted table, the validator accepts casts it should reject, rejects
> valid calls, and resolves user-defined functions to the wrong overload. I
> have not observed a collision with default JVM settings.
> h3. How to reproduce
> The JVM option {{-XX:+UnlockExperimentalVMOptions -XX:hashCode=2}} makes
> every identity hash code equal to 1, so every collision that can happen does
> happen. It is a way to make the defect deterministic, not a setting anyone
> runs in production.
> Gradle init script {{hash.gradle}}:
> {code:groovy}
> allprojects {
> tasks.withType(Test).configureEach {
> jvmArgs '-XX:+UnlockExperimentalVMOptions', '-XX:hashCode=2'
> }
> }
> {code}
> {code:bash}
> ./gradlew -I hash.gradle :core:test --tests
> org.apache.calcite.test.SqlValidatorTest --tests
> org.apache.calcite.test.UdfTest
> {code}
> With the option, the validator accepts this query:
> {code:sql}
> values (cast(DATE '1243-12-01' as TIME))
> {code}
> Expected: validation fails with {{Cast function cannot convert value of type
> DATE NOT NULL to type TIME}}, as {{SqlValidatorTest#testCastFails}} asserts.
> Actual:
> {noformat}
> java.lang.AssertionError: Expected query to throw exception, but it did not;
> query [values (cast(DATE '1243-12-01' as TIME))]; expected [(?s).*Cast
> function cannot convert value of type DATE NOT NULL to type TIME.*]
> at org.apache.calcite.sql.test.SqlTests.checkEx(SqlTests.java:251)
> at
> org.apache.calcite.test.SqlValidatorTest.testCastFails(SqlValidatorTest.java:1626)
> {noformat}
> The table itself is wrong:
> {{SqlTypeMappingRules.instance(false).getTypeMapping().get(SqlTypeName.TIME)}}
> returns {{[DATE]}}, while {{SqlTypeAssignmentRule}} declares TIME assignable
> from {{[TIME]}} only. The same corruption breaks UDF lookup, for example in
> {{UdfTest#testUserDefinedFunctionWithMethodName}}:
> {noformat}
> Suppressed: org.apache.calcite.util.TestUtil$ExtraInformation: With
> materializationsEnabled=false, limit=-1, sql=values "adhoc"."fun1"('Foo Bar')
> Caused by: java.lang.NumberFormatException: For input string: "Foo Bar"
> at
> org.apache.calcite.linq4j.tree.Primitive.checkNumericString(Primitive.java:463)
> at org.apache.calcite.runtime.SqlFunctions.toInt(SqlFunctions.java:5519)
> {noformat}
> and {{UdfTest#testDate}} fails with {{No match found for function signature
> timeFun(<TIME>)}}.
> Locally on JDK 21.0.9 this fails 11 tests: {{SqlValidatorTest#testCastFails}}
> and 10 tests in {{UdfTest}}. In CI ([run
> 40321987597|https://github.com/vlsi/calcite/actions/runs/40321987597], JDK
> 11, 21, and 24 on Linux, macOS, and Windows) the four jobs with the option
> that reached {{:core:test}} each failed the same 41 tests there, among them
> {{JdbcTest}} table macro and table function tests, {{SqlAdvisorJdbcTest}},
> {{testCastDateTime}} in the operator tests, {{InterpreterTest}},
> {{TableFunctionTest}}, and {{sql/spatial.iq}}. Jobs without the option passed.
> h3. Cause
> {{SqlTypeMappingRules.Builder#add}}
> ([SqlTypeMappingRules.java:77|https://github.com/apache/calcite/blob/2df193e496/core/src/main/java/org/apache/calcite/sql/type/SqlTypeMappingRules.java#L77])
> uses the caller's set as the key of a Guava {{LoadingCache}}.
> {{SqlTypeAssignmentRule}} and {{SqlTypeCoercionRule}} pass one {{HashSet}}
> named {{rule}}, which they clear and refill between calls, so every cache
> entry is keyed by the same mutable object. The cache matches an entry by its
> stored hash and then by {{equals}}, and the key always equals itself. Any
> lookup whose set has the hash code of an earlier set therefore returns that
> earlier set. {{Set#hashCode}} is the sum of the element hash codes, so under
> the option every one-element set hashes to 1, and {{\{TIME\}}} receives the
> entry stored for an earlier one-element set such as {{\{DATE\}}}.
> h3. Proposed fix
> Make {{SqlTypeMappingRules.Builder#add}} copy its argument into an immutable
> set before it uses the set as a key:
> {code:java}
> void add(SqlTypeName toType, Set<SqlTypeName> fromTypes) {
> map.put(toType, sets.computeIfAbsent(Sets.immutableEnumSet(fromTypes), k ->
> k));
> }
> {code}
> Here {{sets}} becomes a plain {{HashMap<ImmutableSet<SqlTypeName>,
> ImmutableSet<SqlTypeName>>}}: {{add}} is package-private, and Calcite fills
> its rule tables once, in static initializers, so the builder needs neither
> eviction nor thread safety. Equal sets still share one instance, and the
> values stored in the table are {{ImmutableEnumSet}}s as before. The callers
> can keep reusing their {{rule}} set.
> A regression test in {{SqlTypeUtilTest}} can force the collision without JVM
> options: pass a {{HashSet}} subclass whose {{hashCode()}} returns a constant,
> fill it with DATE, add it for DATE, refill it with TIME, add it for TIME, and
> assert that TIME maps to {{[TIME]}}.
> A local patch that passes {{ImmutableSet.copyOf(fromTypes)}} as the cache key
> makes all 17088 tests of {{:core:test}} pass under the option (0 failures,
> 146 skipped).
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
