fix: 优化执行步骤获取逻辑和SQL生成流程, 解决重新生成死循环问题 #130

[tjx222]

Describe what this PR does / why we need it

解决重新生成sql 死循环，及生成sql 不生效问题

Does this pull request fix one issue?

修复bug #130

Describe how you did it

• 重构 PlanProcessUtil.java 中的 getCurrentExecutionStep 方法，新增重载版本以支持传入 plan 参数， 解决重新生成的sql 未生效。
• 优化 SQL 生成完成后的返回逻辑，将返回最佳 SQL 改为返回空 Flux 避免死循环

Describe how to verify it

Special notes for reviews

[VLSMB]

请在本地终端运行一下mvn spring-javaformat:apply统一一下代码风格，再提交代码

[Copilot]

Pull Request Overview

This PR fixes a critical bug (#130) where SQL regeneration would enter an infinite loop, and addresses an issue where regenerated SQL queries were not being persisted correctly to the execution plan state.

Key Changes:

• Refactored PlanProcessUtil.getCurrentExecutionStep() to support passing a Plan object directly, preventing loss of plan modifications
• Fixed infinite loop in SQL regeneration by returning Flux.empty() instead of Flux.just(bestSql) when terminating the reactive expand operation

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
PlanProcessUtil.java	Added overloaded getCurrentExecutionStep(Plan, Integer) method to retrieve execution step from a plan instance, enabling proper persistence of plan modifications
SqlGenerateNode.java	Updated to use new overloaded method to preserve SQL modifications in plan; fixed infinite loop by returning Flux.empty() to properly terminate the expand operator when SQL generation completes

Comments suppressed due to low confidence (1)

spring-ai-alibaba-data-agent-chat/src/main/java/com/alibaba/cloud/ai/dataagent/util/PlanProcessUtil.java:86

• The new overloaded getCurrentExecutionStep(Plan, Integer) method lacks test coverage. Consider adding unit tests to verify:

1. It correctly retrieves the execution step from a given plan and step number
2. It throws IllegalStateException when the execution plan is null or empty
3. It throws IllegalStateException when the step index is out of range
4. It behaves consistently with the existing getCurrentExecutionStep(OverAllState) method

Other utility classes in this codebase (e.g., DateTimeUtil, MarkdownParserUtil) have comprehensive test coverage.

	public static ExecutionStep getCurrentExecutionStep(Plan plan, Integer currentStep) {
		List<ExecutionStep> executionPlan = plan.getExecutionPlan();
		if (executionPlan == null || executionPlan.isEmpty()) {
			throw new IllegalStateException("执行计划为空");
		}

		int stepIndex = currentStep - 1;
		if (stepIndex < 0 || stepIndex >= executionPlan.size()) {
			throw new IllegalStateException("当前步骤索引超出范围: " + stepIndex);
		}

		return executionPlan.get(stepIndex);
	}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[tjx222]

请在本地终端运行一下mvn spring-javaformat:apply统一一下代码风格，再提交代码
@VLSMB
已格式化

[VLSMB]

lgtm, thx for your contribution.