sqlite3_vtab_in_first

(1) By Roger Binns (rogerbinns) on 2023-01-20 16:24:16 [source]

In the virtual table filter method I would like to always call sqlite3_vtab_in_first on every parameter, without separately having to keep track of sqlite3_vtab_in having been called on it.

https://www3.sqlite.org/c3ref/vtab_in_first.html says that doing so "then these routines return SQLITE_MISUSE or perhaps exhibit some other undefined or harmful behavior". Examining the code shows it will always return SQLITE_MISUSE.

Could the SQLite spec be updated to always return SQLITE_MISUSE, and not potentially undefined/harmful behaviour?

(The for loop on that page is also missing a ; for the middle expression.)

Roger

(2) By Gunter Hick (gunter_hick) on 2023-01-23 07:59:18 in reply to 1 [link] [source]

You are expected to call interfaces in the documented way. Even if the current implementation actually does not do anything harmful or undefined, stating so would place a burden on SQLite development to keep it that way, possibly by adding checking code that costs CPU cycles and maintenance work.

Apart from "wanting to", what reason do you have for coding this way?

(3) By Roger Binns (rogerbinns) on 2023-01-24 23:10:18 in reply to 2 [link] [source]

My code is a wrapper for Python developers to call SQLite. ie I am not the one who is writing the code that is running, nor are the developers writing in C.

In order to only call sqlite3_vtab_in_first in xFilter when sqlite3_vtab_in was called in xBestIndex would require me to track what the argvindex was set to in xBestIndex. That would then require more memory allocation since it could be all of the constraints. I would still have to depend on SQLite implementation details since the multiple constraints could be set to the same argvindex, and I'd have to follow exactly how SQLite behaves in that scenario. And I won't know which xBestIndex call "won" if it got called multiple times anyway.

Currently in xFilter I always call sqlite3_vtab_in_first if sqlite3_value_type() returns SQLITE_NULL, and my test suite will catch it if SQLite then does harmful or undefined things.

Roger

(4) By Gunter Hick (gunter_hick) on 2023-01-25 07:09:12 in reply to 3 [link] [source]

This is exactly the type of information that should be encoded into the idxStr. Maybe like this:

char in_info[p_info->nConstraint+2] = "";

in_info[0] contains the count of IN constraints your xBestIndex decides it wants to process at  once.

in_info[1..nConstraint] contains the (always non-zero) argvIndex assigned to each IN constraint-

The whole buffer can be tagged onto the end of the idxStr, where xFilter can retrieve the information. If you allocate the idxStr with sqlite3_malloc and set needToFreeIdxStr, SQLite will handle deallocating. Also, SQLite passes to xFilter only the "winning" idxStr.

This allows for a maximum of 255 IN constraints on the same virtual table in a query, which should be enough.

(5) By Roger Binns (rogerbinns) on 2023-01-25 14:41:33 in reply to 4 [link] [source]

I understand I can do increasingly clumsy and disproportionate amounts of side channel information to try to track which argvIndex had sqlite3_vtab_in called. That is only necessary because the doc is reserving the right to behave randomly and harmfully if I don't.

I also do thorough testing which means "for a maximum of X IN constraints" is something I would have to cover ensuring X-1, X, X + 1 etc. And I'd have to document that too!

My current judgement is that it is no way worth the effort.

Roger

(6) By Richard Hipp (drh) on 2023-01-25 17:02:48 in reply to 1 [link] [source]

Check-in 144326dc171025dc enhances the sqlite3_vtab_in_first() and sqlite3_vtab_in_next() interfaces so that they now reliably return SQLITE_ERROR if they are invoked on a parameter that did not have multi-value IN clause processing enabled via a prior call to sqlite3_vtab_in().

Prior to that check-in, SQLite could not guarantee that it would reliably detect when the parameter passed to sqlite3_vtab_in_first() or sqlite3_vtab_in_next() was not a the result of sqlite3_vtab_in(). It would usually detect this and return SQLITE_MISUSE. But the detection was not 100% and a hostile application could fake it out, which could lead to memory errors. The check-in linked above fixes that so that the detection is now reliable and therefore the return value is changed to SQLITE_ERROR instead of SQLITE_MISUSE.

This enhancement will appear in version 3.41.0.

(7) By Roger Binns (rogerbinns) on 2023-01-25 18:32:17 in reply to 6 [link] [source]

Thank you for addressing this. The check-in looks good to me.

(8) By Roger Binns (rogerbinns) on 2023-01-26 01:06:14 in reply to 6 [link] [source]

On further valgrind testing it looks like sqlite3_value -> xDel is not always set. That then results in "Conditional jump or move depends on uninitialised value" on the line doing:

    if( pVal->xDel!=sqlite3VdbeValueListFree )

I am only calling sqlite3_vtab_in_first() on values of type SQLITE_NULL and only on the top level (ie not when iterating the in_first/next returns). My guess is that xDel is not set when NULL objects are allocated?

My (absurd) test query is:

    select * from vtable_name where c0 in (1,1.1,'one') and c1 in (?,?,?) and c2 in (?)

With bindings:

    null, x'aabbcc', null, null

I used the source archive corresponding to the commit above and did "make sqlite3.c" with this being the allocation trace (most recent first) for the sqlite3_value with xDel not set:

    malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
    sqlite3MemMalloc (sqlite3.c:25585)
    mallocWithAlarm (sqlite3.c:29277)
    sqlite3Malloc (sqlite3.c:29323)
    sqlite3Malloc (sqlite3.c:29317)
    dbMallocRawFinish (sqlite3.c:29628)
    sqlite3VdbeMakeReady (sqlite3.c:84947)
    sqlite3FinishCoding (sqlite3.c:117668)
    yy_reduce (sqlite3.c:170206)
    sqlite3Parser (sqlite3.c:171914)
    sqlite3RunParser (sqlite3.c:173213)
    sqlite3Prepare (sqlite3.c:137839)
    sqlite3LockAndPrepare (sqlite3.c:137914)
    sqlite3_prepare_v3 (sqlite3.c:138021)

Is this sufficient to diagnose the problem?

Roger

(9.2) By Richard Hipp (drh) on 2023-01-26 14:12:49 edited from 9.1 in reply to 8 [link] [source]

Yeah - I'm seeing similar things here, though I don't have a succinct repro case just yet. Stay alert for further updates to the patch....

Edit: Tests are running on a new patch which I think will work better. I'll know more tomorrow morning.

2nd Edit: All tests are currently passing on the latest trunk version.

(10) By Roger Binns (rogerbinns) on 2023-01-26 16:38:54 in reply to 9.2 [link] [source]

I can confirm all tests good in valgrind even though I added a (temporary) call to sqlite3_vtab_in_first on all sqlite3_value my code accesses. The virtual table destructor change also tested good, allowing for dynamically allocated modules.