Support question mark

[amjith]

Description

Support sqlite prepared statement syntax in favourite queries.

Addresses #45

[amjith]

@zmwangx Can you give this a shot and tell me if this is what you had in mind?

You can install this branch using pip install -U https://github.com/dbcli/litecli/archive/support-question-mark.zip

This only adds support for ? in the queries, not the other bind parameters.

[codecov-io]

Codecov Report

Merging #51 into master will increase coverage by 0.59%. The diff coverage is 87.5%.

@@            Coverage Diff             @@
##           master      #51      +/-   ##
==========================================
+ Coverage    67.6%   68.19%   +0.59%     
==========================================
  Files          21       21              
  Lines        1318     1330      +12     
==========================================
+ Hits          891      907      +16     
+ Misses        427      423       -4

Impacted Files	Coverage Δ
litecli/packages/special/iocommands.py	50.58% <87.5%> (+4.08%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1d061a3...6be87fa. Read the comment docs.

[zmwangx]

Thanks. This is it for the most part, but the way argument quoting is handled makes it somewhat problematic. sqlite3_bind handles escaping automatically; dumb substitution does not — which is still fine until quotes around arguments are stripped by the parser. Consider this:

> CREATE TABLE user (id PRIMARY KEY, name TEXT NOT NULL);
Query OK, 0 rows affected
Time: 0.001s
> INSERT INTO user VALUES (1, "dude");
Query OK, 1 row affected
Time: 0.001s
> \fs user_by_name SELECT * FROM user WHERE name = ?
Saved.
Time: 0.001s
> \f user_by_name "dude"
-- SELECT * FROM user WHERE name = dude;
no such column: dude
> \f user_by_name "SELECT"
-- SELECT * FROM user WHERE name = SELECT;
near "SELECT": syntax error
> \f user_by_name "'dude'; DROP TABLE user"
-- SELECT * FROM user WHERE name = 'dude'; DROP TABLE user;
> SELECT * FROM user WHERE name = 'dude'
+----+------+
| id | name |
+----+------+
| 1  | dude |
+----+------+
Time: 0.012s

> DROP TABLE user
Time: 0.001s

As you can see, to actually substitute in a string, one needs two levels of quoting — the outer level to defeat mangling by the argument parser, so it's not ideal.

Alternatively, one might suggest saving the query as SELECT * FROM user WHERE name = '?', but that's not valid as a prepared statement anymore; the '?' is treated as a string literal, not a parameter that can be bound to.

[amjith]

You're right. Let me think about this and take another stab at it tonight.

[zmwangx]

FWIW, shlex in non-POSIX mode does not strip quotes. It's otherwise pretty hard to extract raw tokens from shlex.

[amjith]

@zmwangx I have fixed the quoting issue. Can you give this another try?

Sorry it took me this long to get back to it. Life gets in the way. :smiley:

[zmwangx]

Hi @amjith, sorry for the slow reply here and thanks for not forgetting about this!

Unfortunately, the new commits seem to have fixed my use case (where question marks are not quoted, as how prepared queries are written, and supplied parameters are quoted, which is okay) while breaking the existing and advertised one, namely smart (?) handling of quotes around parameters. Using the example on https://litecli.com/favorites/:

:memory:> CREATE TABLE users (name TEXT NOT NULL);
...
:memory:> \fs user_by_name select * from users where name = '$1'
...
:memory:> \f user_by_name "Skelly McDermott"
> select * from users where name = '"Skelly McDermott"'

Compare this to existing behavior where the last query would be select * from users where name = 'Skelly McDermott'.

Basically, the deeper problem is that quoting behavior expectations for the shell style (up to debate) and the prepared statement style (rather established, at least IMHO) are somewhat different.

I noticed that the newly added test cases only involve integers without quotes as parameters. Maybe adding ones with quoted strings written in various ways should make the issue clearer. Something like:

# create table `test` with a TEXT field `name` first
run(executor, "\\fs sh_param_not_quoted select * from test where name=$1")
run(executor, "\\fs sh_param_quoted select * from test where name='$1'")
run(executor, "\\fs q_param select * from test where name=?")
run(executor, "\\f sh_param_not_quoted word")  # never worked, probably shouldn't be supported
run(executor, "\\f sh_param_not_quoted 'two words'")  # didn't work in the past, works now because quotes around the parameter aren't stripped
run(executor, "\\f sh_param_quoted word")  # works
run(executor, "\\f sh_param_quoted 'two words'")  # used to work, and documented on https://litecli.com/favorites/, but currently broken since again the quotes aren't stripped
run(executor, "\\f q_param word")  # doesn't work, not sure if should be supported, but I can totally live with having to put quotes around "word"
run(executor, "\\f q_param 'two words'")  # works, hurrah

(I'm typing this after a long day, so I didn't actually inspect the delta to understand what changed, and maybe I'm somehow confused and am testing the wrong thing... If that's the case please excuse me.)

[amjith]

Thank you for the excellent test case addition. This gives me a good path forward. I will give it a shot tonight.

[amjith]

I've been reading up on the python adapter for sqlite3 and it looks like there is a way to bind the variables using the execute() function. https://stackoverflow.com/questions/4360593/python-sqlite-insert-data-from-variables-into-table

But the semantics of that is different from the shell bindings we currently support.

It is not good to change the existing behavior because that will break compatibility for existing users.

So I want to get back to your original request. What is the motivation behind your request? Can you work with the existing implementation?

[zmwangx]

it looks like there is a way to bind the variables using the execute() function.

Yes, it's formally defined in PEP 249 (Python Database API Spec v2). See specifically https://www.python.org/dev/peps/pep-0249/#id14 and https://www.python.org/dev/peps/pep-0249/#paramstyle. Of course the syntax is based on established conventions used by DBMSes themselves. See for instance sqlite3_bind().

What is the motivation behind your request?

The motivation is interoperability. The "shell style" definition AFAIK is only usable in litecli, while the question mark binding style could be used directly in source code and possibly other database companion tools, and it could appear in logs and stuff. E.g. one thing I frequently do while troubleshooting a website/web app backend is to copy a parametrized query (in question mark style, not surprisingly) from logs and play with the query in an REPL. Currently I have to replace the question marks with $1, $2, ... if I want to use the query in litecli.

Now that I think about it, it's probably hard to support both styles in the same code path due to quoting issues (IIRC that's partly why I raised an issue instead of a PR...), but why about a separate set of commands for the question mark style? Say \fqs to save a question-mark-style favorite query, and \fq to use one?

[amjith]

@zmwangx I have update the PR to use the built in sqlite.execute() binding feature. It will only be used when there are ? in the favorite query. If we see $ in the favorite query it will fall back to the substitution method.

I think this achieves what you're looking for. Let me know if I'm mistaken.

BTW, sorry about this long drawn out process.

[amjith]

Merging.