Skip to content

Keep Ctrl+C working in the SQL shell after multi-line statements - #40876

Closed
ahmedabu98 wants to merge 1 commit into
apache:masterfrom
ahmedabu98:sql-shell-ctrl-c
Closed

ahmedabu98 wants to merge 1 commit into
apache:masterfrom
ahmedabu98:sql-shell-ctrl-c

Conversation

@ahmedabu98

Copy link
Copy Markdown
Contributor

Follow-up to #40666. After a multi-line statement, sqlline 1.4's JLine 2 can leave the terminal's interrupt character undefined, so Ctrl+C no longer cancels a running statement. The shell now uses a Unix terminal that always restores the interrupt character it captured at startup.

🤖 Generated with Claude Code

sqlline 1.4 has JLine 2 undefine the terminal's interrupt character while it
reads a line, then restore a value read from a stty cache that can already
say "undefined". Ctrl+C then stopped reaching running statements, so the
cancel from apache#40666 never ran. Restore the character captured at startup.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added java extensions sql size/s S: 10-49 lines changed labels Oct 28, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📋 Review Summary

This PR fixes an issue in the Beam SQL shell where Ctrl+C stops cancelling running queries after a multi-line statement is entered. It works around JLine 2's 1-second stty -a output cache in UnixTerminal by registering an InterruptRestoringTerminal that captures the intr setting once during terminal initialization and reliably restores it after each line read.

🔍 General Feedback

  • The root cause analysis of JLine 2's TerminalLineSettings caching interaction with UnixTerminal.disableInterruptCharacter() during multi-line reads is spot on, and the InterruptRestoringTerminal workaround is clean and minimal.
  • Removing permitUnusedDeclared "jline:jline:2.14.6" in build.gradle properly reflects the new direct usage of JLine classes in BeamSqlLine.

💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.

@ahmedabu98 ahmedabu98 closed this Oct 28, 2026
@ahmedabu98
ahmedabu98 deleted the sql-shell-ctrl-c branch October 28, 2026 23:26
@codecov

codecov Bot commented Oct 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.16%. Comparing base (0f1ac43) to head (f1ff505).
⚠️ Report is 15 commits behind head on master.

Files with missing lines Patch % Lines
...ache/beam/sdk/extensions/sql/jdbc/BeamSqlLine.java 50.00% 9 Missing and 2 partials ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             master   #40876       +/-   ##
=============================================
+ Coverage          0   64.16%   +64.16%     
- Complexity        0     2359     +2359     
=============================================
  Files             0      321      +321     
  Lines             0    15628    +15628     
  Branches          0     2032     +2032     
=============================================
+ Hits              0    10027    +10027     
- Misses            0     4833     +4833     
- Partials          0      768      +768     
Flag Coverage Δ
java 64.16% <50.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant