-
Notifications
You must be signed in to change notification settings - Fork 592
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(expr): add jsonb_populate_record(set)
function
#13421
Conversation
Signed-off-by: Runji Wang <[email protected]>
Signed-off-by: Runji Wang <[email protected]>
Signed-off-by: Runji Wang <[email protected]>
Signed-off-by: Runji Wang <[email protected]>
Regarding of the remaining issue: I think it's okay to keep the status quo i.e. do not support this usage now. I can't see any real benefit or use cases. What do you think? cc. @xiangjinwu Did some quick experiments on PG and here is the result:
While operators can't
|
|
Its major usage is to convert from a loosely typed
I will give an example on how it could help if we decided to use
It is true that we can do That being said, it is always possible to implement a functionality that can only be used in limited cases, and then improved in future iterations. |
Signed-off-by: Runji Wang <[email protected]>
This PR has been open for 60 days with no activity. Could you please update the status? Feel free to ping a reviewer if you are waiting for review. |
Signed-off-by: Runji Wang <[email protected]>
|
GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
---|---|---|---|---|---|
9425213 | Triggered | Generic Password | 505790f | ci/scripts/regress-test.sh | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
Our GitHub checks need improvements? Share your feedbacks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
license-eye has totally checked 5010 files.
Valid | Invalid | Ignored | Fixed |
---|---|---|---|
2150 | 1 | 2859 | 0 |
Click to see the invalid file list
- src/expr/impl/src/scalar/jsonb_record.rs
Signed-off-by: Runji Wang <[email protected]>
Signed-off-by: Runji Wang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: Runji Wang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wondering the following:
- How is frontend agg name
jsonb_populate_recordset
mapped to the proto enum, by the macro? - How does frontend infer the return type is same as first argument for
jsonb_populate_record
?
by the risingwave/src/prost/src/lib.rs Lines 171 to 177 in 86ac793
The risingwave/src/expr/macro/src/gen.rs Lines 113 to 117 in 86ac793
and frontend will call this function at the end of
|
Signed-off-by: Runji Wang <[email protected]>
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
This PR adds the
jsonb_populate_record
function and its set-returning form:To support
jsonb_populate_recordset
, some improvements have been made to the#[function]
macro, in order to supportOption
inputs and theContext
argument for table functions.Remaining issue
Calling the scalar function
jsonb_populate_record
in the FROM clause triggers a panic in the planner.Although calling it in the SELECT list is normal, it seems that the above form is more commonly used.
This PR also implements a similar function
jsonb_to_record(set)
on the backend. However, they are not enabled due to lack of frontend support.These functions are designed to be called in the FROM clause, with type defined by an AS clause.
It seems that we don't support such usage in both parser and frontend.
Checklist
./risedev check
(or alias,./risedev c
)Documentation
Release note
Add 2 functions:
See postgres documents for detailed descriptions.
Note that we have different usages compared to pg.
In Postgres, users need to define a composite type using
CREATE TYPE
before calling these functions.In RisingWave, users should use inline
struct
type instead.