Repository navigation
[fix](regression) stabilize parquet_meta tvf assertions across s3 buckets - #60938
Merged
morningman merged 1 commit intoMar 2, 2026
Merged
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Stabilizes test_parquet_meta_tvf regression assertions in S3 mode by avoiding bucket-/path-dependent fields and updating expected baselines accordingly.
Changes:
- Replaced
select *with explicit projections of stable parquet metadata/schema columns. - Added deterministic
order byclauses for more stable result ordering. - Updated
.outgolden file to match the new projections (removing full S3 URI / file_name assertions).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| regression-test/suites/external_table_p0/tvf/test_parquet_meta_tvf.groovy | Narrows asserted columns to stable metadata and adds ordering to reduce S3 bucket variability. |
| regression-test/data/external_table_p0/tvf/test_parquet_meta_tvf.out | Updates golden outputs to match the new stable projections and reduced S3-path dependence. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+122
to
123
| select count(*) from parquet_meta( | ||
| "uri" = "${basePath}/*meta.parquet", |
There was a problem hiding this comment.
Switching the glob assertion from select file_name to count(*) makes the test significantly weaker (it can pass even if the wrong set of files is returned, as long as the total count matches). A more robust, bucket-stable assertion is to project a stable derived value from file_name (e.g., the basename via a regex) and order by it, so the test still validates which files are present without depending on the bucket/URI prefix.
Comment on lines
+36
to
+39
| select | ||
| row_group_id, row_group_num_rows, row_group_num_columns, row_group_bytes, column_id, | ||
| file_offset, num_values, path_in_schema, type, stats_min, stats_max, stats_null_count, | ||
| stats_distinct_count, stats_min_value |
There was a problem hiding this comment.
The stable column projection list is duplicated across the S3 and default-mode parquet_metadata queries. To reduce maintenance overhead (and avoid future drift), consider extracting this shared column list into a single Groovy variable/string and interpolating it into both query strings.
Comment on lines
+53
to
+56
| select | ||
| row_group_id, row_group_num_rows, row_group_num_columns, row_group_bytes, column_id, | ||
| file_offset, num_values, path_in_schema, type, stats_min, stats_max, stats_null_count, | ||
| stats_distinct_count, stats_min_value |
There was a problem hiding this comment.
The stable column projection list is duplicated across the S3 and default-mode parquet_metadata queries. To reduce maintenance overhead (and avoid future drift), consider extracting this shared column list into a single Groovy variable/string and interpolating it into both query strings.
Comment on lines
+37
to
+39
| row_group_id, row_group_num_rows, row_group_num_columns, row_group_bytes, column_id, | ||
| file_offset, num_values, path_in_schema, type, stats_min, stats_max, stats_null_count, | ||
| stats_distinct_count, stats_min_value |
There was a problem hiding this comment.
The projection includes stats_min_value but not the corresponding stats_max_value. If the intent is to keep the assertion symmetric and interpretable (especially for non-numeric types), consider selecting both stats_min_value and stats_max_value (or neither) so the test coverage of stats doesn’t become lopsided.
Contributor
Author
|
run buildall |
Contributor
|
PR approved by at least one committer and no changes requested. |
github-actions
Bot
added
approved
Contributor
|
PR approved by anyone and no changes requested. |
hubgeter
approved these changes
Mar 2, 2026
xylaaaaa
added a commit
to xylaaaaa/doris
that referenced
this pull request
Mar 16, 2026
…kets (apache#60938) ## Summary - adjust `test_parquet_meta_tvf` S3-mode checks to compare only stable columns - avoid asserting `file_name` / full S3 URI fields that vary by pipeline bucket - update the corresponding `.out` baseline for the changed query projections ## Why Different CI pipelines may use different bucket names, which causes false failures when full URI/file name columns are compared. ## Test - attempted: `./run-regression-test.sh --run -f external_table_p0/tvf/test_parquet_meta_tvf -forceGenOut` - in this environment it failed with S3 `FORBIDDEN` while reading regression parquet files
xylaaaaa
added a commit
to xylaaaaa/doris
that referenced
this pull request
Mar 16, 2026
…kets (apache#60938) ## Summary - adjust `test_parquet_meta_tvf` S3-mode checks to compare only stable columns - avoid asserting `file_name` / full S3 URI fields that vary by pipeline bucket - update the corresponding `.out` baseline for the changed query projections ## Why Different CI pipelines may use different bucket names, which causes false failures when full URI/file name columns are compared. ## Test - attempted: `./run-regression-test.sh --run -f external_table_p0/tvf/test_parquet_meta_tvf -forceGenOut` - in this environment it failed with S3 `FORBIDDEN` while reading regression parquet files
morningman
pushed a commit
to xylaaaaa/doris
that referenced
this pull request
Mar 16, 2026
…kets (apache#60938) ## Summary - adjust `test_parquet_meta_tvf` S3-mode checks to compare only stable columns - avoid asserting `file_name` / full S3 URI fields that vary by pipeline bucket - update the corresponding `.out` baseline for the changed query projections ## Why Different CI pipelines may use different bucket names, which causes false failures when full URI/file name columns are compared. ## Test - attempted: `./run-regression-test.sh --run -f external_table_p0/tvf/test_parquet_meta_tvf -forceGenOut` - in this environment it failed with S3 `FORBIDDEN` while reading regression parquet files
morningman
pushed a commit
to morningman/doris
that referenced
this pull request
Mar 17, 2026
…kets (apache#60938) ## Summary - adjust `test_parquet_meta_tvf` S3-mode checks to compare only stable columns - avoid asserting `file_name` / full S3 URI fields that vary by pipeline bucket - update the corresponding `.out` baseline for the changed query projections ## Why Different CI pipelines may use different bucket names, which causes false failures when full URI/file name columns are compared. ## Test - attempted: `./run-regression-test.sh --run -f external_table_p0/tvf/test_parquet_meta_tvf -forceGenOut` - in this environment it failed with S3 `FORBIDDEN` while reading regression parquet files
xylaaaaa
added a commit
to xylaaaaa/doris
that referenced
this pull request
Mar 18, 2026
…kets (apache#60938) ## Summary - adjust `test_parquet_meta_tvf` S3-mode checks to compare only stable columns - avoid asserting `file_name` / full S3 URI fields that vary by pipeline bucket - update the corresponding `.out` baseline for the changed query projections ## Why Different CI pipelines may use different bucket names, which causes false failures when full URI/file name columns are compared. ## Test - attempted: `./run-regression-test.sh --run -f external_table_p0/tvf/test_parquet_meta_tvf -forceGenOut` - in this environment it failed with S3 `FORBIDDEN` while reading regression parquet files
xylaaaaa
added a commit
to xylaaaaa/doris
that referenced
this pull request
Mar 18, 2026
…kets (apache#60938) ## Summary - adjust `test_parquet_meta_tvf` S3-mode checks to compare only stable columns - avoid asserting `file_name` / full S3 URI fields that vary by pipeline bucket - update the corresponding `.out` baseline for the changed query projections ## Why Different CI pipelines may use different bucket names, which causes false failures when full URI/file name columns are compared. ## Test - attempted: `./run-regression-test.sh --run -f external_table_p0/tvf/test_parquet_meta_tvf -forceGenOut` - in this environment it failed with S3 `FORBIDDEN` while reading regression parquet files
xylaaaaa
added a commit
to xylaaaaa/doris
that referenced
this pull request
Mar 19, 2026
…kets (apache#60938) ## Summary - adjust `test_parquet_meta_tvf` S3-mode checks to compare only stable columns - avoid asserting `file_name` / full S3 URI fields that vary by pipeline bucket - update the corresponding `.out` baseline for the changed query projections ## Why Different CI pipelines may use different bucket names, which causes false failures when full URI/file name columns are compared. ## Test - attempted: `./run-regression-test.sh --run -f external_table_p0/tvf/test_parquet_meta_tvf -forceGenOut` - in this environment it failed with S3 `FORBIDDEN` while reading regression parquet files
morningman
pushed a commit
that referenced
this pull request
Mar 19, 2026
) (#61474) Replacement for #61446 because the original PR head branch could not be updated from this environment. This branch contains: - [feat] add Parquet metadata TVF (#58972) - [fix](regression) stabilize parquet_meta tvf assertions across s3 buckets (#60938) - [fix] remove useless const_cast and explain const_cast for vec (4) (#56603) - [fix] fix parquet metadata TVF compile on branch-4.1 Please review this PR instead of #61446. --------- Co-authored-by: admiring_xm <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
test_parquet_meta_tvfS3-mode checks to compare only stable columnsfile_name/ full S3 URI fields that vary by pipeline bucket.outbaseline for the changed query projectionsWhy
Different CI pipelines may use different bucket names, which causes false failures when full URI/file name columns are compared.
Test
./run-regression-test.sh --run -f external_table_p0/tvf/test_parquet_meta_tvf -forceGenOutFORBIDDENwhile reading regression parquet files