[esql] NDJSON datasource by swallez · Pull Request #142560 · elastic/elasticsearch

swallez · 2026-02-16T16:52:20Z

Add a NDJSON datasource, building on the abstractions added in PR #141678

…ceptions

costin · 2026-02-17T16:19:03Z

Overall structure looks good and follows the CSV/Parquet patterns well.
A few things to address:

QA fixture path typo
resources are under iceberg-features/ but should be iceberg-fixtures/ to match the convention used by CSV/Parquet and what Clusters.java resolves
Resource leaks
NullBlockBuilder.close() doesn't release the block it allocates, and NdJsonPageIterator.close() doesn't guard against partial failures (needs try-finally). There might be others.
Type mapping mismatches
the schema inferrer maps all integers to LONG and doesn't detect dates, but the csv-spec tests expect INTEGER and DATE columns. These tests will fail as-is
Boolean inconsistency between the two test fixtures
unit test data uses native JSON booleans, QA data uses string booleans. The decoder only handles native booleans so the QA fixture will behave differently
readAllEmployees test hardcodes a URL instead of using the {{employees}} template
This will cause issue during execution of the test
Schema inferred twice
you already have the FIXME
Stale comments and missing dependencyLicenses mapping in build.gradle
copied from CSV without updating

…kBuilder)

swallez · 2026-02-17T18:52:14Z

@costin I addressed your comments.

Schema inference now handles ISO-formatted dates and distinguishes integer and long.

I ended up creating a separate csv-spec file: ndjson is the first format where the schema is inferred, and this doesn't fit with tests that involve variants like height.float or height.scaled_float.

bpintea

Looking good, left a few questions.

bpintea · 2026-02-17T17:03:44Z

...-ndjson/src/main/java/org/elasticsearch/xpack/esql/datasource/ndjson/NdJsonPageIterator.java

+    private final NdJsonPageDecoder pageDecoder;
+    private boolean endOfFile = false;
+    private Page nextPage;
+    private InputStream inputStream;


May be final.

Removed it, pageDecoder now owns it.

bpintea · 2026-02-17T17:13:58Z

...e-ndjson/src/main/java/org/elasticsearch/xpack/esql/datasource/ndjson/NdJsonPageDecoder.java

