pg.ddx.io pgsql-committers@postgresql.org mailing list archivehelp / color / mirror / Atom feed
pgsql: Fix mis-deparsing of ORDER BY lists when there is a name conflic 3+ messages / 1 participants [nested] [flat]
* pgsql: Fix mis-deparsing of ORDER BY lists when there is a name conflic @ 2024-08-29 17:24 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 3+ messages in thread From: Tom Lane @ 2024-08-29 17:24 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix mis-deparsing of ORDER BY lists when there is a name conflict. If an ORDER BY item in SELECT is a bare identifier, the parser first seeks it as an output column name of the SELECT (for SQL92 compatibility). However, ruleutils.c is expecting the SQL99 interpretation where such a name is an input column name. So it's possible to produce an incorrect display of a view in the (admittedly pretty ill-advised) case where some other column is renamed in the SELECT output list to match an ORDER BY column. This can be fixed by table-qualifying such names in the dumped view text. To avoid cluttering less-ill-advised queries, we'd like to do so only when there's an actual name conflict. That requires passing the current get_query_def call's resultDesc parameter down to get_variable, so that it can determine what the output column names are. In hopes of reducing rather than increasing notational clutter in ruleutils.c, I moved that value into the deparse_context struct and removed it from the parameter lists of get_query_def's other subroutines. I made a few other cosmetic changes while at it: * Likewise move the colNamesVisible parameter into deparse_context. * Rename deparse_context's windowTList field to targetList, since it's no longer used only in connection with WINDOW clauses. * Replace the special_exprkind field with a bool inGroupBy, since that was all it was being used for, and the apparent flexibility of storing a ParseExprKind proved to be illusory. (We need a separate varInOrderBy field to make this patch work.) * Remove useless save/restore logic in get_select_query_def. In principle, this bug is quite old. However, it seems unreachable before 1b4d280ea, because before that the presence of "new" and "old" entries in a view's rangetable caused us to always table-qualify every Var reference in dumped views. Hence, back-patch to v16 where that came in. Per bug #18589 from Quynh Tran. Discussion: https://postgr.es/m/18589-70091cb81db1a3f1@postgresql.org Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/43f2e7634d838b39fd3fa3aaddb6964392d98312 Modified Files -------------- src/backend/utils/adt/ruleutils.c | 250 ++++++++++++++++++------------ src/test/regress/expected/create_view.out | 51 +++++- src/test/regress/sql/create_view.sql | 16 ++ 3 files changed, 217 insertions(+), 100 deletions(-) ^ permalink raw reply [nested|flat] 3+ messages in thread
* pgsql: Fix mis-deparsing of ORDER BY lists when there is a name conflic @ 2024-08-29 17:24 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 3+ messages in thread From: Tom Lane @ 2024-08-29 17:24 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix mis-deparsing of ORDER BY lists when there is a name conflict. If an ORDER BY item in SELECT is a bare identifier, the parser first seeks it as an output column name of the SELECT (for SQL92 compatibility). However, ruleutils.c is expecting the SQL99 interpretation where such a name is an input column name. So it's possible to produce an incorrect display of a view in the (admittedly pretty ill-advised) case where some other column is renamed in the SELECT output list to match an ORDER BY column. This can be fixed by table-qualifying such names in the dumped view text. To avoid cluttering less-ill-advised queries, we'd like to do so only when there's an actual name conflict. That requires passing the current get_query_def call's resultDesc parameter down to get_variable, so that it can determine what the output column names are. In hopes of reducing rather than increasing notational clutter in ruleutils.c, I moved that value into the deparse_context struct and removed it from the parameter lists of get_query_def's other subroutines. I made a few other cosmetic changes while at it: * Likewise move the colNamesVisible parameter into deparse_context. * Rename deparse_context's windowTList field to targetList, since it's no longer used only in connection with WINDOW clauses. * Replace the special_exprkind field with a bool inGroupBy, since that was all it was being used for, and the apparent flexibility of storing a ParseExprKind proved to be illusory. (We need a separate varInOrderBy field to make this patch work.) * Remove useless save/restore logic in get_select_query_def. In principle, this bug is quite old. However, it seems unreachable before 1b4d280ea, because before that the presence of "new" and "old" entries in a view's rangetable caused us to always table-qualify every Var reference in dumped views. Hence, back-patch to v16 where that came in. Per bug #18589 from Quynh Tran. Discussion: https://postgr.es/m/18589-70091cb81db1a3f1@postgresql.org Branch ------ REL_17_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/a7eb633563c6ba8857cf59c7b43b593614537617 Modified Files -------------- src/backend/utils/adt/ruleutils.c | 250 ++++++++++++++++++------------ src/test/regress/expected/create_view.out | 51 +++++- src/test/regress/sql/create_view.sql | 16 ++ 3 files changed, 217 insertions(+), 100 deletions(-) ^ permalink raw reply [nested|flat] 3+ messages in thread
* pgsql: Fix mis-deparsing of ORDER BY lists when there is a name conflic @ 2024-08-29 17:24 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 3+ messages in thread From: Tom Lane @ 2024-08-29 17:24 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix mis-deparsing of ORDER BY lists when there is a name conflict. If an ORDER BY item in SELECT is a bare identifier, the parser first seeks it as an output column name of the SELECT (for SQL92 compatibility). However, ruleutils.c is expecting the SQL99 interpretation where such a name is an input column name. So it's possible to produce an incorrect display of a view in the (admittedly pretty ill-advised) case where some other column is renamed in the SELECT output list to match an ORDER BY column. This can be fixed by table-qualifying such names in the dumped view text. To avoid cluttering less-ill-advised queries, we'd like to do so only when there's an actual name conflict. That requires passing the current get_query_def call's resultDesc parameter down to get_variable, so that it can determine what the output column names are. In hopes of reducing rather than increasing notational clutter in ruleutils.c, I moved that value into the deparse_context struct and removed it from the parameter lists of get_query_def's other subroutines. I made a few other cosmetic changes while at it: * Likewise move the colNamesVisible parameter into deparse_context. * Rename deparse_context's windowTList field to targetList, since it's no longer used only in connection with WINDOW clauses. * Replace the special_exprkind field with a bool inGroupBy, since that was all it was being used for, and the apparent flexibility of storing a ParseExprKind proved to be illusory. (We need a separate varInOrderBy field to make this patch work.) * Remove useless save/restore logic in get_select_query_def. In principle, this bug is quite old. However, it seems unreachable before 1b4d280ea, because before that the presence of "new" and "old" entries in a view's rangetable caused us to always table-qualify every Var reference in dumped views. Hence, back-patch to v16 where that came in. Per bug #18589 from Quynh Tran. Discussion: https://postgr.es/m/18589-70091cb81db1a3f1@postgresql.org Branch ------ REL_16_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/9fe6319dcc1060c9b65d13f61440bfcfc772803a Modified Files -------------- src/backend/utils/adt/ruleutils.c | 250 ++++++++++++++++++------------ src/test/regress/expected/create_view.out | 51 +++++- src/test/regress/sql/create_view.sql | 16 ++ 3 files changed, 218 insertions(+), 99 deletions(-) ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2024-08-29 17:24 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2024-08-29 17:24 pgsql: Fix mis-deparsing of ORDER BY lists when there is a name conflic Tom Lane <tgl@sss.pgh.pa.us> 2024-08-29 17:24 pgsql: Fix mis-deparsing of ORDER BY lists when there is a name conflic Tom Lane <tgl@sss.pgh.pa.us> 2024-08-29 17:24 pgsql: Fix mis-deparsing of ORDER BY lists when there is a name conflic Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions for how to clone and mirror all data and code used for this inbox