Re: remaining sql/json patches

From: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
To: Amit Langote <amitlangote09(at)gmail(dot)com>
Cc: Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, Erik Rijkers <er(at)xs4all(dot)nl>, jian he <jian(dot)universality(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: remaining sql/json patches
Date: 2024-03-12 20:47:35
Message-ID: 202403122047.avfepoweeadk@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

About 0002:

I think we should just drop it. Look at the changes it produces in the
plans for aliases XMLTABLE:

> @@ -1556,7 +1556,7 @@ SELECT f.* FROM xmldata, LATERAL xmltable('/ROWS/ROW[COUNTRY_NAME="Japan" or COU
> Output: f."COUNTRY_NAME", f."REGION_ID"
> -> Seq Scan on public.xmldata
> Output: xmldata.data
> - -> Table Function Scan on "xmltable" f
> + -> Table Function Scan on "XMLTABLE" f
> Output: f."COUNTRY_NAME", f."REGION_ID"
> Table Function Call: XMLTABLE(('/ROWS/ROW[COUNTRY_NAME="Japan" or COUNTRY_NAME="India"]'::text) PASSING (xmldata.data) COLUMNS "COUNTRY_NAME" text, "REGION_ID" integer)
> Filter: (f."COUNTRY_NAME" = 'Japan'::text)

Here in text-format EXPLAIN, we already have the alias next to the
"xmltable" moniker, when an alias is present. This matches the
query itself as well as the labels used in the "Output:" display.
If an alias is not present, then this says just 'Table Function Scan on "xmltable"'
and the rest of the plans refers to this as "xmltable", so it's also
fine.

> @@ -1591,7 +1591,7 @@ SELECT f.* FROM xmldata, LATERAL xmltable('/ROWS/ROW[COUNTRY_NAME="Japan" or COU
> "Parent Relationship": "Inner", +
> "Parallel Aware": false, +
> "Async Capable": false, +
> - "Table Function Name": "xmltable", +
> + "Table Function Name": "XMLTABLE", +
> "Alias": "f", +
> "Output": ["f.\"COUNTRY_NAME\"", "f.\"REGION_ID\""], +
> "Table Function Call": "XMLTABLE(('/ROWS/ROW[COUNTRY_NAME=\"Japan\" or COUNTRY_NAME=\"India\"]'::text) PASSING (xmldata.data) COLUMNS \"COUNTRY_NAME\" text, \"REGION_ID\" integer)",+

This is the JSON-format explain. Notice that the "Alias" member already
shows the alias "f", so the only thing this change is doing is
uppercasing the "xmltable" to "XMLTABLE". We're not really achieving
anything here.

I think the only salvageable piece from this, **if anything**, is making
the "xmltable" literal string into uppercase. That might bring a little
clarity to the fact that this is a keyword and not a user-introduced
name.

In your 0003 I think this would only have relevance in this query,

+-- JSON_TABLE() with alias
+EXPLAIN (COSTS OFF, VERBOSE)
+SELECT * FROM
+ JSON_TABLE(
+ jsonb 'null', 'lax $[*]' PASSING 1 + 2 AS a, json '"foo"' AS "b c"
+ COLUMNS (
+ id FOR ORDINALITY,
+ "int" int PATH '$',
+ "text" text PATH '$'
+ )) json_table_func;
+ QUERY PLAN

+--------------------------------------------------------------------------------------------------------------------------------------------------------------
----------------------------------------------------------
+ Table Function Scan on "JSON_TABLE" json_table_func
+ Output: id, "int", text
+ Table Function Call: JSON_TABLE('null'::jsonb, '$[*]' AS json_table_path_0 PASSING 3 AS a, '"foo"'::jsonb AS "b c" COLUMNS (id FOR ORDINALITY, "int" integer PATH '$', text text PATH '$') PLAN (json_table_path_0))
+(3 rows)

and I'm curious to see what this would output if this was to be run
without the 0002 patch. If I understand things correctly, the alias
would be displayed anyway, meaning 0002 doesn't get us anything.

Please do add a test with EXPLAIN (FORMAT JSON) in 0003.

Thanks

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"La vida es para el que se aventura"

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2024-03-12 20:55:22 Re: On disable_cost
Previous Message David Rowley 2024-03-12 20:35:03 Re: typo in paths.h