[
https://issues.apache.org/jira/browse/CALCITE-7420?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18060184#comment-18060184
]
Julian Hyde commented on CALCITE-7420:
--------------------------------------
{quote}Should we create a new Jira ticket to implement plan-before and
plan-after first?
{quote}
No, I think adding command(s) to Quidem and refactoring RelOptRulesTest to use
those commands will be a single piece of work. Once the developer knows what
changes are needed in Quidem they should create a GitHub issue that specifies
those changes, followed by a PR.
{quote}I think dumping higher-level plans is the way to go (Logical operators,
not Enumerable).
{quote}
Most tests in {{RelOptRulesTest}} work on logical operators but a few work on
enumerable operators. The {{plan-before}} and {{plan-after}} commands must have
the same effect as the fixture, and therefore produce the same output.
The work that needs to be done in Quidem is implementing inversion-of-control.
The ability to run a command in a Quidem script and have it – via some
callbacks – have the same effect as a test written in Java today that invokes
methods on a fixture.
{quote}1. To execute the results, Logical operators need to be converted to
Enumerable operators. The plans presented by these two types differ somewhat.
For example, the Calc operator (!plan) and !plan-before/plan-after have
different plans in structure.
{quote}
Good point! I should have been more explicit. The {{!ok}} command should
execute the default rules, not the special subset of rules being tested by
{{!plan-before}} and {{{}!plan-after{}}}. All the {{!ok}} command does is put a
SQL string into the session state, and execute it to prove that it is valid.
The subsequent {{!plan-before}} and {{!plan-after}} commands will re-do the
planning process with a special rule set.
{quote}2. In some special cases, such as Aggregate operators with Filter
attributes, the Enumerable plans converted from such Logical plans cannot
currently be executed. In other words, not all Logical plans can be directly
converted to Enumerable plans and executed successfully.
{quote}
I don't know exactly which test cases or operators you are thinking of, but we
can leave a few test cases in {{RelOptRulesTest.java}} in the first phase if
they are not easy to convert.
{quote}3. In RelOptRulesTest, you can use rules directly or in program mode,
but this is not easy to implement flexibly in Quidem.
{quote}
Yes. This task will be a forcing function to transform as many as possible of
those 'non-standard' test cases to a standard pattern. Whichever ones we cannot
transform will remain in {{{}RelOptRulesTest.java{}}}, but hopefully there will
not be many.
It is not surprising that there is some pain as we pivot from an API (the Java
test fixture) to a language (Quidem commands). It's easier to extend an API to
cover a wide variety of cases. But the language is superior because it is
simpler (therefore more people can understand and use it) and can have more
than one implementation (e.g. Java, Rust, Python).
> Convert RelOptRulesTest to Quidem scripts
> -----------------------------------------
>
> Key: CALCITE-7420
> URL: https://issues.apache.org/jira/browse/CALCITE-7420
> Project: Calcite
> Issue Type: Bug
> Reporter: Julian Hyde
> Priority: Major
>
> Many test cases in RelOptRulesTest are simple - involve just a SQL query, set
> of rules, before and after plan. If these were converted to Quidem scripts
> they would be easier to write (because you are writing just one file), more
> maintainable (because Quidem scripts merge more easily than XML), and could
> potentially be run from another language (which would enable ports of Calcite
> to language such as Rust or Python).
> Here is how one test would look:
> {noformat}
> # Pull aggregate through union ====================
> # Tests that AggregateUnionAggregateRule can convert
> # a union of two ’select distinct’ queries to a ’select distinct’
> # of a union.
> !set rules "AGGREGATE_UNION_AGGREGATE"
> select deptno, job
> from (select deptno, job
> from emp as e1
> group by deptno, job
> union all
> select deptno, job
> from emp as e2
> group by deptno, job)
> group by deptno, job;
> +--------+-----------+
> | DEPTNO | JOB |
> +--------+-----------+
> | 20 | CLERK |
> | 30 | SALESMAN |
> | 20 | MANAGER |
> | 30 | MANAGER |
> | 10 | MANAGER |
> | 20 | ANALYST |
> | 10 | PRESIDENT |
> | 30 | CLERK |
> | 10 | CLERK |
> +--------+-----------+
> (9 rows)
> !ok
> LogicalAggregate(group=[{0, 1}])
> LogicalUnion(all=[true])
> LogicalAggregate(group=[{0, 1}])
> LogicalProject(DEPTNO=[$7], JOB=[$2])
> LogicalTableScan(table=[[CATALOG, SALES, EMP]])
> LogicalAggregate(group=[{0, 1}])
> LogicalProject(DEPTNO=[$7], JOB=[$2])
> LogicalTableScan(table=[[CATALOG, SALES, EMP]])
> !plan-before
> LogicalAggregate(group=[{0, 1}])
> LogicalUnion(all=[true])
> LogicalProject(DEPTNO=[$7], JOB=[$2])
> LogicalTableScan(table=[[CATALOG, SALES, EMP]])
> LogicalProject(DEPTNO=[$7], JOB=[$2])
> LogicalTableScan(table=[[CATALOG, SALES, EMP]])
> !plan-after
> {noformat}
>
> It has the same information as the Java method
> {code:java}
> @Test void testPullAggregateThroughUnion() {
> final String sql = "select deptno, job from"
> + " (select deptno, job from emp as e1"
> + " group by deptno,job"
> + " union all"
> + " select deptno, job from emp as e2"
> + " group by deptno,job)"
> + " group by deptno,job";
> sql(sql)
> .withRule(CoreRules.AGGREGATE_UNION_AGGREGATE)
> .check();
> }
> {code}
> and XML resources
> {code:xml}
> <TestCase name="testPullAggregateThroughUnion">
> <Resource name="sql">
> <![CDATA[select deptno, job from (select deptno, job from emp as e1
> group by deptno,job union all select deptno, job from emp as e2 group by
> deptno,job) group by deptno,job]]>
> </Resource>
> <Resource name="planBefore">
> <![CDATA[
> LogicalAggregate(group=[{0, 1}])
> LogicalUnion(all=[true])
> LogicalAggregate(group=[{0, 1}])
> LogicalProject(DEPTNO=[$7], JOB=[$2])
> LogicalTableScan(table=[[CATALOG, SALES, EMP]])
> LogicalAggregate(group=[{0, 1}])
> LogicalProject(DEPTNO=[$7], JOB=[$2])
> LogicalTableScan(table=[[CATALOG, SALES, EMP]])
> ]]>
> </Resource>
> {code}
> It uses {{!set rules}} command (added in CALCITE-6335) and would require new
> {{!plan-before}} and {{!plan-after}} commands.
> I suspect that Claude would do a quite good job migrating tests from .java
> and .xml to .iq. I think it could also group tests by subject area (e.g. join
> and aggregate) so that .iq files are ~1,000 lines.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
