Skip to content

gh-133390: Support SQL keyword completion for sqlite3 CLI#133393

Merged
erlend-aasland merged 54 commits into
python:mainfrom
tanloong:sqlite3-cli-completion
Jun 6, 2025
Merged

gh-133390: Support SQL keyword completion for sqlite3 CLI#133393
erlend-aasland merged 54 commits into
python:mainfrom
tanloong:sqlite3-cli-completion

Conversation

@tanloong

@tanloong tanloong commented May 4, 2025

Copy link
Copy Markdown
Contributor

This adds tab-completion for the 147 SQLite Keywords. A whitespace is appended to completion candidates for users' convenience and to mimic the behavior of sqlite3 tool.

@python-cla-bot

python-cla-bot Bot commented May 4, 2025

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

@bedevere-app

bedevere-app Bot commented May 4, 2025

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@StanFromIreland StanFromIreland left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This needs a whatsnew entry, (maybe a note in the docs?), a NEWS entry and tests.

This is quite limited completion so I guess it could work.

Comment thread Lib/sqlite3/__main__.py Outdated
@picnixz

picnixz commented May 4, 2025

Copy link
Copy Markdown
Member

I think we need a separate nodule for this because if we want to have a smarter completion later the code is better to be isolated, so something like sqlite3._completer.

If possible, can we have an autogenerated list of keywords? and those keywords could also be stored in a (private) module-level list.

Comment thread Lib/sqlite3/__main__.py Outdated
Comment thread Lib/sqlite3/__main__.py Outdated
Comment thread Misc/NEWS.d/next/Library/2025-05-05-03-14-08.gh-issue-133390.AuTggn.rst Outdated
@picnixz

picnixz commented May 4, 2025

Copy link
Copy Markdown
Member

Please add tests

@tanloong

tanloong commented May 4, 2025

Copy link
Copy Markdown
Contributor Author

Yes, tests is coming. I will try to find a way to autogenerate list of keywords.

@StanFromIreland

Copy link
Copy Markdown
Member

You can mark the pr as a draft.

@tanloong tanloong marked this pull request as draft May 4, 2025 21:23
Comment thread Lib/sqlite3/_completer.py Outdated
Comment thread Lib/sqlite3/_completer.py
Comment thread Doc/whatsnew/3.14.rst Outdated
@tanloong tanloong force-pushed the sqlite3-cli-completion branch 8 times, most recently from 782a599 to 5005d85 Compare May 5, 2025 04:47
@tanloong

tanloong commented May 5, 2025

Copy link
Copy Markdown
Contributor Author

Added tests and moved keyword list to module level.

Keywords seems not easy to auto-generate, the SQLite document says they can be accessed by sqlite3_keyword_count() and sqlite3_keyword_name() using C, but I am bad at C and didn't figure out how to call these two at Python level. I think these two functions are not exposed to Python?

@tanloong tanloong marked this pull request as ready for review May 5, 2025 05:08

@StanFromIreland StanFromIreland left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

With #133447 in mind the keywords list would be quite handy (I think hardcoding is fine, it doesn't change often anyway). Maybe it could be moved elsewhere, what do you think Benedikt?

Comment thread Doc/whatsnew/3.14.rst Outdated
Comment thread Lib/sqlite3/_completer.py Outdated
@tanloong tanloong force-pushed the sqlite3-cli-completion branch from 15019ea to 22652ae Compare May 5, 2025 18:20
@tanloong

tanloong commented May 5, 2025

Copy link
Copy Markdown
Contributor Author

Hi @picnixz, as Stan mentioned, beta freeze is around the corner, but I really hope this can be shipped in 3.14, do you think there is a chance to merge it before beta freeze?

@bedevere-bot

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @encukou for commit d681425 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F133393%2Fmerge

The command will test the builders whose names match following regular expression: AMD64.Fedora.Stable.PR

The builders matched are:

  • AMD64 Fedora Stable PR

Comment thread Modules/_sqlite/module.c
Comment thread Modules/_sqlite/module.c Outdated
Comment thread Modules/_sqlite/module.c Outdated
Comment thread Modules/_sqlite/module.c Outdated
Comment thread Modules/_sqlite/module.c Outdated
Comment thread Modules/_sqlite/module.c
for (int i = 0; i < count; i++) {
const char *keyword;
int size;
int result = sqlite3_keyword_name(i, &keyword, &size);

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.

For the record:
We could just assert that the result is SQLITE_OK. This is just an out-of-bounds check, and we know that we are within bounds here. OTOH, we don't know how the SQLite internals may change, so let's keep it like it is.

@erlend-aasland

Copy link
Copy Markdown
Contributor

FTR, CI fails bco. #134077. Let's await a revert or a fix for that.

Otherwise, this looks great. Thank you so much for improving the sqlite3 module, @tanloong. Highly appreciated!

Comment thread Doc/whatsnew/3.15.rst Outdated
@erlend-aasland

Copy link
Copy Markdown
Contributor

FTR, I've tested this with new SQLite variants, as well as SQLite 3.23.0 and 3.24.0. Everything works as expected for all versions. I should really get to setting up a build-bot with a SQLite library matrix.

@erlend-aasland

Copy link
Copy Markdown
Contributor

I only get the "Display all 147 possibilities? (y or n)" prompt with editline, and never on readline. Probably a bug in readline? Or are we missing a configuration option in order to get this to work?

@tanloong

tanloong commented Jun 6, 2025

Copy link
Copy Markdown
Contributor Author

Readline has a completion-query-items (default value 100) option to determine whether or when this prompt should be shown. https://www.gnu.org/software/bash/manual/html_node/Readline-Init-File-Syntax.html#index-completion_002dquery_002ditems

With a default readline config the prompt should have be shown, I suspect there is a config that sets the option to 0 on the testing machine?

@erlend-aasland

erlend-aasland commented Jun 6, 2025

Copy link
Copy Markdown
Contributor

With a default readline config the prompt should have be shown, I suspect there is a config that sets the option to 0 on the testing machine?

Nope. I was a little bit vague in my previous comment: I see the text "Display all 147 possibilities? (y or n)", but it is not shown as a prompt after the first tab. It is only displayed together with the resulting completion list, after the second tab.

IOW: For editline, I get the prompt on the first tab; for readline, I get no prompt, but I see the prompt text after the second tab.

FTR, I already experimented with the completion-query-items option, and it has no impact.

@tanloong

tanloong commented Jun 6, 2025

Copy link
Copy Markdown
Contributor Author

Try set show-all-if-unmodified on in readline and see if it make the prompt shown on just the first tab? https://www.gnu.org/software/bash/manual/html_node/Readline-Init-File-Syntax.html#index-show_002dall_002dif_002dunmodified

Comment thread Lib/sqlite3/_completer.py
@tanloong

tanloong commented Jun 6, 2025

Copy link
Copy Markdown
Contributor Author

Thank you very much for the careful review and thorough test! There are so many corners and difficulties that I would have never considered let alone solved, thanks for everyone's help!

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.

8 participants