Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the empty last line may causes the session to exit directly #4575

Closed

Conversation

Kiss736921
Copy link
Contributor

@Kiss736921 Kiss736921 commented Mar 21, 2023

Why are the changes needed?

If the last line of the file is a blank line, there may be a cmd whose content is empty but the length is not 0 after split by ";". When running to this cmd, it will return false, causing the session to exit directly.

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible
    vim test.sql and write :

image

then start beeline and source this file successfully and session won't exit:

image

image

  • Add screenshots for manual tests if appropriate

  • Run test locally before make a pull request

@codecov-commenter
Copy link

Codecov Report

Merging #4575 (ff3fcb1) into master (824aba6) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##             master    #4575      +/-   ##
============================================
- Coverage     53.28%   53.27%   -0.01%     
  Complexity       13       13              
============================================
  Files           573      573              
  Lines         31501    31502       +1     
  Branches       4239     4239              
============================================
- Hits          16786    16784       -2     
- Misses        13135    13137       +2     
- Partials       1580     1581       +1     
Impacted Files Coverage Δ
...n/java/org/apache/hive/beeline/KyuubiCommands.java 0.00% <0.00%> (ø)

... and 4 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@turboFei turboFei closed this in 06f3884 Mar 22, 2023
turboFei pushed a commit that referenced this pull request Mar 22, 2023
… directly

### _Why are the changes needed?_
If the last line of the file is a blank line, there may be a cmd whose content is empty but the length is not 0 after split by ";". When running to this cmd, it will return false, causing the session to exit directly.

### _How was this patch tested?_
- [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible
vim test.sql and write :
<img width="527" alt="image" src="https://user-images.githubusercontent.com/43542750/226647817-855d866e-836c-4c60-bc81-0bb192d3d6ff.png">
then start beeline and source this file successfully and session won't exit:
<img width="745" alt="image" src="https://user-images.githubusercontent.com/43542750/226652451-823891f3-2768-4165-8816-c052461d1f9c.png">
<img width="598" alt="image" src="https://user-images.githubusercontent.com/43542750/226650769-25a8becf-4655-41ec-bf91-7ae7e28b7d34.png">

- [x] Add screenshots for manual tests if appropriate

- [ ] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request

Closes #4575 from Kiss736921/fix_file_empty_last_line.

Closes #4575

ff3fcb1 [Alex] fix the empty last line may causes the session to exit directly

Authored-by: Alex <zoulimin@kanzhun.com>
Signed-off-by: fwang12 <fwang12@ebay.com>
(cherry picked from commit 06f3884)
Signed-off-by: fwang12 <fwang12@ebay.com>
@turboFei
Copy link
Member

Thanks,merged to master and 1.7

@turboFei turboFei added this to the v1.7.1 milestone Mar 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants