aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
...
* SQL/JSON: Fix JSON_TABLE() column deparsingAmit Langote2024-09-06
| | | | | | | | | | | | | | | | The deparsing code in get_json_expr_options() unnecessarily emitted the default column-specific ON ERROR / EMPTY behavior when the top-level ON ERROR behavior in JSON_TABLE was set to ERROR. Fix that by not overriding the column-specific default, determined based on the column's JsonExprOp in get_json_table_columns(), with JSON_BEHAVIOR_ERROR when that is the top-level ON ERROR behavior. Note that this only removes redundancy; the current deparsing output is not incorrect, just redundant. Reviewed-by: Jian He <jian.universality@gmail.com> Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com Backpatch-through: 17
* Be more careful with error paths in pg_set_regex_collation().Jeff Davis2024-09-05
| | | | | | | | | | | | | | Set global variables after error paths so that they don't end up in an inconsistent state. The inconsistent state doesn't lead to an actual problem, because after an error, pg_set_regex_collation() will be called again before the globals are accessed. Change extracted from patch by Andreas Karlsson, though not discussed explicitly. Discussion: https://postgr.es/m/60929555-4709-40a7-b136-bcb44cff5a3c@proxel.se
* Prevent mis-encoding of "trailing junk after numeric literal" errors.Tom Lane2024-09-05
| | | | | | | | | | | | | | | | | | | | | | | | | | Since commit 2549f0661, we reject an identifier immediately following a numeric literal (without separating whitespace), because that risks ambiguity with hex/octal/binary integers. However, that patch used token patterns like "{integer}{ident_start}", which is problematic because {ident_start} matches only a single byte. If the first character after the integer is a multibyte character, this ends up with flex reporting an error message that includes a partial multibyte character. That can cause assorted bad-encoding problems downstream, both in the report to the client and in the postmaster log file. To fix, use {identifier} not {ident_start} in the "junk" token patterns, so that they will match complete multibyte characters. This seems generally better user experience quite aside from the encoding problem: for "123abc" the error message will now say that the error appeared at or near "123abc" instead of "123a". While at it, add some commentary about why these patterns exist and how they work. Report and patch by Karina Litskevich; review by Pavel Borisov. Back-patch to v15 where the problem came in. Discussion: https://postgr.es/m/CACiT8iZ_diop=0zJ7zuY3BXegJpkKK1Av-PU7xh0EDYHsa5+=g@mail.gmail.com
* Fix misleading error message contextPeter Eisentraut2024-09-05
| | | | | | Author: Pavel Stehule <pavel.stehule@gmail.com> Reviewed-by: Stepan Neretin <sncfmgg@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/CAFj8pRAw+OkVW=FgMKHKyvY3CgtWy3cWdY7XT+S5TJaTttu=oA@mail.gmail.com
* Add callback for backend initialization in pgstatsMichael Paquier2024-09-05
| | | | | | | | | | pgstat_initialize() is currently used by the WAL stats as a code path to take some custom actions when a backend starts. A callback is added to generalize the concept so as all stats kinds can do the same, for builtin and custom kinds, if set. Reviewed-by: Bertrand Drouvot, Kyotaro Horiguchi Discussion: https://postgr.es/m/ZtZr1K4PLdeWclXY@paquier.xyz
* Fix two NULL pointer dereferences when reading custom pgstats from fileMichael Paquier2024-09-05
| | | | | | | | | | | | | | | | | | | There were two spots in pgstat_read_statsfile() where is was possible to finish with a null-pointer-dereference crash for custom pgstats kinds: - When reading stats for a fixed-numbered stats entry. - When reading a variable stats entry with name serialization. For both cases, these issues were reachable by starting a server after changing shared_preload_libraries so as the stats written previously could not be loaded. The code is changed so as the stats are ignored in this case, like the other code paths doing similar sanity checks. Two WARNINGs are added to be able to debug these issues. A test is added for the case of fixed-numbered stats with the module injection_points. Oversights in 7949d9594582, spotted while looking at a different report. Discussion: https://postgr.es/m/Ztj0Jftsn4xXuXtl@paquier.xyz
* Optimize WindowAgg's use of tuplestoresDavid Rowley2024-09-05
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When WindowAgg finished one partition of a PARTITION BY, it previously would call tuplestore_end() to purge all the stored tuples before again calling tuplestore_begin_heap() and carefully setting up all of the tuplestore read pointers exactly as required for the given frameOptions. Since the frameOptions don't change between partitions, this part does not make much sense. For queries that had very few rows per partition, the overhead of this was very large. It seems much better to create the tuplestore and the read pointers once and simply call tuplestore_clear() at the end of each partition. tuplestore_clear() moves all of the read pointers back to the start position and deletes all the previously stored tuples. A simple test query with 1 million partitions and 1 tuple per partition has been shown to run around 40% faster than without this change. The additional effort seems to have mostly been spent in malloc/free. Making this work required adding a new bool field to WindowAggState which had the unfortunate effect of being the 9th bool field in a group resulting in the struct being enlarged. Here we shuffle the fields around a little so that the two bool fields for runcondition relating stuff fit into existing padding. Also, move the "runcondition" field to be near those. This frees up enough space with the other bool fields so that the newly added one fits into the padding bytes. This was done to address a very small but apparent performance regression with queries containing a large number of rows per partition. Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> Reviewed-by: Tatsuo Ishii <ishii@postgresql.org> Discussion: https://postgr.es/m/CAHoyFK9n-QCXKTUWT_xxtXninSMEv%2BgbJN66-y6prM3f4WkEHw%40mail.gmail.com
* Speedup WindowAgg code by moving uncommon code out-of-lineDavid Rowley2024-09-05
| | | | | | | | | | The code to calculate the frame offsets is only performed once per scan. Moving this code out of line gives a small (around 4-5%) speedup when testing with some CPUs. Other tested CPUs are indifferent to the change. Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> Reviewed-by: Tatsuo Ishii <ishii@postgresql.org> Discussion: https://postgr.es/m/CAApHDvqPgFtwme2Zyf75BpMLwYr2mnUstDyPiP%3DEpudYuQTPPQ%40mail.gmail.com
* Remove lc_collate_is_c().Jeff Davis2024-09-04
| | | | | | | Instead just look up the collation and check collate_is_c field. Author: Andreas Karlsson Discussion: https://postgr.es/m/60929555-4709-40a7-b136-bcb44cff5a3c@proxel.se
* Remove a couple of strerror() callsPeter Eisentraut2024-09-04
| | | | | | | | | | | | Change to using %m in the error message string. We need to be a bit careful here to preserve errno until we need to print it. This change avoids the use of not-thread-safe strerror() and unifies some error message strings, and maybe makes the code appear more consistent. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/daa87d79-c044-46c4-8458-8d77241ed7b0%40eisentraut.org
* Unify some error messages to ease work of translatorsMichael Paquier2024-09-04
| | | | | | | | | This commit updates a couple of error messages around control file data, GUCs and server settings, unifying to the same message where possible. This reduces the translation burden a bit. Author: Peter Smith Discussion: https://postgr.es/m/CAHut+Pv-kSN8SkxSdoHano_wPubqcg5789ejhCDZAcLFceBR-w@mail.gmail.com
* Apply more quoting to GUC names in messagesMichael Paquier2024-09-04
| | | | | | | | | This is a continuation of 17974ec25946. More quotes are applied to GUC names in error messages and hints, taking care of what seems to be all the remaining holes currently in the tree for the GUCs. Author: Peter Smith Discussion: https://postgr.es/m/CAHut+Pv-kSN8SkxSdoHano_wPubqcg5789ejhCDZAcLFceBR-w@mail.gmail.com
* Collect statistics about conflicts in logical replication.Amit Kapila2024-09-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit adds columns in view pg_stat_subscription_stats to show the number of times a particular conflict type has occurred during the application of logical replication changes. The following columns are added: confl_insert_exists: Number of times a row insertion violated a NOT DEFERRABLE unique constraint. confl_update_origin_differs: Number of times an update was performed on a row that was previously modified by another origin. confl_update_exists: Number of times that the updated value of a row violates a NOT DEFERRABLE unique constraint. confl_update_missing: Number of times that the tuple to be updated is missing. confl_delete_origin_differs: Number of times a delete was performed on a row that was previously modified by another origin. confl_delete_missing: Number of times that the tuple to be deleted is missing. The update_origin_differs and delete_origin_differs conflicts can be detected only when track_commit_timestamp is enabled. Author: Hou Zhijie Reviewed-by: Shveta Malik, Peter Smith, Anit Kapila Discussion: https://postgr.es/m/OS0PR01MB57160A07BD575773045FC214948F2@OS0PR01MB5716.jpnprd01.prod.outlook.com
* Avoid unnecessary post-sort projectionRichard Guo2024-09-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When generating paths for the ORDER BY clause, one thing we need to ensure is that the output paths project the correct final_target. To achieve this, in create_ordered_paths, we compare the pathtarget of each generated path with the given 'target', and add a post-sort projection step if the two targets do not match. Currently we perform a simple pointer comparison between the two targets. It turns out that this is not sufficient. Each sorted_path generated in create_ordered_paths initially projects the correct target required by the preceding steps of sort. If it is the same pointer as sort_input_target, pointer comparison suffices, because sort_input_target is always identical to final_target when no post-sort projection is needed. However, sorted_path's initial pathtarget may not be the same pointer as sort_input_target, because in apply_scanjoin_target_to_paths, if the target to be applied has the same expressions as the existing reltarget, we only inject the sortgroupref info into the existing pathtargets, rather than create new projection paths. As a result, pointer comparison in create_ordered_paths is not reliable. Instead, we can compare PathTarget.exprs to determine whether a projection step is needed. If the expressions match, we can be confident that a post-sort projection is not required. It could be argued that this change adds extra check cost each time we decide whether a post-sort projection is needed. However, as explained in apply_scanjoin_target_to_paths, by avoiding the creation of projection paths, we save effort both immediately and at plan creation time. This, I think, justifies the extra check cost. There are two ensuing plan changes in the regression tests, but they look reasonable and are exactly what we are fixing here. So no additional test cases are added. No backpatch as this could result in plan changes. Author: Richard Guo Reviewed-by: Peter Eisentraut, David Rowley, Tom Lane Discussion: https://postgr.es/m/CAMbWs48TosSvmnz88663_2yg3hfeOFss-J2PtnENDH6J_rLnRQ@mail.gmail.com
* Check the validity of commutators for merge/hash clausesRichard Guo2024-09-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When creating merge or hash join plans in createplan.c, the merge or hash clauses may need to get commuted to ensure that the outer var is on the left and the inner var is on the right if they are not already in the expected form. This requires that their operators have commutators. Failing to find a commutator at this stage would result in 'ERROR: could not find commutator for operator xxx', with no opportunity to select an alternative plan. Typically, this is not an issue because mergejoinable or hashable operators are expected to always have valid commutators. But in some artificial cases this assumption may not hold true. Therefore, here in this patch we check the validity of commutators for clauses in the form "inner op outer" when selecting mergejoin/hash clauses, and consider a clause unusable for the current pair of outer and inner relations if it lacks a commutator. There are not (and should not be) any such operators built into Postgres that are mergejoinable or hashable but have no commutators; so we leverage the alias type 'int8alias1' created in equivclass.sql to build the test case. This is why the test case is included in equivclass.sql rather than in join.sql. Although this is arguably a bug fix, it cannot be reproduced without installing an incomplete opclass, which is unlikely to happen in practice, so no back-patch. Reported-by: Alexander Pyhalov Author: Richard Guo Reviewed-by: Tom Lane Discussion: https://postgr.es/m/c59ec04a2fef94d9ffc35a9b17dfc081@postgrespro.ru
* Fix inconsistent LWLock tranche name "CommitTsSLRU"Michael Paquier2024-09-04
| | | | | | | | | | | | | | | | This term was using an inconsistent casing between the code and the documentation, using "CommitTsSLRU" in wait_event_names.txt and "CommitTSSLRU" in the code. Let's update the term in the code to reflect what's in the documentation, "CommitTs" being more commonly used, so as pg_stat_activity shows the same term as the documentation. Oversight in 53c2a97a9266. Author: Alexander Lakhin Discussion: https://postgr.es/m/f7e514cf-2446-21f1-a5d2-8c089a6e2168@gmail.com Backpatch-through: 17
* Remember last collation to speed up collation cache.Jeff Davis2024-09-03
| | | | | | | | | This optimization is to avoid a performance regression in an upcoming patch that will remove lc_collate_is_c(). Discussion: https://postgr.es/m/96a559be83329bc66074a3925ebcfa8ceb16dfc5.camel@j-davis.com Discussion: https://postgr.es/m/646f662e145ab38cff1c04d475f4448f53fc5042.camel@j-davis.com Discussion: https://postgr.es/m/54565933-d82f-4d7c-8f47-288b1b570fd8@eisentraut.org
* Standardize "read-ahead advice" terminology.Thomas Munro2024-09-04
| | | | | | | | | Commit 6654bb920 added macOS's equivalent of POSIX_FADV_WILLNEED, and changed some explicit references to posix_fadvise to use this more general name for the concept. Update some remaining references. Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Discussion: https://postgr.es/m/0827edec-1317-4917-a186-035eb1e3241d%40eisentraut.org
* Add block_range_read_stream_cb(), to deduplicate code.Noah Misch2024-09-03
| | | | | | | | | This replaces two functions for iterating over all blocks in a range. A pending patch will use this instead of adding a third. Nazir Bilal Yavuz Discussion: https://postgr.es/m/20240820184742.f2.nmisch@google.com
* Fix typos in code comments and test dataDaniel Gustafsson2024-09-03
| | | | | | | | The typos in 005_negotiate_encryption.pl and pg_combinebackup.c shall be backported to v17 where they were introduced. Backpatch-through: v17 Discussion: https://postgr.es/m/Ztaj7BkN4658OMxF@paquier.xyz
* Add const qualifiers to XLogRegister*() functionsPeter Eisentraut2024-09-03
| | | | | | | | Add const qualifiers to XLogRegisterData() and XLogRegisterBufData(). Several unconstify() calls can be removed. Reviewed-by: Aleksander Alekseev <aleksander@timescale.com> Discussion: https://www.postgresql.org/message-id/dd889784-9ce7-436a-b4f1-52e4a5e577bd@eisentraut.org
* Fix typos and grammar in code comments and docsMichael Paquier2024-09-03
| | | | | Author: Alexander Lakhin Discussion: https://postgr.es/m/f7e514cf-2446-21f1-a5d2-8c089a6e2168@gmail.com
* Define PG_TBLSPC_DIR for path pg_tblspc/ in data folderMichael Paquier2024-09-03
| | | | | | | | | | | | | | Similarly to 2065ddf5e34c, this introduces a define for "pg_tblspc". This makes the style more consistent with the existing PG_STAT_TMP_DIR, for example. There is a difference with the other cases with the introduction of PG_TBLSPC_DIR_SLASH, required in two places for recovery and backups. Author: Bertrand Drouvot Reviewed-by: Ashutosh Bapat, Álvaro Herrera, Yugo Nagata, Michael Paquier Discussion: https://postgr.es/m/ZryVvjqS9SnV1GPP@ip-10-97-1-34.eu-west-3.compute.internal
* Remove support for OpenSSL older than 1.1.0Daniel Gustafsson2024-09-02
| | | | | | | | | | | | | | OpenSSL 1.0.2 has been EOL from the upstream OpenSSL project for some time, and is no longer the default OpenSSL version with any vendor which package PostgreSQL. By retiring support for OpenSSL 1.0.2 we can remove a lot of no longer required complexity for managing state within libcrypto which is now handled by OpenSSL. Reviewed-by: Jacob Champion <jacob.champion@enterprisedb.com> Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/ZG3JNursG69dz1lr@paquier.xyz Discussion: https://postgr.es/m/CA+hUKGKh7QrYzu=8yWEUJvXtMVm_CNWH1L_TLWCbZMwbi1XP2Q@mail.gmail.com
* More use of getpwuid_r() directlyPeter Eisentraut2024-09-02
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Remove src/port/user.c, call getpwuid_r() directly. This reduces some complexity and allows better control of the error behavior. For example, the old code would in some circumstances silently truncate the result string, or produce error message strings that the caller wouldn't use. src/port/user.c used to be called src/port/thread.c and contained various portability complications to support thread-safety. These are all obsolete, and all but the user-lookup functions have already been removed. This patch completes this by also removing the user-lookup functions. Also convert src/backend/libpq/auth.c to use getpwuid_r() for thread-safety. Originally, I tried to be overly correct by using sysconf(_SC_GETPW_R_SIZE_MAX) to get the buffer size for getpwuid_r(), but that doesn't work on FreeBSD. All the OS where I could find the source code internally use 1024 as the suggested buffer size, so I just ended up hardcoding that. The previous code used BUFSIZ, which is an unrelated constant from stdio.h, so its use seemed inappropriate. Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://www.postgresql.org/message-id/flat/5f293da9-ceb4-4937-8e52-82c25db8e4d3%40eisentraut.org
* Rename enum labels of PG_Locale_StrategyMichael Paquier2024-09-02
| | | | | | | | | | | | | | PG_REGEX_BUILTIN was added in f69319f2f1fb but it did not follow the same pattern as the previous labels, i.e. PG_LOCALE_*. In addition to this, the two libc strategies did not include in the name that they were related to this library. The enum labels are renamed as PG_STRATEGY_type[_subtype] to make the code clearer, in accordance to the library and the functions they rely on. Author: Andreas Karlsson Discussion: https://postgr.es/m/6f81200f-68fd-411e-97a1-d1f291d2e222@proxel.se
* Fix unfairness in all-cached parallel seq scan.Thomas Munro2024-08-31
| | | | | | | | | | | | | | | | | | | | | Commit b5a9b18c introduced block streaming infrastructure with a special fast path for all-cached scans, and commit b7b0f3f2 connected the infrastructure up to sequential scans. One of the fast path micro-optimizations had an unintended consequence: it interfered with parallel sequential scan's block range allocator (from commit 56788d21), which has its own ramp-up and ramp-down algorithm when handing out groups of pages to workers. A scan of an all-cached table could give extra blocks to one worker, when others had finished. In some plans (probably already very bad plans, such as the one reported by Alexander), the unfairness could be magnified. An internal buffer of 16 block numbers is removed, keeping just a single block buffer for technical reasons. Back-patch to 17. Reported-by: Alexander Lakhin <exclusion@gmail.com> Discussion: https://postgr.es/m/63a63690-dd92-c809-0b47-af05459e95d1%40gmail.com
* Clarify restrict_nonsystem_relation_kind description.Masahiko Sawada2024-08-30
| | | | | | | | | | | | This change improves the description of the restrict_nonsystem_relation_kind parameter in guc_table.c and the documentation for better clarity. Backpatch to 12, where this GUC parameter was introduced. Reviewed-by: Peter Eisentraut Discussion: https://postgr.es/m/6a96f1af-22b4-4a80-8161-1f26606b9ee2%40eisentraut.org Backpatch-through: 12
* Avoid inserting PlaceHolderVars in cases where pre-v16 PG did not.Tom Lane2024-08-30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 2489d76c4 removed some logic from pullup_replace_vars() that avoided wrapping a PlaceHolderVar around a pulled-up subquery output expression if the expression could be proven to go to NULL anyway (because it contained Vars or PHVs of the pulled-up relation and did not contain non-strict constructs). But removing that logic turns out to cause performance regressions in some cases, because the extra PHV blocks subexpression folding, and will do so even if outer-join reduction later turns it into a no-op with no phnullingrels bits. This can for example prevent an expression from being matched to an index. The reason for always adding a PHV was to ensure we had someplace to put the varnullingrels marker bits of the Var being replaced. However, it turns out we can optimize in exactly the same cases that the previous code did, because we can instead attach the needed varnullingrels bits to the contained Var(s)/PHV(s). This is not a complete solution --- it would be even better if we could remove PHVs after reducing them to no-ops. It doesn't look practical to back-patch such an improvement, but this change seems safe and at least gets rid of the performance-regression cases. Per complaint from Nikhil Raj. Back-patch to v16 where the problem appeared. Discussion: https://postgr.es/m/CAG1ps1xvnTZceKK24OUfMKLPvDP2vjT-d+F2AOCWbw_v3KeEgg@mail.gmail.com
* Define PG_LOGICAL_DIR for path pg_logical/ in data folderMichael Paquier2024-08-30
| | | | | | | | | | This is similar to 2065ddf5e34c, but this time for pg_logical/ itself and its contents, like the paths for snapshots, mappings or origin checkpoints. Author: Bertrand Drouvot Reviewed-by: Ashutosh Bapat, Yugo Nagata, Michael Paquier Discussion: https://postgr.es/m/ZryVvjqS9SnV1GPP@ip-10-97-1-34.eu-west-3.compute.internal
* Define PG_REPLSLOT_DIR for path pg_replslot/ in data folderMichael Paquier2024-08-30
| | | | | | | | | | | This commit replaces most of the hardcoded values of "pg_replslot" by a new PG_REPLSLOT_DIR #define. This makes the style more consistent with the existing PG_STAT_TMP_DIR, for example. More places will follow a similar change. Author: Bertrand Drouvot Reviewed-by: Ashutosh Bapat, Yugo Nagata, Michael Paquier Discussion: https://postgr.es/m/ZryVvjqS9SnV1GPP@ip-10-97-1-34.eu-west-3.compute.internal
* Rename pg_sequence_read_tuple() to pg_get_sequence_data()Michael Paquier2024-08-30
| | | | | | | | | | | | | | | | | | | This commit removes log_cnt from the tuple returned by the SQL function. This field is an internal counter that tracks when a WAL record should be generated for a sequence, and it is reset each time the sequence is restored or recovered. It is not necessary to rebuild the sequence DDL commands for pg_dump and pg_upgrade where this function is used. The field can still be queried with a scan of the "table" created under-the-hood for a sequence. Issue noticed while hacking on a feature that can rely on this new function rather than pg_sequence_last_value(), aimed at making sequence computation more easily pluggable. Bump catalog version. Reviewed-by: Nathan Bossart Discussion: https://postgr.es/m/Zsvka3r-y2ZoXAdH@paquier.xyz
* Fix mis-deparsing of ORDER BY lists when there is a name conflict.Tom Lane2024-08-29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Message style improvementsPeter Eisentraut2024-08-29
|
* Disallow USING clause when altering type of generated columnPeter Eisentraut2024-08-29
| | | | | | | | | | | | | This does not make sense. It would write the output of the USING clause into the converted column, which would violate the generation expression. This adds a check to error out if this is specified. There was a test for this, but that test errored out for a different reason, so it was not effective. Reported-by: Jian He <jian.universality@gmail.com> Reviewed-by: Yugo NAGATA <nagata@sraoss.co.jp> Discussion: https://www.postgresql.org/message-id/flat/c7083982-69f4-4b14-8315-f9ddb20b9834%40eisentraut.org
* Rename some shared memory initialization routinesHeikki Linnakangas2024-08-29
| | | | | | | | | | | | | | | | To make them follow the usual naming convention where FoobarShmemSize() calculates the amount of shared memory needed by Foobar subsystem, and FoobarShmemInit() performs the initialization. I didn't rename CreateLWLocks() and InitShmmeIndex(), because they are a little special. They need to be called before any of the other ShmemInit() functions, because they set up the shared memory bookkeeping itself. I also didn't rename InitProcGlobal(), because unlike other Shmeminit functions, it's not called by individual backends. Reviewed-by: Andreas Karlsson Discussion: https://www.postgresql.org/message-id/c09694ff-2453-47e5-b26c-32a16cd75ce6@iki.fi
* Refactor lock manager initialization to make it a bit less specialHeikki Linnakangas2024-08-29
| | | | | | | | | | Split the shared and local initialization to separate functions, and follow the common naming conventions. With this, we no longer create the LockMethodLocalHash hash table in the postmaster process, which was always pointless. Reviewed-by: Andreas Karlsson Discussion: https://www.postgresql.org/message-id/c09694ff-2453-47e5-b26c-32a16cd75ce6@iki.fi
* Refactor some code for ALTER TABLE SET LOGGED/UNLOGGED in tablecmds.cMichael Paquier2024-08-29
| | | | | | | | | | | | Both sub-commands use the same routine to switch the relpersistence of a relation, duplicated the same checks, and used a style inconsistent with access methods and tablespaces. SET LOGEED/UNLOGGED is refactored to avoid any duplication, setting the reason why a relation rewrite happens within ATPrepChangePersistence(). This shaves some code. Discussion: https://postgr.es/m/ZiiyGFTBNkqcMQi_@paquier.xyz
* Fixup for prefetching support on macOSPeter Eisentraut2024-08-29
| | | | | | | | The new code path (commit 6654bb92047) should call FileAccess() first, like the posix_fadvise() path. Reported-by: Thomas Munro <thomas.munro@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/0827edec-1317-4917-a186-035eb1e3241d%40eisentraut.org
* Rename the conflict types for the origin differ cases.Amit Kapila2024-08-29
| | | | | | | | | | | The conflict types 'update_differ' and 'delete_differ' indicate that a row to be modified was previously altered by another origin. Rename those to 'update_origin_differs' and 'delete_origin_differs' to clarify their meaning. Author: Hou Zhijie Reviewed-by: Shveta Malik, Peter Smith Discussion: https://postgr.es/m/CAA4eK1+HEKwG_UYt4Zvwh5o_HoCKCjEGesRjJX38xAH3OxuuYA@mail.gmail.com
* Add prefetching support on macOSPeter Eisentraut2024-08-28
| | | | | | | | | | | macOS doesn't have posix_fadvise(), but fcntl() with the F_RDADVISE command does the same thing. Some related documentation has been generalized to not mention posix_advise() specifically anymore. Reviewed-by: Thomas Munro <thomas.munro@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/0827edec-1317-4917-a186-035eb1e3241d%40eisentraut.org
* Fix misplaced translator commentsPeter Eisentraut2024-08-27
| | | | They did not immediately precede the code they were applying to.
* Fix identation.Masahiko Sawada2024-08-26
|
* Fix memory counter update in ReorderBuffer.Masahiko Sawada2024-08-26
| | | | | | | | | | | | | | | | | | Commit 5bec1d6bc5e changed the memory usage updates of the ReorderBufferTXN to zero all at once by subtracting txn->size, rather than updating it for each change. However, if TOAST reconstruction data remained in the transaction when freeing it, there were cases where it further subtracted the memory counter from zero, resulting in an assertion failure. This change calculates the memory size for each change and updates the memory usage to precisely the amount that has been freed. Backpatch to v17, where this was introducd. Reviewed-by: Amit Kapila, Shlok Kyal Discussion: https://postgr.es/m/CAD21AoAqkNUvicgKPT_dXzNoOwpPkVTg0QPPxEcWmzT0moCJ1g%40mail.gmail.com Backpatch-through: 17
* Fix nbtree lookahead overflow bug.Peter Geoghegan2024-08-26
| | | | | | | | | | | | | | Add bounds checking to nbtree's lookahead/skip-within-a-page mechanism. Otherwise it's possible for cases with lots of before-array-keys tuples to overflow an int16 variable, causing the mechanism to generate an out of bounds page offset number. Oversight in commit 5bf748b8, which enhanced nbtree ScalarArrayOp execution. Reported-By: Alexander Lakhin <exclusion@gmail.com> Discussion: https://postgr.es/m/6c68ac42-bbb5-8b24-103e-af0e279c536f@gmail.com Backpatch: 17-, where nbtree SAOP execution was enhanced.
* Fix compiler warning in mul_var_short().Dean Rasheed2024-08-26
| | | | | | | Some compilers (e.g., gcc before version 7) mistakenly think "carry" might be used uninitialized. Reported by Tom Lane, per various buildfarm members, e.g. arowana.
* Revert: Avoid looping over all type cache entries in TypeCacheRelCallback()Alexander Korotkov2024-08-26
| | | | | | | | This commit reverts c14d4acb8 as the patch design didn't take into account that TypeCacheEntry could be invalidated during the lookup_type_cache() call. Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/1927cba4-177e-5c23-cbcc-d444a850304f%40gmail.com
* Avoid looping over all type cache entries in TypeCacheRelCallback()Alexander Korotkov2024-08-25
| | | | | | | | | | | | | | | | | | | Currently when a single relcache entry gets invalidated, TypeCacheRelCallback() has to loop over all type cache entries to find appropriate typentry to invalidate. Unfortunately, using the syscache here is impossible, because this callback could be called outside a transaction and this makes impossible catalog lookups. This is why present commit introduces RelIdToTypeIdCacheHash to map relation OID to its composite type OID. We are keeping RelIdToTypeIdCacheHash entry while corresponding type cache entry have something to clean. Therefore, RelIdToTypeIdCacheHash shouldn't get bloat in the case of temporary tables flood. Discussion: https://postgr.es/m/5812a6e5-68ae-4d84-9d85-b443176966a1%40sigaev.ru Author: Teodor Sigaev Reviewed-by: Aleksander Alekseev, Tom Lane, Michael Paquier, Roman Zharkov Reviewed-by: Andrei Lepikhov, Pavel Borisov
* Revert support for ALTER TABLE ... MERGE/SPLIT PARTITION(S) commandsAlexander Korotkov2024-08-24
| | | | | | | | | | | | | | | | | This commit reverts 1adf16b8fb, 87c21bb941, and subsequent fixes and improvements including df64c81ca9, c99ef1811a, 9dfcac8e15, 885742b9f8, 842c9b2705, fcf80c5d5f, 96c7381c4c, f4fc7cb54b, 60ae37a8bc, 259c96fa8f, 449cdcd486, 3ca43dbbb6, 2a679ae94e, 3a82c689fd, fbd4321fd5, d53a4286d7, c086896625, 4e5d6c4091, 04158e7fa3. The reason for reverting is security issues related to repeatable name lookups (CVE-2014-0062). Even though 04158e7fa3 solved part of the problem, there are still remaining issues, which aren't feasible to even carefully analyze before the RC deadline. Reported-by: Noah Misch, Robert Haas Discussion: https://postgr.es/m/20240808171351.a9.nmisch%40google.com Backpatch-through: 17
* thread-safety: gmtime_r(), localtime_r()Peter Eisentraut2024-08-23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Use gmtime_r() and localtime_r() instead of gmtime() and localtime(), for thread-safety. There are a few affected calls in libpq and ecpg's libpgtypes, which are probably effectively bugs, because those libraries already claim to be thread-safe. There is one affected call in the backend. Most of the backend otherwise uses the custom functions pg_gmtime() and pg_localtime(), which are implemented differently. While we're here, change the call in the backend to gmtime*() instead of localtime*(), since for that use time zone behavior is irrelevant, and this side-steps any questions about when time zones are initialized by localtime_r() vs localtime(). Portability: gmtime_r() and localtime_r() are in POSIX but are not available on Windows. Windows has functions gmtime_s() and localtime_s() that can fulfill the same purpose, so we add some small wrappers around them. (Note that these *_s() functions are also different from the *_s() functions in the bounds-checking extension of C11. We are not using those here.) On MinGW, you can get the POSIX-style *_r() functions by defining _POSIX_C_SOURCE appropriately before including <time.h>. This leads to a conflict at least in plpython because apparently _POSIX_C_SOURCE gets defined in some header there, and then our replacement definitions conflict with the system definitions. To avoid that sort of thing, we now always define _POSIX_C_SOURCE on MinGW and use the POSIX-style functions here. Reviewed-by: Stepan Neretin <sncfmgg@gmail.com> Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Reviewed-by: Thomas Munro <thomas.munro@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/eba1dc75-298e-4c46-8869-48ba8aad7d70@eisentraut.org