You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by ha...@apache.org on 2026/08/27 05:55:01 UTC
(skywalking-banyandb) branch main updated: Fix stale query clauses after Canopy resource changes (#1312)
This is an automated email from the ASF dual-hosted git repository.
hanahmily pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/skywalking-banyandb.git
The following commit(s) were added to refs/heads/main by this push:
new 8b4e153a1 Fix stale query clauses after Canopy resource changes (#1312)
8b4e153a1 is described below
commit 8b4e153a159a76389077c02d25dd8575d6a3cf20
Author: Gao Hongtao <ha...@gmail.com>
AuthorDate: Thu Aug 27 13:54:55 2026 +0800
Fix stale query clauses after Canopy resource changes (#1312)
---
canopy/web/src/query/QueryBuilder.test.tsx | 135 ++++++++++++++++++++++++++++-
canopy/web/src/query/QueryBuilder.tsx | 23 ++++-
canopy/web/src/query/QueryConsole.test.tsx | 110 +++++++++++++++++++++++
canopy/web/src/query/QueryConsole.tsx | 28 +++---
canopy/web/src/query/bydbql.test.ts | 23 ++++-
canopy/web/src/query/bydbql.ts | 19 +++-
6 files changed, 319 insertions(+), 19 deletions(-)
diff --git a/canopy/web/src/query/QueryBuilder.test.tsx b/canopy/web/src/query/QueryBuilder.test.tsx
index 41943264b..c3c1bd248 100644
--- a/canopy/web/src/query/QueryBuilder.test.tsx
+++ b/canopy/web/src/query/QueryBuilder.test.tsx
@@ -22,7 +22,8 @@
// where.test.ts / bydbql.test.ts).
import { describe, it, expect, vi } from 'vitest';
-import { render, screen, fireEvent } from '@testing-library/react';
+import { render, screen, fireEvent, waitFor } from '@testing-library/react';
+import { useState } from 'react';
import { MemoryRouter } from 'react-router';
import { QueryBuilder, type QueryBuilderProps } from './QueryBuilder.js';
import type { QBBuilderState, QBWhereLeafWithConn } from './bydbql.js';
@@ -75,6 +76,94 @@ function renderBuilder(state: QBBuilderState, onChange = vi.fn()) {
return { onChange };
}
+function staleMeasureState(): QBBuilderState {
+ return {
+ ...stateWithLeaf({ tag: 'old_tag', op: 'BINARY_OP_EQ', value: 'old-value' }),
+ catalog: 'measures',
+ resource: 'old_measure',
+ select: [{ field: 'old_field', fn: 'MEAN' }],
+ projection: ['old_tag'],
+ groupBy: ['old_tag'],
+ fromResource: 'g1/old_measure',
+ };
+}
+
+/** Mirrors #14043: SELECT entity_id carried from a prior resource into Trace zipkin_span. */
+function staleTraceState(): QBBuilderState {
+ return {
+ ...stateWithLeaf({ tag: 'entity_id', op: 'BINARY_OP_EQ', value: 'svc-1' }),
+ catalog: 'traces',
+ group: 'sw_zipkin',
+ resource: 'old_span',
+ select: [],
+ projection: ['entity_id'],
+ groupBy: [],
+ orderField: 'time',
+ fromResource: 'sw_zipkin/old_span',
+ };
+}
+
+function StatefulQueryBuilder({
+ autoPickOnGroupChange,
+ initialState = staleMeasureState(),
+}: {
+ readonly autoPickOnGroupChange?: boolean;
+ readonly initialState?: QBBuilderState;
+}) {
+ const [state, setState] = useState(initialState);
+ const isTrace = state.catalog === 'traces';
+ const tags = isTrace
+ ? (state.resource === 'zipkin_span' ? ['trace_id', 'span_id', 'name'] : ['entity_id', 'trace_id'])
+ : (state.resource === 'new_measure' ? ['new_tag'] : ['old_tag']);
+ const fields = isTrace
+ ? []
+ : (state.resource === 'new_measure' ? ['new_field'] : ['old_field']);
+ const resourceNames = isTrace
+ ? ['old_span', 'zipkin_span']
+ : (state.group === 'g2' ? ['new_measure'] : ['old_measure', 'new_measure']);
+ const groupNames = isTrace ? ['sw_zipkin'] : ['g1', 'g2'];
+ const onChange = (patch: Partial<QBBuilderState>) => {
+ setState((current) => {
+ const next = { ...current, ...patch };
+ return autoPickOnGroupChange && next.group === 'g2' && !next.resource
+ ? { ...next, resource: 'new_measure' }
+ : next;
+ });
+ };
+
+ return (
+ <MemoryRouter>
+ <QueryBuilder
+ state={state}
+ onChange={onChange}
+ tags={tags}
+ fields={fields}
+ groupNames={groupNames}
+ resourceNames={resourceNames}
+ topnAggNames={[]}
+ groups={[]}
+ groupResources={new Map()}
+ groupTopnAggs={new Map()}
+ onPickResource={vi.fn()}
+ isRunning={false}
+ onEjectToCode={vi.fn()}
+ onRun={vi.fn()}
+ hasRun={false}
+ compact={false}
+ setCompact={vi.fn()}
+ openSection={null}
+ setOpenSection={vi.fn()}
+ />
+ </MemoryRouter>
+ );
+}
+
+function queryPreview(container: HTMLElement): HTMLElement {
+ const preview = container.querySelector('.qb-gen-line');
+ if (!preview) throw new Error('Generated query preview was not rendered');
+ return preview as HTMLElement;
+}
+
describe('QueryBuilder WHERE — MATCH analyzer + operator fields', () => {
it('shows the Analyzer + Match operator controls only for the MATCH operator', () => {
renderBuilder(stateWithLeaf({ tag: 'name', op: 'BINARY_OP_MATCH', value: 'nodea' }));
@@ -105,6 +194,50 @@ describe('QueryBuilder WHERE — MATCH analyzer + operator fields', () => {
});
});
+describe('QueryBuilder resource changes', () => {
+ it('clears dependent clauses when the resource dropdown selects a new schema', async () => {
+ const { container } = render(<StatefulQueryBuilder />);
+
+ fireEvent.change(screen.getByLabelText('Resource'), { target: { value: 'new_measure' } });
+
+ await waitFor(() => expect(queryPreview(container)).toHaveTextContent('FROM MEASURE new_measure'));
+ const query = queryPreview(container).getAttribute('title') ?? '';
+ expect(query).toContain('SELECT new_tag');
+ expect(query).not.toContain('old_tag');
+ expect(query).not.toContain('old_field');
+ expect(query).not.toContain('WHERE');
+ expect(query).not.toContain('GROUP BY');
+ });
+
+ it('does not carry clauses into an auto-selected resource after changing groups', async () => {
+ const { container } = render(<StatefulQueryBuilder autoPickOnGroupChange />);
+
+ fireEvent.change(screen.getByLabelText('Group'), { target: { value: 'g2' } });
+
+ await waitFor(() => expect(screen.getByLabelText('Resource')).toHaveValue('new_measure'));
+ const query = queryPreview(container).getAttribute('title') ?? '';
+ expect(query).toContain('SELECT new_tag');
+ expect(query).not.toContain('old_tag');
+ expect(query).not.toContain('old_field');
+ expect(query).not.toContain('WHERE');
+ expect(query).not.toContain('GROUP BY');
+ });
+
+ it('drops a stale SELECT tag when switching Trace resources (#14043)', async () => {
+ const { container } = render(<StatefulQueryBuilder initialState={staleTraceState()} />);
+
+ fireEvent.change(screen.getByLabelText('Resource'), { target: { value: 'zipkin_span' } });
+
+ await waitFor(() => expect(queryPreview(container)).toHaveTextContent('FROM TRACE zipkin_span'));
+ const query = queryPreview(container).getAttribute('title') ?? '';
+ // Empty projection on Trace expands to the new resource's tag list (not SELECT *).
+ expect(query).toContain('SELECT trace_id, span_id, name');
+ expect(query).not.toContain('entity_id');
+ expect(query).toContain('WHERE trace_id');
+ expect(query).not.toContain('ORDER BY');
+ });
+});
+
// ORDER BY resolves to an index-rule NAME server-side, so the dropdown is
// confined to the rules bound to the current resource once they load.
describe('QueryBuilder ORDER BY — index-rule-confined options', () => {
diff --git a/canopy/web/src/query/QueryBuilder.tsx b/canopy/web/src/query/QueryBuilder.tsx
index 9962786e9..6d3aa8d49 100644
--- a/canopy/web/src/query/QueryBuilder.tsx
+++ b/canopy/web/src/query/QueryBuilder.tsx
@@ -29,7 +29,7 @@
import React from 'react';
import {
QB_CATALOGS, QB_CAT, QB_UI_KW, QB_OPS, QB_OP, QB_AGGS, QB_TOPN_OPS, QB_TOPN_AGGS, QB_TIMES, QB_COMBINATORS,
- qbDataCatalog, qbIsGroup, qbNewCond, qbNewGroup, qbEmptyWhere, qbPruneWhere,
+ qbDataCatalog, qbIsGroup, qbNewCond, qbNewGroup, qbEmptyWhere, qbDefaultTraceWhere, qbPruneWhere,
qbSearchIndex, qbSearchResults, qbConnSummary,
buildBydbQL,
type QBWhereNode, type QBWhereLeafWithConn,
@@ -525,8 +525,23 @@ export function QueryBuilder({
fromResource: null,
});
};
- const setGroup = (group: string) => onChange({ group, resource: '' });
- const setResource = (resource: string) => onChange({ resource });
+ // Match onPickResource / fuzzy From pick: Trace resource changes re-seed an
+ // empty trace_id condition (and clear ORDER BY) so the analyzer stays valid.
+ const resetResourceClauses = (): Partial<QBBuilderState> => {
+ const isTrace = state.catalog === 'traces';
+ return {
+ select: [],
+ projection: [],
+ where: isTrace ? qbDefaultTraceWhere() : qbEmptyWhere(),
+ groupBy: [],
+ ...(isTrace ? { orderField: '' as const } : {}),
+ fromAgg: null,
+ fromResource: null,
+ offset: 0,
+ };
+ };
+ const setGroup = (group: string) => onChange({ group, resource: '', ...resetResourceClauses() });
+ const setResource = (resource: string) => onChange({ resource, ...resetResourceClauses() });
const toggleProjection = (tag: string) => {
const cur = state.projection;
const next = cur.includes(tag) ? cur.filter((t) => t !== tag) : [...cur, tag];
@@ -1059,4 +1074,4 @@ export function QueryBuilder({
}
// Re-export helper so QueryConsole can build a default state.
-export { qbDataCatalog, qbPruneWhere };
\ No newline at end of file
+export { qbDataCatalog, qbPruneWhere };
diff --git a/canopy/web/src/query/QueryConsole.test.tsx b/canopy/web/src/query/QueryConsole.test.tsx
new file mode 100644
index 000000000..e124dce91
--- /dev/null
+++ b/canopy/web/src/query/QueryConsole.test.tsx
@@ -0,0 +1,110 @@
+/*
+ * Licensed to Apache Software Foundation (ASF) under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Apache Software Foundation (ASF) licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import React from 'react';
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { fireEvent, render, screen, within } from '@testing-library/react';
+import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
+import { MemoryRouter } from 'react-router';
+
+import { apiDataSource } from '../data/api.js';
+import { QueryConsole } from './QueryConsole.js';
+import type { QBBuilderState } from './bydbql.js';
+
+vi.mock('../data/api.js', () => ({
+ apiDataSource: {
+ listGroups: vi.fn(),
+ listResourcesInGroup: vi.fn(),
+ listTopNAggregations: vi.fn(),
+ listIndexRuleBindings: vi.fn(),
+ listIndexRules: vi.fn(),
+ runQuery: vi.fn(),
+ },
+}));
+
+const GROUPS = [{ name: 'g1', catalog: 'CATALOG_MEASURE', resourceOpts: { shardNum: 1 } }];
+
+const STALE_STATE: QBBuilderState = {
+ catalog: 'measures',
+ group: 'g1',
+ resource: 'old_measure',
+ select: [{ field: 'old_field', fn: 'MEAN' }],
+ projection: ['old_tag'],
+ where: { combinator: 'AND', children: [{ tag: 'old_tag', op: 'BINARY_OP_EQ', value: 'old-value' }] },
+ groupBy: ['old_tag'],
+ time: { mode: 'relative', rel: '-30m', from: '', to: '' },
+ orderField: 'time',
+ orderDir: 'DESC',
+ limit: 100,
+ offset: 0,
+ trace: false,
+ topN: 10,
+ aggFn: '',
+ fromAgg: null,
+ fromResource: 'g1/old_measure',
+};
+
+function makeWrapper() {
+ const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false }, mutations: { retry: false } } });
+ function Wrapper({ children }: { readonly children: React.ReactNode }) {
+ return <MemoryRouter><QueryClientProvider client={queryClient}>{children}</QueryClientProvider></MemoryRouter>;
+ }
+ Wrapper.displayName = 'QueryConsoleTestWrapper';
+ return Wrapper;
+}
+
+beforeEach(() => {
+ localStorage.clear();
+ vi.clearAllMocks();
+ localStorage.setItem('canopy.query.v3', JSON.stringify({ builder: STALE_STATE }));
+ vi.mocked(apiDataSource.listGroups).mockResolvedValue({ groups: GROUPS } as never);
+ vi.mocked(apiDataSource.listResourcesInGroup).mockResolvedValue([
+ {
+ metadata: { name: 'old_measure', group: 'g1' },
+ tagFamilies: [{ tags: [{ name: 'old_tag', type: 'TAG_TYPE_STRING' }] }],
+ fields: [{ name: 'old_field' }],
+ },
+ {
+ metadata: { name: 'new_measure', group: 'g1' },
+ tagFamilies: [{ tags: [{ name: 'new_tag', type: 'TAG_TYPE_STRING' }] }],
+ fields: [{ name: 'new_field' }],
+ },
+ ] as never);
+ vi.mocked(apiDataSource.listTopNAggregations).mockResolvedValue([] as never);
+ vi.mocked(apiDataSource.listIndexRuleBindings).mockResolvedValue([] as never);
+ vi.mocked(apiDataSource.listIndexRules).mockResolvedValue([] as never);
+});
+
+describe('QueryConsole fuzzy resource pick', () => {
+ it('keeps the fuzzy-pick reset baseline for dependent clauses', async () => {
+ render(<QueryConsole />, { wrapper: makeWrapper() });
+
+ fireEvent.change(screen.getByLabelText('Search resources by name'), { target: { value: 'new_measure' } });
+ const fuzzyResult = await within(screen.getByRole('listbox')).findByRole('option');
+ fireEvent.click(fuzzyResult);
+
+ const query = await screen.findByTitle(/FROM MEASURE new_measure/);
+ const generated = query.getAttribute('title') ?? '';
+ expect(generated).toContain('SELECT new_tag');
+ expect(generated).not.toContain('old_tag');
+ expect(generated).not.toContain('old_field');
+ expect(generated).not.toContain('WHERE');
+ expect(generated).not.toContain('GROUP BY');
+ });
+});
diff --git a/canopy/web/src/query/QueryConsole.tsx b/canopy/web/src/query/QueryConsole.tsx
index 80d8ef50b..80922fecc 100644
--- a/canopy/web/src/query/QueryConsole.tsx
+++ b/canopy/web/src/query/QueryConsole.tsx
@@ -33,7 +33,7 @@ import { TopNResultView } from './results/TopNResultView.js';
import { ResultEmpty } from './results/ResultEmpty.js';
import { ResultError } from './results/ResultError.js';
import {
- buildBydbQL, qbDataCatalog, qbEmptyWhere, qbPruneWhere, qbNewCond,
+ buildBydbQL, qbDataCatalog, qbEmptyWhere, qbDefaultTraceWhere, qbPruneProjection, qbPruneSelect, qbPruneWhere, qbNewCond,
qbProtoCatalog, qbHasAnyFilter, qbHasTraceIdCondition,
type QBBuilderState, type QB_CATALOG_VALUE, type GroupTopnAggMap,
} from './bydbql.js';
@@ -201,9 +201,7 @@ export function QueryConsole() {
// ORDER BY clause; seed a trace_id condition for trace resources as a
// convenient starting point. The user can change the tag freely; if no
// trace_id condition remains, an effect defaults orderField to 'time'.
- const defaultWhere = catalog === 'traces'
- ? { combinator: 'AND' as const, children: [{ tag: 'trace_id', op: 'BINARY_OP_EQ' as const, value: '' }] }
- : qbEmptyWhere();
+ const defaultWhere = catalog === 'traces' ? qbDefaultTraceWhere() : qbEmptyWhere();
patch({
catalog,
group,
@@ -467,13 +465,21 @@ export function QueryConsole() {
return { begin: new Date(beginMs).toISOString(), end: new Date(now).toISOString() };
};
- // Re-prune WHERE tags when resource changes. Guard on a non-empty tag list:
- // right after a pick the new schema hasn't loaded yet (tags === []), and
- // pruning against an empty list would wipe the freshly seeded trace_id
- // condition for traces.
+ // Re-prune WHERE / SELECT / GROUP BY / measure field rows when the resource
+ // schema loads. Guard on a non-empty tag list: right after a pick the new
+ // schema hasn't loaded yet (tags === []), and pruning against an empty list
+ // would wipe the freshly seeded trace_id condition for traces. Measure
+ // select[] is pruned only once fields are known for the same reason.
useEffect(() => {
- if (tags.length > 0) setState((s) => ({ ...s, where: qbPruneWhere(s.where, tags) }));
- }, [state.resource, tags]);
+ if (tags.length === 0) return;
+ setState((s) => ({
+ ...s,
+ projection: qbPruneProjection(s.projection, tags),
+ where: qbPruneWhere(s.where, tags),
+ groupBy: qbPruneProjection(s.groupBy, tags),
+ select: s.catalog === 'measures' && fields.length > 0 ? qbPruneSelect(s.select, fields) : s.select,
+ }));
+ }, [state.resource, tags, fields]);
// Auto-pick a group once groups load so the rail renders real content
// on initial load instead of "— none —". Prefers the canonical handoff
@@ -949,4 +955,4 @@ function ResultViewRouter({ state, response, showTrace, setShowTrace, hasMore, o
}
// Re-export to keep tree-shaking honest
-export { qbNewCond };
\ No newline at end of file
+export { qbNewCond };
diff --git a/canopy/web/src/query/bydbql.test.ts b/canopy/web/src/query/bydbql.test.ts
index 5283521fc..033497860 100644
--- a/canopy/web/src/query/bydbql.test.ts
+++ b/canopy/web/src/query/bydbql.test.ts
@@ -22,7 +22,7 @@ import {
QB_CAT, QB_OP, QB_OPS, QB_UI_KW, QB_CATALOGS, qbDataCatalog,
qbSearchIndex, qbSearchResults,
buildBydbQL, qbNodeSQL, qbQuote, qbTimeSQL,
- qbNewCond, qbEmptyWhere, qbPruneWhere, qbConnSummary,
+ qbNewCond, qbEmptyWhere, qbPruneWhere, qbPruneProjection, qbPruneSelect, qbDefaultTraceWhere, qbConnSummary,
type QBBuilderState, type QBWhereGroupWithConn, type QBWhereLeafWithConn,
type GroupResourcesMap, type GroupTopnAggMap,
} from './bydbql.js';
@@ -472,6 +472,25 @@ describe('WHERE tree helpers', () => {
const p = qbPruneWhere(where, ['keep']);
expect(p.children.map((c) => (c as QBWhereLeafWithConn).tag)).toEqual(['keep']);
});
+ it('prunes WHERE and projected tags when a resource has no tags', () => {
+ const where: QBWhereGroupWithConn = {
+ combinator: 'AND',
+ children: [{ tag: 'old_tag', op: 'BINARY_OP_EQ', value: 'old-value' }],
+ };
+
+ expect(qbPruneWhere(where, []).children).toEqual([]);
+ expect(qbPruneProjection(['old_tag'], [])).toEqual([]);
+ });
+ it('qbPruneSelect drops measure field rows absent from the new measure', () => {
+ expect(qbPruneSelect(
+ [{ field: 'total', fn: 'MEAN' }, { field: 'gone', fn: '' }],
+ ['total', 'value'],
+ )).toEqual([{ field: 'total', fn: 'MEAN' }]);
+ });
+ it('qbDefaultTraceWhere seeds an empty trace_id equality leaf', () => {
+ const where = qbDefaultTraceWhere();
+ expect(where.children).toEqual([{ tag: 'trace_id', op: 'BINARY_OP_EQ', value: '' }]);
+ });
it('qbConnSummary reports AND / OR / mixed', () => {
expect(qbConnSummary({ combinator: 'AND', children: [] })).toBe('AND');
expect(qbConnSummary({ combinator: 'AND', children: [
@@ -485,4 +504,4 @@ describe('WHERE tree helpers', () => {
{ tag: 'c', op: 'BINARY_OP_EQ', value: '3' },
] })).toBe('mixed');
});
-});
\ No newline at end of file
+});
diff --git a/canopy/web/src/query/bydbql.ts b/canopy/web/src/query/bydbql.ts
index e23a4fb81..d72d508ed 100644
--- a/canopy/web/src/query/bydbql.ts
+++ b/canopy/web/src/query/bydbql.ts
@@ -623,6 +623,23 @@ export const qbPruneWhere = (
return { ...group, children: pruned as readonly QBWhereLeafWithConn[] };
};
+/** Drop projected tags that do not exist on the chosen resource. */
+export const qbPruneProjection = (projection: readonly string[], tags: readonly string[]): readonly string[] =>
+ projection.filter((tag) => tags.includes(tag));
+
+/** Drop measure SELECT field rows whose field no longer exists on the chosen measure. */
+export const qbPruneSelect = (
+ rows: readonly { readonly field: string; readonly fn: string }[],
+ fields: readonly string[],
+): readonly { readonly field: string; readonly fn: string }[] =>
+ rows.filter((row) => !row.field || fields.includes(row.field));
+
+/** Empty trace_id equality seed used when switching to a Trace resource. */
+export const qbDefaultTraceWhere = (): QBWhereGroupWithConn => ({
+ combinator: 'AND',
+ children: [{ tag: 'trace_id', op: 'BINARY_OP_EQ', value: '' }],
+});
+
// ── Fuzzy resource search (From-row search box) ─────────────────────────────
/** A resource row in the search index, before scoring. */
@@ -753,4 +770,4 @@ export const qbSearchResults = (
}
scored.sort((a, b) => b.score - a.score);
return scored.slice(0, limit || 8);
-};
\ No newline at end of file
+};