fix(nl2sql): extract last SQL block from model output#522
Open
ofyc-666 wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Describe what this PR does / why we need it
This PR fixes an NL2SQL issue where SQL cleanup may extract the first intermediate SQL block instead of the final SQL when the model output contains multiple fenced SQL code blocks.
In the current flow,
SqlGenerateNodecleans the model output throughNl2SqlService.sqlTrim()and stores the result inSQL_GENERATE_OUTPUT. The downstreamSemanticConsistencyNodeandSqlExecuteNodethen consume this SQL. The previous logic extracted the first fenced code block, which could pass a non-final SQL to downstream nodes if the first SQL block was only an intermediate query.Does this pull request fix one issue?
Fixes #486
Describe how you did it
MarkdownParserUtil.extractRawText()unchanged to avoid affecting other Markdown code block parsing scenarios, such as JSON, Python, and chart configuration parsing.MarkdownParserUtil.extractLastRawText()to extract the last fenced code block.Nl2SqlService.sqlTrim()to useextractLastRawText(), so SQL cleanup uses the last fenced block when multiple code blocks are present.The reason for choosing the last block in the SQL cleanup path is that the model output is generated in one pass. It does not actually execute the first SQL and then generate the second SQL based on runtime results. In multiple-SQL-block outputs, earlier SQL blocks are often intermediate or helper queries, while the later SQL block is more likely to be the final executable SQL that already combines the intermediate logic through a subquery or JOIN. Since the current execution pipeline handles a single SQL statement, passing the last SQL block downstream better matches the existing architecture.
Describe how to verify it
Ran the following local tests:
Result:
Special notes for reviews
This PR only changes code block selection in the SQL cleanup path. It does not modify
SqlGenerateNode, the Graph flow, prompts, or add support for sequential execution of multiple SQL statements.