I think you have identified the issues correctly. I prefer SqlDatetimeSubtractionOperator to SqlTimestampDiffFunction because it is standard and more powerful. But some work is needed to make SQL generation run smoothly. Especially with data types, as you point out.
I would approach this by writing tests in RelToSqlConverterTest with the desired SQL for each database and change RelToSqlConverter to make them pass. Pragmatic, not very elegant. Julian > On Apr 2, 2020, at 6:45 AM, TANG Wen-hui <[email protected]> > wrote: > > Dear All, > > As for issue https://issues.apache.org/jira/browse/CALCITE-3312(I'm working > on this issue), I have noticed several problems. So I am writing this mail > for some advices. > > 1.Calcite support the syntax like that "(datetime - datetime) > IntervalQualifier"; > A simple test in JdbcTest: > @Test public void testTimestampMinus() { > CalciteAssert.that() > .with(CalciteAssert.Config.JDBC_SCOTT) > .query("select (date'2003-12-30' - date'2001-11-30') year from > JDBC_SCOTT.emp") > .returns(""); > } > However, the result returns "+63" which is wrong. This is because that > RelToSqlConverter simply convert it to "SELECT (DATE '2003-12-30' - DATE > '2001-11-30') FROM "SCOTT"."EMP"" (see > SqlDatetimeSubtractionOperator#unparse). > As I konw, most database systems cannot deal with this sitution properly, and > the standard format of datetime value expression is like that > " > <datetime value expression> ::= > <datetime term> > | <interval value expression> <plus sign> <datetime term> > | <datetime value expression> <plus sign> <interval term> > | <datetime value expression> <minus sign> <interval term> > <datetime term> ::= > <datetime factor> > <datetime factor> ::= > <datetime primary> [ <time zone> ] > <datetime primary> ::= > <value expression primary> > | <datetime value function> > <time zone> ::= > AT <time zone specifier> > <time zone specifier> ::= > LOCAL > | TIME ZONE <interval primary> > "[1] > > 2.Calcite will translate SqlTimestampDiffFunction to > SqlDatetimeSubtractionOperator during the conversion from SqlNode to RexNode > for SqlTimestampDiffFunction(see TimestampDiffConvertlet#convertCall). But > the different return types between SqlTimestampDiffFunction whose return type > is SqlTypeName.BIGINT or SqlTypeName.INTEGER and > SqlDatetimeSubtractionOperator whose return type is IntervalSqlType introduce > the internal operator REINTERPRET and /INT(see makeCastIntervalToExact). The > internal operator should not be converted to sql. I did a test that I > overrided the "unparse" method for REINTERPRET and / and still got the wrong > result like above. > > I have a few thoughts about this kind of situation, but I am not sure it goes > right way. > 1.We can forbidden the conversion from Project to JdbcProject for the Project > which contains RexCall whose operator is SqlDatetimeSubtractionOperator and > operands both are datetime. > 2.Or we can translate SqlDatetimeSubtractionOperator to > SqlTimestampDiffFunction or other similar functions during unparsing(for > example, MySQL supports TIMESTAMPDIFF rather than datetime subtraction), but > the internal operator and the situation that databases which support this > feature take different approaches to achieve this feature make thing > confusing. > > Best wishes > Wenhui Tang > > [1] ISO/IEC CD 9075-2 > > > > > [email protected]
