Skip to content

Reject input file segment name templates that reference no match group - #19755

Merged
Jackie-Jiang merged 1 commit into
apache:masterfrom
Jackie-Jiang:input-file-segment-name-template-validation
Oct 5, 2026
Merged

Jackie-Jiang merged 1 commit into
apache:masterfrom
Jackie-Jiang:input-file-segment-name-template-validation

Conversation

@Jackie-Jiang

@Jackie-Jiang Jackie-Jiang commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

PR flow

InputFileSegmentNameGenerator now validates that segment.name.template references a match group of filePathPattern, throwing an exception if not, as verified by a new test.

flowchart TD
  N0["Test#58; testTemplateWithoutMatchGroup #91;F5#93;#39;#125;#44; #123; #40;F5#41;"]:::stAdded
  N1["Constructor #91;F2#93; #40;F2#41;"]:::stModified
  N2["referencesMatchGroup method #91;F2#93; #40;F2#41;"]:::stAdded
  N0 -->|"calls with template lacking match group reference"| N1
  N1 -->|"invokes validation"| N2
  N2 -->|"returns false when no match group referenced"| N1
  classDef stAdded fill:#dafbe1,stroke:#1a7f37,color:#1f2328,stroke-width:2px
  classDef stModified fill:#fff8c5,stroke:#9a6700,color:#1f2328,stroke-width:2px
  classDef stRemoved fill:#ffebe9,stroke:#cf222e,color:#1f2328,stroke-width:2px
  classDef stUnchanged fill:#f6f8fa,stroke:#656d76,color:#1f2328,stroke-width:1px
Loading

AI-generated · Green: added · Yellow: modified · Red: removed · Gray: existing

Diff evidence
  • F2: pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/creator/name/InputFileSegmentNameGenerator.java — before · after
  • F5: pinot-segment-spi/src/test/java/org/apache/pinot/segment/spi/creator/name/InputFileSegmentNameGeneratorTest.java — before · after
  • Regenerate PR flow

Summary

InputFileSegmentNameGenerator now rejects a segment.name.template that does not contain ${filePathPattern:\N} for any match group N of file.path.pattern. Such a template ignores the input file, so every input file gets the same segment name: the pushed segments replace each other, or the push fails when the deep store rejects the name (#19696).

The common way to hit this is a table config (batchIngestionConfig.batchConfigMaps, read by SegmentGenerationAndPushTask). Table config values that are entirely of the form ${NAME:default} are resolved from environment variables and system properties when the config is read, so "segmentNameGenerator.configs.segment.name.template": "${filePathPattern:\\1}" reaches the generator as \1. The error message points to the $${filePathPattern:\N} escape added in #19202.

Also drops the unused @SuppressWarnings("serial") from the segment name generators and logs the unmatched input file path with parameterized logging.

Backward incompatibility

A template that references no match group is now rejected instead of producing a constant segment name. This includes a constant template combined with append.uuid.to.segment.name=true, which produced unique names. Use the fixed generator for a constant name, or simple with append.uuid.to.segment.name for unique names.

Related to #19696

🤖 Generated with Claude Code

@Jackie-Jiang Jackie-Jiang added bug Something is not working as expected release-notes Referenced by PRs that need attention when compiling the next release notes ingestion Related to data ingestion pipeline backward-incompat Introduces a backward-incompatible API or behavior change labels Oct 5, 2026
`InputFileSegmentNameGenerator` now rejects a `segment.name.template` that does not contain `${filePathPattern:\N}`
for any match group `N` of `file.path.pattern`. Such a template gives every input file the same segment name. In a
table config this happens silently when the template is entirely `${filePathPattern:\N}`: table config values of the
form `${NAME:default}` are resolved from environment variables and system properties, which turns it into `\N`. The
error message points to the `$${filePathPattern:\N}` escape.

Also drops the unused `@SuppressWarnings("serial")` from the segment name generators and logs the unmatched input file
path with parameterized logging.
@Jackie-Jiang
Jackie-Jiang force-pushed the input-file-segment-name-template-validation branch from 4bf4475 to ae8500e Compare October 5, 2026 18:54
@codecov-commenter

codecov-commenter commented Oct 5, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 68.32%. Comparing base (b8e2ef2) to head (ae8500e).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...pi/creator/name/InputFileSegmentNameGenerator.java 90.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             master   #19755       +/-   ##
=============================================
+ Coverage     40.03%   68.32%   +28.29%     
- Complexity     1463     1470        +7     
=============================================
  Files          3520     3520               
  Lines        228951   228956        +5     
  Branches      36306    36308        +2     
=============================================
+ Hits          91658   156445    +64787     
+ Misses       129004    60241    -68763     
- Partials       8289    12270     +3981     
Flag Coverage Δ
integration 100.00% <ø> (+100.00%) ⬆️
integration1 100.00% <ø> (?)
integration2 0.00% <ø> (ø)
java-25 68.32% <90.00%> (+28.29%) ⬆️
lane-a 100.00% <ø> (+100.00%) ⬆️
lane-b 0.00% <ø> (ø)
temurin 68.32% <90.00%> (+28.29%) ⬆️
unittests 68.32% <90.00%> (+28.29%) ⬆️
unittests1 58.12% <90.00%> (?)
unittests2 40.03% <0.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Jackie-Jiang
Jackie-Jiang merged commit 6a2ab12 into apache:master Oct 5, 2026
17 of 25 checks passed
@Jackie-Jiang
Jackie-Jiang deleted the input-file-segment-name-template-validation branch October 5, 2026 22:58
xiangfu0 added a commit to pinot-contrib/pinot-docs that referenced this pull request Oct 5, 2026
Documents the template/capture-group validation added by
apache/pinot#19755 in the existing job specification reference.

Co-authored-by: Xiang Fu <xiangfu@Xiang-mac-mtv-2.local>
@xiangfu0

xiangfu0 commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Documentation follow-up merged: pinot-contrib/pinot-docs#1083

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backward-incompat Introduces a backward-incompatible API or behavior change bug Something is not working as expected ingestion Related to data ingestion pipeline release-notes Referenced by PRs that need attention when compiling the next release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants