Horton

Re: remaining sql/json patches

Peter Eisentraut <peter@eisentraut.org>

View thread
From: Peter Eisentraut <peter@eisentraut.org>
To: Amit Langote <amitlangote09@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Andres Freund <andres@anarazel.de>, Andrew Dunstan <andrew@dunslane.net>, PostgreSQL-development <pgsql-hackers@postgresql.org>, jian he <jian.universality@gmail.com>, Erik Rijkers <er@xs4all.nl>
Date: 2023-11-21T07:09:17Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. SQL/JSON: Various improvements to SQL/JSON query function docs

  2. SQL/JSON: Fix some obsolete comments.

  3. SQL/JSON: Fix issues with DEFAULT .. ON ERROR / EMPTY

  4. JSON_TABLE: Add support for NESTED paths and columns

  5. Fix JsonExpr deparsing to emit QUOTES and WRAPPER correctly

  6. Fix typo introduced in 6185c9737

  7. Add basic JSON_TABLE() functionality

  8. Avoid splitting errmsg string to span multiple lines

  9. Add SQL/JSON query functions

  10. Implement various jsonpath methods

  11. Add soft error handling to some expression nodes

  12. Adjust populate_record_field() to handle errors softly

  13. Refactor code used by jsonpath executor to fetch variables

  14. Test EXPLAIN (FORMAT JSON) ... XMLTABLE

  15. Simplify productions for FORMAT JSON [ ENCODING name ]

  16. Add trailing commas to enum definitions

  17. doc: add missing <returnvalue> and whitespace

  18. Add more SQL/JSON constructor functions

  19. Rename a nonterminal used in SQL/JSON grammar

  20. Some refactoring to export json(b) conversion functions

  21. Don't include CaseTestExpr in JsonValueExpr.formatted_expr

  22. Code review for commit b6e1157e7d

  23. Pass constructName to transformJsonValueExpr()

  24. Unify JSON categorize type API and export for external use

  25. Make some indentation in gram.y consistent

  26. Allow most keywords to be used as column labels without requiring AS.

  27. Reduce size of backend scanner's tables.

  28. Use perfect hashing, instead of binary search, for keyword lookup.

Attachments

I looked a bit at the parser additions, because there were some concerns 
expressed that they are quite big.

It looks like the parser rules were mostly literally copied from the BNF 
in the SQL standard.  That's probably a reasonable place to start, but 
now at the end, there is some room for simplification.

Attached are a few patches that apply on top of the 0003 patch.  (I 
haven't gotten to 0004 in detail yet.)  Some explanations:

0001-Put-keywords-in-right-order.patch

This is just an unrelated cleanup.

0002-Remove-js_quotes-union-entry.patch

We usually don't want to put every single node type into the gram.y 
%union.  This one can be trivially removed.

0003-Move-some-code-from-gram.y-to-parse-analysis.patch

Code like this can be postponed to parse analysis, keeping gram.y 
smaller.  The error pointer loses a bit of precision, but I think that's 
ok.  (There is similar code in your 0004 patch, which could be similarly 
moved.)

0004-Remove-JsonBehavior-stuff-from-union.patch

Similar to my 0002.  This adds a few casts as a result, but that is the 
typical style in gram.y.

0005-Get-rid-of-JsonBehaviorClause.patch

I think this two-level wrapping of the behavior clauses is both 
confusing and overkill.  I was trying to just list the on-empty and 
on-error clauses separately in the top-level productions (JSON_VALUE 
etc.), but that led to shift/reduce errors. So the existing rule 
structure is probably ok.  But we don't need a separate node type just 
to combine two values and then unpack them again shortly thereafter.  So 
I just replaced all this with a list.

0006-Get-rid-of-JsonCommon.patch

This is an example where the SQL standard BNF is not sensible to apply 
literally.  I moved those clauses up directly into their callers, thus 
removing one intermediate levels of rules and also nodes.  Also, the 
path name (AS name) stuff is only for JSON_TABLE, so it's not needed in 
this patch.  I removed it here, but it would have to be readded in your 
0004 patch.

Another thing: In your patch, JSON_EXISTS has a RETURNING clause 
(json_returning_clause_opt), but I don't see that in the standard, and 
also not in the Oracle or Db2 docs.  Where did this come from?

With these changes, I think the grammar complexity in your 0003 patch is 
at an acceptable level.  Similar simplification opportunities exist in 
the 0004 patch, but I haven't worked on that yet.  I suggest that you 
focus on getting 0001..0003 committed around this commit fest and then 
deal with 0004 in the next one.  (Also split up the 0005 patch into the 
pieces that apply to 0003 and 0004, respectively.)