Skip to content

fix(text2sql): isolate missing-database errors in batch_compare_queries - #560

Closed
cyberspace-cs wants to merge 1 commit into
OpenDCAI:mainfrom
cyberspace-cs:fix/batch-compare-missing-db-isolation
Closed

cyberspace-cs wants to merge 1 commit into
OpenDCAI:mainfrom
cyberspace-cs:fix/batch-compare-missing-db-isolation

Conversation

@cyberspace-cs

Copy link
Copy Markdown
Contributor

Summary

Fixes #556.

DatabaseManager.batch_compare_queries previously validated every distinct db_id up front and, when any one was missing, returned an error result for all triples:

if not self.database_exists(db_id):
    return [self._create_error_result(error_msg) for _ in query_triples]

Because SQLExecutionClassifier submits the whole dataframe in one call, a single row pointing at a missing database made every other comparison fail with "Predicted SQL failed; Ground truth SQL failed", corrupting the difficulty labels for the entire dataset — even rows whose database exists and whose SQL matches.

Fix

  • Validate each triple individually.
  • Execute only the triples whose database exists (in a single batched batch_execute_queries call, as before).
  • Emit a per-row error result for missing databases, and place results back at their original indices so output order always matches input order.
  • Empty batch still returns []; existing equal / unequal comparison behavior is unchanged.

Behavior

For the reproducer in #556:

Triple Before After
(concert, same sql, same sql) ❌ False, Database 'missing_db' not found ✅ True, []
(missing_db, ...) ❌ error ❌ False, Database 'missing_db' not found

Tests

Added test/cpu_only/test_batch_compare_queries_missing_db.py (cpu-only, SQLite, no model/network):

  • a missing db only fails its own triple, in both orderings
  • each missing db gets its own error message
  • valid but unequal queries are still compared
  • empty batch returns []

pytest -m cpu test/cpu_only/test_batch_compare_queries_missing_db.py → 5 passed.

…es (OpenDCAI#556)

A single triple whose db_id is not registered previously made the whole
batch return an error result, so every valid comparison (including rows
whose database exists and whose SQL matches) failed too.

Validate each triple individually, execute only those whose database
exists, and place a per-row error result for missing databases while
preserving input order. Empty batch still returns [].

Adds cpu-only regression tests covering isolation, order independence,
per-db error messages, the normal unequal path and the empty batch.
@cyberspace-cs

Copy link
Copy Markdown
Contributor Author

Closing this as a duplicate — I missed #557 when I opened it, and it fixes the same root cause (per-row isolation, original order preserved) with the same approach. Two PRs for one issue only splits review attention, so #557 is the one that should land.

If the extra cases in my branch are useful (missing database in the first position, and a batch where every database is missing), I am happy to port them into #557 as a follow-up commit rather than keep this open. Sorry for the noise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DatabaseManager.batch_compare_queries fails every comparison when one db_id is missing

1 participant