Skip to content

chore: show line of error sql#3390

Merged
andygrove merged 2 commits intoapache:mainfrom
peterxcli:chore/show-erro-sql-line
Feb 4, 2026
Merged

chore: show line of error sql#3390
andygrove merged 2 commits intoapache:mainfrom
peterxcli:chore/show-erro-sql-line

Conversation

@peterxcli
Copy link
Member

Which issue does this PR close?

Closes #3388.

Rationale for this change

show the exact file and line location of the sql that cause error in slt. so the user will be easier identify the problem.

What changes are included in this PR?

only the sql test suit and the sql test parser.

  • the result will be like:
- sql-file: expressions/map/map_contains_key.sql [parquet.enable.dictionary=false] *** FAILED *** (6 seconds, 257 milliseconds)
  In SQL file expressions/map/map_contains_key.sql:29, executing query:
  select map_contains_key(map_from_arrays(array(1, 2), array('a', 'b')), 5.0)
  Expected only Comet native operators, but found Project.
  plan:  Project [COMET: Cast from MapType(IntegerType,StringType,false) to MapType(DecimalType(11,1),StringType,false) is not supported]
  +- Scan OneRowRelation 
  • and the original is like:
- sql-file: expressions/map/map_contains_key.sql [parquet.enable.dictionary=false] *** FAILED *** (5 seconds, 608 milliseconds)
  Expected only Comet native operators, but found Project.
  plan:  Project [COMET: map is not supported]
  +- Scan OneRowRelation 

How are these changes tested?

Copilot AI review requested due to automatic review settings February 4, 2026 12:16
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Improves SQL test failure diagnostics by including the originating .sql file and 1-based line number (plus the SQL text) in assertion failure output for SLT runs.

Changes:

  • Extend SqlStatement and SqlQuery records to carry the starting line number of the SQL text.
  • Update SqlFileTestParser to compute and store the start line for each statement/query.
  • Update CometSqlFileTestSuite to wrap execution with clues that surface relativePath:line and the SQL being executed.

Reviewed changes

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

File Description
spark/src/test/scala/org/apache/comet/SqlFileTestParser.scala Track per-record start line numbers when parsing SQL test files.
spark/src/test/scala/org/apache/comet/CometSqlFileTestSuite.scala Include file/line + SQL text in failure output via withClue.

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

Copy link
Member

@andygrove andygrove left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice improvement! Thanks @peterxcli

@andygrove andygrove merged commit 3b18e1a into apache:main Feb 4, 2026
206 of 209 checks passed
@peterxcli peterxcli deleted the chore/show-erro-sql-line branch February 4, 2026 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Show which line of sql cause error in CometSqlFileTestSuite

2 participants