diff --git a/packages/mosaic/core/src/SelectionClause.ts b/packages/mosaic/core/src/SelectionClause.ts index 1c3dafa1f..754c95929 100644 --- a/packages/mosaic/core/src/SelectionClause.ts +++ b/packages/mosaic/core/src/SelectionClause.ts @@ -431,7 +431,12 @@ export function clauseList( ): SelectionClause { field = asNode(field); const listFn = listMatch === 'all' ? listHasAll : listHasAny; - const predicate = value !== undefined ? listFn(field, literal(value)) : null; + // arrays pass through directly so they are quoted as a list, not stringified + const predicate = value === undefined || (Array.isArray(value) && value.length === 0) + ? null + : Array.isArray(value) + ? listFn(field, value as ExprValue[]) + : listFn(field, literal(value)); return { source, clients, fields: [field], value, predicate }; } diff --git a/packages/mosaic/core/test/selection-clause.test.ts b/packages/mosaic/core/test/selection-clause.test.ts new file mode 100644 index 000000000..7d6a528b3 --- /dev/null +++ b/packages/mosaic/core/test/selection-clause.test.ts @@ -0,0 +1,48 @@ +import { describe, it, expect } from 'vitest'; +import { clauseList } from '../src/index.js'; + +describe('clauseList', () => { + const source = {}; + + // Regression tests for https://github.com/uwdata/mosaic/issues/977. + + it('quotes list values that contain spaces', () => { + // previously unquoted, which triggered a DuckDB Parser Error at the space + const clause = clauseList('tags', ['Drafting Lines'], { source }); + expect(String(clause.predicate)).toBe( + `list_has_any("tags", ['Drafting Lines'])` + ); + }); + + it('quotes single-word list values as literals, not identifiers', () => { + // previously produced a bare word, which DuckDB read as a Binder Error + const clause = clauseList('tags', ['Markers'], { source }); + expect(String(clause.predicate)).toBe(`list_has_any("tags", ['Markers'])`); + }); + + it('supports multiple values', () => { + const clause = clauseList('tags', ['Roads', 'Lakes'], { source }); + expect(String(clause.predicate)).toBe( + `list_has_any("tags", ['Roads', 'Lakes'])` + ); + }); + + it('supports listMatch "all"', () => { + const clause = clauseList('tags', ['Roads', 'Lakes'], { + source, + listMatch: 'all' + }); + expect(String(clause.predicate)).toBe( + `list_has_all("tags", ['Roads', 'Lakes'])` + ); + }); + + it('produces a null predicate for undefined values', () => { + expect(clauseList('tags', undefined, { source }).predicate).toBe(null); + }); + + it('produces a null predicate for an empty array (clears selection)', () => { + // an empty list must clear the selection, not filter out every row + expect(clauseList('tags', [], { source }).predicate).toBe(null); + }); +}); diff --git a/packages/vgplot/plot/src/interactors/Nearest.js b/packages/vgplot/plot/src/interactors/Nearest.js index 78cfbd74b..0880d3f2b 100644 --- a/packages/vgplot/plot/src/interactors/Nearest.js +++ b/packages/vgplot/plot/src/interactors/Nearest.js @@ -1,5 +1,5 @@ /** @import { ClauseSource } from '@uwdata/mosaic-core' */ -import { clausePoint, clausePoints, isSelection } from '@uwdata/mosaic-core'; +import { clauseList, clausePoint, clausePoints, isSelection } from '@uwdata/mosaic-core'; import { select, pointer, min } from 'd3'; import { getField } from './util/get-field.js'; @@ -13,12 +13,14 @@ export class Nearest { pointer, channels, fields, - maxRadius = 40 + maxRadius = 40, + listMatch }) { this.mark = mark; this.selection = selection; this.clients = new Set().add(mark); this.pointer = pointer; + this.listMatch = listMatch; this.channels = channels || ( pointer === 'x' ? ['x'] : pointer === 'y' ? ['y'] : ['x', 'y'] ); @@ -28,13 +30,19 @@ export class Nearest { } clause(value) { - const { clients, fields } = this; + const { clients, fields, mark } = this; const opt = { source: /** @type {ClauseSource} */(this), clients }; - // if only one field, use a simpler clause that passes the value - // this allows a single field selection value to act like a param - return fields.length > 1 - ? clausePoints(fields, value ? [value] : value, opt) - : clausePoint(fields[0], value?.[0], opt); + if (fields.length === 1) { + // unnested field: use a list-membership predicate instead of point equality + if (mark.isUnnested(fields[0])) { + const list = value != null ? [value[0]] : undefined; + return clauseList(fields[0], list, { ...opt, listMatch: this.listMatch }); + } + // if only one field, use a simpler clause that passes the value + // this allows a single field selection value to act like a param + return clausePoint(fields[0], value?.[0], opt); + } + return clausePoints(fields, value ? [value] : value, opt); } init(svg) { diff --git a/packages/vgplot/plot/src/interactors/Toggle.js b/packages/vgplot/plot/src/interactors/Toggle.js index fa407fe3f..911654a78 100644 --- a/packages/vgplot/plot/src/interactors/Toggle.js +++ b/packages/vgplot/plot/src/interactors/Toggle.js @@ -1,5 +1,5 @@ /** @import { ClauseSource } from '@uwdata/mosaic-core' */ -import { clausePoints } from '@uwdata/mosaic-core'; +import { clauseList, clausePoints } from '@uwdata/mosaic-core'; import { getDatum } from './util/get-datum.js'; import { neq, neqSome } from './util/neq.js'; @@ -15,12 +15,14 @@ export class Toggle { constructor(mark, { selection, channels, - peers = true + peers = true, + listMatch }) { this.mark = mark; this.value = null; this.selection = selection; this.peers = peers; + this.listMatch = listMatch; const fields = this.fields = []; const as = this.as = []; channels.forEach(c => { @@ -42,10 +44,16 @@ export class Toggle { clause(value) { const { fields, mark } = this; - return clausePoints(fields, value, { - source: /** @type {ClauseSource} */(this), - clients: this.peers ? mark.plot.markSet : new Set().add(mark) - }); + const clients = this.peers ? mark.plot.markSet : new Set().add(mark); + const opt = { source: /** @type {ClauseSource} */(this), clients }; + + // unnested fields use a list-membership predicate instead of point equality + if (fields.length === 1 && mark.isUnnested(fields[0])) { + const list = value?.length ? value.map(v => v[0]) : undefined; + return clauseList(fields[0], list, { ...opt, listMatch: this.listMatch }); + } + + return clausePoints(fields, value, opt); } init(svg, selector, accessor) { diff --git a/packages/vgplot/plot/src/marks/Mark.js b/packages/vgplot/plot/src/marks/Mark.js index 75809b6b0..25212f58a 100644 --- a/packages/vgplot/plot/src/marks/Mark.js +++ b/packages/vgplot/plot/src/marks/Mark.js @@ -1,6 +1,6 @@ /** @import { SelectQuery } from '@uwdata/mosaic-sql' */ import { isParam, MosaicClient, queryFieldInfo, toDataColumns } from '@uwdata/mosaic-core'; -import { Query, collectParams, column, isAggregateExpression, isColumnParam, isColumnRef, isNode, isParamLike } from '@uwdata/mosaic-sql'; +import { Query, collectParams, column, isAggregateExpression, isColumnParam, isColumnRef, isNode, isParamLike, unnest } from '@uwdata/mosaic-sql'; import { isColor } from './util/is-color.js'; import { isConstantOption } from './util/is-constant-option.js'; import { isSymbol } from './util/is-symbol.js'; @@ -9,6 +9,12 @@ import { Transform } from '../symbols.js'; const isColorChannel = channel => channel === 'stroke' || channel === 'fill'; const isOpacityChannel = channel => /opacity$/i.test(channel); const isSymbolChannel = channel => channel === 'symbol'; + +// column name for a plain column reference or name string, else null +const unnestableColumn = field => + isColumnRef(field) ? field.column + : typeof field === 'string' ? field + : null; const isFieldObject = (channel, field) => { return channel !== 'sort' && channel !== 'tip' && field != null && !Array.isArray(field); @@ -104,6 +110,25 @@ export class Mark extends MosaicClient { return table ? (isParam(table) ? table.value : table) : null; } + /** + * The source field names to unnest, as declared by the `unnest` option. + * @returns {string[]} + */ + get unnestFields() { + const { unnest } = this.source?.options ?? {}; + return unnest == null ? [] : [unnest].flat(); + } + + /** + * Check if the given field is unnested by this mark's source. + * @param {*} field A field reference or column name. + * @returns {boolean} + */ + isUnnested(field) { + const name = unnestableColumn(field); + return name != null && this.unnestFields.includes(name); + } + hasOwnData() { return this.source == null || isDataArray(this.source); } @@ -160,7 +185,17 @@ export class Mark extends MosaicClient { */ query(filter = []) { if (this.hasOwnData()) return null; - return markQuery(this.channels, this.sourceTable()).where(filter); + const { unnestFields } = this; + // wrap unnested column fields in UNNEST() so arrays expand into rows + const channels = unnestFields.length === 0 + ? this.channels + : this.channels.map(c => { + const name = unnestableColumn(c.field); + return name != null && unnestFields.includes(name) + ? { ...c, field: unnest(c.field) } + : c; + }); + return markQuery(channels, this.sourceTable()).where(filter); } queryPending() {