CBL-7986: Remove "flags&1=0" in the result of QueryTranslator whereve…

[jianminzhao]

…r warranted by C4Database::isDeletedTableComplete()

[Copilot]

Pull request overview

This PR updates the query translator to avoid emitting redundant flags & 1 = 0 predicates when the database can guarantee deleted-document tracking via the dedicated kv_del_ tables (per C4Database::isDeletedTableComplete()), and extends tests to validate both “complete” and “incomplete” deletion-tracking scenarios.

Changes:

• Add a delegate hook (isDeletedDocsFullyTracked) so the translator can decide when deletion-flag predicates are necessary.
• Add logic to reduce AND ... meta().deleted branches and select kv_del_ tables when the WHERE clause guarantees only deleted docs.
• Update unit tests to cover both states of deleted-table completeness and adjust keyStoreFromTable to accept kv_del_ tables.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
LiteCore/tests/QueryTranslatorTest.hh	Adds test delegate support for reporting deleted-table completeness.
LiteCore/tests/QueryTranslatorTest.cc	Expands expected SQL for both “deleted table complete” and “incomplete” cases; adds new reduction test coverage.
LiteCore/Storage/SQLiteKeyStore.cc	Allows mapping kv_del_ table names back to the correct KeyStore.
LiteCore/Storage/SQLiteDataFile.hh	Declares isDeletedDocsFullyTracked() for the translator delegate implementation.
LiteCore/Storage/SQLiteDataFile.cc	Implements deleted-doc tracking completeness check based on schema version / deleted-table completeness.
LiteCore/Storage/DataFile.hh	Makes isDeletedTableComplete() const.
LiteCore/Storage/DataFile.cc	Updates isDeletedTableComplete() definition to be const.
LiteCore/Query/Translator/SelectNodes.hh	Tracks “only deleted docs” per source; declares reduction helper.
LiteCore/Query/Translator/SelectNodes.cc	Adds WHERE reduction for deleted-meta branches and gates live-doc filtering on deleted-table completeness.
LiteCore/Query/Translator/QueryTranslator.hh	Extends delegate interface with isDeletedDocsFullyTracked().
LiteCore/Query/Translator/QueryTranslator.cc	Uses “only deleted” signal to select kDeletedDocs table when safe; adds RootContext callback.
LiteCore/Query/Translator/Node.hh	Adds RootContext callback for deleted-doc tracking completeness; exposes setNext.
LiteCore/Query/Translator/Node.cc	Implements Node::setNext.
LiteCore/Query/Translator/ExprNodes.hh	Adds OpNode arg-count and swap primitive used by reduction logic.
LiteCore/Query/Translator/ExprNodes.cc	Implements OpNode::swapArg to support AST rewrites.

Suppressed comments (2)

LiteCore/tests/QueryTranslatorTest.cc:954

• Typo in comment: "cascated" should be "cascaded" (twice).

    // 1. The cascated 'AND', demands ['x._deleted'] to be true.
    // 2. By using del table, the above evaluates to true.

LiteCore/tests/QueryTranslatorTest.cc:991

• Grammar in comment is off ("only analyze" / "leave ... unaddresses"). This reads like a typo and makes the explanation harder to follow.

    // Logically, from the ON predicate, we can use kv_del_.library and remove the use of flags, but this demands more
    // detailed analysis among all ON predicates, WHERE predicates, and whether inner join or outer join,etc.
    // Current approach only analyze the WHERE clause, leave the ON predicates unaddresses.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[cbl-bot]

Code Coverage Results:

Type	Percentage
branches	64.63
functions	77.46
instantiations	71.09
lines	76.06
regions	72.29

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

LiteCore/tests/QueryTranslatorTest.cc:953

• Typo in comment: “cascated” should be “cascaded”.

    // 1. The cascated 'AND', demands ['x._deleted'] to be true.

LiteCore/tests/QueryTranslatorTest.cc:991

• Spelling/grammar in comment: “only analyze … unaddresses” should be “only analyzes … unaddressed”.

    // Current approach only analyze the WHERE clause, leave the ON predicates unaddresses.

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (3)

LiteCore/tests/QueryTranslatorTest.cc:991

• Typo/grammar in comment: “only analyze … unaddresses” should be “only analyzes … unaddressed”.

    // Current approach only analyze the WHERE clause, leave the ON predicates unaddresses.

LiteCore/Query/Translator/SelectNodes.cc:537

• The loop in childIdx compares int i against size_t childCount, which can trigger signed/unsigned comparison warnings and is unnecessary here. Use size_t for the loop variable and cast on return.

                    auto childIdx = [](const OpNode* parent, const Node* child, size_t childCount) -> int {
                        for ( int i = 0; i < childCount; ++i )
                            if ( parent->operand(i) == child ) return i;
                        return -1;
                    };

LiteCore/tests/QueryTranslatorTest.cc:953

• Typo in comment: “cascated” should be “cascaded”.

This issue also appears on line 991 of the same file.

    // 1. The cascated 'AND', demands ['x._deleted'] to be true.

[snej]

There's a lot of code in here to remove the AND clause that tests for the deleted flag. Wouldn't it be better to just not generate that AND clause in the first place?