[FLINK-38609][table] Make generation of xml plans deterministic #27183

snuyanzin · 2025-11-02T21:12:08Z

What is the purpose of the change

~~The PR is based on [FLINK-38608][table] Drop abandoned plans without related tests~~
The main idea here is to enable xml plan generation in predictable way (in alphabetical order by test cases within xml).
It means that no need for manual work with copy pasting of generated xml while adding/changing/fixing plans.
Just drop the whole xml, run test and it will regenerate a new version changing only required part instead of reordering everything each time, moreover test will check the order while execution as well.

The downside here: it will trigger to regenerate existing plans, however it will be done only once (in this PR)

Brief change log

plans and DiffRepository (mostly synced with Calcite's version)

Verifying this change

existing tests

Does this pull request potentially affect one of the following parts:

Dependencies (does it add or upgrade a dependency): (no)
The public API, i.e., is any changed class annotated with @Public(Evolving): (no)
The serializers: (no)
The runtime per-record code paths (performance sensitive): (no)
Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (no)
The S3 file system connector: (no)

Documentation

Does this pull request introduce a new feature? ( no)
If yes, how is the feature documented? (not applicable)

snuyanzin · 2025-11-02T21:13:32Z

...e/flink-table-planner/src/test/java/org/apache/flink/table/planner/utils/DiffRepository.java

     * one diff is recorded.
     */
-    private static final Map<Class<?>, DiffRepository> MAP_CLASS_TO_REPOSITORY = new HashMap<>();
+    private static final LoadingCache<Key, DiffRepository> REPOSITORY_CACHE =


Mostly synced with updates from Calcite version (https://github.com/apache/calcite/blob/main/testkit/src/main/java/org/apache/calcite/test/DiffRepository.java), also fixed some bugs both here and in Calcite, like e.g. apache/calcite#4612
There are still some differences between Flink and Calcite versions however now much less.
Probably need a separate task to try to backport Flink's features to Calcite and drop it from Flink repo

snuyanzin · 2025-11-02T21:16:07Z

...able/flink-table-planner/src/test/resources/org/apache/flink/table/api/batch/ExplainTest.xml

    </Resource>
  </TestCase>
-  <TestCase name="testExplainWithAgg[extended=true]">
+  <TestCase name="testExplainWithAgg[extended=false]">


Here and all other xml files: were dropped and completely regenerated

snuyanzin · 2025-11-02T21:17:12Z

...er/src/test/scala/org/apache/flink/table/planner/plan/stream/sql/join/IntervalJoinTest.scala

@@ -107,7 +107,7 @@ class IntervalJoinTest extends TableTestBase {

  /** The time conditions should be an And condition * */
  @Test
-  def testInteravalNotCnfCondition(): Unit = {
+  def testIntervalNotCnfCondition(): Unit = {


snuyanzin · 2025-11-02T21:19:19Z

.../flink-table-planner/src/test/scala/org/apache/flink/table/planner/utils/TableTestBase.scala

+      plans.remove(PlanKind.OPT_REL)
+      // if there is something else in expected plans, then use it for xml generation if needed
+      assertEqualsOrExpand("optimized rel plan", optimizedRelPlan, !plans.isEmpty)


There are some tests requiring both OPT_REL, OPT_REL_WITH_ADVICE
in order to have both generated in xml, need to put true here if both are present

flinkbot · 2025-11-02T21:20:29Z

CI report:

2a36a2f Azure: SUCCESS

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot run azure re-run the last Azure build

twalthr

Thanks for further improving our testing infra.

twalthr · 2025-11-03T16:09:18Z

.../flink-table-planner/src/test/scala/org/apache/flink/table/planner/utils/TableTestBase.scala

@@ -1225,16 +1225,23 @@ abstract class TableTestUtilBase(test: TableTestBase, isStreamingMode: Boolean)
    // check whether the sql equals to the expected if the `relNodes` are translated from sql
    assertSqlEqualsOrExpandFunc()
    // check ast plan
+    val plans = new util.HashSet[PlanKind](expectedPlans.toSeq.asJava)


Can you put the non-xml changes into a separate commit?

yep, sure, splitted into several commits

… xml plans

…#testWithProjectCaseWhenCorrelate`

snuyanzin · 2025-11-03T22:03:13Z

...ink/table/planner/plan/rules/logical/subquery/SubqueryCorrelateVariablesValidationTest.scala

-    assertThatExceptionOfType(classOf[AssertionError])
-      .isThrownBy(() => util.verifyRelPlan(sqlQuery))


Seems since some time it was fixed and this code was continuing throwing exception with a different reason (nothing in xml plans... ). So generated plans for this

snuyanzin · 2025-11-04T07:50:36Z

.editorconfig

+[*Test.xml]
+indent_style = space
+indent_size = 2


sync with DiffRepository where indentation is in spaces

Great 👍 Also had this indentation issue when manually updating xmls

gustavodemorais

Checked for some tests and the ordering looks good.

Thanks a lot, @snuyanzin. It might be a simple change but for people constantly regenerating plans, this is really useful.

gustavodemorais · 2025-11-04T10:02:03Z

.editorconfig

+[*Test.xml]
+indent_style = space
+indent_size = 2


Great 👍 Also had this indentation issue when manually updating xmls

twalthr

Thanks for this improvement @snuyanzin! LGTM

snuyanzin commented Nov 2, 2025

View reviewed changes

snuyanzin force-pushed the flink38609 branch 2 times, most recently from e40a885 to 38a600e Compare November 3, 2025 09:25

twalthr reviewed Nov 3, 2025

View reviewed changes

[hotfix][tests] Make editorconfig for plan tests expecting spaces for…

f0ee7cb

… xml plans

snuyanzin force-pushed the flink38609 branch from 38a600e to 3adbd66 Compare November 3, 2025 20:18

snuyanzin added 4 commits November 3, 2025 22:47

[hotfix][table] Remove trailing spaces and fix typo

e8b03e0

[FLINK-38609][table] Enable `SubqueryCorrelateVariablesValidationTest…

71c24da

…#testWithProjectCaseWhenCorrelate`

[FLINK-38609][table] Make generation of xml plans deterministic

5f13169

[FLINK-38609][table] Regenerate plans in deterministic manner

2a36a2f

snuyanzin force-pushed the flink38609 branch from 3adbd66 to 2a36a2f Compare November 3, 2025 22:00

snuyanzin commented Nov 3, 2025

View reviewed changes

snuyanzin commented Nov 4, 2025

View reviewed changes

gustavodemorais approved these changes Nov 4, 2025

View reviewed changes

twalthr approved these changes Nov 4, 2025

View reviewed changes

snuyanzin closed this in fd273a5 Nov 4, 2025

		assertThatExceptionOfType(classOf[AssertionError])
		.isThrownBy(() => util.verifyRelPlan(sqlQuery))

[FLINK-38609][table] Make generation of xml plans deterministic #27183

[FLINK-38609][table] Make generation of xml plans deterministic #27183

Uh oh!

Conversation

snuyanzin commented Nov 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is the purpose of the change

Brief change log

Verifying this change

Does this pull request potentially affect one of the following parts:

Documentation

Uh oh!

snuyanzin Nov 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

flinkbot commented Nov 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI report:

Uh oh!

twalthr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gustavodemorais left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

twalthr left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

snuyanzin commented Nov 2, 2025 •

edited

Loading

snuyanzin Nov 2, 2025 •

edited

Loading

flinkbot commented Nov 2, 2025 •

edited

Loading