+
+    @Override
+    public void close() {
+        for (var blockBuilder : blockBuilders) {


You may use Releasables#close for this.

bpintea · 2026-02-17T18:45:12Z

...e-ndjson/src/main/java/org/elasticsearch/xpack/esql/datasource/ndjson/NdJsonPageDecoder.java

+
+        Block[] blocks = new Block[blockBuilders.length];
+        for (int i = 0; i < blockBuilders.length; i++) {
+            blocks[i] = blockBuilders[i].build();


Should the building and the closing be decoupled? Maybe have the closing done in a final clause and with the close() method? The caller of this method doesn't call close() on exception either. I see the caller rewraps the IOException, but .build() can raise a CircuitBreakingException.

Good catch. I've refactored this method in 6d22cab so that block builders are in a local variable and are always closed. We don't need to keep them longer than that.

Also adjusted the code so that created blocks are closed even if an error happens when building one. And also moved ownership of the InputStream to this class, as we don't need it higher-level classes.

bpintea · 2026-02-17T18:54:57Z

...e-ndjson/src/main/java/org/elasticsearch/xpack/esql/datasource/ndjson/NdJsonPageDecoder.java

+        // Builders setup independently as we need to create new ones for each page.
+        void setupBuilders() {
+            if (attribute != null) {
+                blockBuilder = switch (attribute.dataType()) {


Can we not use ConstantNullBlock.Builder?

Done. That class wasn't public, which is why I didn't see it. I changed it to public as there's no reason to keep it private.

bpintea · 2026-02-17T18:57:49Z

...e-ndjson/src/main/java/org/elasticsearch/xpack/esql/datasource/ndjson/NdJsonPageDecoder.java

+            }
+        }
+
+        private void decodeValue(JsonParser parser, int position) throws IOException {


token is unused.

bpintea · 2026-02-17T18:58:16Z

...e-ndjson/src/main/java/org/elasticsearch/xpack/esql/datasource/ndjson/NdJsonPageDecoder.java

+                    // Unknown field, skip it
+                    parser.skipChildren();
+                } else {
+                    childDecoder.decodeValue(parser, position);


position is unused.

bpintea · 2026-02-17T19:00:36Z

...e-ndjson/src/main/java/org/elasticsearch/xpack/esql/datasource/ndjson/NdJsonPageDecoder.java

+    }
+
+    // ---------------------------------------------------------------------------------------------
+    // A tree of decoders. Avoids path reconstruction when traversing nested objects.


Can the ones in the c'tor arguments be final?

(I left the review notes in IntelliJ and I think you got to push an update on the file after my checkout; so all comments below apply to code 4 lines below.)

There's no constructor, all fields are set while traversing the list of Attribute 😅

bpintea · 2026-02-17T19:09:00Z

...asource-ndjson/src/main/java/org/elasticsearch/xpack/esql/datasource/ndjson/NdJsonUtils.java

+
+        int c;
+        while ((c = input.read()) != -1) {
+            if (c == '\n' || c == '\r') {


We should add tests with \n\r or \r\n trailing sequences.

bpintea · 2026-02-17T19:10:07Z

...djson/src/main/java/org/elasticsearch/xpack/esql/datasource/ndjson/NdJsonSchemaInferrer.java

+        List<Attribute> attributes = new ArrayList<>();
+        buildSchema(root, null, attributes);
+        return attributes;
+    }


Method's not used.

Removed, a leftover from previous iterations.

bpintea · 2026-02-17T19:12:09Z

...djson/src/main/java/org/elasticsearch/xpack/esql/datasource/ndjson/NdJsonSchemaInferrer.java

+
+    /**
+     * Infers schema from an NDJSON input stream, reading up to maxLines.
+     */


Is the name member ever used in FieldInfo?

Indeed it's not. The key in the parent's children map is what is used.

swallez

@bpintea thanks for the thorough review! I have addressed all your comments.

swallez · 2026-02-18T13:54:32Z

...e-ndjson/src/main/java/org/elasticsearch/xpack/esql/datasource/ndjson/NdJsonPageDecoder.java

+
+        Block[] blocks = new Block[blockBuilders.length];
+        for (int i = 0; i < blockBuilders.length; i++) {
+            blocks[i] = blockBuilders[i].build();


Good catch. I've refactored this method in 6d22cab so that block builders are in a local variable and are always closed. We don't need to keep them longer than that.

Also adjusted the code so that created blocks are closed even if an error happens when building one. And also moved ownership of the InputStream to this class, as we don't need it higher-level classes.

swallez · 2026-02-18T13:55:13Z

...e-ndjson/src/main/java/org/elasticsearch/xpack/esql/datasource/ndjson/NdJsonPageDecoder.java

+
+    @Override
+    public void close() {
+        for (var blockBuilder : blockBuilders) {


swallez · 2026-02-18T14:02:25Z

...e-ndjson/src/main/java/org/elasticsearch/xpack/esql/datasource/ndjson/NdJsonPageDecoder.java

+    }
+
+    // ---------------------------------------------------------------------------------------------
+    // A tree of decoders. Avoids path reconstruction when traversing nested objects.


There's no constructor, all fields are set while traversing the list of Attribute 😅

swallez · 2026-02-18T14:03:43Z

...e-ndjson/src/main/java/org/elasticsearch/xpack/esql/datasource/ndjson/NdJsonPageDecoder.java

+        // Builders setup independently as we need to create new ones for each page.
+        void setupBuilders() {
+            if (attribute != null) {
+                blockBuilder = switch (attribute.dataType()) {


Done. That class wasn't public, which is why I didn't see it. I changed it to public as there's no reason to keep it private.

swallez · 2026-02-18T14:08:44Z

...-ndjson/src/main/java/org/elasticsearch/xpack/esql/datasource/ndjson/NdJsonPageIterator.java

+    private final NdJsonPageDecoder pageDecoder;
+    private boolean endOfFile = false;
+    private Page nextPage;
+    private InputStream inputStream;


Removed it, pageDecoder now owns it.

swallez · 2026-02-18T14:14:46Z

...djson/src/main/java/org/elasticsearch/xpack/esql/datasource/ndjson/NdJsonSchemaInferrer.java

+
+    /**
+     * Infers schema from an NDJSON input stream, reading up to maxLines.
+     */


Indeed it's not. The key in the parent's children map is what is used.

swallez · 2026-02-18T14:15:12Z

...djson/src/main/java/org/elasticsearch/xpack/esql/datasource/ndjson/NdJsonSchemaInferrer.java

+        List<Attribute> attributes = new ArrayList<>();
+        buildSchema(root, null, attributes);
+        return attributes;
+    }


Removed, a leftover from previous iterations.

bpintea

LGTM

romseygeek · 2026-02-23T11:58:29Z

Hey @swallez I think this is causing CI failures - see https://gradle-enterprise.elastic.co/s/ltdo2i6lpx7wa/tests/task/:x-pack:plugin:esql-datasource-ndjson:qa:javaRestTest/details/org.elasticsearch.xpack.esql.qa.ndjson.NdJsonFormatSpecIT/test%20%7Bcsv-spec:external-ndjson.FilterByGender%20%5BGCS%5D%7D?top-execution=1 for example

swallez · 2026-02-23T12:27:44Z

@romseygeek looking at it. Thanks for the heads up!

This reverts commit 605c05b.

Adds an NDJSON ESQL datasource. The schema is inferred from the first 100 lines. Supported types are INTEGER, DOUBLE, LONG, BOOLEAN, DATETIME (strings in ISO format) and KEYWORD. Multi-values are supported, nested objects are flattened by joining property names with a '.' In case of parsing failure, the stream is consumed until the next end of line to recover.

This reverts commit 605c05b.

swallez added 2 commits February 16, 2026 17:37

Add NDJSON datasource

fc45beb

Disable IT tests

7032d6a

swallez requested a review from costin February 16, 2026 16:52

swallez added >feature WIP :Analytics/ES|QL AKA ESQL v9.4.0 labels Feb 16, 2026

[CI] Auto commit changes from spotless

428d30e

swallez changed the title ~~Esql/datasource ndjson~~ [esql] NDJSON datasource Feb 16, 2026

swallez and others added 5 commits February 17, 2026 11:43

Avoid linking buffers when recovering after parsing error. Improve ex…

c547484

…ceptions

Update readme

a999c54

Make sure pages contain all projected fields, and in order

2f1f9ff

[CI] Auto commit changes from spotless

65e125d

Merge branch 'main' into esql/datasource-ndjson

be71779

costin requested a review from bpintea February 17, 2026 16:01

swallez added 3 commits February 17, 2026 19:45

Make ConstantNullBlock.Builder public (fixes leak in removed NullBloc…

7a6acae

…kBuilder)

Fix QA tests, add integer and datetime types

2736ecc

Misc cleanup

0efe65e

bpintea reviewed Feb 17, 2026

View reviewed changes

swallez added 6 commits February 18, 2026 12:14

Use Releasable

050d91c

Make inputStream final

64a63a7

Fix unit tests after addition of INTEGER and DATETIME types

5ca027f

Make sure every block & builder is closed correctly

6d22cab

Remove unused stuff

a8543c7

More cr/lf tests

ab09d8b

swallez commented Feb 18, 2026

View reviewed changes

swallez and others added 2 commits February 18, 2026 18:30

Merge branch 'main' into esql/datasource-ndjson

93c44af

Remove useless test line

d23c95a

bpintea approved these changes Feb 20, 2026

View reviewed changes

swallez merged commit 605c05b into elastic:main Feb 23, 2026
35 checks passed

swallez deleted the esql/datasource-ndjson branch February 23, 2026 08:52

breskeby added a commit that referenced this pull request Feb 23, 2026

Revert "[esql] NDJSON datasource (#142560)"

e6aaa8b

This reverts commit 605c05b.

sidosera pushed a commit to sidosera/elasticsearch that referenced this pull request Feb 24, 2026

Revert "[esql] NDJSON datasource (elastic#142560)"

d6e6691

This reverts commit 605c05b.

tylerperk added the ES|QL|DS ES|QL datasources label Mar 18, 2026

Conversation

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

Uh oh!

Uh oh!

costin commented Feb 17, 2026

Uh oh!

swallez commented Feb 17, 2026

Uh oh!

bpintea 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!

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!

swallez 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!

bpintea left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

romseygeek commented Feb 23, 2026

Uh oh!

swallez commented Feb 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

swallez commented Feb 16, 2026 •

edited

Loading