Add DSL query-type parquet REST test suite (initial)

[Siritejagaddameedi]

Description

Introduces a standalone REST integration module that validates parquet/composite query
behavior against known-correct answers generated on a vanilla (plain Lucene) index.

Each query type is a folder under datasets/<type>/ (mapping.json + bulk.json +
dsl/q1.json + dsl/expected/q1.json). DslQueryTypesIT provisions the dataset into a
parquet-backed index, runs the query, and validates the response against the committed
expected answer via DslResponseValidator (order-independent, numeric tolerance). The
suite runs against an already-running server over HTTP, so the test JVM loads no plugins.

Seed the suite with two query types plus single-valued-tags variants:

• term, bool — docs store tags as a multi-valued array, which parquet rejects at
  ingest ("Cannot accept multiple values for field [tags]").
• term_scalar, bool_scalar — identical mapping and query but tags is a single
  scalar value, which parquet accepts — isolating the multi-value array as the sole cause
  and confirming the query type itself works.

Also includes DslTermQueryIT, a focused term-query correctness test.

The module is designed to grow one folder per query type in follow-up PRs.

How to run (against a running server with the analytics/composite/parquet stack):

./gradlew :sandbox:qa:dsl-query-types:restTest -Dsandbox.enabled=true

Related Issues

N/A

Check List

• Functionality includes testing.
• API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Fragile settings splice injectParquetSettings performs a raw string replace on the literal "number_of_shards". If a future mapping omits number_of_shards, uses a different key ordering, or contains that substring anywhere else (e.g., in a field name or comment inside the JSON), the parquet settings will silently not be injected (or be injected in the wrong place), and the index will be created without parquet — causing tests to falsely pass or fail confusingly. Consider parsing the JSON and inserting settings structurally, or asserting that the replacement occurred. private static String injectParquetSettings(String mappingBody) { return mappingBody.replace( "\"number_of_shards\"", "\"index.pluggable.dataformat.enabled\": true, " + "\"index.pluggable.dataformat\": \"composite\", " + "\"index.composite.primary_data_format\": \"parquet\", " + "\"index.composite.secondary_data_formats\": \"lucene\", " + "\"number_of_shards\"" ); } Static state across test instances provisioned is a static boolean guarded by no synchronization and never reset. If the JVM is reused across test classes/runs or the index is deleted externally between tests, subsequent tests will skip provisioning and fail. Since DatasetProvisioner.provision already deletes/recreates the index, the guard mostly saves time but introduces test-order coupling. Prefer @BeforeClass or removing the flag. private static boolean provisioned = false; @Override protected boolean preserveClusterUponCompletion() { return true; } @Override protected boolean preserveIndicesUponCompletion() { return true; } @Before public void provisionOnce() throws IOException { if (provisioned == false) { DatasetProvisioner.provision(client(), PEOPLE); provisioned = true; } } Ambiguous canonical form canonical emits strings with surrounding quotes but does not escape embedded quotes/backslashes, and numbers/booleans/nulls are all rendered via String.valueOf. A string value "1" and numeric 1 produce distinguishable output only via the surrounding quotes, but a string containing , or } could collide with structural characters, potentially causing two different sources to sort/compare as equal for ordering purposes. Since element-wise valuesEqual is still called afterward this is unlikely to cause false positives, but sort stability across duplicates-with-different-values could pair wrong elements and produce misleading diffs. @SuppressWarnings("unchecked") private static String canonical(Object o) { if (o instanceof Map) { TreeMap<String, Object> sorted = new TreeMap<>((Map<String, Object>) o); StringBuilder sb = new StringBuilder("{"); boolean first = true; for (Map.Entry<String, Object> e : sorted.entrySet()) { if (!first) { sb.append(","); } first = false; sb.append('"').append(e.getKey()).append("\":").append(canonical(e.getValue())); } return sb.append("}").toString(); } if (o instanceof List) { List<String> parts = new ArrayList<>(); for (Object e : (List<Object>) o) { parts.add(canonical(e)); } parts.sort(Comparator.naturalOrder()); return "[" + String.join(",", parts) + "]"; } if (o instanceof String) { return '"' + (String) o + '"'; } return String.valueOf(o); }

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fail loudly if injection anchor is missing injectParquetSettings silently returns the original body if the literal "number_of_shards" token is missing from mapping.json, causing the index to be created without parquet settings and the test to falsely pass under Lucene. Verify replacement occurred and fail loudly otherwise. sandbox/qa/dsl-query-types/src/test/java/org/opensearch/dsl/types/DatasetProvisioner.java [100-109] private static String injectParquetSettings(String mappingBody) { + if (!mappingBody.contains("\"number_of_shards\"")) { + throw new IllegalStateException("mapping.json must contain \"number_of_shards\" as an anchor for parquet settings injection"); + } return mappingBody.replace( "\"number_of_shards\"", "\"index.pluggable.dataformat.enabled\": true, " + "\"index.pluggable.dataformat\": \"composite\", " + "\"index.composite.primary_data_format\": \"parquet\", " + "\"index.composite.secondary_data_formats\": \"lucene\", " + "\"number_of_shards\"" ); } Suggestion importance[1-10]: 7 __ Why: Valid concern: if the anchor token is missing, String.replace silently returns the original body, resulting in a Lucene-only index and a falsely-passing test. Failing fast with a clear message improves robustness.	Medium
	Handle already-open jar filesystem FileSystems.newFileSystem(uri, ...) throws FileSystemAlreadyExistsException if the jar filesystem is already open (e.g. by another test or discovery call in the same JVM). Guard against this by falling back to FileSystems.getFileSystem(uri) when the filesystem is already registered. sandbox/qa/dsl-query-types/src/test/java/org/opensearch/dsl/types/DatasetQueryRunner.java [66-73] URI uri = url.toURI(); Path path; if ("jar".equals(uri.getScheme())) { - fs = FileSystems.newFileSystem(uri, Collections.emptyMap()); + try { + fs = FileSystems.newFileSystem(uri, Collections.emptyMap()); + } catch (java.nio.file.FileSystemAlreadyExistsException e) { + fs = FileSystems.getFileSystem(uri); + } path = fs.getPath(resourceDir); } else { path = PathUtils.get(uri); } Suggestion importance[1-10]: 6 __ Why: Legitimate concern: FileSystems.newFileSystem can throw FileSystemAlreadyExistsException when invoked multiple times against the same jar URI in the same JVM, which is plausible given the parameterized test discovery calls. The fallback is a standard, valuable pattern.	Low
