Skip to content

[CALCITE-7743] RelJson cannot emit hints - #5224

Merged
mihaibudiu merged 1 commit into
apache:mainfrom
microbluey:fix/reljson-hints
Aug 26, 2026
Merged

[CALCITE-7743] RelJson cannot emit hints#5224
mihaibudiu merged 1 commit into
apache:mainfrom
microbluey:fix/reljson-hints

Conversation

@microbluey

Copy link
Copy Markdown
Contributor

RelOptUtil.dumpPlan(..., JSON) throws UnsupportedOperationException for any plan whose LogicalTableScan carries hints: LogicalTableScan.explainTerms emits a hints item (added for the CALCITE-4581 digest fix), but RelJson.toJson(Object) has no branch for RelHint. Reading such a plan back silently dropped the hints.

  • RelJson: serialize RelHint (name, inheritPath, list/kv options, position when not ZERO); toHint/toHints for reading back
  • RelInput: new getHints() with an empty default, so existing implementations are unaffected
  • RelJsonReader reads the hints entry; TableScan(RelInput) passes it through, so hints survive the round trip

Only LogicalTableScan emits hints in explain output today; other Hintable nodes put hints in their digest, which does not go through RelJsonWriter.

Test: RelJsonTest.testHint — SQL with a table hint, dump to JSON (previously threw), read back through RelJsonReader, assert hints round-trip.

Serialize RelHint in RelJson and read the hints entry back in
TableScan(RelInput) via a new RelInput.getHints(), so hints survive
a JSON dump/read round trip instead of crashing the serializer.
@mihaibudiu mihaibudiu added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label Aug 26, 2026
@sonarqubecloud

Copy link
Copy Markdown

@mihaibudiu mihaibudiu removed the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label Aug 26, 2026
@mihaibudiu

Copy link
Copy Markdown
Contributor

Removed the label LGTM to see if checks run. But it seems that checks are still being skipped, although I required all of them to be executed.

@mihaibudiu
mihaibudiu merged commit cd2b705 into apache:main Aug 26, 2026
50 of 51 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants