Обсуждение: pgsql: Add basic JSON_TABLE() functionality
Add basic JSON_TABLE() functionality JSON_TABLE() allows JSON data to be converted into a relational view and thus used, for example, in a FROM clause, like other tabular data. Data to show in the view is selected from a source JSON object using a JSON path expression to get a sequence of JSON objects that's called a "row pattern", which becomes the source to compute the SQL/JSON values that populate the view's output columns. Column values themselves are computed using JSON path expressions applied to each of the JSON objects comprising the "row pattern", for which the SQL/JSON query functions added in 6185c9737cf4 are used. To implement JSON_TABLE() as a table function, this augments the TableFunc and TableFuncScanState nodes that are currently used to support XMLTABLE() with some JSON_TABLE()-specific fields. Note that the JSON_TABLE() spec includes NESTED COLUMNS and PLAN clauses, which are required to provide more flexibility to extract data out of nested JSON objects, but they are not implemented here to keep this commit of manageable size. Author: Nikita Glukhov <n.gluhov@postgrespro.ru> Author: Teodor Sigaev <teodor@sigaev.ru> Author: Oleg Bartunov <obartunov@gmail.com> Author: Alexander Korotkov <aekorotkov@gmail.com> Author: Andrew Dunstan <andrew@dunslane.net> Author: Amit Langote <amitlangote09@gmail.com> Author: Jian He <jian.universality@gmail.com> Reviewers have included (in no particular order): Andres Freund, Alexander Korotkov, Pavel Stehule, Andrew Alsup, Erik Rijkers, Zihong Yu, Himanshu Upadhyaya, Daniel Gustafsson, Justin Pryzby, Álvaro Herrera, Jian He Discussion: https://postgr.es/m/cd0bb935-0158-78a7-08b5-904886deac4b@postgrespro.ru Discussion: https://postgr.es/m/20220616233130.rparivafipt6doj3@alap3.anarazel.de Discussion: https://postgr.es/m/abd9b83b-aa66-f230-3d6d-734817f0995d%40postgresql.org Discussion: https://postgr.es/m/CA+HiwqE4XTdfb1nW=Ojoy_tQSRhYt-q_kb6i5d4xcKyrLC1Nbg@mail.gmail.com Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/de3600452b61d1bc3967e9e37e86db8956c8f577 Modified Files -------------- doc/src/sgml/func.sgml | 334 +++++++++++ src/backend/commands/explain.c | 21 +- src/backend/executor/execExpr.c | 11 +- src/backend/executor/execExprInterp.c | 2 + src/backend/executor/nodeTableFuncscan.c | 41 +- src/backend/nodes/makefuncs.c | 53 ++ src/backend/nodes/nodeFuncs.c | 36 ++ src/backend/parser/Makefile | 1 + src/backend/parser/gram.y | 173 +++++- src/backend/parser/meson.build | 1 + src/backend/parser/parse_clause.c | 14 +- src/backend/parser/parse_expr.c | 53 +- src/backend/parser/parse_jsontable.c | 421 ++++++++++++++ src/backend/parser/parse_relation.c | 6 +- src/backend/parser/parse_target.c | 1 + src/backend/utils/adt/jsonpath_exec.c | 372 ++++++++++++ src/backend/utils/adt/ruleutils.c | 185 +++++- src/include/nodes/execnodes.h | 2 + src/include/nodes/makefuncs.h | 5 + src/include/nodes/parsenodes.h | 66 +++ src/include/nodes/primnodes.h | 56 +- src/include/parser/kwlist.h | 3 + src/include/parser/parse_clause.h | 3 + src/include/utils/jsonpath.h | 3 + src/interfaces/ecpg/test/ecpg_schedule | 1 + .../ecpg/test/expected/sql-sqljson_jsontable.c | 143 +++++ .../test/expected/sql-sqljson_jsontable.stderr | 16 + .../test/expected/sql-sqljson_jsontable.stdout | 1 + src/interfaces/ecpg/test/sql/Makefile | 1 + src/interfaces/ecpg/test/sql/meson.build | 1 + src/interfaces/ecpg/test/sql/sqljson_jsontable.pgc | 29 + src/test/regress/expected/sqljson_jsontable.out | 636 +++++++++++++++++++++ src/test/regress/parallel_schedule | 2 +- src/test/regress/sql/sqljson_jsontable.sql | 290 ++++++++++ src/tools/pgindent/typedefs.list | 12 + 35 files changed, 2943 insertions(+), 52 deletions(-)
Op 4/4/24 om 13:21 schreef Amit Langote:
> Add basic JSON_TABLE() functionality
>
Great that it's now committed. Congrats!
There is one 'uninitialized' muttering during compile (gcc 13.2.0):
In function ‘transformJsonFuncExpr’,
inlined from ‘transformExprRecurse’ at parse_expr.c:373:13:
parse_expr.c:4317:34: warning: ‘default_format’ may be used
uninitialized [-Wmaybe-uninitialized]
4317 | jsexpr->formatted_expr = transformJsonValueExpr(pstate,
func_name,
|
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4318 |
func->context_item,
|
~~~~~~~~~~~~~~~~~~~
4319 |
default_format,
|
~~~~~~~~~~~~~~~
4320 |
JSONBOID,
|
~~~~~~~~~
4321 |
false);
|
~~~~~~
parse_expr.c: In function ‘transformExprRecurse’:
parse_expr.c:4257:24: note: ‘default_format’ was declared here
4257 | JsonFormatType default_format;
| ^~~~~~~~~~~~~~
Thanks!
Erik
On 2024-04-04 Th 07:21, Amit Langote wrote:
Add basic JSON_TABLE() functionality
Excellent!
But I am getting this:
../../../src/interfaces/ecpg/test/sql/sqljson_jsontable.pgc:23: WARNING: unsupported feature will be passed to server
Is that intended?
cheers
andrew
-- Andrew Dunstan EDB: https://www.enterprisedb.com
Hi Erik On Thu, Apr 4, 2024 at 9:30 PM Erik Rijkers <er@xs4all.nl> wrote: > Op 4/4/24 om 13:21 schreef Amit Langote: > > Add basic JSON_TABLE() functionality > > > > Great that it's now committed. Congrats! > > > There is one 'uninitialized' muttering during compile (gcc 13.2.0): > > In function ‘transformJsonFuncExpr’, > inlined from ‘transformExprRecurse’ at parse_expr.c:373:13: > parse_expr.c:4317:34: warning: ‘default_format’ may be used > uninitialized [-Wmaybe-uninitialized] > 4317 | jsexpr->formatted_expr = transformJsonValueExpr(pstate, > func_name, > | > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > 4318 | > func->context_item, > | > ~~~~~~~~~~~~~~~~~~~ > 4319 | > default_format, > | > ~~~~~~~~~~~~~~~ > 4320 | > JSONBOID, > | > ~~~~~~~~~ > 4321 | > false); > | > ~~~~~~ > parse_expr.c: In function ‘transformExprRecurse’: > parse_expr.c:4257:24: note: ‘default_format’ was declared here > 4257 | JsonFormatType default_format; > | ^~~~~~~~~~~~~~ Thanks for the report. Pushed a fix. -- Thanks, Amit Langote
Hi Andrew, On Thu, Apr 4, 2024 at 10:06 PM Andrew Dunstan <andrew@dunslane.net> wrote: > On 2024-04-04 Th 07:21, Amit Langote wrote: > > Add basic JSON_TABLE() functionality > > > Excellent! > > But I am getting this: > > ../../../src/interfaces/ecpg/test/sql/sqljson_jsontable.pgc:23: WARNING: unsupported feature will be passed to server > > Is that intended? I don't get that WARNING here, but I remember seeing it at one point, which I am supposed to have fixed. Can you please share more details about where you're seeing it? -- Thanks, Amit Langote
On 2024-04-04 Th 09:20, Amit Langote wrote:
Hi Andrew, On Thu, Apr 4, 2024 at 10:06 PM Andrew Dunstan <andrew@dunslane.net> wrote:On 2024-04-04 Th 07:21, Amit Langote wrote: Add basic JSON_TABLE() functionality Excellent! But I am getting this: ../../../src/interfaces/ecpg/test/sql/sqljson_jsontable.pgc:23: WARNING: unsupported feature will be passed to server Is that intended?I don't get that WARNING here, but I remember seeing it at one point, which I am supposed to have fixed. Can you please share more details about where you're seeing it?
It's with a meson build:
[2330/2355] /home/andrew/pgl/pg_head/root/HEAD/pgsql.build/src/interfaces/ecpg/preproc/ecpg --regression -I../../../src/interfaces/ecpg/test/sql -I../../../src/interfaces/ecpg/
include/ -o src/interfaces/ecpg/test/sql/sqljson_jsontable.c ../../../src/interfaces/ecpg/test/sql/sqljson_jsontable.pgc
../../../src/interfaces/ecpg/test/sql/sqljson_jsontable.pgc:23: WARNING: unsupported feature will be passed to server
cheers
andrew
-- Andrew Dunstan EDB: https://www.enterprisedb.com
Andrew Dunstan <andrew@dunslane.net> writes:
> On 2024-04-04 Th 09:20, Amit Langote wrote:
>> I don't get that WARNING here, but I remember seeing it at one point,
>> which I am supposed to have fixed.
>>
>> Can you please share more details about where you're seeing it?
> It's with a meson build:
I see it with autoconf builds too, both on RHEL8 and macOS.
Looking at parse.pl, I guess it's probably triggered by some
unconventional use of ERRCODE_FEATURE_NOT_SUPPORTED in your
gram.y changes.
regards, tom lane
I wrote:
> Looking at parse.pl, I guess it's probably triggered by some
> unconventional use of ERRCODE_FEATURE_NOT_SUPPORTED in your
> gram.y changes.
Oh, no, it's parse.pl that is at fault. It's doing this:
if (/ERRCODE_FEATURE_NOT_SUPPORTED/)
{
$feature_not_supported = 1;
next line;
}
... later ...
if ($feature_not_supported == 1)
{
# we found an unsupported feature, but we have to
# filter out ExecuteStmt: CREATE OptTemp TABLE ...
# because the warning there is only valid in some situations
if ($flds->[0] ne 'create' || $flds->[2] ne 'table')
{
add_to_buffer('rules',
'mmerror(PARSE_ERROR, ET_WARNING, "unsupported feature will be passed to server");'
);
}
$feature_not_supported = 0;
}
In English, if a production in gram.y contains the string
ERRCODE_FEATURE_NOT_SUPPORTED anywhere, then the ecpg parser
will be made to spit out this warning when it reduces that
same production.
This works great for cases like
| UNENCRYPTED PASSWORD Sconst
{
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("UNENCRYPTED PASSWORD is no longer supported"),
errhint("Remove UNENCRYPTED to store the password in encrypted form instead."),
parser_errposition(@1)));
}
but not well at all if the error is conditional, as it is in the
json_table production. The cowboy hack for CREATE OptTemp TABLE
was evidently meant to suppress what must have been the only
conditional FEATURE_NOT_SUPPORTED rule at the time, but it doesn't
look like that matches anything at all anymore. If you look through
the generated preproc.y file you'll find quite a few of these that
are being reported inappropriately; I'm surprised that it hasn't
come up before.
We need to either nuke that ecpg warning entirely, or find a more
robust way of detecting whether the gram.y complaint is conditional.
I'll put a brown paper bag on my head before suggesting that maybe
we could pay attention to how far the errcode is indented. Perhaps
a slightly nicer way is to assume it's conditional if any earlier
line in the rule starts with "if".
regards, tom lane
I wrote:
> We need to either nuke that ecpg warning entirely, or find a more
> robust way of detecting whether the gram.y complaint is conditional.
> I'll put a brown paper bag on my head before suggesting that maybe
> we could pay attention to how far the errcode is indented. Perhaps
> a slightly nicer way is to assume it's conditional if any earlier
> line in the rule starts with "if".
The latter seems considerably safer, so I modified parse.pl along
those lines. Eyeball comparison of gram.y with preproc.y verifies
that now ecpg will emit the warning only in rules where the backend
unconditionally reports FEATURE_NOT_SUPPORTED.
I suppose this needs to be back-patched.
regards, tom lane
diff --git a/src/interfaces/ecpg/preproc/parse.pl b/src/interfaces/ecpg/preproc/parse.pl
index 87b512b2a1..fe8d3e5178 100644
--- a/src/interfaces/ecpg/preproc/parse.pl
+++ b/src/interfaces/ecpg/preproc/parse.pl
@@ -34,7 +34,8 @@ my $brace_indent = 0;
my $yaccmode = 0;
my $in_rule = 0;
my $header_included = 0;
-my $feature_not_supported = 0;
+my $has_feature_not_supported = 0;
+my $has_if_command = 0;
my $tokenmode = 0;
my (%buff, $infield, $comment, %tokens, %addons);
@@ -151,12 +152,6 @@ sub main
{
line: while (<$parserfh>)
{
- if (/ERRCODE_FEATURE_NOT_SUPPORTED/)
- {
- $feature_not_supported = 1;
- next line;
- }
-
chomp;
# comment out the line below to make the result file match (blank line wise)
@@ -182,6 +177,13 @@ sub main
$infield = 0;
}
+ if ($yaccmode == 1)
+ {
+ # Check for rules that throw FEATURE_NOT_SUPPORTED
+ $has_feature_not_supported = 1 if /ERRCODE_FEATURE_NOT_SUPPORTED/;
+ $has_if_command = 1 if /^\s*if/;
+ }
+
my $prec = 0;
# Make sure any braces are split
@@ -541,20 +543,17 @@ sub dump_fields
#Normal
add_to_buffer('rules', $ln);
- if ($feature_not_supported == 1)
+ if ($has_feature_not_supported and not $has_if_command)
{
-
- # we found an unsupported feature, but we have to
- # filter out ExecuteStmt: CREATE OptTemp TABLE ...
- # because the warning there is only valid in some situations
- if ($flds->[0] ne 'create' || $flds->[2] ne 'table')
- {
- add_to_buffer('rules',
- 'mmerror(PARSE_ERROR, ET_WARNING, "unsupported feature will be passed to server");'
- );
- }
- $feature_not_supported = 0;
+ # The backend unconditionally reports
+ # FEATURE_NOT_SUPPORTED in this rule, so let's emit
+ # a warning on the ecpg side.
+ add_to_buffer('rules',
+ 'mmerror(PARSE_ERROR, ET_WARNING, "unsupported feature will be passed to server");'
+ );
}
+ $has_feature_not_supported = 0;
+ $has_if_command = 0;
if ($len == 0)
{
On Fri, Apr 5, 2024 at 1:55 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > I wrote: > > We need to either nuke that ecpg warning entirely, or find a more > > robust way of detecting whether the gram.y complaint is conditional. > > I'll put a brown paper bag on my head before suggesting that maybe > > we could pay attention to how far the errcode is indented. Perhaps > > a slightly nicer way is to assume it's conditional if any earlier > > line in the rule starts with "if". > > The latter seems considerably safer, so I modified parse.pl along > those lines. Eyeball comparison of gram.y with preproc.y verifies > that now ecpg will emit the warning only in rules where the backend > unconditionally reports FEATURE_NOT_SUPPORTED. > > I suppose this needs to be back-patched. Thanks for taking care of this and the missing .gitignore entries as well. Just to make a correction to what I said upthread in reply to Andrew, it seems like I did get the WARNING but missed it. Wonder if, for consistency, I should change the coding so that the FEATURE_NOT_SUPPORTED is reported in transformJsonTable() or something? -- Thanks, Amit Langote
Amit Langote <amitlangote09@gmail.com> writes:
> Wonder if, for consistency, I should change the coding so that the
> FEATURE_NOT_SUPPORTED is reported in transformJsonTable() or
> something?
Consistency with what? There are plenty of other gram.y productions
that throw FEATURE_NOT_SUPPORTED conditionally.
regards, tom lane
On Fri, Apr 5, 2024 at 11:56 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Amit Langote <amitlangote09@gmail.com> writes: > > Wonder if, for consistency, I should change the coding so that the > > FEATURE_NOT_SUPPORTED is reported in transformJsonTable() or > > something? > > Consistency with what? There are plenty of other gram.y productions > that throw FEATURE_NOT_SUPPORTED conditionally. Ah, yes, I see. I thought maybe not when I read "unconventional use of" in your first email. -- Thanks, Amit Langote
Amit Langote <amitlangote09@gmail.com> writes:
> On Fri, Apr 5, 2024 at 11:56 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Consistency with what? There are plenty of other gram.y productions
>> that throw FEATURE_NOT_SUPPORTED conditionally.
> Ah, yes, I see. I thought maybe not when I read "unconventional use
> of" in your first email.
That was my first guess before I'd looked at any of the code,
but really the json_table production isn't doing anything
unusual.
regards, tom lane