Replace $ with something else will have a large impact. E.g. projects using
Calcite might have tests that verify plans by plan.toString() thus $ is
exposed in many places.
Can we only use Kotlin for tests that does not evaluate plans? I think
there are still many tests that does not have "$" can benefit from
multiple strings.
-Rui
On Sat, Dec 14, 2019 at 3:03 PM Haisheng Yuan <h.y...@alibaba-inc.com>
wrote:
> The plan becomes not so easier to read, which is more important to me.
> Unless we change $ to other symbol, I am inclined to keep using Java.
>
> I am not sure why schemas in MaterializationTest require column names to
> be quoted, If we can change that, queries will be much easier to read (even
> for multiline strings), like RelOptRulesTest does.
>
> - Haisheng
>
> ------------------------------------------------------------------
> 发件人：Vladimir Sitnikov<sitnikov.vladi...@gmail.com>
> 日 期：2019年12月15日 06:09:37
> 收件人：Apache Calcite dev list<dev@calcite.apache.org>
> 主 题：[DISCUSS] Tests vs multiline strings
>
> Hi,
>
> Calcite tests sometimes require multiline strings.
> For instance: input SQL, output plan.
>
> TL;DR: let's use Kotlin to improve test code readability. What do you
> think?
>
> Unfortunately, Java <14 lacks multiline strings, so the current tests are
> not that pretty :(
> They are hard to read, hard to understand, and it is hard to maintain them.
>
> Sample test (from MaterializationTest):
>
> @Test public void testFilterQueryOnProjectView5() {
> checkMaterialize(
> "select \"deptno\" - 10 as \"x\", \"empid\" + 1 as ee, \"name\"\n"
> + "from \"emps\"",
> "select \"name\", \"empid\" + 1 as e\n"
> + "from \"emps\" where \"deptno\" - 10 = 2",
> HR_FKUK_MODEL,
> CalciteAssert.checkResultContains(
> "EnumerableCalc(expr#0..2=[{inputs}], expr#3=[2], "
> + "expr#4=[=($t0, $t3)], name=[$t2], E=[$t1],
> $condition=[$t4])\n"
> + " EnumerableTableScan(table=[[hr, m0]]"));
> }
>
> What do you think if we migrate those types of tests to Kotlin?
> Note: I'm well aware there are Quidem-based tests, and those do not replace
> Java-based ones.
>
> Here's how the same test might look like in Kotlin:
>
> @Test
> fun `filter query on project view5`() {
> checkMaterialize(
> """select "deptno" - 10 as "x", "empid" + 1 as ee, "name" from
> "emps" """,
> """select "name", "empid" + 1 as e from "emps" where "deptno" -
> 10 = 2""",
> HR_FKUK_MODEL,
> CalciteAssert.checkResultContains(
> """
> EnumerableCalc(expr#0..2=[{inputs}], expr#3=[2],
> expr#4=[=(${'$'}t0, ${'$'}t3)], name=[${'$'}t2], E=[${'$'}t1],
> ${'$'}condition=[${'$'}t4])
> EnumerableTableScan(table=[[hr, m0]]
> """.trimIndent()
> )
> )
> }
>
> Pros:
> * SQLs are much easier to read since all the slashes are gone
> * Multiline text is much easier to follow
> * ctrl+c/v works much better: I can copy a substring, even multiple lines
> from the middle, and it just works
> * PR review would be simplified as SQL in PRs would be easier to read
>
> Cons:
> * A bit of a sad point is $ is a special symbol there, and it needs to be
> escaped.
> A (not that?) crazy idea might be to make Calcite use another character
> instead of $ (as $ is used in string templating languages quite often
> anyway),
> * If a class is converted from Java to Kotlin, then default Git annotate
> fails to attribute lines to commits (== the file looks like a brand new
> one)
> * Something else?
>
> I don't suggest to go ahead and convert all the things, however, in my
> opinion, converting at least some of the tests or using multiline strings
> for newly added tests makes sense.
>
> What do you think?
>
> Vladimir
>
>