General	Compare key sets, not just sizes compareSource is reused via valuesEqual for nested Map comparison and returns a non-null diff string on inequality — but callers of valuesEqual only check for null vs non-null. That's fine, but using compareSource here couples nested equality semantics to field-count strictness, which will incorrectly report unequal for nested maps that legitimately differ in key sets due to omitted null-valued fields. Consider a dedicated deep-equals for nested maps or documenting this strictness. sandbox/qa/dsl-query-types/src/test/java/org/opensearch/dsl/types/DslResponseValidator.java [119-128] private static String compareSource(Map<String, Object> expected, Map<String, Object> actual, String label) { - if (expected.size() != actual.size()) { + // Strict: field count and key set must match exactly. Used both at top-level and recursively + // via valuesEqual for nested map values. + if (!expected.keySet().equals(actual.keySet())) { return String.format( Locale.ROOT, - "%s: field count mismatch - expected %s, got %s", + "%s: field set mismatch - expected %s, got %s", label, new TreeMap<>(expected).keySet(), new TreeMap<>(actual).keySet() ); } Suggestion importance[1-10]: 5 __ Why: Comparing key sets rather than only sizes yields more accurate mismatch reporting and avoids false positives when sizes coincidentally match but keys differ. Reasonable improvement to diagnostic quality.	Low
	Make one-time provisioning thread-safe The provisioned static flag persists across test-suite runs within the same JVM but the index does not necessarily persist across separate test JVMs, and worse, if the class is reused across gradle test workers the flag may prevent re-provisioning after cluster state changes. Also, using a static mutable flag with parallel test execution is unsafe. Consider using @BeforeClass or synchronizing access. sandbox/qa/dsl-query-types/src/test/java/org/opensearch/dsl/types/DslTermQueryIT.java [45-64] private static final Dataset PEOPLE = new Dataset("people", "dsl_people"); -private static boolean provisioned = false; +private static volatile boolean provisioned = false; @Override protected boolean preserveClusterUponCompletion() { return true; } @Override protected boolean preserveIndicesUponCompletion() { return true; } @Before -public void provisionOnce() throws IOException { +public synchronized void provisionOnce() throws IOException { if (provisioned == false) { DatasetProvisioner.provision(client(), PEOPLE); provisioned = true; } } Suggestion importance[1-10]: 3 __ Why: Minor improvement; JUnit test methods within a single class typically run sequentially, so thread-safety concerns are limited. The suggestion is a defensive nicety with low practical impact.	Low

[github-actions]

✅ Gradle check result for 77eff9f: SUCCESS

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.39%. Comparing base (0456d83) to head (77eff9f).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #22608      +/-   ##
============================================
- Coverage     71.39%   71.39%   -0.01%     
- Complexity    76760    76777      +17     
============================================
  Files          6148     6148              
  Lines        357951   357951              
  Branches      52170    52170              
============================================
- Hits         255556   255554       -2     
- Misses        82024    82054      +30     
+ Partials      20371    20343      -28     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.