pgsql: Fix a bug in how we generate partition constraints.
Fix a bug in how we generate partition constraints.
Move the code for doing parent attnos to child attnos mapping for Vars
in partition constraint expressions to a separate function
map_partition_varattnos() and call it from the appropriate places.
Doing it in get_qual_from_partbound(), as is now, would produce wrong
result in certain multi-level partitioning cases, because it only
considers the current pair of parent-child relations. In certain
multi-level partitioning cases, attnums for the same key attribute(s)
might differ between various levels causing the same attribute to be
numbered differently in different instances of the Var corresponding
to a given attribute.
With this commit, in generate_partition_qual(), we first generate the
the whole partition constraint (considering all levels of partitioning)
and then do the mapping, so that Vars in the final expression are
numbered according the leaf relation (to which it is supposed to apply).
Amit Langote, reviewed by me.
Branch
------
master
Details
-------
http://git.postgresql.org/pg/commitdiff/0563a3a8b59150bf3cc8b2b7077f684e0eaf8aff
Modified Files
--------------
src/backend/catalog/partition.c | 99 +++++++++++++++----------------
src/backend/commands/tablecmds.c | 7 ++-
src/include/catalog/partition.h | 1 +
src/test/regress/expected/alter_table.out | 30 ++++++++++
src/test/regress/sql/alter_table.sql | 25 ++++++++
5 files changed, 110 insertions(+), 52 deletions(-)
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
Robert Haas wrote:
Fix a bug in how we generate partition constraints.
Move the code for doing parent attnos to child attnos mapping for Vars
in partition constraint expressions to a separate function
map_partition_varattnos() and call it from the appropriate places.
Hmm, we were discussing this stuff a few days ago, see
/messages/by-id/20170109182800.qrkae62kmur3gfeg@alvherre.pgsql
and commit 3957b58b8885441c8d03bc1cfc00e47cf8cd7975. Part of this code
duplicates that ...
--
Ãlvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Jan 13, 2017 at 2:18 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
Robert Haas wrote:
Fix a bug in how we generate partition constraints.
Move the code for doing parent attnos to child attnos mapping for Vars
in partition constraint expressions to a separate function
map_partition_varattnos() and call it from the appropriate places.Hmm, we were discussing this stuff a few days ago, see
/messages/by-id/20170109182800.qrkae62kmur3gfeg@alvherre.pgsql
and commit 3957b58b8885441c8d03bc1cfc00e47cf8cd7975. Part of this code
duplicates that ...
Is that bad?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Jan 13, 2017 at 2:31 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Fri, Jan 13, 2017 at 2:18 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:Robert Haas wrote:
Fix a bug in how we generate partition constraints.
Move the code for doing parent attnos to child attnos mapping for Vars
in partition constraint expressions to a separate function
map_partition_varattnos() and call it from the appropriate places.Hmm, we were discussing this stuff a few days ago, see
/messages/by-id/20170109182800.qrkae62kmur3gfeg@alvherre.pgsql
and commit 3957b58b8885441c8d03bc1cfc00e47cf8cd7975. Part of this code
duplicates that ...Is that bad?
If you are expressing a concern about who wrote this code, I took
Amit's word for it that he did. His patch file says:
Reported by: n/a
Patch by: Amit Langote
Reports: n/a
If that ain't right, that's bad.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Robert Haas wrote:
On Fri, Jan 13, 2017 at 2:31 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Fri, Jan 13, 2017 at 2:18 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
Hmm, we were discussing this stuff a few days ago, see
/messages/by-id/20170109182800.qrkae62kmur3gfeg@alvherre.pgsql
and commit 3957b58b8885441c8d03bc1cfc00e47cf8cd7975. Part of this code
duplicates that ...Is that bad?
If you are expressing a concern about who wrote this code, I took
Amit's word for it that he did.
I'm just saying that the problem at hand is already solved for a related
feature, so ISTM this new code should use the recently added routine
rather than doing the same thing in a different way.
--
Ãlvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Jan 13, 2017 at 3:09 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
Robert Haas wrote:
On Fri, Jan 13, 2017 at 2:31 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Fri, Jan 13, 2017 at 2:18 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:Hmm, we were discussing this stuff a few days ago, see
/messages/by-id/20170109182800.qrkae62kmur3gfeg@alvherre.pgsql
and commit 3957b58b8885441c8d03bc1cfc00e47cf8cd7975. Part of this code
duplicates that ...Is that bad?
If you are expressing a concern about who wrote this code, I took
Amit's word for it that he did.I'm just saying that the problem at hand is already solved for a related
feature, so ISTM this new code should use the recently added routine
rather than doing the same thing in a different way.
Oh, I see. Amit, thoughts?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, Jan 14, 2017 at 6:10 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Fri, Jan 13, 2017 at 3:09 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
I'm just saying that the problem at hand is already solved for a related
feature, so ISTM this new code should use the recently added routine
rather than doing the same thing in a different way.Oh, I see. Amit, thoughts?
Hm, perhaps. The code in map_partition_varattnos() that creates the
map could be replaced by a call to the new
convert_tuples_by_name_map(). In fact, it could even have used the
old version of it (convert_tuples_by_name()). I guess I just aped
what other callers of map_variable_attnos() were doing, which is to
generate the map themselves (not that they ought to be changed to use
convert_tuples_by_name_map).
I will send a patch at my earliest convenience. Thanks to Alvaro for
pointing that out.
Thanks,
Amit
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2017/01/14 13:36, Amit Langote wrote:
On Sat, Jan 14, 2017 at 6:10 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Fri, Jan 13, 2017 at 3:09 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
I'm just saying that the problem at hand is already solved for a related
feature, so ISTM this new code should use the recently added routine
rather than doing the same thing in a different way.Oh, I see. Amit, thoughts?
Hm, perhaps. The code in map_partition_varattnos() that creates the
map could be replaced by a call to the new
convert_tuples_by_name_map(). In fact, it could even have used the
old version of it (convert_tuples_by_name()). I guess I just aped
what other callers of map_variable_attnos() were doing, which is to
generate the map themselves (not that they ought to be changed to use
convert_tuples_by_name_map).I will send a patch at my earliest convenience. Thanks to Alvaro for
pointing that out.
And here is the patch.
Thanks,
Amit