Skip to content

feat: add function rewrite rule for json_get with cast#7631

Merged
sunng87 merged 9 commits intoGreptimeTeam:mainfrom
sunng87:feature/function-expr-rewrite
Mar 9, 2026
Merged

feat: add function rewrite rule for json_get with cast#7631
sunng87 merged 9 commits intoGreptimeTeam:mainfrom
sunng87:feature/function-expr-rewrite

Conversation

@sunng87
Copy link
Copy Markdown
Member

@sunng87 sunng87 commented Jan 28, 2026

I hereby agree to the terms of the GreptimeDB CLA.

Refer to a related PR or issue link (optional)

What's changed and what's your intention?

This patch provides support for sql like

SELECT json_get(parse_json('{"a":1}'), 'a')::text;

SELECT json_get(parse_json('{"a":1}'), 'a')::int8;

It also changes the third argument of json_get to accept value type instead of type name.

Note that we have a sql transform rewrite type_alias that rewrites ::int8 to arrow_cast function calls. So we will deal with both arrow_cast function call and cast expression here.

PR Checklist

Please convert it to a draft if some of the following conditions are not met.

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.
  • This PR requires documentation updates.
  • API changes are backward compatible.
  • Schema or data changes are backward compatible.

@github-actions github-actions bot added size/S docs-not-required This change does not impact docs. labels Jan 28, 2026
@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello @sunng87, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the SQL query engine's capabilities by introducing a robust function rewrite framework. This framework is immediately leveraged to provide more intuitive and powerful type casting for the json_get function, allowing users to directly specify the desired return type using standard SQL cast syntax. This change streamlines JSON data extraction and type conversion, making queries more concise and user-friendly.

Highlights

  • Function Rewrite Mechanism: Introduced a new FunctionRewrite mechanism within the FunctionRegistry to allow for dynamic modification of expressions in the query plan. This enables more flexible and powerful SQL syntax transformations.
  • JSON_GET Type Casting Support: Implemented a JsonGetRewriter that specifically handles json_get function calls followed by a type cast (e.g., json_get(..., 'path')::int). This rewriter transforms the expression json_get(expr, path)::type into json_get(expr, path, NULL::type).
  • Enhanced JSON_GET Function: The json_get function's JsonGetWithType variant was updated to interpret this new third argument (NULL::type) to infer the desired return type, simplifying type conversion for JSON extraction.
  • Query Optimizer Integration: The ApplyFunctionRewrites analyzer rule was integrated into the query planning and optimization pipeline, ensuring that registered function rewrite rules, like the JsonGetRewriter, are applied during logical plan analysis.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a function rewrite rule to support CAST operations on the json_get function, allowing for a more natural SQL syntax like json_get(...)::type. The implementation involves creating a FunctionRewrite rule, registering it with the query engine, and modifying the json_get function to handle the rewritten expression.

My review has found a critical issue where the new functionality appears to be broken in the provided tests. Additionally, I've identified a potential robustness issue in the new rewrite rule. Please see my detailed comments.

@sunng87 sunng87 marked this pull request as ready for review January 29, 2026 14:52
@sunng87 sunng87 requested review from a team, discord9, evenyag and waynexia as code owners January 29, 2026 14:52
@github-actions github-actions bot added size/M and removed size/S labels Jan 29, 2026
@waynexia
Copy link
Copy Markdown
Member

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9ef77cd07f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@sunng87 sunng87 marked this pull request as draft January 30, 2026 03:04
@sunng87
Copy link
Copy Markdown
Member Author

sunng87 commented Jan 30, 2026

Convert to draft for now. I'm going to remove the arrow_cast trasformation in type_alias.rs if possible. That will simplify the rewrite quite a bit.

Update: not removable. We still need arrow_cast for timestamps

@sunng87 sunng87 marked this pull request as ready for review February 2, 2026 03:51
@github-actions github-actions bot added docs-required This change requires docs update. and removed docs-not-required This change does not impact docs. labels Feb 6, 2026
@killme2008
Copy link
Copy Markdown
Member

killme2008 commented Feb 25, 2026

@sunng87 I think this feature could be implemented as a TransformRule:

pub(crate) trait TransformRule: Send + Sync {

Similar to how type aliasing is handled:

https://github.com/GreptimeTeam/greptimedb/tree/main/src/sql/src/statements/transform

It doesn't need to be implemented as an analyzer rule.

Co-authored-by: Ruihang Xia <waynestxia@gmail.com>
@sunng87
Copy link
Copy Markdown
Member Author

sunng87 commented Feb 25, 2026

@killme2008 Yes it's possible. This change is to use datafusion function rewriter API: https://docs.rs/datafusion/latest/datafusion/logical_expr/expr_rewriter/trait.FunctionRewrite.html and make this API work on our implementation.

@sunng87 sunng87 enabled auto-merge February 25, 2026 06:12
@killme2008
Copy link
Copy Markdown
Member

@sunng87 Conflicts must be resolved.

@sunng87 sunng87 added this pull request to the merge queue Mar 9, 2026
Merged via the queue into GreptimeTeam:main with commit 9b1288f Mar 9, 2026
45 checks passed
@sunng87 sunng87 deleted the feature/function-expr-rewrite branch March 9, 2026 03:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs-required This change requires docs update. size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants