zabetak commented on code in PR #3445:
URL: https://github.com/apache/calcite/pull/3445#discussion_r1351997263
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -9722,6 +9725,15 @@ private static void
checkArrayConcatAggFuncFails(SqlOperatorFixture t) {
false);
}
+ /** Test case for <a
href="https://issues.apache.org/jira/browse/CALCITE-6029";>
+ * [CALCITE-6029] SqlOperatorTest cannot test operators that require the
Babel parser</a>. */
+ @Test void testBabel() {
+ final SqlOperatorFixture f = fixture().withLibrary(SqlLibrary.POSTGRESQL)
+ .withParserConfig(p ->
p.withParserFactory(SqlBabelParserImpl.FACTORY));
+ f.checkScalar("10::integer",
+ "10", "INTEGER NOT NULL");
+ }
Review Comment:
At the moment, I am rather neutral on this PR, don't have a strong opinion
about merging it or not.
> None of the fixtures in the Babel tests seems to check operator behavior.
Not sure what exactly you mean by operator behavior. I see that it is
possible to parse, validate, and execute operators using `BabelParserTest` and
`BabelTest`. Which operator behavior we are not able to test?
> Alternatively, if we don't accept this change, we should build
infrastructure similar to SqlOperatorTest for Babel tests...
Can you elaborate why the existing `BabelXx` test classes are not
sufficient? If it is easy/equivalent to add new tests there then in terms of
maintenance it may be better to keep the Babel tests centralized.
If your ultimate goal is to replace/unify those `BabelXx` classes with
`SqlOperatorTest` then that's a different discussion and I would be very
positive to move into that direction.
> Note that, although some operators are Babel, their implementation is
still in core.
The babel module depends on core so I see no big problem around this.
> Another weird thing is that CalciteSqlOperatorTests is in core, while
SqlOperatorTest is in testkit
Yes I noticed this as well. It is indeed a bit weird, but since it is
already like that I don't mind adding another loop.
--
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]
