sqlite: reject plans with unusable virtual table parameters

Return SQLITE_CONSTRAINT from xBestIndex() when a hidden column has
an equality constraint that is not usable at the current point in the
query plan. Accepting such a plan makes xFilter() call rows() with
null, silently dropping rows and returning an empty result, e.g. for
"SELECT DISTINCT * FROM m(t.a) JOIN t ON t.a = m.a". Cost estimates
alone cannot enforce parameter dependencies; rejecting the plan lets
SQLite pick an ordering where the parameter value is available.

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: opencode
PR-URL: #66215
Fixes: #66214
Reviewed-By: Guilherme Araújo <arauujogui@gmail.com>

Author: trivikr

src/node_sqlite.cc

@@ -1201,6 +1201,7 @@ int VirtualTableModule::xBestIndex(sqlite3_vtab* pVTab,
   for (int hidden_idx = 0; hidden_idx < num_hidden; hidden_idx++) {
     int col = mod->hidden_col_indices_[hidden_idx];
 
+    int args_before = argv_index;
     for (int i = 0; i < pInfo->nConstraint; i++) {
       if (pInfo->aConstraint[i].iColumn == col &&
           pInfo->aConstraint[i].usable &&
@@ -1215,6 +1216,20 @@ int VirtualTableModule::xBestIndex(sqlite3_vtab* pVTab,
         break;
       }
     }
+
+    // No usable constraint means the query supplied a parameter whose value is
+    // not available at this point in the plan. Accepting it would pass null to
+    // rows() and silently return empty results, so reject the plan and let
+    // SQLite pick a different ordering instead.
+    // https://www.sqlite.org/vtab.html#enforcing_required_parameters_on_table_valued_functions
+    if (argv_index == args_before) {
+      for (int i = 0; i < pInfo->nConstraint; i++) {
+        if (pInfo->aConstraint[i].iColumn == col &&
+            pInfo->aConstraint[i].op == SQLITE_INDEX_CONSTRAINT_EQ) {
+          return SQLITE_CONSTRAINT;
+        }
+      }
+    }
   }
 
   if (!idx_str.empty()) {

test/parallel/test-sqlite-virtual-table.js

@@ -1,6 +1,6 @@
 // Flags: --expose-gc
 'use strict';
-const { skipIfSQLiteMissing } = require('../common');
+const { skipIfSQLiteMissing, mustCallAtLeast } = require('../common');
 skipIfSQLiteMissing();
 const assert = require('node:assert');
 const { Database } = require('node:sqlite');
@@ -345,6 +345,40 @@ suite('Database.prototype.createModule()', () => {
       assert.strictEqual(received.length, paramCount);
       assert.strictEqual(received[paramCount - 1], 7);
     });
+
+    test('does not pass null for parameters that are unavailable in a plan', () => {
+      const db = new DatabaseSync(':memory:');
+
+      db.createModule('join_params', {
+        columns: [
+          { name: 'value', type: 'INTEGER' },
+          { name: 'param', type: 'INTEGER', hidden: true },
+        ],
+        rows: mustCallAtLeast(function*(param) {
+          assert.notStrictEqual(param, null,
+                                'rows() must not be called with an unavailable ' +
+                                'parameter');
+          if (param !== null) {
+            yield [param];
+          }
+        }),
+      });
+
+      db.exec('CREATE TABLE t (a INTEGER)');
+      db.exec('INSERT INTO t VALUES (1), (2), (3)');
+
+      // With DISTINCT, SQLite may consider a plan where the parameter is read
+      // from the inner table and is not yet available, which used to make
+      // xBestIndex accept it and call rows(null), producing an empty result.
+      const result = db.prepare(
+        'SELECT DISTINCT value FROM join_params, t WHERE join_params.param = t.a'
+      ).all();
+      assert.deepStrictEqual(result, [
+        { __proto__: null, value: 1 },
+        { __proto__: null, value: 2 },
+        { __proto__: null, value: 3 },
+      ]);
+    });
   });
 
   suite('type conversions', () => {