Re: remaining sql/json patches
От | Amit Langote |
---|---|
Тема | Re: remaining sql/json patches |
Дата | |
Msg-id | CA+HiwqGT9X4VcTuiM2s0By+R6V31FtTXd2XpofM5o+LWss9-eQ@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: remaining sql/json patches (jian he <jian.universality@gmail.com>) |
Ответы |
Re: remaining sql/json patches
Re: remaining sql/json patches |
Список | pgsql-hackers |
On Wed, Apr 3, 2024 at 4:16 PM jian he <jian.universality@gmail.com> wrote: > On Wed, Apr 3, 2024 at 11:30 AM jian he <jian.universality@gmail.com> wrote: > > > > On Tue, Apr 2, 2024 at 9:57 PM Amit Langote <amitlangote09@gmail.com> wrote: > > > > > > Please let me know if you have further comments on 0001. I'd like to > > > get that in before spending more energy on 0002. > > > > > -- a/src/backend/parser/parse_target.c > +++ b/src/backend/parser/parse_target.c > @@ -2019,6 +2019,9 @@ FigureColnameInternal(Node *node, char **name) > case JSON_VALUE_OP: > *name = "json_value"; > return 2; > + case JSON_TABLE_OP: > + *name = "json_table"; > + return 2; > default: > elog(ERROR, "unrecognized JsonExpr op: %d", > (int) ((JsonFuncExpr *) node)->op); > > "case JSON_TABLE_OP part", no need? > json_table output must provide column name and type? That seems to be the case, so removed. > I did some minor refactor transformJsonTableColumns, make the comments > align with the function intention. Thanks, but that seems a bit verbose. I've reduced it down to what gives enough information. > in v48-0001, in transformJsonTableColumns we can `Assert(rawc->name);`. > since non-nested JsonTableColumn must specify column name. > in v48-0002, we can change to `if (rawc->coltype != JTC_NESTED) > Assert(rawc->name);` Ok, done. > SELECT * FROM JSON_TABLE(jsonb '1', '$' COLUMNS (a int PATH '$.a' ) > ERROR ON ERROR) jt; > ERROR: no SQL/JSON item > > I thought it should just return NULL. > In this case, I thought that > (not column-level) ERROR ON ERROR should not interfere with "COLUMNS > (a int PATH '$.a' )". I think it does in another database's implementation, which must be why the original authors decided that the table-level ERROR should also be used for columns unless overridden. But I agree that keeping the two separate is better, so changed that way. Attached updated patches. I have addressed your doc comments on 0001, but not 0002 yet. > > +-- Other miscellanous checks > "miscellanous" should be "miscellaneous". > > > overall the coverage is pretty high. > the current test output looks fine. -- Thanks, Amit Langote
Вложения
В списке pgsql-hackers по дате отправления: