Skip to content

fix: apply the catalog's targetSegmentRows to MSQ ingestion - #20561

Merged
FrankChen021 merged 1 commit into
apache:masterfrom
FrankChen021:codex/fix-catalog-rows-per-segment
Oct 17, 2026
Merged

FrankChen021 merged 1 commit into
apache:masterfrom
FrankChen021:codex/fix-catalog-rows-per-segment

Conversation

@FrankChen021

Copy link
Copy Markdown
Member

Description

When an INSERT or REPLACE targets a catalog table with targetSegmentRows, DruidSqlValidator copies that value into the query context. It used the key msqRowsPerSegment, but MSQ reads rowsPerSegment (MultiStageQueryContext.CTX_ROWS_PER_SEGMENT). Nothing ever read msqRowsPerSegment, so the catalog value was ignored and MSQ used its default of 3,000,000 rows. The mismatch dates back to #15908.

This PR changes DruidSqlValidator.CTX_ROWS_PER_SEGMENT to rowsPerSegment. A rowsPerSegment set in the query context still takes precedence over the catalog value.

Release note

For SQL-based ingestion into a catalog table, the table's targetSegmentRows property now sets the target number of rows per segment. Previously it was ignored, and the default of 3,000,000 rows was used.


Key changed/added classes in this PR
  • DruidSqlValidator
  • CalciteCatalogIngestionDmlTest

This PR has:

  • been self-reviewed.
  • a release note entry in the PR description.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.

DruidSqlValidator wrote the catalog's targetSegmentRows under msqRowsPerSegment, a key MSQ never reads, so MSQ
ignored it and used its default. Write it under rowsPerSegment, the key MSQ reads.
Copilot AI balanced review requested due to automatic review settings October 13, 2026 09:58

This comment was marked as resolved.

@FrankChen021 FrankChen021 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The corrected context key matches MSQ’s rowsPerSegment key, so catalog targetSegmentRows reaches MSQ tuning while explicit query-context values retain precedence. No actionable issue blocks merging.

Reviewed 2 of 2 changed files.

Validation: git diff --check passed. No builds or tests were run.


This is an automated review by Codex GPT-5.6-Luna(max)

@FrankChen021
FrankChen021 merged commit 2f7bedc into apache:master Oct 17, 2026
42 of 43 checks passed
@FrankChen021
FrankChen021 deleted the codex/fix-catalog-rows-per-segment branch October 17, 2026 15:26
@github-actions github-actions Bot added this to the 39.0.0 milestone Oct 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants