[CALCITE-3924] Fix flakey test to handle TIMESTAMP and TIMESTAMP(0) correctly

• Type: Bug
• Status: Closed
• Priority: Critical
• Resolution: Fixed
• Affects Version/s: 1.22.0
• Fix Version/s: 1.23.0
• Component/s: core
• Labels: pull-request-available
• Assignee: Danny Chen
• Reporter: neoremind
• Created: 15/Apr/20 01:27
• Updated: 24/May/20 12:00
• Resolved: 15/Apr/20 10:02

Description

Currently, building is not stable, there are frequently flakey tests on Linux(JDK 11) related with TIMESTAMP(0) vs TIMESTAMP.

For example,

https://github.com/apache/calcite/runs/584340845

https://github.com/apache/calcite/runs/576436249

https://github.com/apache/calcite/runs/585695121

Below is first failure test of a consecutive failure tests from the three builds:
org.apache.calcite.test.SqlLimitsTest > testPrintLimits() (see attachment log for detail)
The only diff is TIMESTAMP(0) vs TIMESTAMP

 

The root cause is:

In BasicSqlType, the value of `toString` result and instance member `digest` might be different. For TIMESTAMP without precision (precision = -1), toString returns TIMESTAMP and digest is TIMESTAMP(0), which conflicts with real TIMESTAMP(0) with 0 as precision. 

`Interner<RelDataType> DATATYPE_CACHE = Interners.newWeakInterner()` makes sure SqlType is singleton between invocations, TIMESTAMP(0) might return SqlType - TIMESTAMP without precision literally which is wrong. Because the global cache uses weak reference, so the cache would usually invalidate after Java GC, which mitigates the impact of the hidden bug. As for the specific environment Linux JDK 11 situation, I could not give a reasonable explanation yet, but I suppose the cause is clear.

The fix is simple, making the digest of TIMESTAMP without precision (precision = -1) as TIMESTAMP instead of TIMESTAMP(0) would solve the problem. This could expand to all TIME and TIMESTAMP related sql types.

Issue Links

• links to: GitHub Pull Request #1916