Skip to content

[fix](fe) Preserve SQL-rendering fields in Expr Gson serialization - #67159

Open
0AyanamiRei wants to merge 2 commits into
apache:masterfrom
0AyanamiRei:fix/expr-gson-semantic-serialization
Open

[fix](fe) Preserve SQL-rendering fields in Expr Gson serialization#67159
0AyanamiRei wants to merge 2 commits into
apache:masterfrom
0AyanamiRei:fix/expr-gson-semantic-serialization

Conversation

@0AyanamiRei

@0AyanamiRei 0AyanamiRei commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: None

Related PR: #62900

Problem Summary:

Doris metadata Gson instances use an exclusion strategy that omits fields without @SerializedName.

ExprGsonSerializationTest previously verified the restored concrete subtype and JSON idempotence. Those assertions can still pass after a field has already been omitted from the first serialized JSON, so they do not prove that an expression keeps the same canonical SQL.

This PR adds stable serialized names for state used by SQL reconstruction in:

  • FunctionCallExpr and its nested OrderByElement
  • MatchPredicate
  • PlaceHolderExpr
  • SearchPredicate
  • SlotRef
  • TimeV2Literal
  • VariableExpr

It also strengthens the shared Expr serialization contract:

  • the source Expr hierarchy, both Gson subtype registries, and the sample corpus must cover the same concrete subtypes;
  • every direct Expr instance field must be serialized or explicitly classified as derived, non-durable state;
  • every concrete sample must preserve ExprToSqlVisitor output with and without table names through both GsonUtils.GSON and GsonUtilsCatalog.GSON;
  • non-default nested function ORDER BY state and placeholder MySQL protocol state are checked directly.

A colocated AGENTS.md records these persistence and test requirements as a review checklist for future Expr and metadata-consumer changes.

The durable contract is concrete subtype plus canonical SQL reconstruction. Analyzer, planner, translation, and execution caches remain intentionally non-durable and are rebuilt by later analysis. The newly added JSON fields are additive; metadata written before this change cannot recover values that were never serialized.

Release note

None

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
      • ./run-fe-ut.sh --run org.apache.doris.analysis.ExprGsonSerializationTest
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason
  • Behavior changed:

    • No. User-facing SQL syntax and interfaces are unchanged; newly written Expr JSON preserves the covered SQL-rendering state.
    • Yes.
  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

### What problem does this PR solve?

Issue Number: None

Related PR: apache#62900

Problem Summary: Doris metadata Gson instances omit fields without @SerializedName. The existing Expr Gson test checked subtype restoration and JSON idempotence, but both checks can pass after a semantic field has already been dropped from the first JSON representation.

Add stable serialized names for Expr state required to reconstruct canonical SQL, including function ORDER BY metadata and placeholder protocol state. Strengthen the shared Expr test so the source hierarchy, both Gson subtype registries, and the sample corpus remain aligned; every direct Expr field is serialized or explicitly classified as derived state; and every concrete sample preserves SQL with and without table names through both Gson instances. Add direct round-trip checks for nested ORDER BY elements and the placeholder MySQL type code.

Analyzer, planner, and execution caches remain non-durable and must be rebuilt after SQL reconstruction and analysis.

### Release note

None

### Check List (For Author)

- Test: Unit Test
    - ./run-fe-ut.sh --run org.apache.doris.analysis.ExprGsonSerializationTest
- Behavior changed: No user-facing SQL syntax or interface changes. Newly written Expr JSON preserves the covered SQL-rendering state.
- Does this need documentation: No
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

### What problem does this PR solve?

Issue Number: None

Related PR: apache#62900

Problem Summary: Expr metadata persistence relies on stable Gson field annotations and semantic round-trip coverage. Add a colocated review guide that makes the durable-field, subtype-registration, sample-quality, and metadata-consumer test requirements explicit for future Expr changes.

### Release note

None

### Check List (For Author)

- Test: No need to test; this commit only adds contributor review guidance.
- Behavior changed: No.
- Does this need documentation: No; this commit is the contributor guide.
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