Repository navigation
branch-4.1: [fix](be) Prevent score leakage across Boolean search branches #69053 - #69160
Conversation
(cherry picked from commit a49a9b2)
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
### What problem does this PR solve? Issue Number: None Related PR: apache#69160 Problem Summary: CI session fuzzing can disable expression pushdown or inverted-index queries, but SEARCH cannot evaluate through a row scan. Explicitly enable these prerequisites in the Boolean scoring suite. ### Release note None. This updates test setup and verification. ### Check List (For Author) - Test: The original suite fails with SearchExpr should not be executed without inverted index when both prerequisites start disabled. The fixed suite passes in the same environment; the default-environment suite also passes. - Unit Test / Regression test as described above - Behavior changed: No production behavior change - Does this need documentation: No
There was a problem hiding this comment.
Request changes: one verified P1 correctness issue remains. A nested incomplete scored branch can have its error swallowed when its sibling contributes no scores, allowing matching rows to be returned with score 0. All overlapping observations are merged into the single inline finding; there are no existing blocking comment IDs. No additional user focus was supplied.
Convergence: converged after 2 rounds. All three reviewers returned NO_NEW_VALUABLE_FINDINGS in the latest round; the final sweep found no unresolved candidate or unreviewed changed scope. Full six-file static coverage is complete; R1 is the inline finding and R2 is dismissed with ownership/cache/bitmap evidence.
Critical checkpoint conclusions:
- Goal and behavior: complete indexed AND/OR branches now publish only scores in their final true bitmap, and NOT contributes no positive score. The nested incomplete-evaluation failure contract is broken as described inline.
- Scope: production changes are localized to VCompoundPred and CollectionSimilarity, with BE and SQL regression tests.
- Concurrency/locks: no new threads or locks. SegmentIterator owns a private IndexQueryContext; MATCH and SEARCH score producers run synchronously against it.
- Lifecycle/static initialization: Defer restores the parent collector on Status and exception exits; temporary maps release normally. No new ownership cycle or cross-TU initialization dependency.
- Configuration: no production configuration added; the SQL suite enables the existing required index/pushdown session settings.
- Compatibility: no storage or wire format change; no FE/BE function contract change. The segment evaluation path is shared by cloud and local scans.
- Thrift/proto: unchanged; cross-release ID comparison does not apply.
- Parallel paths: MATCH, SEARCH, non-scoring evaluation, complete/empty/incomplete branches, NOT, scored cache bypass, and score materialization were traced.
- Conditions: OR continues full evaluation with an active collector; AND can stop when no row is true. The new nonempty-score completeness guard is the reported defect.
- Test coverage: all three BE cases and six SQL golden cases plus the expected-error case were inspected. The BE file is registered by the recursive CMake glob. The nested error with an empty/non-scoring sibling is missing.
- Expected results: bitmap/score assertions and every SQL output row/rounded BM25 value are consistent with static arithmetic. This does not verify golden generation or executed behavior.
- Observability: existing child/segment error logs provide expression context; no new distributed operation needs metrics. Error swallowing is covered by the finding.
- Transactions/persistence: no changes to transactions, EditLog, visible versions, delete bitmaps, or persistent state.
- Writes/atomicity: query-only change; no new durable-write, crash-recovery, or atomicity path.
- FE/BE variables: none added or altered.
- Performance/memory: move-based map handoff avoids a full map copy; branch maps add expected transient work. No separate substantiated memory/complexity defect was found; ordinary flat Boolean translation builds balanced trees.
- Additional sweep: collector restoration, nullable/const boundaries, bitmap copy ownership, cache behavior, empty initialized results, NOT and nested errors were revisited; no second verified issue.
Validation and limits: static inspection only. No builds, tests, sanitizers, source changes, or submodule initialization were performed. Author-reported CI/build/test results are not treated as execution evidence from this review. The new suite lacks actual NULL rows, selective LIMIT, ASC and score-range cases; those related production paths were inspected, but their runtime behavior and .out generation provenance were not independently validated.
Review converged after 2 round(s).
|
|
||
| if (branch_similarity != nullptr && _op != TExprOpcode::COMPOUND_NOT) { | ||
| auto scores = branch_similarity->release_scores(); | ||
| if (!scores.empty() && (!all_pass || res.is_empty())) { |
There was a problem hiding this comment.
[P1] Propagate incomplete-scoring failures through parent compounds
Using the new regression table, WHERE (title MATCH_ANY 'apple' AND id > 0) OR tags MATCH_ANY 'missing_token' ORDER BY score() DESC LIMIT 100 reaches OR(AND(scored MATCH, unindexed predicate), empty MATCH). The inner AND moves out and discards its scores before returning INVERTED_INDEX_NOT_SUPPORTED. The outer OR logs that error and continues, but its own map is empty because the other MATCH has no hits, so this !scores.empty() guard returns OK instead of preserving the failure. SegmentIterator then keeps the predicate for residual evaluation and materializes the empty root collector as zero scores; the cached MATCH bitmap still lets the apple rows pass. This silently changes valid positive scores to 0 and can corrupt ranking/score-range results. Propagate the incomplete-scoring error independently of remaining score-map contents, and cover the nested error with an empty/non-scoring sibling.
### What problem does this PR solve? Issue Number: None Related PR: apache#69160 Problem Summary: A nested scored compound discards its temporary scores before reporting unsupported evaluation. An outer AND or OR could swallow that error when its other child contributes no scores, allowing zero-score residual results. Preserve the error through scored parents independently of map contents. ### Release note Nested incomplete scored predicates report an error instead of silently returning zero scores. ### Check List (For Author) - Test: 31 targeted ASAN unit tests, ASAN BE build, and Boolean SQL regression including 16 new expected-error combinations passed. The new SQL case failed on the prior binary. - Behavior changed: Yes. Preserve incomplete-scoring errors through nested compounds. - Does this need documentation: No.
There was a problem hiding this comment.
Reviewed the complete six-file PR at e33634f using the supplied aggregate diff. One new P2 finding: the added per-compound score maps allocate significant temporary memory outside query/workload accounting. No new P0/P1 finding was verified.
Existing P1 thread 4295209565 is fixed on this head: scored parent AND/OR now return INVERTED_INDEX_NOT_SUPPORTED before consulting their score maps; NOT propagates it and SegmentIterator does not downgrade it. It is neither reposted nor retained as an applicable existing blocker.
Review converged after 2 rounds. Both complete reviewers and the separate risk reviewer returned NO_NEW_VALUABLE_FINDINGS in the latest round beyond the unchanged accepted finding. Every changed file, candidate and risk focus has an explicit disposition; no unresolved concrete candidate or uncovered static-review scope remains.
Critical checkpoint conclusions:
- Goal and proof: scoped collectors, true-row filtering and NOT score discard address Boolean branch leakage; nested errors now propagate. Four BE test cases including a matrix and the SQL suite cover these mechanisms in source; execution was prohibited.
- Scope and clarity: production changes are limited to compound index evaluation and two score-map helpers, with corresponding tests and goldens.
- Concurrency and locks: segment-local contexts and cloned expression contexts use synchronous evaluation/collection. No new thread, lock, lock-order or heavy locked operation was introduced.
- Lifecycle and static initialization: shared owners retain callers and branches; Defer restores the collector on normal/error/unwind exits. No cycle, leak or cross-translation-unit initializer was introduced. The inline finding concerns allocation accounting and peak memory.
- Configuration: no setting was added; existing WAND options are consumed through existing paths.
- Compatibility and rolling upgrades: no wire/storage format or existing function signature changed; the behavior is local to query execution.
- Thrift/protobuf: no definitions changed, so release-branch field/enum comparison is inapplicable.
- Parallel paths: AND/OR/NOT, unscored fallback, MATCH legacy readers, SEARCH scorers, bitmap caches and final score materialization were traced. Scored caches are bypassed; FE disables leaf SEARCH top-k for SQL compound predicates.
- Special conditions: absent bitmap storage differs from an allocated empty result; AND empty-true-result exit cannot omit a valid branch contribution, and scored OR evaluates all children. Error-propagation comments explain the new guards.
- Test breadth: BE expectations, branch interfaces and CMake discovery were inspected; SQL covers branch order, failed AND, NOT, repeated queries, nested errors and WAND settings.
- Changed results: all six golden blocks, comprising 47 rows, agree with independently recomputed current-branch BM25 values and row membership. This is arithmetic/static evidence, not executed SQL or proof of golden-generation provenance.
- Observability: existing child/segment logs and explicit incomplete-scoring Status provide diagnostics; no new distributed stage or metrics need was identified.
- Transactions, persistence and failover: no transaction or EditLog path changed; query visibility, rowset versions and delete-bitmap write invariants are unaffected.
- Data writes and crash atomicity: this is read-side evaluation with scoped temporary owners; durable data is not modified.
- FE-BE variables: no new transmitted variable; existing score limit, order and range handling remain connected.
- Performance and memory: moving avoids copying during handoff; merges visit accumulated score entries per compound. The new default-allocator map storage is the verified P2. No separate CPU-scaling defect was substantiated.
- Additional checks: SQL three-valued true/null bitmaps and parent-score preservation were checked; no new nullable/const column conversion or silent incomplete-scoring success was established.
User focus: no additional focus was supplied; the full PR and the existing thread were reviewed.
Validation limits: static review only. No builds, BE/SQL tests, memory measurements, cache/concurrency stress or benchmarks were run, and author/CI execution claims were not treated as executed evidence. Added functional data does not directly exercise NULL rows, ASC, score-range filters or competitive small LIMITs; the relevant implementation paths were inspected. Source files and submodules were left unchanged.
Review converged after 2 round(s).
| query_context != nullptr ? query_context->collection_similarity : nullptr; | ||
| CollectionSimilarityPtr branch_similarity; | ||
| if (parent_similarity != nullptr) { | ||
| branch_similarity = std::make_shared<CollectionSimilarity>(); |
There was a problem hiding this comment.
[P2] Track the new per-compound score-map allocations
For a broad scored predicate such as (title MATCH_ANY 'apple' AND content MATCH_ANY 'search') OR tags MATCH_ANY 'database', these new collectors can each hold an entry per matching segment row, and the child map remains alive while merge_scores grows its parent. ScoreMap is a raw phmap::flat_hash_map using the default allocator; this branch's malloc hook forwards directly to jemalloc, whereas MemTracker accounting occurs in Allocator::consume_memory. The new segment-sized temporary maps therefore bypass query/workload memory accounting and limits, increasing untracked peak memory with Boolean nesting and concurrent segment scans. Please use a tracked allocator for the score maps (for example the existing doris::flat_hash_map alias), preserving balanced accounting across release/merge/destruction.
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
1 similar comment
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
There was a problem hiding this comment.
Reviewed all seven changed paths at head 83a8026. No new verified findings. The main review and three independent reviewers converged in one round; all reviewers returned NO_NEW_VALUABLE_FINDINGS.
The requested allocator focus is addressed: ScoreMap now uses doris::flat_hash_map and its tracked Allocator_; reserve/growth count toward query memory accounting and existing limit/reservation policy. release_scores moves ownership without a second allocation charge; merge keeps both source and destination charged until their storage is released. Scanner evaluation and teardown attach the query tracker. Existing P2 comment 4295606943 is fixed. Existing P1 comment 4295209565 is also fixed: scored AND/OR propagate a nested unsupported-scoring error regardless of remaining score-map contents, NOT forwards the error, and SegmentIterator does not downgrade it. Neither comment is reposted; no previously reported P0/P1 remains confirmed applicable.
Critical checkpoint conclusions:
- Goal: final positive bitmaps filter branch scores; failing AND branches and NOT do not add positive scores. The added unit/SQL expectations agree with this behavior.
- Scope: production edits stay within compound scoring and collector allocation/merge APIs, with focused tests.
- Concurrency/locks: recursive evaluation uses per-segment contexts and cloned expression contexts; no new threads, shared cross-segment collector or lock ordering.
- Lifecycle/static initialization: scoped collectors and Defer restore parents on success/error/unwind; moved maps have local ownership and query-tracker teardown. No cycle or new static initializer.
- Configuration: no configuration item added or dynamic semantics changed.
- Compatibility: internal container/API changes only; no wire or persisted format change.
- Thrift/protobuf: no definitions changed; cross-release field-ID comparison is inapplicable.
- Parallel paths: MATCH/SEARCH, AND/OR/NOT, unscored evaluation and score materialization checked. FE disables independent SEARCH top-k when nested among other predicates; scored paths bypass bitmap-only result caches.
- Special conditions: scored OR retains all score contributions; empty-result AND and explicit incomplete-scoring rejection preserve the intended restriction on partial index evaluation.
- Test coverage: registered unit tests cover branch filtering, NOT, nested errors, parent restoration, map transfer/merge/destruction/reuse and initial allocation rejection; SQL adds ranking and negative cases.
- Expected results: all new row sets and rounded BM25 values were independently checked against the checked-in scoring implementation; output generation provenance is not established.
- Observability: existing child/segment error logging and explicit incomplete-scoring errors cover these paths; no additional distributed operation or metric need identified.
- Transactions/persistence: no EditLog, failover, transaction visibility or durable metadata change.
- Data writes/crash atomicity: query-local evaluation only; no write/publication protocol change.
- FE-BE variables: none added; existing score-limit/range plumbing inspected.
- Performance: score handoff moves the map; merge scans source scores with bitmap membership filtering. Temporary-map coexistence is now tracked; no additional actionable performance defect verified.
- Other invariants: no visible-version, MoW/delete-bitmap or column-shape contract change; investigated generic-error and conservative empty-partial-AND cases yielded no introduced defect.
Validation limits: static review only; no builds or tests executed, no source modifications or submodule initialization. The added data has no NULL rows; the WAND toggle surrounds MATCH rather than the SEARCH WAND branch; repeated scored queries bypass bitmap-only caches. Existing collector tests and source paths were inspected for nullable/range/order behavior. External parallel_hashmap headers were absent; allocator integration and move ownership were checked from available source. Runtime growth-failure/fault-injection behavior and author-reported test runs were not independently executed. These are evidence limits, with no unreviewed changed path or unresolved finding. Converged: true; rounds: 1.
Review converged after 1 round(s).
There was a problem hiding this comment.
Reviewed all eight changed paths at eb3f81e695329189c3d73cf775f78082925b08bb against the supplied aggregate diff. No new actionable findings. The production, tests/session, and additional risk reviewers all returned NO_NEW_VALUABLE_FINDINGS; review converged in round 1 with no unresolved candidate or uncovered changed path.
User focus: test_search_score_raw_index.groovy sets enable_common_expr_pushdown=true and enable_inverted_index_query=true before any query. Both settings reach the actual execution gates. The authoritative raw-index diff adds only those settings and their comment; all existing SQL, V2/V3 and WAND combinations, golden results, and positive/bounded-score assertions remain intact. The separate fuzzy inverted-expression switch does not gate the traced SEARCH path on this branch. Boolean score isolation, nested error propagation, and tracked-memory fixes are retained.
Existing findings independently revalidated: P1 #4295209565 is addressed by immediate propagation of nested INVERTED_INDEX_NOT_SUPPORTED through scored AND/OR, independent of remaining score-map contents; NOT already propagates child failures. P2 #4295606943 is addressed by the Doris tracked ScoreMap allocator, including allocation/free accounting and move/merge ownership. Neither is reposted, and no existing P0/P1 remains applicable.
Critical checkpoint conclusions:
- Goal and tests: completed AND/OR branches publish scores only for their true rows; NOT contributes no positive score. Unit and SQL cases cover overlapping branches, failed AND branches, NOT, and nested incomplete evaluation.
- Scope and clarity: changes are confined to score collection/accounting, supporting tests, and required session prerequisites. Branch compaction paths and compile-check guards are preserved.
- Concurrency and locks: new state is local or belongs to a segment-specific query context; recursive evaluation is synchronous. Scanner tasks attach the query memory context. No new threads, locks, lock-order change, or heavy work under locks.
- Lifecycle and initialization: collector construction precedes replacement; Defer restores the parent on success, error, and exception. Maps have RAII ownership and move handoff; scanner cleanup restores the query tracker. No new static initialization or ownership cycle.
- Configuration: no new production configuration; suites set existing session gates explicitly, and existing FE serialization forwards them to BE.
- Compatibility: no wire, persisted format, function registration, or rolling-upgrade protocol change; the map alias is internal memory storage.
- Thrift/protobuf: no generated-definition changes; cross-release field/enum comparison is not applicable.
- Parallel paths: checked scored/unscored AND/OR/NOT, MATCH/SEARCH, warm bitmap-cache paths, WAND/non-WAND, multiple conjuncts, and score materialization/range filtering. Existing FE suppression of SEARCH leaf TopK under SQL compounds preserves full branch bitmaps.
- Special conditions: unsupported child errors do not depend on score contents. An uninitialized bitmap differs from a valid zero-hit bitmap. Score filtering uses true rows while existing null-bitmap logic remains unchanged.
- Test coverage: both C++ files are included by recursive CMake globs. The nested unit matrix covers eight combinations; SQL covers 16 WAND/sibling/order/repeat error cases. Regression output is deterministically ordered and expected failures use the standard test/exception DSL.
- Expected results: all six Boolean golden sections' membership and scores agree with independent arithmetic using the checked-in BM25 formula. Raw-index golden results are unchanged.
- Observability: existing expression/status logging, allocation tracker diagnostics, index timers and cache counters cover the modified paths; no additional distributed-operation metric is required here.
- Transactions, persistence and failover: no EditLog, transaction, metadata/version publication, or persisted-state change.
- Data writes and crash safety: production changes affect query-local read-time maps. Existing write atomicity and crash recovery paths are unchanged; local cleanup uses RAII.
- FE-BE propagation: no new transmitted variable. Existing score order/limit/range and both session settings reach their current consumers; no additional sending path is needed.
- Performance: score handoff moves map ownership; merge makes one pass over branch scores with bitmap membership checks. Coexisting child/parent maps remain charged until released; no actionable additional performance regression was substantiated.
- Other invariants and final sweep: committed visibility, version ceilings and delete-bitmap behavior are unaffected. Rechecked empty/short-circuit branches, NOT, cache reuse, exception restoration, segment isolation, and allocation cleanup; no further verified issue.
Validation limits: this was a static-only review. No builds, tests, sanitizer runs, or submodule initialization were performed. Author-reported validation is not treated as execution evidence from this invocation. The new Boolean suite declares nullable columns but has no NULL-row fixture; relevant null behavior was traced statically. Static inspection cannot establish golden-file generation provenance or prove runtime allocator fault handling. These are validation limits, with no unresolved code candidate remaining.
Review converged after 1 round(s).
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
### What problem does this PR solve? Issue Number: None Related PR: apache#69159 Problem Summary: The raw SEARCH score suite requires expression pushdown and inverted-index execution. Fuzzy sessions can disable expression pushdown, leaving SEARCH to row evaluation and causing an internal error. Set both options explicitly before the suite runs. ### Release note None ### Check List (For Author) - Test: The identical suite file passed CloudP0 in apache#69160 (build 1072387); git diff --check passed. CI for this PR is pending. - Behavior changed: No. Test setup only. - Does this need documentation: No.
|
run buildall |
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
|
PR approved by anyone and no changes requested. |
|
PR approved by at least one committer and no changes requested. |
…nches apache#69053 (apache#69160) ### What problem does this PR solve? Related PR: apache#69053 Problem Summary: Backport the Boolean SEARCH score-isolation fix to `branch-4.1`. Scores from failing AND branches must not leak into rows matched by another OR branch; NOT contributes no positive score. Incomplete scored evaluation returns an explicit error. #### Branch adaptations Preserve branch-4.1 compaction paths and compile-check guards, add move-based score handoff, and adapt tests to the branch-4.1 IndexExecContext constructor and expression interface. No master merge or storage-directory refactor is included. #### Nested error propagation A nested scored child releases its temporary score map before reporting unsupported index evaluation. Its parent AND/OR previously swallowed this error when no sibling contributed scores, returning matching rows with score zero. Scored parents now propagate this error independently of remaining map contents. Unscored evaluation keeps its existing fallback behavior. The SQL suite explicitly enables SEARCH's required expression-pushdown and inverted-index settings under CI session fuzzing. #### Score-map memory accounting Compound score tables now use the existing Doris tracked allocator. Allocations count toward query memory limits; move handoff preserves the charge, and merging retains charges for both maps until their storage is released.
What problem does this PR solve?
Related PR: #69053
Problem Summary: Backport the Boolean SEARCH score-isolation fix to
branch-4.1. Scores from failing AND branches must not leak into rows matched by another OR branch; NOT contributes no positive score. Incomplete scored evaluation returns an explicit error.Branch adaptations
Preserve branch-4.1 compaction paths and compile-check guards, add move-based score handoff, and adapt tests to the branch-4.1 IndexExecContext constructor and expression interface. No master merge or storage-directory refactor is included.
Nested error propagation
A nested scored child releases its temporary score map before reporting unsupported index evaluation. Its parent AND/OR previously swallowed this error when no sibling contributed scores, returning matching rows with score zero. Scored parents now propagate this error independently of remaining map contents. Unscored evaluation keeps its existing fallback behavior.
The SQL suite explicitly enables SEARCH's required expression-pushdown and inverted-index settings under CI session fuzzing.
Score-map memory accounting
Compound score tables now use the existing Doris tracked allocator. Allocations count toward query memory limits; move handoff preserves the charge, and merging retains charges for both maps until their storage is released.
Release note
Boolean SEARCH scores exclude unmatched branches and NOT predicates. Nested incomplete scored predicates report an error instead of silently returning zero scores. Temporary score tables respect query memory limits.
Check List (For Author)
Validation
Local validation for
83a8026247934c9101590a85d1ea354d57657369:f3789613c03a60930179a068dfae4dfedfed2876; the complete PR English check passed. The BE ELF requires GLIBC no newer than 2.10 (baseline 2.17).Latest revision
eb3f81e695329189c3d73cf775f78082925b08bbmerges branch-4.1f3789613c03a60930179a068dfae4dfedfed2876and fixes the inherited raw-index SEARCH suite's required session settings. Cloud P0 1072342 on the previous revision finished with one non-muted failure at the first unscored SEARCH query. The suite now explicitly enables expression pushdown and inverted-index queries; its SQL, golden results, result assertions and scoring checks are unchanged.Fresh validation on the merged target:
The Cloud FE configuration enables fuzzy session variables, and expression pushdown can be randomized. The exact value in the failed CI connection has not been directly recovered; the missing suite prerequisite and its red/green reproduction are established separately.
The actual automated static review completed successfully for this exact revision (review 5556705069, workflow run 39010960648), with no new actionable findings and zero new inline comments. It confirmed the required raw-suite session settings preserve the existing SQL, golden results and score assertions, and revalidated the previous P1/P2 fixes. Static review is separate from the executed local validation above. Current-head TeamCity compile 1072377, Cloud P0 1072387 and Vault P0 1072388 finished successfully. Cloud P0 reports 4,766 passed, zero non-muted failures and two muted tests. Both the raw-index and Boolean score suites executed successfully. The raw suite occurrence reports SUCCESS and muted=false for this build, while currentlyMuted=true at lookup; the Boolean suite reports SUCCESS with both mute flags false. These results are reported separately from the current global mute state. BE UT 1072378 also finished successfully with 11,252 passed and 75 ignored; P0 1072385 finished successfully with 5,732 passed and two muted tests. External 1072386 finished successfully with 710 passed, zero non-muted failures and four muted tests. NonConcurrent 1072389 finished successfully with 456 passed, zero non-muted failures and one muted test. All seven compilation, unit-test and regression configurations listed above have now finished successfully. Current-head coverage builds 1072419 (Merge P0 UT) and 1072420 (FE coverage) also finished successfully. All nine listed TeamCity configurations have now finished successfully at this exact revision, with no builds running or queued at verification. Coverage results from older revisions are not counted as validation of this revision. All listed builds were verified at this exact head; no duplicate dispatch was made. Earlier builds for
83a8026247934c9101590a85d1ea354d57657369do not validate this revision. Maintainer approval remains pending.