Re: [DISCUSS] Tests vs multiline strings
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
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
> 日 期：2019年12月15日 06:09:37
> 收件人：Apache Calcite dev list
> 主 题：[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
>
>
Re: [DISCUSS] Tests vs multiline strings
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
日 期：2019年12月15日 06:09:37
收件人：Apache Calcite dev list
主 题：[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
[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
Re: How to keep quotes in SqlIdentifier?
I don't find it correct to have quotes, double-quotes, backticks or anything other than the identifier name in the SqlNode's identifier. If you need it, you can just add it your self. But why do you want to have the identifier this way ? May be there is another way to achieve your goal ? On Sat, Dec 14, 2019 at 4:03 AM 月宫的木马兔 wrote: > Hi, > > I got the SqlNode by using SqlParse.parseQuery() (Sql bellow), but I can > not find the back_tick in SqlIdentifer > > Sql: > > SELECT `sql`,id1 FROM testdata > > Debug with IDEA: > > >
