From 057274094df061b6d8e6a75d99139adb45c93190 Mon Sep 17 00:00:00 2001 From: "lei.chen" Date: Fri, 7 Aug 2026 08:40:50 +0000 Subject: [PATCH] fix(apiserver): apply CompConditions in ExecuteQuery so job-by-id actually filters (bug-1490 adjacent) GetJobByIDs (GET /xce/v1/jobs/:jobids) built its job_id filter via ConvertToNewFilterArray into QueryOptions.CompConditions, but ExecuteQuery only ever applied opts.Conditions and silently dropped CompConditions. Every per-id job query therefore returned the ENTIRE job table, and the MCP scheduler_get_job tool returned the first job in the store regardless of the requested id (observed: get_job(73930445842022400) -> "post-clean"). Fix: add FilterCompConditions (per-field AND, OR across ConditionArrays, main-condition AND OR(sub-conditions) within an array) and run it in ExecuteQuery after the flat filter; extract the bitmask-aware per-row match into rowMatchesCondition shared by FilterRows and FilterCompConditions. Verified via the MCP server: scheduler_get_job now returns the requested job (mcp-e2e, EXIT), and the full submit->hold->release->cancel->get cycle still works. Co-Authored-By: Claude --- src/frontend/apiserver/store/query.go | 78 ++++++++++++++--- src/frontend/apiserver/store/query_test.go | 97 ++++++++++++++++++++++ 2 files changed, 163 insertions(+), 12 deletions(-) diff --git a/src/frontend/apiserver/store/query.go b/src/frontend/apiserver/store/query.go index 631852e..b603c11 100644 --- a/src/frontend/apiserver/store/query.go +++ b/src/frontend/apiserver/store/query.go @@ -17,10 +17,16 @@ func ExecuteQuery(rows []utils.OrderMap, opts *utils.QueryOptions, fields []stri return SelectFields(rows, fields), "" } - // 1. Filter + // 1. Filter — both the flat AND-conditions AND the compound (per-field OR, + // array-aware) conditions. bug-1490-adjacent: CompConditions used to be + // silently dropped here, so GET /xce/v1/jobs/:jobids returned the whole + // job table (the mcp scheduler_get_job tool never saw the requested job). if len(opts.Conditions) > 0 { rows = FilterRows(rows, opts.Conditions) } + if len(opts.CompConditions) > 0 { + rows = FilterCompConditions(rows, opts.CompConditions) + } // 2. Sort if len(opts.Orders) > 0 { @@ -105,28 +111,76 @@ func IsBitmaskField(field string) bool { return field == "job_status" || field == "host_status" } +// rowMatchesCondition evaluates a single condition against a row, dispatching +// bitmask fields (job_status/host_status) to MatchesBitmaskCondition and all +// others to MatchesCondition. +func rowMatchesCondition(row utils.OrderMap, cond utils.Condition) bool { + if IsBitmaskField(cond.Column) { + bitmask, ok := cond.Value.Value.(int64) + if !ok { + return false + } + return MatchesBitmaskCondition(row, cond.Column, bitmask, cond.Operator) + } + return MatchesCondition(row, cond) +} + // FilterRows applies all conditions (AND logic) to rows. func FilterRows(rows []utils.OrderMap, conditions []utils.Condition) []utils.OrderMap { result := make([]utils.OrderMap, 0, len(rows)) for _, row := range rows { match := true for _, cond := range conditions { - if IsBitmaskField(cond.Column) { - bitmask, ok := cond.Value.Value.(int64) - if !ok { - match = false + if !rowMatchesCondition(row, cond) { + match = false + break + } + } + if match { + result = append(result, row) + } + } + return result +} + +// FilterCompConditions applies compound conditions — the per-field OR structure +// built by ConvertToNewFilterArray for array-aware queries (e.g. job_id with +// array_idx sub-ranges in GET /xce/v1/jobs/:jobids, or job_name filters in +// GET /xce/v1/jobs). A row must match EVERY field key present (AND across +// fields); within a field, the row matches if ANY of its ConditionArrays +// matches. A ConditionArray matches when its MainCondition holds AND either it +// has no SubORConditions or any sub-condition holds. +func FilterCompConditions(rows []utils.OrderMap, comp utils.CompCondition) []utils.OrderMap { + if len(comp) == 0 { + return rows + } + result := make([]utils.OrderMap, 0, len(rows)) + for _, row := range rows { + match := true + for _, arrays := range comp { + fieldMatch := false + for _, arr := range arrays { + if !rowMatchesCondition(row, arr.MainCondition) { + continue + } + if len(arr.SubORConditions) == 0 { + fieldMatch = true break } - if !MatchesBitmaskCondition(row, cond.Column, bitmask, cond.Operator) { - match = false - break - } - } else { - if !MatchesCondition(row, cond) { - match = false + for _, sub := range arr.SubORConditions { + if rowMatchesCondition(row, sub) { + fieldMatch = true + break + } + } + if fieldMatch { break } } + if !fieldMatch { + match = false + break + } } if match { result = append(result, row) diff --git a/src/frontend/apiserver/store/query_test.go b/src/frontend/apiserver/store/query_test.go index a1d1f9b..f980492 100644 --- a/src/frontend/apiserver/store/query_test.go +++ b/src/frontend/apiserver/store/query_test.go @@ -83,3 +83,100 @@ func TestFilterRows_JobStatusNotinExitDone(t *testing.T) { t.Fatalf("job_status notin(EXIT,DONE): got %d rows, want exactly the RUN job", len(got)) } } + +// makeJobRowIdx builds a job row with a job_id and array_idx, as StoreToOrderMaps +// produces for array-job rows. +func makeJobRowIdx(jobid int64, arrayIdx int, status int) utils.OrderMap { + row := makeJobRow(jobid, status) + row.Data["array_idx"] = arrayIdx + return row +} + +// bug-1490-adjacent: ExecuteQuery ignored CompConditions, so GET /xce/v1/jobs/:jobids +// (GetJobByIDs) never applied its job_id filter and returned the ENTIRE job +// table. The mcp scheduler_get_job tool therefore returned the first job in the +// store regardless of the requested id. +func TestExecuteQuery_CompConditions_JobIDFilter(t *testing.T) { + rows := []utils.OrderMap{ + makeJobRow(1, 0x08), // DONE + makeJobRow(2, 0x04), // EXIT + makeJobRow(3, 0x02), // RUN + } + comp := utils.CompCondition{ + "job_id": []utils.ConditionArray{ + { + MainCondition: utils.Condition{ + Column: "job_id", + Operator: utils.OpEQ, + Value: utils.Value{Type: utils.TypeInt, Value: int64(2)}, + }, + }, + }, + } + + got, _ := ExecuteQuery(rows, &utils.QueryOptions{CompConditions: comp}, nil) + if len(got) != 1 || got[0].Data["job_id"] != int64(2) { + t.Fatalf("comp job_id eq 2: got %d rows %v, want only job 2", len(got), got) + } +} + +// Array-job id queries carry array_idx sub-ranges as OR conditions under the +// job_id main condition (e.g. "100[1-3,5-7]"). A row matches when the main +// condition holds AND any sub-condition holds. +func TestExecuteQuery_CompConditions_ArrayIdxSubRanges(t *testing.T) { + rows := []utils.OrderMap{} + for _, idx := range []int{1, 2, 3, 4, 5, 6, 7} { + rows = append(rows, makeJobRowIdx(100, idx, 0x01)) // PEND + } + comp := utils.CompCondition{ + "job_id": []utils.ConditionArray{ + { + MainCondition: utils.Condition{ + Column: "job_id", + Operator: utils.OpEQ, + Value: utils.Value{Type: utils.TypeInt, Value: int64(100)}, + }, + SubORConditions: []utils.Condition{ + {Column: "array_idx", Operator: utils.OpBETWEEN, Value: utils.Value{Type: utils.TypeArray, Value: []utils.Value{{Value: int64(1)}, {Value: int64(3)}}}}, + {Column: "array_idx", Operator: utils.OpIN, Value: utils.Value{Type: utils.TypeArray, Value: []utils.Value{{Value: int64(5)}, {Value: int64(7)}}}}, + }, + }, + }, + } + + got, _ := ExecuteQuery(rows, &utils.QueryOptions{CompConditions: comp}, nil) + if len(got) != 5 { + t.Fatalf("array-idx sub-ranges: got %d rows, want 5 (idx 1,2,3,5,7)", len(got)) + } + wantIdx := map[any]bool{1: true, 2: true, 3: true, 5: true, 7: true} + for _, row := range got { + if !wantIdx[row.Data["array_idx"]] { + t.Errorf("unexpected array_idx %v selected", row.Data["array_idx"]) + } + } +} + +// Multiple FilterConditionArrays under one field are OR-ed (e.g. two plain job +// ids in one request): any array matching selects the row. +func TestExecuteQuery_CompConditions_ORAcrossArrays(t *testing.T) { + rows := []utils.OrderMap{ + makeJobRow(1, 0x08), + makeJobRow(2, 0x04), + makeJobRow(3, 0x02), + } + comp := utils.CompCondition{ + "job_id": []utils.ConditionArray{ + { + MainCondition: utils.Condition{Column: "job_id", Operator: utils.OpEQ, Value: utils.Value{Type: utils.TypeInt, Value: int64(1)}}, + }, + { + MainCondition: utils.Condition{Column: "job_id", Operator: utils.OpEQ, Value: utils.Value{Type: utils.TypeInt, Value: int64(3)}}, + }, + }, + } + + got, _ := ExecuteQuery(rows, &utils.QueryOptions{CompConditions: comp}, nil) + if len(got) != 2 { + t.Fatalf("OR across arrays: got %d rows, want 2 (jobs 1 and 3)", len(got)) + } +}