akshat-lakhera opened a new pull request, #20795:
URL: https://github.com/apache/druid/pull/20795
Part of #20525.
### Description
This is the second slice of the "DataSegment constructors and no-argument
builder()" group from the table in #20525. The 9 and 10 argument DataSegment
constructors and the no-argument DataSegment.builder() are deprecated in favour
of DataSegment.builder(SegmentId), so I moved the remaining callers outside the
server module over to the builder. The first slice (#20788) covered the deep
storage extension tests. This one covers the other extensions (datasketches,
histogram, compressed-bigdecimal, exact-count-bitmap, spectator-histogram,
tdigestsketch, bloom-filter, stats), processing, sql, benchmarks, quidem-ut,
indexing-service and multi-stage-query, which comes to 73 files and 176 calls.
Only the server module is left, and that will be the last PR.
Each constructor call is now DataSegment.builder(SegmentId.of(dataSource,
interval, version, partition)) followed by loadSpec, dimensions, metrics,
shardSpec, binaryVersion and size. I set shardSpec, binaryVersion and size
explicitly because the builder's defaults differ from what the old calls
passed. Each no-argument builder() chain keeps all its setters, and dataSource,
interval and version move into SegmentId.of. One chain in
DruidSchemaInternRowSignatureBenchmark sets the interval later inside a loop,
so I left it alone.
The no-argument builder() and the SegmentId builder also differ in two
defaults: the old one starts with an empty loadSpec and a null binaryVersion,
while the new one starts with a null loadSpec and binaryVersion 0. The three
calls in main code (GeneratorSchemaInfo, KttmNestedComponentSupplier and
TombstoneHelper) now set those two fields explicitly, so they build exactly the
same segments as before. I didn't add them to the test chains. Test segments
that never set a loadSpec now carry null instead of an empty map, which is what
slice 1 did as well. I checked that none of the changed tests read loadSpec or
binaryVersion from those segments. If you'd rather keep the old defaults in the
tests too, I can add the two setters there.
#### Testing
Checkstyle and forbiddenapis pass for all the touched modules. I ran the
unit tests of the changed classes with and without this change on my Windows
machine and got the same results both times. CompactionTaskTest,
TombstoneHelperTest, SegmentAllocateActionTest, the concurrent replace tests,
KillUnusedSegmentsTaskTest and IndexerTableInputSpecSlicerTest pass. The tests
that build real segments or start the SQL test framework (the SQL aggregator
tests, MSQReplaceTest, TaskLifecycleTest and similar) fail the same way before
and after, because of locked memory mapped temp files and colons in paths on
Windows, so I'm relying on CI for those.
<hr>
##### Key changed/added classes in this PR
* GeneratorSchemaInfo, KttmNestedComponentSupplier, TombstoneHelper
* TombstoneHelperTest, SegmentAllocateActionTest, CompactionTaskTest,
MSQReplaceTest and the other indexing-service and multi-stage-query tests
* TestDataBuilder and the sql tests
* the SQL aggregator tests in the datasketches, histogram, bloom-filter,
stats and contrib extensions
* the processing tests and the benchmarks
<hr>
This PR has:
- [x] been self-reviewed.
- [x] a behaviour-preserving refactor covered by the existing unit tests of
the changed classes.
PTAL @FrankChen021
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
