Skip to content
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

Support json_build_object #179

Closed
swennemans opened this issue Aug 12, 2023 · 6 comments · Fixed by #193
Closed

Support json_build_object #179

swennemans opened this issue Aug 12, 2023 · 6 comments · Fixed by #193

Comments

@swennemans
Copy link

First off, I must say I genuinely enjoy using safeql. It's been a great asset to my projects, and I truly appreciate the hard work behind it. 🙌

While working with it recently, I stumbled upon a minor hiccup concerning type inference when using the json_build_object Postgres function. This function can be especially useful when using in tandem with array_agg.

Scenario

Let's consider these tables for context:

CREATE TABLE users (
  id SERIAL PRIMARY KEY,
  name TEXT NOT NULL
);

CREATE TABLE posts (
  id SERIAL PRIMARY KEY,
  user_id INTEGER REFERENCES users(id),
  title TEXT NOT NULL,
  content TEXT
);

With a goal to fetch user details along with their post details in a structured manner, I used the following query:

SELECT
    u.id AS user_id,
    u.name,
    array_agg(json_build_object('post_id', p.id, 'title', p.title)) AS post_details
FROM
    users u
JOIN
    posts p ON u.id = p.user_id
GROUP BY
    u.id,
    u.name;

Describe the solution you'd like

  • Actual

{ user_id: number | null; name: string | null; post_details: any[] | null }

  • Expected

{ user_id: number | null; name: string | null; post_details: { post_id: number, title: string }[] | null }

Describe alternatives you've considered

A workaround for getting the correct types on arrays can be by using multiple arrays and combine the data based on the array index.

SELECT
    u.id AS user_id,
    u.name,
    array_agg(p.id) AS post_ids,
    array_agg(p.title) AS post_titles
FROM
    users u
    JOIN posts p ON u.id = p.user_id
GROUP BY
    u.id,
    u.name;

returns the following type:

{
  user_id: number | null;
  name: string | null;
  post_ids: number[] | null;
  post_titles: string[] | null;
}[]

Nevertheless, incorporating json_build_object directly would be a significant enhancement.

Thank you for your time, and have a nice day!

@swennemans swennemans changed the title Type inference issue with json_build_object in SQL query Support json_build_object Aug 12, 2023
@Newbie012
Copy link
Collaborator

Currently, SafeQL mainly relies on PostgreSQL's RowDescription which explains the query structure without executing it.

PostgreSQL idiomatically infers the post_details row as json (which SafeQL output as any but is configurable).

Due to that, SafeQL is unable to infer the exact result of json_build_object.

I'll keep this open since I do plan eventually to support these kinds of limitations by moving away from RowDescription.

@karlhorky
Copy link
Collaborator

Nice, that sounds pretty good for the future plan 🙌 Potentially also relevant: json_agg as mentioned in this issue:

@swennemans
Copy link
Author

Awesome, thanks for the reply and thoughtful explanation :)

@karlhorky
Copy link
Collaborator

karlhorky commented Nov 30, 2023

@Newbie012 for the future design, one additional idea that I came across while writing my SQL tooling thread on Twitter/X was how @kristiandupont handles it in Kanel:

You can specify the type using a column comment. So if you have a table user with a column address that is JSON(B), you can run

COMMENT ON COLUMN user.address IS '@type:string ..and the rest of your comment, if any';

and now Kanel should turn it into a string. If you want to specify a type that needs to be imported, you can use @type(MyType, 'my-library', false, true, false)

Source: kristiandupont/kanel#429 (comment)

Sounds like it could be nice, to be able to specify the type information in the database 👍

@Newbie012
Copy link
Collaborator

Update - #190 (comment)

@Newbie012
Copy link
Collaborator

fixed in 3.0.0

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 a pull request may close this issue.

3 participants