Add embedded test for object-storage-encoding format for nested field by cecemei · Pull Request #18818 · apache/druid

cecemei · 2025-12-05T22:36:13Z

Description

Add embedded test for object-storage-encoding format for nested field.

This PR has:

kfaraz

Thanks for adding the new test, @cecemei !
I have left some minor suggestions, even though the PR is still in draft.

services/src/test/java/org/apache/druid/testing/embedded/EmbeddedClusterApis.java

...ed-tests/src/test/java/org/apache/druid/testing/embedded/indexing/NestedDataFormatsTest.java

processing/src/main/java/org/apache/druid/query/http/ClientSqlQuery.java

services/src/test/java/org/apache/druid/testing/embedded/indexing/Resources.java

processing/src/test/java/org/apache/druid/segment/TestDataSource.java

services/src/test/java/org/apache/druid/testing/embedded/EmbeddedClusterApis.java

kfaraz · 2025-12-09T03:07:34Z

pom.xml

                    <failsOnError>true</failsOnError>
                    <excludes>
-                        *com/fasterxml/jackson/databind/*
+                        *com/fasterxml/jackson/databind/*,**/NestedDataFormatsTest.java


Why is the new test excluded from checkstyle?

parsing error

...ed-tests/src/test/java/org/apache/druid/testing/embedded/indexing/NestedDataFormatsTest.java

kfaraz · 2025-12-09T03:11:22Z

...ed-tests/src/test/java/org/apache/druid/testing/embedded/indexing/NestedDataFormatsTest.java

+    final String scanQuery = "select event, to_json_string(agent) as agent from %s where json_value(event, '$.type') = 'PercentClear' and json_value(agent, '$.os') = 'Android' order by __time asc limit 1";
+    final String scanQueryResultDefaultFormat = cluster.callApi().runSql(scanQuery, defaultFormat);
+    final String scanQueryResultNoneObjectStorage = cluster.callApi().runSql(scanQuery, dataSource);
+    // CHECKSTYLE: text blocks not supported in current Checkstyle version


Let's consider updating the checkstyle version in that case.

only later version of checkstyle support the """ style (java 15+), and had to exclude this file since the error thrown was parsing error. in the later version of checkstyle a bunch of other things failed as well, so prefer to make checkstyle a separate pr besides this one.

Okay, thanks for the details.

I think you can also use // CHECKSTYLE.OFF to turn off checkstyle without having to exclude the entire file in pom. e.g. see StringUtils class.

it still complains about error, i think parsing was before style check

Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.0.0:checkstyle (default-cli) on project druid-embedded-tests: An error has occurred in Checkstyle report generation.: Failed during checkstyle configuration: Exception was thrown while processing /Users/cecemei/codespace/bitmap/embedded-tests/src/test/java/org/apache/druid/testing/embedded/indexing/NestedDataFormatsTest.java: IllegalStateException occurred while parsing file /Users/cecemei/codespace/bitmap/embedded-tests/src/test/java/org/apache/druid/testing/embedded/indexing/NestedDataFormatsTest.java. /Users/cecemei/codespace/bitmap/embedded-tests/src/test/java/org/apache/druid/testing/embedded/indexing/NestedDataFormatsTest.java:118:5: unexpected token: Assertions -> [Help 1]

I see, thanks for checking. Let's just exclude it in the pom until we upgrade the checkstyle version in the follow up PR.

...ed-tests/src/test/java/org/apache/druid/testing/embedded/indexing/NestedDataFormatsTest.java

…/indexing/NestedDataFormatsTest.java Co-authored-by: Kashif Faraz <kashif.faraz@gmail.com>

cecemei · 2025-12-09T05:19:05Z

Thanks for adding the new test, @cecemei ! I have left some minor suggestions, even though the PR is still in draft.

Updated, please take another look, thx!

kfaraz

LGTM, thanks for the updates, @cecemei !

kfaraz · 2025-12-09T06:08:38Z

...ed-tests/src/test/java/org/apache/druid/testing/embedded/indexing/NestedDataFormatsTest.java

+    cluster.callApi().waitForAllSegmentsToBeAvailable(dataSource, coordinator, broker);
+
+    // Test ingesting with skipping raw json smile format works, same row count, with ~20% storage saving
+    final String metadata = "select sum(num_rows), sum(size) from sys.segments where datasource = '%s' group by datasource";


I think the GROUP BY clause can be omitted.

Suggested change

final String metadata = "select sum(num_rows), sum(size) from sys.segments where datasource = '%s' group by datasource";

final String metadataSql = "select sum(num_rows), sum(size) from sys.segments where datasource = '%s'";

kfaraz · 2025-12-09T06:09:22Z

...ed-tests/src/test/java/org/apache/druid/testing/embedded/indexing/NestedDataFormatsTest.java

+    Assertions.assertEquals(queryResultDefaultFormat, queryResultNoneObjectStorage);
+
+    // Test reconstruct json column works, the ordering of the fields has changed, but all values are perserved.
+    final String scanQuery = "select event, to_json_string(agent) as agent from %s where json_value(event, '$.type') = 'PercentClear' and json_value(agent, '$.os') = 'Android' order by __time asc limit 1";


Please try to break the SQL string into multiple lines.

jtuglu1 · 2025-12-10T18:57:27Z

Hey folks, looks like this updated to JDK 17 compiler. This was a change previously reverted here: #18759 as it breaks a few things.

nested-it

868cf2f

github-actions bot added Area - Dependencies Area - Ingestion labels Dec 5, 2025

cecemei added 5 commits December 5, 2025 14:41

Merge branch 'master' into nested-it

3ee90cc

checkstyle

fc63ca0

license

f2eb056

fix-test

4d1d6e8

emitter-config

299766b

kfaraz reviewed Dec 7, 2025

View reviewed changes

cecemei added 2 commits December 8, 2025 11:25

setup

1deaef6

Merge branch 'master' into nested-it

fb1de07

cecemei marked this pull request as ready for review December 8, 2025 19:26

cecemei added 2 commits December 8, 2025 12:08

prefix

e79b5c7

small-update

4c20bde

kfaraz reviewed Dec 9, 2025

View reviewed changes

cecemei and others added 8 commits December 8, 2025 20:09

Update embedded-tests/src/test/java/org/apache/druid/testing/embedded…

2eceaa9

…/indexing/NestedDataFormatsTest.java Co-authored-by: Kashif Faraz <kashif.faraz@gmail.com>

datasourceWithDefaultFormat

2aca113

prefix

09bdf30

Update embedded-tests/src/test/java/org/apache/druid/testing/embedded…

170d354

…/indexing/NestedDataFormatsTest.java Co-authored-by: Kashif Faraz <kashif.faraz@gmail.com>

style

1eaace7

sql

0786920

nested

285204c

client-sql

11b1f5b

kfaraz approved these changes Dec 9, 2025

View reviewed changes

format

1d44512

cecemei merged commit 8a2d2ed into apache:master Dec 9, 2025
55 checks passed

kgyrtkirk added this to the 36.0.0 milestone Jan 19, 2026

	final String metadata = "select sum(num_rows), sum(size) from sys.segments where datasource = '%s' group by datasource";
	final String metadataSql = "select sum(num_rows), sum(size) from sys.segments where datasource = '%s'";

Conversation

cecemei commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

kfaraz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

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!

Uh oh!

Uh oh!

cecemei commented Dec 9, 2025

Uh oh!

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

Uh oh!

jtuglu1 commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Comments

cecemei commented Dec 5, 2025 •

edited

Loading

jtuglu1 commented Dec 10, 2025 •

edited

Loading