pg.ddx.io pgsql-committers@postgresql.org mailing list archivehelp / color / mirror / Atom feed
pgsql: Fix possible crash in pg_dump with identity sequences. 6+ messages / 1 participants [nested] [flat]
* pgsql: Fix possible crash in pg_dump with identity sequences. @ 2024-12-13 19:21 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2024-12-13 19:21 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix possible crash in pg_dump with identity sequences. If an owned sequence is considered interesting, force its owning table to be marked interesting too. This ensures, in particular, that we'll fetch the owning table's column names so we have the data needed for ALTER TABLE ... ADD GENERATED. Previously there were edge cases where pg_dump could get SIGSEGV due to not having filled in the column names. (The known case is where the owning table has been made part of an extension while its identity sequence is not a member; but there may be others.) Also, if it's an identity sequence, force its dumped-components mask to exactly match the owning table: dump definition only if we're dumping the table's definition, dump data only if we're dumping the table's data, etc. This generalizes the code introduced in commit b965f2617 that set the sequence's dump mask to NONE if the owning table's mask is NONE. That's insufficient to prevent failures, because for example the table's mask might only request dumping ACLs, which would lead us to still emit ALTER TABLE ADD GENERATED even though we didn't create the table. It seems better to treat an identity sequence as though it were an inseparable aspect of the table, matching the treatment used in the backend's dependency logic. Perhaps this policy needs additional refinement, but let's wait to see some field use-cases before changing it further. While here, add a comment in pg_dump.h warning against writing tests like "if (dobj->dump == DUMP_COMPONENT_NONE)", which was a bug in this case. There is one other example in getPublicationNamespaces, which if it's not a bug is at least remarkably unclear and under-documented. Changing that requires a separate discussion, however. Per report from Artur Zakirov. Back-patch to all supported branches. Discussion: https://postgr.es/m/CAKNkYnwXFBf136=u9UqUxFUVagevLQJ=zGd5BsLhCsatDvQsKQ@mail.gmail.com Branch ------ REL_15_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/6978129b4e95aa9d75171ede5ff22f6cf8612055 Modified Files -------------- src/bin/pg_dump/pg_dump.c | 32 +++++++++++++++++--------------- src/bin/pg_dump/pg_dump.h | 12 ++++++++++-- 2 files changed, 27 insertions(+), 17 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix possible crash in pg_dump with identity sequences. @ 2024-12-13 19:21 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2024-12-13 19:21 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix possible crash in pg_dump with identity sequences. If an owned sequence is considered interesting, force its owning table to be marked interesting too. This ensures, in particular, that we'll fetch the owning table's column names so we have the data needed for ALTER TABLE ... ADD GENERATED. Previously there were edge cases where pg_dump could get SIGSEGV due to not having filled in the column names. (The known case is where the owning table has been made part of an extension while its identity sequence is not a member; but there may be others.) Also, if it's an identity sequence, force its dumped-components mask to exactly match the owning table: dump definition only if we're dumping the table's definition, dump data only if we're dumping the table's data, etc. This generalizes the code introduced in commit b965f2617 that set the sequence's dump mask to NONE if the owning table's mask is NONE. That's insufficient to prevent failures, because for example the table's mask might only request dumping ACLs, which would lead us to still emit ALTER TABLE ADD GENERATED even though we didn't create the table. It seems better to treat an identity sequence as though it were an inseparable aspect of the table, matching the treatment used in the backend's dependency logic. Perhaps this policy needs additional refinement, but let's wait to see some field use-cases before changing it further. While here, add a comment in pg_dump.h warning against writing tests like "if (dobj->dump == DUMP_COMPONENT_NONE)", which was a bug in this case. There is one other example in getPublicationNamespaces, which if it's not a bug is at least remarkably unclear and under-documented. Changing that requires a separate discussion, however. Per report from Artur Zakirov. Back-patch to all supported branches. Discussion: https://postgr.es/m/CAKNkYnwXFBf136=u9UqUxFUVagevLQJ=zGd5BsLhCsatDvQsKQ@mail.gmail.com Branch ------ REL_13_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/b6df2d6e5d83e6204bcb74977c12d4ccbf18e8cd Modified Files -------------- src/bin/pg_dump/pg_dump.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix possible crash in pg_dump with identity sequences. @ 2024-12-13 19:21 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2024-12-13 19:21 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix possible crash in pg_dump with identity sequences. If an owned sequence is considered interesting, force its owning table to be marked interesting too. This ensures, in particular, that we'll fetch the owning table's column names so we have the data needed for ALTER TABLE ... ADD GENERATED. Previously there were edge cases where pg_dump could get SIGSEGV due to not having filled in the column names. (The known case is where the owning table has been made part of an extension while its identity sequence is not a member; but there may be others.) Also, if it's an identity sequence, force its dumped-components mask to exactly match the owning table: dump definition only if we're dumping the table's definition, dump data only if we're dumping the table's data, etc. This generalizes the code introduced in commit b965f2617 that set the sequence's dump mask to NONE if the owning table's mask is NONE. That's insufficient to prevent failures, because for example the table's mask might only request dumping ACLs, which would lead us to still emit ALTER TABLE ADD GENERATED even though we didn't create the table. It seems better to treat an identity sequence as though it were an inseparable aspect of the table, matching the treatment used in the backend's dependency logic. Perhaps this policy needs additional refinement, but let's wait to see some field use-cases before changing it further. While here, add a comment in pg_dump.h warning against writing tests like "if (dobj->dump == DUMP_COMPONENT_NONE)", which was a bug in this case. There is one other example in getPublicationNamespaces, which if it's not a bug is at least remarkably unclear and under-documented. Changing that requires a separate discussion, however. Per report from Artur Zakirov. Back-patch to all supported branches. Discussion: https://postgr.es/m/CAKNkYnwXFBf136=u9UqUxFUVagevLQJ=zGd5BsLhCsatDvQsKQ@mail.gmail.com Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/7b8cb9cd6a76b1b6a77032a1c352b258b6abfd39 Modified Files -------------- src/bin/pg_dump/pg_dump.c | 32 +++++++++++++++++--------------- src/bin/pg_dump/pg_dump.h | 12 ++++++++++-- 2 files changed, 27 insertions(+), 17 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix possible crash in pg_dump with identity sequences. @ 2024-12-13 19:21 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2024-12-13 19:21 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix possible crash in pg_dump with identity sequences. If an owned sequence is considered interesting, force its owning table to be marked interesting too. This ensures, in particular, that we'll fetch the owning table's column names so we have the data needed for ALTER TABLE ... ADD GENERATED. Previously there were edge cases where pg_dump could get SIGSEGV due to not having filled in the column names. (The known case is where the owning table has been made part of an extension while its identity sequence is not a member; but there may be others.) Also, if it's an identity sequence, force its dumped-components mask to exactly match the owning table: dump definition only if we're dumping the table's definition, dump data only if we're dumping the table's data, etc. This generalizes the code introduced in commit b965f2617 that set the sequence's dump mask to NONE if the owning table's mask is NONE. That's insufficient to prevent failures, because for example the table's mask might only request dumping ACLs, which would lead us to still emit ALTER TABLE ADD GENERATED even though we didn't create the table. It seems better to treat an identity sequence as though it were an inseparable aspect of the table, matching the treatment used in the backend's dependency logic. Perhaps this policy needs additional refinement, but let's wait to see some field use-cases before changing it further. While here, add a comment in pg_dump.h warning against writing tests like "if (dobj->dump == DUMP_COMPONENT_NONE)", which was a bug in this case. There is one other example in getPublicationNamespaces, which if it's not a bug is at least remarkably unclear and under-documented. Changing that requires a separate discussion, however. Per report from Artur Zakirov. Back-patch to all supported branches. Discussion: https://postgr.es/m/CAKNkYnwXFBf136=u9UqUxFUVagevLQJ=zGd5BsLhCsatDvQsKQ@mail.gmail.com Branch ------ REL_16_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/782cc1aa3d158906cdfb9ca809c9ca4241a33041 Modified Files -------------- src/bin/pg_dump/pg_dump.c | 32 +++++++++++++++++--------------- src/bin/pg_dump/pg_dump.h | 12 ++++++++++-- 2 files changed, 27 insertions(+), 17 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix possible crash in pg_dump with identity sequences. @ 2024-12-13 19:21 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2024-12-13 19:21 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix possible crash in pg_dump with identity sequences. If an owned sequence is considered interesting, force its owning table to be marked interesting too. This ensures, in particular, that we'll fetch the owning table's column names so we have the data needed for ALTER TABLE ... ADD GENERATED. Previously there were edge cases where pg_dump could get SIGSEGV due to not having filled in the column names. (The known case is where the owning table has been made part of an extension while its identity sequence is not a member; but there may be others.) Also, if it's an identity sequence, force its dumped-components mask to exactly match the owning table: dump definition only if we're dumping the table's definition, dump data only if we're dumping the table's data, etc. This generalizes the code introduced in commit b965f2617 that set the sequence's dump mask to NONE if the owning table's mask is NONE. That's insufficient to prevent failures, because for example the table's mask might only request dumping ACLs, which would lead us to still emit ALTER TABLE ADD GENERATED even though we didn't create the table. It seems better to treat an identity sequence as though it were an inseparable aspect of the table, matching the treatment used in the backend's dependency logic. Perhaps this policy needs additional refinement, but let's wait to see some field use-cases before changing it further. While here, add a comment in pg_dump.h warning against writing tests like "if (dobj->dump == DUMP_COMPONENT_NONE)", which was a bug in this case. There is one other example in getPublicationNamespaces, which if it's not a bug is at least remarkably unclear and under-documented. Changing that requires a separate discussion, however. Per report from Artur Zakirov. Back-patch to all supported branches. Discussion: https://postgr.es/m/CAKNkYnwXFBf136=u9UqUxFUVagevLQJ=zGd5BsLhCsatDvQsKQ@mail.gmail.com Branch ------ REL_17_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/ad950ea98e54ab0b00305a3c4e7c5cec4996460e Modified Files -------------- src/bin/pg_dump/pg_dump.c | 32 +++++++++++++++++--------------- src/bin/pg_dump/pg_dump.h | 12 ++++++++++-- 2 files changed, 27 insertions(+), 17 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix possible crash in pg_dump with identity sequences. @ 2024-12-13 19:21 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2024-12-13 19:21 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix possible crash in pg_dump with identity sequences. If an owned sequence is considered interesting, force its owning table to be marked interesting too. This ensures, in particular, that we'll fetch the owning table's column names so we have the data needed for ALTER TABLE ... ADD GENERATED. Previously there were edge cases where pg_dump could get SIGSEGV due to not having filled in the column names. (The known case is where the owning table has been made part of an extension while its identity sequence is not a member; but there may be others.) Also, if it's an identity sequence, force its dumped-components mask to exactly match the owning table: dump definition only if we're dumping the table's definition, dump data only if we're dumping the table's data, etc. This generalizes the code introduced in commit b965f2617 that set the sequence's dump mask to NONE if the owning table's mask is NONE. That's insufficient to prevent failures, because for example the table's mask might only request dumping ACLs, which would lead us to still emit ALTER TABLE ADD GENERATED even though we didn't create the table. It seems better to treat an identity sequence as though it were an inseparable aspect of the table, matching the treatment used in the backend's dependency logic. Perhaps this policy needs additional refinement, but let's wait to see some field use-cases before changing it further. While here, add a comment in pg_dump.h warning against writing tests like "if (dobj->dump == DUMP_COMPONENT_NONE)", which was a bug in this case. There is one other example in getPublicationNamespaces, which if it's not a bug is at least remarkably unclear and under-documented. Changing that requires a separate discussion, however. Per report from Artur Zakirov. Back-patch to all supported branches. Discussion: https://postgr.es/m/CAKNkYnwXFBf136=u9UqUxFUVagevLQJ=zGd5BsLhCsatDvQsKQ@mail.gmail.com Branch ------ REL_14_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/c7f3c414fdedbb9a48d597a309c53fdf1ac3652a Modified Files -------------- src/bin/pg_dump/pg_dump.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2024-12-13 19:21 UTC | newest] Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2024-12-13 19:21 pgsql: Fix possible crash in pg_dump with identity sequences. Tom Lane <tgl@sss.pgh.pa.us> 2024-12-13 19:21 pgsql: Fix possible crash in pg_dump with identity sequences. Tom Lane <tgl@sss.pgh.pa.us> 2024-12-13 19:21 pgsql: Fix possible crash in pg_dump with identity sequences. Tom Lane <tgl@sss.pgh.pa.us> 2024-12-13 19:21 pgsql: Fix possible crash in pg_dump with identity sequences. Tom Lane <tgl@sss.pgh.pa.us> 2024-12-13 19:21 pgsql: Fix possible crash in pg_dump with identity sequences. Tom Lane <tgl@sss.pgh.pa.us> 2024-12-13 19:21 pgsql: Fix possible crash in pg_dump with identity sequences. 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