Do we think there are any table implementations that rely on this behavior? Any thoughts on a strategy to determine that?

Do we think there are any table implementations that rely on this behavior? Any thoughts on a strategy to determine that?

Feels unlikely. When I've brought this up previously, people thought (a) didn't work this way or (b) was inherent to the implementation and unfixable.

Yeah definitely it wasn't possible to do before, they added sqlite3_vtab_in 2 years ago.

The question here is if the IN operator gets translated to EQUALS in our code.

The problem I see here is that if the tables do not account for this (multiple constraints values for the same constraint), then the result is wrong, because they will filter only with the first value, while with the old method, the table logic would've been called multiple times, each with one of the values in the IN list, so it would've worked correctly.

If IN constraints are matched by EQUALS then we have to check the tables as I mentioned above

Here is a case, in the users table, where it should work fine for instance, but again, not all tables loop on the returned constraints.

if (context.constraints["uid"].exists(EQUALS)) {
    auto uids = context.constraints["uid"].getAll(EQUALS);
    for (const auto& uid : uids) {
      auto const auid_exp = tryTo<long>(uid, 10);
      if (auid_exp.isValue()) {
        getpwuid_r(auid_exp.get(), &pwd, buf.get(), bufsize, &pwd_results);
        if (pwd_results != nullptr) {
          genUser(pwd_results, results);
        }
      }
    }
  }

Also I think due to this, none of the table logic inside the loop has been tested with a loop count > 1. So I suspect we will encounter bugs.

We talked about this briefly at Office Hours today.

@Smjert suggest that incorrect loops should be relatively to audit for -- we can examine tables that have getAll. But I'm not sure we can make comprehensive tests or catch things.

We talked about this briefly at Office Hours today.

@Smjert suggest that incorrect loops should be relatively to audit for -- we can examine tables that have getAll. But I'm not sure we can make comprehensive tests or catch things.

Just to clarify, there are 2 potential issues:

1. Tables that do not handle multiple constraint values on a column
2. Tables that do handle it, but the looping code has never been tested (which I fear is all of them)

For 1., the code will use getAll as the example above. The problematic code won't loop over the elements and will access only the first.

Sorry that I've been silent on this thread, I've been testing/fixing the various table generations this would affect. This was a big oversight I made on my part for the initial push.

For now I'm restricting this to only apply to indexed columns and not columns where additional=True as I felt refactoring all of the generates that would use it would be too big a scope for this PR.

I believe I've pushed all of the changes needed for safely utilizing this sqlite optimization. I'm still doing testing and will update the pr once I complete that.

I had realized what I was trying to do with the index and additional options didn't quite make sense to use for the IN optimization and I wanted to create another option for this change.

I found there was an old artifact in the ColumnOptions enum for an optimized option. The description for this option fits what I'm trying to achieve with this PR. It's not being used anywhere, so I've repurposed it to be used for the IN constraint optimization.

I've removed all of the places where I modified the table generate's use of query context. I'll put those changes into another PR if this one goes through. I figured it'd be more trackable this way and less to review for this PR.

I removed the optimized changes from all of the tables except for curl_certificate to keep as an example. I still have those in another branch, but I wanted to make this one a bit more digestible.