Scc 5168 by danamansana · Pull Request #643 · NYPL/discovery-api

danamansana · 2026-02-11T19:16:18Z

Initial work for NYQL. Still a work in progress, but so far we have:

A mapping defining the fields used for different types of search scopes. Where applicable, I've based this on the existing search scopes, or filters
A grammar for parsing CQL queries
Methods to transform CQL queries into ElasticSearch
EDIT: Merged in some other tickets which add:
date-related queries
exact match (==) queries
compatibility with filters
and related tests
some error handling/error messages
a compact representation of the parsed query, returned in the API response

(this covers SCC-5168/69/70)

Still to come:

The queries currently being generated are pretty complicated, if this affects performance we may want to come back and try to compress them, but I am not sure yet if this is going to be an issue
handling ES responses, particularly for inner_hits (this may be unnecessary)
need some more attention to issues around escaping
syntactic sugar in the grammar to make it more permissive
the tests are pretty literal currently, ideally I can find a way to make them a little more flexible

Scc 5203

lib/elasticsearch/cql_grammar.js

nonword · 2026-02-27T19:08:13Z

lib/elasticsearch/cql_grammar.js

+}
+
+function displayParsed (string) {
+  const parsed = rightCqlParser.getAST(reverseString(string))


Is there a way to better ensure that the chain of calls you're making to parse the string in this case matches the chain of calls in parseWithRightCql?

👍 good point, actually parseWithRightCql returns an AST so we can just call it directly

lib/resources.js

nonword · 2026-02-27T19:08:59Z

lib/elasticsearch/cql_query_builder.js

+  const queryJson = ElasticQueryBuilder.forApiRequest(request).query.toJson()
+  if (queryJson.bool && queryJson.bool.filter) {
+    return { filter: queryJson.bool.filter }
+  }


I assume this is a temporary measure until we can migrate the filter stuff into something else?

That makes sense, we could factor out the filter stuff into a separate module shared by the cql_query_builder and ElasticQueryBuilder.

lib/elasticsearch/cql_query_builder.js

nonword · 2026-03-09T16:03:15Z

lib/elasticsearch/cql_query_builder.js

+    })
+    .map(([fieldType, fieldNames]) => fieldNames)
+    .flat()
+}


Having trouble following what the types of these vars are and what they represent. If this part of the code is feeling final, let's add some documentation.

yeah this is complicated, I've added a comment, let me know if it needs work

charmingduchess

Nothing blocking merge into QA - mostly formatting/importing suggestions and clarifying questions.
Great work!

charmingduchess · 2026-03-09T17:29:56Z

lib/elasticsearch/cql/index-mapping.js

Do we want to ensure that the fields are the same for equivalent search_scopes? Or do we have a reason to keep these distinct?

I think this has to do something more complicated than the search_scopes mapping, and also doesn't use the boosting. Might be a way to combine them but not sure

charmingduchess · 2026-03-09T17:31:37Z

lib/elasticsearch/config.js

    // We do custom field matching for this search-scope
-  }
+  },
+  cql: {}


maybe worth a comment directing the reader to cql/index_mapping.js

charmingduchess · 2026-03-09T17:32:13Z

lib/elasticsearch/cql/mapping-from-es.json

what is this for?

good catch, I've deleted that

charmingduchess · 2026-03-09T17:35:54Z

lib/elasticsearch/cql_query_builder.js

