Thread: pgsql: Make parser rely more heavily on the ParseNamespaceItem data str
Make parser rely more heavily on the ParseNamespaceItem data structure. When I added the ParseNamespaceItem data structure (in commit 5ebaaa494), it wasn't very tightly integrated into the parser's APIs. In the wake of adding p_rtindex to that struct (commit b541e9acc), there is a good reason to make more use of it: by passing around ParseNamespaceItem pointers instead of bare RTE pointers, we can get rid of various messy methods for passing back or deducing the rangetable index of an RTE during parsing. Hence, refactor the addRangeTableEntryXXX functions to build and return a ParseNamespaceItem struct, not just the RTE proper; and replace addRTEtoQuery with addNSItemToQuery, which is passed a ParseNamespaceItem rather than building one internally. Also, add per-column data (a ParseNamespaceColumn array) to each ParseNamespaceItem. These arrays are built during addRangeTableEntryXXX, where we have column type data at hand so that it's nearly free to fill the data structure. Later, when we need to build Vars referencing RTEs, we can use the ParseNamespaceColumn info to avoid the rather expensive operations done in get_rte_attribute_type() or expandRTE(). get_rte_attribute_type() is indeed dead code now, so I've removed it. This makes for a useful improvement in parse analysis speed, around 20% in one moderately-complex test query. The ParseNamespaceColumn structs also include Var identity information (varno/varattno). That info isn't actually being used in this patch, except that p_varno == 0 is a handy test for a dropped column. A follow-on patch will make more use of it. Discussion: https://postgr.es/m/2461.1577764221@sss.pgh.pa.us Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/5815696bc66b3092f6361f53e0394909647042c8 Modified Files -------------- src/backend/catalog/heap.c | 16 +- src/backend/commands/copy.c | 10 +- src/backend/commands/policy.c | 63 +-- src/backend/commands/tablecmds.c | 23 +- src/backend/commands/trigger.c | 24 +- src/backend/commands/view.c | 20 +- src/backend/optimizer/plan/subselect.c | 12 +- src/backend/parser/analyze.c | 139 +++-- src/backend/parser/parse_clause.c | 436 +++++++-------- src/backend/parser/parse_coerce.c | 7 +- src/backend/parser/parse_expr.c | 4 +- src/backend/parser/parse_relation.c | 648 +++++++++++++---------- src/backend/parser/parse_target.c | 11 +- src/backend/parser/parse_utilcmd.c | 100 ++-- src/backend/replication/logical/tablesync.c | 4 +- src/backend/rewrite/rewriteHandler.c | 12 +- src/include/parser/parse_node.h | 57 +- src/include/parser/parse_relation.h | 106 ++-- src/include/parser/parsetree.h | 10 +- src/test/modules/test_rls_hooks/test_rls_hooks.c | 19 +- 20 files changed, 927 insertions(+), 794 deletions(-)
Tom Lane <tgl@sss.pgh.pa.us> writes: > Make parser rely more heavily on the ParseNamespaceItem data structure. Hmm, the buildfarm seems to think this screwed up something affecting collations. Looking ... regards, tom lane
Re: pgsql: Make parser rely more heavily on the ParseNamespaceItemdata str
From
Julien Rouhaud
Date:
On Thu, Jan 2, 2020 at 6:41 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Tom Lane <tgl@sss.pgh.pa.us> writes: > > Make parser rely more heavily on the ParseNamespaceItem data structure. > > Hmm, the buildfarm seems to think this screwed up something affecting > collations. Looking ... My animal is using fr_FR collation by default, and https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=lapwing&dt=2020-01-02%2016%3A40%3A21 is reporting: diff -U3 /home/postgres/buildroot/HEAD/pgsql.build/src/test/regress/expected/collate.out /home/postgres/buildroot/HEAD/pgsql.build/src/test/regress/results/collate.out --- /home/postgres/buildroot/HEAD/pgsql.build/src/test/regress/expected/collate.out 2019-06-04 18:32:50.356908034 +0000 +++ /home/postgres/buildroot/HEAD/pgsql.build/src/test/regress/results/collate.out 2020-01-02 17:21:27.696908000 +0000 @@ -540,18 +540,18 @@ SELECT * FROM unnest((SELECT array_agg(b ORDER BY b) FROM collate_test1)) ORDER BY 1; unnest -------- - ABD - Abc abc + Abc + ABD bbc (4 rows) SELECT * FROM unnest((SELECT array_agg(b ORDER BY b) FROM collate_test2)) ORDER BY 1; unnest -------- - ABD - Abc abc + Abc + ABD bbc (4 rows) Is ORDER BY 1 COLLATE "C" an option here?
Julien Rouhaud <rjuju123@gmail.com> writes: > On Thu, Jan 2, 2020 at 6:41 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Hmm, the buildfarm seems to think this screwed up something affecting >> collations. Looking ... > Is ORDER BY 1 COLLATE "C" an option here? Nah, that would just mask the bug not fix it. See 4d02eb017. regards, tom lane
Re: pgsql: Make parser rely more heavily on the ParseNamespaceItemdata str
From
Julien Rouhaud
Date:
On Thu, Jan 2, 2020 at 7:50 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Julien Rouhaud <rjuju123@gmail.com> writes: > > On Thu, Jan 2, 2020 at 6:41 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > >> Hmm, the buildfarm seems to think this screwed up something affecting > >> collations. Looking ... > > > Is ORDER BY 1 COLLATE "C" an option here? > > Nah, that would just mask the bug not fix it. See 4d02eb017. Ah indeed I see, thanks.