+
+function buildBoolean (operator, queries) {
+  if (['NOT', 'AND NOT'].includes(operator)) return buildNegation(queries)
+  const esOperator = operator === 'AND' ? 'must' : 'should'


Is it correct to assume that there is validation upstream that would ensure these would only be AND or OR? Also, would that validation allow for OR NOT?

Yep, that validation is done by the parser, which only allows operators that match the grammar

charmingduchess · 2026-03-09T17:54:54Z

lib/elasticsearch/cql_grammar.js

+  return children
+}
+
+function displayParsed (string) {


Can you write a comment describing the left/right/reverse switcheration that (I think) is happening here?

charmingduchess · 2026-03-09T17:57:55Z

lib/elasticsearch/cql_query_builder.js

+function buildNegation (queries) {
+  return {
+    bool: {
+      must: [buildEsQueryFromTree(queries[0])],


At what point do the queries get split into this array?

This is done by the buildEsQueryFromTree, which filters out the subqueries and hands them to buildBoolean in case of a boolean query

charmingduchess · 2026-03-09T18:05:23Z

lib/elasticsearch/cql_query_builder.js

+}
+
+const table = {
+  exact: { term: 'term', prefix: 'prefix', fields: 'X', exact_fields: 'term' },


What does 'X' represent here?

These are fields that we don't use in that particular case. E.g. in the exact case, don't use the basic fields, since these are text, instead use the matching exact_fields. I've added a comment

lib/elasticsearch/cql_query_builder.js

charmingduchess · 2026-03-09T18:11:52Z

test/fixtures/cql_fixtures.js

+                    multi_match: {
+                      query: 'Hamlet',
+                      fields: [
+                        'title',


Maybe import these fields from the index-mapping.json? Would be less brittle if/when we update scopes.

Yeah I want to revisit the whole testing approach eventually

charmingduchess · 2026-03-09T18:19:18Z

lib/elasticsearch/cql_query_builder.js

this file is quite long. are there any clear groupings that could be split across different files, or even just demarcated with comments?

perhaps split the boolean and atomic cases?

danamansana added 21 commits December 1, 2025 11:15

Fix query structure

579eec5

Add more permissive key structure

ae28b72

Remove console logs and commented code

8b6000f

Fix linter errors

e5b61dc

Exclude parentheses in query term

6ca7e33

Make keyphrase/non_ws_key lowercase

39c0920

Change callNumber to callnumber to enable callnumber searches

6714e24

Add finding text by key for atomic queries

cd4c24f

Merge branch 'main' into scc-5168

58a0d9f

Add initial bnf

8b3e4e1

Update packages

87c1e32

Add alternate grammars and comment for atomic

a55444d

Add reverseGrammar and related methods

05dfb3f

Apparently working left associating cql

f373383

Clean up grammar file

951a8e8

Use parseWithRightCql in query builder and tests

3a03d81

Remove console log and commented code'

1952828

Fix some param passing and start adding query tests

3a83857

Add tests for atomic queries and some small corrections

d2109f5

Add initial boolean tests

baf803c

Add tests for negation

8842a88

danamansana requested a review from nonword February 11, 2026 19:16

danamansana assigned yossariano Feb 11, 2026

danamansana requested a review from charmingduchess February 11, 2026 19:16

danamansana unassigned yossariano Feb 11, 2026

danamansana requested a review from yossariano February 11, 2026 19:16

danamansana added 4 commits February 11, 2026 14:30

Fix linting/tests/small errors

fd8ff37

Add date queries

271c692

Add filters to cql query builder

ffe585f

Fix some small errors

1473e3d

danamansana added 5 commits February 20, 2026 11:22

Add initial filter implementation for cql

ed4c14a

Fix tests

74f97a4

Add date and filter features to cql

d5cee7f

Add some more useful display of parsing and errors

e2e43d0

Fix linting

93c042e

danamansana marked this pull request as ready for review February 26, 2026 19:54

danamansana added 3 commits February 27, 2026 11:15

Merge pull request #650 from NYPL/scc-5203

9c98372

Scc 5203

Add new strategy for handling keyword vs text fields

0ebe3bd

Add exact match query

b2ebcab

nonword reviewed Mar 9, 2026

View reviewed changes

charmingduchess approved these changes Mar 9, 2026

View reviewed changes

danamansana added 8 commits March 13, 2026 15:10

Fixes in response to PR comments

36e6302

Add check for whether query has fields before adding

d5ec597

Remove console log

4f26b20

Fix tests

94d0d61

Add reversing strings in nested array

e0e8f6b

Remove double reversing in display

d65a3d3

Fix double nesting of should array for dates

330e170

Fix date test fixtures

b0f7585

danamansana merged commit e046813 into main Mar 20, 2026
7 checks passed

Conversation

danamansana commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

charmingduchess left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

danamansana commented Feb 11, 2026 •

edited

Loading